Re: MACsec hardware offloading

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Jul 13, 2018 at 04:46:08PM +0200, Antoine Tenart wrote:
> Hello,
> 
> Linux has a software implementation of the MACsec standard but so far,
> to my knowledge, no hardware offloading support was developed and sent
> upstream.

Mellonex is usually the leader in this sort of thing. Adding Jiri to
Cc: However, they often do their PHY control in firmware, rather than
using Linux.

> My main idea would be to reuse and leverage the actual software
> implementation in Linux. The advantages would be to reuse existing code,
> data structure and to configure this using the exact same tools from
> user-space.

I fully agree with this approach. That is how we do all other
offloading to accelerators.

> One important point about adding MACsec offloading in Linux is this can
> be done in either the MAC or the PHY. While I'll be working on making
> this work in a PHY, I know for sure some MAC do have the same capability
> (including for example the Intel ixgbe NIC).

I see in your current code, you check if the netdev has a phydev, and
if the phydev supports macsec, and then go straight to the phy. That
means there is no need to modify the MAC driver, it should just work.
If not, you call the MAC.

I would add support for the PHY to return -EOPNOTSUPP and then fall
back to trying the MAC.

Also, your current checks are
inconsistent. macsec_hw_offload_capable() checks for
phydev->drv->macsec, where as macsec_hw_offload() just checks for
dev->real_dev->phydev->drv. It looks like
dev->real_dev->phydev->drv->macsec could be a NULL pointer and bad
things happen.

For switchdev, when we offload to a switch, the switch nearly always
has the option to say it could not accept the offload. We then fall
back to performing the needed action in software. At the moment, i
don't see you checking the return code of macsec_hw_offload(). So it
is not clear to me if this software fallback works.

The Switchdev API is also transaction based, with a prepare and a
commit phase. All resource allocation happens in the prepare phase and
at this stage, you can return errors indicating offload is not
possible. The commit phase is not allowed to fail. You probably want
to go look at the mailing list archive and look at why this
architecture was decided on.

Maybe the MAC part of this should actually use the switchdev API?  You
then get a lot of infrastructure for free.

     Andrew