[PATCH iproute2-next 0/2] macsec: add offloading support

[PATCH iproute2-next 0/2] macsec: add offloading support

[Antoine Tenart]

Hello,

This series adds support for selecting and reporting the offloading mode
of a MACsec interface. Available modes are for now 'off' and 'phy',
'off' being the default when an interface is created. Modes are not only
'off' and 'on' as the MACsec operations can be offloaded to multiple
kinds of specialized hardware devices, at least to PHYs and Ethernet
MACs. The later isn't currently supported in the kernel though.

The first patch adds support for reporting the offloading mode currently
selected for a given MACsec interface through the `ip macsec show`
command:

   # ip macsec show
   18: macsec0: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay off
       cipher suite: GCM-AES-128, using ICV length 16
       TXSC: 3e5035b67c860001 on SA 0
           0: PN 1, state on, key 00000000000000000000000000000000
       RXSC: b4969112700f0001, state on
           0: PN 1, state on, key 01000000000000000000000000000000
->     offload: phy
   19: macsec1: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay off
       cipher suite: GCM-AES-128, using ICV length 16
       TXSC: 3e5035b67c880001 on SA 0
           1: PN 1, state on, key 00000000000000000000000000000000
       RXSC: b4969112700f0001, state on
           1: PN 1, state on, key 01000000000000000000000000000000
->     offload: off

The second patch allows an user to change the offloading mode at runtime
through a new subcommand, `ip macsec offload`:

  # ip macsec offload macsec0 phy
  # ip macsec offload macsec0 off

If a mode isn't supported, `ip macsec offload` will report an issue
(-EOPNOTSUPP).

One thing not supported in this series would be the ability to list all
supported modes (for now 'off' and 'phy') depending on the h/w interface
capabilities. This can come up in a later patch, as this is not critical
to get the feature used, but I would like this to be compatible with the
current series. I can think of 2 possibilities: either through
`ip macsec show` or through `ip macsec offload` (for example when no
argument is given). What are your thoughts on this?

Thanks!
Antoine

Antoine Tenart (2):
  macsec: report the offloading mode currently selected
  macsec: add support for changing the offloading mode

 ip/ipmacsec.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

-- 
2.24.1

[PATCH iproute2-next 1/2] macsec: report the offloading mode currently selected

[Antoine Tenart]

This patch adds support to report the MACsec offloading mode currently
being enabled, which as of now can either be 'off' or 'phy'. This
information is reported through the `ip macsec show` command:

  # ip macsec show
  18: macsec0: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay off
      cipher suite: GCM-AES-128, using ICV length 16
      TXSC: 3e5035b67c860001 on SA 0
          0: PN 1, state on, key 00000000000000000000000000000000
      RXSC: b4969112700f0001, state on
          0: PN 1, state on, key 01000000000000000000000000000000
      offload: phy
  19: macsec1: protect on validate strict sc off sa off encrypt on send_sci on end_station off scb off replay off
      cipher suite: GCM-AES-128, using ICV length 16
      TXSC: 3e5035b67c880001 on SA 0
          1: PN 1, state on, key 00000000000000000000000000000000
      RXSC: b4969112700f0001, state on
          1: PN 1, state on, key 01000000000000000000000000000000
      offload: off

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 ip/ipmacsec.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index ad6ad7d6b79f..db7202ceb0a7 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -31,6 +31,11 @@ static const char * const validate_str[] = {
 	[MACSEC_VALIDATE_STRICT] = "strict",
 };
 
+static const char * const offload_str[] = {
+	[MACSEC_OFFLOAD_OFF] = "off",
+	[MACSEC_OFFLOAD_PHY] = "phy",
+};
+
 struct sci {
 	__u64 sci;
 	__u16 port;
@@ -997,6 +1002,19 @@ static int process(struct nlmsghdr *n, void *arg)
 	if (attrs[MACSEC_ATTR_RXSC_LIST])
 		print_rxsc_list(attrs[MACSEC_ATTR_RXSC_LIST]);
 
+	if (attrs[MACSEC_ATTR_OFFLOAD]) {
+		struct rtattr *attrs_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
+		__u8 offload;
+
+		parse_rtattr_nested(attrs_offload, MACSEC_OFFLOAD_ATTR_MAX,
+				    attrs[MACSEC_ATTR_OFFLOAD]);
+
+		offload = rta_getattr_u8(attrs_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
+		print_string(PRINT_ANY, "offload",
+			     "    offload: %s ", offload_str[offload]);
+		print_nl();
+	}
+
 	close_json_object();
 
 	return 0;
-- 
2.24.1

[PATCH iproute2-next 2/2] macsec: add support for changing the offloading mode

[Antoine Tenart]

MacSEC can now be offloaded to specialized hardware devices. Offloading
is off by default when creating a new MACsec interface, but the mode can
be updated at runtime. This patch adds a new subcommand,
`ip macsec offload`, to allow users to select the offloading mode of a
MACsec interface. It takes the mode to switch to as an argument, which
can for now either be 'off' or 'phy':

  # ip macsec offload macsec0 phy
  # ip macsec offload macsec0 off

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 ip/ipmacsec.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index db7202ceb0a7..28272bc25a19 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -98,6 +98,7 @@ static void ipmacsec_usage(void)
 		"       ip macsec del DEV rx SCI sa { 0..3 }\n"
 		"       ip macsec show\n"
 		"       ip macsec show DEV\n"
+		"       ip macsec offload DEV [ off | phy ]\n"
 		"where  OPTS := [ pn <u32> ] [ on | off ]\n"
 		"       ID   := 128-bit hex string\n"
 		"       KEY  := 128-bit or 256-bit hex string\n"
@@ -359,6 +360,7 @@ enum cmd {
 	CMD_ADD,
 	CMD_DEL,
 	CMD_UPD,
+	CMD_OFFLOAD,
 	__CMD_MAX
 };
 
@@ -375,6 +377,9 @@ static const enum macsec_nl_commands macsec_commands[__CMD_MAX][2][2] = {
 		[0] = {-1, MACSEC_CMD_DEL_RXSC},
 		[1] = {MACSEC_CMD_DEL_TXSA, MACSEC_CMD_DEL_RXSA},
 	},
+	[CMD_OFFLOAD] = {
+		[0] = {-1, MACSEC_CMD_UPD_OFFLOAD },
+	},
 };
 
 static int do_modify_nl(enum cmd c, enum macsec_nl_commands cmd, int ifindex,
@@ -534,6 +539,44 @@ static int do_modify(enum cmd c, int argc, char **argv)
 	return -1;
 }
 
+static int do_offload(enum cmd c, int argc, char **argv)
+{
+	enum macsec_offload offload;
+	struct rtattr *attr;
+	int ifindex, ret;
+
+	if (argc == 0)
+		ipmacsec_usage();
+
+	ifindex = ll_name_to_index(*argv);
+	if (!ifindex) {
+		fprintf(stderr, "Device \"%s\" does not exist.\n", *argv);
+		return -1;
+	}
+	argc--; argv++;
+
+	if (argc == 0)
+		ipmacsec_usage();
+
+	ret = one_of("offload", *argv, offload_str, ARRAY_SIZE(offload_str),
+		     (int *)&offload);
+	if (ret)
+		ipmacsec_usage();
+
+	MACSEC_GENL_REQ(req, MACSEC_BUFLEN, macsec_commands[c][0][1], NLM_F_REQUEST);
+
+	addattr32(&req.n, MACSEC_BUFLEN, MACSEC_ATTR_IFINDEX, ifindex);
+
+	attr = addattr_nest(&req.n, MACSEC_BUFLEN, MACSEC_ATTR_OFFLOAD);
+	addattr8(&req.n, MACSEC_BUFLEN, MACSEC_OFFLOAD_ATTR_TYPE, offload);
+	addattr_nest_end(&req.n, attr);
+
+	if (rtnl_talk(&genl_rth, &req.n, NULL) < 0)
+		return -2;
+
+	return 0;
+}
+
 /* dump/show */
 static struct {
 	int ifindex;
@@ -1086,6 +1129,8 @@ int do_ipmacsec(int argc, char **argv)
 		return do_modify(CMD_UPD, argc-1, argv+1);
 	if (matches(*argv, "delete") == 0)
 		return do_modify(CMD_DEL, argc-1, argv+1);
+	if (matches(*argv, "offload") == 0)
+		return do_offload(CMD_OFFLOAD, argc-1, argv+1);
 
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip macsec help\".\n",
 		*argv);
-- 
2.24.1

Re: [PATCH iproute2-next 1/2] macsec: report the offloading mode currently selected

[David Ahern]

On 1/20/20 1:18 PM, Antoine Tenart wrote:
> @@ -997,6 +1002,19 @@ static int process(struct nlmsghdr *n, void *arg)
>  	if (attrs[MACSEC_ATTR_RXSC_LIST])
>  		print_rxsc_list(attrs[MACSEC_ATTR_RXSC_LIST]);
>  
> +	if (attrs[MACSEC_ATTR_OFFLOAD]) {
> +		struct rtattr *attrs_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
> +		__u8 offload;
> +
> +		parse_rtattr_nested(attrs_offload, MACSEC_OFFLOAD_ATTR_MAX,
> +				    attrs[MACSEC_ATTR_OFFLOAD]);
> +
> +		offload = rta_getattr_u8(attrs_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
> +		print_string(PRINT_ANY, "offload",
> +			     "    offload: %s ", offload_str[offload]);

you should be an accessor around offload_str[offload] to handle a future
change adding a new type.

Re: [PATCH iproute2-next 2/2] macsec: add support for changing the offloading mode

[David Ahern]

On 1/20/20 1:18 PM, Antoine Tenart wrote:
> MacSEC can now be offloaded to specialized hardware devices. Offloading
> is off by default when creating a new MACsec interface, but the mode can
> be updated at runtime. This patch adds a new subcommand,
> `ip macsec offload`, to allow users to select the offloading mode of a
> MACsec interface. It takes the mode to switch to as an argument, which
> can for now either be 'off' or 'phy':
> 
>   # ip macsec offload macsec0 phy
>   # ip macsec offload macsec0 off

seems like this should fall under 'ip macsec set ...'

Sabrina: thoughts?

Re: [PATCH iproute2-next 2/2] macsec: add support for changing the offloading mode

[Sabrina Dubroca]

2020-01-27, 09:44:09 -0700, David Ahern wrote:
> On 1/20/20 1:18 PM, Antoine Tenart wrote:
> > MacSEC can now be offloaded to specialized hardware devices. Offloading
> > is off by default when creating a new MACsec interface, but the mode can
> > be updated at runtime. This patch adds a new subcommand,
> > `ip macsec offload`, to allow users to select the offloading mode of a
> > MACsec interface. It takes the mode to switch to as an argument, which
> > can for now either be 'off' or 'phy':
> > 
> >   # ip macsec offload macsec0 phy
> >   # ip macsec offload macsec0 off
> 
> seems like this should fall under 'ip macsec set ...'
> 
> Sabrina: thoughts?

The difference is that the other "set" commands also have an
"add"/"del" counterpart. "offload" would only have "set", so that
would be a bit inconsistent. Either way seems acceptable.

Another possibility is to see offloading as a property of the macsec
interface. Then it could be set on creation (ip link add ... type
macsec offload phy), or modified by link change, like other
device-wide properties (say, icvlen). But then I guess the netlink API
would need to be different... In that case, the "offload: X" line of
the output should also be integrated with the other device properties
(icvlen etc).

-- 
Sabrina

Re: [PATCH iproute2-next 0/2] macsec: add offloading support

[Sabrina Dubroca]

2020-01-20, 21:18:21 +0100, Antoine Tenart wrote:
> If a mode isn't supported, `ip macsec offload` will report an issue
> (-EOPNOTSUPP).
> 
> One thing not supported in this series would be the ability to list all
> supported modes (for now 'off' and 'phy') depending on the h/w interface
> capabilities. This can come up in a later patch, as this is not critical
> to get the feature used, but I would like this to be compatible with the
> current series. I can think of 2 possibilities: either through
> `ip macsec show` or through `ip macsec offload` (for example when no
> argument is given). What are your thoughts on this?

I don't think that's really helpful. The device could change between
listing available modes and enabling offloading. The failure of "ip
macsec offload blah" (or whatever the command ends up being) will do
the same job anyway.

-- 
Sabrina

Re: [PATCH iproute2-next 1/2] macsec: report the offloading mode currently selected

[Antoine Tenart]

Hello David,

On Mon, Jan 27, 2020 at 09:41:21AM -0700, David Ahern wrote:
> On 1/20/20 1:18 PM, Antoine Tenart wrote:
> > @@ -997,6 +1002,19 @@ static int process(struct nlmsghdr *n, void *arg)
> >  	if (attrs[MACSEC_ATTR_RXSC_LIST])
> >  		print_rxsc_list(attrs[MACSEC_ATTR_RXSC_LIST]);
> >  
> > +	if (attrs[MACSEC_ATTR_OFFLOAD]) {
> > +		struct rtattr *attrs_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
> > +		__u8 offload;
> > +
> > +		parse_rtattr_nested(attrs_offload, MACSEC_OFFLOAD_ATTR_MAX,
> > +				    attrs[MACSEC_ATTR_OFFLOAD]);
> > +
> > +		offload = rta_getattr_u8(attrs_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
> > +		print_string(PRINT_ANY, "offload",
> > +			     "    offload: %s ", offload_str[offload]);
> 
> you should be an accessor around offload_str[offload] to handle a future
> change adding a new type.

Good idea, I'll do that.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com