__mptcp_subflow_connect() is currently called from the path-managers,
which have all the required information to create subflows. No need to
call the PM again to re-iterate over the list of entries with RCU lock
to get more info.
Instead, it is possible to pass a mptcp_pm_addr_entry structure, instead
of a mptcp_addr_info one. The former contains the ifindex and the flags
that are required when creating the new subflow.
This is a partial revert of commit ee285257a9c1 ("mptcp: drop flags and
ifindex arguments").
While at it, the local ID can also be set if it is known and 0, to avoid
having to set it in the 'rebuild_header' hook, which will cause a new
iteration of the endpoint entries.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
- This patch is for net-next
- v4:
- Avoid multiple copies of an addr entry in
fill_local_addresses_vec().
- Rebased on top of "mptcp: fix endpoints with 'signal' and 'subflow'
flags", v4.
---
net/mptcp/pm.c | 11 -----------
net/mptcp/pm_netlink.c | 48 ++++++++++++------------------------------------
net/mptcp/pm_userspace.c | 19 +------------------
net/mptcp/protocol.h | 10 +---------
net/mptcp/subflow.c | 29 ++++++++++++++++++-----------
5 files changed, 32 insertions(+), 85 deletions(-)
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index ddad51210971..54fabd386b04 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -416,17 +416,6 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
return mptcp_pm_nl_get_local_id(msk, &skc_local);
}
-int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
- u8 *flags, int *ifindex)
-{
- *flags = 0;
- *ifindex = 0;
-
- if (mptcp_pm_is_userspace(msk))
- return mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
- return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
-}
-
int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
{
if (info->attrs[MPTCP_PM_ATTR_TOKEN])
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 45a1aa0a40bf..a316951f8762 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -625,7 +625,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
spin_unlock_bh(&msk->pm.lock);
for (i = 0; i < nr; i++)
- __mptcp_subflow_connect(sk, &local.addr, &addrs[i]);
+ __mptcp_subflow_connect(sk, &local, &addrs[i]);
spin_lock_bh(&msk->pm.lock);
}
mptcp_pm_nl_check_work_pending(msk);
@@ -646,7 +646,7 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
*/
static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
struct mptcp_addr_info *remote,
- struct mptcp_addr_info *addrs)
+ struct mptcp_pm_addr_entry *entries)
{
struct sock *sk = (struct sock *)msk;
struct mptcp_pm_addr_entry *entry;
@@ -670,14 +670,14 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
continue;
if (msk->pm.subflows < subflows_max) {
- msk->pm.subflows++;
- addrs[i] = entry->addr;
+ memcpy(&entries[i], entry, sizeof(entries[i]));
/* Special case for ID0: set the correct ID */
if (msk->first &&
mptcp_addresses_equal(&entry->addr, &mpc_addr, entry->addr.port))
- addrs[i].id = 0;
+ entries[i].addr.id = 0;
+ msk->pm.subflows++;
i++;
}
}
@@ -687,21 +687,19 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
* 'IPADDRANY' local address
*/
if (!i) {
- struct mptcp_addr_info local;
-
- memset(&local, 0, sizeof(local));
- local.family =
+ memset(&entries[i], 0, sizeof(entries[i]));
+ entries[i].addr.family =
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
remote->family == AF_INET6 &&
ipv6_addr_v4mapped(&remote->addr6) ? AF_INET :
#endif
remote->family;
- if (!mptcp_pm_addr_families_match(sk, &local, remote))
+ if (!mptcp_pm_addr_families_match(sk, &entries[i].addr, remote))
return 0;
msk->pm.subflows++;
- addrs[i++] = local;
+ i++;
}
return i;
@@ -709,7 +707,7 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
{
- struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX];
+ struct mptcp_pm_addr_entry entries[MPTCP_PM_ADDR_MAX];
struct sock *sk = (struct sock *)msk;
unsigned int add_addr_accept_max;
struct mptcp_addr_info remote;
@@ -738,13 +736,13 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
/* connect to the specified remote address, using whatever
* local address the routing configuration will pick.
*/
- nr = fill_local_addresses_vec(msk, &remote, addrs);
+ nr = fill_local_addresses_vec(msk, &remote, entries);
if (nr == 0)
return;
spin_unlock_bh(&msk->pm.lock);
for (i = 0; i < nr; i++)
- if (__mptcp_subflow_connect(sk, &addrs[i], &remote) == 0)
+ if (__mptcp_subflow_connect(sk, &entries[i], &remote) == 0)
sf_created = true;
spin_lock_bh(&msk->pm.lock);
@@ -1395,28 +1393,6 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
return ret;
}
-int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
- u8 *flags, int *ifindex)
-{
- struct mptcp_pm_addr_entry *entry;
- struct sock *sk = (struct sock *)msk;
- struct net *net = sock_net(sk);
-
- /* No entries with ID 0 */
- if (id == 0)
- return 0;
-
- rcu_read_lock();
- entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id);
- if (entry) {
- *flags = entry->flags;
- *ifindex = entry->ifindex;
- }
- rcu_read_unlock();
-
- return 0;
-}
-
static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
const struct mptcp_addr_info *addr)
{
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index f0a4590506c6..97b09dffff6d 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -119,23 +119,6 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
return NULL;
}
-int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
- unsigned int id,
- u8 *flags, int *ifindex)
-{
- struct mptcp_pm_addr_entry *match;
-
- spin_lock_bh(&msk->pm.lock);
- match = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
- spin_unlock_bh(&msk->pm.lock);
- if (match) {
- *flags = match->flags;
- *ifindex = match->ifindex;
- }
-
- return 0;
-}
-
int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
struct mptcp_addr_info *skc)
{
@@ -394,7 +377,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
lock_sock(sk);
- err = __mptcp_subflow_connect(sk, &local.addr, &addr_r);
+ err = __mptcp_subflow_connect(sk, &local, &addr_r);
release_sock(sk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f2eb5273d752..259e247b0862 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -722,7 +722,7 @@ bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
void mptcp_local_address(const struct sock_common *skc, struct mptcp_addr_info *addr);
/* called with sk socket lock held */
-int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
+int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
const struct mptcp_addr_info *remote);
int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
struct socket **new_sock);
@@ -1015,14 +1015,6 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
struct mptcp_pm_add_entry *
mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
const struct mptcp_addr_info *addr);
-int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
- unsigned int id,
- u8 *flags, int *ifindex);
-int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
- u8 *flags, int *ifindex);
-int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
- unsigned int id,
- u8 *flags, int *ifindex);
int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 39e2cbdf3801..0835e71118b9 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1544,26 +1544,24 @@ void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
#endif
}
-int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
+int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
const struct mptcp_addr_info *remote)
{
struct mptcp_sock *msk = mptcp_sk(sk);
struct mptcp_subflow_context *subflow;
+ int local_id = local->addr.id;
struct sockaddr_storage addr;
int remote_id = remote->id;
- int local_id = loc->id;
int err = -ENOTCONN;
struct socket *sf;
struct sock *ssk;
u32 remote_token;
int addrlen;
- int ifindex;
- u8 flags;
if (!mptcp_is_fully_established(sk))
goto err_out;
- err = mptcp_subflow_create_socket(sk, loc->family, &sf);
+ err = mptcp_subflow_create_socket(sk, local->addr.family, &sf);
if (err)
goto err_out;
@@ -1573,23 +1571,32 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
get_random_bytes(&subflow->local_nonce, sizeof(u32));
} while (!subflow->local_nonce);
- if (local_id)
+ /* if 'IPADDRANY', the ID will be set later, after the routing */
+ if (local->addr.family == AF_INET) {
+ if (!local->addr.addr.s_addr)
+ local_id = -1;
+#if IS_ENABLED(CONFIG_IPV6)
+ } else if (sk->sk_family == AF_INET6) {
+ if (ipv6_addr_any(&local->addr.addr6))
+ local_id = -1;
+#endif
+ }
+
+ if (local_id >= 0)
subflow_set_local_id(subflow, local_id);
- mptcp_pm_get_flags_and_ifindex_by_id(msk, local_id,
- &flags, &ifindex);
subflow->remote_key_valid = 1;
subflow->remote_key = READ_ONCE(msk->remote_key);
subflow->local_key = READ_ONCE(msk->local_key);
subflow->token = msk->token;
- mptcp_info2sockaddr(loc, &addr, ssk->sk_family);
+ mptcp_info2sockaddr(&local->addr, &addr, ssk->sk_family);
addrlen = sizeof(struct sockaddr_in);
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
if (addr.ss_family == AF_INET6)
addrlen = sizeof(struct sockaddr_in6);
#endif
- ssk->sk_bound_dev_if = ifindex;
+ ssk->sk_bound_dev_if = local->ifindex;
err = kernel_bind(sf, (struct sockaddr *)&addr, addrlen);
if (err)
goto failed;
@@ -1600,7 +1607,7 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
subflow->remote_token = remote_token;
WRITE_ONCE(subflow->remote_id, remote_id);
subflow->request_join = 1;
- subflow->request_bkup = !!(flags & MPTCP_PM_ADDR_FLAG_BACKUP);
+ subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
subflow->subflow_id = msk->subflow_id++;
mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
--
2.45.2On 22/07/2024 21:36, Matthieu Baerts (NGI0) wrote:
> __mptcp_subflow_connect() is currently called from the path-managers,
> which have all the required information to create subflows. No need to
> call the PM again to re-iterate over the list of entries with RCU lock
> to get more info.
>
> Instead, it is possible to pass a mptcp_pm_addr_entry structure, instead
> of a mptcp_addr_info one. The former contains the ifindex and the flags
> that are required when creating the new subflow.
>
> This is a partial revert of commit ee285257a9c1 ("mptcp: drop flags and
> ifindex arguments").
>
> While at it, the local ID can also be set if it is known and 0, to avoid
> having to set it in the 'rebuild_header' hook, which will cause a new
> iteration of the endpoint entries.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Notes:
> - This patch is for net-next
> - v4:
> - Avoid multiple copies of an addr entry in
> fill_local_addresses_vec().
> - Rebased on top of "mptcp: fix endpoints with 'signal' and 'subflow'
> flags", v4.
> ---
> net/mptcp/pm.c | 11 -----------
> net/mptcp/pm_netlink.c | 48 ++++++++++++------------------------------------
> net/mptcp/pm_userspace.c | 19 +------------------
> net/mptcp/protocol.h | 10 +---------
> net/mptcp/subflow.c | 29 ++++++++++++++++++-----------
> 5 files changed, 32 insertions(+), 85 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index ddad51210971..54fabd386b04 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -416,17 +416,6 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> return mptcp_pm_nl_get_local_id(msk, &skc_local);
> }
>
> -int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
> - u8 *flags, int *ifindex)
> -{
> - *flags = 0;
> - *ifindex = 0;
> -
> - if (mptcp_pm_is_userspace(msk))
> - return mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
> - return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
> -}
> -
> int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
> {
> if (info->attrs[MPTCP_PM_ATTR_TOKEN])
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 45a1aa0a40bf..a316951f8762 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -625,7 +625,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
>
> spin_unlock_bh(&msk->pm.lock);
> for (i = 0; i < nr; i++)
> - __mptcp_subflow_connect(sk, &local.addr, &addrs[i]);
> + __mptcp_subflow_connect(sk, &local, &addrs[i]);
> spin_lock_bh(&msk->pm.lock);
> }
> mptcp_pm_nl_check_work_pending(msk);
> @@ -646,7 +646,7 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
> */
> static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
> struct mptcp_addr_info *remote,
> - struct mptcp_addr_info *addrs)
> + struct mptcp_pm_addr_entry *entries)
> {
> struct sock *sk = (struct sock *)msk;
> struct mptcp_pm_addr_entry *entry;
> @@ -670,14 +670,14 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
> continue;
>
> if (msk->pm.subflows < subflows_max) {
> - msk->pm.subflows++;
> - addrs[i] = entry->addr;
> + memcpy(&entries[i], entry, sizeof(entries[i]));
>
> /* Special case for ID0: set the correct ID */
> if (msk->first &&
> mptcp_addresses_equal(&entry->addr, &mpc_addr, entry->addr.port))
> - addrs[i].id = 0;
> + entries[i].addr.id = 0;
>
> + msk->pm.subflows++;
> i++;
> }
> }
> @@ -687,21 +687,19 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
> * 'IPADDRANY' local address
> */
> if (!i) {
> - struct mptcp_addr_info local;
> -
> - memset(&local, 0, sizeof(local));
> - local.family =
> + memset(&entries[i], 0, sizeof(entries[i]));
> + entries[i].addr.family =
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> remote->family == AF_INET6 &&
> ipv6_addr_v4mapped(&remote->addr6) ? AF_INET :
> #endif
> remote->family;
>
> - if (!mptcp_pm_addr_families_match(sk, &local, remote))
> + if (!mptcp_pm_addr_families_match(sk, &entries[i].addr, remote))
> return 0;
>
> msk->pm.subflows++;
> - addrs[i++] = local;
> + i++;
> }
>
> return i;
> @@ -709,7 +707,7 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
>
> static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> {
> - struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX];
> + struct mptcp_pm_addr_entry entries[MPTCP_PM_ADDR_MAX];
> struct sock *sk = (struct sock *)msk;
> unsigned int add_addr_accept_max;
> struct mptcp_addr_info remote;
> @@ -738,13 +736,13 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> /* connect to the specified remote address, using whatever
> * local address the routing configuration will pick.
> */
> - nr = fill_local_addresses_vec(msk, &remote, addrs);
> + nr = fill_local_addresses_vec(msk, &remote, entries);
> if (nr == 0)
> return;
>
> spin_unlock_bh(&msk->pm.lock);
> for (i = 0; i < nr; i++)
> - if (__mptcp_subflow_connect(sk, &addrs[i], &remote) == 0)
> + if (__mptcp_subflow_connect(sk, &entries[i], &remote) == 0)
> sf_created = true;
> spin_lock_bh(&msk->pm.lock);
>
> @@ -1395,28 +1393,6 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
> return ret;
> }
>
> -int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
> - u8 *flags, int *ifindex)
> -{
> - struct mptcp_pm_addr_entry *entry;
> - struct sock *sk = (struct sock *)msk;
> - struct net *net = sock_net(sk);
> -
> - /* No entries with ID 0 */
> - if (id == 0)
> - return 0;
> -
> - rcu_read_lock();
> - entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id);
> - if (entry) {
> - *flags = entry->flags;
> - *ifindex = entry->ifindex;
> - }
> - rcu_read_unlock();
> -
> - return 0;
> -}
> -
> static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
> const struct mptcp_addr_info *addr)
> {
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index f0a4590506c6..97b09dffff6d 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -119,23 +119,6 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
> return NULL;
> }
>
> -int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
> - unsigned int id,
> - u8 *flags, int *ifindex)
> -{
> - struct mptcp_pm_addr_entry *match;
> -
> - spin_lock_bh(&msk->pm.lock);
> - match = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
> - spin_unlock_bh(&msk->pm.lock);
> - if (match) {
> - *flags = match->flags;
> - *ifindex = match->ifindex;
> - }
> -
> - return 0;
> -}
> -
> int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
> struct mptcp_addr_info *skc)
> {
> @@ -394,7 +377,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>
> lock_sock(sk);
>
> - err = __mptcp_subflow_connect(sk, &local.addr, &addr_r);
> + err = __mptcp_subflow_connect(sk, &local, &addr_r);
>
> release_sock(sk);
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index f2eb5273d752..259e247b0862 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -722,7 +722,7 @@ bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
> void mptcp_local_address(const struct sock_common *skc, struct mptcp_addr_info *addr);
>
> /* called with sk socket lock held */
> -int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> +int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
> const struct mptcp_addr_info *remote);
> int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
> struct socket **new_sock);
> @@ -1015,14 +1015,6 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
> struct mptcp_pm_add_entry *
> mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
> const struct mptcp_addr_info *addr);
> -int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
> - unsigned int id,
> - u8 *flags, int *ifindex);
> -int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
> - u8 *flags, int *ifindex);
> -int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
> - unsigned int id,
> - u8 *flags, int *ifindex);
> int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
> int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
> int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 39e2cbdf3801..0835e71118b9 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1544,26 +1544,24 @@ void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> #endif
> }
>
> -int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> +int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
> const struct mptcp_addr_info *remote)
> {
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct mptcp_subflow_context *subflow;
> + int local_id = local->addr.id;
> struct sockaddr_storage addr;
> int remote_id = remote->id;
> - int local_id = loc->id;
> int err = -ENOTCONN;
> struct socket *sf;
> struct sock *ssk;
> u32 remote_token;
> int addrlen;
> - int ifindex;
> - u8 flags;
>
> if (!mptcp_is_fully_established(sk))
> goto err_out;
>
> - err = mptcp_subflow_create_socket(sk, loc->family, &sf);
> + err = mptcp_subflow_create_socket(sk, local->addr.family, &sf);
> if (err)
> goto err_out;
>
> @@ -1573,23 +1571,32 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> get_random_bytes(&subflow->local_nonce, sizeof(u32));
> } while (!subflow->local_nonce);
>
> - if (local_id)
> + /* if 'IPADDRANY', the ID will be set later, after the routing */
> + if (local->addr.family == AF_INET) {
> + if (!local->addr.addr.s_addr)
> + local_id = -1;
> +#if IS_ENABLED(CONFIG_IPV6)
As reported by kbot, it should be
-#if IS_ENABLED(CONFIG_IPV6)
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
I will not send a v5 just for that, I think there are already enough
versions and the series became too large :)
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
Hi Matthieu,
kernel test robot noticed the following build errors:
[auto build test ERROR on 140ff27ee47286bb0a270f3aa275fc319724da8d]
url: https://github.com/intel-lab-lkp/linux/commits/Matthieu-Baerts-NGI0/mptcp-fully-established-after-ADD_ADDR-echo-on-MPJ/20240723-035843
base: 140ff27ee47286bb0a270f3aa275fc319724da8d
patch link: https://lore.kernel.org/r/20240722-mptcp-pm-avail-v4-23-15bfd73de384%40kernel.org
patch subject: [PATCH mptcp-net v4 23/23] mptcp: pm: reduce entries iterations on connect
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240723/202407231303.CsUy96BP-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project ad154281230d83ee551e12d5be48bb956ef47ed3)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240723/202407231303.CsUy96BP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407231303.CsUy96BP-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from net/mptcp/subflow.c:10:
In file included from include/linux/module.h:19:
In file included from include/linux/elf.h:6:
In file included from arch/s390/include/asm/elf.h:181:
In file included from arch/s390/include/asm/mmu_context.h:11:
In file included from arch/s390/include/asm/pgalloc.h:18:
In file included from include/linux/mm.h:2258:
include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
500 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
501 | item];
| ~~~~
include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
507 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
508 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
514 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
519 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
520 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
528 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
529 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
In file included from net/mptcp/subflow.c:11:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
| ^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
| ^
In file included from net/mptcp/subflow.c:11:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
| ^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
| ^
In file included from net/mptcp/subflow.c:11:
In file included from include/linux/netdevice.h:38:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:28:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:93:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
693 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
701 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
709 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
718 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
727 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
736 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
>> net/mptcp/subflow.c:1580:34: error: no member named 'addr6' in 'struct mptcp_addr_info'; did you mean 'addr'?
1580 | if (ipv6_addr_any(&local->addr.addr6))
| ^~~~~
| addr
include/net/mptcp.h:55:18: note: 'addr' declared here
55 | struct in_addr addr;
| ^
17 warnings and 1 error generated.
vim +1580 net/mptcp/subflow.c
1546
1547 int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
1548 const struct mptcp_addr_info *remote)
1549 {
1550 struct mptcp_sock *msk = mptcp_sk(sk);
1551 struct mptcp_subflow_context *subflow;
1552 int local_id = local->addr.id;
1553 struct sockaddr_storage addr;
1554 int remote_id = remote->id;
1555 int err = -ENOTCONN;
1556 struct socket *sf;
1557 struct sock *ssk;
1558 u32 remote_token;
1559 int addrlen;
1560
1561 if (!mptcp_is_fully_established(sk))
1562 goto err_out;
1563
1564 err = mptcp_subflow_create_socket(sk, local->addr.family, &sf);
1565 if (err)
1566 goto err_out;
1567
1568 ssk = sf->sk;
1569 subflow = mptcp_subflow_ctx(ssk);
1570 do {
1571 get_random_bytes(&subflow->local_nonce, sizeof(u32));
1572 } while (!subflow->local_nonce);
1573
1574 /* if 'IPADDRANY', the ID will be set later, after the routing */
1575 if (local->addr.family == AF_INET) {
1576 if (!local->addr.addr.s_addr)
1577 local_id = -1;
1578 #if IS_ENABLED(CONFIG_IPV6)
1579 } else if (sk->sk_family == AF_INET6) {
> 1580 if (ipv6_addr_any(&local->addr.addr6))
1581 local_id = -1;
1582 #endif
1583 }
1584
1585 if (local_id >= 0)
1586 subflow_set_local_id(subflow, local_id);
1587
1588 subflow->remote_key_valid = 1;
1589 subflow->remote_key = READ_ONCE(msk->remote_key);
1590 subflow->local_key = READ_ONCE(msk->local_key);
1591 subflow->token = msk->token;
1592 mptcp_info2sockaddr(&local->addr, &addr, ssk->sk_family);
1593
1594 addrlen = sizeof(struct sockaddr_in);
1595 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
1596 if (addr.ss_family == AF_INET6)
1597 addrlen = sizeof(struct sockaddr_in6);
1598 #endif
1599 ssk->sk_bound_dev_if = local->ifindex;
1600 err = kernel_bind(sf, (struct sockaddr *)&addr, addrlen);
1601 if (err)
1602 goto failed;
1603
1604 mptcp_crypto_key_sha(subflow->remote_key, &remote_token, NULL);
1605 pr_debug("msk=%p remote_token=%u local_id=%d remote_id=%d", msk,
1606 remote_token, local_id, remote_id);
1607 subflow->remote_token = remote_token;
1608 WRITE_ONCE(subflow->remote_id, remote_id);
1609 subflow->request_join = 1;
1610 subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
1611 subflow->subflow_id = msk->subflow_id++;
1612 mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
1613
1614 sock_hold(ssk);
1615 list_add_tail(&subflow->node, &msk->conn_list);
1616 err = kernel_connect(sf, (struct sockaddr *)&addr, addrlen, O_NONBLOCK);
1617 if (err && err != -EINPROGRESS)
1618 goto failed_unlink;
1619
1620 /* discard the subflow socket */
1621 mptcp_sock_graft(ssk, sk->sk_socket);
1622 iput(SOCK_INODE(sf));
1623 WRITE_ONCE(msk->allow_infinite_fallback, false);
1624 mptcp_stop_tout_timer(sk);
1625 return 0;
1626
1627 failed_unlink:
1628 list_del(&subflow->node);
1629 sock_put(mptcp_subflow_tcp_sock(subflow));
1630
1631 failed:
1632 subflow->disposable = 1;
1633 sock_release(sf);
1634
1635 err_out:
1636 /* we account subflows before the creation, and this failures will not
1637 * be caught by sk_state_change()
1638 */
1639 mptcp_pm_close_subflow(msk);
1640 return err;
1641 }
1642
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Matthieu,
kernel test robot noticed the following build errors:
[auto build test ERROR on 140ff27ee47286bb0a270f3aa275fc319724da8d]
url: https://github.com/intel-lab-lkp/linux/commits/Matthieu-Baerts-NGI0/mptcp-fully-established-after-ADD_ADDR-echo-on-MPJ/20240723-035843
base: 140ff27ee47286bb0a270f3aa275fc319724da8d
patch link: https://lore.kernel.org/r/20240722-mptcp-pm-avail-v4-23-15bfd73de384%40kernel.org
patch subject: [PATCH mptcp-net v4 23/23] mptcp: pm: reduce entries iterations on connect
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240723/202407231046.JRpmrtkT-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240723/202407231046.JRpmrtkT-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407231046.JRpmrtkT-lkp@intel.com/
All errors (new ones prefixed by >>):
net/mptcp/subflow.c: In function '__mptcp_subflow_connect':
>> net/mptcp/subflow.c:1580:48: error: 'const struct mptcp_addr_info' has no member named 'addr6'; did you mean 'addr'?
1580 | if (ipv6_addr_any(&local->addr.addr6))
| ^~~~~
| addr
vim +1580 net/mptcp/subflow.c
1546
1547 int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
1548 const struct mptcp_addr_info *remote)
1549 {
1550 struct mptcp_sock *msk = mptcp_sk(sk);
1551 struct mptcp_subflow_context *subflow;
1552 int local_id = local->addr.id;
1553 struct sockaddr_storage addr;
1554 int remote_id = remote->id;
1555 int err = -ENOTCONN;
1556 struct socket *sf;
1557 struct sock *ssk;
1558 u32 remote_token;
1559 int addrlen;
1560
1561 if (!mptcp_is_fully_established(sk))
1562 goto err_out;
1563
1564 err = mptcp_subflow_create_socket(sk, local->addr.family, &sf);
1565 if (err)
1566 goto err_out;
1567
1568 ssk = sf->sk;
1569 subflow = mptcp_subflow_ctx(ssk);
1570 do {
1571 get_random_bytes(&subflow->local_nonce, sizeof(u32));
1572 } while (!subflow->local_nonce);
1573
1574 /* if 'IPADDRANY', the ID will be set later, after the routing */
1575 if (local->addr.family == AF_INET) {
1576 if (!local->addr.addr.s_addr)
1577 local_id = -1;
1578 #if IS_ENABLED(CONFIG_IPV6)
1579 } else if (sk->sk_family == AF_INET6) {
> 1580 if (ipv6_addr_any(&local->addr.addr6))
1581 local_id = -1;
1582 #endif
1583 }
1584
1585 if (local_id >= 0)
1586 subflow_set_local_id(subflow, local_id);
1587
1588 subflow->remote_key_valid = 1;
1589 subflow->remote_key = READ_ONCE(msk->remote_key);
1590 subflow->local_key = READ_ONCE(msk->local_key);
1591 subflow->token = msk->token;
1592 mptcp_info2sockaddr(&local->addr, &addr, ssk->sk_family);
1593
1594 addrlen = sizeof(struct sockaddr_in);
1595 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
1596 if (addr.ss_family == AF_INET6)
1597 addrlen = sizeof(struct sockaddr_in6);
1598 #endif
1599 ssk->sk_bound_dev_if = local->ifindex;
1600 err = kernel_bind(sf, (struct sockaddr *)&addr, addrlen);
1601 if (err)
1602 goto failed;
1603
1604 mptcp_crypto_key_sha(subflow->remote_key, &remote_token, NULL);
1605 pr_debug("msk=%p remote_token=%u local_id=%d remote_id=%d", msk,
1606 remote_token, local_id, remote_id);
1607 subflow->remote_token = remote_token;
1608 WRITE_ONCE(subflow->remote_id, remote_id);
1609 subflow->request_join = 1;
1610 subflow->request_bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
1611 subflow->subflow_id = msk->subflow_id++;
1612 mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
1613
1614 sock_hold(ssk);
1615 list_add_tail(&subflow->node, &msk->conn_list);
1616 err = kernel_connect(sf, (struct sockaddr *)&addr, addrlen, O_NONBLOCK);
1617 if (err && err != -EINPROGRESS)
1618 goto failed_unlink;
1619
1620 /* discard the subflow socket */
1621 mptcp_sock_graft(ssk, sk->sk_socket);
1622 iput(SOCK_INODE(sf));
1623 WRITE_ONCE(msk->allow_infinite_fallback, false);
1624 mptcp_stop_tout_timer(sk);
1625 return 0;
1626
1627 failed_unlink:
1628 list_del(&subflow->node);
1629 sock_put(mptcp_subflow_tcp_sock(subflow));
1630
1631 failed:
1632 subflow->disposable = 1;
1633 sock_release(sf);
1634
1635 err_out:
1636 /* we account subflows before the creation, and this failures will not
1637 * be caught by sk_state_change()
1638 */
1639 mptcp_pm_close_subflow(msk);
1640 return err;
1641 }
1642
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2024 Red Hat, Inc.