All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH main v2 1/3] macsec: add option to pass flag list to ip link add command
@ 2022-08-02  6:18 ehakim
  2022-08-02  6:18 ` [PATCH main v2 2/3] macsec: add Extended Packet Number support ehakim
  2022-08-02  6:18 ` [PATCH main v2 3/3] macsec: add user manual description for extended packet number feature ehakim
  0 siblings, 2 replies; 7+ messages in thread
From: ehakim @ 2022-08-02  6:18 UTC (permalink / raw)
  To: dsahern, netdev; +Cc: raeds, tariqt, sd, Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

This patch introduces a new flag list option to ip link add
command using type macsec, the patch prepares a framework for
passing and parsing flag list for future features like macsec
extended packet number (XPN) to use.

Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
 ip/ipmacsec.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index bf48e8b5..9aeaafcc 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -1256,9 +1256,28 @@ static void usage(FILE *f)
 		"                  [ validate { strict | check | disabled } ]\n"
 		"                  [ encodingsa { 0..3 } ]\n"
 		"                  [ offload { mac | phy | off } ]\n"
+		"                  [ flag FLAG-LIST ]\n"
+		"FLAG-LIST :=      [ FLAG-LIST ] FLAG\n"
+		"FLAG :=\n"
 		);
 }
 
+static int macsec_flag_parse(__u8 *flags, int *argcp, char ***argvp)
+{
+	int argc = *argcp;
+	char **argv = *argvp;
+
+	while (1) {
+		/* parse flag list */
+		break;
+	}
+
+	*argcp = argc;
+	*argvp = argv;
+
+	return 0;
+}
+
 static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			    struct nlmsghdr *n)
 {
@@ -1271,6 +1290,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 	bool es = false, scb = false, send_sci = false;
 	int replay_protect = -1;
 	struct sci sci = { 0 };
+	__u8 flags = 0;
 
 	ret = get_sci_portaddr(&sci, &argc, &argv, true, true);
 	if (ret < 0) {
@@ -1388,6 +1408,9 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 				return ret;
 			addattr8(n, MACSEC_BUFLEN,
 				 IFLA_MACSEC_OFFLOAD, offload);
+		} else if (strcmp(*argv, "flag") == 0) {
+			NEXT_ARG();
+			macsec_flag_parse(&flags, &argc, &argv);
 		} else {
 			fprintf(stderr, "macsec: unknown command \"%s\"?\n",
 				*argv);
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH main v2 2/3] macsec: add Extended Packet Number support
  2022-08-02  6:18 [PATCH main v2 1/3] macsec: add option to pass flag list to ip link add command ehakim
@ 2022-08-02  6:18 ` ehakim
  2022-08-04 16:48   ` Sabrina Dubroca
  2022-08-02  6:18 ` [PATCH main v2 3/3] macsec: add user manual description for extended packet number feature ehakim
  1 sibling, 1 reply; 7+ messages in thread
From: ehakim @ 2022-08-02  6:18 UTC (permalink / raw)
  To: dsahern, netdev; +Cc: raeds, tariqt, sd, Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

This patch adds support for extended packet number (XPN).
XPN can be configured by passing 'xpn' as flag in the ip
link add command using macsec type, in addition using
'xpn' keyword instead of the 'pn' and passing a 12 bytes salt
using the 'salt' keyword as part of the ip macsec command is
also required (see example).
XPN uses a 32 bit short secure channel id (ssci) instead of
the 64 bit secure channel id (sci) for the initialization vector
(IV) calculation, this patch also allows the user to optionally
pass his own ssci using the 'ssci' keyword in the ip macsec
command, in case of non provided ssci, the value 1 will be passed
as the default ssci value.

e.g:

create a MACsec device on link eth0 with enabled xpn
  # ip link add link eth0 macsec0 type macsec port 11
	encrypt on flag xpn

configure a secure association on the device (here ssci is default)
  # ip macsec add macsec0 tx sa 0 xpn 1024
	salt 838383838383838383838383 on
	key 01 81818181818181818181818181818181

configure a secure association on the device with ssci = 5
  # ip macsec add macsec0 tx sa 0 xpn 1024 ssci 5
	salt 838383838383838383838383 on
	key 01 81818181818181818181818181818181

Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
 include/uapi/linux/if_macsec.h |   2 +
 ip/ipmacsec.c                  | 117 +++++++++++++++++++++++++++++----
 2 files changed, 108 insertions(+), 11 deletions(-)

diff --git a/include/uapi/linux/if_macsec.h b/include/uapi/linux/if_macsec.h
index eee31cec..6edfea0a 100644
--- a/include/uapi/linux/if_macsec.h
+++ b/include/uapi/linux/if_macsec.h
@@ -22,6 +22,8 @@
 
 #define MACSEC_KEYID_LEN 16
 
+#define MACSEC_SALT_LEN 12
+
 /* cipher IDs as per IEEE802.1AE-2018 (Table 14-1) */
 #define MACSEC_CIPHER_ID_GCM_AES_128 0x0080C20001000001ULL
 #define MACSEC_CIPHER_ID_GCM_AES_256 0x0080C20001000002ULL
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index 9aeaafcc..54ab5f39 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -41,13 +41,33 @@ struct sci {
 	char abuf[6];
 };
 
+union __pn {
+	struct {
+#  if __BYTE_ORDER == __LITTLE_ENDIAN
+		__u32 lower;
+		__u32 upper;
+#endif
+# if __BYTE_ORDER == __BIG_ENDIAN
+		__u32 upper;
+		__u32 lower;
+#endif
+# if __BYTE_ORDER != __BIG_ENDIAN && __BYTE_ORDER != __LITTLE_ENDIAN
+#error  "Please fix byteorder defines"
+#endif
+	};
+	__u64 full64;
+};
+
 struct sa_desc {
 	__u8 an;
-	__u32 pn;
+	union __pn pn;
 	__u8 key_id[MACSEC_KEYID_LEN];
 	__u32 key_len;
 	__u8 key[MACSEC_MAX_KEY_LEN];
 	__u8 active;
+	__u8 salt[MACSEC_SALT_LEN];
+	__u32 ssci;
+	bool xpn;
 };
 
 struct cipher_args {
@@ -71,8 +91,14 @@ struct rxsc_desc {
 	__u8 active;
 };
 
+/* masks to set a bit in flags, hence to set bit i in flags,
+ * we need to use and bitwise with the value 2^(i-1)
+ */
+#define MACSEC_FLAGS_XPN 1
+
 #define MACSEC_BUFLEN 1024
 
+#define DEFAULT_SSCI 1
 
 /* netlink socket */
 static struct rtnl_handle genl_rth;
@@ -98,7 +124,7 @@ static void ipmacsec_usage(void)
 		"       ip macsec show\n"
 		"       ip macsec show DEV\n"
 		"       ip macsec offload DEV [ off | phy | mac ]\n"
-		"where  OPTS := [ pn <u32> ] [ on | off ]\n"
+		"where  OPTS := [ pn <u32> ] [ xpn <u64> ] [ salt <u96> ] [ ssci <u32> ] [ on | off ]\n"
 		"       ID   := 128-bit hex string\n"
 		"       KEY  := 128-bit or 256-bit hex string\n"
 		"       SCI  := { sci <u64> | port { 1..2^16-1 } address <lladdr> }\n");
@@ -174,14 +200,34 @@ static int parse_sa_args(int *argcp, char ***argvp, struct sa_desc *sa)
 
 	while (argc > 0) {
 		if (strcmp(*argv, "pn") == 0) {
-			if (sa->pn != 0)
+			if (sa->pn.lower != 0)
 				duparg2("pn", "pn");
 			NEXT_ARG();
-			ret = get_u32(&sa->pn, *argv, 0);
+			ret = get_u32(&sa->pn.lower, *argv, 0);
+			if (ret)
+				invarg("expected pn", *argv);
+			if (sa->pn.lower == 0)
+				invarg("expected pn != 0", *argv);
+		} else if (strcmp(*argv, "xpn") == 0) {
+			if (sa->pn.full64 != 0)
+				duparg2("xpn", "xpn");
+			NEXT_ARG();
+			ret = get_u64(&sa->pn.full64, *argv, 0);
 			if (ret)
 				invarg("expected pn", *argv);
-			if (sa->pn == 0)
+			if (sa->pn.full64 == 0)
 				invarg("expected pn != 0", *argv);
+			sa->xpn = true;
+		} else if (strcmp(*argv, "salt") == 0) {
+			unsigned int len;
+
+			NEXT_ARG();
+			if (!hexstring_a2n(*argv, sa->salt, MACSEC_SALT_LEN,
+					   &len))
+				invarg("expected salt", *argv);
+		} else if (strcmp(*argv, "ssci") == 0) {
+			NEXT_ARG();
+			ret = get_u32(&sa->ssci, *argv, 0);
 		} else if (strcmp(*argv, "key") == 0) {
 			unsigned int len;
 
@@ -392,9 +438,29 @@ static int do_modify_nl(enum cmd c, enum macsec_nl_commands cmd, int ifindex,
 	addattr8(&req.n, MACSEC_BUFLEN, MACSEC_SA_ATTR_AN, sa->an);
 
 	if (c != CMD_DEL) {
-		if (sa->pn)
+		if (sa->xpn) {
+			if (sa->pn.full64)
+				addattr64(&req.n, MACSEC_BUFLEN,
+					  MACSEC_SA_ATTR_PN, sa->pn.full64);
+			if (c == CMD_ADD) {
+				addattr_l(&req.n, MACSEC_BUFLEN,
+					  MACSEC_SA_ATTR_SALT,
+					  sa->salt, MACSEC_SALT_LEN);
+				if (sa->ssci != 0)
+					addattr32(&req.n, MACSEC_BUFLEN,
+						  MACSEC_SA_ATTR_SSCI,
+						  sa->ssci);
+				else
+					addattr32(&req.n, MACSEC_BUFLEN,
+						  MACSEC_SA_ATTR_SSCI,
+						  DEFAULT_SSCI);
+			}
+		}
+
+		if (sa->pn.lower && !sa->xpn) {
 			addattr32(&req.n, MACSEC_BUFLEN, MACSEC_SA_ATTR_PN,
-				  sa->pn);
+				  sa->pn.lower);
+		}
 
 		if (sa->key_len) {
 			addattr_l(&req.n, MACSEC_BUFLEN, MACSEC_SA_ATTR_KEYID,
@@ -426,10 +492,17 @@ static bool check_sa_args(enum cmd c, struct sa_desc *sa)
 			return -1;
 		}
 
-		if (sa->pn == 0) {
+		if (sa->pn.full64 == 0) {
 			fprintf(stderr, "must specify a packet number != 0\n");
 			return -1;
 		}
+
+		if (sa->xpn && sa->salt[0] == '\0') {
+			fprintf(stderr,
+				"xpn set, but no salt set.\n");
+			return -1;
+		}
+
 	} else if (c == CMD_UPD) {
 		if (sa->key_len) {
 			fprintf(stderr, "cannot change key on SA\n");
@@ -615,6 +688,9 @@ static void print_key(struct rtattr *key)
 
 #define CIPHER_NAME_GCM_AES_128 "GCM-AES-128"
 #define CIPHER_NAME_GCM_AES_256 "GCM-AES-256"
+#define CIPHER_NAME_GCM_AES_XPN_128 "GCM-AES-XPN-128"
+#define CIPHER_NAME_GCM_AES_XPN_256 "GCM-AES-XPN-256"
+
 #define DEFAULT_CIPHER_NAME CIPHER_NAME_GCM_AES_128
 
 static const char *cs_id_to_name(__u64 cid)
@@ -627,6 +703,10 @@ static const char *cs_id_to_name(__u64 cid)
 		return CIPHER_NAME_GCM_AES_128;
 	case MACSEC_CIPHER_ID_GCM_AES_256:
 		return CIPHER_NAME_GCM_AES_256;
+	case MACSEC_CIPHER_ID_GCM_AES_XPN_128:
+		return CIPHER_NAME_GCM_AES_XPN_128;
+	case MACSEC_CIPHER_ID_GCM_AES_XPN_256:
+		return CIPHER_NAME_GCM_AES_XPN_256;
 	default:
 		return "(unknown)";
 	}
@@ -1258,7 +1338,7 @@ static void usage(FILE *f)
 		"                  [ offload { mac | phy | off } ]\n"
 		"                  [ flag FLAG-LIST ]\n"
 		"FLAG-LIST :=      [ FLAG-LIST ] FLAG\n"
-		"FLAG :=\n"
+		"FLAG :=           xpn\n"
 		);
 }
 
@@ -1268,8 +1348,16 @@ static int macsec_flag_parse(__u8 *flags, int *argcp, char ***argvp)
 	char **argv = *argvp;
 
 	while (1) {
-		/* parse flag list */
-		break;
+		if (strcmp(*argv, "xpn") == 0) {
+			*flags |= MACSEC_FLAGS_XPN;
+		} else {
+			PREV_ARG(); /* back track */
+			break;
+		}
+
+		if (!NEXT_ARG_OK())
+			break;
+		NEXT_ARG();
 	}
 
 	*argcp = argc;
@@ -1438,6 +1526,13 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 		return -1;
 	}
 
+	if (flags & MACSEC_FLAGS_XPN) {
+		if (cipher.id == MACSEC_CIPHER_ID_GCM_AES_256)
+			cipher.id = MACSEC_CIPHER_ID_GCM_AES_XPN_256;
+		else
+			cipher.id = MACSEC_CIPHER_ID_GCM_AES_XPN_128;
+	}
+
 	if (cipher.id)
 		addattr_l(n, MACSEC_BUFLEN, IFLA_MACSEC_CIPHER_SUITE,
 			  &cipher.id, sizeof(cipher.id));
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH main v2 3/3] macsec: add user manual description for extended packet number feature
  2022-08-02  6:18 [PATCH main v2 1/3] macsec: add option to pass flag list to ip link add command ehakim
  2022-08-02  6:18 ` [PATCH main v2 2/3] macsec: add Extended Packet Number support ehakim
@ 2022-08-02  6:18 ` ehakim
  1 sibling, 0 replies; 7+ messages in thread
From: ehakim @ 2022-08-02  6:18 UTC (permalink / raw)
  To: dsahern, netdev; +Cc: raeds, tariqt, sd, Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

Update the user manual describing how to use extended packet number (XPN)
feature for macsec. As part of configuring XPN, providing ssci and salt is
required hence update user manual on  how to provide the above as part of
the ip macsec command.

Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
 man/man8/ip-macsec.8 | 52 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/man/man8/ip-macsec.8 b/man/man8/ip-macsec.8
index bb816157..67bb2c23 100644
--- a/man/man8/ip-macsec.8
+++ b/man/man8/ip-macsec.8
@@ -24,6 +24,8 @@ ip-macsec \- MACsec device configuration
 .BR validate " { " strict " | " check " | " disabled " } ] ["
 .BI encodingsa " SA"
 ] [
+.BI flag " FLAG"
+] [
 .BR offload " { " off " | " phy " | " mac " }"
 ]
 
@@ -64,8 +66,17 @@ ip-macsec \- MACsec device configuration
 .IR OPTS " := [ "
 .BR pn " { "
 .IR 1..2^32-1 " } ] ["
+.BR xpn " { "
+.IR 1..2^64-1 " } ] ["
+.B salt
+.IR <u94> " ] ["
+.B ssci
+.IR <u32> " ] ["
 .BR on " | " off " ]"
 .br
+.IR FLAG " := "
+.BR xpn "
+.br
 .IR SCI " := { "
 .B sci
 .IR <u64> " | "
@@ -116,6 +127,29 @@ type.
 .nf
 # ip link add link eth0 macsec0 type macsec port 11 encrypt on offload mac
 
+.SH EXTENDED PACKET NUMBER EXAMPLES
+.PP
+.SS Create a MACsec device on link eth0 with enabled extended packet number (offload is disabled by default)
+.nf
+# ip link add link eth0 macsec0 type macsec port 11 encrypt on flag xpn
+.PP
+.SS Configure a secure association on that device
+.nf
+# ip macsec add macsec0 tx sa 0 xpn 1024 salt 838383838383838383838383 on key 01 81818181818181818181818181818181
+.PP
+.SS Configure a receive channel
+.nf
+# ip macsec add macsec0 rx port 1234 address c6:19:52:8f:e6:a0
+.PP
+.SS Configure a receive association
+.nf
+# ip macsec add macsec0 rx port 1234 address c6:19:52:8f:e6:a0 sa 0 xpn 1 salt 838383838383838383838383 on key 00 82828282828282828282828282828282
+.PP
+.SS Display MACsec configuration
+.nf
+# ip macsec show
+.PP
+
 .SH NOTES
 This tool can be used to configure the 802.1AE keys of the interface. Note that 802.1AE uses GCM-AES
 with a initialization vector (IV) derived from the packet number. The same key must not be used
@@ -125,6 +159,24 @@ that reconfigures the keys. It is wrong to just configure the keys statically an
 indefinitely. The suggested and standardized way for key management is 802.1X-2010, which is implemented
 by wpa_supplicant.
 
+.SH EXTENDED PACKET NUMBER NOTES
+Passing flag
+.B xpn
+to
+.B ip link add
+command using the
+.I macsec
+type requires using the keyword
+.B 'xpn'
+instead of
+.B 'pn'
+in addition to providing a salt using the
+.B 'salt'
+keyword when using the
+.B ip macsec
+command.
+
+
 .SH SEE ALSO
 .br
 .BR ip-link (8)
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH main v2 2/3] macsec: add Extended Packet Number support
  2022-08-02  6:18 ` [PATCH main v2 2/3] macsec: add Extended Packet Number support ehakim
@ 2022-08-04 16:48   ` Sabrina Dubroca
  2022-08-04 18:36     ` David Ahern
  0 siblings, 1 reply; 7+ messages in thread
From: Sabrina Dubroca @ 2022-08-04 16:48 UTC (permalink / raw)
  To: ehakim; +Cc: dsahern, netdev, raeds, tariqt

Hi Emeel,

2022-08-02, 09:18:12 +0300, ehakim@nvidia.com wrote:
> diff --git a/include/uapi/linux/if_macsec.h b/include/uapi/linux/if_macsec.h
> index eee31cec..6edfea0a 100644
> --- a/include/uapi/linux/if_macsec.h
> +++ b/include/uapi/linux/if_macsec.h
> @@ -22,6 +22,8 @@
>  
>  #define MACSEC_KEYID_LEN 16
>  
> +#define MACSEC_SALT_LEN 12

That's not in the kernel's uapi file (probably was forgotten), I
don't think we can just add it here.

[...]
> diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
> index 9aeaafcc..54ab5f39 100644
> --- a/ip/ipmacsec.c
> +++ b/ip/ipmacsec.c
> @@ -41,13 +41,33 @@ struct sci {
>  	char abuf[6];
>  };
>  
> +union __pn {
> +	struct {
> +#  if __BYTE_ORDER == __LITTLE_ENDIAN
> +		__u32 lower;
> +		__u32 upper;
> +#endif
> +# if __BYTE_ORDER == __BIG_ENDIAN
> +		__u32 upper;
> +		__u32 lower;
> +#endif
> +# if __BYTE_ORDER != __BIG_ENDIAN && __BYTE_ORDER != __LITTLE_ENDIAN
> +#error  "Please fix byteorder defines"
> +#endif
> +	};
> +	__u64 full64;
> +};

That's quite complicated and I don't really see the benefit,
especially given that upper isn't used at all here. I'd just put the
union straight in sa_desc:

>  struct sa_desc {
>  	__u8 an;
> -	__u32 pn;

+	union {
+		__u32 pn32;
+		__u64 pn64;
+	};

> +	union __pn pn;
>  	__u8 key_id[MACSEC_KEYID_LEN];
>  	__u32 key_len;
>  	__u8 key[MACSEC_MAX_KEY_LEN];
>  	__u8 active;
> +	__u8 salt[MACSEC_SALT_LEN];
> +	__u32 ssci;
> +	bool xpn;
>  };

[...]
> @@ -98,7 +124,7 @@ static void ipmacsec_usage(void)
>  		"       ip macsec show\n"
>  		"       ip macsec show DEV\n"
>  		"       ip macsec offload DEV [ off | phy | mac ]\n"
> -		"where  OPTS := [ pn <u32> ] [ on | off ]\n"
> +		"where  OPTS := [ pn <u32> ] [ xpn <u64> ] [ salt <u96> ] [ ssci <u32> ] [ on | off ]\n"

Only one of pn and xpn can be set, so that should be
	[ pn <u32> | pn64 <u64> ]

And salt is a hex string like key/keyid (it doesn't take the 0x
prefix).


[...]
> @@ -392,9 +438,29 @@ static int do_modify_nl(enum cmd c, enum macsec_nl_commands cmd, int ifindex,
>  	addattr8(&req.n, MACSEC_BUFLEN, MACSEC_SA_ATTR_AN, sa->an);
>  
>  	if (c != CMD_DEL) {
> -		if (sa->pn)
> +		if (sa->xpn) {
> +			if (sa->pn.full64)
> +				addattr64(&req.n, MACSEC_BUFLEN,
> +					  MACSEC_SA_ATTR_PN, sa->pn.full64);
> +			if (c == CMD_ADD) {
> +				addattr_l(&req.n, MACSEC_BUFLEN,
> +					  MACSEC_SA_ATTR_SALT,
> +					  sa->salt, MACSEC_SALT_LEN);
> +				if (sa->ssci != 0)
> +					addattr32(&req.n, MACSEC_BUFLEN,
> +						  MACSEC_SA_ATTR_SSCI,
> +						  sa->ssci);
> +				else
> +					addattr32(&req.n, MACSEC_BUFLEN,
> +						  MACSEC_SA_ATTR_SSCI,
> +						  DEFAULT_SSCI);

I'd rather not add a default ssci at all. If the user didn't provide
it, don't add the attribute. That would allow us to test that part of
the uapi using iproute.

Same with the 'c == CMD_ADD' test: pass the attribute to the kernel if
they're provided, let the kernel decide.

[...]
> @@ -426,10 +492,17 @@ static bool check_sa_args(enum cmd c, struct sa_desc *sa)
>  			return -1;
>  		}
>  
> -		if (sa->pn == 0) {
> +		if (sa->pn.full64 == 0) {
>  			fprintf(stderr, "must specify a packet number != 0\n");
>  			return -1;
>  		}
> +
> +		if (sa->xpn && sa->salt[0] == '\0') {
> +			fprintf(stderr,
> +				"xpn set, but no salt set.\n");
> +			return -1;

I would also allow that to be empty, same as the ssci. Let the kernel
reject invalid requests.

> +		}
> +
>  	} else if (c == CMD_UPD) {
>  		if (sa->key_len) {
>  			fprintf(stderr, "cannot change key on SA\n");
[...]

> @@ -1268,8 +1348,16 @@ static int macsec_flag_parse(__u8 *flags, int *argcp, char ***argvp)
>  	char **argv = *argvp;
>  
>  	while (1) {
> -		/* parse flag list */
> -		break;
> +		if (strcmp(*argv, "xpn") == 0) {
> +			*flags |= MACSEC_FLAGS_XPN;
> +		} else {
> +			PREV_ARG(); /* back track */
> +			break;
> +		}
> +
> +		if (!NEXT_ARG_OK())
> +			break;
> +		NEXT_ARG();
>  	}

This whole thing looks a bit over-complicated to me. Why not just put
'bool xpn = false;' in macsec_parse_opt() and match "xpn" on its own
(without "flags" in front of it) at the same level as cipher, icvlen,
etc?



I don't see anything on the print side in your patch. PNs provided by
userspace can be 64b with XPN, and SSCIs are also part of the dump and
need to be handled.

-- 
Sabrina


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH main v2 2/3] macsec: add Extended Packet Number support
  2022-08-04 16:48   ` Sabrina Dubroca
@ 2022-08-04 18:36     ` David Ahern
  2022-08-07  7:01       ` Emeel Hakim
  0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2022-08-04 18:36 UTC (permalink / raw)
  To: Sabrina Dubroca, ehakim; +Cc: netdev, raeds, tariqt

On 8/4/22 10:48 AM, Sabrina Dubroca wrote:
> Hi Emeel,
> 
> 2022-08-02, 09:18:12 +0300, ehakim@nvidia.com wrote:
>> diff --git a/include/uapi/linux/if_macsec.h b/include/uapi/linux/if_macsec.h
>> index eee31cec..6edfea0a 100644
>> --- a/include/uapi/linux/if_macsec.h
>> +++ b/include/uapi/linux/if_macsec.h
>> @@ -22,6 +22,8 @@
>>  
>>  #define MACSEC_KEYID_LEN 16
>>  
>> +#define MACSEC_SALT_LEN 12
> 
> That's not in the kernel's uapi file (probably was forgotten), I
> don't think we can just add it here.
> 

can't. uapi files are synched with kernel releases, so that change would
disappear on the next sync.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH main v2 2/3] macsec: add Extended Packet Number support
  2022-08-04 18:36     ` David Ahern
@ 2022-08-07  7:01       ` Emeel Hakim
  2022-08-07  8:13         ` Tariq Toukan
  0 siblings, 1 reply; 7+ messages in thread
From: Emeel Hakim @ 2022-08-07  7:01 UTC (permalink / raw)
  To: David Ahern, Sabrina Dubroca; +Cc: netdev, Raed Salem, Tariq Toukan



> -----Original Message-----
> From: David Ahern <dsahern@kernel.org>
> Sent: Thursday, 4 August 2022 21:37
> To: Sabrina Dubroca <sd@queasysnail.net>; Emeel Hakim
> <ehakim@nvidia.com>
> Cc: netdev@vger.kernel.org; Raed Salem <raeds@nvidia.com>; Tariq Toukan
> <tariqt@nvidia.com>
> Subject: Re: [PATCH main v2 2/3] macsec: add Extended Packet Number
> support
> 
> External email: Use caution opening links or attachments
> 
> 
> On 8/4/22 10:48 AM, Sabrina Dubroca wrote:
> > Hi Emeel,
> >
> > 2022-08-02, 09:18:12 +0300, ehakim@nvidia.com wrote:
> >> diff --git a/include/uapi/linux/if_macsec.h
> >> b/include/uapi/linux/if_macsec.h index eee31cec..6edfea0a 100644
> >> --- a/include/uapi/linux/if_macsec.h
> >> +++ b/include/uapi/linux/if_macsec.h
> >> @@ -22,6 +22,8 @@
> >>
> >>  #define MACSEC_KEYID_LEN 16
> >>
> >> +#define MACSEC_SALT_LEN 12
> >
> > That's not in the kernel's uapi file (probably was forgotten), I don't
> > think we can just add it here.
> >
> 
> can't. uapi files are synched with kernel releases, so that change would
> disappear on the next sync.

ACK,
I can see that we have this define in the kernel
(not in a uapi file but as part of include/net/macsec.h), if I want to use
such a define what is the process here? do I need to move the define in
the kernel to the uapi file?
Also, in such a case Would those patches get accepted using such a define
while the kernel change is not accepted yet?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH main v2 2/3] macsec: add Extended Packet Number support
  2022-08-07  7:01       ` Emeel Hakim
@ 2022-08-07  8:13         ` Tariq Toukan
  0 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2022-08-07  8:13 UTC (permalink / raw)
  To: Emeel Hakim, David Ahern, Sabrina Dubroca
  Cc: netdev, Raed Salem, Tariq Toukan



On 8/7/2022 10:01 AM, Emeel Hakim wrote:
> 
> 
>> -----Original Message-----
>> From: David Ahern <dsahern@kernel.org>
>> Sent: Thursday, 4 August 2022 21:37
>> To: Sabrina Dubroca <sd@queasysnail.net>; Emeel Hakim
>> <ehakim@nvidia.com>
>> Cc: netdev@vger.kernel.org; Raed Salem <raeds@nvidia.com>; Tariq Toukan
>> <tariqt@nvidia.com>
>> Subject: Re: [PATCH main v2 2/3] macsec: add Extended Packet Number
>> support
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 8/4/22 10:48 AM, Sabrina Dubroca wrote:
>>> Hi Emeel,
>>>
>>> 2022-08-02, 09:18:12 +0300, ehakim@nvidia.com wrote:
>>>> diff --git a/include/uapi/linux/if_macsec.h
>>>> b/include/uapi/linux/if_macsec.h index eee31cec..6edfea0a 100644
>>>> --- a/include/uapi/linux/if_macsec.h
>>>> +++ b/include/uapi/linux/if_macsec.h
>>>> @@ -22,6 +22,8 @@
>>>>
>>>>   #define MACSEC_KEYID_LEN 16
>>>>
>>>> +#define MACSEC_SALT_LEN 12
>>>
>>> That's not in the kernel's uapi file (probably was forgotten), I don't
>>> think we can just add it here.
>>>
>>
>> can't. uapi files are synched with kernel releases, so that change would
>> disappear on the next sync.
> 
> ACK,
> I can see that we have this define in the kernel
> (not in a uapi file but as part of include/net/macsec.h), if I want to use
> such a define what is the process here? do I need to move the define in
> the kernel to the uapi file?
> Also, in such a case Would those patches get accepted using such a define
> while the kernel change is not accepted yet?

Emeel,
Please complete the kernel changes and get them accepted first.
If a define exists in the kernel and needs to be exposed to the user, 
just move it to the uapi file and make sure it's still visible to all 
kernel usages.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-07  8:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02  6:18 [PATCH main v2 1/3] macsec: add option to pass flag list to ip link add command ehakim
2022-08-02  6:18 ` [PATCH main v2 2/3] macsec: add Extended Packet Number support ehakim
2022-08-04 16:48   ` Sabrina Dubroca
2022-08-04 18:36     ` David Ahern
2022-08-07  7:01       ` Emeel Hakim
2022-08-07  8:13         ` Tariq Toukan
2022-08-02  6:18 ` [PATCH main v2 3/3] macsec: add user manual description for extended packet number feature ehakim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.