diff mbox series

[2/2] mptcp: Make v1 changes for ADD_ADDR and RM_ADDR options

Message ID 20200208013930.16640-3-peter.krystad@linux.intel.com
State Superseded, archived
Delegated to: Mat Martineau
Headers show
Series Add support for v1 of ADD_ADDR option | expand

Commit Message

Peter Krystad Feb. 8, 2020, 1:39 a.m. UTC
Use union to minimize space used in struct mptcp_received_options.
Remove family field and add hmac and port fields.

squashto: Add ADD_ADDR handling

Signed-off-by: Peter Krystad <peter.krystad@linux.intel.com>
---
 include/linux/tcp.h  |  22 +++---
 include/net/mptcp.h  |   2 +
 net/mptcp/options.c  | 180 ++++++++++++++++++++++++++++++++++---------
 net/mptcp/protocol.h |  17 ++--
 4 files changed, 170 insertions(+), 51 deletions(-)

Comments

Mat Martineau Feb. 12, 2020, 11:58 p.m. UTC | #1
On Fri, 7 Feb 2020, Peter Krystad wrote:

> Use union to minimize space used in struct mptcp_received_options.
> Remove family field and add hmac and port fields.
>
> squashto: Add ADD_ADDR handling
>
> Signed-off-by: Peter Krystad <peter.krystad@linux.intel.com>
> ---
> include/linux/tcp.h  |  22 +++---
> include/net/mptcp.h  |   2 +
> net/mptcp/options.c  | 180 ++++++++++++++++++++++++++++++++++---------
> net/mptcp/protocol.h |  17 ++--
> 4 files changed, 170 insertions(+), 51 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 0c75d7919f9a..42522be45cf7 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -86,9 +86,20 @@ struct mptcp_options_received {
> 	u64	data_seq;
> 	u32	subflow_seq;
> 	u16	data_len;
> +	struct in_addr	addr;
> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> +	struct in6_addr	addr6;
> +#endif

The commit message says "Use union...", but it looks like this moved addr 
and addr6 out of a union. Could you clarify the intent here?

> +	u64	ahmac;
> +	u8	addr_id;
> +	u8	rm_id;
> 	u8	mp_capable : 1,
> 		mp_join : 1,
> 		dss : 1,
> +		add_addr : 1,
> +		add_addr6 : 1,
> +		rm_addr : 1,
> +		echo : 1,
> 		backup : 1;
> 	u8	join_id;
> 	u32	token;
> @@ -102,16 +113,6 @@ struct mptcp_options_received {
> 		ack64:1,
> 		mpc_map:1,
> 		__unused:2;
> -	u8	add_addr : 1,
> -		rm_addr : 1,
> -		family : 4;
> -	u8	addr_id;
> -	union {
> -		struct	in_addr	addr;
> -#if IS_ENABLED(CONFIG_IPV6)
> -		struct	in6_addr addr6;
> -#endif
> -	};
> };
> #endif
>
> @@ -148,6 +149,7 @@ static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
> 	rx_opt->mptcp.mp_capable = 0;
> 	rx_opt->mptcp.mp_join = 0;
> 	rx_opt->mptcp.add_addr = 0;
> +	rx_opt->mptcp.add_addr6 = 0;
> 	rx_opt->mptcp.rm_addr = 0;
> 	rx_opt->mptcp.dss = 0;
> #endif
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 7489f9267640..0e7c5471010b 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -42,6 +42,8 @@ struct mptcp_out_options {
> #endif
> 	};
> 	u8 addr_id;
> +	u64 ahmac;
> +	u8 rm_id;
> 	u8 join_id;
> 	u8 backup;
> 	u32 nonce;
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 97d2328fbb52..43f65847799d 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -212,45 +212,56 @@ void mptcp_parse_option(const struct sk_buff *skb, const unsigned char *ptr,
> 		break;
>
> 	case MPTCPOPT_ADD_ADDR:
> -		if (opsize != TCPOLEN_MPTCP_ADD_ADDR &&
> -		    opsize != TCPOLEN_MPTCP_ADD_ADDR6)
> -			break;
> -		mp_opt->family = *ptr++ & MPTCP_ADDR_FAMILY_MASK;
> -		if (mp_opt->family != MPTCP_ADDR_IPVERSION_4 &&
> -		    mp_opt->family != MPTCP_ADDR_IPVERSION_6)
> -			break;
> -
> -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4 &&
> -		    opsize != TCPOLEN_MPTCP_ADD_ADDR)
> -			break;
> +		mp_opt->echo = (*ptr++) & MPTCP_ADDR_ECHO;
> +		if (!mp_opt->echo) {
> +			if (opsize == TCPOLEN_MPTCP_ADD_ADDR ||
> +			    opsize == TCPOLEN_MPTCP_ADD_ADDR_PORT)
> +				mp_opt->add_addr = 1;
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_6 &&
> -		    opsize != TCPOLEN_MPTCP_ADD_ADDR6)
> -			break;
> +			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6 ||
> +				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_PORT)
> +				mp_opt->add_addr6 = 1;
> +#endif
> +			else
> +				break;
> +		} else {
> +			if (opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE ||
> +			    opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT)
> +				mp_opt->add_addr = 1;
> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> +			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE ||
> +				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT)
> +				mp_opt->add_addr6 = 1;
> #endif
> +			else
> +				break;
> +		}
> +
> 		mp_opt->addr_id = *ptr++;
> -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4) {
> -			mp_opt->add_addr = 1;
> +		pr_debug("ADD_ADDR: id=%d", mp_opt->addr_id);
> +		if (mp_opt->add_addr == 1) {
> 			memcpy((u8 *)&mp_opt->addr.s_addr, (u8 *)ptr, 4);
> -			pr_debug("ADD_ADDR: addr=%x, id=%d",
> -				 mp_opt->addr.s_addr, mp_opt->addr_id);
> +			ptr += 4;
> 		}
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> 		else {
> -			mp_opt->add_addr = 1;
> 			memcpy(mp_opt->addr6.s6_addr, (u8 *)ptr, 16);
> -			pr_debug("ADD_ADDR: addr6=, id=%d", mp_opt->addr_id);
> +			ptr += 16;
> 		}
> #endif
> +		if (!mp_opt->echo) {
> +			mp_opt->ahmac = get_unaligned_be64(ptr);
> +			ptr += 8;
> +		}
> 		break;
>
> 	case MPTCPOPT_RM_ADDR:
> -		if (opsize != TCPOLEN_MPTCP_RM_ADDR)
> +		if (opsize != TCPOLEN_MPTCP_RM_ADDR_BASE)
> 			break;
>
> 		mp_opt->rm_addr = 1;
> -		mp_opt->addr_id = *ptr++;
> -		pr_debug("RM_ADDR: id=%d", mp_opt->addr_id);
> +		mp_opt->rm_id = *ptr++;
> +		pr_debug("RM_ADDR: id=%d", mp_opt->rm_id);
> 		break;
>
> 	default:
> @@ -482,6 +493,38 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> 	return true;
> }
>
> +static u64 add_addr_generate_hmac(u64 key1, u64 key2, u8 addr_id,
> +				  struct in_addr *addr)
> +{
> +	u8 hmac[MPTCPOPT_HMAC_LEN];
> +	u8 msg[7];
> +
> +	msg[0] = addr_id;
> +	memcpy(&msg[1], &addr->s_addr, 4);
> +	msg[5] = 0;
> +	msg[6] = 0;
> +
> +	mptcp_crypto_hmac_sha(key1, key2, msg, 7, (u32 *)hmac);
> +
> +	return get_unaligned_be64(hmac);
> +}
> +
> +static u64 add_addr6_generate_hmac(u64 key1, u64 key2, u8 addr_id,
> +				   struct in6_addr *addr)
> +{
> +	u8 hmac[MPTCPOPT_HMAC_LEN];
> +	u8 msg[19];
> +
> +	msg[0] = addr_id;
> +	memcpy(&msg[1], &addr->s6_addr, 16);
> +	msg[17] = 0;
> +	msg[18] = 0;
> +
> +	mptcp_crypto_hmac_sha(key1, key2, msg, 19, (u32 *)hmac);

Why the cast to u32* here?


Thanks,

Mat


> +
> +	return get_unaligned_be64(hmac);
> +}
> +
> static bool mptcp_established_options_addr(struct sock *sk,
> 					   unsigned int *size,
> 					   unsigned int remaining,
> @@ -507,6 +550,11 @@ static bool mptcp_established_options_addr(struct sock *sk,
> 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
> 		opts->addr_id = id;
> 		opts->addr = ((struct sockaddr_in *)&saddr)->sin_addr;
> +		opts->ahmac = add_addr_generate_hmac(subflow->local_key,
> +						     subflow->remote_key,
> +						     opts->addr_id,
> +						     &opts->addr);
> +		pr_debug("addr_id=%d, ahmac=%llu", opts->addr_id, opts->ahmac);
> 		*size = TCPOLEN_MPTCP_ADD_ADDR;
> 	}
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> @@ -516,6 +564,11 @@ static bool mptcp_established_options_addr(struct sock *sk,
> 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR6;
> 		opts->addr_id = id;
> 		opts->addr6 = ((struct sockaddr_in6 *)&saddr)->sin6_addr;
> +		opts->ahmac = add_addr6_generate_hmac(subflow->local_key,
> +						      subflow->remote_key,
> +						      opts->addr_id,
> +						      &opts->addr6);
> +		pr_debug("addr_id=%d, ahmac=%llu", opts->addr_id, opts->ahmac);
> 		*size = TCPOLEN_MPTCP_ADD_ADDR6;
> 	}
> #endif
> @@ -656,6 +709,36 @@ static void update_una(struct mptcp_sock *msk,
> 	}
> }
>
> +static bool add_addr_hmac_valid(struct mptcp_subflow_context *subflow,
> +				struct mptcp_options_received *mp_opt)
> +{
> +	u64 ahmac;
> +
> +	ahmac = add_addr_generate_hmac(subflow->remote_key, subflow->local_key,
> +				       mp_opt->addr_id, &mp_opt->addr);
> +
> +	pr_debug("subflow=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
> +		 subflow, (unsigned long long)ahmac,
> +		 (unsigned long long)mp_opt->ahmac);
> +
> +	return ahmac == mp_opt->ahmac;
> +}
> +
> +static bool add_addr6_hmac_valid(struct mptcp_subflow_context *subflow,
> +				 struct mptcp_options_received *mp_opt)
> +{
> +	u64 ahmac;
> +
> +	ahmac = add_addr6_generate_hmac(subflow->remote_key, subflow->local_key,
> +					mp_opt->addr_id, &mp_opt->addr6);
> +
> +	pr_debug("subflow=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
> +		 subflow, (unsigned long long)ahmac,
> +		 (unsigned long long)mp_opt->ahmac);
> +
> +	return ahmac == mp_opt->ahmac;
> +}
> +
> void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
> 			    struct tcp_options_received *opt_rx)
> {
> @@ -669,15 +752,20 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
> 		return;
>
> 	if (msk && mp_opt->add_addr) {
> -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4)
> -			mptcp_pm_add_addr(msk, &mp_opt->addr, mp_opt->addr_id);
> +		pr_debug("subflow=%p, ahmac=%llu", subflow, mp_opt->ahmac);
> +		if (!mp_opt->echo && add_addr_hmac_valid(subflow, mp_opt))
> +			mptcp_pm_add_addr(msk, &mp_opt->addr,
> +					  mp_opt->addr_id);
> +		mp_opt->add_addr = 0;
> +	}
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> -		else if (mp_opt->family == MPTCP_ADDR_IPVERSION_6)
> +	else if (msk && mp_opt->add_addr6) {
> +		if (!mp_opt->echo && add_addr6_hmac_valid(subflow, mp_opt))
> 			mptcp_pm_add_addr6(msk, &mp_opt->addr6,
> 					   mp_opt->addr_id);
> -#endif
> -		mp_opt->add_addr = 0;
> +		mp_opt->add_addr6 = 0;
> 	}
> +#endif
>
> 	if (!mp_opt->dss)
> 		return;
> @@ -760,25 +848,47 @@ void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts)
>
> mp_capable_done:
> 	if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
> -		*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, TCPOLEN_MPTCP_ADD_ADDR,
> -				      MPTCP_ADDR_IPVERSION_4, opts->addr_id);
> +		if (opts->ahmac)
> +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> +					      TCPOLEN_MPTCP_ADD_ADDR, 0,
> +					      opts->addr_id);
> +		else
> +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> +					      TCPOLEN_MPTCP_ADD_ADDR_BASE,
> +					      MPTCP_ADDR_ECHO,
> +					      opts->addr_id);
> 		memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4);
> 		ptr += 1;
> +		if (opts->ahmac) {
> +			put_unaligned_be64(opts->ahmac, ptr);
> +			ptr += 2;
> +		}
> 	}
>
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> 	if (OPTION_MPTCP_ADD_ADDR6 & opts->suboptions) {
> -		*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> -				      TCPOLEN_MPTCP_ADD_ADDR6,
> -				      MPTCP_ADDR_IPVERSION_6, opts->addr_id);
> +		if (opts->ahmac)
> +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> +					      TCPOLEN_MPTCP_ADD_ADDR6, 0,
> +					      opts->addr_id);
> +		else
> +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> +					      TCPOLEN_MPTCP_ADD_ADDR6_BASE,
> +					      MPTCP_ADDR_ECHO,
> +					      opts->addr_id);
> 		memcpy((u8 *)ptr, opts->addr6.s6_addr, 16);
> 		ptr += 4;
> +		if (opts->ahmac) {
> +			put_unaligned_be64(opts->ahmac, ptr);
> +			ptr += 2;
> +		}
> 	}
> #endif
>
> 	if (OPTION_MPTCP_RM_ADDR & opts->suboptions) {
> -		*ptr++ = mptcp_option(MPTCPOPT_RM_ADDR, TCPOLEN_MPTCP_RM_ADDR,
> -				      0, opts->addr_id);
> +		*ptr++ = mptcp_option(MPTCPOPT_RM_ADDR,
> +				      TCPOLEN_MPTCP_RM_ADDR_BASE,
> +				      0, opts->rm_id);
> 	}
>
> 	if (OPTION_MPTCP_MPJ_SYN & opts->suboptions) {
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index e10b24ba1636..c6f9e56b59f2 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -48,9 +48,16 @@
> #define TCPOLEN_MPTCP_DSS_MAP32		10
> #define TCPOLEN_MPTCP_DSS_MAP64		14
> #define TCPOLEN_MPTCP_DSS_CHECKSUM	2
> -#define TCPOLEN_MPTCP_ADD_ADDR		8
> -#define TCPOLEN_MPTCP_ADD_ADDR6		20
> -#define TCPOLEN_MPTCP_RM_ADDR		4
> +#define TCPOLEN_MPTCP_ADD_ADDR		16
> +#define TCPOLEN_MPTCP_ADD_ADDR_PORT	18
> +#define TCPOLEN_MPTCP_ADD_ADDR_BASE	8
> +#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT	10
> +#define TCPOLEN_MPTCP_ADD_ADDR6		28
> +#define TCPOLEN_MPTCP_ADD_ADDR6_PORT	30
> +#define TCPOLEN_MPTCP_ADD_ADDR6_BASE	20
> +#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT	22
> +#define TCPOLEN_MPTCP_PORT_LEN		2
> +#define TCPOLEN_MPTCP_RM_ADDR_BASE	4
>
> /* MPTCP MP_JOIN flags */
> #define MPTCPOPT_BACKUP		BIT(0)
> @@ -73,9 +80,7 @@
> #define MPTCP_DSS_FLAG_MASK	(0x1F)
>
> /* MPTCP ADD_ADDR flags */
> -#define MPTCP_ADDR_FAMILY_MASK	(0x0F)
> -#define MPTCP_ADDR_IPVERSION_4	4
> -#define MPTCP_ADDR_IPVERSION_6	6
> +#define MPTCP_ADDR_ECHO		BIT(0)
>
> /* MPTCP socket flags */
> #define MPTCP_DATA_READY	BIT(0)
> -- 
> 2.17.2

--
Mat Martineau
Intel
Peter Krystad Feb. 24, 2020, 8:46 p.m. UTC | #2
On Wed, 2020-02-12 at 15:58 -0800, Mat Martineau wrote:
> On Fri, 7 Feb 2020, Peter Krystad wrote:
> 
> > Use union to minimize space used in struct mptcp_received_options.
> > Remove family field and add hmac and port fields.
> > 
> > squashto: Add ADD_ADDR handling
> > 
> > Signed-off-by: Peter Krystad <peter.krystad@linux.intel.com>
> > ---
> > include/linux/tcp.h  |  22 +++---
> > include/net/mptcp.h  |   2 +
> > net/mptcp/options.c  | 180 ++++++++++++++++++++++++++++++++++---------
> > net/mptcp/protocol.h |  17 ++--
> > 4 files changed, 170 insertions(+), 51 deletions(-)
> > 
> > diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> > index 0c75d7919f9a..42522be45cf7 100644
> > --- a/include/linux/tcp.h
> > +++ b/include/linux/tcp.h
> > @@ -86,9 +86,20 @@ struct mptcp_options_received {
> > 	u64	data_seq;
> > 	u32	subflow_seq;
> > 	u16	data_len;
> > +	struct in_addr	addr;
> > +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > +	struct in6_addr	addr6;
> > +#endif
> 
> The commit message says "Use union...", but it looks like this moved addr 
> and addr6 out of a union. Could you clarify the intent here?

I will revise the commit message, and for now will stick with addr and addr6
in a union. This commit message was from old 'optimized
mptcp_options_received' commit.

> 
> > +	u64	ahmac;
> > +	u8	addr_id;
> > +	u8	rm_id;
> > 	u8	mp_capable : 1,
> > 		mp_join : 1,
> > 		dss : 1,
> > +		add_addr : 1,
> > +		add_addr6 : 1,
> > +		rm_addr : 1,
> > +		echo : 1,
> > 		backup : 1;
> > 	u8	join_id;
> > 	u32	token;
> > @@ -102,16 +113,6 @@ struct mptcp_options_received {
> > 		ack64:1,
> > 		mpc_map:1,
> > 		__unused:2;
> > -	u8	add_addr : 1,
> > -		rm_addr : 1,
> > -		family : 4;
> > -	u8	addr_id;
> > -	union {
> > -		struct	in_addr	addr;
> > -#if IS_ENABLED(CONFIG_IPV6)
> > -		struct	in6_addr addr6;
> > -#endif
> > -	};
> > };
> > #endif
> > 
> > @@ -148,6 +149,7 @@ static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
> > 	rx_opt->mptcp.mp_capable = 0;
> > 	rx_opt->mptcp.mp_join = 0;
> > 	rx_opt->mptcp.add_addr = 0;
> > +	rx_opt->mptcp.add_addr6 = 0;
> > 	rx_opt->mptcp.rm_addr = 0;
> > 	rx_opt->mptcp.dss = 0;
> > #endif
> > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > index 7489f9267640..0e7c5471010b 100644
> > --- a/include/net/mptcp.h
> > +++ b/include/net/mptcp.h
> > @@ -42,6 +42,8 @@ struct mptcp_out_options {
> > #endif
> > 	};
> > 	u8 addr_id;
> > +	u64 ahmac;
> > +	u8 rm_id;
> > 	u8 join_id;
> > 	u8 backup;
> > 	u32 nonce;
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index 97d2328fbb52..43f65847799d 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -212,45 +212,56 @@ void mptcp_parse_option(const struct sk_buff *skb, const unsigned char *ptr,
> > 		break;
> > 
> > 	case MPTCPOPT_ADD_ADDR:
> > -		if (opsize != TCPOLEN_MPTCP_ADD_ADDR &&
> > -		    opsize != TCPOLEN_MPTCP_ADD_ADDR6)
> > -			break;
> > -		mp_opt->family = *ptr++ & MPTCP_ADDR_FAMILY_MASK;
> > -		if (mp_opt->family != MPTCP_ADDR_IPVERSION_4 &&
> > -		    mp_opt->family != MPTCP_ADDR_IPVERSION_6)
> > -			break;
> > -
> > -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4 &&
> > -		    opsize != TCPOLEN_MPTCP_ADD_ADDR)
> > -			break;
> > +		mp_opt->echo = (*ptr++) & MPTCP_ADDR_ECHO;
> > +		if (!mp_opt->echo) {
> > +			if (opsize == TCPOLEN_MPTCP_ADD_ADDR ||
> > +			    opsize == TCPOLEN_MPTCP_ADD_ADDR_PORT)
> > +				mp_opt->add_addr = 1;
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_6 &&
> > -		    opsize != TCPOLEN_MPTCP_ADD_ADDR6)
> > -			break;
> > +			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6 ||
> > +				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_PORT)
> > +				mp_opt->add_addr6 = 1;
> > +#endif
> > +			else
> > +				break;
> > +		} else {
> > +			if (opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE ||
> > +			    opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT)
> > +				mp_opt->add_addr = 1;
> > +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > +			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE ||
> > +				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT)
> > +				mp_opt->add_addr6 = 1;
> > #endif
> > +			else
> > +				break;
> > +		}
> > +
> > 		mp_opt->addr_id = *ptr++;
> > -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4) {
> > -			mp_opt->add_addr = 1;
> > +		pr_debug("ADD_ADDR: id=%d", mp_opt->addr_id);
> > +		if (mp_opt->add_addr == 1) {
> > 			memcpy((u8 *)&mp_opt->addr.s_addr, (u8 *)ptr, 4);
> > -			pr_debug("ADD_ADDR: addr=%x, id=%d",
> > -				 mp_opt->addr.s_addr, mp_opt->addr_id);
> > +			ptr += 4;
> > 		}
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > 		else {
> > -			mp_opt->add_addr = 1;
> > 			memcpy(mp_opt->addr6.s6_addr, (u8 *)ptr, 16);
> > -			pr_debug("ADD_ADDR: addr6=, id=%d", mp_opt->addr_id);
> > +			ptr += 16;
> > 		}
> > #endif
> > +		if (!mp_opt->echo) {
> > +			mp_opt->ahmac = get_unaligned_be64(ptr);
> > +			ptr += 8;
> > +		}
> > 		break;
> > 
> > 	case MPTCPOPT_RM_ADDR:
> > -		if (opsize != TCPOLEN_MPTCP_RM_ADDR)
> > +		if (opsize != TCPOLEN_MPTCP_RM_ADDR_BASE)
> > 			break;
> > 
> > 		mp_opt->rm_addr = 1;
> > -		mp_opt->addr_id = *ptr++;
> > -		pr_debug("RM_ADDR: id=%d", mp_opt->addr_id);
> > +		mp_opt->rm_id = *ptr++;
> > +		pr_debug("RM_ADDR: id=%d", mp_opt->rm_id);
> > 		break;
> > 
> > 	default:
> > @@ -482,6 +493,38 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> > 	return true;
> > }
> > 
> > +static u64 add_addr_generate_hmac(u64 key1, u64 key2, u8 addr_id,
> > +				  struct in_addr *addr)
> > +{
> > +	u8 hmac[MPTCPOPT_HMAC_LEN];
> > +	u8 msg[7];
> > +
> > +	msg[0] = addr_id;
> > +	memcpy(&msg[1], &addr->s_addr, 4);
> > +	msg[5] = 0;
> > +	msg[6] = 0;
> > +
> > +	mptcp_crypto_hmac_sha(key1, key2, msg, 7, (u32 *)hmac);
> > +
> > +	return get_unaligned_be64(hmac);
> > +}
> > +
> > +static u64 add_addr6_generate_hmac(u64 key1, u64 key2, u8 addr_id,
> > +				   struct in6_addr *addr)
> > +{
> > +	u8 hmac[MPTCPOPT_HMAC_LEN];
> > +	u8 msg[19];
> > +
> > +	msg[0] = addr_id;
> > +	memcpy(&msg[1], &addr->s6_addr, 16);
> > +	msg[17] = 0;
> > +	msg[18] = 0;
> > +
> > +	mptcp_crypto_hmac_sha(key1, key2, msg, 19, (u32 *)hmac);
> 
> Why the cast to u32* here?

I'll remove these casts, they are holdovers from when the final param was a
u32*. It was changed to void* in Paolo's 'move from sha1 (v0) to sha256 (v1)' commit.

Peter.

> 
> Thanks,
> 
> Mat
> 
> 
> > +
> > +	return get_unaligned_be64(hmac);
> > +}
> > +
> > static bool mptcp_established_options_addr(struct sock *sk,
> > 					   unsigned int *size,
> > 					   unsigned int remaining,
> > @@ -507,6 +550,11 @@ static bool mptcp_established_options_addr(struct sock *sk,
> > 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
> > 		opts->addr_id = id;
> > 		opts->addr = ((struct sockaddr_in *)&saddr)->sin_addr;
> > +		opts->ahmac = add_addr_generate_hmac(subflow->local_key,
> > +						     subflow->remote_key,
> > +						     opts->addr_id,
> > +						     &opts->addr);
> > +		pr_debug("addr_id=%d, ahmac=%llu", opts->addr_id, opts->ahmac);
> > 		*size = TCPOLEN_MPTCP_ADD_ADDR;
> > 	}
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > @@ -516,6 +564,11 @@ static bool mptcp_established_options_addr(struct sock *sk,
> > 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR6;
> > 		opts->addr_id = id;
> > 		opts->addr6 = ((struct sockaddr_in6 *)&saddr)->sin6_addr;
> > +		opts->ahmac = add_addr6_generate_hmac(subflow->local_key,
> > +						      subflow->remote_key,
> > +						      opts->addr_id,
> > +						      &opts->addr6);
> > +		pr_debug("addr_id=%d, ahmac=%llu", opts->addr_id, opts->ahmac);
> > 		*size = TCPOLEN_MPTCP_ADD_ADDR6;
> > 	}
> > #endif
> > @@ -656,6 +709,36 @@ static void update_una(struct mptcp_sock *msk,
> > 	}
> > }
> > 
> > +static bool add_addr_hmac_valid(struct mptcp_subflow_context *subflow,
> > +				struct mptcp_options_received *mp_opt)
> > +{
> > +	u64 ahmac;
> > +
> > +	ahmac = add_addr_generate_hmac(subflow->remote_key, subflow->local_key,
> > +				       mp_opt->addr_id, &mp_opt->addr);
> > +
> > +	pr_debug("subflow=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
> > +		 subflow, (unsigned long long)ahmac,
> > +		 (unsigned long long)mp_opt->ahmac);
> > +
> > +	return ahmac == mp_opt->ahmac;
> > +}
> > +
> > +static bool add_addr6_hmac_valid(struct mptcp_subflow_context *subflow,
> > +				 struct mptcp_options_received *mp_opt)
> > +{
> > +	u64 ahmac;
> > +
> > +	ahmac = add_addr6_generate_hmac(subflow->remote_key, subflow->local_key,
> > +					mp_opt->addr_id, &mp_opt->addr6);
> > +
> > +	pr_debug("subflow=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
> > +		 subflow, (unsigned long long)ahmac,
> > +		 (unsigned long long)mp_opt->ahmac);
> > +
> > +	return ahmac == mp_opt->ahmac;
> > +}
> > +
> > void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
> > 			    struct tcp_options_received *opt_rx)
> > {
> > @@ -669,15 +752,20 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
> > 		return;
> > 
> > 	if (msk && mp_opt->add_addr) {
> > -		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4)
> > -			mptcp_pm_add_addr(msk, &mp_opt->addr, mp_opt->addr_id);
> > +		pr_debug("subflow=%p, ahmac=%llu", subflow, mp_opt->ahmac);
> > +		if (!mp_opt->echo && add_addr_hmac_valid(subflow, mp_opt))
> > +			mptcp_pm_add_addr(msk, &mp_opt->addr,
> > +					  mp_opt->addr_id);
> > +		mp_opt->add_addr = 0;
> > +	}
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > -		else if (mp_opt->family == MPTCP_ADDR_IPVERSION_6)
> > +	else if (msk && mp_opt->add_addr6) {
> > +		if (!mp_opt->echo && add_addr6_hmac_valid(subflow, mp_opt))
> > 			mptcp_pm_add_addr6(msk, &mp_opt->addr6,
> > 					   mp_opt->addr_id);
> > -#endif
> > -		mp_opt->add_addr = 0;
> > +		mp_opt->add_addr6 = 0;
> > 	}
> > +#endif
> > 
> > 	if (!mp_opt->dss)
> > 		return;
> > @@ -760,25 +848,47 @@ void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts)
> > 
> > mp_capable_done:
> > 	if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
> > -		*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, TCPOLEN_MPTCP_ADD_ADDR,
> > -				      MPTCP_ADDR_IPVERSION_4, opts->addr_id);
> > +		if (opts->ahmac)
> > +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> > +					      TCPOLEN_MPTCP_ADD_ADDR, 0,
> > +					      opts->addr_id);
> > +		else
> > +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> > +					      TCPOLEN_MPTCP_ADD_ADDR_BASE,
> > +					      MPTCP_ADDR_ECHO,
> > +					      opts->addr_id);
> > 		memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4);
> > 		ptr += 1;
> > +		if (opts->ahmac) {
> > +			put_unaligned_be64(opts->ahmac, ptr);
> > +			ptr += 2;
> > +		}
> > 	}
> > 
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > 	if (OPTION_MPTCP_ADD_ADDR6 & opts->suboptions) {
> > -		*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> > -				      TCPOLEN_MPTCP_ADD_ADDR6,
> > -				      MPTCP_ADDR_IPVERSION_6, opts->addr_id);
> > +		if (opts->ahmac)
> > +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> > +					      TCPOLEN_MPTCP_ADD_ADDR6, 0,
> > +					      opts->addr_id);
> > +		else
> > +			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
> > +					      TCPOLEN_MPTCP_ADD_ADDR6_BASE,
> > +					      MPTCP_ADDR_ECHO,
> > +					      opts->addr_id);
> > 		memcpy((u8 *)ptr, opts->addr6.s6_addr, 16);
> > 		ptr += 4;
> > +		if (opts->ahmac) {
> > +			put_unaligned_be64(opts->ahmac, ptr);
> > +			ptr += 2;
> > +		}
> > 	}
> > #endif
> > 
> > 	if (OPTION_MPTCP_RM_ADDR & opts->suboptions) {
> > -		*ptr++ = mptcp_option(MPTCPOPT_RM_ADDR, TCPOLEN_MPTCP_RM_ADDR,
> > -				      0, opts->addr_id);
> > +		*ptr++ = mptcp_option(MPTCPOPT_RM_ADDR,
> > +				      TCPOLEN_MPTCP_RM_ADDR_BASE,
> > +				      0, opts->rm_id);
> > 	}
> > 
> > 	if (OPTION_MPTCP_MPJ_SYN & opts->suboptions) {
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index e10b24ba1636..c6f9e56b59f2 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -48,9 +48,16 @@
> > #define TCPOLEN_MPTCP_DSS_MAP32		10
> > #define TCPOLEN_MPTCP_DSS_MAP64		14
> > #define TCPOLEN_MPTCP_DSS_CHECKSUM	2
> > -#define TCPOLEN_MPTCP_ADD_ADDR		8
> > -#define TCPOLEN_MPTCP_ADD_ADDR6		20
> > -#define TCPOLEN_MPTCP_RM_ADDR		4
> > +#define TCPOLEN_MPTCP_ADD_ADDR		16
> > +#define TCPOLEN_MPTCP_ADD_ADDR_PORT	18
> > +#define TCPOLEN_MPTCP_ADD_ADDR_BASE	8
> > +#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT	10
> > +#define TCPOLEN_MPTCP_ADD_ADDR6		28
> > +#define TCPOLEN_MPTCP_ADD_ADDR6_PORT	30
> > +#define TCPOLEN_MPTCP_ADD_ADDR6_BASE	20
> > +#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT	22
> > +#define TCPOLEN_MPTCP_PORT_LEN		2
> > +#define TCPOLEN_MPTCP_RM_ADDR_BASE	4
> > 
> > /* MPTCP MP_JOIN flags */
> > #define MPTCPOPT_BACKUP		BIT(0)
> > @@ -73,9 +80,7 @@
> > #define MPTCP_DSS_FLAG_MASK	(0x1F)
> > 
> > /* MPTCP ADD_ADDR flags */
> > -#define MPTCP_ADDR_FAMILY_MASK	(0x0F)
> > -#define MPTCP_ADDR_IPVERSION_4	4
> > -#define MPTCP_ADDR_IPVERSION_6	6
> > +#define MPTCP_ADDR_ECHO		BIT(0)
> > 
> > /* MPTCP socket flags */
> > #define MPTCP_DATA_READY	BIT(0)
> > -- 
> > 2.17.2
> 
> --
> Mat Martineau
> Intel
diff mbox series

Patch

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 0c75d7919f9a..42522be45cf7 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -86,9 +86,20 @@  struct mptcp_options_received {
 	u64	data_seq;
 	u32	subflow_seq;
 	u16	data_len;
+	struct in_addr	addr;
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+	struct in6_addr	addr6;
+#endif
+	u64	ahmac;
+	u8	addr_id;
+	u8	rm_id;
 	u8	mp_capable : 1,
 		mp_join : 1,
 		dss : 1,
+		add_addr : 1,
+		add_addr6 : 1,
+		rm_addr : 1,
+		echo : 1,
 		backup : 1;
 	u8	join_id;
 	u32	token;
@@ -102,16 +113,6 @@  struct mptcp_options_received {
 		ack64:1,
 		mpc_map:1,
 		__unused:2;
-	u8	add_addr : 1,
-		rm_addr : 1,
-		family : 4;
-	u8	addr_id;
-	union {
-		struct	in_addr	addr;
-#if IS_ENABLED(CONFIG_IPV6)
-		struct	in6_addr addr6;
-#endif
-	};
 };
 #endif
 
@@ -148,6 +149,7 @@  static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
 	rx_opt->mptcp.mp_capable = 0;
 	rx_opt->mptcp.mp_join = 0;
 	rx_opt->mptcp.add_addr = 0;
+	rx_opt->mptcp.add_addr6 = 0;
 	rx_opt->mptcp.rm_addr = 0;
 	rx_opt->mptcp.dss = 0;
 #endif
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 7489f9267640..0e7c5471010b 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -42,6 +42,8 @@  struct mptcp_out_options {
 #endif
 	};
 	u8 addr_id;
+	u64 ahmac;
+	u8 rm_id;
 	u8 join_id;
 	u8 backup;
 	u32 nonce;
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 97d2328fbb52..43f65847799d 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -212,45 +212,56 @@  void mptcp_parse_option(const struct sk_buff *skb, const unsigned char *ptr,
 		break;
 
 	case MPTCPOPT_ADD_ADDR:
-		if (opsize != TCPOLEN_MPTCP_ADD_ADDR &&
-		    opsize != TCPOLEN_MPTCP_ADD_ADDR6)
-			break;
-		mp_opt->family = *ptr++ & MPTCP_ADDR_FAMILY_MASK;
-		if (mp_opt->family != MPTCP_ADDR_IPVERSION_4 &&
-		    mp_opt->family != MPTCP_ADDR_IPVERSION_6)
-			break;
-
-		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4 &&
-		    opsize != TCPOLEN_MPTCP_ADD_ADDR)
-			break;
+		mp_opt->echo = (*ptr++) & MPTCP_ADDR_ECHO;
+		if (!mp_opt->echo) {
+			if (opsize == TCPOLEN_MPTCP_ADD_ADDR ||
+			    opsize == TCPOLEN_MPTCP_ADD_ADDR_PORT)
+				mp_opt->add_addr = 1;
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-		if (mp_opt->family == MPTCP_ADDR_IPVERSION_6 &&
-		    opsize != TCPOLEN_MPTCP_ADD_ADDR6)
-			break;
+			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6 ||
+				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_PORT)
+				mp_opt->add_addr6 = 1;
+#endif
+			else
+				break;
+		} else {
+			if (opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE ||
+			    opsize == TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT)
+				mp_opt->add_addr = 1;
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+			else if (opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE ||
+				 opsize == TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT)
+				mp_opt->add_addr6 = 1;
 #endif
+			else
+				break;
+		}
+
 		mp_opt->addr_id = *ptr++;
-		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4) {
-			mp_opt->add_addr = 1;
+		pr_debug("ADD_ADDR: id=%d", mp_opt->addr_id);
+		if (mp_opt->add_addr == 1) {
 			memcpy((u8 *)&mp_opt->addr.s_addr, (u8 *)ptr, 4);
-			pr_debug("ADD_ADDR: addr=%x, id=%d",
-				 mp_opt->addr.s_addr, mp_opt->addr_id);
+			ptr += 4;
 		}
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 		else {
-			mp_opt->add_addr = 1;
 			memcpy(mp_opt->addr6.s6_addr, (u8 *)ptr, 16);
-			pr_debug("ADD_ADDR: addr6=, id=%d", mp_opt->addr_id);
+			ptr += 16;
 		}
 #endif
+		if (!mp_opt->echo) {
+			mp_opt->ahmac = get_unaligned_be64(ptr);
+			ptr += 8;
+		}
 		break;
 
 	case MPTCPOPT_RM_ADDR:
-		if (opsize != TCPOLEN_MPTCP_RM_ADDR)
+		if (opsize != TCPOLEN_MPTCP_RM_ADDR_BASE)
 			break;
 
 		mp_opt->rm_addr = 1;
-		mp_opt->addr_id = *ptr++;
-		pr_debug("RM_ADDR: id=%d", mp_opt->addr_id);
+		mp_opt->rm_id = *ptr++;
+		pr_debug("RM_ADDR: id=%d", mp_opt->rm_id);
 		break;
 
 	default:
@@ -482,6 +493,38 @@  static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
 	return true;
 }
 
+static u64 add_addr_generate_hmac(u64 key1, u64 key2, u8 addr_id,
+				  struct in_addr *addr)
+{
+	u8 hmac[MPTCPOPT_HMAC_LEN];
+	u8 msg[7];
+
+	msg[0] = addr_id;
+	memcpy(&msg[1], &addr->s_addr, 4);
+	msg[5] = 0;
+	msg[6] = 0;
+
+	mptcp_crypto_hmac_sha(key1, key2, msg, 7, (u32 *)hmac);
+
+	return get_unaligned_be64(hmac);
+}
+
+static u64 add_addr6_generate_hmac(u64 key1, u64 key2, u8 addr_id,
+				   struct in6_addr *addr)
+{
+	u8 hmac[MPTCPOPT_HMAC_LEN];
+	u8 msg[19];
+
+	msg[0] = addr_id;
+	memcpy(&msg[1], &addr->s6_addr, 16);
+	msg[17] = 0;
+	msg[18] = 0;
+
+	mptcp_crypto_hmac_sha(key1, key2, msg, 19, (u32 *)hmac);
+
+	return get_unaligned_be64(hmac);
+}
+
 static bool mptcp_established_options_addr(struct sock *sk,
 					   unsigned int *size,
 					   unsigned int remaining,
@@ -507,6 +550,11 @@  static bool mptcp_established_options_addr(struct sock *sk,
 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
 		opts->addr_id = id;
 		opts->addr = ((struct sockaddr_in *)&saddr)->sin_addr;
+		opts->ahmac = add_addr_generate_hmac(subflow->local_key,
+						     subflow->remote_key,
+						     opts->addr_id,
+						     &opts->addr);
+		pr_debug("addr_id=%d, ahmac=%llu", opts->addr_id, opts->ahmac);
 		*size = TCPOLEN_MPTCP_ADD_ADDR;
 	}
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
@@ -516,6 +564,11 @@  static bool mptcp_established_options_addr(struct sock *sk,
 		opts->suboptions |= OPTION_MPTCP_ADD_ADDR6;
 		opts->addr_id = id;
 		opts->addr6 = ((struct sockaddr_in6 *)&saddr)->sin6_addr;
+		opts->ahmac = add_addr6_generate_hmac(subflow->local_key,
+						      subflow->remote_key,
+						      opts->addr_id,
+						      &opts->addr6);
+		pr_debug("addr_id=%d, ahmac=%llu", opts->addr_id, opts->ahmac);
 		*size = TCPOLEN_MPTCP_ADD_ADDR6;
 	}
 #endif
@@ -656,6 +709,36 @@  static void update_una(struct mptcp_sock *msk,
 	}
 }
 
+static bool add_addr_hmac_valid(struct mptcp_subflow_context *subflow,
+				struct mptcp_options_received *mp_opt)
+{
+	u64 ahmac;
+
+	ahmac = add_addr_generate_hmac(subflow->remote_key, subflow->local_key,
+				       mp_opt->addr_id, &mp_opt->addr);
+
+	pr_debug("subflow=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
+		 subflow, (unsigned long long)ahmac,
+		 (unsigned long long)mp_opt->ahmac);
+
+	return ahmac == mp_opt->ahmac;
+}
+
+static bool add_addr6_hmac_valid(struct mptcp_subflow_context *subflow,
+				 struct mptcp_options_received *mp_opt)
+{
+	u64 ahmac;
+
+	ahmac = add_addr6_generate_hmac(subflow->remote_key, subflow->local_key,
+					mp_opt->addr_id, &mp_opt->addr6);
+
+	pr_debug("subflow=%p, ahmac=%llu, mp_opt->ahmac=%llu\n",
+		 subflow, (unsigned long long)ahmac,
+		 (unsigned long long)mp_opt->ahmac);
+
+	return ahmac == mp_opt->ahmac;
+}
+
 void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
 			    struct tcp_options_received *opt_rx)
 {
@@ -669,15 +752,20 @@  void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
 		return;
 
 	if (msk && mp_opt->add_addr) {
-		if (mp_opt->family == MPTCP_ADDR_IPVERSION_4)
-			mptcp_pm_add_addr(msk, &mp_opt->addr, mp_opt->addr_id);
+		pr_debug("subflow=%p, ahmac=%llu", subflow, mp_opt->ahmac);
+		if (!mp_opt->echo && add_addr_hmac_valid(subflow, mp_opt))
+			mptcp_pm_add_addr(msk, &mp_opt->addr,
+					  mp_opt->addr_id);
+		mp_opt->add_addr = 0;
+	}
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-		else if (mp_opt->family == MPTCP_ADDR_IPVERSION_6)
+	else if (msk && mp_opt->add_addr6) {
+		if (!mp_opt->echo && add_addr6_hmac_valid(subflow, mp_opt))
 			mptcp_pm_add_addr6(msk, &mp_opt->addr6,
 					   mp_opt->addr_id);
-#endif
-		mp_opt->add_addr = 0;
+		mp_opt->add_addr6 = 0;
 	}
+#endif
 
 	if (!mp_opt->dss)
 		return;
@@ -760,25 +848,47 @@  void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts)
 
 mp_capable_done:
 	if (OPTION_MPTCP_ADD_ADDR & opts->suboptions) {
-		*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR, TCPOLEN_MPTCP_ADD_ADDR,
-				      MPTCP_ADDR_IPVERSION_4, opts->addr_id);
+		if (opts->ahmac)
+			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
+					      TCPOLEN_MPTCP_ADD_ADDR, 0,
+					      opts->addr_id);
+		else
+			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
+					      TCPOLEN_MPTCP_ADD_ADDR_BASE,
+					      MPTCP_ADDR_ECHO,
+					      opts->addr_id);
 		memcpy((u8 *)ptr, (u8 *)&opts->addr.s_addr, 4);
 		ptr += 1;
+		if (opts->ahmac) {
+			put_unaligned_be64(opts->ahmac, ptr);
+			ptr += 2;
+		}
 	}
 
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 	if (OPTION_MPTCP_ADD_ADDR6 & opts->suboptions) {
-		*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
-				      TCPOLEN_MPTCP_ADD_ADDR6,
-				      MPTCP_ADDR_IPVERSION_6, opts->addr_id);
+		if (opts->ahmac)
+			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
+					      TCPOLEN_MPTCP_ADD_ADDR6, 0,
+					      opts->addr_id);
+		else
+			*ptr++ = mptcp_option(MPTCPOPT_ADD_ADDR,
+					      TCPOLEN_MPTCP_ADD_ADDR6_BASE,
+					      MPTCP_ADDR_ECHO,
+					      opts->addr_id);
 		memcpy((u8 *)ptr, opts->addr6.s6_addr, 16);
 		ptr += 4;
+		if (opts->ahmac) {
+			put_unaligned_be64(opts->ahmac, ptr);
+			ptr += 2;
+		}
 	}
 #endif
 
 	if (OPTION_MPTCP_RM_ADDR & opts->suboptions) {
-		*ptr++ = mptcp_option(MPTCPOPT_RM_ADDR, TCPOLEN_MPTCP_RM_ADDR,
-				      0, opts->addr_id);
+		*ptr++ = mptcp_option(MPTCPOPT_RM_ADDR,
+				      TCPOLEN_MPTCP_RM_ADDR_BASE,
+				      0, opts->rm_id);
 	}
 
 	if (OPTION_MPTCP_MPJ_SYN & opts->suboptions) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e10b24ba1636..c6f9e56b59f2 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -48,9 +48,16 @@ 
 #define TCPOLEN_MPTCP_DSS_MAP32		10
 #define TCPOLEN_MPTCP_DSS_MAP64		14
 #define TCPOLEN_MPTCP_DSS_CHECKSUM	2
-#define TCPOLEN_MPTCP_ADD_ADDR		8
-#define TCPOLEN_MPTCP_ADD_ADDR6		20
-#define TCPOLEN_MPTCP_RM_ADDR		4
+#define TCPOLEN_MPTCP_ADD_ADDR		16
+#define TCPOLEN_MPTCP_ADD_ADDR_PORT	18
+#define TCPOLEN_MPTCP_ADD_ADDR_BASE	8
+#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT	10
+#define TCPOLEN_MPTCP_ADD_ADDR6		28
+#define TCPOLEN_MPTCP_ADD_ADDR6_PORT	30
+#define TCPOLEN_MPTCP_ADD_ADDR6_BASE	20
+#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT	22
+#define TCPOLEN_MPTCP_PORT_LEN		2
+#define TCPOLEN_MPTCP_RM_ADDR_BASE	4
 
 /* MPTCP MP_JOIN flags */
 #define MPTCPOPT_BACKUP		BIT(0)
@@ -73,9 +80,7 @@ 
 #define MPTCP_DSS_FLAG_MASK	(0x1F)
 
 /* MPTCP ADD_ADDR flags */
-#define MPTCP_ADDR_FAMILY_MASK	(0x0F)
-#define MPTCP_ADDR_IPVERSION_4	4
-#define MPTCP_ADDR_IPVERSION_6	6
+#define MPTCP_ADDR_ECHO		BIT(0)
 
 /* MPTCP socket flags */
 #define MPTCP_DATA_READY	BIT(0)