netdev - Re: [Intel-wired-lan] [PATCH] i40e: avoid redundant VF link state updates

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: <5578e792-2dd6-42db-8ad6-b12cd05c2617@molgen.mpg.de>
Date: Wed, 22 Oct 2025 14:24:29 +0200
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
 Robert Malz <robert.malz@...onical.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 Jamie Bainbridge <jamie.bainbridge@...il.com>,
 Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
 Dennis Chen <dechen@...hat.com>,
 Przemyslaw Kitszel <przemyslaw.kitszel@...el.com>,
 Lukasz Czapnik <lukasz.czapnik@...el.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, Eric Dumazet <edumazet@...gle.com>,
 Anthony L Nguyen <anthony.l.nguyen@...el.com>,
 Simon Horman <horms@...nel.org>, "Keller, Jacob E"
 <jacob.e.keller@...el.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, "David S . Miller" <davem@...emloft.net>
Subject: Re: [Intel-wired-lan] [PATCH] i40e: avoid redundant VF link state
 updates

Dear Alex,


Thank you for your input.

Am 22.10.25 um 14:06 schrieb Loktionov, Aleksandr:

>> -----Original Message-----
>> From: Paul Menzel <pmenzel@...gen.mpg.de>
>> Sent: Wednesday, October 22, 2025 1:49 PM

>> Am 21.10.25 um 17:44 schrieb Robert Malz:
>>> From: Jay Vosburgh <jay.vosburgh@...onical.com>
>>>
>>> Multiple sources can request VF link state changes with identical
>>> parameters. For example, Neutron may request to set the VF link state
>>> to
>>
>> What is Neutron?
>>
>>> IFLA_VF_LINK_STATE_AUTO during every initialization or user can issue:
>>> `ip link set <ifname> vf 0 state auto` multiple times. Currently, the
>>> i40e driver processes each of these requests, even if the requested
>>> state is the same as the current one. This leads to unnecessary VF
>>> resets and can cause performance degradation or instability in the VF
>>> driver - particularly in DPDK environment.
>>
>> What is DPDK?
>>
> I think Robert needs:
> - to expand acronyms in the commit message (Neutron → OpenStack Neutron, DPDK → Data Plane Development Kit).
> - to fix the comment style as per coding guidelines.
> - add a short note in the commit message about how to reproduce the issue.
> @Paul Menzel right?

Correct.

Maybe also mention how to force it, as there seems to be such an option 
judging from the diff.

>>> With this patch i40e will skip VF link state change requests when the
>>> desired link state matches the current configuration. This prevents
>>> unnecessary VF resets and reduces PF-VF communication overhead.
>>
>> Add a test (with `ip link …`) case to show, that it works now.
>>
>>> Co-developed-by: Robert Malz <robert.malz@...onical.com>
>>> Signed-off-by: Robert Malz <robert.malz@...onical.com>
>>> Signed-off-by: Jay Vosburgh <jay.vosburgh@...onical.com>
>>> ---
>>>    drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>>> b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>>> index 081a4526a2f0..0fe0d52c796b 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
>>> @@ -4788,6 +4788,7 @@ int i40e_ndo_set_vf_link_state(struct net_device *netdev, int vf_id, int link)
>>>    	unsigned long q_map;
>>>    	struct i40e_vf *vf;
>>>    	int abs_vf_id;
>>> +	int old_link;
>>>    	int ret = 0;
>>>    	int tmp;
>>>
>>> @@ -4806,6 +4807,17 @@ int i40e_ndo_set_vf_link_state(struct net_device *netdev, int vf_id, int link)
>>>    	vf = &pf->vf[vf_id];
>>>    	abs_vf_id = vf->vf_id + hw->func_caps.vf_base_id;
>>>
>>> +	/* skip VF link state change if requested state is already set */
>>> +	if (!vf->link_forced)
>>> +		old_link = IFLA_VF_LINK_STATE_AUTO;
>>> +	else if (vf->link_up)
>>> +		old_link = IFLA_VF_LINK_STATE_ENABLE;
>>> +	else
>>> +		old_link = IFLA_VF_LINK_STATE_DISABLE;
>>> +
>>> +	if (link == old_link)
>>> +		goto error_out;
>>
>> Should a debug message be added?
> 
> I think adding one would be redundant since skipping identical state
> changes is expected behavior.

My thinking was, if something does not work as expected for a user, like 
issuing the command to force a reset, that it might be useful to see 
something in the logs.

>>> +
>>>    	pfe.event = VIRTCHNL_EVENT_LINK_CHANGE;
>>>    	pfe.severity = PF_EVENT_SEVERITY_INFO;

Kind regards,

Paul