Message ID | 20240726-mptcp-pm-avail-v5-7-fb1117ddeef6@kernel.org (mailing list archive) |
---|---|
State | Accepted, archived |
Commit | 91dc97dedafac4faa7432788ae1c5a52d4ca0d94 |
Delegated to: | Matthieu Baerts |
Headers | show |
Series | mptcp: fix endpoints with 'signal' and 'subflow' flags | expand |
Context | Check | Description |
---|---|---|
matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 19 lines checked |
matttbe/shellcheck | success | MPTCP selftests files have not been modified |
matttbe/build | success | Build and static analysis OK |
matttbe/KVM_Validation__normal | success | Success! ✅ |
matttbe/KVM_Validation__debug | success | Success! ✅ |
matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
On Fri, 2024-07-26 at 16:28 +0200, Matthieu Baerts (NGI0) wrote: > The ID 0 is specific per MPTCP connections. The per netns entries > cannot > have this special ID 0 then. > > But that's different for the userspace PM where the entries are per > connection, they can then use this special ID 0. > > Fixes: f40be0db0b76 ("mptcp: unify pm get_flags_and_ifindex_by_id") > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > net/mptcp/pm.c | 3 --- > net/mptcp/pm_netlink.c | 4 ++++ > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index 925123e99889..3e6e0f5510bb 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -434,9 +434,6 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct > mptcp_sock *msk, unsigned int id > *flags = 0; > *ifindex = 0; > > - if (!id) > - return 0; > - > if (mptcp_pm_is_userspace(msk)) > return > mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, > ifindex); > return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, > flags, ifindex); > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 3cb02fe359c0..6a1495fec7ae 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -1395,6 +1395,10 @@ int > mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, > unsigned int > struct sock *sk = (struct sock *)msk; > struct net *net = sock_net(sk); > > + /* No entries with ID 0 */ > + if (id == 0) > + return 0; > + I think this check needs to be added in mptcp_userspace_pm_get_flags_and_ifindex_by_id() too. > rcu_read_lock(); > entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id); > if (entry) { >
Hi Geliang, On 29/07/2024 03:12, Geliang Tang wrote: > On Fri, 2024-07-26 at 16:28 +0200, Matthieu Baerts (NGI0) wrote: >> The ID 0 is specific per MPTCP connections. The per netns entries >> cannot >> have this special ID 0 then. >> >> But that's different for the userspace PM where the entries are per >> connection, they can then use this special ID 0. >> >> Fixes: f40be0db0b76 ("mptcp: unify pm get_flags_and_ifindex_by_id") >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> net/mptcp/pm.c | 3 --- >> net/mptcp/pm_netlink.c | 4 ++++ >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c >> index 925123e99889..3e6e0f5510bb 100644 >> --- a/net/mptcp/pm.c >> +++ b/net/mptcp/pm.c >> @@ -434,9 +434,6 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct >> mptcp_sock *msk, unsigned int id >> *flags = 0; >> *ifindex = 0; >> >> - if (!id) >> - return 0; >> - >> if (mptcp_pm_is_userspace(msk)) >> return >> mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, >> ifindex); >> return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, >> flags, ifindex); >> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c >> index 3cb02fe359c0..6a1495fec7ae 100644 >> --- a/net/mptcp/pm_netlink.c >> +++ b/net/mptcp/pm_netlink.c >> @@ -1395,6 +1395,10 @@ int >> mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, >> unsigned int >> struct sock *sk = (struct sock *)msk; >> struct net *net = sock_net(sk); >> >> + /* No entries with ID 0 */ >> + if (id == 0) >> + return 0; >> + > > I think this check needs to be added in > mptcp_userspace_pm_get_flags_and_ifindex_by_id() too. We don't need it for the userspace PM, because the endpoints are managed per MPTCP connection, they can have an ID 0. On the other hand, the in-kernel PM manages the endpoints globally, they cannot have the ID 0 because it is reserved to the address used by the initial subflow, which can be different per MPTCP connection. Please see the commit message for more details. > >> rcu_read_lock(); >> entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id); >> if (entry) { >> > Cheers, Matt
On Mon, 2024-07-29 at 11:29 +0200, Matthieu Baerts wrote: > Hi Geliang, > > On 29/07/2024 03:12, Geliang Tang wrote: > > On Fri, 2024-07-26 at 16:28 +0200, Matthieu Baerts (NGI0) wrote: > > > The ID 0 is specific per MPTCP connections. The per netns entries > > > cannot > > > have this special ID 0 then. > > > > > > But that's different for the userspace PM where the entries are > > > per > > > connection, they can then use this special ID 0. > > > > > > Fixes: f40be0db0b76 ("mptcp: unify pm > > > get_flags_and_ifindex_by_id") > > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > > --- > > > net/mptcp/pm.c | 3 --- > > > net/mptcp/pm_netlink.c | 4 ++++ > > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > > > index 925123e99889..3e6e0f5510bb 100644 > > > --- a/net/mptcp/pm.c > > > +++ b/net/mptcp/pm.c > > > @@ -434,9 +434,6 @@ int > > > mptcp_pm_get_flags_and_ifindex_by_id(struct > > > mptcp_sock *msk, unsigned int id > > > *flags = 0; > > > *ifindex = 0; > > > > > > - if (!id) > > > - return 0; > > > - > > > if (mptcp_pm_is_userspace(msk)) > > > return > > > mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, > > > ifindex); > > > return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, > > > flags, ifindex); > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > > index 3cb02fe359c0..6a1495fec7ae 100644 > > > --- a/net/mptcp/pm_netlink.c > > > +++ b/net/mptcp/pm_netlink.c > > > @@ -1395,6 +1395,10 @@ int > > > mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, > > > unsigned int > > > struct sock *sk = (struct sock *)msk; > > > struct net *net = sock_net(sk); > > > > > > + /* No entries with ID 0 */ > > > + if (id == 0) > > > + return 0; > > > + > > > > I think this check needs to be added in > > mptcp_userspace_pm_get_flags_and_ifindex_by_id() too. > > We don't need it for the userspace PM, because the endpoints are > managed > per MPTCP connection, they can have an ID 0. On the other hand, the > in-kernel PM manages the endpoints globally, they cannot have the ID > 0 > because it is reserved to the address used by the initial subflow, > which > can be different per MPTCP connection. > > Please see the commit message for more details. Thanks for your explanation. Acked-by: Geliang Tang <geliang@kernel.org> > > > > > > rcu_read_lock(); > > > entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id); > > > if (entry) { > > > > > > > Cheers, > Matt
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 925123e99889..3e6e0f5510bb 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -434,9 +434,6 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id *flags = 0; *ifindex = 0; - if (!id) - return 0; - if (mptcp_pm_is_userspace(msk)) return mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, ifindex); return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, flags, ifindex); diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 3cb02fe359c0..6a1495fec7ae 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1395,6 +1395,10 @@ int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int struct sock *sk = (struct sock *)msk; struct net *net = sock_net(sk); + /* No entries with ID 0 */ + if (id == 0) + return 0; + rcu_read_lock(); entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id); if (entry) {
The ID 0 is specific per MPTCP connections. The per netns entries cannot have this special ID 0 then. But that's different for the userspace PM where the entries are per connection, they can then use this special ID 0. Fixes: f40be0db0b76 ("mptcp: unify pm get_flags_and_ifindex_by_id") Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- net/mptcp/pm.c | 3 --- net/mptcp/pm_netlink.c | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-)