netdev - Re: [(RFC) PATCH ] be2net: Allow a VF to use physical link state.

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: <20200609145839.36f1cbec@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 9 Jun 2020 14:58:39 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     David Wilder <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 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?

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.