Message-ID: <20200610142036.30c13bb9@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 10 Jun 2020 14:20:36 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     dwilder <dwilder@...ibm.com>
Cc:     netdev@...r.kernel.org, wilder@...ibm.com,
        ajit.khaparde@...adcom.com, sriharsha.basavapatna@...adcom.com,
        somnath.kotur@...adcom.com
Subject: Re: [(RFC) PATCH ] be2net: Allow a VF to use physical link state.

On Wed, 10 Jun 2020 10:22:22 -0700 dwilder wrote:
> On 2020-06-09 14:58, Jakub Kicinski wrote:
> > On Mon,  8 Jun 2020 17:00:59 -0700 David Wilder wrote:  
> >> Hyper-visors owning a PF are allowed by Emulex specification to 
> >> provide
> >> a VF with separate physical and/or logical link states. However, on
> >> linux, a VF driver must chose one or the other.
> >> 
> >> My scenario is a proprietary driver controlling the PF, be2net is the 
> >> VF.
> >> When I do a physical cable pull test the PF sends a link event
> >> notification to the VF with the "physical" link status but this is
> >> ignored in be2net (see be_async_link_state_process() ).
> >> 
> >> The PF is reporting the adapter type as:
> >> 0xe228   /* Device id for VF in Lancer */
> >> 
> >> I added a module parameter "use_pf_link_state". When set the VF should
> >> ignore logical link state and use the physical link state.
> >> 
> >> However I have an issue making this work.  When the cable is pulled I
> >> see two link statuses reported:
> >> [1706100.767718] be2net 8002:01:00.0 eth1: Link is Down
> >> [1706101.189298] be2net 8002:01:00.0 eth1: Link is Up
> >> 
> >> be_link_status_update() is called twice, the first with the physical 
> >> link
> >> status called from be_async_link_state_process(), and the second with 
> >> the
> >> logical link status from be_get_link_ksettings().
> >> 
> >> I am unsure why be_async_link_state_process() is called from
> >> be_get_link_ksettings(), it results in multiple link state changes
> >> (even in the un-patched case). If I eliminate this call then it works.
> >> But I am un-sure of this change.
> >> 
> >> Signed-off-by: David Wilder <dwilder@...ibm.com>  
> > 
> > Hm. Just looking at the code in __be_cmd_set_logical_link_config():
> > 
> > 
> > 	if (link_state == IFLA_VF_LINK_STATE_ENABLE ||
> > 	    link_state == IFLA_VF_LINK_STATE_AUTO)
> > 		link_config |= PLINK_ENABLE;
> > 
> > 	if (link_state == IFLA_VF_LINK_STATE_AUTO)
> > 		link_config |= PLINK_TRACK;
> > 
> > Maybe we shouldn't set ENABLE for AUTO?  
> 
> If I am understanding this correctly, this is used by the linux PF 
> driver to configure how link status is delivered to a VF.
> 
> My problem is one of interoperability between the PF (not linux) and the 
> VF is running on linux.

I see.

> The PF driver is implemented to the Emulex/Broadcom spec, which allows a 
> PF driver to be configured such that the VF can be notified of both 
> physical and logical link status, separately.

We'd need a first-class support for this behavior both on PF and VF
sides. A module parameter in one vendor driver to enable support for
this behavior when running with a non-Linux PF driver is unlikely to
succeed.

> > The module parameter is definitely not a good idea, what you're asking
> > for seems to be well within the expectation from the
> > .ndo_set_vf_link_state config, so it seems the driver / firmware is 
> > just
> > not implementing that right.  
> 
> I am attempting to resolve an issue that the linux implementation cant 
> conform to the the Emulex specification due to the implementation on 
> linux.

Sadly I'd think that it's Emulex that needs to conform to Linux APIs,
not the other way around.

Powered by blists - more mailing lists