Re: [RFC PATCH v1 17/30] sysctl: net: use sysctl_field in SCTP sysctls

[Linus Torvalds]

I detest this series.

Maybe it makes technical sense, but as long as it makes no human
visual sense, I'm NAK'ing it as being garbage.

On Wed, 26 Aug 2026 at 12:43, Alexey Gladkov <legion@xxxxxxxxxx> wrote:
>
 [..]

The whole series is full of complete illegible noise like this:

> +static const struct sysctl_field sctp_net_table[] = {
> +       SYSCTL_FIELD_CUSTOM("rto_min", 0644, sizeof(unsigned int),
> +                        sctp_rto_min_data, proc_sctp_do_rto_min),
> +       SYSCTL_FIELD_CUSTOM("rto_max", 0644, sizeof(unsigned int),
> +                        sctp_rto_max_data, proc_sctp_do_rto_max),
> +       SYSCTL_FIELD_INT_MINMAX("pf_retrans", 0644, sctp_pf_retrans_data,
> +                            SYSCTL_ZERO, sctp_ps_retrans_data),
> +       SYSCTL_FIELD_INT_MINMAX("ps_retrans", 0644, sctp_ps_retrans_data,
> +                            sctp_pf_retrans_data, sctp_ps_retrans_max_data),
> +       SYSCTL_FIELD_STATIC_UINT_MINMAX("rto_initial", 0644,
> +                                    sctp_rto_initial_data,
> +                                    SYSCTL_UINT_ONE, &timer_max),
[...]

where apparently the indentation has been decided by a rodent on crack
who was given an Ouija board and instructed to ask his dead ancestors
what indentation to use.

So no.

That kind of complete random code is simply not acceptable.

I don't know what the correct answer is, but it is *not* this series.
It needs to be consistent and visually parseable by humans *without*
asking your dead ancestors for help.

And it's not just the indentation. That SYSCTL_FIELD_CUSTOM() thing
needs to be usable and able to do some minimal type checking - not
just passed a random sizeof() in a random argumentt. I'd suggest
passing the actual type, and then checking that the type *matches* the
data pointer it is passed too.

The old code may be ugly too, and have various other warts, but at
least it had somewhat legible and understandable initializers:

        {
                .procname       = "prsctp_enable",
                .data           = &init_net.sctp.prsctp_enable,
                .maxlen         = sizeof(int),
                .mode           = 0644,
                .proc_handler   = proc_dointvec,
        },

is at least something that can be read by a human and those things had
consistent whitespace rather than some quantum randomness.

And this is also just complete line noise that only makes the code worse:

    +#define SCTP_DATA(type, field)                                         \
    +static type *sctp_ ## field ## _data(const struct sysctl_context *ctx) \
    +{                                                                      \
    +       return &ctx->ns.net_ns->sctp.field;                             \
    +}
    +
    +#define SCTP_CUSTOM_DATA(field)
             \
    +static void *sctp_ ## field ## _data(const struct sysctl_context *ctx) \
    +{                                                                      \
    +       return &ctx->ns.net_ns->sctp.field;                             \
    +}
    +
    +SCTP_CUSTOM_DATA(rto_min)
    +SCTP_CUSTOM_DATA(rto_max)
    +SCTP_DATA(int, pf_retrans)
    +SCTP_DATA(int, ps_retrans)
    +SCTP_DATA(unsigned int, rto_initial)
    [...]

If we're doing these kinds of changes, the end result has to look
*BETTER* than the thing it replaces, not worse.

Yes, a few of the patches did look better. But the majority of them
only looked worse. Some of it should be easy to fix: use consistent
whitespace, and use sane argument ordering.

But honestly, the old setup didn't *rely* on argument ordering, and
used named initializers to make things more legible and robust.

So I suspect that should be what you should aim for in the new setup
too, and that probably means "completely different approach".

               Linus