netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "allan.nielsen@microchip.com" <allan.nielsen@microchip.com>
To: Sabrina Dubroca <sd@queasysnail.net>
Cc: Antoine Tenart <antoine.tenart@bootlin.com>,
	Igor Russkikh <Igor.Russkikh@aquantia.com>,
	Andrew Lunn <andrew@lunn.ch>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"camelia.groza@nxp.com" <camelia.groza@nxp.com>,
	Simon Edelhaus <Simon.Edelhaus@aquantia.com>,
	Pavel Belous <Pavel.Belous@aquantia.com>
Subject: Re: [PATCH net-next v2 6/9] net: macsec: hardware offloading infrastructure
Date: Wed, 21 Aug 2019 11:24:08 +0200	[thread overview]
Message-ID: <20190821092406.p66uto4kuozakels@lx-anielsen.microsemi.net> (raw)
In-Reply-To: <20190820144119.GA28714@bistromath.localdomain>

Hi,

I can add some information to the HW Antoine is working on, general design of it
and the thoughts behind it. See below.

The 08/20/2019 16:41, Sabrina Dubroca wrote:
> 2019-08-20, 12:01:40 +0200, Antoine Tenart wrote:
> > So it seems the ability to enable or disable the offloading on a given
> > interface is the main missing feature. I'll add that, however I'll
> > probably (at least at first):
> > 
> > - Have the interface to be fully offloaded or fully handled in s/w (with
> >   errors being thrown if a given configuration isn't supported). Having
> >   both at the same time on a given interface would be tricky because of
> >   the MACsec validation parameter.
> > 
> > - Won't allow to enable/disable the offloading of there are rules in
> >   place, as we're not sure the same rules would be accepted by the other
> >   implementation.
> 
> That's probably quite problematic actually, because to do that you
> need to be able to resync the state between software and hardware,
> particularly packet numbers. So, yeah, we're better off having it
> completely blocked until we have a working implementation (if that
> ever happens).
> 
> However, that means in case the user wants to set up something that's
> not offloadable, they'll have to:
>  - configure the offloaded version until EOPNOTSUPP
>  - tear everything down
>  - restart from scratch without offloading
> 
> That's inconvenient.
> 
> Talking about packet numbers, can you describe how PN exhaustion is
> handled?  I couldn't find much about packet numbers at all in the
> driver patches (I hope the hw doesn't wrap around from 2^32-1 to 0 on
> the same SA).
New SA's are suppose to be installed ahead of time. The HW will automatic move
to the next SA and reset the PN.

> At some point userspace needs to know that we're
> getting close to 2^32 and that it's time to re-key.  Since the whole
> TX path of the software implementation is bypassed, it looks like the
> PN (as far as drivers/net/macsec.c is concerned) never increases, so
> userspace can't know when to negotiate a new SA.
> 
> > I'm not sure if we should allow to mix the implementations on a given
> > physical interface (by having two MACsec interfaces attached) as the
> > validation would be impossible to do (we would have no idea if a
> > packet was correctly handled by the offloading part or just not being
> > a MACsec packet in the first place, in Rx).
> 
> That's something that really bothers me with this proposal. Quoting
> from the commit message:
> 
> > the packets seen by the networking stack on both the physical and
> > MACsec virtual interface are exactly the same
> 
> If the HW/driver is expected to strip the sectag, I don't see how we
> could ever have multiple secy's at the same time and demultiplex
> properly between them. That's part of the reason why I chose to have
> virtual interfaces: without them, picking the right secy on TX gets
> weird.

The HW does frame clasification, and use the claisfication to associate frames
to a given secy.

We we in SW have eth0, with 2 vlan-sub interfaces, and enable macsec on those,
then we have:

eth0
eth0.10
eth0.10.macsec
eth0.20
eth0.20.macsec

In this case the HW needs to be configured to match vlan 10 to one secy, and
vlan 20 to an other one.

This is nor supported in the current patch, but is something we can add later.
We just wanted to get the basic functionallity done right before moving on to
this.

But in the current design, there is nothing that prevent us from adding this.

If anyone is interested in the details of this then it is described in section
3.6.3 in http://ww1.microchip.com/downloads/en/DeviceDoc/VMDS-10455.pdf

It is possible to construct an encapsulation that the HW can not classify
correctly. If that is the case, then we should reject the HW offload, and use
the SW.

But it is a good point, which I think is missing. We should properly reject
MACsec HW offload (with this driver, in this state) on virtual interfaces, if
the encapsulation can not be handled (could start by reject all virtual
interfaces)

> AFAICT, it means that any filters (tc, tcpdump) need to be different
> between offloaded and non-offloaded cases.
It will see the result of the offloaded operation. But I guess that this is no
different from when 'tc' operations are offloaded to HW.

> How does the driver distinguish traffic that should pass through unmodified
> from traffic that the HW needs to encapsulate and encrypt?
It relay on frame classification (I think Antoine is missing a flow
configuration to bypass all MKA traffic).

> If you look at IPsec offloading, the networking stack builds up the
> ESP header, and passes the unencrypted data down to the driver. I'm
> wondering if the same would be possible with MACsec offloading: the
> macsec virtual interface adds the header (and maybe a dummy ICV), and
> then the HW does the encryption. In case of HW that needs to add the
> sectag itself, the driver would first strip the headers that the stack
> created. On receive, the driver would recreate a sectag and the macsec
> interface would just skip all verification (decrypt, PN).
I do not think this is possible with this HW, nor do I think this is desirable.

One of the big differences between MACsec and IPsec, is the fact that it is a L2
protocol, designed to be running on switches (this is how MACsec claims to limit
key-distributions issues, as all frames are decrypted when entering a switch,
and encrypted with a new key when leaving).

If a SW stack needs to add a tag to the frame, then we cannot offload traffic
being bridged in HW, and never goes to the CPU.

/Allan

  parent reply	other threads:[~2019-08-21  9:24 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-08 14:05 [PATCH net-next v2 0/9] net: macsec: initial support for hardware offloading Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 1/9] net: introduce the MACSEC netdev feature Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 2/9] net: macsec: move some definitions in a dedicated header Antoine Tenart
2019-08-10 12:19   ` Igor Russkikh
2019-08-12  8:17     ` Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 3/9] net: macsec: introduce the macsec_context structure Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 4/9] net: introduce MACsec ops and add a reference in net_device Antoine Tenart
2019-08-09 20:35   ` Jakub Kicinski
2019-08-12  8:18     ` Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 5/9] net: phy: add MACsec ops in phy_device Antoine Tenart
2019-08-14 23:15   ` Florian Fainelli
2019-08-20 10:07     ` Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 6/9] net: macsec: hardware offloading infrastructure Antoine Tenart
2019-08-10 13:20   ` Igor Russkikh
2019-08-13  8:58     ` Antoine Tenart
2019-08-13 13:17       ` Andrew Lunn
2019-08-13 16:18         ` Igor Russkikh
2019-08-13 16:28           ` Andrew Lunn
2019-08-14  8:32             ` Antoine Tenart
2019-08-14 23:28             ` Florian Fainelli
2019-08-16 13:26             ` Sabrina Dubroca
2019-08-20 10:03             ` Antoine Tenart
2019-08-14  8:31           ` Antoine Tenart
2019-08-16 13:29           ` Sabrina Dubroca
2019-08-20 10:01             ` Antoine Tenart
2019-08-20 14:41               ` Sabrina Dubroca
2019-08-21  0:01                 ` Andrew Lunn
2019-08-21  9:20                 ` Igor Russkikh
2019-08-21  9:27                   ` allan.nielsen
2019-08-21  9:24                 ` allan.nielsen [this message]
2019-08-21 10:01                 ` Antoine Tenart
2019-08-21 12:01                   ` Igor Russkikh
2019-08-16 13:25       ` Sabrina Dubroca
2019-08-20 10:07         ` Antoine Tenart
2019-08-10 16:34   ` Andrew Lunn
2019-08-12  8:15     ` Antoine Tenart
2019-08-13 11:46     ` Igor Russkikh
2019-08-08 14:05 ` [PATCH net-next v2 7/9] net: phy: export __phy_read_page/__phy_write_page Antoine Tenart
2019-08-08 14:05 ` [PATCH net-next v2 8/9] net: phy: mscc: macsec initialization Antoine Tenart
2019-08-10 16:53   ` Andrew Lunn
2019-08-12  8:12     ` Antoine Tenart
2019-08-08 14:06 ` [PATCH net-next v2 9/9] net: phy: mscc: macsec support Antoine Tenart
2019-08-09 11:23 ` [PATCH net-next v2 0/9] net: macsec: initial support for hardware offloading Allan W. Nielsen
2019-08-09 11:40   ` Antoine Tenart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190821092406.p66uto4kuozakels@lx-anielsen.microsemi.net \
    --to=allan.nielsen@microchip.com \
    --cc=Igor.Russkikh@aquantia.com \
    --cc=Pavel.Belous@aquantia.com \
    --cc=Simon.Edelhaus@aquantia.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=antoine.tenart@bootlin.com \
    --cc=camelia.groza@nxp.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sd@queasysnail.net \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).