[PATCH iproute2-next v2] mptcp: add id check for deleting address

Geliang Tang posted 1 patch 2 weeks ago
Failed in applying to current master (apply log)
ip/ipmptcp.c        | 11 +++++++++--
man/man8/ip-mptcp.8 |  5 ++++-
2 files changed, 13 insertions(+), 3 deletions(-)

[PATCH iproute2-next v2] mptcp: add id check for deleting address

Posted by Geliang Tang 2 weeks ago
This patch added the id check for deleting address in mptcp_parse_opt().
The ADDRESS argument is invalid for the non-zero id address, only needed
for the id 0 address.

 # ip mptcp endpoint delete id 1
 # ip mptcp endpoint delete id 0 10.0.1.1

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/171
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 v2:
 - rebase for iproute2-next.git
 - drop patch 2 & 3 in v1.
 - change ip-mptcp.8 too.
---
 ip/ipmptcp.c        | 11 +++++++++--
 man/man8/ip-mptcp.8 |  5 ++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 10dcb1ea..f85c49a8 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -24,7 +24,7 @@ static void usage(void)
 	fprintf(stderr,
 		"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 delete id ID [ ADDRESS ]\n"
 		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
 		"	ip mptcp endpoint show [ id ID ]\n"
 		"	ip mptcp endpoint flush\n"
@@ -103,6 +103,7 @@ static int get_flags(const char *arg, __u32 *flags)
 static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 {
 	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
+	bool deling = cmd == MPTCP_PM_CMD_DEL_ADDR;
 	struct rtattr *attr_addr;
 	bool addr_set = false;
 	inet_prefix address;
@@ -156,8 +157,14 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 	if (!addr_set && adding)
 		missarg("ADDRESS");
 
-	if (!id_set && !adding)
+	if (!id_set && deling)
 		missarg("ID");
+	else if (id_set && deling) {
+		if (id && addr_set)
+			invarg("invalid for non-zero id address\n", "ADDRESS");
+		else if (!id && !addr_set)
+			invarg("address is needed for deleting id 0 address\n", "ID");
+	}
 
 	if (port && !(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
 		invarg("flags must have signal when using port", "port");
diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
index 0e6e1532..1b8b8c7c 100644
--- a/man/man8/ip-mptcp.8
+++ b/man/man8/ip-mptcp.8
@@ -31,8 +31,11 @@ ip-mptcp \- MPTCP path manager configuration
 .RB "] "
 
 .ti -8
-.BR "ip mptcp endpoint del id "
+.BR "ip mptcp endpoint delete id "
 .I ID
+.RB "[ "
+.I ADDRESS
+.RB "] "
 
 .ti -8
 .BR "ip mptcp endpoint change id "
-- 
2.31.1


Re: [PATCH iproute2-next v2] mptcp: add id check for deleting address

Posted by Mat Martineau 2 weeks ago
On Mon, 10 Jan 2022, Geliang Tang wrote:

> This patch added the id check for deleting address in mptcp_parse_opt().
> The ADDRESS argument is invalid for the non-zero id address, only needed
> for the id 0 address.
>
> # ip mptcp endpoint delete id 1
> # ip mptcp endpoint delete id 0 10.0.1.1
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/171
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> v2:
> - rebase for iproute2-next.git
> - drop patch 2 & 3 in v1.
> - change ip-mptcp.8 too.

Thanks for updating this Geliang. The code looks good to me, and I have a 
couple suggestions for the man page below.

> ---
> ip/ipmptcp.c        | 11 +++++++++--
> man/man8/ip-mptcp.8 |  5 ++++-
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> index 10dcb1ea..f85c49a8 100644
> --- a/ip/ipmptcp.c
> +++ b/ip/ipmptcp.c
> @@ -24,7 +24,7 @@ static void usage(void)
> 	fprintf(stderr,
> 		"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 delete id ID [ ADDRESS ]\n"
> 		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
> 		"	ip mptcp endpoint show [ id ID ]\n"
> 		"	ip mptcp endpoint flush\n"
> @@ -103,6 +103,7 @@ static int get_flags(const char *arg, __u32 *flags)
> static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
> {
> 	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
> +	bool deling = cmd == MPTCP_PM_CMD_DEL_ADDR;
> 	struct rtattr *attr_addr;
> 	bool addr_set = false;
> 	inet_prefix address;
> @@ -156,8 +157,14 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
> 	if (!addr_set && adding)
> 		missarg("ADDRESS");
>
> -	if (!id_set && !adding)
> +	if (!id_set && deling)
> 		missarg("ID");
> +	else if (id_set && deling) {
> +		if (id && addr_set)
> +			invarg("invalid for non-zero id address\n", "ADDRESS");
> +		else if (!id && !addr_set)
> +			invarg("address is needed for deleting id 0 address\n", "ID");
> +	}
>
> 	if (port && !(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> 		invarg("flags must have signal when using port", "port");
> diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
> index 0e6e1532..1b8b8c7c 100644
> --- a/man/man8/ip-mptcp.8
> +++ b/man/man8/ip-mptcp.8
> @@ -31,8 +31,11 @@ ip-mptcp \- MPTCP path manager configuration
> .RB "] "
>
> .ti -8
> -.BR "ip mptcp endpoint del id "
> +.BR "ip mptcp endpoint delete id "
> .I ID
> +.RB "[ "
> +.I ADDRESS

Since the 'ip mptcp endpoint add' section above uses IFADDR instead of 
ADDRESS, it is better to use IFADDR again here.

> +.RB "] "
>
> .ti -8
> .BR "ip mptcp endpoint change id "

I suggest adding this before the ".TP\n.IR PORT" section at line 113:


.TP
.IR IFADDR
An IPv4 or IPv6 address. When used with the
.B delete id
operation, an
.B IFADDR
is only included when the
.B ID
is 0.


I also noticed a formatting bug in the existing markup, there should be a 
".TP" line just before ".IR ID" - can you add that too?


Thanks,

--
Mat Martineau
Intel