* [MPTCP] [MPTCP][PATCH v3 mptcp-next 0/7] refactor mptcp_addr_info and cleanups @ 2021-03-17 7:36 Geliang Tang 2021-03-17 7:36 ` [MPTCP] [MPTCP][PATCH v3 mptcp-next 1/7] mptcp: move flags and ifindex out of mptcp_addr_info Geliang Tang [not found] ` <22cecfe9-709a-f4f2-b137-63df1227ed43@linux.intel.com> 0 siblings, 2 replies; 6+ messages in thread From: Geliang Tang @ 2021-03-17 7:36 UTC (permalink / raw) To: mptcp [-- Attachment #1: Type: text/plain, Size: 1268 bytes --] v3: - Add new parameters flags and ifindex to __mptcp_subflow_connect. - Drop the patch "mptcp: drop unnecessary CONFIG_MPTCP_IPV6" in v2. - Add a new selftest patch. v2: - Patch 1, avoid changing __mptcp_subflow_connect's parameter, use container_of to get the entry. - No change in patches 2-7. The patch set refactored struct mptcp_addr_info, and use it in both mptcp_out_options and mptcp_out_options. Then drop the duplicate code and do cleanups. Geliang Tang (7): mptcp: move flags and ifindex out of mptcp_addr_info mptcp: use mptcp_addr_info in mptcp_out_options mptcp: drop OPTION_MPTCP_ADD_ADDR6 mptcp: use mptcp_addr_info in mptcp_options_received mptcp: drop MPTCP_ADDR_IPVERSION_4/6 mptcp: unify add_addr(6)_generate_hmac selftests: mptcp: add the net device name testcase include/net/mptcp.h | 21 ++- net/mptcp/options.c | 169 ++++++------------ net/mptcp/pm_netlink.c | 41 +++-- net/mptcp/protocol.h | 38 +--- net/mptcp/subflow.c | 7 +- .../testing/selftests/net/mptcp/mptcp_join.sh | 8 + 6 files changed, 110 insertions(+), 174 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [MPTCP] [MPTCP][PATCH v3 mptcp-next 1/7] mptcp: move flags and ifindex out of mptcp_addr_info @ 2021-03-17 7:36 ` Geliang Tang 2021-03-17 7:36 ` [MPTCP] [MPTCP][PATCH v3 mptcp-next 2/7] mptcp: use mptcp_addr_info in mptcp_out_options Geliang Tang 0 siblings, 1 reply; 6+ messages in thread From: Geliang Tang @ 2021-03-17 7:36 UTC (permalink / raw) To: mptcp [-- Attachment #1: Type: text/plain, Size: 8571 bytes --] This patch moved the flags and ifindex fields from struct mptcp_addr_info to struct mptcp_pm_addr_entry. Add the flags and ifindex values as two new parameters to __mptcp_subflow_connect. In mptcp_pm_create_subflow_or_signal_addr, pass the local address entry's flags and ifindex fields to __mptcp_subflow_connect. In mptcp_pm_nl_add_addr_received, just pass two zeros to it. Signed-off-by: Geliang Tang <geliangtang(a)gmail.com> --- net/mptcp/pm_netlink.c | 41 ++++++++++++++++++++++------------------- net/mptcp/protocol.h | 5 ++--- net/mptcp/subflow.c | 7 ++++--- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index a62f887c5198..745073ddded8 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -25,6 +25,8 @@ static int pm_nl_pernet_id; 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; }; @@ -168,7 +170,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 +208,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; @@ -459,7 +461,8 @@ 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); + __mptcp_subflow_connect(sk, &local->addr, &remote, + local->flags, local->ifindex); spin_lock_bh(&msk->pm.lock); return; } @@ -514,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); + __mptcp_subflow_connect(sk, &local, &remote, 0, 0); spin_lock_bh(&msk->pm.lock); add_addr_echo: @@ -683,7 +686,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 +738,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 +844,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 +962,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 +1221,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 +1341,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 +1572,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 +1582,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..9005ccc2bc7d 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) @@ -557,7 +555,8 @@ 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); + const struct mptcp_addr_info *remote, + u8 flags, int ifindex); 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, diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 6af443a18bac..5fc3cada11dd 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1251,7 +1251,8 @@ 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) + const struct mptcp_addr_info *remote, + u8 flags, int ifindex) { struct mptcp_sock *msk = mptcp_sk(sk); struct mptcp_subflow_context *subflow; @@ -1295,7 +1296,7 @@ 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; + ssk->sk_bound_dev_if = ifindex; err = kernel_bind(sf, (struct sockaddr *)&addr, addrlen); if (err) goto failed; @@ -1307,7 +1308,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 = !!(flags & MPTCP_PM_ADDR_FLAG_BACKUP); mptcp_info2sockaddr(remote, &addr, ssk->sk_family); mptcp_add_pending_subflow(msk, subflow); -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [MPTCP] [MPTCP][PATCH v3 mptcp-next 2/7] mptcp: use mptcp_addr_info in mptcp_out_options @ 2021-03-17 7:36 ` Geliang Tang 2021-03-19 4:13 ` [MPTCP] " Mat Martineau 0 siblings, 1 reply; 6+ messages in thread From: Geliang Tang @ 2021-03-17 7:36 UTC (permalink / raw) To: mptcp [-- Attachment #1: Type: text/plain, Size: 5949 bytes --] This patch moved the mptcp_addr_info struct from protocol.h to mptcp.h, added a new struct mptcp_addr_info member addr in struct mptcp_out_options, and dropped the original addr, addr6, addr_id and port fields in it. Then we can use opts->addr to get the adding address from PM directly using mptcp_pm_add_addr_signal. Since the port number became as a big-endian order now, use ntohs to convert it before printing it out. Signed-off-by: Geliang Tang <geliangtang(a)gmail.com> --- include/net/mptcp.h | 21 +++++++++++++-------- net/mptcp/options.c | 42 ++++++++++++++++++------------------------ net/mptcp/protocol.h | 12 ------------ 3 files changed, 31 insertions(+), 44 deletions(-) diff --git a/include/net/mptcp.h b/include/net/mptcp.h index 16fe34d139c3..80d98a7db3c6 100644 --- a/include/net/mptcp.h +++ b/include/net/mptcp.h @@ -41,20 +41,25 @@ struct mptcp_rm_list { u8 nr; }; +struct mptcp_addr_info { + u8 id; + sa_family_t family : 4; + __be16 port; + union { + struct in_addr addr; +#if IS_ENABLED(CONFIG_MPTCP_IPV6) + struct in6_addr addr6; +#endif + }; +}; + struct mptcp_out_options { #if IS_ENABLED(CONFIG_MPTCP) u16 suboptions; u64 sndr_key; u64 rcvr_key; - union { - struct in_addr addr; -#if IS_ENABLED(CONFIG_MPTCP_IPV6) - struct in6_addr addr6; -#endif - }; - u8 addr_id; - u16 port; u64 ahmac; + struct mptcp_addr_info addr; struct mptcp_rm_list rm_list; u8 join_id; u8 backup; diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 4b7119eb2c31..7e01f44ed885 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -626,7 +626,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * struct mptcp_sock *msk = mptcp_sk(subflow->conn); bool drop_other_suboptions = false; unsigned int opt_size = *size; - struct mptcp_addr_info saddr; bool echo; bool port; int len; @@ -643,45 +642,40 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * } if (!mptcp_pm_should_add_signal(msk) || - !(mptcp_pm_add_addr_signal(msk, remaining, &saddr, &echo, &port))) + !(mptcp_pm_add_addr_signal(msk, remaining, &opts->addr, &echo, &port))) return false; - len = mptcp_add_addr_len(saddr.family, echo, port); + len = mptcp_add_addr_len(opts->addr.family, echo, port); if (remaining < len) return false; *size = len; if (drop_other_suboptions) *size -= opt_size; - opts->addr_id = saddr.id; - if (port) - opts->port = ntohs(saddr.port); - if (saddr.family == AF_INET) { + if (opts->addr.family == AF_INET) { opts->suboptions |= OPTION_MPTCP_ADD_ADDR; - opts->addr = saddr.addr; if (!echo) { opts->ahmac = add_addr_generate_hmac(msk->local_key, msk->remote_key, - opts->addr_id, - &opts->addr, - opts->port); + opts->addr.id, + &opts->addr.addr, + opts->addr.port); } } #if IS_ENABLED(CONFIG_MPTCP_IPV6) - else if (saddr.family == AF_INET6) { + else if (opts->addr.family == AF_INET6) { opts->suboptions |= OPTION_MPTCP_ADD_ADDR6; - opts->addr6 = saddr.addr6; if (!echo) { opts->ahmac = add_addr6_generate_hmac(msk->local_key, msk->remote_key, - opts->addr_id, - &opts->addr6, - opts->port); + opts->addr.id, + &opts->addr.addr6, + opts->addr.port); } } #endif pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d", - opts->addr_id, opts->ahmac, echo, opts->port); + opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port)); return true; } @@ -1217,7 +1211,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, len = TCPOLEN_MPTCP_ADD_ADDR6_BASE; #endif - if (opts->port) + if (opts->addr.port) len += TCPOLEN_MPTCP_PORT_LEN; if (opts->ahmac) { @@ -1226,19 +1220,19 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, } *ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, - len, echo, opts->addr_id); + len, echo, opts->addr.id); if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) { - memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4); + memcpy((u8 *)ptr, (u8 *)&opts->addr.addr.s_addr, 4); ptr += 1; } #if IS_ENABLED(CONFIG_MPTCP_IPV6) else if (OPTION_MPTCP_ADD_ADDR6 & opts->suboptions) { - memcpy((u8 *)ptr, opts->addr6.s6_addr, 16); + memcpy((u8 *)ptr, opts->addr.addr6.s6_addr, 16); ptr += 4; } #endif - if (!opts->port) { + if (!opts->addr.port) { if (opts->ahmac) { put_unaligned_be64(opts->ahmac, ptr); ptr += 2; @@ -1247,7 +1241,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, if (opts->ahmac) { u8 *bptr = (u8 *)ptr; - put_unaligned_be16(opts->port, bptr); + put_unaligned_be16(opts->addr.port, bptr); bptr += 2; put_unaligned_be64(opts->ahmac, bptr); bptr += 8; @@ -1256,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, ptr += 3; } else { - put_unaligned_be32(opts->port << 16 | + put_unaligned_be32(opts->addr.port << 16 | TCPOPT_NOP << 8 | TCPOPT_NOP, ptr); ptr += 1; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 9005ccc2bc7d..b993e372c4ad 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -169,18 +169,6 @@ static inline __be32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field) ((nib & 0xF) << 8) | field); } -struct mptcp_addr_info { - sa_family_t family; - __be16 port; - u8 id; - union { - struct in_addr addr; -#if IS_ENABLED(CONFIG_MPTCP_IPV6) - struct in6_addr addr6; -#endif - }; -}; - enum mptcp_pm_status { MPTCP_PM_ADD_ADDR_RECEIVED, MPTCP_PM_ADD_ADDR_SEND_ACK, -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 2/7] mptcp: use mptcp_addr_info in mptcp_out_options @ 2021-03-19 4:13 ` Mat Martineau 2021-03-19 7:14 ` Geliang Tang 0 siblings, 1 reply; 6+ messages in thread From: Mat Martineau @ 2021-03-19 4:13 UTC (permalink / raw) To: mptcp [-- Attachment #1: Type: text/plain, Size: 6816 bytes --] On Wed, 17 Mar 2021, Geliang Tang wrote: > This patch moved the mptcp_addr_info struct from protocol.h to mptcp.h, > added a new struct mptcp_addr_info member addr in struct mptcp_out_options, > and dropped the original addr, addr6, addr_id and port fields in it. Then > we can use opts->addr to get the adding address from PM directly using > mptcp_pm_add_addr_signal. > > Since the port number became as a big-endian order now, use ntohs to > convert it before printing it out. > > Signed-off-by: Geliang Tang <geliangtang(a)gmail.com> > --- > include/net/mptcp.h | 21 +++++++++++++-------- > net/mptcp/options.c | 42 ++++++++++++++++++------------------------ > net/mptcp/protocol.h | 12 ------------ > 3 files changed, 31 insertions(+), 44 deletions(-) > > diff --git a/include/net/mptcp.h b/include/net/mptcp.h > index 16fe34d139c3..80d98a7db3c6 100644 > --- a/include/net/mptcp.h > +++ b/include/net/mptcp.h > @@ -41,20 +41,25 @@ struct mptcp_rm_list { > u8 nr; > }; > > +struct mptcp_addr_info { > + u8 id; > + sa_family_t family : 4; It's unusual to use a bitfield size specifier with a special type like sa_family_t. This patch moves the mptcp_addr_info struct from protocol.h to mptcp.h, and the deleted struct below does not have the " : 4" for the family. Was this intentional? While AF_INET and AF_INET6 do fit in 4 bits, and AF_MAX is only 45, I think this code should either use a bitfield to represent IPv4/v6, or use a whole sa_family_t. Maybe this is why MPTCP_ADDR_IPVERSION_? values were defined. Mat > + __be16 port; > + union { > + struct in_addr addr; > +#if IS_ENABLED(CONFIG_MPTCP_IPV6) > + struct in6_addr addr6; > +#endif > + }; > +}; > + > struct mptcp_out_options { > #if IS_ENABLED(CONFIG_MPTCP) > u16 suboptions; > u64 sndr_key; > u64 rcvr_key; > - union { > - struct in_addr addr; > -#if IS_ENABLED(CONFIG_MPTCP_IPV6) > - struct in6_addr addr6; > -#endif > - }; > - u8 addr_id; > - u16 port; > u64 ahmac; > + struct mptcp_addr_info addr; > struct mptcp_rm_list rm_list; > u8 join_id; > u8 backup; > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > index 4b7119eb2c31..7e01f44ed885 100644 > --- a/net/mptcp/options.c > +++ b/net/mptcp/options.c > @@ -626,7 +626,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * > struct mptcp_sock *msk = mptcp_sk(subflow->conn); > bool drop_other_suboptions = false; > unsigned int opt_size = *size; > - struct mptcp_addr_info saddr; > bool echo; > bool port; > int len; > @@ -643,45 +642,40 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * > } > > if (!mptcp_pm_should_add_signal(msk) || > - !(mptcp_pm_add_addr_signal(msk, remaining, &saddr, &echo, &port))) > + !(mptcp_pm_add_addr_signal(msk, remaining, &opts->addr, &echo, &port))) > return false; > > - len = mptcp_add_addr_len(saddr.family, echo, port); > + len = mptcp_add_addr_len(opts->addr.family, echo, port); > if (remaining < len) > return false; > > *size = len; > if (drop_other_suboptions) > *size -= opt_size; > - opts->addr_id = saddr.id; > - if (port) > - opts->port = ntohs(saddr.port); > - if (saddr.family == AF_INET) { > + if (opts->addr.family == AF_INET) { > opts->suboptions |= OPTION_MPTCP_ADD_ADDR; > - opts->addr = saddr.addr; > if (!echo) { > opts->ahmac = add_addr_generate_hmac(msk->local_key, > msk->remote_key, > - opts->addr_id, > - &opts->addr, > - opts->port); > + opts->addr.id, > + &opts->addr.addr, > + opts->addr.port); > } > } > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > - else if (saddr.family == AF_INET6) { > + else if (opts->addr.family == AF_INET6) { > opts->suboptions |= OPTION_MPTCP_ADD_ADDR6; > - opts->addr6 = saddr.addr6; > if (!echo) { > opts->ahmac = add_addr6_generate_hmac(msk->local_key, > msk->remote_key, > - opts->addr_id, > - &opts->addr6, > - opts->port); > + opts->addr.id, > + &opts->addr.addr6, > + opts->addr.port); > } > } > #endif > pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d", > - opts->addr_id, opts->ahmac, echo, opts->port); > + opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port)); > > return true; > } > @@ -1217,7 +1211,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > len = TCPOLEN_MPTCP_ADD_ADDR6_BASE; > #endif > > - if (opts->port) > + if (opts->addr.port) > len += TCPOLEN_MPTCP_PORT_LEN; > > if (opts->ahmac) { > @@ -1226,19 +1220,19 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > } > > *ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, > - len, echo, opts->addr_id); > + len, echo, opts->addr.id); > if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) { > - memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4); > + memcpy((u8 *)ptr, (u8 *)&opts->addr.addr.s_addr, 4); > ptr += 1; > } > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > else if (OPTION_MPTCP_ADD_ADDR6 & opts->suboptions) { > - memcpy((u8 *)ptr, opts->addr6.s6_addr, 16); > + memcpy((u8 *)ptr, opts->addr.addr6.s6_addr, 16); > ptr += 4; > } > #endif > > - if (!opts->port) { > + if (!opts->addr.port) { > if (opts->ahmac) { > put_unaligned_be64(opts->ahmac, ptr); > ptr += 2; > @@ -1247,7 +1241,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > if (opts->ahmac) { > u8 *bptr = (u8 *)ptr; > > - put_unaligned_be16(opts->port, bptr); > + put_unaligned_be16(opts->addr.port, bptr); > bptr += 2; > put_unaligned_be64(opts->ahmac, bptr); > bptr += 8; > @@ -1256,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > > ptr += 3; > } else { > - put_unaligned_be32(opts->port << 16 | > + put_unaligned_be32(opts->addr.port << 16 | > TCPOPT_NOP << 8 | > TCPOPT_NOP, ptr); > ptr += 1; > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > index 9005ccc2bc7d..b993e372c4ad 100644 > --- a/net/mptcp/protocol.h > +++ b/net/mptcp/protocol.h > @@ -169,18 +169,6 @@ static inline __be32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field) > ((nib & 0xF) << 8) | field); > } > > -struct mptcp_addr_info { > - sa_family_t family; > - __be16 port; > - u8 id; > - union { > - struct in_addr addr; > -#if IS_ENABLED(CONFIG_MPTCP_IPV6) > - struct in6_addr addr6; > -#endif > - }; > -}; > - > enum mptcp_pm_status { > MPTCP_PM_ADD_ADDR_RECEIVED, > MPTCP_PM_ADD_ADDR_SEND_ACK, > -- > 2.30.2 -- Mat Martineau Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 2/7] mptcp: use mptcp_addr_info in mptcp_out_options 2021-03-19 4:13 ` [MPTCP] " Mat Martineau @ 2021-03-19 7:14 ` Geliang Tang 0 siblings, 0 replies; 6+ messages in thread From: Geliang Tang @ 2021-03-19 7:14 UTC (permalink / raw) To: Mat Martineau; +Cc: mptcp Hi Mat, Thanks for your review. Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年3月19日周五 下午12:13写道: > > On Wed, 17 Mar 2021, Geliang Tang wrote: > > > This patch moved the mptcp_addr_info struct from protocol.h to mptcp.h, > > added a new struct mptcp_addr_info member addr in struct mptcp_out_options, > > and dropped the original addr, addr6, addr_id and port fields in it. Then > > we can use opts->addr to get the adding address from PM directly using > > mptcp_pm_add_addr_signal. > > > > Since the port number became as a big-endian order now, use ntohs to > > convert it before printing it out. > > > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > > --- > > include/net/mptcp.h | 21 +++++++++++++-------- > > net/mptcp/options.c | 42 ++++++++++++++++++------------------------ > > net/mptcp/protocol.h | 12 ------------ > > 3 files changed, 31 insertions(+), 44 deletions(-) > > > > diff --git a/include/net/mptcp.h b/include/net/mptcp.h > > index 16fe34d139c3..80d98a7db3c6 100644 > > --- a/include/net/mptcp.h > > +++ b/include/net/mptcp.h > > @@ -41,20 +41,25 @@ struct mptcp_rm_list { > > u8 nr; > > }; > > > > +struct mptcp_addr_info { > > + u8 id; > > + sa_family_t family : 4; > > It's unusual to use a bitfield size specifier with a special type like > sa_family_t. This patch moves the mptcp_addr_info struct from protocol.h > to mptcp.h, and the deleted struct below does not have the " : 4" for the > family. Was this intentional? > > While AF_INET and AF_INET6 do fit in 4 bits, and AF_MAX is only 45, I > think this code should either use a bitfield to represent IPv4/v6, or use > a whole sa_family_t. Maybe this is why MPTCP_ADDR_IPVERSION_? values were > defined. I prefer to use a whole sa_family_t, since that can avoid converting the type of the address family. I just sent a squash-to patch (Squash to "mptcp: use mptcp_addr_info in mptcp_out_options") to fix this. Thanks. -Geliang > > Mat > > > > + __be16 port; > > + union { > > + struct in_addr addr; > > +#if IS_ENABLED(CONFIG_MPTCP_IPV6) > > + struct in6_addr addr6; > > +#endif > > + }; > > +}; > > + > > struct mptcp_out_options { > > #if IS_ENABLED(CONFIG_MPTCP) > > u16 suboptions; > > u64 sndr_key; > > u64 rcvr_key; > > - union { > > - struct in_addr addr; > > -#if IS_ENABLED(CONFIG_MPTCP_IPV6) > > - struct in6_addr addr6; > > -#endif > > - }; > > - u8 addr_id; > > - u16 port; > > u64 ahmac; > > + struct mptcp_addr_info addr; > > struct mptcp_rm_list rm_list; > > u8 join_id; > > u8 backup; > > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > > index 4b7119eb2c31..7e01f44ed885 100644 > > --- a/net/mptcp/options.c > > +++ b/net/mptcp/options.c > > @@ -626,7 +626,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * > > struct mptcp_sock *msk = mptcp_sk(subflow->conn); > > bool drop_other_suboptions = false; > > unsigned int opt_size = *size; > > - struct mptcp_addr_info saddr; > > bool echo; > > bool port; > > int len; > > @@ -643,45 +642,40 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * > > } > > > > if (!mptcp_pm_should_add_signal(msk) || > > - !(mptcp_pm_add_addr_signal(msk, remaining, &saddr, &echo, &port))) > > + !(mptcp_pm_add_addr_signal(msk, remaining, &opts->addr, &echo, &port))) > > return false; > > > > - len = mptcp_add_addr_len(saddr.family, echo, port); > > + len = mptcp_add_addr_len(opts->addr.family, echo, port); > > if (remaining < len) > > return false; > > > > *size = len; > > if (drop_other_suboptions) > > *size -= opt_size; > > - opts->addr_id = saddr.id; > > - if (port) > > - opts->port = ntohs(saddr.port); > > - if (saddr.family == AF_INET) { > > + if (opts->addr.family == AF_INET) { > > opts->suboptions |= OPTION_MPTCP_ADD_ADDR; > > - opts->addr = saddr.addr; > > if (!echo) { > > opts->ahmac = add_addr_generate_hmac(msk->local_key, > > msk->remote_key, > > - opts->addr_id, > > - &opts->addr, > > - opts->port); > > + opts->addr.id, > > + &opts->addr.addr, > > + opts->addr.port); > > } > > } > > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > > - else if (saddr.family == AF_INET6) { > > + else if (opts->addr.family == AF_INET6) { > > opts->suboptions |= OPTION_MPTCP_ADD_ADDR6; > > - opts->addr6 = saddr.addr6; > > if (!echo) { > > opts->ahmac = add_addr6_generate_hmac(msk->local_key, > > msk->remote_key, > > - opts->addr_id, > > - &opts->addr6, > > - opts->port); > > + opts->addr.id, > > + &opts->addr.addr6, > > + opts->addr.port); > > } > > } > > #endif > > pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d", > > - opts->addr_id, opts->ahmac, echo, opts->port); > > + opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port)); > > > > return true; > > } > > @@ -1217,7 +1211,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > > len = TCPOLEN_MPTCP_ADD_ADDR6_BASE; > > #endif > > > > - if (opts->port) > > + if (opts->addr.port) > > len += TCPOLEN_MPTCP_PORT_LEN; > > > > if (opts->ahmac) { > > @@ -1226,19 +1220,19 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > > } > > > > *ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, > > - len, echo, opts->addr_id); > > + len, echo, opts->addr.id); > > if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) { > > - memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4); > > + memcpy((u8 *)ptr, (u8 *)&opts->addr.addr.s_addr, 4); > > ptr += 1; > > } > > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > > else if (OPTION_MPTCP_ADD_ADDR6 & opts->suboptions) { > > - memcpy((u8 *)ptr, opts->addr6.s6_addr, 16); > > + memcpy((u8 *)ptr, opts->addr.addr6.s6_addr, 16); > > ptr += 4; > > } > > #endif > > > > - if (!opts->port) { > > + if (!opts->addr.port) { > > if (opts->ahmac) { > > put_unaligned_be64(opts->ahmac, ptr); > > ptr += 2; > > @@ -1247,7 +1241,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > > if (opts->ahmac) { > > u8 *bptr = (u8 *)ptr; > > > > - put_unaligned_be16(opts->port, bptr); > > + put_unaligned_be16(opts->addr.port, bptr); > > bptr += 2; > > put_unaligned_be64(opts->ahmac, bptr); > > bptr += 8; > > @@ -1256,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp, > > > > ptr += 3; > > } else { > > - put_unaligned_be32(opts->port << 16 | > > + put_unaligned_be32(opts->addr.port << 16 | > > TCPOPT_NOP << 8 | > > TCPOPT_NOP, ptr); > > ptr += 1; > > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > > index 9005ccc2bc7d..b993e372c4ad 100644 > > --- a/net/mptcp/protocol.h > > +++ b/net/mptcp/protocol.h > > @@ -169,18 +169,6 @@ static inline __be32 mptcp_option(u8 subopt, u8 len, u8 nib, u8 field) > > ((nib & 0xF) << 8) | field); > > } > > > > -struct mptcp_addr_info { > > - sa_family_t family; > > - __be16 port; > > - u8 id; > > - union { > > - struct in_addr addr; > > -#if IS_ENABLED(CONFIG_MPTCP_IPV6) > > - struct in6_addr addr6; > > -#endif > > - }; > > -}; > > - > > enum mptcp_pm_status { > > MPTCP_PM_ADD_ADDR_RECEIVED, > > MPTCP_PM_ADD_ADDR_SEND_ACK, > > -- > > 2.30.2 > > -- > Mat Martineau > Intel _______________________________________________ mptcp mailing list -- mptcp@lists.01.org To unsubscribe send an email to mptcp-leave@lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <22cecfe9-709a-f4f2-b137-63df1227ed43@linux.intel.com>]
[parent not found: <76cc940b-096b-91f3-6cd0-23def55d5ba1@tessares.net>]
* Re: [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 0/7] refactor mptcp_addr_info and cleanups [not found] ` <76cc940b-096b-91f3-6cd0-23def55d5ba1@tessares.net> @ 2021-03-22 13:44 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2021-03-22 13:44 UTC (permalink / raw) To: Mat Martineau, Geliang Tang; +Cc: mptcp, MPTCP Upstream Hello, On 20/03/2021 12:13, Matthieu Baerts wrote: > Hi Geliang, Mat, > > On 19/03/2021 22:49, Mat Martineau wrote: >> On Wed, 17 Mar 2021, Geliang Tang wrote: >> >>> v3: >>> - Add new parameters flags and ifindex to __mptcp_subflow_connect. >>> - Drop the patch "mptcp: drop unnecessary CONFIG_MPTCP_IPV6" in v2. >>> - Add a new selftest patch. >>> >> >> Thanks, Geliang. v3 with the squash-to patch from today looks good to >> merge. >> >> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > > Thank you for the patches and the reviews! > > These patches have been added to the tree with Mat's RvB tag: > > - 7d9c5e39d716: mptcp: move flags and ifindex out of mptcp_addr_info > - adbca536f2e0: mptcp: use mptcp_addr_info in mptcp_out_options > - f7431619f09f: mptcp: drop OPTION_MPTCP_ADD_ADDR6 > - d535bedb2c1d: mptcp: use mptcp_addr_info in mptcp_options_received > - 5f105703ff28: mptcp: drop MPTCP_ADDR_IPVERSION_4/6 > - 3f11612d8302: mptcp: unify add_addr(6)_generate_hmac > - aa87ce506334: selftests: mptcp: add the net device name testcase > - Results: 6fc4aa6765fd..9bc1436cd72d > > And the squash-to one: > > - 79c9e16f9824: "squashed" in "mptcp: use mptcp_addr_info in > mptcp_out_options" > - Results: 9bc1436cd72d..7a40c945f992 > > Tests + export are in progress! FYI, the export has failed because there are some new sparse warnings. I'm looking at them. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-22 13:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-17 7:36 [MPTCP] [MPTCP][PATCH v3 mptcp-next 0/7] refactor mptcp_addr_info and cleanups Geliang Tang 2021-03-17 7:36 ` [MPTCP] [MPTCP][PATCH v3 mptcp-next 1/7] mptcp: move flags and ifindex out of mptcp_addr_info Geliang Tang 2021-03-17 7:36 ` [MPTCP] [MPTCP][PATCH v3 mptcp-next 2/7] mptcp: use mptcp_addr_info in mptcp_out_options Geliang Tang 2021-03-19 4:13 ` [MPTCP] " Mat Martineau 2021-03-19 7:14 ` Geliang Tang [not found] ` <22cecfe9-709a-f4f2-b137-63df1227ed43@linux.intel.com> [not found] ` <76cc940b-096b-91f3-6cd0-23def55d5ba1@tessares.net> 2021-03-22 13:44 ` [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 0/7] refactor mptcp_addr_info and cleanups Matthieu Baerts
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).