netdev - RE: [PATCH net-next 1/4] vlan: Add MACsec offload operations for VLAN interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <IA1PR12MB6353B5E1BC80B60993267190AB889@IA1PR12MB6353.namprd12.prod.outlook.com>
Date:   Tue, 28 Mar 2023 06:54:11 +0000
From:   Emeel Hakim <ehakim@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "sd@...asysnail.net" <sd@...asysnail.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 1/4] vlan: Add MACsec offload operations for VLAN
 interface



> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Monday, 27 March 2023 19:44
> To: Emeel Hakim <ehakim@...dia.com>
> Cc: davem@...emloft.net; pabeni@...hat.com; edumazet@...gle.com;
> sd@...asysnail.net; netdev@...r.kernel.org
> Subject: Re: [PATCH net-next 1/4] vlan: Add MACsec offload operations for VLAN
> interface
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sun, 26 Mar 2023 10:26:33 +0300 Emeel Hakim wrote:
> > @@ -572,6 +573,9 @@ static int vlan_dev_init(struct net_device *dev)
> >                          NETIF_F_HIGHDMA | NETIF_F_SCTP_CRC |
> >                          NETIF_F_ALL_FCOE;
> >
> > +     if (real_dev->features & NETIF_F_HW_MACSEC)
> > +             dev->hw_features |= NETIF_F_HW_MACSEC;
> > +
> >       dev->features |= dev->hw_features | NETIF_F_LLTX;
> >       netif_inherit_tso_max(dev, real_dev);
> >       if (dev->features & NETIF_F_VLAN_FEATURES) @@ -660,6 +664,9 @@
> > static netdev_features_t vlan_dev_fix_features(struct net_device *dev,
> >       features |= old_features & (NETIF_F_SOFT_FEATURES |
> NETIF_F_GSO_SOFTWARE);
> >       features |= NETIF_F_LLTX;
> >
> > +     if (real_dev->features & NETIF_F_HW_MACSEC)
> > +             features |= NETIF_F_HW_MACSEC;
> > +
> >       return features;
> >  }
> 
> Shouldn't vlan_features be consulted somehow?

I did consider including the vlan_features, but after careful consideration, I couldn't see how they were relevant to the task at hand.
However, if you have any specific suggestions on how I could incorporate them to improve the code, I would be happy to hear them.

> > @@ -803,6 +810,49 @@ static int vlan_dev_fill_forward_path(struct
> net_device_path_ctx *ctx,
> >       return 0;
> >  }
> >
> > +#if IS_ENABLED(CONFIG_MACSEC)
> > +#define VLAN_MACSEC_MDO(mdo) \
> > +static int vlan_macsec_ ## mdo(struct macsec_context *ctx) \ { \
> > +     const struct macsec_ops *ops; \
> > +     ops =  vlan_dev_priv(ctx->netdev)->real_dev->macsec_ops; \
> > +     return ops ? ops->mdo_ ## mdo(ctx) : -EOPNOTSUPP; \ }
> > +
> > +#define VLAN_MACSEC_DECLARE_MDO(mdo) vlan_macsec_ ## mdo
> > +
> > +VLAN_MACSEC_MDO(add_txsa);
> > +VLAN_MACSEC_MDO(upd_txsa);
> > +VLAN_MACSEC_MDO(del_txsa);
> > +
> > +VLAN_MACSEC_MDO(add_rxsa);
> > +VLAN_MACSEC_MDO(upd_rxsa);
> > +VLAN_MACSEC_MDO(del_rxsa);
> > +
> > +VLAN_MACSEC_MDO(add_rxsc);
> > +VLAN_MACSEC_MDO(upd_rxsc);
> > +VLAN_MACSEC_MDO(del_rxsc);
> > +
> > +VLAN_MACSEC_MDO(add_secy);
> > +VLAN_MACSEC_MDO(upd_secy);
> > +VLAN_MACSEC_MDO(del_secy);
> 
> -1
> 
> impossible to grep for the functions :( but maybe others don't care

Thank you for bringing up the issue you noticed. However, I decided to go with this approach
because the functions are simple and look very similar, so there wasn't much to debug.
Using a macro allowed for cleaner code instead of having to resort to ugly code duplication.