netdev - Re: [EXT] Re: [net-next PATCH 1/2] octeontx2-pf: Add devlink param to init and de-init serdes

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: <20211108075450.1dbdedc3@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 8 Nov 2021 07:54:50 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     sundeep subbaraya <sundeep.lkml@...il.com>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Hariprasad Kelam <hkelam@...vell.com>,
        Geethasowjanya Akula <gakula@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Rakesh Babu Saladi <rsaladi2@...vell.com>,
        Saeed Mahameed <saeed@...nel.org>,
        "anthony.l.nguyen@...el.com" <anthony.l.nguyen@...el.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Andrew Lunn <andrew@...n.ch>, argeorge@...co.com
Subject: Re: [EXT] Re: [net-next PATCH 1/2] octeontx2-pf: Add devlink param
 to init and de-init serdes

On Sun, 7 Nov 2021 11:21:17 +0200 Ido Schimmel wrote:
> > This looks good. Please note that we need the behavior such that after changing
> > the flag a subsequent ifconfig command is not needed by the user.
> > 
> > auto : in ndo_open, ndo_close check the physical link flag is auto and
> > send command
> >           to firmware for bringing physical link up/down.
> > up: send command to firmware instantaneously for physical link UP
> > down: send command to firmware instantaneously for physical link DOWN  
> 
> TBH, I'm not that happy with my ethtool suggestion. It is not very clear
> which hardware entities the attribute controls.

Last week I heard a request to also be able to model NC-SI disruption.
Control if the NIC should be reset and newly flashed FW activated when
host is rebooted (vs full server power cycle).

That adds another dimension to the problem, even though that particular
use case may be better answered thru the devlink flashing/reset APIs.

Trying to organize the requirements we have 3 entities which may hold
the link up:
 - SFP power policy
 - NC-SI / BMC
 - SR-IOV (legacy)

I'd think auto/up as possible options still make sense, although in
case of NC-SI many NICs may not allow overriding the "up". And the
policy may change without notification if BMC selects / activates 
a port - it may go from auto to up with no notification.

Presumably we want to track "who's holding the link up" per consumer.
Just a bitset with 1s for every consumer holding "up"? 

Or do we expect there will be "more to it" and should create bespoke
nests?

> Maybe it's better to
> implement it as a rtnetlink attribute that controls the carrier (e.g.,
> "carrier_policy")? Note that we already have ndo_change_carrier(), but
> the kdoc comment explicitly mentions that it shouldn't be used by
> physical devices:
>
>  * int (*ndo_change_carrier)(struct net_device *dev, bool new_carrier);
>  *	Called to change device carrier. Soft-devices (like dummy, team, etc)
>  *	which do not represent real hardware may define this to allow their
>  *	userspace components to manage their virtual carrier state. Devices
>  *	that determine carrier state from physical hardware properties (eg
>  *	network cables) or protocol-dependent mechanisms (eg
>  *	USB_CDC_NOTIFY_NETWORK_CONNECTION) should NOT implement this function.

New NDO seems reasonable. 

What are your thoughts on the SFP policy? We can still reshuffle it.