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