[PATCH iproute2-next] mptcp: add support for fullmesh flag

Paolo Abeni posted 1 patch 2 years, 5 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
ip/ipmptcp.c        |  3 ++-
man/man8/ip-mptcp.8 | 47 ++++++++++++++++++++++++++++++++++-----------
2 files changed, 38 insertions(+), 12 deletions(-)
[PATCH iproute2-next] mptcp: add support for fullmesh flag
Posted by Paolo Abeni 2 years, 5 months ago
The link kernel supports this endpoint flag since v5.15, let's
expose it to user-space. It allows creation on fullmesh topolgy
via MPTCP subflow.

Additionally update the related man-page, clarifying the behavior
of related options.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
notes:
- currently sent only to mptcp ML, looking for "editorial" reviews :)
- as a side doc-update effect, this could possibly close issue/237

---
 ip/ipmptcp.c        |  3 ++-
 man/man8/ip-mptcp.8 | 47 ++++++++++++++++++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 0f5b6e2d..433fa68d 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -31,7 +31,7 @@ static void usage(void)
 		"	ip mptcp limits show\n"
 		"	ip mptcp monitor\n"
 		"FLAG-LIST := [ FLAG-LIST ] FLAG\n"
-		"FLAG  := [ signal | subflow | backup ]\n");
+		"FLAG  := [ signal | subflow | backup | fullmesh ]\n");
 
 	exit(-1);
 }
@@ -53,6 +53,7 @@ static const struct {
 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
+	{ "fullmesh",		MPTCP_PM_ADDR_FLAG_FULLMESH },
 };
 
 static void print_mptcp_addr_flags(unsigned int flags)
diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
index 22335b61..367ecc64 100644
--- a/man/man8/ip-mptcp.8
+++ b/man/man8/ip-mptcp.8
@@ -53,6 +53,8 @@ ip-mptcp \- MPTCP path manager configuration
 .B subflow
 .RB "|"
 .B backup
+.RB "|"
+.B fullmesh
 .RB  "]"
 
 .ti -8
@@ -103,22 +105,36 @@ is a unique numeric identifier for the given endpoint
 
 .TP
 .BR signal
-the endpoint will be announced/signalled to each peer via an ADD_ADDR MPTCP
-sub-option
+the endpoint will be announced/signaled to each peer via an ADD_ADDR MPTCP
+sub-option. Upon reception of an ADD_ADDR option, the peer can try to create
+additional subflows, see
+.BR ADD_ADDR_ACCEPTED_NR.
 
 .TP
 .BR subflow
-if additional subflow creation is allowed by MPTCP limits, the endpoint will
-be used as the source address to create an additional subflow after that
-the MPTCP connection is established.
+if additional subflow creation is allowed by MPTCP limits, after that the
+MPTCP connection is established, the MPTCP path manager will try to create
+an additional subflow using this endpoint as the source address.
 
 .TP
 .BR backup
-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
+if this is a
+.BR subflow
+endpoint, the subflows created using this endpoint will be backup ones.
+Backup subflows are used only when all non-backup subflows are unavailable.
+
+.TP
+.BR fullmesh
+if this is a
+.BR subflow
+endpoint, if additional subflow creation is allowed by MPTCP limits, after that the
+each MPTCP connection is established, the MPTCP path manager will try to create
+an additional subflow for each known server address using this endpoint as
+the source address. If the peer did not announce any additional address via
+ADD_ADDR MPTCP sub-option, this has no additional effect compared to a plain
 .BR subflow
-endpoint
+endpoint. Otherwise, this will create an additional subflow for each received
+ADD_ADDR, generating a fullmesh topology.
 
 .sp
 .PP
@@ -143,8 +159,17 @@ endpoints, additional subflows started by the peer.
 .TP
 .IR ADD_ADDR_ACCEPTED_NR
 specifies the maximum number of ADD_ADDR suboptions accepted for each MPTCP
-connection. The MPTCP path manager will try to create a new subflow for
-each accepted ADD_ADDR option, respecting the
+connection. When and ADD_ADDR suboption is accepted, if there are no local
+.BR fullmesh
+endpoint, the MPTCP path manager will try to create a new subflow towards the
+address specified by the ADD_ADDR option, using as source the local address
+obtained by the routing resolution. Otherwise, the MPTCP path manager will try
+to create a new subflow for each
+.BR fullmesh
+endpoint, using the fullmesh endpoint as the source address and the address
+specified by the ADD_ADDR option as the destination. In both cases the MPTCP
+path manager enforces
+the
 .IR SUBFLOW_NR
 limit.
 
-- 
2.33.1


Re: [PATCH iproute2-next] mptcp: add support for fullmesh flag
Posted by Mat Martineau 2 years, 5 months ago
On Thu, 18 Nov 2021, Paolo Abeni wrote:

> The link kernel supports this endpoint flag since v5.15, let's
> expose it to user-space. It allows creation on fullmesh topolgy
> via MPTCP subflow.
>
> Additionally update the related man-page, clarifying the behavior
> of related options.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> notes:
> - currently sent only to mptcp ML, looking for "editorial" reviews :)
> - as a side doc-update effect, this could possibly close issue/237
>
> ---
> ip/ipmptcp.c        |  3 ++-
> man/man8/ip-mptcp.8 | 47 ++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> index 0f5b6e2d..433fa68d 100644
> --- a/ip/ipmptcp.c
> +++ b/ip/ipmptcp.c
> @@ -31,7 +31,7 @@ static void usage(void)
> 		"	ip mptcp limits show\n"
> 		"	ip mptcp monitor\n"
> 		"FLAG-LIST := [ FLAG-LIST ] FLAG\n"
> -		"FLAG  := [ signal | subflow | backup ]\n");
> +		"FLAG  := [ signal | subflow | backup | fullmesh ]\n");
>
> 	exit(-1);
> }
> @@ -53,6 +53,7 @@ static const struct {
> 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
> 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
> 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
> +	{ "fullmesh",		MPTCP_PM_ADDR_FLAG_FULLMESH },
> };
>
> static void print_mptcp_addr_flags(unsigned int flags)
> diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
> index 22335b61..367ecc64 100644
> --- a/man/man8/ip-mptcp.8
> +++ b/man/man8/ip-mptcp.8
> @@ -53,6 +53,8 @@ ip-mptcp \- MPTCP path manager configuration
> .B subflow
> .RB "|"
> .B backup
> +.RB "|"
> +.B fullmesh
> .RB  "]"
>
> .ti -8
> @@ -103,22 +105,36 @@ is a unique numeric identifier for the given endpoint
>
> .TP
> .BR signal
> -the endpoint will be announced/signalled to each peer via an ADD_ADDR MPTCP
> -sub-option
> +the endpoint will be announced/signaled to each peer via an ADD_ADDR MPTCP

    ^^^
Suggest capitalizing the first "The" since the preceeding "signal" is not 
part of the sentence.

> +sub-option. Upon reception of an ADD_ADDR option, the peer can try to create
> +additional subflows, see
> +.BR ADD_ADDR_ACCEPTED_NR.
>
> .TP
> .BR subflow
> -if additional subflow creation is allowed by MPTCP limits, the endpoint will
> -be used as the source address to create an additional subflow after that
> -the MPTCP connection is established.
> +if additional subflow creation is allowed by MPTCP limits, after that the

    ^^
Capitalize here too.

> +MPTCP connection is established, the MPTCP path manager will try to create
> +an additional subflow using this endpoint as the source address.

I would rearrange this one:

If additional subflow creation is allowed by the MPTCP limits, the MPTCP
path manager will try to create an additional subflow using this endpoint
as the source address after the MPTCP connection is established.

>
> .TP
> .BR backup
> -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
> +if this is a

    ^^

Capitalize.

> +.BR subflow
> +endpoint, the subflows created using this endpoint will be backup ones.
> +Backup subflows are used only when all non-backup subflows are unavailable.
> +

Suggestion:

If this is a
.BR subflow

endpoint, the subflows created using this endpoint will have the backup 
flag set during the connection process. This flag instructs the peer to 
only send data on a given subflow when all non-backup subflows are 
unavailable. This does not affect outgoing data, where subflow priority 
is determined by the backup/non-backup flag received from the peer.


> +.TP
> +.BR fullmesh
> +if this is a

    ^^
"If"

> +.BR subflow
> +endpoint, if additional subflow creation is allowed by MPTCP limits, after that the
> +each MPTCP connection is established, the MPTCP path manager will try to create
> +an additional subflow for each known server address using this endpoint as
> +the source address. If the peer did not announce any additional address via
> +ADD_ADDR MPTCP sub-option, this has no additional effect compared to a plain
> .BR subflow
> -endpoint
> +endpoint. Otherwise, this will create an additional subflow for each received
> +ADD_ADDR, generating a fullmesh topology.

Suggestion:

If this is a
.BR subflow
endpoint and additional subflow creation is allowed by the MPTCP limits, 
the MPTCP path manager will try to create an additional subflow for each 
known peer address, using this endpoint as the source address. This will 
occur after the MPTCP connection is established. If the peer did not 
announce any additional addresses using the ADD_ADDR MPTCP sub-option, 
this will behave the same as a plain
.BR subflow
endpoint. When the peer does announce addresses, each received ADD_ADDR 
will trigger creation of an additional subflow to generate a full mesh 
topology.


>
> .sp
> .PP
> @@ -143,8 +159,17 @@ endpoints, additional subflows started by the peer.
> .TP
> .IR ADD_ADDR_ACCEPTED_NR
> specifies the maximum number of ADD_ADDR suboptions accepted for each MPTCP
> -connection. The MPTCP path manager will try to create a new subflow for
> -each accepted ADD_ADDR option, respecting the
> +connection.

splitting here for edits

> When and ADD_ADDR suboption is accepted, if there are no local
> +.BR fullmesh
> +endpoint, the MPTCP path manager will try to create a new subflow towards the
> +address specified by the ADD_ADDR option, using as source the local address
> +obtained by the routing resolution.

"When an ADD_ADDR sub-option is received and there are no local fullmesh 
endpoints, the MPTCP path manager will try to create a new subflow using 
the address in the ADD_ADDR as the destination address and a source 
address determined using local routing resolution. When fullmesh endpoints 
are available, the MPTCP path manager will try to create new subflows 
using each fullmesh endpoint as a source address and the peer's ADD_ADDR 
address as the destination. In both cases the SUBFLOW_NR limit is 
enforced."

(I hope I kept the intended meaning here?)


I think there is a mix of "ADD_ADDR option", "ADD_ADDR suboption", and 
"ADD_ADDR sub-option" above, even after my edits. Should probably choose 
one and be consistent with it!


-Mat


> Otherwise, the MPTCP path manager will try
> +to create a new subflow for each
> +.BR fullmesh
> +endpoint, using the fullmesh endpoint as the source address and the address
> +specified by the ADD_ADDR option as the destination. In both cases the MPTCP
> +path manager enforces
> +the
> .IR SUBFLOW_NR
> limit.
>
> -- 
> 2.33.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH iproute2-next] mptcp: add support for fullmesh flag
Posted by Paolo Abeni 2 years, 5 months ago
On Thu, 2021-11-18 at 17:31 -0800, Mat Martineau wrote:
> On Thu, 18 Nov 2021, Paolo Abeni wrote:
> 
> > The link kernel supports this endpoint flag since v5.15, let's
> > expose it to user-space. It allows creation on fullmesh topolgy
> > via MPTCP subflow.
> > 
> > Additionally update the related man-page, clarifying the behavior
> > of related options.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > notes:
> > - currently sent only to mptcp ML, looking for "editorial" reviews :)
> > - as a side doc-update effect, this could possibly close issue/237
> > 
> > ---
> > ip/ipmptcp.c        |  3 ++-
> > man/man8/ip-mptcp.8 | 47 ++++++++++++++++++++++++++++++++++-----------
> > 2 files changed, 38 insertions(+), 12 deletions(-)
> > 
> > diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> > index 0f5b6e2d..433fa68d 100644
> > --- a/ip/ipmptcp.c
> > +++ b/ip/ipmptcp.c
> > @@ -31,7 +31,7 @@ static void usage(void)
> > 		"	ip mptcp limits show\n"
> > 		"	ip mptcp monitor\n"
> > 		"FLAG-LIST := [ FLAG-LIST ] FLAG\n"
> > -		"FLAG  := [ signal | subflow | backup ]\n");
> > +		"FLAG  := [ signal | subflow | backup | fullmesh ]\n");
> > 
> > 	exit(-1);
> > }
> > @@ -53,6 +53,7 @@ static const struct {
> > 	{ "signal",		MPTCP_PM_ADDR_FLAG_SIGNAL },
> > 	{ "subflow",		MPTCP_PM_ADDR_FLAG_SUBFLOW },
> > 	{ "backup",		MPTCP_PM_ADDR_FLAG_BACKUP },
> > +	{ "fullmesh",		MPTCP_PM_ADDR_FLAG_FULLMESH },
> > };
> > 
> > static void print_mptcp_addr_flags(unsigned int flags)
> > diff --git a/man/man8/ip-mptcp.8 b/man/man8/ip-mptcp.8
> > index 22335b61..367ecc64 100644
> > --- a/man/man8/ip-mptcp.8
> > +++ b/man/man8/ip-mptcp.8
> > @@ -53,6 +53,8 @@ ip-mptcp \- MPTCP path manager configuration
> > .B subflow
> > .RB "|"
> > .B backup
> > +.RB "|"
> > +.B fullmesh
> > .RB  "]"
> > 
> > .ti -8
> > @@ -103,22 +105,36 @@ is a unique numeric identifier for the given endpoint
> > 
> > .TP
> > .BR signal
> > -the endpoint will be announced/signalled to each peer via an ADD_ADDR MPTCP
> > -sub-option
> > +the endpoint will be announced/signaled to each peer via an ADD_ADDR MPTCP
> 
>     ^^^
> Suggest capitalizing the first "The" since the preceeding "signal" is not 
> part of the sentence.
> 
> > +sub-option. Upon reception of an ADD_ADDR option, the peer can try to create
> > +additional subflows, see
> > +.BR ADD_ADDR_ACCEPTED_NR.
> > 
> > .TP
> > .BR subflow
> > -if additional subflow creation is allowed by MPTCP limits, the endpoint will
> > -be used as the source address to create an additional subflow after that
> > -the MPTCP connection is established.
> > +if additional subflow creation is allowed by MPTCP limits, after that the
> 
>     ^^
> Capitalize here too.
> 
> > +MPTCP connection is established, the MPTCP path manager will try to create
> > +an additional subflow using this endpoint as the source address.
> 
> I would rearrange this one:
> 
> If additional subflow creation is allowed by the MPTCP limits, the MPTCP
> path manager will try to create an additional subflow using this endpoint
> as the source address after the MPTCP connection is established.
> 
> > 
> > .TP
> > .BR backup
> > -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
> > +if this is a
> 
>     ^^
> 
> Capitalize.
> 
> > +.BR subflow
> > +endpoint, the subflows created using this endpoint will be backup ones.
> > +Backup subflows are used only when all non-backup subflows are unavailable.
> > +
> 
> Suggestion:
> 
> If this is a
> .BR subflow
> 
> endpoint, the subflows created using this endpoint will have the backup 
> flag set during the connection process. This flag instructs the peer to 
> only send data on a given subflow when all non-backup subflows are 
> unavailable. This does not affect outgoing data, where subflow priority 
> is determined by the backup/non-backup flag received from the peer.

Agreed
> 
> 
> > +.TP
> > +.BR fullmesh
> > +if this is a
> 
>     ^^
> "If"
> 
> > +.BR subflow
> > +endpoint, if additional subflow creation is allowed by MPTCP limits, after that the
> > +each MPTCP connection is established, the MPTCP path manager will try to create
> > +an additional subflow for each known server address using this endpoint as
> > +the source address. If the peer did not announce any additional address via
> > +ADD_ADDR MPTCP sub-option, this has no additional effect compared to a plain
> > .BR subflow
> > -endpoint
> > +endpoint. Otherwise, this will create an additional subflow for each received
> > +ADD_ADDR, generating a fullmesh topology.
> 
> Suggestion:
> 
> If this is a
> .BR subflow
> endpoint and additional subflow creation is allowed by the MPTCP limits, 
> the MPTCP path manager will try to create an additional subflow for each 
> known peer address, using this endpoint as the source address. This will 
> occur after the MPTCP connection is established. If the peer did not 
> announce any additional addresses using the ADD_ADDR MPTCP sub-option, 
> this will behave the same as a plain
> .BR subflow
> endpoint. When the peer does announce addresses, each received ADD_ADDR 
> will trigger creation of an additional subflow to generate a full mesh 
> topology.

Agreed

> 
> 
> > 
> > .sp
> > .PP
> > @@ -143,8 +159,17 @@ endpoints, additional subflows started by the peer.
> > .TP
> > .IR ADD_ADDR_ACCEPTED_NR
> > specifies the maximum number of ADD_ADDR suboptions accepted for each MPTCP
> > -connection. The MPTCP path manager will try to create a new subflow for
> > -each accepted ADD_ADDR option, respecting the
> > +connection.
> 
> splitting here for edits
> 
> > When and ADD_ADDR suboption is accepted, if there are no local
> > +.BR fullmesh
> > +endpoint, the MPTCP path manager will try to create a new subflow towards the
> > +address specified by the ADD_ADDR option, using as source the local address
> > +obtained by the routing resolution.
> 
> "When an ADD_ADDR sub-option is received and there are no local fullmesh 
> endpoints, the MPTCP path manager will try to create a new subflow using 
> the address in the ADD_ADDR as the destination address and a source 
> address determined using local routing resolution. When fullmesh endpoints 
> are available, the MPTCP path manager will try to create new subflows 
> using each fullmesh endpoint as a source address and the peer's ADD_ADDR 
> address as the destination. In both cases the SUBFLOW_NR limit is 
> enforced."
> 
> (I hope I kept the intended meaning here?)

I think it's would be better retaining "sub-option is accepted" instead
of "received", so we related to the above paragraph.
> 
> 
> I think there is a mix of "ADD_ADDR option", "ADD_ADDR suboption", and 
> "ADD_ADDR sub-option" above, even after my edits. Should probably choose 
> one and be consistent with it!

I will try to stick to "ADD_ADDR sub-option"

Thanks!

Paolo