Hi Antoine,

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.

Regards,
  Igor

Reply via email to