Re: [PATCH iproute2-next v7] mptcp: add the fullmesh flag setting support

[Geliang Tang <geliang.tang@suse.com>]

On Fri, Jan 21, 2022 at 05:03:12PM -0800, Mat Martineau wrote:
> On Fri, 21 Jan 2022, Geliang Tang wrote:
> 
> > This patch added the fullmesh flag setting support, use it like this:
> > 
> > ip mptcp endpoint change id 1 fullmesh
> > ip mptcp endpoint change id 1 nofullmesh
> > ip mptcp endpoint change id 1 backup fullmesh
> > ip mptcp endpoint change id 1 nobackup nofullmesh
> > 
> > Add the fullmesh flag check for the adding address, the fullmesh flag
> > can't be used with the signal flag in that case.
> > 
> > Update the port keyword check for the setting flags, allow to use the
> > port keyword with the non-signal flags. Don't allow to use the port
> > keyword with the id number.
> > 
> > Update the usage of 'ip mptcp endpoint change', it can be used in two
> > forms, using the address directly or the id number of the address:
> > 
> > ip mptcp endpoint change id 1 fullmesh
> > ip mptcp endpoint change 10.0.2.1 fullmesh
> > ip mptcp endpoint change 10.0.2.1 port 10100 fullmesh
> > 
> > Acked-by: Paolo Abeni <pabeni@redhat.com>
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> > v7:
> > - allow to use the port keyword with the non-signal flags for the
> >   setting flags, then we can use the address argument for "ip mptcp
> >   endpoint change" too.
> > - update the usage and man-page.
> 
> The v6->v7 changes look good, but both versions of the patch only apply to
> the iproute2 repo, not iproute2-next. Please rebase on iproute2-next before
> sending to netdev.

Hi Mat,

Sorry for the trouble. This patch is based on iproute2-next
(commit db5305290c2fc8f266fbba7806e2bd2efee574e1), depending on my
another queued patch "mptcp: add id check for deleting address"
(https://patchwork.kernel.org/project/mptcp/patch/59ec6f5ba44972ef5e7e400f5333852084ff0087.1642262155.git.geliang.tang@suse.com/).

Thanks,
-Geliang

> 
> -Mat
> 
> > 
> > v6:
> > - update the flags check as Mat suggested.
> > 
> > v5:
> > - support to set backup and fullmesh flags together.
> > ---
> > ip/ipmptcp.c        | 26 +++++++++++++++++++-------
> > man/man8/ip-mptcp.8 | 17 ++++++++++++-----
> > 2 files changed, 31 insertions(+), 12 deletions(-)
> > 
> > diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> > index eef7c6f4..efda7ef2 100644
> > --- a/ip/ipmptcp.c
> > +++ b/ip/ipmptcp.c
> > @@ -25,14 +25,15 @@ 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 [ ADDRESS ]\n"
> > -		"	ip mptcp endpoint change id ID [ backup | nobackup ]\n"
> > +		"	ip mptcp endpoint change [ id ID ] [ ADDRESS ] CHANGE-OPT\n"
> > 		"	ip mptcp endpoint show [ id ID ]\n"
> > 		"	ip mptcp endpoint flush\n"
> > 		"	ip mptcp limits set [ subflows NR ] [ add_addr_accepted NR ]\n"
> > 		"	ip mptcp limits show\n"
> > 		"	ip mptcp monitor\n"
> > 		"FLAG-LIST := [ FLAG-LIST ] FLAG\n"
> > -		"FLAG  := [ signal | subflow | backup | fullmesh ]\n");
> > +		"FLAG  := [ signal | subflow | backup | fullmesh ]\n"
> > +		"CHANGE-OPT := [ backup | nobackup | fullmesh | nofullmesh ]\n");
> > 
> > 	exit(-1);
> > }
> > @@ -46,7 +47,7 @@ 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
> > +#define MPTCP_PM_ADDR_FLAG_NONE 0x0
> > 
> > /* Mapping from argument to address flag mask */
> > static const struct {
> > @@ -57,7 +58,8 @@ static const struct {
> > 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
> > 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
> > 	{ "fullmesh",		MPTCP_PM_ADDR_FLAG_FULLMESH },
> > -	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NOBACKUP }
> > +	{ "nobackup",		MPTCP_PM_ADDR_FLAG_NONE },
> > +	{ "nofullmesh",		MPTCP_PM_ADDR_FLAG_NONE }
> > };
> > 
> > static void print_mptcp_addr_flags(unsigned int flags)
> > @@ -102,6 +104,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 setting = cmd == MPTCP_PM_CMD_SET_FLAGS;
> > 	bool adding = cmd == MPTCP_PM_CMD_ADD_ADDR;
> > 	bool deling = cmd == MPTCP_PM_CMD_DEL_ADDR;
> > 	struct rtattr *attr_addr;
> > @@ -116,9 +119,15 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
> > 	ll_init_map(&rth);
> > 	while (argc > 0) {
> > 		if (get_flags(*argv, &flags) == 0) {
> > -			/* allow changing the 'backup' flag only */
> > +			if (adding &&
> > +			    (flags & MPTCP_PM_ADDR_FLAG_SIGNAL) &&
> > +			    (flags & MPTCP_PM_ADDR_FLAG_FULLMESH))
> > +				invarg("invalid flags\n", *argv);
> > +
> > +			/* allow changing the 'backup' and 'fullmesh' flags only */
> > 			if (cmd == MPTCP_PM_CMD_SET_FLAGS &&
> > -			    (flags & ~MPTCP_PM_ADDR_FLAG_BACKUP))
> > +			    (flags & ~(MPTCP_PM_ADDR_FLAG_BACKUP |
> > +				       MPTCP_PM_ADDR_FLAG_FULLMESH)))
> > 				invarg("invalid flags\n", *argv);
> > 
> > 		} else if (matches(*argv, "id") == 0) {
> > @@ -166,9 +175,12 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
> > 			invarg("address is needed for deleting id 0 address\n", "ID");
> > 	}
> > 
> > -	if (port && !(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> > +	if (adding && port && !(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> > 		invarg("flags must have signal when using port", "port");
> > 
> > +	if (id_set && setting && port)
> > +		invarg("port can't be used with id", "port");
> > +
> > 	attr_addr = addattr_nest(n, MPTCP_BUFLEN,
> > 				 MPTCP_PM_ATTR_ADDR | NLA_F_NESTED);
> > 	if (id_set)
> > diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
> > index 0e789225..72762f49 100644
> > --- a/man/man8/ip-mptcp.8
> > +++ b/man/man8/ip-mptcp.8
> > @@ -38,11 +38,14 @@ ip-mptcp \- MPTCP path manager configuration
> > .RB "] "
> > 
> > .ti -8
> > -.BR "ip mptcp endpoint change id "
> > +.BR "ip mptcp endpoint change "
> > +.RB "[ " id
> > .I ID
> > -.RB "[ "
> > -.I BACKUP-OPT
> > -.RB "] "
> > +.RB "] [ "
> > +.IR IFADDR
> > +.RB "] [ " port
> > +.IR PORT " ]"
> > +.RB "CHANGE-OPT"
> > 
> > .ti -8
> > .BR "ip mptcp endpoint show "
> > @@ -68,10 +71,14 @@ ip-mptcp \- MPTCP path manager configuration
> > .RB  "]"
> > 
> > .ti -8
> > -.IR BACKUP-OPT " := ["
> > +.IR CHANGE-OPT " := ["
> > .B backup
> > .RB "|"
> > .B nobackup
> > +.RB "|"
> > +.B fullmesh
> > +.RB "|"
> > +.B nofullmesh
> > .RB  "]"
> > 
> > .ti -8
> > -- 
> > 2.31.1
> > 
> > 
> > 
> 
> --
> Mat Martineau
> Intel
>