| Message ID | cover.1652368648.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | UDP/IPv6 refactoring | expand |
Hello, On Fri, 2022-05-13 at 16:26 +0100, Pavel Begunkov wrote: > Refactor UDP/IPv6 and especially udpv6_sendmsg() paths. The end result looks > cleaner than it was before and the series also removes a bunch of instructions > and other overhead from the hot path positively affecting performance. > > Testing over dummy netdev with 16 byte packets yields 2240481 tx/s, > comparing to 2203417 tx/s previously, which is around +1.6% I personally feel that some patches in this series have a relevant chance of introducing functional regressions and e.g. syzbot will not help to catch them. That risk is IMHO relevant considered that the performance gain here looks quite limited. There are a few individual changes that IMHO looks like nice cleanup e.g. patch 5, 6, 8, 9 and possibly even patch 1. I suggest to reduce the patchset scope to them. Thanks! Paolo
On 5/16/22 7:48 AM, Paolo Abeni wrote: > Hello, > > On Fri, 2022-05-13 at 16:26 +0100, Pavel Begunkov wrote: >> Refactor UDP/IPv6 and especially udpv6_sendmsg() paths. The end result looks >> cleaner than it was before and the series also removes a bunch of instructions >> and other overhead from the hot path positively affecting performance. >> >> Testing over dummy netdev with 16 byte packets yields 2240481 tx/s, >> comparing to 2203417 tx/s previously, which is around +1.6% > > I personally feel that some patches in this series have a relevant > chance of introducing functional regressions and e.g. syzbot will not > help to catch them. That risk is IMHO relevant considered that the > performance gain here looks quite limited. > > There are a few individual changes that IMHO looks like nice cleanup > e.g. patch 5, 6, 8, 9 and possibly even patch 1. > > I suggest to reduce the patchset scope to them. > I agree with that sentiment. The set also needs testcases that captures the various permutations.
On 5/16/22 14:48, Paolo Abeni wrote: > Hello, > > On Fri, 2022-05-13 at 16:26 +0100, Pavel Begunkov wrote: >> Refactor UDP/IPv6 and especially udpv6_sendmsg() paths. The end result looks >> cleaner than it was before and the series also removes a bunch of instructions >> and other overhead from the hot path positively affecting performance. >> >> Testing over dummy netdev with 16 byte packets yields 2240481 tx/s, >> comparing to 2203417 tx/s previously, which is around +1.6% > > I personally feel that some patches in this series have a relevant > chance of introducing functional regressions and e.g. syzbot will not > help to catch them. That risk is IMHO relevant considered that the > performance gain here looks quite limited. I can't say I agree with that. First, I do think the code is much cleaner having just one block checking corking instead of a couple of random ifs in different places. Same for sin6. Not to mention negative line count. Also, assuming this 1.6% translates to ~0.5-1% with fast NICs, that's still huge, especially when we get >5GB/s in single core zc tests b/w servers. If maintainers are not merging it, I think I'll delay the series until I get another batch of planned optimisations implemented on top. > There are a few individual changes that IMHO looks like nice cleanup > e.g. patch 5, 6, 8, 9 and possibly even patch 1. > > I suggest to reduce the patchset scope to them.
Refactor UDP/IPv6 and especially udpv6_sendmsg() paths. The end result looks cleaner than it was before and the series also removes a bunch of instructions and other overhead from the hot path positively affecting performance. Testing over dummy netdev with 16 byte packets yields 2240481 tx/s, comparing to 2203417 tx/s previously, which is around +1.6% v2: no code changes, just resending properly v3: remove patch moving getfrag callback assignment add benchmark numbers Pavel Begunkov (10): ipv6: optimise ipcm6 cookie init udp/ipv6: move pending section of udpv6_sendmsg udp/ipv6: prioritise the ip6 path over ip4 checks udp/ipv6: optimise udpv6_sendmsg() daddr checks udp/ipv6: optimise out daddr reassignment udp/ipv6: clean up udpv6_sendmsg's saddr init ipv6: partially inline fl6_update_dst() ipv6: refactor opts push in __ip6_make_skb() ipv6: improve opt-less __ip6_make_skb() ipv6: clean up ip6_setup_cork include/net/ipv6.h | 24 +++---- net/ipv6/datagram.c | 4 +- net/ipv6/exthdrs.c | 15 ++--- net/ipv6/ip6_output.c | 53 +++++++-------- net/ipv6/raw.c | 8 +-- net/ipv6/udp.c | 153 ++++++++++++++++++++---------------------- net/l2tp/l2tp_ip6.c | 8 +-- 7 files changed, 120 insertions(+), 145 deletions(-)