[MPTCP][PATCH v2 mptcp-next] mptcp: use OPTIONS_MPTCP_MPC

Geliang Tang posted 1 patch 3 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/dcaratti/mptcp_net-next tags/patchew/10d25ba91ac41cacf02608b9e9a6e884d1eae996.1630573884.git.geliangtang@xiaomi.com
Maintainers: Jakub Kicinski <kuba@kernel.org>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Mat Martineau <mathew.j.martineau@linux.intel.com>
net/mptcp/options.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

[MPTCP][PATCH v2 mptcp-next] mptcp: use OPTIONS_MPTCP_MPC

Posted by Geliang Tang 3 weeks, 2 days ago
From: Geliang Tang <geliangtang@xiaomi.com>

Since OPTIONS_MPTCP_MPC has been defined, use it instead of open-coding.

Acked-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
---
 v2:
 - switch of yoda-style in mptcp_established_options_mp_prio as Paolo
   suggested.
 - keep yoda-style in mptcp_write_options, since the other options in
   this fuction all use the yoda-style conditions.
---
 net/mptcp/options.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 1ec6529c4326..422f4acfb3e6 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -748,9 +748,7 @@ static bool mptcp_established_options_mp_prio(struct sock *sk,
 	/* can't send MP_PRIO with MPC, as they share the same option space:
 	 * 'backup'. Also it makes no sense at all
 	 */
-	if (!subflow->send_mp_prio ||
-	    ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
-	      OPTION_MPTCP_MPC_ACK) & opts->suboptions))
+	if (!subflow->send_mp_prio || (opts->suboptions & OPTIONS_MPTCP_MPC))
 		return false;
 
 	/* account for the trailing 'nop' option */
@@ -1327,8 +1325,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 						   TCPOPT_NOP << 8 | TCPOPT_NOP, ptr);
 			}
 		}
-	} else if ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
-		    OPTION_MPTCP_MPC_ACK) & opts->suboptions) {
+	} else if (OPTIONS_MPTCP_MPC & opts->suboptions) {
 		u8 len, flag = MPTCP_CAP_HMAC_SHA256;
 
 		if (OPTION_MPTCP_MPC_SYN & opts->suboptions) {
-- 
2.31.1


Re: [MPTCP][PATCH v2 mptcp-next] mptcp: use OPTIONS_MPTCP_MPC

Posted by Matthieu Baerts 3 weeks, 1 day ago
Hi Geliang, Paolo, Mat,

On 02/09/2021 11:15, Geliang Tang wrote:
> From: Geliang Tang <geliangtang@xiaomi.com>
> 
> Since OPTIONS_MPTCP_MPC has been defined, use it instead of open-coding.

Thank you for this patch and the review!

Now in our tree, feature for net-next:

- ee243e703f11: mptcp: use OPTIONS_MPTCP_MPC
- Results: e1015c28d860..396fa2905e69

Builds and tests are now in progress:

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

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

Re: [MPTCP][PATCH v2 mptcp-next] mptcp: use OPTIONS_MPTCP_MPC

Posted by Mat Martineau 3 weeks, 2 days ago
On Thu, 2 Sep 2021, Geliang Tang wrote:

> From: Geliang Tang <geliangtang@xiaomi.com>
>
> Since OPTIONS_MPTCP_MPC has been defined, use it instead of open-coding.
>
> Acked-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>

Looks good to me, thanks Geliang.

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



> ---
> v2:
> - switch of yoda-style in mptcp_established_options_mp_prio as Paolo
>   suggested.
> - keep yoda-style in mptcp_write_options, since the other options in
>   this fuction all use the yoda-style conditions.
> ---
> net/mptcp/options.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 1ec6529c4326..422f4acfb3e6 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -748,9 +748,7 @@ static bool mptcp_established_options_mp_prio(struct sock *sk,
> 	/* can't send MP_PRIO with MPC, as they share the same option space:
> 	 * 'backup'. Also it makes no sense at all
> 	 */
> -	if (!subflow->send_mp_prio ||
> -	    ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
> -	      OPTION_MPTCP_MPC_ACK) & opts->suboptions))
> +	if (!subflow->send_mp_prio || (opts->suboptions & OPTIONS_MPTCP_MPC))
> 		return false;
>
> 	/* account for the trailing 'nop' option */
> @@ -1327,8 +1325,7 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> 						   TCPOPT_NOP << 8 | TCPOPT_NOP, ptr);
> 			}
> 		}
> -	} else if ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
> -		    OPTION_MPTCP_MPC_ACK) & opts->suboptions) {
> +	} else if (OPTIONS_MPTCP_MPC & opts->suboptions) {
> 		u8 len, flag = MPTCP_CAP_HMAC_SHA256;
>
> 		if (OPTION_MPTCP_MPC_SYN & opts->suboptions) {
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel