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
