netdev - Re: [net-next 3/4] i40e: Add support for a new feature: Total Port Shutdown

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: <20200421191359.0a48133b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 21 Apr 2020 19:13:59 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        "Kwapulinski, Piotr" <piotr.kwapulinski@...el.com>,
        "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
Subject: Re: [net-next 3/4] i40e: Add support for a new feature: Total Port
 Shutdown

On Tue, 21 Apr 2020 22:36:21 +0000 Kirsher, Jeffrey T wrote:
> > -----Original Message-----
> > From: Jakub Kicinski <kuba@...nel.org>
> > Sent: Tuesday, April 21, 2020 10:56
> > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@...el.com>
> > Cc: davem@...emloft.net; Kubalewski, Arkadiusz
> > <arkadiusz.kubalewski@...el.com>; netdev@...r.kernel.org;
> > nhorman@...hat.com; sassmann@...hat.com; Kwapulinski, Piotr
> > <piotr.kwapulinski@...el.com>; Loktionov, Aleksandr
> > <aleksandr.loktionov@...el.com>; Bowers, AndrewX
> > <andrewx.bowers@...el.com>
> > Subject: Re: [net-next 3/4] i40e: Add support for a new feature: Total Port
> > Shutdown
> > 
> > On Mon, 20 Apr 2020 18:49:31 -0700 Jeff Kirsher wrote:  
> > > From: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
> > >
> > > Currently after requesting to down a link on a physical network port,
> > > the traffic is no longer being processed but the physical link with a
> > > link partner is still established.
> > >
> > > Total Port Shutdown allows to completely shutdown the port on the
> > > link-down procedure by physically removing the link from the port.
> > >
> > > Introduced changes:
> > > - probe NVM if the feature was enabled at initialization of the port
> > > - special handling on link-down procedure to let FW physically
> > > shutdown the port if the feature was enabled  
> > 
> > How is this different than link-down-on-close?  
> [Kirsher, Jeffrey T] 
> 
> First of all total-port-shutdown is a read only flag, the user cannot set it
> from the OS.  It is possible to set it in bios, but only if the motherboard
> supports it and the NIC has that capability.  Also, the behavior on both
> slightly differs, link-down-on-close brings the link down by sending
> (to firmware) phy_type=0, while total-port-shutdown does not, the
> phy_type is not changed, instead firmware is using
> I40E_AQ_PHY_ENABLE_LINK flag. 

I see. IOW it's a flag that says the other flag is hard wired to on.

Why is it important to prevent user from performing the configuration?
What if an old kernel is run which won't prevent it?

Let's drill down into what we actually want to express here and then
look at the API. Michal has already converted ethtool link info to
netlink..

> > Perhaps it'd be good to start documenting the private flags in Documentation/  
> [Kirsher, Jeffrey T] 
> 
> We could look at adding that information into our kernel documentation, I am
> planning on updating the driver documentation in a follow-up patch set.  Would
> a descriptive code comment help in this case? 

Documentation should be sufficient, IMHO, if it's coming soon.