Re: [dpdk-dev] [PATCH] net/i40e: fix remove MAC/VLAN addresses error

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

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