[iwl-net,2/2] ixgbe: downgrade logging of unsupported VF API version to debug

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.

Reported-by: Yifei Liu <yifei.l.liu@oracle.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@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(-)

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.

> Reported-by: Yifei Liu <yifei.l.liu@oracle.com>

Do you have an Link: for this report?

> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@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?

>   
>   	return -1;
>   }


Kind regards,

Paul

Hi Paul and Jake, 
Please see inline. 

> On Nov 1, 2024, at 11:53 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> 
> 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.
It could be easily reproduced by running
echo 1 > /sys/class/net/<NIC name>device/sriov_numvfs
> 
>> Reported-by: Yifei Liu <yifei.l.liu@oracle.com>
> 
> Do you have an Link: for this report?
Hope this link could help: https://patchwork.kernel.org/project/netdevbpf/patch/20240301235837.3741422-1-yifei.l.liu@oracle.com/ 
> 
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@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?
> 
>>     return -1;
>>  }
> 
> 
> Kind regards,
> 
> Paul
Thanks
Yifei

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Friday, November 1, 2024 11:54 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Yifei Liu <yifei.l.liu@oracle.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@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@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@intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@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

On 11/1/2024 4:05 PM, Jacob Keller wrote:
> 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.
> 
> Reported-by: Yifei Liu <yifei.l.liu@oracle.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Fixes: 339f28964147 ("ixgbevf: Add support for new mailbox communication
between PF and VF")

> ---
>  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);
>  
>  	return -1;
>  }
>

On 11/1/2024 11:53 PM, Paul Menzel wrote:
> 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.
> 
>> Reported-by: Yifei Liu <yifei.l.liu@oracle.com>
> 
> Do you have an Link: for this report?
> 
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@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 would prefer to improve the message via a cleanup on next after this
is merged and next merges with net.

Thanks,
Jake

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jacob
> Keller
> Sent: Tuesday, November 5, 2024 11:50 PM
> To: Paul Menzel <pmenzel@molgen.mpg.de>
> Cc: intel-wired-lan@lists.osuosl.org; Yifei Liu <yifei.l.liu@oracle.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ixgbe: downgrade logging of
> unsupported VF API version to debug
> 
> 
> 
> On 11/1/2024 11:53 PM, Paul Menzel wrote:
> > 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.
> >
> >> Reported-by: Yifei Liu <yifei.l.liu@oracle.com>
> >
> > Do you have an Link: for this report?
> >
> >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> >> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@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);


Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>