| Message ID | 1244ff06f1715f714ccc21bb164c6c082e8a26f5.1627543032.git.geliangtang@xiaomi.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 8235745392a17ad86ec3e2909c773241d4b648db |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | fullmesh path manager support | expand |
On Thu, 2021-07-29 at 15:20 +0800, Geliang Tang wrote: > From: Geliang Tang <geliangtang@xiaomi.com> > > This patch added a new helper mptcp_pm_get_flags_and_ifindex_by_id(), > and used it in __mptcp_subflow_connect() to get the flags and ifindex > values. > > Then the two arguments flags and ifindex of __mptcp_subflow_connect() > can be dropped. > > Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> > --- > net/mptcp/pm_netlink.c | 25 ++++++++++++++++++++++--- > net/mptcp/protocol.h | 5 +++-- > net/mptcp/subflow.c | 7 +++++-- > 3 files changed, 30 insertions(+), 7 deletions(-) > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index ba0e1d71504d..94c68d6093de 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -462,8 +462,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > check_work_pending(msk); > remote_address((struct sock_common *)sk, &remote); > spin_unlock_bh(&msk->pm.lock); > - __mptcp_subflow_connect(sk, &local->addr, &remote, > - local->flags, local->ifindex); > + __mptcp_subflow_connect(sk, &local->addr, &remote); > spin_lock_bh(&msk->pm.lock); > return; > } > @@ -518,7 +517,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > local.family = remote.family; > > spin_unlock_bh(&msk->pm.lock); > - __mptcp_subflow_connect(sk, &local, &remote, 0, 0); > + __mptcp_subflow_connect(sk, &local, &remote); > spin_lock_bh(&msk->pm.lock); > > add_addr_echo: > @@ -1103,6 +1102,26 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) > return NULL; > } > > +int mptcp_pm_get_flags_and_ifindex_by_id(struct net *net, unsigned int id, > + u8 *flags, int *ifindex) > +{ > + struct mptcp_pm_addr_entry *entry; > + > + rcu_read_lock(); > + entry = __lookup_addr_by_id(net_generic(net, pm_nl_pernet_id), id); > + rcu_read_unlock(); > + > + if (entry) { > + *flags = entry->flags; > + *ifindex = entry->ifindex; > + } else { > + *flags = 0; > + *ifindex = 0; > + } You need to extend the RCU section after the entry reference above, or on preempt-enabled build, the entry could be deleted in between the rcu unlock and the later access. I'm wondering if there are issues with the 0 id ?!? /P
Paolo Abeni <pabeni@redhat.com> 于2021年7月29日周四 下午7:12写道： > > On Thu, 2021-07-29 at 15:20 +0800, Geliang Tang wrote: > > From: Geliang Tang <geliangtang@xiaomi.com> > > > > This patch added a new helper mptcp_pm_get_flags_and_ifindex_by_id(), > > and used it in __mptcp_subflow_connect() to get the flags and ifindex > > values. > > > > Then the two arguments flags and ifindex of __mptcp_subflow_connect() > > can be dropped. > > > > Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> > > --- > > net/mptcp/pm_netlink.c | 25 ++++++++++++++++++++++--- > > net/mptcp/protocol.h | 5 +++-- > > net/mptcp/subflow.c | 7 +++++-- > > 3 files changed, 30 insertions(+), 7 deletions(-) > > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > index ba0e1d71504d..94c68d6093de 100644 > > --- a/net/mptcp/pm_netlink.c > > +++ b/net/mptcp/pm_netlink.c > > @@ -462,8 +462,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > > check_work_pending(msk); > > remote_address((struct sock_common *)sk, &remote); > > spin_unlock_bh(&msk->pm.lock); > > - __mptcp_subflow_connect(sk, &local->addr, &remote, > > - local->flags, local->ifindex); > > + __mptcp_subflow_connect(sk, &local->addr, &remote); > > spin_lock_bh(&msk->pm.lock); > > return; > > } > > @@ -518,7 +517,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > > local.family = remote.family; > > > > spin_unlock_bh(&msk->pm.lock); > > - __mptcp_subflow_connect(sk, &local, &remote, 0, 0); > > + __mptcp_subflow_connect(sk, &local, &remote); > > spin_lock_bh(&msk->pm.lock); > > > > add_addr_echo: > > @@ -1103,6 +1102,26 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) > > return NULL; > > } > > > > +int mptcp_pm_get_flags_and_ifindex_by_id(struct net *net, unsigned int id, > > + u8 *flags, int *ifindex) > > +{ > > + struct mptcp_pm_addr_entry *entry; > > + > > + rcu_read_lock(); > > + entry = __lookup_addr_by_id(net_generic(net, pm_nl_pernet_id), id); > > + rcu_read_unlock(); > > + > > + if (entry) { > > + *flags = entry->flags; > > + *ifindex = entry->ifindex; > > + } else { > > + *flags = 0; > > + *ifindex = 0; > > + } > > You need to extend the RCU section after the entry reference above, or > on preempt-enabled build, the entry could be deleted in between the rcu > unlock and the later access. Thanks Paolo, I'll send a squash-to patch to fix this. > > I'm wondering if there are issues with the 0 id ?!? The 0 id address isn't on the local_addr_list, we should set it's flags and ifindex all to 0. I'll add a non-zero check in the squash-to patch too. Thanks, -Geliang > > /P >
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index ba0e1d71504d..94c68d6093de 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -462,8 +462,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) check_work_pending(msk); remote_address((struct sock_common *)sk, &remote); spin_unlock_bh(&msk->pm.lock); - __mptcp_subflow_connect(sk, &local->addr, &remote, - local->flags, local->ifindex); + __mptcp_subflow_connect(sk, &local->addr, &remote); spin_lock_bh(&msk->pm.lock); return; } @@ -518,7 +517,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) local.family = remote.family; spin_unlock_bh(&msk->pm.lock); - __mptcp_subflow_connect(sk, &local, &remote, 0, 0); + __mptcp_subflow_connect(sk, &local, &remote); spin_lock_bh(&msk->pm.lock); add_addr_echo: @@ -1103,6 +1102,26 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id) return NULL; } +int mptcp_pm_get_flags_and_ifindex_by_id(struct net *net, unsigned int id, + u8 *flags, int *ifindex) +{ + struct mptcp_pm_addr_entry *entry; + + rcu_read_lock(); + entry = __lookup_addr_by_id(net_generic(net, pm_nl_pernet_id), id); + rcu_read_unlock(); + + if (entry) { + *flags = entry->flags; + *ifindex = entry->ifindex; + } else { + *flags = 0; + *ifindex = 0; + } + + return 0; +} + static bool remove_anno_list_by_saddr(struct mptcp_sock *msk, struct mptcp_addr_info *addr) { diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index e8a36ff52af6..d276ce16f126 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -576,8 +576,7 @@ struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk); /* called with sk socket lock held */ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, - const struct mptcp_addr_info *remote, - u8 flags, int ifindex); + const struct mptcp_addr_info *remote); int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock); void mptcp_info2sockaddr(const struct mptcp_addr_info *info, struct sockaddr_storage *addr, @@ -732,6 +731,8 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk, struct mptcp_pm_add_entry * mptcp_lookup_anno_list_by_saddr(struct mptcp_sock *msk, struct mptcp_addr_info *addr); +int mptcp_pm_get_flags_and_ifindex_by_id(struct net *net, unsigned int id, + u8 *flags, int *ifindex); int mptcp_pm_announce_addr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr, diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 1151926d335b..8c43aa14897a 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1355,8 +1355,7 @@ void mptcp_info2sockaddr(const struct mptcp_addr_info *info, } int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, - const struct mptcp_addr_info *remote, - u8 flags, int ifindex) + const struct mptcp_addr_info *remote) { struct mptcp_sock *msk = mptcp_sk(sk); struct mptcp_subflow_context *subflow; @@ -1367,6 +1366,8 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, struct sock *ssk; u32 remote_token; int addrlen; + int ifindex; + u8 flags; int err; if (!mptcp_is_fully_established(sk)) @@ -1390,6 +1391,8 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc, local_id = err; } + mptcp_pm_get_flags_and_ifindex_by_id(sock_net(sk), local_id, + &flags, &ifindex); subflow->remote_key = msk->remote_key; subflow->local_key = msk->local_key; subflow->token = msk->token;