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

[Joel Granados]

On Sat, Aug 29, 2026 at 06:14:43PM +0200, Alexey Gladkov wrote:
> On Wed, Aug 26, 2026 at 01:29:00PM -0700, Linus Torvalds wrote:
> > 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.
> 
> You are right that my approach added too many macros and made the tables
> harder to read. Sorry.
> 
> > 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
> > 
> 
> I tried an alternative based on offsets. The following four patches add the
> infrastructure and convert IPC, SCTP, and MPLS as examples.

The last 4 patches landed intertwined with the initial 30+ that you sent. Will
you send a V2 with just the latest 4?

Best

Attachment: signature.asc
Description: PGP signature