Re: [PATCH v4 01/27] net/ixgbe: remove MAC type check macros

Bruce Richardson
2026-02-17T15:32:19Z

On Tue, Feb 17, 2026 at 03:23:00PM +0100, Burakov, Anatoly wrote:
> On 2/17/2026 1:58 PM, Bruce Richardson wrote:
> > On Tue, Feb 17, 2026 at 01:50:36PM +0100, Burakov, Anatoly wrote:
> > > On 2/16/2026 5:58 PM, Bruce Richardson wrote:
> > > > On Fri, Feb 13, 2026 at 10:26:12AM +0000, Anatoly Burakov wrote:
> > > > > The macros used were not informative and did not add any value beyond code
> > > > > golf, so remove them and make MAC type checks explicit.
> > > > >
> > > > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > > > ---
> > > > > drivers/net/intel/ixgbe/ixgbe_ethdev.h | 12 ------------
> > > > > drivers/net/intel/ixgbe/ixgbe_flow.c | 20 +++++++++++++++++---
> > > > > 2 files changed, 17 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.h b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
> > > > > index 5dbd659941..7dc02a472b 100644
> > > > > --- a/drivers/net/intel/ixgbe/ixgbe_ethdev.h
> > > > > +++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
> > > > > @@ -137,18 +137,6 @@
> > > > > #define IXGBE_MAX_FDIR_FILTER_NUM (1024 * 32)
> > > > > #define IXGBE_MAX_L2_TN_FILTER_NUM 128
> > > > > -#define MAC_TYPE_FILTER_SUP_EXT(type) do {\
> > > > > - if ((type) != ixgbe_mac_82599EB && (type) != ixgbe_mac_X540)\
> > > > > - return -ENOTSUP;\
> > > > > -} while (0)
> > > > > -
> > > > > -#define MAC_TYPE_FILTER_SUP(type) do {\
> > > > > - if ((type) != ixgbe_mac_82599EB && (type) != ixgbe_mac_X540 &&\
> > > > > - (type) != ixgbe_mac_X550 && (type) != ixgbe_mac_X550EM_x &&\
> > > > > - (type) != ixgbe_mac_X550EM_a && (type) != ixgbe_mac_E610)\
> > > > > - return -ENOTSUP;\
> > > > > -} while (0)
> > > > > -
> > > >
> > > > Ack for removing the former. For the latter, since the list is longer and
> > > > the code is used twice, I'd be tempted to convert to an inline function
> > > > taking in struct hw and returning type bool. WDYT?
> > > >
> > >
> > > I don't want to use a macro/inline function just to save on code, it has to
> > > have some semantic meaning. Do you have any suggestions on what it is that
> > > we'd be checking in these cases?
> > >
> > From the title of the macro I assumed it was whether mac filters are
> > supported or not? However, if that's not really the case and this is an
> > arbitrary set of MAC types for some particular use case, then yes, agree
> > that it's best to remove the macro completely and inline.
> >
>
> My reading of the name of the macros are that they are "filtering supported
> features by mac type" (MAC_TYPE_FILTER), and there are two varieties -
> "supported" and "supported extended" (SUP and SUP_EXT), but with no actual
> semantic meaning.
>
Ack, then inlining as you suggest seems reasonable.