All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2-next] mptcp: add support for changing the backup flag
@ 2021-12-03 15:01 Davide Caratti
  2021-12-08  1:59 ` Mat Martineau
  0 siblings, 1 reply; 8+ messages in thread
From: Davide Caratti @ 2021-12-03 15:01 UTC (permalink / raw)
  To: mptcp

Linux supports 'MPTCP_PM_CMD_SET_FLAGS' since v5.12, and this control has
recently been extended to allow setting flags for a given endpoint id.
Although there is no use for changing 'signal' or 'subflow' flags, it can
be helpful to set/clear the backup bit on existing endpoints: add the 'ip
mptcp endpoint change <...>' command for this purpose.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 ip/ipmptcp.c        | 20 ++++++++++++++++----
 man/man8/ip-mptcp.8 | 14 ++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 857004446aa3..7fb48a420a6b 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -25,6 +25,7 @@ static void usage(void)
 		"Usage:	ip mptcp endpoint add ADDRESS [ dev NAME ] [ id ID ]\n"
 		"				      [ port NR ] [ FLAG-LIST ]\n"
 		"	ip mptcp endpoint delete id ID\n"
+		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
 		"	ip mptcp endpoint show [ id ID ]\n"
 		"	ip mptcp endpoint flush\n"
 		"	ip mptcp limits set [ subflows NR ] [ add_addr_accepted NR ]\n"
@@ -45,6 +46,8 @@ static int genl_family = -1;
 	GENL_REQUEST(_req, MPTCP_BUFLEN, genl_family, 0,	\
 		     MPTCP_PM_VER, _cmd, _flags)
 
+#define MPTCP_PM_ADDR_FLAG_NOBACKUP 0x0
+
 /* Mapping from argument to address flag mask */
 static const struct {
 	const char *name;
@@ -53,6 +56,7 @@ static const struct {
 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
+	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NOBACKUP }
 };
 
 static void print_mptcp_addr_flags(unsigned int flags)
@@ -95,9 +99,9 @@ static int get_flags(const char *arg, __u32 *flags)
 	return -1;
 }
 
-static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
-			 bool adding)
+static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 {
+	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
 	struct rtattr *attr_addr;
 	bool addr_set = false;
 	inet_prefix address;
@@ -110,6 +114,11 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
 	ll_init_map(&rth);
 	while (argc > 0) {
 		if (get_flags(*argv, &flags) == 0) {
+			/* allow changing the 'backup' flag only */
+			if (cmd == MPTCP_PM_CMD_SET_FLAGS &&
+			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP))
+				invarg("invalid flags\n", *argv);
+
 		} else if (matches(*argv, "id") == 0) {
 			NEXT_ARG();
 
@@ -182,7 +191,7 @@ static int mptcp_addr_modify(int argc, char **argv, int cmd)
 	MPTCP_REQUEST(req, cmd, NLM_F_REQUEST);
 	int ret;
 
-	ret = mptcp_parse_opt(argc, argv, &req.n, cmd == MPTCP_PM_CMD_ADD_ADDR);
+	ret = mptcp_parse_opt(argc, argv, &req.n, cmd);
 	if (ret)
 		return ret;
 
@@ -298,7 +307,7 @@ static int mptcp_addr_show(int argc, char **argv)
 	if (argc <= 0)
 		return mptcp_addr_dump();
 
-	ret = mptcp_parse_opt(argc, argv, &req.n, false);
+	ret = mptcp_parse_opt(argc, argv, &req.n, MPTCP_PM_CMD_GET_ADDR);
 	if (ret)
 		return ret;
 
@@ -534,6 +543,9 @@ int do_mptcp(int argc, char **argv)
 		if (matches(*argv, "add") == 0)
 			return mptcp_addr_modify(argc-1, argv+1,
 						 MPTCP_PM_CMD_ADD_ADDR);
+		if (matches(*argv, "change") == 0)
+			return mptcp_addr_modify(argc-1, argv+1,
+						 MPTCP_PM_CMD_SET_FLAGS);
 		if (matches(*argv, "delete") == 0)
 			return mptcp_addr_modify(argc-1, argv+1,
 						 MPTCP_PM_CMD_DEL_ADDR);
diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
index 22335b6129ae..574cb60dc7c0 100644
--- a/man/man8/ip-mptcp.8
+++ b/man/man8/ip-mptcp.8
@@ -34,6 +34,13 @@ ip-mptcp \- MPTCP path manager configuration
 .BR "ip mptcp endpoint del id "
 .I ID
 
+.ti -8
+.BR "ip mptcp endpoint change id "
+.I ID
+.RB "[ "
+.I BACKUP-OPT
+.RB "] "
+
 .ti -8
 .BR "ip mptcp endpoint show "
 .RB "[ " id
@@ -55,6 +62,13 @@ ip-mptcp \- MPTCP path manager configuration
 .B backup
 .RB  "]"
 
+.ti -8
+.IR BACKUP-OPT " := ["
+.B backup
+.RB "|"
+.B nobackup
+.RB  "]"
+
 .ti -8
 .BR "ip mptcp limits set "
 .RB "[ "
-- 
2.31.1


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

* Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag
  2021-12-03 15:01 [PATCH iproute2-next] mptcp: add support for changing the backup flag Davide Caratti
@ 2021-12-08  1:59 ` Mat Martineau
  2021-12-08 10:23   ` Matthieu Baerts
  0 siblings, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2021-12-08  1:59 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp

On Fri, 3 Dec 2021, Davide Caratti wrote:

> Linux supports 'MPTCP_PM_CMD_SET_FLAGS' since v5.12, and this control has
> recently been extended to allow setting flags for a given endpoint id.
> Although there is no use for changing 'signal' or 'subflow' flags, it can
> be helpful to set/clear the backup bit on existing endpoints: add the 'ip
> mptcp endpoint change <...>' command for this purpose.
>
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Matthieu -

Since you already Acked, what should be the status of this in patchwork?


-Mat



> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
> ip/ipmptcp.c        | 20 ++++++++++++++++----
> man/man8/ip-mptcp.8 | 14 ++++++++++++++
> 2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> index 857004446aa3..7fb48a420a6b 100644
> --- a/ip/ipmptcp.c
> +++ b/ip/ipmptcp.c
> @@ -25,6 +25,7 @@ static void usage(void)
> 		"Usage:	ip mptcp endpoint add ADDRESS [ dev NAME ] [ id ID ]\n"
> 		"				      [ port NR ] [ FLAG-LIST ]\n"
> 		"	ip mptcp endpoint delete id ID\n"
> +		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
> 		"	ip mptcp endpoint show [ id ID ]\n"
> 		"	ip mptcp endpoint flush\n"
> 		"	ip mptcp limits set [ subflows NR ] [ add_addr_accepted NR ]\n"
> @@ -45,6 +46,8 @@ static int genl_family = -1;
> 	GENL_REQUEST(_req, MPTCP_BUFLEN, genl_family, 0,	\
> 		     MPTCP_PM_VER, _cmd, _flags)
>
> +#define MPTCP_PM_ADDR_FLAG_NOBACKUP 0x0
> +
> /* Mapping from argument to address flag mask */
> static const struct {
> 	const char *name;
> @@ -53,6 +56,7 @@ static const struct {
> 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
> 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
> 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
> +	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NOBACKUP }
> };
>
> static void print_mptcp_addr_flags(unsigned int flags)
> @@ -95,9 +99,9 @@ static int get_flags(const char *arg, __u32 *flags)
> 	return -1;
> }
>
> -static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
> -			 bool adding)
> +static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
> {
> +	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
> 	struct rtattr *attr_addr;
> 	bool addr_set = false;
> 	inet_prefix address;
> @@ -110,6 +114,11 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
> 	ll_init_map(&rth);
> 	while (argc > 0) {
> 		if (get_flags(*argv, &flags) == 0) {
> +			/* allow changing the 'backup' flag only */
> +			if (cmd == MPTCP_PM_CMD_SET_FLAGS &&
> +			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP))
> +				invarg("invalid flags\n", *argv);
> +
> 		} else if (matches(*argv, "id") == 0) {
> 			NEXT_ARG();
>
> @@ -182,7 +191,7 @@ static int mptcp_addr_modify(int argc, char **argv, int cmd)
> 	MPTCP_REQUEST(req, cmd, NLM_F_REQUEST);
> 	int ret;
>
> -	ret = mptcp_parse_opt(argc, argv, &req.n, cmd == MPTCP_PM_CMD_ADD_ADDR);
> +	ret = mptcp_parse_opt(argc, argv, &req.n, cmd);
> 	if (ret)
> 		return ret;
>
> @@ -298,7 +307,7 @@ static int mptcp_addr_show(int argc, char **argv)
> 	if (argc <= 0)
> 		return mptcp_addr_dump();
>
> -	ret = mptcp_parse_opt(argc, argv, &req.n, false);
> +	ret = mptcp_parse_opt(argc, argv, &req.n, MPTCP_PM_CMD_GET_ADDR);
> 	if (ret)
> 		return ret;
>
> @@ -534,6 +543,9 @@ int do_mptcp(int argc, char **argv)
> 		if (matches(*argv, "add") == 0)
> 			return mptcp_addr_modify(argc-1, argv+1,
> 						 MPTCP_PM_CMD_ADD_ADDR);
> +		if (matches(*argv, "change") == 0)
> +			return mptcp_addr_modify(argc-1, argv+1,
> +						 MPTCP_PM_CMD_SET_FLAGS);
> 		if (matches(*argv, "delete") == 0)
> 			return mptcp_addr_modify(argc-1, argv+1,
> 						 MPTCP_PM_CMD_DEL_ADDR);
> diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
> index 22335b6129ae..574cb60dc7c0 100644
> --- a/man/man8/ip-mptcp.8
> +++ b/man/man8/ip-mptcp.8
> @@ -34,6 +34,13 @@ ip-mptcp \- MPTCP path manager configuration
> .BR "ip mptcp endpoint del id "
> .I ID
>
> +.ti -8
> +.BR "ip mptcp endpoint change id "
> +.I ID
> +.RB "[ "
> +.I BACKUP-OPT
> +.RB "] "
> +
> .ti -8
> .BR "ip mptcp endpoint show "
> .RB "[ " id
> @@ -55,6 +62,13 @@ ip-mptcp \- MPTCP path manager configuration
> .B backup
> .RB  "]"
>
> +.ti -8
> +.IR BACKUP-OPT " := ["
> +.B backup
> +.RB "|"
> +.B nobackup
> +.RB  "]"
> +
> .ti -8
> .BR "ip mptcp limits set "
> .RB "[ "
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

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

* Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag
  2021-12-08  1:59 ` Mat Martineau
@ 2021-12-08 10:23   ` Matthieu Baerts
  2021-12-08 23:19     ` Mat Martineau
  0 siblings, 1 reply; 8+ messages in thread
From: Matthieu Baerts @ 2021-12-08 10:23 UTC (permalink / raw)
  To: Mat Martineau, Davide Caratti; +Cc: mptcp

Hi Mat, Davide

On 08/12/2021 02:59, Mat Martineau wrote:
> On Fri, 3 Dec 2021, Davide Caratti wrote:
> 
>> Linux supports 'MPTCP_PM_CMD_SET_FLAGS' since v5.12, and this control has
>> recently been extended to allow setting flags for a given endpoint id.
>> Although there is no use for changing 'signal' or 'subflow' flags, it can
>> be helpful to set/clear the backup bit on existing endpoints: add the 'ip
>> mptcp endpoint change <...>' command for this purpose.
>>
>> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
>> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> 
> Matthieu -
> 
> Since you already Acked, what should be the status of this in patchwork?

I just set the status as "Awaiting Upstream". Is it OK for you for
patches for other trees?

(Note: I just noticed before sending this email I already "supported"
this case in our wiki [1] :) )

@Davide: now that the linked kernel patch is in net-next, may you send
this one to netdev please?

Cheers,
Mat

[1] https://github.com/multipath-tcp/mptcp_net-next/wiki/Patchwork
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

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

* Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag
  2021-12-08 10:23   ` Matthieu Baerts
@ 2021-12-08 23:19     ` Mat Martineau
  0 siblings, 0 replies; 8+ messages in thread
From: Mat Martineau @ 2021-12-08 23:19 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: Davide Caratti, mptcp

On Wed, 8 Dec 2021, Matthieu Baerts wrote:

> Hi Mat, Davide
>
> On 08/12/2021 02:59, Mat Martineau wrote:
>> On Fri, 3 Dec 2021, Davide Caratti wrote:
>>
>>> Linux supports 'MPTCP_PM_CMD_SET_FLAGS' since v5.12, and this control has
>>> recently been extended to allow setting flags for a given endpoint id.
>>> Although there is no use for changing 'signal' or 'subflow' flags, it can
>>> be helpful to set/clear the backup bit on existing endpoints: add the 'ip
>>> mptcp endpoint change <...>' command for this purpose.
>>>
>>> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
>>> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>
>> Matthieu -
>>
>> Since you already Acked, what should be the status of this in patchwork?
>
> I just set the status as "Awaiting Upstream". Is it OK for you for
> patches for other trees?
>

Sure, that status makes sense.


--
Mat Martineau
Intel

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

* Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag
  2021-12-09  9:10 Davide Caratti
@ 2021-12-14  3:25 ` David Ahern
  0 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2021-12-14  3:25 UTC (permalink / raw)
  To: Davide Caratti, netdev, David Ahern, Stephen Hemminger, Andrea Claudi
  Cc: Matthieu Baerts

On 12/9/21 2:10 AM, Davide Caratti wrote:
> Linux supports 'MPTCP_PM_CMD_SET_FLAGS' since v5.12, and this control has
> recently been extended to allow setting flags for a given endpoint id.
> Although there is no use for changing 'signal' or 'subflow' flags, it can
> be helpful to set/clear the backup bit on existing endpoints: add the 'ip
> mptcp endpoint change <...>' command for this purpose.
> 
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
>  ip/ipmptcp.c        | 20 ++++++++++++++++----
>  man/man8/ip-mptcp.8 | 14 ++++++++++++++
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 

does not apply to iproute2-next; please rebase.


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

* [PATCH iproute2-next] mptcp: add support for changing the backup flag
@ 2021-12-09  9:10 Davide Caratti
  2021-12-14  3:25 ` David Ahern
  0 siblings, 1 reply; 8+ messages in thread
From: Davide Caratti @ 2021-12-09  9:10 UTC (permalink / raw)
  To: netdev, David Ahern, Stephen Hemminger, Andrea Claudi; +Cc: Matthieu Baerts

Linux supports 'MPTCP_PM_CMD_SET_FLAGS' since v5.12, and this control has
recently been extended to allow setting flags for a given endpoint id.
Although there is no use for changing 'signal' or 'subflow' flags, it can
be helpful to set/clear the backup bit on existing endpoints: add the 'ip
mptcp endpoint change <...>' command for this purpose.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 ip/ipmptcp.c        | 20 ++++++++++++++++----
 man/man8/ip-mptcp.8 | 14 ++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 857004446aa3..7fb48a420a6b 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -25,6 +25,7 @@ static void usage(void)
 		"Usage:	ip mptcp endpoint add ADDRESS [ dev NAME ] [ id ID ]\n"
 		"				      [ port NR ] [ FLAG-LIST ]\n"
 		"	ip mptcp endpoint delete id ID\n"
+		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
 		"	ip mptcp endpoint show [ id ID ]\n"
 		"	ip mptcp endpoint flush\n"
 		"	ip mptcp limits set [ subflows NR ] [ add_addr_accepted NR ]\n"
@@ -45,6 +46,8 @@ static int genl_family = -1;
 	GENL_REQUEST(_req, MPTCP_BUFLEN, genl_family, 0,	\
 		     MPTCP_PM_VER, _cmd, _flags)
 
+#define MPTCP_PM_ADDR_FLAG_NOBACKUP 0x0
+
 /* Mapping from argument to address flag mask */
 static const struct {
 	const char *name;
@@ -53,6 +56,7 @@ static const struct {
 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
+	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NOBACKUP }
 };
 
 static void print_mptcp_addr_flags(unsigned int flags)
@@ -95,9 +99,9 @@ static int get_flags(const char *arg, __u32 *flags)
 	return -1;
 }
 
-static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
-			 bool adding)
+static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 {
+	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
 	struct rtattr *attr_addr;
 	bool addr_set = false;
 	inet_prefix address;
@@ -110,6 +114,11 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
 	ll_init_map(&rth);
 	while (argc > 0) {
 		if (get_flags(*argv, &flags) == 0) {
+			/* allow changing the 'backup' flag only */
+			if (cmd == MPTCP_PM_CMD_SET_FLAGS &&
+			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP))
+				invarg("invalid flags\n", *argv);
+
 		} else if (matches(*argv, "id") == 0) {
 			NEXT_ARG();
 
@@ -182,7 +191,7 @@ static int mptcp_addr_modify(int argc, char **argv, int cmd)
 	MPTCP_REQUEST(req, cmd, NLM_F_REQUEST);
 	int ret;
 
-	ret = mptcp_parse_opt(argc, argv, &req.n, cmd == MPTCP_PM_CMD_ADD_ADDR);
+	ret = mptcp_parse_opt(argc, argv, &req.n, cmd);
 	if (ret)
 		return ret;
 
@@ -298,7 +307,7 @@ static int mptcp_addr_show(int argc, char **argv)
 	if (argc <= 0)
 		return mptcp_addr_dump();
 
-	ret = mptcp_parse_opt(argc, argv, &req.n, false);
+	ret = mptcp_parse_opt(argc, argv, &req.n, MPTCP_PM_CMD_GET_ADDR);
 	if (ret)
 		return ret;
 
@@ -534,6 +543,9 @@ int do_mptcp(int argc, char **argv)
 		if (matches(*argv, "add") == 0)
 			return mptcp_addr_modify(argc-1, argv+1,
 						 MPTCP_PM_CMD_ADD_ADDR);
+		if (matches(*argv, "change") == 0)
+			return mptcp_addr_modify(argc-1, argv+1,
+						 MPTCP_PM_CMD_SET_FLAGS);
 		if (matches(*argv, "delete") == 0)
 			return mptcp_addr_modify(argc-1, argv+1,
 						 MPTCP_PM_CMD_DEL_ADDR);
diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
index 22335b6129ae..574cb60dc7c0 100644
--- a/man/man8/ip-mptcp.8
+++ b/man/man8/ip-mptcp.8
@@ -34,6 +34,13 @@ ip-mptcp \- MPTCP path manager configuration
 .BR "ip mptcp endpoint del id "
 .I ID
 
+.ti -8
+.BR "ip mptcp endpoint change id "
+.I ID
+.RB "[ "
+.I BACKUP-OPT
+.RB "] "
+
 .ti -8
 .BR "ip mptcp endpoint show "
 .RB "[ " id
@@ -55,6 +62,13 @@ ip-mptcp \- MPTCP path manager configuration
 .B backup
 .RB  "]"
 
+.ti -8
+.IR BACKUP-OPT " := ["
+.B backup
+.RB "|"
+.B nobackup
+.RB  "]"
+
 .ti -8
 .BR "ip mptcp limits set "
 .RB "[ "
-- 
2.31.1


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

* Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag
  2021-11-26  9:38 Davide Caratti
@ 2021-11-26 17:06 ` Matthieu Baerts
  0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2021-11-26 17:06 UTC (permalink / raw)
  To: Davide Caratti, mptcp

Hi Davide,

On 26/11/2021 10:38, Davide Caratti wrote:
> allow setting / clearing the 'backup' bit on existing endpoints.

Thank you for the patch!

Do you mind adding a bit more context in the commit message here? e.g.
the kernel support "MPTCP_PM_CMD_SET_FLAGS" since v5.x, so let's add the
support here for userspace (...) before, we were able to mark an
endpoint as backup but not to change this backup attribute later (...)

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/158

Ideally, we would need a new packetdrill test to close this ticket but
maybe you already have one?

Also, because this commit will not be applied in our "export" branch, it
will not close the ticket automatically. Maybe you can use 'Link:' tag
instead when sending it upstream?

(...)

> diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
> index 22335b6129ae..6e7f8269a4f8 100644
> --- a/man/man8/ip-mptcp.8
> +++ b/man/man8/ip-mptcp.8

(...)

> @@ -118,7 +132,7 @@ the endpoint will be announced as a backup address, if this is a
>  .BR signal
>  endpoint, or the subflow will be created as a backup one if this is a
>  .BR subflow
> -endpoint
> +endpoint.

Small detail: best to avoid this modification as this will conflict with
Paolo's patch he sent a few hours ago to netdev.

For the rest, it looks good to me, thank you!

Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>


Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

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

* [PATCH iproute2-next] mptcp: add support for changing the backup flag
@ 2021-11-26  9:38 Davide Caratti
  2021-11-26 17:06 ` Matthieu Baerts
  0 siblings, 1 reply; 8+ messages in thread
From: Davide Caratti @ 2021-11-26  9:38 UTC (permalink / raw)
  To: mptcp, Matthieu Baerts

allow setting / clearing the 'backup' bit on existing endpoints.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/158
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 ip/ipmptcp.c        | 20 ++++++++++++++++----
 man/man8/ip-mptcp.8 | 16 +++++++++++++++-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 0f5b6e2d08ba..bae27c3e8c6a 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -25,6 +25,7 @@ static void usage(void)
 		"Usage:	ip mptcp endpoint add ADDRESS [ dev NAME ] [ id ID ]\n"
 		"				      [ port NR ] [ FLAG-LIST ]\n"
 		"	ip mptcp endpoint delete id ID\n"
+		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
 		"	ip mptcp endpoint show [ id ID ]\n"
 		"	ip mptcp endpoint flush\n"
 		"	ip mptcp limits set [ subflows NR ] [ add_addr_accepted NR ]\n"
@@ -45,6 +46,8 @@ static int genl_family = -1;
 	GENL_REQUEST(_req, MPTCP_BUFLEN, genl_family, 0,	\
 		     MPTCP_PM_VER, _cmd, _flags)
 
+#define MPTCP_PM_ADDR_FLAG_NOBACKUP 0x0
+
 /* Mapping from argument to address flag mask */
 static const struct {
 	const char *name;
@@ -53,6 +56,7 @@ static const struct {
 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
+	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NOBACKUP }
 };
 
 static void print_mptcp_addr_flags(unsigned int flags)
@@ -95,9 +99,9 @@ static int get_flags(const char *arg, __u32 *flags)
 	return -1;
 }
 
-static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
-			 bool adding)
+static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 {
+	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
 	struct rtattr *attr_addr;
 	bool addr_set = false;
 	inet_prefix address;
@@ -110,6 +114,11 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n,
 	ll_init_map(&rth);
 	while (argc > 0) {
 		if (get_flags(*argv, &flags) == 0) {
+			/* allow changing the 'backup' flag only */
+			if (cmd == MPTCP_PM_CMD_SET_FLAGS &&
+			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP))
+				invarg("invalid flags\n", *argv);
+
 		} else if (matches(*argv, "id") == 0) {
 			NEXT_ARG();
 
@@ -182,7 +191,7 @@ static int mptcp_addr_modify(int argc, char **argv, int cmd)
 	MPTCP_REQUEST(req, cmd, NLM_F_REQUEST);
 	int ret;
 
-	ret = mptcp_parse_opt(argc, argv, &req.n, cmd == MPTCP_PM_CMD_ADD_ADDR);
+	ret = mptcp_parse_opt(argc, argv, &req.n, cmd);
 	if (ret)
 		return ret;
 
@@ -298,7 +307,7 @@ static int mptcp_addr_show(int argc, char **argv)
 	if (argc <= 0)
 		return mptcp_addr_dump();
 
-	ret = mptcp_parse_opt(argc, argv, &req.n, false);
+	ret = mptcp_parse_opt(argc, argv, &req.n, MPTCP_PM_CMD_GET_ADDR);
 	if (ret)
 		return ret;
 
@@ -530,6 +539,9 @@ int do_mptcp(int argc, char **argv)
 		if (matches(*argv, "add") == 0)
 			return mptcp_addr_modify(argc-1, argv+1,
 						 MPTCP_PM_CMD_ADD_ADDR);
+		if (matches(*argv, "change") == 0)
+			return mptcp_addr_modify(argc-1, argv+1,
+						 MPTCP_PM_CMD_SET_FLAGS);
 		if (matches(*argv, "delete") == 0)
 			return mptcp_addr_modify(argc-1, argv+1,
 						 MPTCP_PM_CMD_DEL_ADDR);
diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
index 22335b6129ae..6e7f8269a4f8 100644
--- a/man/man8/ip-mptcp.8
+++ b/man/man8/ip-mptcp.8
@@ -34,6 +34,13 @@ ip-mptcp \- MPTCP path manager configuration
 .BR "ip mptcp endpoint del id "
 .I ID
 
+.ti -8
+.BR "ip mptcp endpoint change id "
+.I ID
+.RB "[ "
+.I BACKUP-OPT
+.RB "] "
+
 .ti -8
 .BR "ip mptcp endpoint show "
 .RB "[ " id
@@ -55,6 +62,13 @@ ip-mptcp \- MPTCP path manager configuration
 .B backup
 .RB  "]"
 
+.ti -8
+.IR BACKUP-OPT " := ["
+.B backup
+.RB "|"
+.B nobackup
+.RB  "]"
+
 .ti -8
 .BR "ip mptcp limits set "
 .RB "[ "
@@ -118,7 +132,7 @@ the endpoint will be announced as a backup address, if this is a
 .BR signal
 endpoint, or the subflow will be created as a backup one if this is a
 .BR subflow
-endpoint
+endpoint.
 
 .sp
 .PP
-- 
2.31.1


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

end of thread, other threads:[~2021-12-14  3:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03 15:01 [PATCH iproute2-next] mptcp: add support for changing the backup flag Davide Caratti
2021-12-08  1:59 ` Mat Martineau
2021-12-08 10:23   ` Matthieu Baerts
2021-12-08 23:19     ` Mat Martineau
  -- strict thread matches above, loose matches on Subject: below --
2021-12-09  9:10 Davide Caratti
2021-12-14  3:25 ` David Ahern
2021-11-26  9:38 Davide Caratti
2021-11-26 17:06 ` Matthieu Baerts

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.