LRO num of frags limit

LRO num of frags limit

[Eli Cohen]

Hi,

looking at the LRO code, at __lro_proc_segment(), it seems that the
network driver can configure lro_mgr->max_aggr to any value it wants
while the number of fragments aggregated must not exceed MAX_SKB_FRAGS
(since we only use a single SKB to aggregate fragments, allocated by
lro_gen_skb()). Moreover, even if the driver does limit
lro_mgr->max_aggr to MAX_SKB_FRAGS, it might still cause overflow
since subsequent aggregations are done at lro_add_frags() which is
called before checking whether we overflow.
If the above observation is correct, I can send a patch.

Thanks,
Eli

Re: LRO num of frags limit

[Ben Hutchings]

On Tue, 2008-09-16 at 10:36 +0300, Eli Cohen wrote:
> Hi,
> 
> looking at the LRO code, at __lro_proc_segment(), it seems that the
> network driver can configure lro_mgr->max_aggr to any value it wants
> while the number of fragments aggregated must not exceed MAX_SKB_FRAGS

Correct.

> (since we only use a single SKB to aggregate fragments, allocated by
> lro_gen_skb()). Moreover, even if the driver does limit
> lro_mgr->max_aggr to MAX_SKB_FRAGS, it might still cause overflow
> since subsequent aggregations are done at lro_add_frags() which is
> called before checking whether we overflow.

So you must set max_aggr to
MAX_SKB_FRAGS - max number of frags added at once + 1.

> If the above observation is correct, I can send a patch.

I would be interested to see that, anyway.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: LRO num of frags limit

[Eli Cohen]

On Tue, Sep 16, 2008 at 11:40:17AM +0100, Ben Hutchings wrote:
> On Tue, 2008-09-16 at 10:36 +0300, Eli Cohen wrote:
> > Hi,
> > 
> > looking at the LRO code, at __lro_proc_segment(), it seems that the
> > network driver can configure lro_mgr->max_aggr to any value it wants
> > while the number of fragments aggregated must not exceed MAX_SKB_FRAGS
> 
> Correct.
> 
> > (since we only use a single SKB to aggregate fragments, allocated by
> > lro_gen_skb()). Moreover, even if the driver does limit
> > lro_mgr->max_aggr to MAX_SKB_FRAGS, it might still cause overflow
> > since subsequent aggregations are done at lro_add_frags() which is
> > called before checking whether we overflow.
> 
> So you must set max_aggr to
> MAX_SKB_FRAGS - max number of frags added at once + 1.
> 
> > If the above observation is correct, I can send a patch.
> 
> I would be interested to see that, anyway.
> 
By the way, we need to introduce two kinds of "max_aggr" fields to
struct net_lro_mgr. The reason is that when LRO is used in the mode in
which SKBs are linked, the above limitation does not exist. One will
be used for drivers which use lro_receive_skb() (which does not have a
limit) and one for drivers which use lro_receive_frags(). This will
require changing all the drivers that use LRO.