Re: [PATCH v3] net:Add sysctl_max_skb_frags

[Hannes Frederic Sowa]

On 03.02.2016 12:25, Herbert Xu wrote:
> On Wed, Feb 03, 2016 at 09:26:57AM +0100, Hans Westgaard Ry wrote:
> > Devices may have limits on the number of fragments in an skb they support.
> > Current codebase uses a constant as maximum for number of fragments one
> > skb can hold and use.
> > When enabling scatter/gather and running traffic with many small messages
> > the codebase uses the maximum number of fragments and may thereby violate
> > the max for certain devices.
> > The patch introduces a global variable as max number of fragments.
> >
> > Signed-off-by: Hans Westgaard Ry <hans.westgaard.ry@xxxxxxxxxx>
> > Reviewed-by: HÃkon Bugge <haakon.bugge@xxxxxxxxxx>
>
> I have to say this seems rather dirty.  I mean if taken to the
> extreme wouldn't this mean that we should disable frags altogether
> if some NIC can't handle them at all?
>
> Someone suggested earlier to partially linearise the skb, why
> couldn't we do that? IOW let's handle this craziness in the crazy
> drivers and not in the general stack.

Agreed that it feels like a hack, but a rather simple one. I would
consider this to be just a performance improvement. We certainly need
a slow-path when virtio drivers submit gso packets to the stack (and
already discussed with Hans). The sysctl can't help here. But without
the sysctl the packets would constantly hit the slow-path in case of
e.g. IPoIB and that would also be rather bad.

Thanks,
Hannes