[Intel-wired-lan] [PATCH iwl-net 2/2] ixgbe: downgrade logging of unsupported VF API version to debug

[Keller, Jacob E]

> -----Original Message-----
> From: Paul Menzel <pmenzel at molgen.mpg.de>
> Sent: Friday, November 1, 2024 11:54 PM
> To: Keller, Jacob E <jacob.e.keller at intel.com>
> Cc: intel-wired-lan at lists.osuosl.org; Yifei Liu <yifei.l.liu at oracle.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel at intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ixgbe: downgrade logging of
> unsupported VF API version to debug
> 
> Dear Jacob,
> 
> 
> Thank you for the patch.
> 
> Am 02.11.24 um 00:05 schrieb Jacob Keller:
> > The ixgbe PF driver logs an info message when a VF attempts to negotiate an
> > API version which it does not support:
> >
> >    VF 0 requested invalid api version 6
> >
> > The ixgbevf driver attempts to load with mailbox API v1.5, which is
> > required for best compatibility with other hosts such as the ESX VMWare PF.
> >
> > The Linux PF only supports API v1.4, and does not currently have support
> > for the v1.5 API.
> >
> > The logged message can confuse users, as the v1.5 API is valid, but just
> > happens to not currently be supported by the Linux PF.
> >
> > Downgrade the info message to a debug message, and fix the language to
> > use 'unsupported' instead of 'invalid' to improve message clarity.
> >
> > Long term, we should investigate whether the improvements in the v1.5 API
> > make sense for the Linux PF, and if so implement them properly. This may
> > require yet another API version to resolve issues with negotiating IPSEC
> > offload support.
> 
> It’d be great if you described the exact test setup for how to reproduce it.
> 

If you load the builtin ixgbevf and ixgbe drivers, you'll see this message.

> > Reported-by: Yifei Liu <yifei.l.liu at oracle.com>
> 
> Do you have an Link: for this report?
> 

I do not, it was reported to me privately over email.

> > Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel at intel.com>
> > ---
> >   drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 2 ++
> >   drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c  | 2 +-
> >   2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> > index 6493abf189de..6639069ad528 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> > @@ -194,6 +194,8 @@ u32 ixgbe_read_reg(struct ixgbe_hw *hw, u32 reg);
> >   	dev_err(&adapter->pdev->dev, format, ## arg)
> >   #define e_dev_notice(format, arg...) \
> >   	dev_notice(&adapter->pdev->dev, format, ## arg)
> > +#define e_dbg(msglvl, format, arg...) \
> > +	netif_dbg(adapter, msglvl, adapter->netdev, format, ## arg)
> >   #define e_info(msglvl, format, arg...) \
> >   	netif_info(adapter, msglvl, adapter->netdev, format, ## arg)
> >   #define e_err(msglvl, format, arg...) \
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > index e71715f5da22..20415c1238ef 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> > @@ -1047,7 +1047,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter
> *adapter,
> >   		break;
> >   	}
> >
> > -	e_info(drv, "VF %d requested invalid api version %u\n", vf, api);
> > +	e_dbg(drv, "VF %d requested unsupported api version %u\n", vf, api);
> 
> Is there a way to translate `api` to the API version scheme used in the
> commit message? So, 1.5 instead of 6? Maybe also add, that only the v1.4
> API is supported?
> 

I suppose I could add a enum to string converter. I didn't really feel that was worthwhile in a net fix.

My primary goal here is to stop complaining about v1.5 API since it’s a "known" but not compatible with the current ixgbe code. Users see the warning and get confused, so the only change I care about for net is converting to e_dbg so that it stops showing up without explicit developer interaction. Its not helpful to most end users.

> >
> >   	return -1;
> >   }
> 
> 
> Kind regards,
> 
> Paul