Messages in this thread | | | Date | Mon, 13 Jan 2020 15:57:58 +0100 | From | Antoine Tenart <> | Subject | Re: [PATCH net-next v5 05/15] net: macsec: hardware offloading infrastructure |
| |
Hello Jiri,
On Mon, Jan 13, 2020 at 03:34:52PM +0100, Jiri Pirko wrote: > Fri, Jan 10, 2020 at 05:20:00PM CET, antoine.tenart@bootlin.com wrote: > > Couple nitpicks I randomly spotted: > > [...] > > > >+static bool macsec_is_offloaded(struct macsec_dev *macsec) > >+{ > >+ if (macsec->offload == MACSEC_OFFLOAD_PHY) > >+ return true; > >+ > >+ return false; > > Just: > return macsec->offload == MACSEC_OFFLOAD_PHY;
This construction is because I split the phy offloading support from the MAC one, and this check becomes more complex when the MAC offloading support is applied. I could check it's not MACSEC_OFFLOAD_OFF, but I think it's nice having the default set to false when reading the code.
> >+/* Checks if underlying layers implement MACsec offloading functions. */ > >+static bool macsec_check_offload(enum macsec_offload offload, > >+ struct macsec_dev *macsec) > >+{ > >+ if (!macsec || !macsec->real_dev) > >+ return false; > >+ > >+ if (offload == MACSEC_OFFLOAD_PHY) > > You have a helper for this already - macsec_is_offloaded(). No need for > "offload" arg then.
Same here, except the _PHY case is different from the _MAC one. So the check needs to be specific to _PHY.
> >+ return macsec->real_dev->phydev && > >+ macsec->real_dev->phydev->macsec_ops; > >+ > >+ return false; > >+} > >+ > >+static const struct macsec_ops *__macsec_get_ops(enum macsec_offload offload, > >+ struct macsec_dev *macsec, > >+ struct macsec_context *ctx) > >+{ > >+ if (ctx) { > >+ memset(ctx, 0, sizeof(*ctx)); > >+ ctx->offload = offload; > >+ > >+ if (offload == MACSEC_OFFLOAD_PHY) > > Same here.
Same here.
Thanks, Antoine
-- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
| |