linux-kernel - Re: [EXT] Re: [PATCH v12 net-next 12/15] net: mvpp2: add BM protection underrun feature support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPv3WKd48fiZmdnP+NN_FRCT1h6xmu9zO4BWAz_pgTXW2fQt9w@mail.gmail.com>
Date:   Thu, 11 Feb 2021 20:13:24 +0100
From:   Marcin Wojtas <mw@...ihalf.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Stefan Chulski <stefanc@...vell.com>,
        David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "thomas.petazzoni@...tlin.com" <thomas.petazzoni@...tlin.com>,
        Nadav Haklai <nadavh@...vell.com>,
        Yan Markman <ymarkman@...vell.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "rmk+kernel@...linux.org.uk" <rmk+kernel@...linux.org.uk>,
        "atenart@...nel.org" <atenart@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "sebastian.hesselbarth@...il.com" <sebastian.hesselbarth@...il.com>,
        "gregory.clement@...tlin.com" <gregory.clement@...tlin.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [EXT] Re: [PATCH v12 net-next 12/15] net: mvpp2: add BM
 protection underrun feature support

Hi,

czw., 11 lut 2021 o 15:19 Andrew Lunn <andrew@...n.ch> napisał(a):
>
> On Thu, Feb 11, 2021 at 08:22:19AM +0000, Stefan Chulski wrote:
> >
> > >
> > > ----------------------------------------------------------------------
> > > From: <stefanc@...vell.com>
> > > Date: Wed, 10 Feb 2021 11:48:17 +0200
> > >
> > > >
> > > > +static int bm_underrun_protect = 1;
> > > > +
> > > > +module_param(bm_underrun_protect, int, 0444);
> > > > +MODULE_PARM_DESC(bm_underrun_protect, "Set BM underrun protect
> > > > +feature (0-1), def=1");
> > >
> > > No new module parameters, please.
> >
> > Ok, I would remove new module parameters.
> > By the way why new module parameters forbitten?
>
> Historically, module parameters are a bad interface for
> configuration. Vendors have stuffed all sorts of random junk into
> module parameters. There is little documentation. Different drivers
> can have similar looking module parameters which do different
> things. Or different module parameters, which actually do the same
> thing. But maybe with slightly different parameters.
>
> We get a much better overall result if you stop and think for a
> while. How can this be made a generic configuration knob which
> multiple vendors could use? And then add it to ethtool. Extend the
> ethtool -h text and the man page. Maybe even hack some other vendors
> driver to make use of it.
>
> Or we have also found out, that pushing back on parameters like this,
> the developers goes back and looks at the code, and sometimes figures
> out a way to automatically do the right thing, removing the
> configuration knob, and just making it all simpler for the user to
> use.

I think of 2 alternatives:
* `ethtool --set-priv-flags` - in such case there is a question if
switching this particular feature in runtime is a good idea.
* New DT/ACPI property - it is a hardware feature after all, so maybe
let the user decide whether to enable it on the platform description
level.

What do you think?

Best regards,
Marcin