Drop the port parameter of mptcp_pm_add_addr_signal() and reflect it to
avoid passing too many parameters.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/options.c  | 5 ++---
 net/mptcp/pm.c       | 7 ++++---
 net/mptcp/protocol.h | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 7b615dc10897..4e516e88ab88 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -652,7 +652,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
 	bool drop_other_suboptions = false;
 	unsigned int opt_size = *size;
 	bool echo;
-	bool port;
 	int len;
 
 	/* add addr will strip the existing options, be sure to avoid breaking
@@ -661,12 +660,12 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
 	if (!mptcp_pm_should_add_signal(msk) ||
 	    (opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
 	    !mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &opts->addr,
-		    &echo, &port, &drop_other_suboptions))
+		    &echo, &drop_other_suboptions))
 		return false;
 
 	if (drop_other_suboptions)
 		remaining += opt_size;
-	len = mptcp_add_addr_len(opts->addr.family, echo, port);
+	len = mptcp_add_addr_len(opts->addr.family, echo, !!opts->addr.port);
 	if (remaining < len)
 		return false;
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 1f8878cc29e3..99db7270e461 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -284,11 +284,12 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
 bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
 			      unsigned int opt_size, unsigned int remaining,
 			      struct mptcp_addr_info *addr, bool *echo,
-			      bool *port, bool *drop_other_suboptions)
+			      bool *drop_other_suboptions)
 {
 	int ret = false;
 	u8 add_addr;
 	u8 family;
+	bool port;
 
 	spin_lock_bh(&msk->pm.lock);
 
@@ -306,10 +307,10 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
 	}
 
 	*echo = mptcp_pm_should_add_signal_echo(msk);
-	*port = !!(*echo ? msk->pm.remote.port : msk->pm.local.port);
+	port = !!(*echo ? msk->pm.remote.port : msk->pm.local.port);
 
 	family = *echo ? msk->pm.remote.family : msk->pm.local.family;
-	if (remaining < mptcp_add_addr_len(family, *echo, *port))
+	if (remaining < mptcp_add_addr_len(family, *echo, port))
 		goto out_unlock;
 
 	if (*echo) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f37f087caab3..0eebfc9f39bc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -835,7 +835,7 @@ static inline int mptcp_rm_addr_len(const struct mptcp_rm_list *rm_list)
 bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
 			      unsigned int opt_size, unsigned int remaining,
 			      struct mptcp_addr_info *addr, bool *echo,
-			      bool *port, bool *drop_other_suboptions);
+			      bool *drop_other_suboptions);
 bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 			     struct mptcp_rm_list *rm_list);
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
-- 
2.34.1

On Tue, 8 Feb 2022, Geliang Tang wrote:

> Drop the port parameter of mptcp_pm_add_addr_signal() and reflect it to
> avoid passing too many parameters.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>

For this patch, looks like a helpful bit of cleanup. Thanks!

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


> ---
> net/mptcp/options.c  | 5 ++---
> net/mptcp/pm.c       | 7 ++++---
> net/mptcp/protocol.h | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 7b615dc10897..4e516e88ab88 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -652,7 +652,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> 	bool drop_other_suboptions = false;
> 	unsigned int opt_size = *size;
> 	bool echo;
> -	bool port;
> 	int len;
>
> 	/* add addr will strip the existing options, be sure to avoid breaking
> @@ -661,12 +660,12 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> 	if (!mptcp_pm_should_add_signal(msk) ||
> 	    (opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
> 	    !mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &opts->addr,
> -		    &echo, &port, &drop_other_suboptions))
> +		    &echo, &drop_other_suboptions))
> 		return false;
>
> 	if (drop_other_suboptions)
> 		remaining += opt_size;
> -	len = mptcp_add_addr_len(opts->addr.family, echo, port);
> +	len = mptcp_add_addr_len(opts->addr.family, echo, !!opts->addr.port);
> 	if (remaining < len)
> 		return false;
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 1f8878cc29e3..99db7270e461 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -284,11 +284,12 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
> bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
> 			      unsigned int opt_size, unsigned int remaining,
> 			      struct mptcp_addr_info *addr, bool *echo,
> -			      bool *port, bool *drop_other_suboptions)
> +			      bool *drop_other_suboptions)
> {
> 	int ret = false;
> 	u8 add_addr;
> 	u8 family;
> +	bool port;
>
> 	spin_lock_bh(&msk->pm.lock);
>
> @@ -306,10 +307,10 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
> 	}
>
> 	*echo = mptcp_pm_should_add_signal_echo(msk);
> -	*port = !!(*echo ? msk->pm.remote.port : msk->pm.local.port);
> +	port = !!(*echo ? msk->pm.remote.port : msk->pm.local.port);
>
> 	family = *echo ? msk->pm.remote.family : msk->pm.local.family;
> -	if (remaining < mptcp_add_addr_len(family, *echo, *port))
> +	if (remaining < mptcp_add_addr_len(family, *echo, port))
> 		goto out_unlock;
>
> 	if (*echo) {
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index f37f087caab3..0eebfc9f39bc 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -835,7 +835,7 @@ static inline int mptcp_rm_addr_len(const struct mptcp_rm_list *rm_list)
> bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
> 			      unsigned int opt_size, unsigned int remaining,
> 			      struct mptcp_addr_info *addr, bool *echo,
> -			      bool *port, bool *drop_other_suboptions);
> +			      bool *drop_other_suboptions);
> bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
> 			     struct mptcp_rm_list *rm_list);
> int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel

Hi Geliang, Mat,

On 09/02/2022 02:20, Mat Martineau wrote:
> On Tue, 8 Feb 2022, Geliang Tang wrote:
> 
>> Drop the port parameter of mptcp_pm_add_addr_signal() and reflect it to
>> avoid passing too many parameters.
>>
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> 
> For this patch, looks like a helpful bit of cleanup. Thanks!
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thank you for the patch and the review.

I just applied this patch in our tree (not the rest of the series) with
Mat's RvB tag:

- 3afdb53fb2eb: mptcp: drop port parameter of mptcp_pm_add_addr_signal
- Results: e0f3a1a43be7..56333e0f13bc

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220209T113813
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net