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

[Douglas]

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
>