* [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal"
@ 2021-07-23 13:31 Paolo Abeni
2021-07-25 13:40 ` Geliang Tang
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Paolo Abeni @ 2021-07-23 13:31 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang, Yonglong Li
We don't need to add a 2nd mptcp_addr_info inside out_options:
it's quite huge and only one of local or remote is used.
revert back to a single 'addr' field.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/mptcp.h | 3 +--
net/mptcp/options.c | 13 +++++--------
net/mptcp/pm.c | 4 ++--
3 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index d0b9e4a7121f..8b5af683a818 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -61,8 +61,7 @@ struct mptcp_out_options {
u64 sndr_key;
u64 rcvr_key;
u64 ahmac;
- struct mptcp_addr_info local;
- struct mptcp_addr_info remote;
+ 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 6803de5d4209..eafdb9408f3a 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -665,7 +665,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
unsigned int opt_size = *size;
bool echo;
bool port;
- u8 family;
int len;
if (!mptcp_pm_should_add_signal(msk) ||
@@ -675,8 +674,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
if (drop_other_suboptions)
remaining += opt_size;
- family = echo ? opts->remote.family : opts->local.family;
- len = mptcp_add_addr_len(family, echo, port);
+ len = mptcp_add_addr_len(opts->addr.family, echo, port);
if (remaining < len)
return false;
@@ -692,11 +690,10 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
if (!echo) {
opts->ahmac = add_addr_generate_hmac(msk->local_key,
msk->remote_key,
- &opts->local);
+ &opts->addr);
}
- pr_debug("local_id=%d, local_port=%d, remote_id=%d, remote_port=%d, ahmac=%llu, echo=%d",
- opts->local.id, ntohs(opts->local.port), opts->remote.id,
- ntohs(opts->remote.port), opts->ahmac, echo);
+ pr_debug("addr_id=%d, addr_port=%d, ahmac=%llu, echo=%d",
+ opts->addr.id, ntohs(opts->addr.port), opts->ahmac, echo);
return true;
}
@@ -1253,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
mp_capable_done:
if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
- struct mptcp_addr_info *addr = opts->ahmac ? &opts->local : &opts->remote;
+ struct mptcp_addr_info *addr = &opts->addr;
u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
u8 echo = MPTCP_ADDR_ECHO;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 692614c1f552..4d1828fd2482 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -282,10 +282,10 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
goto out_unlock;
if (*echo) {
- opts->remote = msk->pm.remote;
+ opts->addr = msk->pm.remote;
add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_ECHO);
} else {
- opts->local = msk->pm.local;
+ opts->addr = msk->pm.local;
add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_SIGNAL);
}
WRITE_ONCE(msk->pm.addr_signal, add_addr);
--
2.26.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal"
2021-07-23 13:31 [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal" Paolo Abeni
@ 2021-07-25 13:40 ` Geliang Tang
2021-07-27 0:20 ` Mat Martineau
2021-07-28 7:07 ` Matthieu Baerts
2 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2021-07-25 13:40 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp, Yonglong Li
Paolo Abeni <pabeni@redhat.com> 于2021年7月23日周五 下午9:31写道：
>
> We don't need to add a 2nd mptcp_addr_info inside out_options:
> it's quite huge and only one of local or remote is used.
>
> revert back to a single 'addr' field.
Acked-and-tested-by: Geliang Tang <geliangtang@gmail.com>
I think more code needs to be revert too, no need to change the argument
of mptcp_pm_add_addr_signal and no need to modify the code in
mptcp_write_options. And the commit log need to update.
I'll sent another squash-to patch to do these cleanups.
Thanks,
-Geliang
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> include/net/mptcp.h | 3 +--
> net/mptcp/options.c | 13 +++++--------
> net/mptcp/pm.c | 4 ++--
> 3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index d0b9e4a7121f..8b5af683a818 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -61,8 +61,7 @@ struct mptcp_out_options {
> u64 sndr_key;
> u64 rcvr_key;
> u64 ahmac;
> - struct mptcp_addr_info local;
> - struct mptcp_addr_info remote;
> + 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 6803de5d4209..eafdb9408f3a 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -665,7 +665,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> unsigned int opt_size = *size;
> bool echo;
> bool port;
> - u8 family;
> int len;
>
> if (!mptcp_pm_should_add_signal(msk) ||
> @@ -675,8 +674,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
>
> if (drop_other_suboptions)
> remaining += opt_size;
> - family = echo ? opts->remote.family : opts->local.family;
> - len = mptcp_add_addr_len(family, echo, port);
> + len = mptcp_add_addr_len(opts->addr.family, echo, port);
> if (remaining < len)
> return false;
>
> @@ -692,11 +690,10 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> if (!echo) {
> opts->ahmac = add_addr_generate_hmac(msk->local_key,
> msk->remote_key,
> - &opts->local);
> + &opts->addr);
> }
> - pr_debug("local_id=%d, local_port=%d, remote_id=%d, remote_port=%d, ahmac=%llu, echo=%d",
> - opts->local.id, ntohs(opts->local.port), opts->remote.id,
> - ntohs(opts->remote.port), opts->ahmac, echo);
> + pr_debug("addr_id=%d, addr_port=%d, ahmac=%llu, echo=%d",
> + opts->addr.id, ntohs(opts->addr.port), opts->ahmac, echo);
>
> return true;
> }
> @@ -1253,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
>
> mp_capable_done:
> if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
> - struct mptcp_addr_info *addr = opts->ahmac ? &opts->local : &opts->remote;
> + struct mptcp_addr_info *addr = &opts->addr;
> u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
> u8 echo = MPTCP_ADDR_ECHO;
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 692614c1f552..4d1828fd2482 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -282,10 +282,10 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
> goto out_unlock;
>
> if (*echo) {
> - opts->remote = msk->pm.remote;
> + opts->addr = msk->pm.remote;
> add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_ECHO);
> } else {
> - opts->local = msk->pm.local;
> + opts->addr = msk->pm.local;
> add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_SIGNAL);
> }
> WRITE_ONCE(msk->pm.addr_signal, add_addr);
> --
> 2.26.3
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal"
2021-07-23 13:31 [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal" Paolo Abeni
2021-07-25 13:40 ` Geliang Tang
@ 2021-07-27 0:20 ` Mat Martineau
2021-07-28 7:07 ` Matthieu Baerts
2 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2021-07-27 0:20 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp, Geliang Tang, Yonglong Li
On Fri, 23 Jul 2021, Paolo Abeni wrote:
> We don't need to add a 2nd mptcp_addr_info inside out_options:
> it's quite huge and only one of local or remote is used.
>
> revert back to a single 'addr' field.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> include/net/mptcp.h | 3 +--
> net/mptcp/options.c | 13 +++++--------
> net/mptcp/pm.c | 4 ++--
> 3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index d0b9e4a7121f..8b5af683a818 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -61,8 +61,7 @@ struct mptcp_out_options {
> u64 sndr_key;
> u64 rcvr_key;
> u64 ahmac;
> - struct mptcp_addr_info local;
> - struct mptcp_addr_info remote;
> + 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 6803de5d4209..eafdb9408f3a 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -665,7 +665,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> unsigned int opt_size = *size;
> bool echo;
> bool port;
> - u8 family;
> int len;
>
> if (!mptcp_pm_should_add_signal(msk) ||
> @@ -675,8 +674,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
>
> if (drop_other_suboptions)
> remaining += opt_size;
> - family = echo ? opts->remote.family : opts->local.family;
> - len = mptcp_add_addr_len(family, echo, port);
> + len = mptcp_add_addr_len(opts->addr.family, echo, port);
> if (remaining < len)
> return false;
>
> @@ -692,11 +690,10 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> if (!echo) {
> opts->ahmac = add_addr_generate_hmac(msk->local_key,
> msk->remote_key,
> - &opts->local);
> + &opts->addr);
> }
> - pr_debug("local_id=%d, local_port=%d, remote_id=%d, remote_port=%d, ahmac=%llu, echo=%d",
> - opts->local.id, ntohs(opts->local.port), opts->remote.id,
> - ntohs(opts->remote.port), opts->ahmac, echo);
> + pr_debug("addr_id=%d, addr_port=%d, ahmac=%llu, echo=%d",
> + opts->addr.id, ntohs(opts->addr.port), opts->ahmac, echo);
>
> return true;
> }
> @@ -1253,7 +1250,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
>
> mp_capable_done:
> if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
> - struct mptcp_addr_info *addr = opts->ahmac ? &opts->local : &opts->remote;
> + struct mptcp_addr_info *addr = &opts->addr;
> u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
> u8 echo = MPTCP_ADDR_ECHO;
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 692614c1f552..4d1828fd2482 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -282,10 +282,10 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
> goto out_unlock;
>
> if (*echo) {
> - opts->remote = msk->pm.remote;
> + opts->addr = msk->pm.remote;
> add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_ECHO);
> } else {
> - opts->local = msk->pm.local;
> + opts->addr = msk->pm.local;
> add_addr = msk->pm.addr_signal & ~BIT(MPTCP_ADD_ADDR_SIGNAL);
> }
> WRITE_ONCE(msk->pm.addr_signal, add_addr);
> --
> 2.26.3
I agree that it's worthwhile to keep the size of mptcp_out_options down.
There's already a "!mptcp_pm_should_add_signal()" check earlier in
mptcp_pm_add_addr_signal(), so I don't see a risk of opts->addr getting
accidentally clobbered now that the mptcp_addr_info storage is shared
again.
Looks good to me:
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal"
2021-07-23 13:31 [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal" Paolo Abeni
2021-07-25 13:40 ` Geliang Tang
2021-07-27 0:20 ` Mat Martineau
@ 2021-07-28 7:07 ` Matthieu Baerts
2 siblings, 0 replies; 4+ messages in thread
From: Matthieu Baerts @ 2021-07-28 7:07 UTC (permalink / raw)
To: Paolo Abeni, Geliang Tang, Mat Martineau; +Cc: Yonglong Li, mptcp
Hi Paolo, Geliang, Mat,
On 23/07/2021 15:31, Paolo Abeni wrote:
> We don't need to add a 2nd mptcp_addr_info inside out_options:
> it's quite huge and only one of local or remote is used.
>
> revert back to a single 'addr' field.
Thank you for the patch, reviews and tests!
Now in our tree:
- 19d088641b4a: "squashed" in "mptcp: build ADD_ADDR/echo-ADD_ADDR
option according pm.add_signal"
- b333d139d5f8: "Signed-off-by" + "Co-developed-by"
- Results: 2fb0dbb4c7c8..fcc436834cac
Builds and tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210728T070703
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210728T070703
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-28 7:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-23 13:31 [PATCH mptcp-next] Squash-to: "mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal" Paolo Abeni
2021-07-25 13:40 ` Geliang Tang
2021-07-27 0:20 ` Mat Martineau
2021-07-28 7:07 ` 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.