Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Fri, 18 Nov 2022 09:29:13 +0100 | Subject | Re: [PATCH net-next] tcp: Fix tcp_syn_flood_action() if CONFIG_IPV6=n |
| |
Hi Jamie,
On Fri, Nov 18, 2022 at 2:50 AM Jamie Bainbridge <jamie.bainbridge@gmail.com> wrote: > 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 > }
IMHO this is worse, as the #ifdef/#endif is spread across the two branches of an if-conditional.
Hence this is usually written as:
if (cond1) { expensive_call1(); } #ifdef cond2_enabled else { expensive_call1(); } #endif
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |