Re: [RFC PATCH 2/4] net: enable UDP gro on demand.

[Paolo Abeni <pabeni@redhat.com>]

On Sun, 2018-09-16 at 14:23 -0400, Willem de Bruijn wrote:
> That udp gro implementation is clearly less complete than yours in
> this patchset. The point I wanted to bring up for discussion is not the
> protocol implementation, but the infrastructure for enabling it
> conditionally.

I'm still [trying to] processing your patchset ;) So please perdon me
for any obvious interpretation mistakes...

> Assuming cycle cost is comparable, what do you think of  using the
> existing sk offload callbacks to enable this on a per-socket basis?

I have no objection about that, if there are no performance drawbacks.
In my measures retpoline costs is relevant for every indirect call
added. Using the existing sk offload approach will require an
additional indirect call per packet compared to the implementation
here.

> As for the protocol-wide knob, I do strongly prefer something that can
> work for all protocols, not just UDP. 

I like the general infrastructure idea. I think there is some agreement
in avoiding the addition of more user-controllable knobs, as we already
have a lot of them. If I read your patch correctly, user-space need to
enable/disable the UDO GSO explicitly via procfs, right?

I tried to look for something that does not require user action.

> I also implemented a version that
> atomically swaps the struct ptr instead of the flag based approach I sent
> for review. I'm fairly agnostic about that point. 

I think/fear security oriented guys may scream for the somewhat large
deconstification ?!?

> One subtle issue is that I
> believe we need to keep the gro_complete callbacks enabled, as gro
> packets may be queued for completion when gro_receive gets disabled.

Good point, thanks! I missed that.

Cheers,

Paolo