[PATCH iproute2-next] mptcp: add support for changing the backup flag

[PATCH iproute2-next] mptcp: add support for changing the backup flag

[Davide Caratti]

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

Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag

[Mat Martineau]

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

Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag

[Matthieu Baerts]

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

Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag

[Mat Martineau]

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

[PATCH iproute2-next] mptcp: add support for changing the backup flag

[Davide Caratti]

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

Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag

[David Ahern]

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.

[PATCH iproute2-next] mptcp: add support for changing the backup flag

[Davide Caratti]

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

Re: [PATCH iproute2-next] mptcp: add support for changing the backup flag

[Matthieu Baerts]

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