[MPTCP] Re: [MPTCP][PATCH v2 mptcp-next 7/7] mptcp: drop unnecessary CONFIG_MPTCP_IPV6

[Mat Martineau <mathew.j.martineau at linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]

On Tue, 16 Mar 2021, Geliang Tang wrote:

> This patch dropped all the unnecessary '#if IS_ENABLED(CONFIG_MPTCP_IPV6)'
> and '#endif'.
>

If IPV6 is not enabled, what's the benefit of including the extra IPV6 
code? I can see a possible benefit to parsing and sending echo responses 
to peers that might send ADD_ADDR with IPv6 (because they would not know 
that the other peer does not have IPv6 enabled), but I don't think that 
accounts for all the removals in this commit. It would help to explain the 
motivation in the commit message!

Thanks,

Mat


> Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
> ---
> net/mptcp/options.c    | 6 ------
> net/mptcp/pm_netlink.c | 8 +-------
> 2 files changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index e3fcd2b0ffd7..913cfc83bae4 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -221,22 +221,18 @@ static void mptcp_parse_option(const struct sk_buff *skb,
> 			if (opsize == TCPOLEN_MPTCP_ADD_ADDR ||
> 			    opsize == TCPOLEN_MPTCP_ADD_ADDR_PORT)
> 				mp_opt->addr.family = AF_INET;
> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> 			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6 ||
> 				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_PORT)
> 				mp_opt->addr.family = AF_INET6;
> -#endif
> 			else
> 				break;
> 		} else {
> 			if (opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE ||
> 			    opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT)
> 				mp_opt->addr.family = AF_INET;
> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> 			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE ||
> 				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT)
> 				mp_opt->addr.family = AF_INET6;
> -#endif
> 			else
> 				break;
> 		}
> @@ -1155,10 +1151,8 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> 		u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
> 		u8 echo = MPTCP_ADDR_ECHO;
>
> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> 		if (opts->addr.family == AF_INET6)
> 			len = TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> -#endif
>
> 		if (opts->addr.port)
> 			len += TCPOLEN_MPTCP_PORT_LEN;
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 181d8048cac1..5691ad101a95 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -889,10 +889,8 @@ static const struct nla_policy mptcp_pm_policy[MPTCP_PM_ATTR_MAX + 1] = {
>
> static int mptcp_pm_family_to_addr(int family)
> {
> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> 	if (family == AF_INET6)
> 		return MPTCP_PM_ADDR_ATTR_ADDR6;
> -#endif
> 	return MPTCP_PM_ADDR_ATTR_ADDR4;
> }
>
> @@ -925,11 +923,7 @@ static int mptcp_pm_parse_addr(struct nlattr *attr, struct genl_info *info,
> 	}
>
> 	entry->addr.family = nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_FAMILY]);
> -	if (entry->addr.family != AF_INET
> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> -	    && entry->addr.family != AF_INET6
> -#endif
> -	    ) {
> +	if (entry->addr.family != AF_INET && entry->addr.family != AF_INET6) {
> 		NL_SET_ERR_MSG_ATTR(info->extack, attr,
> 				    "unknown address family");
> 		return -EINVAL;
> -- 
> 2.30.2

--
Mat Martineau
Intel