netdev - RE: [net-next 1/4] e1000e: allow turning s0ix flows on for systems with ME

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <DM6PR19MB263628DAC7F032E575C5E5EAFAF50@DM6PR19MB2636.namprd19.prod.outlook.com>
Date:   Mon, 30 Nov 2020 22:15:55 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@...l.com>
To:     Alexander Duyck <alexander.duyck@...il.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>
CC:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Netdev <netdev@...r.kernel.org>,
        Stefan Assmann <sassmann@...hat.com>,
        "Neftin, Sasha" <sasha.neftin@...el.com>,
        Aaron Brown <aaron.f.brown@...el.com>
Subject: RE: [net-next 1/4] e1000e: allow turning s0ix flows on for systems
 with ME

> 
> Generally the use of module parameters and sysfs usage are frowned
> upon. 

I was trying to build on the existing module parameters that existed
already for e1000e.  So I guess I would ask, why are those not done in
ethtool?  Should those parameters go away and they migrate to ethtool
for the same reasons as this?

> Based on the configuration isn't this something that could just
> be controlled via an ethtool priv flag? Couldn't you just have this
> default to whatever the heuristics decide at probe on and then support
> enabling/disabling it via the priv flag? You could look at
> igb_get_priv_flags/igb_set_priv_flags for an example of how to do what
> I am proposing.

I don't disagree this solution would work, but it adds a new dependency
on having ethtool and the kernel move together to enable it.

One advantage of the way this is done it allows shipping a system with an
older Linux kernel that isn't yet recognized in the kernel heuristics to
turn on by default with a small udev rule or kernel command line change.

For example systems that aren't yet released could have this documented on
RHEL certification pages at release time for older RHEL releases before a
patch to add to the heuristics has been backported.

> 
> I think it would simplify this quite a bit since you wouldn't have to
> implement sysfs show/store operations for the value and would instead
> be allowing for reading/setting via the get_priv_flags/set_priv_flags
> operations. In addition you could leave the code for checking what
> supports this in place and have it set a flag that can be read or
> overwritten.

If the consensus is to move in this direction, yes I'll redo the patch
series and modify ethtool as well.