include/uapi/linux/mptcp.h | 1 + net/mptcp/pm_netlink.c | 84 ++++++------ net/mptcp/protocol.c | 3 + net/mptcp/protocol.h | 3 +- net/mptcp/subflow.c | 67 ++++++++-- .../testing/selftests/net/mptcp/mptcp_join.sh | 121 +++++++++++++++++- tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 7 + 7 files changed, 234 insertions(+), 52 deletions(-)
This iteration tries to address the feedback from Mat on v4. patch 1/4 is new and tries to address the problem behind issues/225 with stricted RFC compliance patch 2 and 3 clean-up endpoint management as per past discussion patch 4 introduce new self-tests, as deserved the previous patch Paolo Abeni (4): mptcp: more careful RM_ADDR generation mptcp: introduce implicit endpoints mptcp: strict local address ID selection. selftests: mptcp: add implicit endpoint test case include/uapi/linux/mptcp.h | 1 + net/mptcp/pm_netlink.c | 84 ++++++------ net/mptcp/protocol.c | 3 + net/mptcp/protocol.h | 3 +- net/mptcp/subflow.c | 67 ++++++++-- .../testing/selftests/net/mptcp/mptcp_join.sh | 121 +++++++++++++++++- tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 7 + 7 files changed, 234 insertions(+), 52 deletions(-) -- 2.34.1
On Thu, 17 Feb 2022, Paolo Abeni wrote:
> This iteration tries to address the feedback from Mat on v4.
>
> patch 1/4 is new and tries to address the problem behind issues/225 with
> stricted RFC compliance
> patch 2 and 3 clean-up endpoint management as per past discussion
> patch 4 introduce new self-tests, as deserved the previous patch
>
Hi Paolo -
These changes and tests are working well on my system and fits with what
we discussed on the previous revisions - thanks!
I was taking another look at the RFC and saw this paragraph:
The MP_JOIN option includes an "Address ID". This is an identifier
generated by the sender of the option, used to identify the source
address of this packet, even if the IP header has been changed in
transit by a middlebox. The numeric value of this field is generated
by the sender and must map uniquely to a source IP address for the
sending host. The Address ID allows address removal (Section 3.4.2)
without needing to know what the source address at the receiver is,
thus allowing address removal through NATs. The Address ID also
allows correlation between new subflow setup attempts and address
signaling (Section 3.4.1), to prevent setting up duplicate subflows
on the same path, if an MP_JOIN and ADD_ADDR are sent at the same
time.
That made me wonder if we are supposed to treat this address ID as a type
of advertisement which would require us to not reuse id 0? I think the
answer is "no" because nothing in the REMOVE_ADDR section (3.4.2) seems to
apply to joins, and it's also pretty permissive about not sending
REMOVE_ADDR. We also won't be sending REMOVE_ADDR for id 0, so the new
code seems safe in that regard.
Hopefully you agree with the above and I'm just catching up to what you
understood about the RFC already :)
That's a long way to say, I think this patch set looks ok for the export
branch:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Paolo Abeni (4):
> mptcp: more careful RM_ADDR generation
> mptcp: introduce implicit endpoints
> mptcp: strict local address ID selection.
> selftests: mptcp: add implicit endpoint test case
>
> include/uapi/linux/mptcp.h | 1 +
> net/mptcp/pm_netlink.c | 84 ++++++------
> net/mptcp/protocol.c | 3 +
> net/mptcp/protocol.h | 3 +-
> net/mptcp/subflow.c | 67 ++++++++--
> .../testing/selftests/net/mptcp/mptcp_join.sh | 121 +++++++++++++++++-
> tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 7 +
> 7 files changed, 234 insertions(+), 52 deletions(-)
>
> --
> 2.34.1
>
>
>
--
Mat Martineau
Intel
On Thu, 2022-02-17 at 17:35 -0800, Mat Martineau wrote: > On Thu, 17 Feb 2022, Paolo Abeni wrote: > > > This iteration tries to address the feedback from Mat on v4. > > > > patch 1/4 is new and tries to address the problem behind issues/225 with > > stricted RFC compliance > > patch 2 and 3 clean-up endpoint management as per past discussion > > patch 4 introduce new self-tests, as deserved the previous patch > > > > Hi Paolo - > > These changes and tests are working well on my system and fits with what > we discussed on the previous revisions - thanks! > > I was taking another look at the RFC and saw this paragraph: > > The MP_JOIN option includes an "Address ID". This is an identifier > generated by the sender of the option, used to identify the source > address of this packet, even if the IP header has been changed in > transit by a middlebox. The numeric value of this field is generated > by the sender and must map uniquely to a source IP address for the > sending host. The Address ID allows address removal (Section 3.4.2) > without needing to know what the source address at the receiver is, > thus allowing address removal through NATs. The Address ID also > allows correlation between new subflow setup attempts and address > signaling (Section 3.4.1), to prevent setting up duplicate subflows > on the same path, if an MP_JOIN and ADD_ADDR are sent at the same > time. > > That made me wonder if we are supposed to treat this address ID as a type > of advertisement which would require us to not reuse id 0? I think the > answer is "no" because nothing in the REMOVE_ADDR section (3.4.2) seems to > apply to joins, and it's also pretty permissive about not sending > REMOVE_ADDR. We also won't be sending REMOVE_ADDR for id 0, so the new > code seems safe in that regard. Double checking if I parsed the above correctly. The main point/question is: which ID should use additional MPJ subflows with the same source address of the initial MPC subflow? My answer/what the current code is doing prior and after this patch is 'id 0'. I read the above as this is also your answer. Please correct me if I misinterpret something! > > Hopefully you agree with the above and I'm just catching up to what you > understood about the RFC already :) > > That's a long way to say, I think this patch set looks ok for the export > branch: > > Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Thanks! /P
On Fri, 18 Feb 2022, Paolo Abeni wrote: > On Thu, 2022-02-17 at 17:35 -0800, Mat Martineau wrote: >> On Thu, 17 Feb 2022, Paolo Abeni wrote: >> >>> This iteration tries to address the feedback from Mat on v4. >>> >>> patch 1/4 is new and tries to address the problem behind issues/225 with >>> stricted RFC compliance >>> patch 2 and 3 clean-up endpoint management as per past discussion >>> patch 4 introduce new self-tests, as deserved the previous patch >>> >> >> Hi Paolo - >> >> These changes and tests are working well on my system and fits with what >> we discussed on the previous revisions - thanks! >> >> I was taking another look at the RFC and saw this paragraph: >> >> The MP_JOIN option includes an "Address ID". This is an identifier >> generated by the sender of the option, used to identify the source >> address of this packet, even if the IP header has been changed in >> transit by a middlebox. The numeric value of this field is generated >> by the sender and must map uniquely to a source IP address for the >> sending host. The Address ID allows address removal (Section 3.4.2) >> without needing to know what the source address at the receiver is, >> thus allowing address removal through NATs. The Address ID also >> allows correlation between new subflow setup attempts and address >> signaling (Section 3.4.1), to prevent setting up duplicate subflows >> on the same path, if an MP_JOIN and ADD_ADDR are sent at the same >> time. >> >> That made me wonder if we are supposed to treat this address ID as a type >> of advertisement which would require us to not reuse id 0? I think the >> answer is "no" because nothing in the REMOVE_ADDR section (3.4.2) seems to >> apply to joins, and it's also pretty permissive about not sending >> REMOVE_ADDR. We also won't be sending REMOVE_ADDR for id 0, so the new >> code seems safe in that regard. > > Double checking if I parsed the above correctly. The main > point/question is: which ID should use additional MPJ subflows with the > same source address of the initial MPC subflow? My answer/what the > current code is doing prior and after this patch is 'id 0'. I read the > above as this is also your answer. > > Please correct me if I misinterpret something! > That's part of it. I also wasn't sure if the MPJ syn could also be sent with addr id 0 if the kernel picked some other interface for sending that syn that had not been configured in the path manager - a corner case that I thought had been mentioned earlier in discussion of this patch series. If the outgoing MPJ only has addr id 0 if the MPC source addr is used then that's definitely not any kind of problem. -- Mat Martineau Intel
On Fri, 2022-02-18 at 12:33 -0800, Mat Martineau wrote: > On Fri, 18 Feb 2022, Paolo Abeni wrote: > > > On Thu, 2022-02-17 at 17:35 -0800, Mat Martineau wrote: > > > On Thu, 17 Feb 2022, Paolo Abeni wrote: > > > > > > > This iteration tries to address the feedback from Mat on v4. > > > > > > > > patch 1/4 is new and tries to address the problem behind issues/225 with > > > > stricted RFC compliance > > > > patch 2 and 3 clean-up endpoint management as per past discussion > > > > patch 4 introduce new self-tests, as deserved the previous patch > > > > > > > > > > Hi Paolo - > > > > > > These changes and tests are working well on my system and fits with what > > > we discussed on the previous revisions - thanks! > > > > > > I was taking another look at the RFC and saw this paragraph: > > > > > > The MP_JOIN option includes an "Address ID". This is an identifier > > > generated by the sender of the option, used to identify the source > > > address of this packet, even if the IP header has been changed in > > > transit by a middlebox. The numeric value of this field is generated > > > by the sender and must map uniquely to a source IP address for the > > > sending host. The Address ID allows address removal (Section 3.4.2) > > > without needing to know what the source address at the receiver is, > > > thus allowing address removal through NATs. The Address ID also > > > allows correlation between new subflow setup attempts and address > > > signaling (Section 3.4.1), to prevent setting up duplicate subflows > > > on the same path, if an MP_JOIN and ADD_ADDR are sent at the same > > > time. > > > > > > That made me wonder if we are supposed to treat this address ID as a type > > > of advertisement which would require us to not reuse id 0? I think the > > > answer is "no" because nothing in the REMOVE_ADDR section (3.4.2) seems to > > > apply to joins, and it's also pretty permissive about not sending > > > REMOVE_ADDR. We also won't be sending REMOVE_ADDR for id 0, so the new > > > code seems safe in that regard. > > > > Double checking if I parsed the above correctly. The main > > point/question is: which ID should use additional MPJ subflows with the > > same source address of the initial MPC subflow? My answer/what the > > current code is doing prior and after this patch is 'id 0'. I read the > > above as this is also your answer. > > > > Please correct me if I misinterpret something! > > > > That's part of it. I also wasn't sure if the MPJ syn could also be sent > with addr id 0 if the kernel picked some other interface for sending that > syn that had not been configured in the path manager - a corner case that > I thought had been mentioned earlier in discussion of this patch series. I think now it's much clearer to me, thanks! > If the outgoing MPJ only has addr id 0 if the MPC source addr is used then > that's definitely not any kind of problem. That is addressed by patch 3/4: prior to such patch, MPJ subflows using local addresses not mapped by user-defined endpoint would select the 0 ID. After that patch such subflows will create a new IMPLICIT endpoint at rebuild_header time will use an unique ID number. Thanks, Paolo
Hi Paolo, Mat, On 17/02/2022 22:44, Paolo Abeni wrote: > This iteration tries to address the feedback from Mat on v4. > > patch 1/4 is new and tries to address the problem behind issues/225 with > stricted RFC compliance > patch 2 and 3 clean-up endpoint management as per past discussion > patch 4 introduce new self-tests, as deserved the previous patch Thank you for the patches and reviews! Now in our tree (feat. for net-next) with Mat's RvB tags and minor modifications on the last patch: - 1c0e3277fab9: mptcp: more careful RM_ADDR generation - 1908304713ea: mptcp: introduce implicit endpoints - 25124126a6c0: mptcp: strict local address ID selection - 7532760df8bc: selftests: mptcp: add implicit endpoint test case - With minor modifications (missing local + help usage) - https://paste.opendev.org/show/812824 - Results: 78bc7e06c12d..785fa0377037 Builds and tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220219T171438 https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
© 2016 - 2024 Red Hat, Inc.