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
| ||
|
Message-ID: <20230328182033.GA982617@electric-eye.fr.zoreil.com> Date: Tue, 28 Mar 2023 20:20:33 +0200 From: Francois Romieu <romieu@...zoreil.com> To: Emeel Hakim <ehakim@...dia.com> Cc: Jakub Kicinski <kuba@...nel.org>, "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 Emeel Hakim <ehakim@...dia.com> : > > -----Original Message----- > > From: Jakub Kicinski <kuba@...nel.org> [...] > > > +#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. Sometime it's also nice to be able to use such modern tools as tags and grep. While it still implies some duplication and it doesn't automagically prevent wrongly mixing vlan_macsec_foo() and ops->mdo_bar(), the code below may be considered as a trade-off: #define _B(mdo) \ const struct macsec_ops *ops; \ ops = vlan_dev_priv(ctx->netdev)->real_dev->macsec_ops; \ return ops ? ops->mdo_ ## mdo(ctx) : -EOPNOTSUPP; \ static int vlan_macsec_add_txsa(struct macsec_context *ctx) { _B(add_txsa) } static int vlan_macsec_upd_txsa(struct macsec_context *ctx) { _B(upd_txsa) } [...] #undef _B On a tangent topic, both codes expand 12 times the accessor vlan_dev_priv(ctx->netdev)->real_dev->macsec_ops. It imvho deserves an helper function so that the compiler can make some choice. As a final remark, VLAN_MACSEC_DECLARE_MDO above does not buy much. Compare: static const struct macsec_ops macsec_offload_ops = { .mdo_add_txsa = VLAN_MACSEC_DECLARE_MDO(add_txsa), .mdo_upd_txsa = VLAN_MACSEC_DECLARE_MDO(upd_txsa), [...] vs static const struct macsec_ops macsec_offload_ops = { .mdo_add_txsa = vlan_macsec_add_txsa, .mdo_upd_txsa = vlan_macsec_upd_txsa, [...] This one could probably be: #define _I(mdo) .mdo_ ## mdo = vlan_macsec_ ## mdo static const struct macsec_ops macsec_offload_ops = { _I(add_txsa), _I(upd_txsa), [...] #undef _I -- Ueimor
Powered by blists - more mailing lists