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

[Alexey Gladkov <legion@kernel.org>]

On Wed, Sep 16, 2026 at 02:26:01PM +0200, Joel Granados wrote:
> 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@kernel.org> 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?

Yes, I want to prepare a patchset with a new implementation this weekend.

I want to split the patchset into several parts to make the review easier.
First, I want to create patches for proc_sysctls that add a new mechanism
and a few simple users, such as ipc_sysctls and mq_sysctls. Then I’ll
create a patchset for the network subsystem, ucounts and other large users
of dynamic sysctls.

I need to rebase to the current master branch because sashiko can't verify
my patches.

-- 
Rgrds, legion