diff mbox series

net/i40e: fix remove MAC/VLAN addresses error

Message ID 20210928084042.1227848-1-robinx.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series net/i40e: fix remove MAC/VLAN addresses error |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-testing fail build patch failure

Commit Message

Robin Zhang Sept. 28, 2021, 8:40 a.m. UTC 
  Firmware will return I40E_AQ_RC_ENOENT when try to delete non-existent
MAC/VLAN addresses from the HW filtering, this should not be considered as
an Admin Queue error. But in i40e_asq_send_command, it will return
I40E_ERR_ADMIN_QUEUE_ERROR if the return value of Admin Queue command
processed by Firmware is not I40E_AQ_RC_OK or I40E_AQ_RC_EBUSY.

Use i40e_aq_remove_macvlan_v2 instead so that we can get the corresponding
Admin Queue status, and not report as an error in DPDK when Firmware
return I40E_AQ_RC_ENOENT.

Fixes: 4861cde46116 ("i40e: new poll mode driver")

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

Kevin Traynor Sept. 28, 2021, 9:53 a.m. UTC | #1 
On 28/09/2021 09:40, Robin Zhang wrote:
> Firmware will return I40E_AQ_RC_ENOENT when try to delete non-existent
> MAC/VLAN addresses from the HW filtering, this should not be considered as
> an Admin Queue error. But in i40e_asq_send_command, it will return
> I40E_ERR_ADMIN_QUEUE_ERROR if the return value of Admin Queue command
> processed by Firmware is not I40E_AQ_RC_OK or I40E_AQ_RC_EBUSY.
> 
> Use i40e_aq_remove_macvlan_v2 instead so that we can get the corresponding
> Admin Queue status, and not report as an error in DPDK when Firmware
> return I40E_AQ_RC_ENOENT.
> 
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> 

Is it relevant to stable releases (20.11/19.11) with earlier firmware?
https://git.dpdk.org/dpdk/tree/doc/guides/nics/i40e.rst#n101

> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> ---
>   drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index acbe7380b1..fdc9943034 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -7036,6 +7036,7 @@ i40e_remove_macvlan_filters(struct i40e_vsi *vsi,
>   	int ret = I40E_SUCCESS;
>   	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
>   	struct i40e_aqc_remove_macvlan_element_data *req_list;
> +	enum i40e_admin_queue_err aq_status;
>   
>   	if (filter == NULL  || total == 0)
>   		return I40E_ERR_PARAM;
> @@ -7083,11 +7084,17 @@ i40e_remove_macvlan_filters(struct i40e_vsi *vsi,
>   			req_list[i].flags = rte_cpu_to_le_16(flags);
>   		}
>   
> -		ret = i40e_aq_remove_macvlan(hw, vsi->seid, req_list,
> -						actual_num, NULL);
> +		ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, req_list,
> +						actual_num, NULL, &aq_status);
> +
>   		if (ret != I40E_SUCCESS) {
> -			PMD_DRV_LOG(ERR, "Failed to remove macvlan filter");
> -			goto DONE;
> +			/* Do not report as an error when firmware returns ENOENT */
> +			if (aq_status == I40E_AQ_RC_ENOENT) {
> +				ret = I40E_SUCCESS;
> +			} else {
> +				PMD_DRV_LOG(ERR, "Failed to remove macvlan filter");
> +				goto DONE;
> +			}
>   		}
>   		num += actual_num;
>   	} while (num < total);
>
Robin Zhang Sept. 28, 2021, 10:36 a.m. UTC | #2 
Hi,

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday, September 28, 2021 5:54 PM
> To: Zhang, RobinX <robinx.zhang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>; Yang, SteveX <stevex.yang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix remove MAC/VLAN addresses
> error
> 
> On 28/09/2021 09:40, Robin Zhang wrote:
> > Firmware will return I40E_AQ_RC_ENOENT when try to delete non-
> existent
> > MAC/VLAN addresses from the HW filtering, this should not be
> > considered as an Admin Queue error. But in i40e_asq_send_command, it
> > will return I40E_ERR_ADMIN_QUEUE_ERROR if the return value of Admin
> > Queue command processed by Firmware is not I40E_AQ_RC_OK or
> I40E_AQ_RC_EBUSY.
> >
> > Use i40e_aq_remove_macvlan_v2 instead so that we can get the
> > corresponding Admin Queue status, and not report as an error in DPDK
> > when Firmware return I40E_AQ_RC_ENOENT.
> >
> > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> >
> 
> Is it relevant to stable releases (20.11/19.11) with earlier firmware?
> https://git.dpdk.org/dpdk/tree/doc/guides/nics/i40e.rst#n101
> 

No, i40e_aq_remove_macvlan_v2 is added in latest i40e share code.
So this patch cannot cc stable either.

> > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > ---
> >   drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index acbe7380b1..fdc9943034 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -7036,6 +7036,7 @@ i40e_remove_macvlan_filters(struct i40e_vsi *vsi,
> >   	int ret = I40E_SUCCESS;
> >   	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
> >   	struct i40e_aqc_remove_macvlan_element_data *req_list;
> > +	enum i40e_admin_queue_err aq_status;
> >
> >   	if (filter == NULL  || total == 0)
> >   		return I40E_ERR_PARAM;
> > @@ -7083,11 +7084,17 @@ i40e_remove_macvlan_filters(struct i40e_vsi
> *vsi,
> >   			req_list[i].flags = rte_cpu_to_le_16(flags);
> >   		}
> >
> > -		ret = i40e_aq_remove_macvlan(hw, vsi->seid, req_list,
> > -						actual_num, NULL);
> > +		ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, req_list,
> > +						actual_num, NULL,
> &aq_status);
> > +
> >   		if (ret != I40E_SUCCESS) {
> > -			PMD_DRV_LOG(ERR, "Failed to remove macvlan
> filter");
> > -			goto DONE;
> > +			/* Do not report as an error when firmware returns
> ENOENT */
> > +			if (aq_status == I40E_AQ_RC_ENOENT) {
> > +				ret = I40E_SUCCESS;
> > +			} else {
> > +				PMD_DRV_LOG(ERR, "Failed to remove
> macvlan filter");
> > +				goto DONE;
> > +			}
> >   		}
> >   		num += actual_num;
> >   	} while (num < total);
> >
Qi Zhang Oct. 9, 2021, 9:21 a.m. UTC | #3 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Zhang, RobinX
> Sent: Tuesday, September 28, 2021 6:36 PM
> To: Kevin Traynor <ktraynor@redhat.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>; Yang, SteveX <stevex.yang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix remove MAC/VLAN addresses
> error
> 
> Hi,
> 
> > -----Original Message-----
> > From: Kevin Traynor <ktraynor@redhat.com>
> > Sent: Tuesday, September 28, 2021 5:54 PM
> > To: Zhang, RobinX <robinx.zhang@intel.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Guo, Junfeng
> > <junfeng.guo@intel.com>; Yang, SteveX <stevex.yang@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix remove MAC/VLAN
> > addresses error
> >
> > On 28/09/2021 09:40, Robin Zhang wrote:
> > > Firmware will return I40E_AQ_RC_ENOENT when try to delete non-
> > existent
> > > MAC/VLAN addresses from the HW filtering, this should not be
> > > considered as an Admin Queue error. But in i40e_asq_send_command, it
> > > will return I40E_ERR_ADMIN_QUEUE_ERROR if the return value of Admin
> > > Queue command processed by Firmware is not I40E_AQ_RC_OK or
> > I40E_AQ_RC_EBUSY.
> > >
> > > Use i40e_aq_remove_macvlan_v2 instead so that we can get the
> > > corresponding Admin Queue status, and not report as an error in DPDK
> > > when Firmware return I40E_AQ_RC_ENOENT.
> > >
> > > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> > >
> >
> > Is it relevant to stable releases (20.11/19.11) with earlier firmware?
> > https://git.dpdk.org/dpdk/tree/doc/guides/nics/i40e.rst#n101
> >
> 
> No, i40e_aq_remove_macvlan_v2 is added in latest i40e share code.
> So this patch cannot cc stable either.

To avoid confuse, lets remove the "fix" from this patch, 

Actually this is an AQ command upgrade to support new firmware and keep compatible with old firmware

So the title could be "upgrade AQ command of MAC/VLAN remove"

There might be the requirement to support new firmware in LTS release, but that should be covered by separated patches.


> 
> > > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > > ---
> > >   drivers/net/i40e/i40e_ethdev.c | 15 +++++++++++----
> > >   1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index acbe7380b1..fdc9943034 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -7036,6 +7036,7 @@ i40e_remove_macvlan_filters(struct i40e_vsi
> *vsi,
> > >   	int ret = I40E_SUCCESS;
> > >   	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
> > >   	struct i40e_aqc_remove_macvlan_element_data *req_list;
> > > +	enum i40e_admin_queue_err aq_status;
> > >
> > >   	if (filter == NULL  || total == 0)
> > >   		return I40E_ERR_PARAM;
> > > @@ -7083,11 +7084,17 @@ i40e_remove_macvlan_filters(struct i40e_vsi
> > *vsi,
> > >   			req_list[i].flags = rte_cpu_to_le_16(flags);
> > >   		}
> > >
> > > -		ret = i40e_aq_remove_macvlan(hw, vsi->seid, req_list,
> > > -						actual_num, NULL);
> > > +		ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, req_list,
> > > +						actual_num, NULL,
> > &aq_status);
> > > +
> > >   		if (ret != I40E_SUCCESS) {
> > > -			PMD_DRV_LOG(ERR, "Failed to remove macvlan
> > filter");
> > > -			goto DONE;
> > > +			/* Do not report as an error when firmware returns
> > ENOENT */
> > > +			if (aq_status == I40E_AQ_RC_ENOENT) {
> > > +				ret = I40E_SUCCESS;
> > > +			} else {
> > > +				PMD_DRV_LOG(ERR, "Failed to remove
> > macvlan filter");
> > > +				goto DONE;
> > > +			}
> > >   		}
> > >   		num += actual_num;
> > >   	} while (num < total);
> > >
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index acbe7380b1..fdc9943034 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -7036,6 +7036,7 @@  i40e_remove_macvlan_filters(struct i40e_vsi *vsi,
 	int ret = I40E_SUCCESS;
 	struct i40e_hw *hw = I40E_VSI_TO_HW(vsi);
 	struct i40e_aqc_remove_macvlan_element_data *req_list;
+	enum i40e_admin_queue_err aq_status;
 
 	if (filter == NULL  || total == 0)
 		return I40E_ERR_PARAM;
@@ -7083,11 +7084,17 @@  i40e_remove_macvlan_filters(struct i40e_vsi *vsi,
 			req_list[i].flags = rte_cpu_to_le_16(flags);
 		}
 
-		ret = i40e_aq_remove_macvlan(hw, vsi->seid, req_list,
-						actual_num, NULL);
+		ret = i40e_aq_remove_macvlan_v2(hw, vsi->seid, req_list,
+						actual_num, NULL, &aq_status);
+
 		if (ret != I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Failed to remove macvlan filter");
-			goto DONE;
+			/* Do not report as an error when firmware returns ENOENT */
+			if (aq_status == I40E_AQ_RC_ENOENT) {
+				ret = I40E_SUCCESS;
+			} else {
+				PMD_DRV_LOG(ERR, "Failed to remove macvlan filter");
+				goto DONE;
+			}
 		}
 		num += actual_num;
 	} while (num < total);