> -----Original Message----- > From: Paul Menzel <[email protected]> > Sent: Wednesday, October 22, 2025 1:49 PM > To: Robert Malz <[email protected]> > Cc: [email protected]; [email protected]; Jamie > Bainbridge <[email protected]>; Michal Swiatkowski > <[email protected]>; Dennis Chen <[email protected]>; > Kitszel, Przemyslaw <[email protected]>; Czapnik, Lukasz > <[email protected]>; Loktionov, Aleksandr > <[email protected]>; Andrew Lunn <[email protected]>; > Eric Dumazet <[email protected]>; Nguyen, Anthony L > <[email protected]>; Simon Horman <[email protected]>; Keller, > Jacob E <[email protected]>; Jakub Kicinski <[email protected]>; > Paolo Abeni <[email protected]>; David S . Miller > <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH] i40e: avoid redundant VF link > state updates > > Dear Robert, > > > Thank you for your patch. > > Am 21.10.25 um 17:44 schrieb Robert Malz: > > From: Jay Vosburgh <[email protected]> > > > > 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?
> > 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 <[email protected]> > > Signed-off-by: Robert Malz <[email protected]> > > Signed-off-by: Jay Vosburgh <[email protected]> > > --- > > 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. Thank you Alex > > + > > pfe.event = VIRTCHNL_EVENT_LINK_CHANGE; > > pfe.severity = PF_EVENT_SEVERITY_INFO; > > > > > Kind regards, > > Paul
