Hi Igor,
On Thu, Jan 24, 2019 at 02:58:45PM +0000, Igor Russkikh wrote:
>
> Its great to see macsec hw offload infrastructure happening!
>
> > @@ -1441,6 +1445,10 @@ struct net_device_ops {
> > u32 flags);
> > int (*ndo_xsk_async_xmit)(struct net_device *dev,
> > u32 queue_id);
> > +#ifdef CONFIG_MACSEC
> > + int (*ndo_macsec)(struct net_device *dev,
> > + struct netdev_macsec *macsec);
> > +#endif
> > };
>
> Most of ndo's are named by action verbs. This ndo is abit misleading, reader
> may have difficulties understanding what
>
> + if (phydev->drv->macsec)
> + ret = phydev->drv->macsec(phydev, macsec);
>
> is actually doing.
>
> May be it'd be better renaming to at least ndo_do_macsec(), or
> ndo_setup_macsec()
> ?
>
> Similar comment is for
>
> +struct netdev_macsec {
>
> It reads like a macsec device context, but it is a macsec configuration
> command.
OK, I'll rename the functions so that they contain a verb.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
