| Message ID | 20250117-net-next-mptcp-pm-misc-cleanup-2-v2-11-61d4fe0586e8@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mptcp: pm: misc cleanups, part 2 | expand |
Hi Matt, On Fri, 2025-01-17 at 19:41 +0100, Matthieu Baerts (NGI0) wrote: > From: Geliang Tang <tanggeliang@kylinos.cn> > > The address id is parsed both in mptcp_pm_nl_get_addr() and > mptcp_userspace_pm_get_addr(), this makes the code somewhat > repetitive. > > So this patch adds a new parameter 'id' for all get_addr() > interfaces. > The address id is only parsed in mptcp_pm_nl_get_addr_doit(), then > pass > it to both mptcp_pm_nl_get_addr() and mptcp_userspace_pm_get_addr(). > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > v2: > - Fix 'attr' no longer being set in mptcp_pm_nl_get_addr(), but still > used in this patch (no longer in the next one). (Simon) mptcp_userspace_pm_get_addr() needs to be updated too. > --- > net/mptcp/pm.c | 20 ++++++++++++++++---- > net/mptcp/pm_netlink.c | 16 ++++------------ > net/mptcp/pm_userspace.c | 14 +++----------- > net/mptcp/protocol.h | 4 ++-- > 4 files changed, 25 insertions(+), 29 deletions(-) > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index > 526e5bca1fa1bb67acb8532ad8b8b819d2f5151c..caf5bfc3cd1ddeb22799c28dec3 > d19b30467b169 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -434,16 +434,28 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, > struct sock_common *skc) > return mptcp_pm_nl_is_backup(msk, &skc_local); > } > > -static int mptcp_pm_get_addr(struct genl_info *info) > +static int mptcp_pm_get_addr(u8 id, struct genl_info *info) > { > if (info->attrs[MPTCP_PM_ATTR_TOKEN]) > - return mptcp_userspace_pm_get_addr(info); > - return mptcp_pm_nl_get_addr(info); > + return mptcp_userspace_pm_get_addr(id, info); > + return mptcp_pm_nl_get_addr(id, info); > } > > int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info > *info) > { > - return mptcp_pm_get_addr(info); > + struct mptcp_pm_addr_entry addr; > + struct nlattr *attr; > + int ret; > + > + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) > + return -EINVAL; > + > + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; > + ret = mptcp_pm_parse_entry(attr, info, false, &addr); > + if (ret < 0) > + return ret; > + > + return mptcp_pm_get_addr(addr.addr.id, info); > } > > static int mptcp_pm_dump_addr(struct sk_buff *msg, struct > netlink_callback *cb) > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index > 853b1ea8680ae753fcb882d8b8f4486519798503..f7da750ab94f7bbffafb258cb0d > 6ff01ad59c0b0 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -1773,23 +1773,15 @@ int mptcp_nl_fill_addr(struct sk_buff *skb, > return -EMSGSIZE; > } > > -int mptcp_pm_nl_get_addr(struct genl_info *info) > +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info) > { > + struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; > struct pm_nl_pernet *pernet = genl_info_pm_nl(info); > - struct mptcp_pm_addr_entry addr, *entry; > + struct mptcp_pm_addr_entry *entry; > struct sk_buff *msg; > - struct nlattr *attr; > void *reply; > int ret; > > - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) > - return -EINVAL; > - > - attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; > - ret = mptcp_pm_parse_entry(attr, info, false, &addr); > - if (ret < 0) > - return ret; > - > msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); > if (!msg) > return -ENOMEM; > @@ -1803,7 +1795,7 @@ int mptcp_pm_nl_get_addr(struct genl_info > *info) > } > > rcu_read_lock(); > - entry = __lookup_addr_by_id(pernet, addr.addr.id); > + entry = __lookup_addr_by_id(pernet, id); > if (!entry) { > NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not > found"); > ret = -EINVAL; > diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c > index > 1246063598c8152eb908586dc2e3bcacaaba0a91..79e2d12e088805ff3f59ecf41f5 > 092df9823c1b4 100644 > --- a/net/mptcp/pm_userspace.c > +++ b/net/mptcp/pm_userspace.c > @@ -684,9 +684,9 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff > *msg, > return ret; > } > > -int mptcp_userspace_pm_get_addr(struct genl_info *info) > +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info) > { > - struct mptcp_pm_addr_entry addr, *entry; > + struct mptcp_pm_addr_entry *entry; > struct mptcp_sock *msk; > struct sk_buff *msg; > struct nlattr *attr; > @@ -694,20 +694,12 @@ int mptcp_userspace_pm_get_addr(struct > genl_info *info) > struct sock *sk; > void *reply; > > - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) > - return ret; > - > msk = mptcp_userspace_pm_get_sock(info); > if (!msk) > return ret; > > sk = (struct sock *)msk; > > - attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; Needs to keep this assignment of 'attr' too ... > - ret = mptcp_pm_parse_entry(attr, info, false, &addr); > - if (ret < 0) > - goto out; > - > msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); > if (!msg) { > ret = -ENOMEM; > @@ -724,7 +716,7 @@ int mptcp_userspace_pm_get_addr(struct genl_info > *info) > > lock_sock(sk); > spin_lock_bh(&msk->pm.lock); > - entry = mptcp_userspace_pm_lookup_addr_by_id(msk, > addr.addr.id); > + entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id); > if (!entry) { > NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not > found"); ... since 'attr' is still used here. I just sent a squash-to patch for this to MPTCP mail list. Thanks, -Geliang > ret = -EINVAL; > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > index > 7fe91a2e170dd40a830c4301960b484017fd11d2..e77920c932442ce1d317fcda8d2 > 561e11d0c2a12 100644 > --- a/net/mptcp/protocol.h > +++ b/net/mptcp/protocol.h > @@ -1132,8 +1132,8 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg, > struct netlink_callback *cb); > int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, > struct netlink_callback *cb); > -int mptcp_pm_nl_get_addr(struct genl_info *info); > -int mptcp_userspace_pm_get_addr(struct genl_info *info); > +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info); > +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info); > > static inline u8 subflow_get_local_id(const struct > mptcp_subflow_context *subflow) > { >
Hi Geliang, On 18/01/2025 00:28, Geliang Tang wrote: > Hi Matt, > > On Fri, 2025-01-17 at 19:41 +0100, Matthieu Baerts (NGI0) wrote: >> From: Geliang Tang <tanggeliang@kylinos.cn> >> >> The address id is parsed both in mptcp_pm_nl_get_addr() and >> mptcp_userspace_pm_get_addr(), this makes the code somewhat >> repetitive. >> >> So this patch adds a new parameter 'id' for all get_addr() >> interfaces. >> The address id is only parsed in mptcp_pm_nl_get_addr_doit(), then >> pass >> it to both mptcp_pm_nl_get_addr() and mptcp_userspace_pm_get_addr(). >> >> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> >> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> v2: >> - Fix 'attr' no longer being set in mptcp_pm_nl_get_addr(), but still >> used in this patch (no longer in the next one). (Simon) > > mptcp_userspace_pm_get_addr() needs to be updated too. Arf, I didn't think about pm_userspace.c :( I should have definitively squashed patch 11 and 12... That's OK, I will resend them later, simply moving the line removal from patch 11 to 12. Cheers, Matt
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 526e5bca1fa1bb67acb8532ad8b8b819d2f5151c..caf5bfc3cd1ddeb22799c28dec3d19b30467b169 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -434,16 +434,28 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc) return mptcp_pm_nl_is_backup(msk, &skc_local); } -static int mptcp_pm_get_addr(struct genl_info *info) +static int mptcp_pm_get_addr(u8 id, struct genl_info *info) { if (info->attrs[MPTCP_PM_ATTR_TOKEN]) - return mptcp_userspace_pm_get_addr(info); - return mptcp_pm_nl_get_addr(info); + return mptcp_userspace_pm_get_addr(id, info); + return mptcp_pm_nl_get_addr(id, info); } int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info) { - return mptcp_pm_get_addr(info); + struct mptcp_pm_addr_entry addr; + struct nlattr *attr; + int ret; + + if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) + return -EINVAL; + + attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; + ret = mptcp_pm_parse_entry(attr, info, false, &addr); + if (ret < 0) + return ret; + + return mptcp_pm_get_addr(addr.addr.id, info); } static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 853b1ea8680ae753fcb882d8b8f4486519798503..f7da750ab94f7bbffafb258cb0d6ff01ad59c0b0 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1773,23 +1773,15 @@ int mptcp_nl_fill_addr(struct sk_buff *skb, return -EMSGSIZE; } -int mptcp_pm_nl_get_addr(struct genl_info *info) +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info) { + struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; struct pm_nl_pernet *pernet = genl_info_pm_nl(info); - struct mptcp_pm_addr_entry addr, *entry; + struct mptcp_pm_addr_entry *entry; struct sk_buff *msg; - struct nlattr *attr; void *reply; int ret; - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) - return -EINVAL; - - attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; - ret = mptcp_pm_parse_entry(attr, info, false, &addr); - if (ret < 0) - return ret; - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) return -ENOMEM; @@ -1803,7 +1795,7 @@ int mptcp_pm_nl_get_addr(struct genl_info *info) } rcu_read_lock(); - entry = __lookup_addr_by_id(pernet, addr.addr.id); + entry = __lookup_addr_by_id(pernet, id); if (!entry) { NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); ret = -EINVAL; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 1246063598c8152eb908586dc2e3bcacaaba0a91..79e2d12e088805ff3f59ecf41f5092df9823c1b4 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -684,9 +684,9 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, return ret; } -int mptcp_userspace_pm_get_addr(struct genl_info *info) +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info) { - struct mptcp_pm_addr_entry addr, *entry; + struct mptcp_pm_addr_entry *entry; struct mptcp_sock *msk; struct sk_buff *msg; struct nlattr *attr; @@ -694,20 +694,12 @@ int mptcp_userspace_pm_get_addr(struct genl_info *info) struct sock *sk; void *reply; - if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR)) - return ret; - msk = mptcp_userspace_pm_get_sock(info); if (!msk) return ret; sk = (struct sock *)msk; - attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR]; - ret = mptcp_pm_parse_entry(attr, info, false, &addr); - if (ret < 0) - goto out; - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); if (!msg) { ret = -ENOMEM; @@ -724,7 +716,7 @@ int mptcp_userspace_pm_get_addr(struct genl_info *info) lock_sock(sk); spin_lock_bh(&msk->pm.lock); - entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id); + entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id); if (!entry) { NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found"); ret = -EINVAL; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 7fe91a2e170dd40a830c4301960b484017fd11d2..e77920c932442ce1d317fcda8d2561e11d0c2a12 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1132,8 +1132,8 @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); int mptcp_userspace_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb); -int mptcp_pm_nl_get_addr(struct genl_info *info); -int mptcp_userspace_pm_get_addr(struct genl_info *info); +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info); +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info); static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow) {