* [MPTCP] Re: [MPTCP][PATCH v2 mptcp-next 1/7] mptcp: move ifindex and flags out of mptcp_addr_info
@ 2021-03-17 0:41 Mat Martineau
0 siblings, 0 replies; only message in thread
From: Mat Martineau @ 2021-03-17 0:41 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 9060 bytes --]
On Tue, 16 Mar 2021, Geliang Tang wrote:
> This patch moved the ifindex and flags fields from struct mptcp_addr_info
> to struct mptcp_pm_addr_entry.
>
> It __mptcp_subflow_connect, use container_of to get mptcp_pm_addr_entry
> from its parameter loc, then read out the entry's ifindex and flags values.
>
> Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
> ---
> net/mptcp/pm_netlink.c | 47 ++++++++++++++++++------------------------
> net/mptcp/protocol.h | 11 ++++++++--
> net/mptcp/subflow.c | 6 ++++--
> 3 files changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index a62f887c5198..181d8048cac1 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -22,13 +22,6 @@ static struct genl_family mptcp_genl_family;
>
> static int pm_nl_pernet_id;
>
> -struct mptcp_pm_addr_entry {
> - struct list_head list;
> - struct mptcp_addr_info addr;
> - struct rcu_head rcu;
> - struct socket *lsk;
> -};
> -
> struct mptcp_pm_add_entry {
> struct list_head list;
> struct mptcp_addr_info addr;
> @@ -168,7 +161,7 @@ select_local_address(const struct pm_nl_pernet *pernet,
> rcu_read_lock();
> __mptcp_flush_join_list(msk);
> list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
> - if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
> continue;
>
> if (entry->addr.family != sk->sk_family) {
> @@ -206,7 +199,7 @@ select_signal_address(struct pm_nl_pernet *pernet, unsigned int pos)
> * can lead to additional addresses not being announced.
> */
> list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
> - if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> continue;
> if (i++ == pos) {
> ret = entry;
> @@ -484,8 +477,8 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> {
> struct sock *sk = (struct sock *)msk;
> unsigned int add_addr_accept_max;
> + struct mptcp_pm_addr_entry local;
> struct mptcp_addr_info remote;
> - struct mptcp_addr_info local;
> unsigned int subflows_max;
>
> add_addr_accept_max = mptcp_pm_get_add_addr_accept_max(msk);
> @@ -511,10 +504,10 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> if (!remote.port)
> remote.port = sk->sk_dport;
> memset(&local, 0, sizeof(local));
> - local.family = remote.family;
> + local.addr.family = remote.family;
>
> spin_unlock_bh(&msk->pm.lock);
> - __mptcp_subflow_connect(sk, &local, &remote);
> + __mptcp_subflow_connect(sk, &local.addr, &remote);
> spin_lock_bh(&msk->pm.lock);
>
> add_addr_echo:
> @@ -683,7 +676,7 @@ void mptcp_pm_nl_work(struct mptcp_sock *msk)
>
> static bool address_use_port(struct mptcp_pm_addr_entry *entry)
> {
> - return (entry->addr.flags &
> + return (entry->flags &
> (MPTCP_PM_ADDR_FLAG_SIGNAL | MPTCP_PM_ADDR_FLAG_SUBFLOW)) ==
> MPTCP_PM_ADDR_FLAG_SIGNAL;
> }
> @@ -735,11 +728,11 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> if (entry->addr.id > pernet->next_id)
> pernet->next_id = entry->addr.id;
>
> - if (entry->addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> + if (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> addr_max = pernet->add_addr_signal_max;
> WRITE_ONCE(pernet->add_addr_signal_max, addr_max + 1);
> }
> - if (entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
> + if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
> addr_max = pernet->local_addr_max;
> WRITE_ONCE(pernet->local_addr_max, addr_max + 1);
> }
> @@ -841,10 +834,10 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> return -ENOMEM;
>
> entry->addr = skc_local;
> - entry->addr.ifindex = 0;
> - entry->addr.flags = 0;
> entry->addr.id = 0;
> entry->addr.port = 0;
> + entry->ifindex = 0;
> + entry->flags = 0;
> entry->lsk = NULL;
> ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> if (ret < 0)
> @@ -959,14 +952,14 @@ static int mptcp_pm_parse_addr(struct nlattr *attr, struct genl_info *info,
> if (tb[MPTCP_PM_ADDR_ATTR_IF_IDX]) {
> u32 val = nla_get_s32(tb[MPTCP_PM_ADDR_ATTR_IF_IDX]);
>
> - entry->addr.ifindex = val;
> + entry->ifindex = val;
> }
>
> if (tb[MPTCP_PM_ADDR_ATTR_ID])
> entry->addr.id = nla_get_u8(tb[MPTCP_PM_ADDR_ATTR_ID]);
>
> if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
> - entry->addr.flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
> + entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
>
> if (tb[MPTCP_PM_ADDR_ATTR_PORT])
> entry->addr.port = htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
> @@ -1218,11 +1211,11 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
> spin_unlock_bh(&pernet->lock);
> return -EINVAL;
> }
> - if (entry->addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> + if (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> addr_max = pernet->add_addr_signal_max;
> WRITE_ONCE(pernet->add_addr_signal_max, addr_max - 1);
> }
> - if (entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
> + if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
> addr_max = pernet->local_addr_max;
> WRITE_ONCE(pernet->local_addr_max, addr_max - 1);
> }
> @@ -1338,10 +1331,10 @@ static int mptcp_nl_fill_addr(struct sk_buff *skb,
> goto nla_put_failure;
> if (nla_put_u8(skb, MPTCP_PM_ADDR_ATTR_ID, addr->id))
> goto nla_put_failure;
> - if (nla_put_u32(skb, MPTCP_PM_ADDR_ATTR_FLAGS, entry->addr.flags))
> + if (nla_put_u32(skb, MPTCP_PM_ADDR_ATTR_FLAGS, entry->flags))
> goto nla_put_failure;
> - if (entry->addr.ifindex &&
> - nla_put_s32(skb, MPTCP_PM_ADDR_ATTR_IF_IDX, entry->addr.ifindex))
> + if (entry->ifindex &&
> + nla_put_s32(skb, MPTCP_PM_ADDR_ATTR_IF_IDX, entry->ifindex))
> goto nla_put_failure;
>
> if (addr->family == AF_INET &&
> @@ -1569,7 +1562,7 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> if (ret < 0)
> return ret;
>
> - if (addr.addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> + if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> bkup = 1;
>
> list_for_each_entry(entry, &pernet->local_addr_list, list) {
> @@ -1579,9 +1572,9 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> return ret;
>
> if (bkup)
> - entry->addr.flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
> + entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
> else
> - entry->addr.flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
> + entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
> }
> }
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index e53a9568d587..af7624419fb0 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -173,8 +173,6 @@ struct mptcp_addr_info {
> sa_family_t family;
> __be16 port;
> u8 id;
> - u8 flags;
> - int ifindex;
> union {
> struct in_addr addr;
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> @@ -183,6 +181,15 @@ struct mptcp_addr_info {
> };
> };
>
> +struct mptcp_pm_addr_entry {
> + struct list_head list;
> + struct mptcp_addr_info addr;
> + u8 flags;
> + int ifindex;
> + struct rcu_head rcu;
> + struct socket *lsk;
> +};
> +
> enum mptcp_pm_status {
> MPTCP_PM_ADD_ADDR_RECEIVED,
> MPTCP_PM_ADD_ADDR_SEND_ACK,
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 6af443a18bac..f4d754546c2a 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1255,6 +1255,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> {
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct mptcp_subflow_context *subflow;
> + struct mptcp_pm_addr_entry *entry;
> struct sockaddr_storage addr;
> int remote_id = remote->id;
> int local_id = loc->id;
> @@ -1295,7 +1296,8 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> if (addr.ss_family == AF_INET6)
> addrlen = sizeof(struct sockaddr_in6);
> #endif
> - ssk->sk_bound_dev_if = loc->ifindex;
> + entry = container_of(loc, struct mptcp_pm_addr_entry, addr);
I prefer the v1 approach of changing the function parameter. If you'd like
to avoid moving the mptcp_pm_addr_entry struct to protocol.h, maybe add
parameters to this function for flags and ifindex instead?
> + ssk->sk_bound_dev_if = entry->ifindex;
> err = kernel_bind(sf, (struct sockaddr *)&addr, addrlen);
> if (err)
> goto failed;
> @@ -1307,7 +1309,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> subflow->local_id = local_id;
> subflow->remote_id = remote_id;
> subflow->request_join = 1;
> - subflow->request_bkup = !!(loc->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> + subflow->request_bkup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
>
> mptcp_add_pending_subflow(msk, subflow);
> --
> 2.30.2
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-03-17 0:41 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17 0:41 [MPTCP] Re: [MPTCP][PATCH v2 mptcp-next 1/7] mptcp: move ifindex and flags out of mptcp_addr_info Mat Martineau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).