Re: [lwip-devel] Q. PBUF_NEEDS_COPY relevant for input pbufs?

[Douglas]

Unfortunately it was not so simple. For example etharp_query() can queue
a buffer, and it increases the reference count to two, and when it
finally outputs this buffer a buffer with that reference count of two is
passed to the link output function. It is only along the return paths
(return back up the layers) that the callers can decrease their
reference counts, so I see how this design works well and what I failed
to understand was that a buffer can be deferred at multiple layers.

Douglas

On 08/05/2017 10:25 PM, Douglas wrote:
> Hi Simon,
> 
> Thank you for the explanations, and they helped orientate me getting up
> to speed on some of these code paths.
> 
> Fwiw I tried adding an extra argument to the link output function, which
> when true handed the pbuf to the link layer to free so the caller did
> not have to free it. Many of the callers were similarly modified back to
> points at which the caller was freeing the pbuf. Most of the callers
> follow the pattern of freeing the pbuf.
> 
> The main exception is in tcp_output_segment but exploring the code I see
> how that works, and that it checks that the pbuf ref is one before
> reusing it.
> 
> Even crossing the 'api' messages, this pattern seems to hold, but I did
> not bother coding that far and stopped at lwip_netconn_do_send.
> 
> Not sure if this helped make the code easier to work with. It did seem
> to better match the common pattern. Callers not handing the pbuf over to
> the link layer, and wanting to reuse it, do need to take some care that
> was not immediately obvious to me anyway.
> 
> Douglas
> 
> On 08/01/2017 04:07 AM, address@hidden wrote:
>> Douglas wrote:
>>> Looking at the TX side further, there is one common pattern in which the
>>> caller issues linkoutput() then pbuf_free(). It appears that with the
>>> esp8266 sdk only this pattern will work correctly.
>>
>> And this is how it should be like. Theoretically, the pbuf *could* be
>> reused if the ref
>> count is still 1 after the TX function returned as any delayed usage
>> should increase
>> the ref count. But with *not* reusing a pbuf, you're on the safe side.
>>
>>> Other code appears to assume that after linkoutput() returns that the
>>> caller continues to own the pbuf. For example lowpan6_frag() appears to
>>> allocate a pbuf and reuse the payload buffer after calling linkoutput().
>>
>> Unfortunately, this does not seem to be the only bug in the 6LowPAN code
>> :-(
>> It doesn't seem to be widely used, yet...
>>
>>> Being new to this code, the semantics of linkoutput() are not easy to
>>> understand.
>>>
>>> * Does the caller continue to own the pbuf after the call to
>>> linkoutput()?
>> That depends on the ref count. The caller still owns its reference (and
>> has to give it up
>> via pbuf_free), so if the ref count is 1 after the TX function returned,
>> it is still the single
>> and full ownder of the pbuf.
>>
>>> * Can linkoutput() modify the payload, and perhaps separately define if
>>> it can modify the link layer header area?
>>
>> Unless PBUF_TYPE_FLAG_DATA_VOLATILE is set, the linkoutput function can
>> modify
>> some parts the payload. For example, TCP retransmission takes extra care
>> that it is the
>> only owner of a pbuf and resets the pbufs payload to the TCP header on
>> retransmission.
>> As such the pbuf contents should remain the same from layer 4 up...
>>
>> However, linkoutput is free to add/modify link layer headers (and
>> possibly even layer 3)
>> on transmission, possibly by chaining a pbuf in front.
>>
>>> * Was the PBUF_TYPE_FLAG_DATA_VOLATILE flag intended to help optimise
>>> any of these patterns on the TX side?
>>
>> Yes. If your driver uses delayed TX (e.g. queued DMA transfers), you
>> cannot rely on payload
>> data that is marked as VOLATILE to be still valid later once the
>> transfer is done.
>>
>>> * If in general the caller does continue to own the pbuf, and
>>> linkoutput() is not to modify the payload, then might it worth exploring
>>> another linkoutput() variant that hands the pbuf to the link layer to
>>> optimize that common pattern?
>>
>> I don't understand that last part.
>>
>> Cheers,
>> Simon
>>
>> _______________________________________________
>> lwip-devel mailing list
>> address@hidden
>> https://lists.nongnu.org/mailman/listinfo/lwip-devel
>>
> 
> 
> _______________________________________________
> lwip-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/lwip-devel
>