Messages in this thread | | | From | Jamie Bainbridge <> | Date | Fri, 18 Nov 2022 11:45:42 +1000 | Subject | Re: [PATCH net-next] tcp: Fix tcp_syn_flood_action() if CONFIG_IPV6=n |
| |
On Thu, 17 Nov 2022 at 08:15, Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 17 Nov 2022 08:39:43 +1100 Jamie Bainbridge wrote: > > > if (v6) { > > > #ifdef v6 > > > expensive_call6(); > > > #endif > > > } else { > > > expensive_call6(); > > > } > > > > These should work, but I expect they cause a comparison which can't be > > optimised out at compile time. This is probably why the first style > > exists. > > > > In this SYN flood codepath optimisation doesn't matter because we're > > doing ratelimited logging anyway. But if we're breaking with existing > > style, then wouldn't the others also have to change to this style? I > > haven't reviewed all the other usage to tell if they're in an oft-used > > fastpath where such a thing might matter. > > I think the word style already implies subjectivity.
You are right. Looking further, there are many other ways IF_ENABLED(CONFIG_IPV6) is used, including similar to the ways you have suggested.
I don't mind Geert's original patch, but if you want a different style, I like your suggestion with v4 first:
if (v4) { expensive_call4(); #ifdef v6 } else { expensive_call6(); #endif }
Jamie
| |