[v1] mptcp: add id check for deleting address

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

Posted by Geliang Tang 2 years, 3 months 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>
Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 ip/ipmptcp.c        | 11 +++++++++--
 man/man8/ip-mptcp.8 | 16 +++++++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index e7150138..4363e753 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..0e789225 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 IFADDR
+.RB "] "
 
 .ti -8
 .BR "ip mptcp endpoint change id "
@@ -107,6 +110,16 @@ ip mptcp endpoint show	get existing MPTCP endpoint
 ip mptcp endpoint flush	flush all existing MPTCP endpoints
 .TE
 
+.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.
+
 .TP
 .IR PORT
 When a port number is specified, incoming MPTCP subflows for already
@@ -114,6 +127,7 @@ established MPTCP sockets will be accepted on the specified port, regardless
 the original listener port accepting the first MPTCP subflow and/or
 this peer being actually on the client side.
 
+.TP
 .IR ID
 is a unique numeric identifier for the given endpoint
 
-- 
2.31.1

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

Posted by David Ahern 2 years, 3 months ago

On 1/14/22 5:10 PM, 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

meaning bug fix? If so please add a Fixes tag with the commit that
should have required the id.

> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  ip/ipmptcp.c        | 11 +++++++++--
>  man/man8/ip-mptcp.8 | 16 +++++++++++++++-
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> index e7150138..4363e753 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) {

brackets on the 'if () { .. }' since they are needed on the else.

> +		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");

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

Posted by Geliang Tang 2 years, 3 months ago

Hi David,

Thanks for your review.

On Sat, Jan 15, 2022 at 08:42:25AM -0700, David Ahern wrote:
> On 1/14/22 5:10 PM, 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
> 
> meaning bug fix? If so please add a Fixes tag with the commit that
> should have required the id.

This patch isn't a fix, no Fixes tag needed. I dropped the Closes tag in
v2.

> 
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> > ---
> >  ip/ipmptcp.c        | 11 +++++++++--
> >  man/man8/ip-mptcp.8 | 16 +++++++++++++++-
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> > index e7150138..4363e753 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) {
> 
> brackets on the 'if () { .. }' since they are needed on the else.

Updated in v2. And v2 was sent out a few days ago.

Thanks,
-Geliang

> 
> > +		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");
>