Re: [dpdk-dev] [PATCH 42/49] net/ice/base: change how VMDq capability is wrapped

["Stillwell Jr, Paul M" <paul.m.stillwell.jr@intel.com>]

There will be a newer version of this patch that removes this code.

Paul

> -----Original Message-----
> From: Venkataramanan, Anirudh
> Sent: Tuesday, June 4, 2019 12:17 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>
> Cc: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH 42/49] net/ice/base: change how VMDq capability is
> wrapped
> 
> NACK. Please see below.
> 
> On Tue, 2019-06-04 at 13:42 +0800, Rong, Leyi wrote:
> > This patch exposes the VMDq capability when at least one among
> > VMDQ_SUPPORT, OFFLOAD_MACVLAN_SUPPORT or ADQ_SUPPORT (ADQ
> uses
> > VMDQ as well) is defined.
> >
> > Signed-off-by: Anirudh Venkataramanan <
> > anirudh.venkataramanan@intel.com>
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> > ---
> >  drivers/net/ice/base/ice_adminq_cmd.h | 3 +++
> >  drivers/net/ice/base/ice_common.c     | 7 +++++++
> >  drivers/net/ice/base/ice_type.h       | 8 ++++++++
> >  3 files changed, 18 insertions(+)
> >
> > diff --git a/drivers/net/ice/base/ice_adminq_cmd.h
> > b/drivers/net/ice/base/ice_adminq_cmd.h
> > index 4e6bce18c..7642a923b 100644
> > --- a/drivers/net/ice/base/ice_adminq_cmd.h
> > +++ b/drivers/net/ice/base/ice_adminq_cmd.h
> > @@ -111,6 +111,9 @@ struct ice_aqc_list_caps_elem {
> >  	__le16 cap;
> >  #define ICE_AQC_CAPS_VALID_FUNCTIONS			0x0005
> >  #define ICE_AQC_MAX_VALID_FUNCTIONS			0x8
> > +#if defined(VMDQ_SUPPORT) ||
> defined(OFFLOAD_MACVLAN_SUPPORT) ||
> > defined(ADQ_SUPPORT) || defined(FW_SUPPORT)
> > +#define ICE_AQC_CAPS_VMDQ				0x0014
> > +#endif /* VMDQ_SUPPORT || OFFLOAD_MACVLAN_SUPPORT ||
> ADQ_SUPPORT ||
> > FW_SUPPORT */
> 
> There doesn't seem to be anything in the makefile that defines any of
> these *_SUPPORT defines. Patch should be updated to not wrap these
> fields. Commit message should be updated as well.
> 
> >  #define ICE_AQC_CAPS_VSI				0x0017
> >  #define ICE_AQC_CAPS_DCB				0x0018
> >  #define ICE_AQC_CAPS_RSS				0x0040
> > diff --git a/drivers/net/ice/base/ice_common.c
> > b/drivers/net/ice/base/ice_common.c
> > index f9a5d43e6..1d54f3d71 100644
> > --- a/drivers/net/ice/base/ice_common.c
> > +++ b/drivers/net/ice/base/ice_common.c
> > @@ -1980,6 +1980,13 @@ ice_parse_caps(struct ice_hw *hw, void *buf,
> > u32 cap_count,
> >  				  "%s: valid functions = %d\n", prefix,
> >  				  caps->valid_functions);
> >  			break;
> > +#if defined(VMDQ_SUPPORT) ||
> defined(OFFLOAD_MACVLAN_SUPPORT) ||
> > defined(ADQ_SUPPORT)
> > +		case ICE_AQC_CAPS_VMDQ:
> > +			caps->vmdq = (number == 1);
> > +			ice_debug(hw, ICE_DBG_INIT,
> > +				  "%s: VMDQ = %d\n", prefix, caps-
> > >vmdq);
> > +			break;
> > +#endif /* VMDQ_SUPPORT || OFFLOAD_MACVLAN_SUPPORT ||
> ADQ_SUPPORT */
> 
> ditto
> 
> >  		case ICE_AQC_CAPS_VSI:
> >  			if (dev_p) {
> >  				dev_p->num_vsi_allocd_to_host = number;
> > diff --git a/drivers/net/ice/base/ice_type.h
> > b/drivers/net/ice/base/ice_type.h
> > index f76be2b58..f30b37985 100644
> > --- a/drivers/net/ice/base/ice_type.h
> > +++ b/drivers/net/ice/base/ice_type.h
> > @@ -207,6 +207,9 @@ enum ice_vsi_type {
> >  #ifdef ADQ_SUPPORT
> >  	ICE_VSI_CHNL = 4,
> >  #endif /* ADQ_SUPPORT */
> > +#ifdef OFFLOAD_MACVLAN_SUPPORT
> > +	ICE_VSI_OFFLOAD_MACVLAN = 5,
> > +#endif /* OFFLOAD_MACVLAN_SUPPORT */
> 
> ditto
> 
> >  	ICE_VSI_LB = 6,
> >  };
> >
> > @@ -353,6 +356,11 @@ struct ice_hw_common_caps {
> >  #define ICE_MAX_SUPPORTED_GPIO_SDP	8
> >  	u8 led[ICE_MAX_SUPPORTED_GPIO_LED];
> >  	u8 sdp[ICE_MAX_SUPPORTED_GPIO_SDP];
> > +#if defined(VMDQ_SUPPORT) ||
> defined(OFFLOAD_MACVLAN_SUPPORT) ||
> > defined(ADQ_SUPPORT)
> > +
> > +	/* VMDQ */
> > +	u8 vmdq;			/* VMDQ supported */
> > +#endif /* VMDQ_SUPPORT || OFFLOAD_MACVLAN_SUPPORT ||
> ADQ_SUPPORT */
> 
> ditto
> 
> >
> >  	/* EVB capabilities */
> >  	u8 evb_802_1_qbg;		/* Edge Virtual Bridging */