[lwip-devel] Re: [patch #6253] Added csum to struct pbuf.

[Andrey Volkov]

Hello Jonathan,

Jonathan Larmour wrote:
> Follow-up Comment #3, patch #6253 (project lwip):
> 
> struct pbuf is very good right now for being exactly 32 bytes, which is very
> amenable to keeping pbuf buffer pools aligned on hardware that supports DMA -
> usually there are alignment constraints, particularly if caching is also
> involved, in which case you have to keep packet data and other data (including
> struct pbuf itself) in different cache lines.
But I don't change size of pbuf everywhere, they changed only if someone
defined CHECKSUM_HARDWARE_SUPPORT (and LWIP_PBUF_CB_SIZE, which i plane
to introduce in next patch).

> Now this isn't something which
> is very easy in lwIP at the moment, but only because it's been deliberately
> postponed until after 1.3. I think there's a task for it somewhere. But I have
> made my own modifications in my source base, and when the time comes after
> 1.3, I was going to raise the issue.
> 
> So increasing its size should be avoided if at all possible. Even an option
> isn't so good - some hardware has hardware-assisted checksum computation, and
> devices like that would probably support DMA too.
> 
> But I think there are further issues that make such a change difficult - this
> causes every pbuf that is to be sent to be increased too. And every pbuf in a
> pbuf chain, when there only need be one per whole packet.
> 
No, each packet should contain this field, since, if yore hwd. have
support of csum/DMA, then your PBUF_POOL_BUFSIZE will be equal to eth
frame size, isn't it?

> There are other possibilities, but the one that occurs to me most is to
> include the checksum in the pbuf contents somewhere, either the start or end.
> This fact could be indicated by a pbuf flag (or two, if you want to allow
> choice of start or end).
> 
If you add 'abstract' control block, you are lost alignment in worst
case too. So I don't see difference from my suggestion.

> There may well be other solutions, particularly if you consider the
> possibility of reducing u16_t ref to a u8_t.
> 
> 
>     _______________________________________________________
> 
> Reply to this item at:
> 
>   <http://savannah.nongnu.org/patch/?6253>
> 
> _______________________________________________
>   Message sent via/by Savannah
>   http://savannah.nongnu.org/
>