Re: [PATCH net-next] tcp: Fix tcp_syn_flood_action() if CONFIG_IPV6=n

On Thu, 17 Nov 2022 at 08:15, Jakub Kicinski [off-list ref] wrote:

On Thu, 17 Nov 2022 08:39:43 +1100 Jamie Bainbridge wrote:

quoted

quoted

        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