[Intel-wired-lan] [PATCH iwl-net 0/2] ixgbe: fix incompatibility with Mailbox API v1.5

[Intel-wired-lan] [PATCH iwl-net 0/2] ixgbe: fix incompatibility with Mailbox API v1.5

[Jacob Keller]

The ixgbevf driver gained support for the v1.5 API of the PF to VF mailbox
communication API in commit 339f28964147 ("ixgbevf: Add support for new
mailbox communication between PF and VF")

This commit accidentally enabled IPSEC offload for the v1.5 API, which is
incorrect as the API is only supported by the v1.4 API.

Other hosts including the VMWare ESX PF have implemented the v1.5 API, but
no Linux PF has.

As far as I can tell after digging, the v1.4 API is only supported by the
in-kernel ixgbe PF as the way of supporting IPSEC offload. Other hosts do
not appear to have implemented this API. In particular, the hosts
implementing v1.5 of the API do not have the IPSEC offload support.

The current situation results in two issues:

1) The ixgbevf attempts to enable IPSEC offload support for PFs operating
   on the v1.5 Mailbox API. This will not work as the PF will not support
   it. As far as I can tell, this results in all calls to
   ixgbevf_ipsec_set_pf_sa failing with an error, preventing IPSEC
   functionality from working.

2) When the in-kernel ixgbevf driver is loaded on an in-kernel ixgbe host,
   the driver logs a warning about an invalid API:

     VF 0 requested invalid api version 6

   This message confuses system administrators, as it implies that the VF
   is doing something wrong.

This series fixes the two issues, first by disabling IPSEC offload for any
API version other than v1.4. Second, the e_info message is downgraded into
a debug message to avoid logging it by default.

I do not yet fully understand the improvements of the v1.5 API, but
currently no Linux PF has implemented it fully. The Intel out-of-tree
releases appear to have some code to support v1.5, but it is incomplete,
and the v1.5 API is not advertised or accepted during negotiation.

If we ever plan to upstream the v1.5 improvements, then something will need
to be done to resolve the IPSEC negotiation -- existing v1.5 hosts do not
support IPSEC. It is not great to have to dedicate entire API versions just
to support IPSEC offload. I believe a proper solution should introduce an
API which can check if IPSEC is supported, and which all hosts can
implement to report whether IPSEC should be enabled. This is more flexible
than having a mailbox API which is not supported by all hosts.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Jacob Keller (2):
      ixgbevf: stop attempting IPSEC offload on Mailbox API 1.5
      ixgbe: downgrade logging of unsupported VF API version to debug

 drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c  | 2 +-
 drivers/net/ethernet/intel/ixgbevf/ipsec.c      | 1 -
 3 files changed, 3 insertions(+), 2 deletions(-)
---
base-commit: 0144c06c5890d1ad0eea65df074cffaf4eea5a3c
change-id: 20241028-jk-ixgbevf-mailbox-v1-5-fixes-b9ed56673e99

Best regards,
-- 
Jacob Keller <jacob.e.keller@intel.com>

[Intel-wired-lan] [PATCH iwl-net 1/2] ixgbevf: stop attempting IPSEC offload on Mailbox API 1.5

[Jacob Keller]

Commit 339f28964147 ("ixgbevf: Add support for new mailbox communication
between PF and VF") added support for v1.5 of the PF to VF mailbox
communication API. This commit mistakenly enabled IPSEC offload for API
v1.5.

No implementation of the v1.5 API has support for IPSEC offload. This
offload is only supported by the Linux PF as mailbox API v1.4. In fact, the
v1.5 API is not implemented in any Linux PF.

Attempting to enable IPSEC offload on a PF which supports v1.5 API will not
work. Only the Linux upstream ixgbe and ixgbevf support IPSEC offload, and
only as part of the v1.4 API.

Fix the ixgbevf Linux driver to stop attempting IPSEC offload when
the mailbox API does not support it.

The existing API design choice makes it difficult to support future API
versions, as other non-Linux hosts do not implement IPSEC offload. If we
add support for v1.5 to the Linux PF, then we lose support for IPSEC
offload.

A full solution likely requires a new mailbox API with a proper negotiation
to check that IPSEC is actually supported by the host.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ipsec.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ipsec.c b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
index 66cf17f19408..f804b35d79c7 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
@@ -629,7 +629,6 @@ void ixgbevf_init_ipsec_offload(struct ixgbevf_adapter *adapter)
 
 	switch (adapter->hw.api_version) {
 	case ixgbe_mbox_api_14:
-	case ixgbe_mbox_api_15:
 		break;
 	default:
 		return;

-- 
2.47.0.265.g4ca455297942

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

[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.

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(-)

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;
 }

-- 
2.47.0.265.g4ca455297942

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

[Paul Menzel]

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

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

[Yifei Liu]

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

Re: [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@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

Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ixgbevf: stop attempting IPSEC offload on Mailbox API 1.5

[Jacob Keller]

On 11/1/2024 4:05 PM, Jacob Keller wrote:
> Commit 339f28964147 ("ixgbevf: Add support for new mailbox communication
> between PF and VF") added support for v1.5 of the PF to VF mailbox
> communication API. This commit mistakenly enabled IPSEC offload for API
> v1.5.
> 
> No implementation of the v1.5 API has support for IPSEC offload. This
> offload is only supported by the Linux PF as mailbox API v1.4. In fact, the
> v1.5 API is not implemented in any Linux PF.
> 
> Attempting to enable IPSEC offload on a PF which supports v1.5 API will not
> work. Only the Linux upstream ixgbe and ixgbevf support IPSEC offload, and
> only as part of the v1.4 API.
> 
> Fix the ixgbevf Linux driver to stop attempting IPSEC offload when
> the mailbox API does not support it.
> 
> The existing API design choice makes it difficult to support future API
> versions, as other non-Linux hosts do not implement IPSEC offload. If we
> add support for v1.5 to the Linux PF, then we lose support for IPSEC
> offload.
> 
> A full solution likely requires a new mailbox API with a proper negotiation
> to check that IPSEC is actually supported by the host.
> 

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

> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbevf/ipsec.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ipsec.c b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> index 66cf17f19408..f804b35d79c7 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> @@ -629,7 +629,6 @@ void ixgbevf_init_ipsec_offload(struct ixgbevf_adapter *adapter)
>  
>  	switch (adapter->hw.api_version) {
>  	case ixgbe_mbox_api_14:
> -	case ixgbe_mbox_api_15:
>  		break;
>  	default:
>  		return;
> 

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

[Jacob Keller]

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;
>  }
> 

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

[Jacob Keller]

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

Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ixgbevf: stop attempting IPSEC offload on Mailbox API 1.5

[Romanowski, Rafal]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jacob
> Keller
> Sent: Tuesday, November 5, 2024 11:42 PM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ixgbevf: stop attempting IPSEC
> offload on Mailbox API 1.5
> 
> 
> 
> On 11/1/2024 4:05 PM, Jacob Keller wrote:
> > Commit 339f28964147 ("ixgbevf: Add support for new mailbox
> > communication between PF and VF") added support for v1.5 of the PF to
> > VF mailbox communication API. This commit mistakenly enabled IPSEC
> > offload for API v1.5.
> >
> > No implementation of the v1.5 API has support for IPSEC offload. This
> > offload is only supported by the Linux PF as mailbox API v1.4. In
> > fact, the
> > v1.5 API is not implemented in any Linux PF.
> >
> > Attempting to enable IPSEC offload on a PF which supports v1.5 API
> > will not work. Only the Linux upstream ixgbe and ixgbevf support IPSEC
> > offload, and only as part of the v1.4 API.
> >
> > Fix the ixgbevf Linux driver to stop attempting IPSEC offload when the
> > mailbox API does not support it.
> >
> > The existing API design choice makes it difficult to support future
> > API versions, as other non-Linux hosts do not implement IPSEC offload.
> > If we add support for v1.5 to the Linux PF, then we lose support for
> > IPSEC offload.
> >
> > A full solution likely requires a new mailbox API with a proper
> > negotiation to check that IPSEC is actually supported by the host.
> >
> 
> Fixes: 339f28964147 ("ixgbevf: Add support for new mailbox communication
> between PF and VF")
> 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbevf/ipsec.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> > b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> > index 66cf17f19408..f804b35d79c7 100644
> > --- a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> > +++ b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> > @@ -629,7 +629,6 @@ void ixgbevf_init_ipsec_offload(struct


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

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

[Romanowski, Rafal]

> -----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>