RE: LRO ip_summed

2008-02-05 Thread Kostya B

1. Let's assume a driver which has a HW csum capability, however it has a 
privilege to assign any value for ip_summed.
For example the driver assigns unnecessary for most of the packets, however 
it fails to verify the packets less than 96 bytes (checksum_none).
Now, the LRO will aggregate all of them and assign the value used for all 
aggregated packets - that is wrong, 

Of course, it is possible to avoid that situation in driver, however [proposal] 
from the point of view of the *generic* LRO it could be better to check each 
newly aggregated skb whether it has the same ip_summed as the first one 
(preserve coherency). Thus, there is no need to keep ip_summed field in the  
struct net_lro_mgr.

2. Now, when a skb could not be aggregated (modes: aggregating SKBs) I think 
where's no need to override its original ip_summed (see out label of 
__lro_proc_skb()) The comment says  Original SKB has to be posted to stack. I 
would be wrong, but I don't get the reason to possible change of ip_summed.

Regards,
- Kostya


 From: [EMAIL PROTECTED]
 To: [EMAIL PROTECTED]
 Subject: Re: LRO ip_summed
 Date: Mon, 4 Feb 2008 10:14:23 +0100
 CC: netdev@vger.kernel.org
 
 On Sunday 03 February 2008 10:48, Kostya B wrote:
 
 Hi,
 
 The mail is related to the way LRO manipulates the ip_summed value. Could 
 anybody (author) explain why to overwrite the original value of 
 skb-ip_summed, when it's processing by __lro_proc_skb ?
 E.g. in out: label
 
 Why not to preserve the coherency of csum status of each incoming to LRO 
 packet, the same way the IP defragmentation does? That means - all skb in 
 descriptor have the same value of ip_summed.
 
 
 Hi,
 
 I'm not sure if I understand your proposal correctly.
 Currently the driver decides via the struct net_lro_mgr
 a) which ip_summed value will be used for all aggregated packets
 b) which one to use for those packets that can not be aggregated
 
 to a) this one should always be the same. The driver knows whether its HW is
   capable of performing ip checksum checking for this kind of traffic 
 (TCP)
 
 There are two modes: aggregating SKBs or aggregating fragments. 
 When fragments are aggregated, there is no SKB with a filled ip_summed 
 available.
 
 Please outline which parts of which mode you suggest to change.
 
 Regards,
 Jan-Bernd
 
 --
 To unsubscribe from this list: send the line unsubscribe netdev in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

_
Express yourself instantly with MSN Messenger! Download today it's FREE!
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: LRO ip_summed

2008-02-05 Thread Jan-Bernd Themann
On Tuesday 05 February 2008 09:56, Kostya B wrote:
 
 1. Let's assume a driver which has a HW csum capability, however it has a 
 privilege to assign any value for ip_summed.
 For example the driver assigns unnecessary for most of the packets, however 
 it fails to verify the packets less than 96 bytes (checksum_none).
 Now, the LRO will aggregate all of them and assign the value used for all 
 aggregated packets - that is wrong, 
 
 Of course, it is possible to avoid that situation in driver, however 
 [proposal] from the point of view of the *generic* LRO it could be better to 
 check each newly aggregated skb whether it has the same ip_summed as the 
 first one (preserve coherency). Thus, there is no need to keep ip_summed 
 field in the  struct net_lro_mgr.

Yes, checking if ip_summed is equal for all aggregated packets would work
for the aggregating SKB mode. For the other mode this information is 
currently 
not passed to LRO but could of course be provided as parameter by the driver.
The question is if there is any HW which would benefit from this (meaning 
multiple
ip_summed values for TCP packets of different sizes) so that this additional
check is really useful. The less required checks the better.

 
 2. Now, when a skb could not be aggregated (modes: aggregating SKBs) I think 
 where's no need to override its original ip_summed (see out label of 
 __lro_proc_skb()) The comment says  Original SKB has to be posted to stack. 
 I would be wrong, but I don't get the reason to possible change of ip_summed.
 

Good point. This assignment seems to be unnecessary.

Regards,
Jan-Bernd
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: LRO ip_summed

2008-02-04 Thread Jan-Bernd Themann
On Sunday 03 February 2008 10:48, Kostya B wrote:
 
 Hi,
 
 The mail is related to the way LRO manipulates the ip_summed value. Could 
 anybody (author) explain why to overwrite the original value of 
 skb-ip_summed, when it's processing by __lro_proc_skb ?
 E.g. in out: label
 
 Why not to preserve the coherency of csum status of each incoming to LRO 
 packet, the same way the IP defragmentation does? That means - all skb in 
 descriptor have the same value of ip_summed.
 

Hi,

I'm not sure if I understand your proposal correctly.
Currently the driver decides via the struct net_lro_mgr
a) which ip_summed value will be used for all aggregated packets
b) which one to use for those packets that can not be aggregated

to a) this one should always be the same. The driver knows whether its HW is
  capable of performing ip checksum checking for this kind of traffic (TCP)

There are two modes: aggregating SKBs or aggregating fragments. 
When fragments are aggregated, there is no SKB with a filled ip_summed 
available.

Please outline which parts of which mode you suggest to change.

Regards,
Jan-Bernd

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html