* [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
* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.