[patch #6253] Added csum to struct pbuf.

[Arthur Marsh-2]

URL:
  <http://savannah.nongnu.org/patch/?6253>

                 Summary: Added csum to struct pbuf.
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: avolkov
            Submitted on: Среда 31.10.2007 at 17:41
                Category: None
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email:
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None

    _______________________________________________________

Details:

Hello!

This patch add csum field in struct pbuf. This field mainly will be speed up
inet csum calculation in in case when hardware support it (like Analog Devices
BF537).

--
Regards
Andrey Volkov




    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Среда 31.10.2007 at 17:41  Name: pbuf_csum.diff  Size: 3kB   By:
avolkov

<http://savannah.nongnu.org/patch/download.php?file_id=14264>

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #1, patch #6253 (project lwip):

I'm pretty sure that some developers will ask you to enable that with an
option in opt.h/lwipopts.h (if they hardware doesn't support it, it's not
necessary to add this code and to increase pbuf struct).



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Andrey Volkov]

Frédéric Bernon wrote:
> Follow-up Comment #1, patch #6253 (project lwip):
>
> I'm pretty sure that some developers will ask you to enable that with an
> option in opt.h/lwipopts.h (if they hardware doesn't support it, it's not
> necessary to add this code and to increase pbuf struct).
Ok,
Is HWD_SUPPORT_CSUM will be appropriate?

Andrey





_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #2, patch #6253 (project lwip):

>Is HWD_SUPPORT_CSUM will be appropriate?
>
>Andrey

CHECKSUM_HARDWARE_SUPPORT (to be in the same spirit than other CHECKSUM_xxx)
?

Do you have read "task #6849 : Test how checksum on copy could be integrated
into the stack" (https://savannah.nongnu.org/task/?6849) ?



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

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. 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.

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).

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/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #4, patch #6253 (project lwip):

> I'm pretty sure that some developers will ask you to enable
> that with an option

Of course!

> Do you have read "task #6849 : Test how checksum on copy could
> be integrated into the stack"

In fact I did some work on that task already and it also included adding
flags to the struct pbuf. Only in my case, this was needed because I was
adding one checksum per pbuf (when copying into the pbuf for sending). Of
course this must be an option and you trade speed for size!

Andrey's suggested patch is different, of course, since it would turn off
checksum generation for the TX side because those are automatically generated
by the HW, I suppose (my checksum-checking-HW discarded packets with wrong
checksums immediately, by the way, which seems to be even faster than Andrey's
patch).


> 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

That's true of course, but it's not covered today! We would need a second
memory alignment setting for DMA data. Then, when adding fields to the struct
pbuf, the struct would automatically be aligned for DMA needs.


To sum it up, for this patch, a 'checksum-per-packet'-field would be better
(especially when looking at the modification of inet_chksum_pseudo_partial(),
which I can't really read without comments. BTW: I wonder if the patch works
at all since this function is used for UDPLite only), while one flag per pbuf
would merge quite nice with my work on task #6849.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Andrey Volkov]

Frédéric Bernon wrote:
> Follow-up Comment #2, patch #6253 (project lwip):
>
>> Is HWD_SUPPORT_CSUM will be appropriate?
>>
>> Andrey
>
> CHECKSUM_HARDWARE_SUPPORT (to be in the same spirit than other
> CHECKSUM_xxx) ?
>
Ok, accepted.

> Do you have read "task #6849 : Test how checksum on copy could be
> integrated into the stack" (https://savannah.nongnu.org/task/?6849) ?
>
>
Yes, but I introduce this field mainly for next purposes:
 1) speed up reassembly of incoming ip packets. In reassembly case,
    ip payload csum of each fragment should be stored somewhere.
    (IMHO, pbuf is very appropriate for it).
 2) Hardware usually could calculate ip payload/headers checksum,
    but not tcp/udp pseudo checksum, hence ip csum should be stored.
    Also, I dislike to mix different levels of stack in one piece
    of code. I.e. if I've ip checksum in eth driver, I could undef
    CHECKSUM_CHECK_IP, but not CHECKSUM_CHECK_TCP/CHECKSUM_CHECK_UDP,
    since I don't know it in driver level.
And,
 3) In my hwd. driver, I don't 'memcpy' incoming packets, DMA do it
    directly to pbuf.

--
Regards
Andrey Volkov



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #5, patch #6253 (project lwip):

> include the checksum in the pbuf contents somewhere, either
> the start or end.

Good idea.  

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

When talking about bugs/tasks/patches that have an item in the bug
tracker, could you please answer using the bug-tracker at
https://savannah.nongnu.org/patch/?6253 so all thoughts and answers for
this item are easily found in one place.

Simon

Andrey Volkov schrieb:

> Frédéric Bernon wrote:
>  
>> Follow-up Comment #2, patch #6253 (project lwip):
>>
>>    
>>> Is HWD_SUPPORT_CSUM will be appropriate?
>>>
>>> Andrey
>>>      
>> CHECKSUM_HARDWARE_SUPPORT (to be in the same spirit than other
>> CHECKSUM_xxx) ?
>>
>>    
> Ok, accepted.
>
>  
>> Do you have read "task #6849 : Test how checksum on copy could be
>> integrated into the stack" (https://savannah.nongnu.org/task/?6849) ?
>>
>>
>>    
> Yes, but I introduce this field mainly for next purposes:
>  1) speed up reassembly of incoming ip packets. In reassembly case,
>     ip payload csum of each fragment should be stored somewhere.
>     (IMHO, pbuf is very appropriate for it).
>  2) Hardware usually could calculate ip payload/headers checksum,
>     but not tcp/udp pseudo checksum, hence ip csum should be stored.
>     Also, I dislike to mix different levels of stack in one piece
>     of code. I.e. if I've ip checksum in eth driver, I could undef
>     CHECKSUM_CHECK_IP, but not CHECKSUM_CHECK_TCP/CHECKSUM_CHECK_UDP,
>     since I don't know it in driver level.
> And,
>  3) In my hwd. driver, I don't 'memcpy' incoming packets, DMA do it
>     directly to pbuf.
>
> --
> Regards
> Andrey Volkov
>
>
>
> _______________________________________________
> lwip-devel mailing list
> lwip-devel@...
> http://lists.nongnu.org/mailman/listinfo/lwip-devel
>
>
>  

_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[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/
>

_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #6, patch #6253 (project lwip):

Andrey Volkov wrote:
> Yes, but I introduce this field mainly for next purposes:
>  1) speed up reassembly of incoming ip packets. In reassembly case,
>     ip payload csum of each fragment should be stored somewhere.
>     (IMHO, pbuf is very appropriate for it).

That might be solved best by a change of the IP implementation by not
checking reassembled packets again (we check a checksum we generate ourselves,
anyway...)

>  2) Hardware usually could calculate ip payload/headers checksum,
>     but not tcp/udp pseudo checksum, hence ip csum should be stored.
>     Also, I dislike to mix different levels of stack in one piece
>     of code. I.e. if I've ip checksum in eth driver, I could undef
>     CHECKSUM_CHECK_IP, but not CHECKSUM_CHECK_TCP/CHECKSUM_CHECK_UDP,
>     since I don't know it in driver level.

I don't think I really understand you here. Could you maybe describe your
changes to inet_chksum_pseudo_partial()? What I understand from reading the
code, you use p->csum for the last pbuf in a linked list only, so effectively
per packet, not per pbuf! (But then again, if your pbufs are big enough to
hold a full packet - no pbuf chains - then this is the same for you.)

> And,
>  3) In my hwd. driver, I don't 'memcpy' incoming packets, DMA do it
>     directly to pbuf.

Of course, that's because task #6849 wants to speed up systems that don't
have HW-checksum engines (by only loading the data once into the processor by
combining memcpy with checksum generation).

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Andrey Volkov]

goldsimon@... wrote:
> When talking about bugs/tasks/patches that have an item in the bug
> tracker, could you please answer using the bug-tracker at
> https://savannah.nongnu.org/patch/?6253 so all thoughts and answers for
> this item are easily found in one place.
>
> Simon
>
Oops, sorry, I'll be have it in my mind.

Andrey



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #7, patch #6253 (project lwip):

[ Moving mail from lwip-devel to the patch, to keep discussion in one place
]
Andrey Volkov wrote:

> Jonathan Larmour wrote:
>> 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 plan to introduce in next patch).

Indeed I'm aware - that's why I was pointing out the issues even when it's an
option.
 

>> 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?

No it isn't. Some MACs I have worked have either hard fixed buffer sizes of
low granularity (e.g. 128 bytes), some have soft configurable fixed sizes (so
you can make them chains of whatever size you like, and the hardware will only
consume as many as it needs), and a few have been fixed at the full ethernet
frame size. The last is of course very wasteful of memory.

If alignment is an issue, then that would be a time to append the checksum to
the end of the packet. Then alignment is unaffected.
 
>> 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.

pbufs already have flags. My proposal does not increase the struct pbuf
size.
 
Jifl


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #8, patch #6253 (project lwip):

>> 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
>
> That's true of course, but it's not covered today!

Yes, I know that it's been put off till after 1.3. That doesn't stop it being
relevant.

> We would need a second memory alignment setting for DMA data.
> Then, when adding fields to the struct pbuf, the struct would
> automatically be aligned for DMA needs.

To be clear, I already have working and deployed code for this. This is not
vapourware.

But if you increase a struct pbuf from 32 to 36 bytes, then those systems
with DMA and caches will have to align to the cache line, which is often 32
bytes, so you are now wasting 28 bytes per pbuf. Other systems place other
constraints on DMA alignment, even when cache line alignment is irrelevant,
for example on Coldfire.

Anyway, this problem is avoidable.



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #9, patch #6253 (project lwip):

> Yes, I know that it's been put off till after 1.3. That doesn't stop it
being relevant.

No, of course it's not!

> To be clear, I already have working and deployed code for this. This is not
vapourware.

I don't think I understand. What do you mean with 'this'? DMA alignment? I
wouldn't add fields to struct pbuf until we have code that takes care for DMA
alignment or else my code doesn't work any more, either!

> ... so you are now wasting 28 bytes per pbuf.

Of course, that's wasted memory. I'll take a look at the extra space for
checksum at the beginning (or end?) of a pbuf for checksum-on-copy, that
sounds quite nice to me!

Anyway, both checksum-on-copy AND the change this patch suggests should be
held back until after 1.3.0 release!

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #10, patch #6253 (project lwip):

Oh, I hate it when it turns out we've been fervently agreeing all along :-).

> I don't think I understand. What do you mean with 'this'? DMA
> alignment? I wouldn't add fields to struct pbuf until we have
> code that takes care for DMA alignment or else my code doesn't
> work any more, either!

Yes, I meant alignment for DMA/cache/whatever. My changes to the core code
were modest. The syntax of enforcing alignment is non-portable (unless you
start to let things get wasteful), so I thought the best thing would be to
allow the driver or port to override how the pbuf pools are declared. I then
also added a hook that could be called on freeing.

But the patch is not against CVS, so I'd need to port it to the current code
base, after updating my own port to CVS.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #11, patch #6253 (project lwip):

Simon Goldschmidt wrote:

> Follow-up Comment #6, patch #6253 (project lwip):
>
> Andrey Volkov wrote:
>> Yes, but I introduce this field mainly for next purposes:
>>  1) speed up reassembly of incoming ip packets. In reassembly case,
>>     ip payload csum of each fragment should be stored somewhere.
>>     (IMHO, pbuf is very appropriate for it).
>
> That might be solved best by a change of the IP implementation by not
> checking reassembled packets again (we check a checksum we generate

ourselves,

> anyway...)
>
>>  2) Hardware usually could calculate ip payload/headers checksum,
>>     but not tcp/udp pseudo checksum, hence ip csum should be stored.
>>     Also, I dislike to mix different levels of stack in one piece
>>     of code. I.e. if I've ip checksum in eth driver, I could undef
>>     CHECKSUM_CHECK_IP, but not CHECKSUM_CHECK_TCP/CHECKSUM_CHECK_UDP,
>>     since I don't know it in driver level.
>
> I don't think I really understand you here. Could you maybe describe your
> changes to inet_chksum_pseudo_partial()? What I understand from reading

the
> code, you use p->csum for the last pbuf in a linked list only, so
effectively
> per packet, not per pbuf! (But then again, if your pbufs are big enough to
> hold a full packet - no pbuf chains - then this is the same for you.)
Yes, it is misunderstandings from your side ;).

Well I'll try to describe how I currently use lwip:

1) BF537 have _chained_ DMA (i.e. could send/receive N chained pbufs)
   and could calculate ip payload/headers csum only for a single
   _incoming_ eth packet.
2) In my app. I need to transfer large udp packets (i.e. they fragmented
   almost always).
3) When I fill payload for outgoing udp d-gramms, I calculate csum for
   it _before_ I call udp_send (and do it fast in asm).
   So only what lwip is need to do, is calculate csum for udp/ip
   headers. I.e. on input of inet_chksum_pseudo in udp_send
   I've chain minimum from 2 pbufs:
   udp header (without valid csum) and ref to my large udp payload
  (with valid csum).
4) When stack receive fragmented udp, ip csum, calculated by hwd.,
   presented not in every ip fragment (mainly pbuf's csum must be
   recalculated if pbuf_realloc in ip_input really truncate pbuf).
   So, when inet_chksum_pseudo called from udp_input for reassembled
   udp, it have again pbuf's chain with partially calculated csum as
   input arg :(.

In both cases I need csum field in _every_ pbuf, and I'm ready to
sacrifice memory space for any other pbuf usage.

Now about inet_chksum_pseudo: it check flag in _every_ pbuf of chain,
and if it set then use value of csum field, if not then calculate it.
If, in simplest case, chain contain only one pbuf with valid csum, then
inet_chksum_pseudo should do nothing. Same for inet_chksum_pseudo_partial
(checksum_len, in general case, could be greater then size of one pbuf).

>
>> And,
>>  3) In my hwd. driver, I don't 'memcpy' incoming packets, DMA do it
>>     directly to pbuf.
>
> Of course, that's because task #6849 wants to speed up systems that don't
> have HW-checksum engines (by only loading the data once into the processor
by
> combining memcpy with checksum generation).



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #12, patch #6253 (project lwip):

Andrey, I know this comment is a little late, I'm only starting to work on
this again now:

Why do you "calculate csum for it before I call udp_send (and do it fast in
asm)" instead of providing this fast asm routing as LWIP_CHKSUM macro?

Anyway, from your description, it seems to me as one checksum per packet
could also be sufficient...

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Follow-up Comment #13, patch #6253 (project lwip):

Hi Simon

> Why do you "calculate csum for it before I call udp_send (and do
> it fast in asm)" instead of providing this fast asm routing as
> LWIP_CHKSUM macro?

Since, when my app. generate udp payload, it do it directly to preallocated
buffer, and do it in asm routine, i.e. payload csum calculation is part of
payload data generation process.

> Anyway, from your description, it seems to me as one checksum
> per packet could also be sufficient...

Only and only if packet == pbuf. But in my case, for any _outgoing_ udp
packet, I need 2 pbufs in chain - first for udp/ip/eth headers (and lwip will
calculate csum for it in udp_sendto_if), second - for payload (with
precalculated csum).

Also, for each _incoming_ ethernet packet, AD BF537 MAC calculate 2 csums -
for ip header and for ip payload. So, in this case, lwip need NOT to
recalculate checksums for incoming ip packets, if driver store they somewhere
(in pbufs).

--
Andrey

P.S. Oops, I'm sorry, that don't tell it before, but I use only low level
api, so udp_send called directly from my application.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Arthur Marsh-2]

Update of patch #6253 (project lwip):

                  Status:                    None => In Progress            
             Assigned to:                    None => goldsimon              

    _______________________________________________________

Follow-up Comment #14:

> Since, when my app. generate udp payload, it do it directly to
> preallocated buffer, and do it in asm routine, i.e. payload csum
> calculation is part of payload data generation process.

The standard lwIP procedure for this kind of MAC would be to check the 2
checksums for correctness in your driver (i.e. before passing them to the lwIP
input functions) and to discard packets with invalid checksums right away. You
can then turn off checksum checking on receive for protocols you know (this
would be tcp, udp and icmp presumably). While this involves a little bit of
'hacked' code (processing IP and transaport layer in MAC driver), it is more
performant and also what is done in more advanced MACs (discarding packets
with wrong checksums).

As to sending packets: From what you say, I think saving the checksum per
packet (not per pbuf) would still be OK: You would include the transport
checksum (data only, not header) when filling the pbuf, this transport
checksum could be recalculated when the transport header is added; the IP
checksum can't be calculated while copying as there is no copying involved
when filling the header (i.e. IP checksum will be generated as it is now). For
eth header, there is no checksum!

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6253>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel