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 for Android: free password hash cracker in your pocket

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

Message-ID: <61CC2BC414934749BD9F5BF3D5D94044986705C1@ORSMSX112.amr.corp.intel.com>
Date:   Wed, 22 Apr 2020 02:26:31 +0000
From:   "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
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

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Tuesday, April 21, 2020 19:14
> 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>
> 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..
[Kirsher, Jeffrey T] 

I know this feature was driven from a customer request/demand where the
link-down-on-close was not sufficient or caused issues.  I want to confirm what
I believe to be the answers to your questions before I respond to them.

> 
> > > 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.
[Kirsher, Jeffrey T] 

It will be for the 5.8 kernel...  Probably won't be next week, will most likely be
1-2 weeks before I get to those changes/patch.