[MPTCP] [PATCH net-next v5 00/11] Multipath TCP: Prerequisites

[MPTCP] [PATCH net-next v5 00/11] Multipath TCP: Prerequisites

[Mat Martineau]

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

v4 -> v5: Cover letter subject fix. No changes to commits.

v3 -> v4: Update coalesce/collapse of incoming MPTCP skbs (patch 7)

v2 -> v3: Ensure sk_type alignment in struct sock (patch 2)

v1 -> v2: sk_pacing_shift left as a regular struct member (patch 2), and
modified SACK space check based on recent -net fix (patch 9).


The MPTCP upstreaming community has been collaborating on an
upstreamable MPTCP implementation that complies with RFC 8684. A minimal
set of features to comply with the specification involves a sizeable set
of code changes, so David requested that we split this work in to
multiple, smaller patch sets to build up MPTCP infrastructure.

The minimal MPTCP feature set we are proposing for review in the v5.6
timeframe begins with these three parts:

Part 1 (this patch set): MPTCP prerequisites. Introduce some MPTCP
definitions, additional ULP and skb extension features, TCP option space
checking, and a few exported symbols.

Part 2: Single subflow implementation and self tests.

Part 3: Switch from MPTCP v0 (RFC 6824) to MPTCP v1 (new RFC 8684,
publication expected in the next few days).

We plan to send those over the next week. Additional patches for
multiple subflow support, path management, active backup, and other
features are in the pipeline for submission after making progress with
the above reviews.

Clone/fetch:
https://github.com/multipath-tcp/mptcp_net-next.git (tag: netdev-v5-part1)

Browse:
https://github.com/multipath-tcp/mptcp_net-next/tree/netdev-v5-part1

Thank you for your review. You can find us at mptcp(a)lists.01.org and
https://is.gd/mptcp_upstream


Mat Martineau (9):
  net: Make sock protocol value checks more specific
  sock: Make sk_protocol a 16-bit value
  tcp: Define IPPROTO_MPTCP
  tcp: Add MPTCP option number
  tcp, ulp: Add clone operation to tcp_ulp_ops
  mptcp: Add MPTCP to skb extensions
  tcp: coalesce/collapse must respect MPTCP extensions
  tcp: Export TCP functions and ops struct
  tcp: Check for filled TCP option space before SACK

Paolo Abeni (2):
  tcp: clean ext on tx recycle
  skb: add helpers to allocate ext independently from sk_buff

 MAINTAINERS                     | 10 ++++
 include/linux/skbuff.h          |  6 +++
 include/net/mptcp.h             | 81 +++++++++++++++++++++++++++++++++
 include/net/sock.h              | 12 ++---
 include/net/tcp.h               | 22 +++++++++
 include/trace/events/sock.h     |  5 +-
 include/uapi/linux/in.h         |  2 +
 net/ax25/af_ax25.c              |  2 +-
 net/core/skbuff.c               | 42 ++++++++++++++++-
 net/decnet/af_decnet.c          |  2 +-
 net/ipv4/inet_connection_sock.c |  2 +
 net/ipv4/tcp.c                  |  6 +--
 net/ipv4/tcp_input.c            | 11 +++--
 net/ipv4/tcp_ipv4.c             |  2 +-
 net/ipv4/tcp_output.c           | 12 +++--
 net/ipv4/tcp_ulp.c              | 12 +++++
 net/ipv6/tcp_ipv6.c             |  6 +--
 tools/include/uapi/linux/in.h   |  2 +
 18 files changed, 211 insertions(+), 26 deletions(-)
 create mode 100644 include/net/mptcp.h

-- 
2.24.1

[PATCH net-next v5 00/11] Multipath TCP: Prerequisites

[Mat Martineau]

v4 -> v5: Cover letter subject fix. No changes to commits.

v3 -> v4: Update coalesce/collapse of incoming MPTCP skbs (patch 7)

v2 -> v3: Ensure sk_type alignment in struct sock (patch 2)

v1 -> v2: sk_pacing_shift left as a regular struct member (patch 2), and
modified SACK space check based on recent -net fix (patch 9).


The MPTCP upstreaming community has been collaborating on an
upstreamable MPTCP implementation that complies with RFC 8684. A minimal
set of features to comply with the specification involves a sizeable set
of code changes, so David requested that we split this work in to
multiple, smaller patch sets to build up MPTCP infrastructure.

The minimal MPTCP feature set we are proposing for review in the v5.6
timeframe begins with these three parts:

Part 1 (this patch set): MPTCP prerequisites. Introduce some MPTCP
definitions, additional ULP and skb extension features, TCP option space
checking, and a few exported symbols.

Part 2: Single subflow implementation and self tests.

Part 3: Switch from MPTCP v0 (RFC 6824) to MPTCP v1 (new RFC 8684,
publication expected in the next few days).

We plan to send those over the next week. Additional patches for
multiple subflow support, path management, active backup, and other
features are in the pipeline for submission after making progress with
the above reviews.

Clone/fetch:
https://github.com/multipath-tcp/mptcp_net-next.git (tag: netdev-v5-part1)

Browse:
https://github.com/multipath-tcp/mptcp_net-next/tree/netdev-v5-part1

Thank you for your review. You can find us at mptcp@lists.01.org and
https://is.gd/mptcp_upstream


Mat Martineau (9):
  net: Make sock protocol value checks more specific
  sock: Make sk_protocol a 16-bit value
  tcp: Define IPPROTO_MPTCP
  tcp: Add MPTCP option number
  tcp, ulp: Add clone operation to tcp_ulp_ops
  mptcp: Add MPTCP to skb extensions
  tcp: coalesce/collapse must respect MPTCP extensions
  tcp: Export TCP functions and ops struct
  tcp: Check for filled TCP option space before SACK

Paolo Abeni (2):
  tcp: clean ext on tx recycle
  skb: add helpers to allocate ext independently from sk_buff

 MAINTAINERS                     | 10 ++++
 include/linux/skbuff.h          |  6 +++
 include/net/mptcp.h             | 81 +++++++++++++++++++++++++++++++++
 include/net/sock.h              | 12 ++---
 include/net/tcp.h               | 22 +++++++++
 include/trace/events/sock.h     |  5 +-
 include/uapi/linux/in.h         |  2 +
 net/ax25/af_ax25.c              |  2 +-
 net/core/skbuff.c               | 42 ++++++++++++++++-
 net/decnet/af_decnet.c          |  2 +-
 net/ipv4/inet_connection_sock.c |  2 +
 net/ipv4/tcp.c                  |  6 +--
 net/ipv4/tcp_input.c            | 11 +++--
 net/ipv4/tcp_ipv4.c             |  2 +-
 net/ipv4/tcp_output.c           | 12 +++--
 net/ipv4/tcp_ulp.c              | 12 +++++
 net/ipv6/tcp_ipv6.c             |  6 +--
 tools/include/uapi/linux/in.h   |  2 +
 18 files changed, 211 insertions(+), 26 deletions(-)
 create mode 100644 include/net/mptcp.h

-- 
2.24.1

[MPTCP] [PATCH net-next v5 01/11] net: Make sock protocol value checks more specific

[Mat Martineau]

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

SK_PROTOCOL_MAX is only used in two places, for DECNet and AX.25. The
limits have more to do with the those protocol definitions than they do
with the data type of sk_protocol, so remove SK_PROTOCOL_MAX and use
U8_MAX directly.

Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/sock.h     | 1 -
 net/ax25/af_ax25.c     | 2 +-
 net/decnet/af_decnet.c | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 87d54ef57f00..81dc811aad2e 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -458,7 +458,6 @@ struct sock {
 				sk_userlocks : 4,
 				sk_protocol  : 8,
 				sk_type      : 16;
-#define SK_PROTOCOL_MAX U8_MAX
 	u16			sk_gso_max_segs;
 	u8			sk_pacing_shift;
 	unsigned long	        sk_lingertime;
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 324306d6fde0..ff57ea89c27e 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -808,7 +808,7 @@ static int ax25_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	ax25_cb *ax25;
 
-	if (protocol < 0 || protocol > SK_PROTOCOL_MAX)
+	if (protocol < 0 || protocol > U8_MAX)
 		return -EINVAL;
 
 	if (!net_eq(net, &init_net))
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index e19a92a62e14..0a46ea3bddd5 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -670,7 +670,7 @@ static int dn_create(struct net *net, struct socket *sock, int protocol,
 {
 	struct sock *sk;
 
-	if (protocol < 0 || protocol > SK_PROTOCOL_MAX)
+	if (protocol < 0 || protocol > U8_MAX)
 		return -EINVAL;
 
 	if (!net_eq(net, &init_net))
-- 
2.24.1

[PATCH net-next v5 01/11] net: Make sock protocol value checks more specific

[Mat Martineau]

SK_PROTOCOL_MAX is only used in two places, for DECNet and AX.25. The
limits have more to do with the those protocol definitions than they do
with the data type of sk_protocol, so remove SK_PROTOCOL_MAX and use
U8_MAX directly.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/sock.h     | 1 -
 net/ax25/af_ax25.c     | 2 +-
 net/decnet/af_decnet.c | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 87d54ef57f00..81dc811aad2e 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -458,7 +458,6 @@ struct sock {
 				sk_userlocks : 4,
 				sk_protocol  : 8,
 				sk_type      : 16;
-#define SK_PROTOCOL_MAX U8_MAX
 	u16			sk_gso_max_segs;
 	u8			sk_pacing_shift;
 	unsigned long	        sk_lingertime;
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 324306d6fde0..ff57ea89c27e 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -808,7 +808,7 @@ static int ax25_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	ax25_cb *ax25;
 
-	if (protocol < 0 || protocol > SK_PROTOCOL_MAX)
+	if (protocol < 0 || protocol > U8_MAX)
 		return -EINVAL;
 
 	if (!net_eq(net, &init_net))
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index e19a92a62e14..0a46ea3bddd5 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -670,7 +670,7 @@ static int dn_create(struct net *net, struct socket *sock, int protocol,
 {
 	struct sock *sk;
 
-	if (protocol < 0 || protocol > SK_PROTOCOL_MAX)
+	if (protocol < 0 || protocol > U8_MAX)
 		return -EINVAL;
 
 	if (!net_eq(net, &init_net))
-- 
2.24.1

[MPTCP] [PATCH net-next v5 02/11] sock: Make sk_protocol a 16-bit value

[Mat Martineau]

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

Match the 16-bit width of skbuff->protocol. Fills an 8-bit hole so
sizeof(struct sock) does not change.

v2 -> v3:
 - keep 'sk_type' 2 bytes aligned (Eric)
v1 -> v2:
 - preserve sk_pacing_shift as bit field (Eric)

Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Co-developed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/sock.h          | 10 +++++-----
 include/trace/events/sock.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 81dc811aad2e..b93cadba1a3b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -451,15 +451,15 @@ struct sock {
 #define SK_FL_TYPE_MASK    0xffff0000
 #endif
 
-	unsigned int		sk_padding : 1,
+	u8			sk_padding : 1,
 				sk_kern_sock : 1,
 				sk_no_check_tx : 1,
 				sk_no_check_rx : 1,
-				sk_userlocks : 4,
-				sk_protocol  : 8,
-				sk_type      : 16;
-	u16			sk_gso_max_segs;
+				sk_userlocks : 4;
 	u8			sk_pacing_shift;
+	u16			sk_type;
+	u16			sk_protocol;
+	u16			sk_gso_max_segs;
 	unsigned long	        sk_lingertime;
 	struct proto		*sk_prot_creator;
 	rwlock_t		sk_callback_lock;
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 51fe9f6719eb..3ff12b90048d 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -147,7 +147,7 @@ TRACE_EVENT(inet_sock_set_state,
 		__field(__u16, sport)
 		__field(__u16, dport)
 		__field(__u16, family)
-		__field(__u8, protocol)
+		__field(__u16, protocol)
 		__array(__u8, saddr, 4)
 		__array(__u8, daddr, 4)
 		__array(__u8, saddr_v6, 16)
-- 
2.24.1

[PATCH net-next v5 02/11] sock: Make sk_protocol a 16-bit value

[Mat Martineau]

Match the 16-bit width of skbuff->protocol. Fills an 8-bit hole so
sizeof(struct sock) does not change.

v2 -> v3:
 - keep 'sk_type' 2 bytes aligned (Eric)
v1 -> v2:
 - preserve sk_pacing_shift as bit field (Eric)

Co-developed-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/sock.h          | 10 +++++-----
 include/trace/events/sock.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 81dc811aad2e..b93cadba1a3b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -451,15 +451,15 @@ struct sock {
 #define SK_FL_TYPE_MASK    0xffff0000
 #endif
 
-	unsigned int		sk_padding : 1,
+	u8			sk_padding : 1,
 				sk_kern_sock : 1,
 				sk_no_check_tx : 1,
 				sk_no_check_rx : 1,
-				sk_userlocks : 4,
-				sk_protocol  : 8,
-				sk_type      : 16;
-	u16			sk_gso_max_segs;
+				sk_userlocks : 4;
 	u8			sk_pacing_shift;
+	u16			sk_type;
+	u16			sk_protocol;
+	u16			sk_gso_max_segs;
 	unsigned long	        sk_lingertime;
 	struct proto		*sk_prot_creator;
 	rwlock_t		sk_callback_lock;
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 51fe9f6719eb..3ff12b90048d 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -147,7 +147,7 @@ TRACE_EVENT(inet_sock_set_state,
 		__field(__u16, sport)
 		__field(__u16, dport)
 		__field(__u16, family)
-		__field(__u8, protocol)
+		__field(__u16, protocol)
 		__array(__u8, saddr, 4)
 		__array(__u8, daddr, 4)
 		__array(__u8, saddr_v6, 16)
-- 
2.24.1

[MPTCP] [PATCH net-next v5 03/11] tcp: Define IPPROTO_MPTCP

[Mat Martineau]

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

To open a MPTCP socket with socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP),
IPPROTO_MPTCP needs a value that differs from IPPROTO_TCP. The existing
IPPROTO numbers mostly map directly to IANA-specified protocol numbers.
MPTCP does not have a protocol number allocated because MPTCP packets
use the TCP protocol number. Use private number not used OTA.

Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/trace/events/sock.h   | 3 ++-
 include/uapi/linux/in.h       | 2 ++
 tools/include/uapi/linux/in.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 3ff12b90048d..a966d4b5ab37 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -19,7 +19,8 @@
 #define inet_protocol_names		\
 		EM(IPPROTO_TCP)			\
 		EM(IPPROTO_DCCP)		\
-		EMe(IPPROTO_SCTP)
+		EM(IPPROTO_SCTP)		\
+		EMe(IPPROTO_MPTCP)
 
 #define tcp_state_names			\
 		EM(TCP_ESTABLISHED)		\
diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index e7ad9d350a28..1521073b6348 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -76,6 +76,8 @@ enum {
 #define IPPROTO_MPLS		IPPROTO_MPLS
   IPPROTO_RAW = 255,		/* Raw IP packets			*/
 #define IPPROTO_RAW		IPPROTO_RAW
+  IPPROTO_MPTCP = 262,		/* Multipath TCP connection		*/
+#define IPPROTO_MPTCP		IPPROTO_MPTCP
   IPPROTO_MAX
 };
 #endif
diff --git a/tools/include/uapi/linux/in.h b/tools/include/uapi/linux/in.h
index e7ad9d350a28..1521073b6348 100644
--- a/tools/include/uapi/linux/in.h
+++ b/tools/include/uapi/linux/in.h
@@ -76,6 +76,8 @@ enum {
 #define IPPROTO_MPLS		IPPROTO_MPLS
   IPPROTO_RAW = 255,		/* Raw IP packets			*/
 #define IPPROTO_RAW		IPPROTO_RAW
+  IPPROTO_MPTCP = 262,		/* Multipath TCP connection		*/
+#define IPPROTO_MPTCP		IPPROTO_MPTCP
   IPPROTO_MAX
 };
 #endif
-- 
2.24.1

[PATCH net-next v5 03/11] tcp: Define IPPROTO_MPTCP

[Mat Martineau]

To open a MPTCP socket with socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP),
IPPROTO_MPTCP needs a value that differs from IPPROTO_TCP. The existing
IPPROTO numbers mostly map directly to IANA-specified protocol numbers.
MPTCP does not have a protocol number allocated because MPTCP packets
use the TCP protocol number. Use private number not used OTA.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/trace/events/sock.h   | 3 ++-
 include/uapi/linux/in.h       | 2 ++
 tools/include/uapi/linux/in.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 3ff12b90048d..a966d4b5ab37 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -19,7 +19,8 @@
 #define inet_protocol_names		\
 		EM(IPPROTO_TCP)			\
 		EM(IPPROTO_DCCP)		\
-		EMe(IPPROTO_SCTP)
+		EM(IPPROTO_SCTP)		\
+		EMe(IPPROTO_MPTCP)
 
 #define tcp_state_names			\
 		EM(TCP_ESTABLISHED)		\
diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index e7ad9d350a28..1521073b6348 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -76,6 +76,8 @@ enum {
 #define IPPROTO_MPLS		IPPROTO_MPLS
   IPPROTO_RAW = 255,		/* Raw IP packets			*/
 #define IPPROTO_RAW		IPPROTO_RAW
+  IPPROTO_MPTCP = 262,		/* Multipath TCP connection		*/
+#define IPPROTO_MPTCP		IPPROTO_MPTCP
   IPPROTO_MAX
 };
 #endif
diff --git a/tools/include/uapi/linux/in.h b/tools/include/uapi/linux/in.h
index e7ad9d350a28..1521073b6348 100644
--- a/tools/include/uapi/linux/in.h
+++ b/tools/include/uapi/linux/in.h
@@ -76,6 +76,8 @@ enum {
 #define IPPROTO_MPLS		IPPROTO_MPLS
   IPPROTO_RAW = 255,		/* Raw IP packets			*/
 #define IPPROTO_RAW		IPPROTO_RAW
+  IPPROTO_MPTCP = 262,		/* Multipath TCP connection		*/
+#define IPPROTO_MPTCP		IPPROTO_MPTCP
   IPPROTO_MAX
 };
 #endif
-- 
2.24.1

[MPTCP] [PATCH net-next v5 04/11] tcp: Add MPTCP option number

[Mat Martineau]

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

TCP option 30 is allocated for MPTCP by the IANA.

Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/tcp.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 86b9a8766648..d4b6bf2c5d3c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -182,6 +182,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
 #define TCPOPT_SACK             5       /* SACK Block */
 #define TCPOPT_TIMESTAMP	8	/* Better RTT estimations/PAWS */
 #define TCPOPT_MD5SIG		19	/* MD5 Signature (RFC2385) */
+#define TCPOPT_MPTCP		30	/* Multipath TCP (RFC6824) */
 #define TCPOPT_FASTOPEN		34	/* Fast open (RFC7413) */
 #define TCPOPT_EXP		254	/* Experimental */
 /* Magic number to be after the option value for sharing TCP
-- 
2.24.1

[PATCH net-next v5 04/11] tcp: Add MPTCP option number

[Mat Martineau]

TCP option 30 is allocated for MPTCP by the IANA.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/tcp.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 86b9a8766648..d4b6bf2c5d3c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -182,6 +182,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
 #define TCPOPT_SACK             5       /* SACK Block */
 #define TCPOPT_TIMESTAMP	8	/* Better RTT estimations/PAWS */
 #define TCPOPT_MD5SIG		19	/* MD5 Signature (RFC2385) */
+#define TCPOPT_MPTCP		30	/* Multipath TCP (RFC6824) */
 #define TCPOPT_FASTOPEN		34	/* Fast open (RFC7413) */
 #define TCPOPT_EXP		254	/* Experimental */
 /* Magic number to be after the option value for sharing TCP
-- 
2.24.1

[MPTCP] [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Mat Martineau]

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

If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
copied when the listener is cloned. Sometimes the clone is immediately
deleted, which will invoke the release op on the clone and likely
corrupt the listening socket's icsk_ulp_data.

The clone operation is invoked immediately after the clone is copied and
gives the ULP type an opportunity to set up the clone socket and its
icsk_ulp_data.

Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/tcp.h               |  5 +++++
 net/ipv4/inet_connection_sock.c |  2 ++
 net/ipv4/tcp_ulp.c              | 12 ++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d4b6bf2c5d3c..c82b2f75d024 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2145,6 +2145,9 @@ struct tcp_ulp_ops {
 	/* diagnostic */
 	int (*get_info)(const struct sock *sk, struct sk_buff *skb);
 	size_t (*get_info_size)(const struct sock *sk);
+	/* clone ulp */
+	void (*clone)(const struct request_sock *req, struct sock *newsk,
+		      const gfp_t priority);
 
 	char		name[TCP_ULP_NAME_MAX];
 	struct module	*owner;
@@ -2155,6 +2158,8 @@ int tcp_set_ulp(struct sock *sk, const char *name);
 void tcp_get_available_ulp(char *buf, size_t len);
 void tcp_cleanup_ulp(struct sock *sk);
 void tcp_update_ulp(struct sock *sk, struct proto *p);
+void tcp_clone_ulp(const struct request_sock *req,
+		   struct sock *newsk, const gfp_t priority);
 
 #define MODULE_ALIAS_TCP_ULP(name)				\
 	__MODULE_INFO(alias, alias_userspace, name);		\
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index e4c6e8b40490..d667f2569f8e 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -810,6 +810,8 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
 		/* Deinitialize accept_queue to trap illegal accesses. */
 		memset(&newicsk->icsk_accept_queue, 0, sizeof(newicsk->icsk_accept_queue));
 
+		tcp_clone_ulp(req, newsk, priority);
+
 		security_inet_csk_clone(newsk, req);
 	}
 	return newsk;
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
index 12ab5db2b71c..e7a2589d69ee 100644
--- a/net/ipv4/tcp_ulp.c
+++ b/net/ipv4/tcp_ulp.c
@@ -130,6 +130,18 @@ void tcp_cleanup_ulp(struct sock *sk)
 	icsk->icsk_ulp_ops = NULL;
 }
 
+void tcp_clone_ulp(const struct request_sock *req, struct sock *newsk,
+		   const gfp_t priority)
+{
+	struct inet_connection_sock *icsk = inet_csk(newsk);
+
+	if (!icsk->icsk_ulp_ops)
+		return;
+
+	if (icsk->icsk_ulp_ops->clone)
+		icsk->icsk_ulp_ops->clone(req, newsk, priority);
+}
+
 static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
-- 
2.24.1

[PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Mat Martineau]

If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
copied when the listener is cloned. Sometimes the clone is immediately
deleted, which will invoke the release op on the clone and likely
corrupt the listening socket's icsk_ulp_data.

The clone operation is invoked immediately after the clone is copied and
gives the ULP type an opportunity to set up the clone socket and its
icsk_ulp_data.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/tcp.h               |  5 +++++
 net/ipv4/inet_connection_sock.c |  2 ++
 net/ipv4/tcp_ulp.c              | 12 ++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d4b6bf2c5d3c..c82b2f75d024 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2145,6 +2145,9 @@ struct tcp_ulp_ops {
 	/* diagnostic */
 	int (*get_info)(const struct sock *sk, struct sk_buff *skb);
 	size_t (*get_info_size)(const struct sock *sk);
+	/* clone ulp */
+	void (*clone)(const struct request_sock *req, struct sock *newsk,
+		      const gfp_t priority);
 
 	char		name[TCP_ULP_NAME_MAX];
 	struct module	*owner;
@@ -2155,6 +2158,8 @@ int tcp_set_ulp(struct sock *sk, const char *name);
 void tcp_get_available_ulp(char *buf, size_t len);
 void tcp_cleanup_ulp(struct sock *sk);
 void tcp_update_ulp(struct sock *sk, struct proto *p);
+void tcp_clone_ulp(const struct request_sock *req,
+		   struct sock *newsk, const gfp_t priority);
 
 #define MODULE_ALIAS_TCP_ULP(name)				\
 	__MODULE_INFO(alias, alias_userspace, name);		\
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index e4c6e8b40490..d667f2569f8e 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -810,6 +810,8 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
 		/* Deinitialize accept_queue to trap illegal accesses. */
 		memset(&newicsk->icsk_accept_queue, 0, sizeof(newicsk->icsk_accept_queue));
 
+		tcp_clone_ulp(req, newsk, priority);
+
 		security_inet_csk_clone(newsk, req);
 	}
 	return newsk;
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
index 12ab5db2b71c..e7a2589d69ee 100644
--- a/net/ipv4/tcp_ulp.c
+++ b/net/ipv4/tcp_ulp.c
@@ -130,6 +130,18 @@ void tcp_cleanup_ulp(struct sock *sk)
 	icsk->icsk_ulp_ops = NULL;
 }
 
+void tcp_clone_ulp(const struct request_sock *req, struct sock *newsk,
+		   const gfp_t priority)
+{
+	struct inet_connection_sock *icsk = inet_csk(newsk);
+
+	if (!icsk->icsk_ulp_ops)
+		return;
+
+	if (icsk->icsk_ulp_ops->clone)
+		icsk->icsk_ulp_ops->clone(req, newsk, priority);
+}
+
 static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
-- 
2.24.1

[MPTCP] [PATCH net-next v5 06/11] mptcp: Add MPTCP to skb extensions

[Mat Martineau]

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

Add enum value for MPTCP and update config dependencies

Co-developed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 MAINTAINERS            | 10 ++++++++++
 include/linux/skbuff.h |  3 +++
 include/net/mptcp.h    | 27 +++++++++++++++++++++++++++
 net/core/skbuff.c      |  7 +++++++
 4 files changed, 47 insertions(+)
 create mode 100644 include/net/mptcp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a28c77ee6b0d..132eb3f9fbb7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11568,6 +11568,16 @@ F:	net/ipv6/calipso.c
 F:	net/netfilter/xt_CONNSECMARK.c
 F:	net/netfilter/xt_SECMARK.c
 
+NETWORKING [MPTCP]
+M:	Mat Martineau <mathew.j.martineau(a)linux.intel.com>
+M:	Matthieu Baerts <matthieu.baerts(a)tessares.net>
+L:	netdev(a)vger.kernel.org
+L:	mptcp(a)lists.01.org
+W:	https://github.com/multipath-tcp/mptcp_net-next/wiki
+B:	https://github.com/multipath-tcp/mptcp_net-next/issues
+S:	Maintained
+F:	include/net/mptcp.h
+
 NETWORKING [TCP]
 M:	Eric Dumazet <edumazet(a)google.com>
 L:	netdev(a)vger.kernel.org
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e9133bcf0544..1a261c3ee074 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4091,6 +4091,9 @@ enum skb_ext_id {
 #endif
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 	TC_SKB_EXT,
+#endif
+#if IS_ENABLED(CONFIG_MPTCP)
+	SKB_EXT_MPTCP,
 #endif
 	SKB_EXT_NUM, /* must be last */
 };
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
new file mode 100644
index 000000000000..f9f668ac4339
--- /dev/null
+++ b/include/net/mptcp.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Multipath TCP
+ *
+ * Copyright (c) 2017 - 2019, Intel Corporation.
+ */
+
+#ifndef __NET_MPTCP_H
+#define __NET_MPTCP_H
+
+#include <linux/types.h>
+
+/* MPTCP sk_buff extension data */
+struct mptcp_ext {
+	u64		data_ack;
+	u64		data_seq;
+	u32		subflow_seq;
+	u16		data_len;
+	u8		use_map:1,
+			dsn64:1,
+			data_fin:1,
+			use_ack:1,
+			ack64:1,
+			__unused:2;
+};
+
+#endif /* __NET_MPTCP_H */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 973a71f4bc89..fa67036dd928 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -68,6 +68,7 @@
 #include <net/ip6_checksum.h>
 #include <net/xfrm.h>
 #include <net/mpls.h>
+#include <net/mptcp.h>
 
 #include <linux/uaccess.h>
 #include <trace/events/skb.h>
@@ -4109,6 +4110,9 @@ static const u8 skb_ext_type_len[] = {
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 	[TC_SKB_EXT] = SKB_EXT_CHUNKSIZEOF(struct tc_skb_ext),
 #endif
+#if IS_ENABLED(CONFIG_MPTCP)
+	[SKB_EXT_MPTCP] = SKB_EXT_CHUNKSIZEOF(struct mptcp_ext),
+#endif
 };
 
 static __always_inline unsigned int skb_ext_total_length(void)
@@ -4122,6 +4126,9 @@ static __always_inline unsigned int skb_ext_total_length(void)
 #endif
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 		skb_ext_type_len[TC_SKB_EXT] +
+#endif
+#if IS_ENABLED(CONFIG_MPTCP)
+		skb_ext_type_len[SKB_EXT_MPTCP] +
 #endif
 		0;
 }
-- 
2.24.1

[PATCH net-next v5 06/11] mptcp: Add MPTCP to skb extensions

[Mat Martineau]

Add enum value for MPTCP and update config dependencies

Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 MAINTAINERS            | 10 ++++++++++
 include/linux/skbuff.h |  3 +++
 include/net/mptcp.h    | 27 +++++++++++++++++++++++++++
 net/core/skbuff.c      |  7 +++++++
 4 files changed, 47 insertions(+)
 create mode 100644 include/net/mptcp.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a28c77ee6b0d..132eb3f9fbb7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11568,6 +11568,16 @@ F:	net/ipv6/calipso.c
 F:	net/netfilter/xt_CONNSECMARK.c
 F:	net/netfilter/xt_SECMARK.c
 
+NETWORKING [MPTCP]
+M:	Mat Martineau <mathew.j.martineau@linux.intel.com>
+M:	Matthieu Baerts <matthieu.baerts@tessares.net>
+L:	netdev@vger.kernel.org
+L:	mptcp@lists.01.org
+W:	https://github.com/multipath-tcp/mptcp_net-next/wiki
+B:	https://github.com/multipath-tcp/mptcp_net-next/issues
+S:	Maintained
+F:	include/net/mptcp.h
+
 NETWORKING [TCP]
 M:	Eric Dumazet <edumazet@google.com>
 L:	netdev@vger.kernel.org
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e9133bcf0544..1a261c3ee074 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4091,6 +4091,9 @@ enum skb_ext_id {
 #endif
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 	TC_SKB_EXT,
+#endif
+#if IS_ENABLED(CONFIG_MPTCP)
+	SKB_EXT_MPTCP,
 #endif
 	SKB_EXT_NUM, /* must be last */
 };
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
new file mode 100644
index 000000000000..f9f668ac4339
--- /dev/null
+++ b/include/net/mptcp.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Multipath TCP
+ *
+ * Copyright (c) 2017 - 2019, Intel Corporation.
+ */
+
+#ifndef __NET_MPTCP_H
+#define __NET_MPTCP_H
+
+#include <linux/types.h>
+
+/* MPTCP sk_buff extension data */
+struct mptcp_ext {
+	u64		data_ack;
+	u64		data_seq;
+	u32		subflow_seq;
+	u16		data_len;
+	u8		use_map:1,
+			dsn64:1,
+			data_fin:1,
+			use_ack:1,
+			ack64:1,
+			__unused:2;
+};
+
+#endif /* __NET_MPTCP_H */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 973a71f4bc89..fa67036dd928 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -68,6 +68,7 @@
 #include <net/ip6_checksum.h>
 #include <net/xfrm.h>
 #include <net/mpls.h>
+#include <net/mptcp.h>
 
 #include <linux/uaccess.h>
 #include <trace/events/skb.h>
@@ -4109,6 +4110,9 @@ static const u8 skb_ext_type_len[] = {
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 	[TC_SKB_EXT] = SKB_EXT_CHUNKSIZEOF(struct tc_skb_ext),
 #endif
+#if IS_ENABLED(CONFIG_MPTCP)
+	[SKB_EXT_MPTCP] = SKB_EXT_CHUNKSIZEOF(struct mptcp_ext),
+#endif
 };
 
 static __always_inline unsigned int skb_ext_total_length(void)
@@ -4122,6 +4126,9 @@ static __always_inline unsigned int skb_ext_total_length(void)
 #endif
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 		skb_ext_type_len[TC_SKB_EXT] +
+#endif
+#if IS_ENABLED(CONFIG_MPTCP)
+		skb_ext_type_len[SKB_EXT_MPTCP] +
 #endif
 		0;
 }
-- 
2.24.1

[MPTCP] [PATCH net-next v5 07/11] tcp: coalesce/collapse must respect MPTCP extensions

[Mat Martineau]

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

Coalesce and collapse of packets carrying MPTCP extensions is allowed
when the newer packet has no extension or the extensions carried by both
packets are equal.

This allows merging of TSO packet trains and even cross-TSO packets, and
does not require any additional action when moving data into existing
SKBs.

v3 -> v4:
 - allow collapsing, under mptcp_skb_can_collapse() constraint

Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/mptcp.h   | 54 +++++++++++++++++++++++++++++++++++++++++++
 include/net/tcp.h     |  8 +++++++
 net/ipv4/tcp_input.c  | 11 ++++++---
 net/ipv4/tcp_output.c |  2 +-
 4 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index f9f668ac4339..8e27e33861ab 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -8,6 +8,7 @@
 #ifndef __NET_MPTCP_H
 #define __NET_MPTCP_H
 
+#include <linux/skbuff.h>
 #include <linux/types.h>
 
 /* MPTCP sk_buff extension data */
@@ -24,4 +25,57 @@ struct mptcp_ext {
 			__unused:2;
 };
 
+#ifdef CONFIG_MPTCP
+
+/* move the skb extension owership, with the assumption that 'to' is
+ * newly allocated
+ */
+static inline void mptcp_skb_ext_move(struct sk_buff *to,
+				      struct sk_buff *from)
+{
+	if (!skb_ext_exist(from, SKB_EXT_MPTCP))
+		return;
+
+	if (WARN_ON_ONCE(to->active_extensions))
+		skb_ext_put(to);
+
+	to->active_extensions = from->active_extensions;
+	to->extensions = from->extensions;
+	from->active_extensions = 0;
+}
+
+static inline bool mptcp_ext_matches(const struct mptcp_ext *to_ext,
+				     const struct mptcp_ext *from_ext)
+{
+	return !from_ext ||
+	       (to_ext && from_ext &&
+	        !memcmp(from_ext, to_ext, sizeof(struct mptcp_ext)));
+}
+
+/* check if skbs can be collapsed.
+ * MPTCP collapse is allowed if neither @to or @from carry an mptcp data
+ * mapping, or if the extension of @to is the same as @from.
+ * Collapsing is not possible if @to lacks an extension, but @from carries one.
+ */
+static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
+					  const struct sk_buff *from)
+{
+	return mptcp_ext_matches(skb_ext_find(to, SKB_EXT_MPTCP),
+				 skb_ext_find(from, SKB_EXT_MPTCP));
+}
+
+#else
+
+static inline void mptcp_skb_ext_move(struct sk_buff *to,
+				      const struct sk_buff *from)
+{
+}
+
+static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
+					  const struct sk_buff *from)
+{
+	return true;
+}
+
+#endif /* CONFIG_MPTCP */
 #endif /* __NET_MPTCP_H */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index c82b2f75d024..f4cddd42d52a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -39,6 +39,7 @@
 #include <net/tcp_states.h>
 #include <net/inet_ecn.h>
 #include <net/dst.h>
+#include <net/mptcp.h>
 
 #include <linux/seq_file.h>
 #include <linux/memcontrol.h>
@@ -978,6 +979,13 @@ static inline bool tcp_skb_can_collapse_to(const struct sk_buff *skb)
 	return likely(!TCP_SKB_CB(skb)->eor);
 }
 
+static inline bool tcp_skb_can_collapse(const struct sk_buff *to,
+					const struct sk_buff *from)
+{
+	return likely(tcp_skb_can_collapse_to(to) &&
+		      mptcp_skb_can_collapse(to, from));
+}
+
 /* Events passed to congestion control interface */
 enum tcp_ca_event {
 	CA_EVENT_TX_START,	/* first transmit when no packets in flight */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 88b987ca9ebb..a16d9f2a0529 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1422,7 +1422,7 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
 	if ((TCP_SKB_CB(prev)->sacked & TCPCB_TAGBITS) != TCPCB_SACKED_ACKED)
 		goto fallback;
 
-	if (!tcp_skb_can_collapse_to(prev))
+	if (!tcp_skb_can_collapse(prev, skb))
 		goto fallback;
 
 	in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
@@ -4420,6 +4420,9 @@ static bool tcp_try_coalesce(struct sock *sk,
 	if (TCP_SKB_CB(from)->seq != TCP_SKB_CB(to)->end_seq)
 		return false;
 
+	if (!mptcp_skb_can_collapse(to, from))
+		return false;
+
 #ifdef CONFIG_TLS_DEVICE
 	if (from->decrypted != to->decrypted)
 		return false;
@@ -4929,7 +4932,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		/* The first skb to collapse is:
 		 * - not SYN/FIN and
 		 * - bloated or contains data before "start" or
-		 *   overlaps to the next one.
+		 *   overlaps to the next one and mptcp allow collapsing.
 		 */
 		if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
 		    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
@@ -4938,7 +4941,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 			break;
 		}
 
-		if (n && n != tail &&
+		if (n && n != tail && mptcp_skb_can_collapse(skb, n) &&
 		    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
 			end_of_skbs = false;
 			break;
@@ -4971,6 +4974,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		else
 			__skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
 		skb_set_owner_r(nskb, sk);
+		mptcp_skb_ext_move(nskb, skb);
 
 		/* Copy data, releasing collapsed skbs. */
 		while (copy > 0) {
@@ -4990,6 +4994,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 				skb = tcp_collapse_one(sk, skb, list, root);
 				if (!skb ||
 				    skb == tail ||
+				    !mptcp_skb_can_collapse(nskb, skb) ||
 				    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
 					goto end;
 #ifdef CONFIG_TLS_DEVICE
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b184f03d7437..9e04d45bc0e4 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2854,7 +2854,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
 		if (!tcp_can_collapse(sk, skb))
 			break;
 
-		if (!tcp_skb_can_collapse_to(to))
+		if (!tcp_skb_can_collapse(to, skb))
 			break;
 
 		space -= skb->len;
-- 
2.24.1

[PATCH net-next v5 07/11] tcp: coalesce/collapse must respect MPTCP extensions

[Mat Martineau]

Coalesce and collapse of packets carrying MPTCP extensions is allowed
when the newer packet has no extension or the extensions carried by both
packets are equal.

This allows merging of TSO packet trains and even cross-TSO packets, and
does not require any additional action when moving data into existing
SKBs.

v3 -> v4:
 - allow collapsing, under mptcp_skb_can_collapse() constraint

Co-developed-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/mptcp.h   | 54 +++++++++++++++++++++++++++++++++++++++++++
 include/net/tcp.h     |  8 +++++++
 net/ipv4/tcp_input.c  | 11 ++++++---
 net/ipv4/tcp_output.c |  2 +-
 4 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index f9f668ac4339..8e27e33861ab 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -8,6 +8,7 @@
 #ifndef __NET_MPTCP_H
 #define __NET_MPTCP_H
 
+#include <linux/skbuff.h>
 #include <linux/types.h>
 
 /* MPTCP sk_buff extension data */
@@ -24,4 +25,57 @@ struct mptcp_ext {
 			__unused:2;
 };
 
+#ifdef CONFIG_MPTCP
+
+/* move the skb extension owership, with the assumption that 'to' is
+ * newly allocated
+ */
+static inline void mptcp_skb_ext_move(struct sk_buff *to,
+				      struct sk_buff *from)
+{
+	if (!skb_ext_exist(from, SKB_EXT_MPTCP))
+		return;
+
+	if (WARN_ON_ONCE(to->active_extensions))
+		skb_ext_put(to);
+
+	to->active_extensions = from->active_extensions;
+	to->extensions = from->extensions;
+	from->active_extensions = 0;
+}
+
+static inline bool mptcp_ext_matches(const struct mptcp_ext *to_ext,
+				     const struct mptcp_ext *from_ext)
+{
+	return !from_ext ||
+	       (to_ext && from_ext &&
+	        !memcmp(from_ext, to_ext, sizeof(struct mptcp_ext)));
+}
+
+/* check if skbs can be collapsed.
+ * MPTCP collapse is allowed if neither @to or @from carry an mptcp data
+ * mapping, or if the extension of @to is the same as @from.
+ * Collapsing is not possible if @to lacks an extension, but @from carries one.
+ */
+static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
+					  const struct sk_buff *from)
+{
+	return mptcp_ext_matches(skb_ext_find(to, SKB_EXT_MPTCP),
+				 skb_ext_find(from, SKB_EXT_MPTCP));
+}
+
+#else
+
+static inline void mptcp_skb_ext_move(struct sk_buff *to,
+				      const struct sk_buff *from)
+{
+}
+
+static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
+					  const struct sk_buff *from)
+{
+	return true;
+}
+
+#endif /* CONFIG_MPTCP */
 #endif /* __NET_MPTCP_H */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index c82b2f75d024..f4cddd42d52a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -39,6 +39,7 @@
 #include <net/tcp_states.h>
 #include <net/inet_ecn.h>
 #include <net/dst.h>
+#include <net/mptcp.h>
 
 #include <linux/seq_file.h>
 #include <linux/memcontrol.h>
@@ -978,6 +979,13 @@ static inline bool tcp_skb_can_collapse_to(const struct sk_buff *skb)
 	return likely(!TCP_SKB_CB(skb)->eor);
 }
 
+static inline bool tcp_skb_can_collapse(const struct sk_buff *to,
+					const struct sk_buff *from)
+{
+	return likely(tcp_skb_can_collapse_to(to) &&
+		      mptcp_skb_can_collapse(to, from));
+}
+
 /* Events passed to congestion control interface */
 enum tcp_ca_event {
 	CA_EVENT_TX_START,	/* first transmit when no packets in flight */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 88b987ca9ebb..a16d9f2a0529 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1422,7 +1422,7 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
 	if ((TCP_SKB_CB(prev)->sacked & TCPCB_TAGBITS) != TCPCB_SACKED_ACKED)
 		goto fallback;
 
-	if (!tcp_skb_can_collapse_to(prev))
+	if (!tcp_skb_can_collapse(prev, skb))
 		goto fallback;
 
 	in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
@@ -4420,6 +4420,9 @@ static bool tcp_try_coalesce(struct sock *sk,
 	if (TCP_SKB_CB(from)->seq != TCP_SKB_CB(to)->end_seq)
 		return false;
 
+	if (!mptcp_skb_can_collapse(to, from))
+		return false;
+
 #ifdef CONFIG_TLS_DEVICE
 	if (from->decrypted != to->decrypted)
 		return false;
@@ -4929,7 +4932,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		/* The first skb to collapse is:
 		 * - not SYN/FIN and
 		 * - bloated or contains data before "start" or
-		 *   overlaps to the next one.
+		 *   overlaps to the next one and mptcp allow collapsing.
 		 */
 		if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
 		    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
@@ -4938,7 +4941,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 			break;
 		}
 
-		if (n && n != tail &&
+		if (n && n != tail && mptcp_skb_can_collapse(skb, n) &&
 		    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
 			end_of_skbs = false;
 			break;
@@ -4971,6 +4974,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		else
 			__skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
 		skb_set_owner_r(nskb, sk);
+		mptcp_skb_ext_move(nskb, skb);
 
 		/* Copy data, releasing collapsed skbs. */
 		while (copy > 0) {
@@ -4990,6 +4994,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 				skb = tcp_collapse_one(sk, skb, list, root);
 				if (!skb ||
 				    skb == tail ||
+				    !mptcp_skb_can_collapse(nskb, skb) ||
 				    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
 					goto end;
 #ifdef CONFIG_TLS_DEVICE
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b184f03d7437..9e04d45bc0e4 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2854,7 +2854,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
 		if (!tcp_can_collapse(sk, skb))
 			break;
 
-		if (!tcp_skb_can_collapse_to(to))
+		if (!tcp_skb_can_collapse(to, skb))
 			break;
 
 		space -= skb->len;
-- 
2.24.1

[MPTCP] [PATCH net-next v5 08/11] tcp: Export TCP functions and ops struct

[Mat Martineau]

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

MPTCP will make use of tcp_send_mss() and tcp_push() when sending
data to specific TCP subflows.

tcp_request_sock_ipvX_ops and ipvX_specific will be referenced
during TCP subflow creation.

Co-developed-by: Peter Krystad <peter.krystad(a)linux.intel.com>
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/tcp.h   | 8 ++++++++
 net/ipv4/tcp.c      | 6 +++---
 net/ipv4/tcp_ipv4.c | 2 +-
 net/ipv6/tcp_ipv6.c | 6 +++---
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index f4cddd42d52a..0a3533c019c6 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -330,6 +330,9 @@ int tcp_sendpage_locked(struct sock *sk, struct page *page, int offset,
 			size_t size, int flags);
 ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 		 size_t size, int flags);
+int tcp_send_mss(struct sock *sk, int *size_goal, int flags);
+void tcp_push(struct sock *sk, int flags, int mss_now, int nonagle,
+	      int size_goal);
 void tcp_release_cb(struct sock *sk);
 void tcp_wfree(struct sk_buff *skb);
 void tcp_write_timer_handler(struct sock *sk);
@@ -2002,6 +2005,11 @@ struct tcp_request_sock_ops {
 			   enum tcp_synack_type synack_type);
 };
 
+extern const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops;
+#if IS_ENABLED(CONFIG_IPV6)
+extern const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops;
+#endif
+
 #ifdef CONFIG_SYN_COOKIES
 static inline __u32 cookie_init_sequence(const struct tcp_request_sock_ops *ops,
 					 const struct sock *sk, struct sk_buff *skb,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 93fe77c5b02d..dc9bd27f4222 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -690,8 +690,8 @@ static bool tcp_should_autocork(struct sock *sk, struct sk_buff *skb,
 	       refcount_read(&sk->sk_wmem_alloc) > skb->truesize;
 }
 
-static void tcp_push(struct sock *sk, int flags, int mss_now,
-		     int nonagle, int size_goal)
+void tcp_push(struct sock *sk, int flags, int mss_now,
+	      int nonagle, int size_goal)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct sk_buff *skb;
@@ -925,7 +925,7 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
 	return max(size_goal, mss_now);
 }
 
-static int tcp_send_mss(struct sock *sk, int *size_goal, int flags)
+int tcp_send_mss(struct sock *sk, int *size_goal, int flags)
 {
 	int mss_now;
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 26637fce324d..c3b91789de65 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1372,7 +1372,7 @@ struct request_sock_ops tcp_request_sock_ops __read_mostly = {
 	.syn_ack_timeout =	tcp_syn_ack_timeout,
 };
 
-static const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops = {
+const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops = {
 	.mss_clamp	=	TCP_MSS_DEFAULT,
 #ifdef CONFIG_TCP_MD5SIG
 	.req_md5_lookup	=	tcp_v4_md5_lookup,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index df5fd9109696..30dceac8a608 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -75,7 +75,7 @@ static void	tcp_v6_reqsk_send_ack(const struct sock *sk, struct sk_buff *skb,
 static int	tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb);
 
 static const struct inet_connection_sock_af_ops ipv6_mapped;
-static const struct inet_connection_sock_af_ops ipv6_specific;
+const struct inet_connection_sock_af_ops ipv6_specific;
 #ifdef CONFIG_TCP_MD5SIG
 static const struct tcp_sock_af_ops tcp_sock_ipv6_specific;
 static const struct tcp_sock_af_ops tcp_sock_ipv6_mapped_specific;
@@ -785,7 +785,7 @@ struct request_sock_ops tcp6_request_sock_ops __read_mostly = {
 	.syn_ack_timeout =	tcp_syn_ack_timeout,
 };
 
-static const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = {
+const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = {
 	.mss_clamp	=	IPV6_MIN_MTU - sizeof(struct tcphdr) -
 				sizeof(struct ipv6hdr),
 #ifdef CONFIG_TCP_MD5SIG
@@ -1739,7 +1739,7 @@ static struct timewait_sock_ops tcp6_timewait_sock_ops = {
 	.twsk_destructor = tcp_twsk_destructor,
 };
 
-static const struct inet_connection_sock_af_ops ipv6_specific = {
+const struct inet_connection_sock_af_ops ipv6_specific = {
 	.queue_xmit	   = inet6_csk_xmit,
 	.send_check	   = tcp_v6_send_check,
 	.rebuild_header	   = inet6_sk_rebuild_header,
-- 
2.24.1

[PATCH net-next v5 08/11] tcp: Export TCP functions and ops struct

[Mat Martineau]

MPTCP will make use of tcp_send_mss() and tcp_push() when sending
data to specific TCP subflows.

tcp_request_sock_ipvX_ops and ipvX_specific will be referenced
during TCP subflow creation.

Co-developed-by: Peter Krystad <peter.krystad@linux.intel.com>
Signed-off-by: Peter Krystad <peter.krystad@linux.intel.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/tcp.h   | 8 ++++++++
 net/ipv4/tcp.c      | 6 +++---
 net/ipv4/tcp_ipv4.c | 2 +-
 net/ipv6/tcp_ipv6.c | 6 +++---
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index f4cddd42d52a..0a3533c019c6 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -330,6 +330,9 @@ int tcp_sendpage_locked(struct sock *sk, struct page *page, int offset,
 			size_t size, int flags);
 ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 		 size_t size, int flags);
+int tcp_send_mss(struct sock *sk, int *size_goal, int flags);
+void tcp_push(struct sock *sk, int flags, int mss_now, int nonagle,
+	      int size_goal);
 void tcp_release_cb(struct sock *sk);
 void tcp_wfree(struct sk_buff *skb);
 void tcp_write_timer_handler(struct sock *sk);
@@ -2002,6 +2005,11 @@ struct tcp_request_sock_ops {
 			   enum tcp_synack_type synack_type);
 };
 
+extern const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops;
+#if IS_ENABLED(CONFIG_IPV6)
+extern const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops;
+#endif
+
 #ifdef CONFIG_SYN_COOKIES
 static inline __u32 cookie_init_sequence(const struct tcp_request_sock_ops *ops,
 					 const struct sock *sk, struct sk_buff *skb,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 93fe77c5b02d..dc9bd27f4222 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -690,8 +690,8 @@ static bool tcp_should_autocork(struct sock *sk, struct sk_buff *skb,
 	       refcount_read(&sk->sk_wmem_alloc) > skb->truesize;
 }
 
-static void tcp_push(struct sock *sk, int flags, int mss_now,
-		     int nonagle, int size_goal)
+void tcp_push(struct sock *sk, int flags, int mss_now,
+	      int nonagle, int size_goal)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct sk_buff *skb;
@@ -925,7 +925,7 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
 	return max(size_goal, mss_now);
 }
 
-static int tcp_send_mss(struct sock *sk, int *size_goal, int flags)
+int tcp_send_mss(struct sock *sk, int *size_goal, int flags)
 {
 	int mss_now;
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 26637fce324d..c3b91789de65 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1372,7 +1372,7 @@ struct request_sock_ops tcp_request_sock_ops __read_mostly = {
 	.syn_ack_timeout =	tcp_syn_ack_timeout,
 };
 
-static const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops = {
+const struct tcp_request_sock_ops tcp_request_sock_ipv4_ops = {
 	.mss_clamp	=	TCP_MSS_DEFAULT,
 #ifdef CONFIG_TCP_MD5SIG
 	.req_md5_lookup	=	tcp_v4_md5_lookup,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index df5fd9109696..30dceac8a608 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -75,7 +75,7 @@ static void	tcp_v6_reqsk_send_ack(const struct sock *sk, struct sk_buff *skb,
 static int	tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb);
 
 static const struct inet_connection_sock_af_ops ipv6_mapped;
-static const struct inet_connection_sock_af_ops ipv6_specific;
+const struct inet_connection_sock_af_ops ipv6_specific;
 #ifdef CONFIG_TCP_MD5SIG
 static const struct tcp_sock_af_ops tcp_sock_ipv6_specific;
 static const struct tcp_sock_af_ops tcp_sock_ipv6_mapped_specific;
@@ -785,7 +785,7 @@ struct request_sock_ops tcp6_request_sock_ops __read_mostly = {
 	.syn_ack_timeout =	tcp_syn_ack_timeout,
 };
 
-static const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = {
+const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = {
 	.mss_clamp	=	IPV6_MIN_MTU - sizeof(struct tcphdr) -
 				sizeof(struct ipv6hdr),
 #ifdef CONFIG_TCP_MD5SIG
@@ -1739,7 +1739,7 @@ static struct timewait_sock_ops tcp6_timewait_sock_ops = {
 	.twsk_destructor = tcp_twsk_destructor,
 };
 
-static const struct inet_connection_sock_af_ops ipv6_specific = {
+const struct inet_connection_sock_af_ops ipv6_specific = {
 	.queue_xmit	   = inet6_csk_xmit,
 	.send_check	   = tcp_v6_send_check,
 	.rebuild_header	   = inet6_sk_rebuild_header,
-- 
2.24.1

[MPTCP] [PATCH net-next v5 09/11] tcp: Check for filled TCP option space before SACK

[Mat Martineau]

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

Update the SACK check to work with zero option space available, a case
that's possible with MPTCP but not MD5+TS. Maintained only one
conditional branch for insufficient SACK space.

v1 -> v2:
- Moves the check inside the SACK branch by taking recent SACK fix:

    9424e2e7ad93 (tcp: md5: fix potential overestimation of TCP option space)

  in to account, but modifies it to work in MPTCP scenarios beyond the
  MD5+TS corner case.

Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 net/ipv4/tcp_output.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9e04d45bc0e4..e797ca6c6d7d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -751,13 +751,17 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
 	eff_sacks = tp->rx_opt.num_sacks + tp->rx_opt.dsack;
 	if (unlikely(eff_sacks)) {
 		const unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
+		if (unlikely(remaining < TCPOLEN_SACK_BASE_ALIGNED +
+					 TCPOLEN_SACK_PERBLOCK))
+			return size;
+
 		opts->num_sack_blocks =
 			min_t(unsigned int, eff_sacks,
 			      (remaining - TCPOLEN_SACK_BASE_ALIGNED) /
 			      TCPOLEN_SACK_PERBLOCK);
-		if (likely(opts->num_sack_blocks))
-			size += TCPOLEN_SACK_BASE_ALIGNED +
-				opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
+
+		size += TCPOLEN_SACK_BASE_ALIGNED +
+			opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
 	}
 
 	return size;
-- 
2.24.1

[PATCH net-next v5 09/11] tcp: Check for filled TCP option space before SACK

[Mat Martineau]

Update the SACK check to work with zero option space available, a case
that's possible with MPTCP but not MD5+TS. Maintained only one
conditional branch for insufficient SACK space.

v1 -> v2:
- Moves the check inside the SACK branch by taking recent SACK fix:

    9424e2e7ad93 (tcp: md5: fix potential overestimation of TCP option space)

  in to account, but modifies it to work in MPTCP scenarios beyond the
  MD5+TS corner case.

Co-developed-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 net/ipv4/tcp_output.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9e04d45bc0e4..e797ca6c6d7d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -751,13 +751,17 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
 	eff_sacks = tp->rx_opt.num_sacks + tp->rx_opt.dsack;
 	if (unlikely(eff_sacks)) {
 		const unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
+		if (unlikely(remaining < TCPOLEN_SACK_BASE_ALIGNED +
+					 TCPOLEN_SACK_PERBLOCK))
+			return size;
+
 		opts->num_sack_blocks =
 			min_t(unsigned int, eff_sacks,
 			      (remaining - TCPOLEN_SACK_BASE_ALIGNED) /
 			      TCPOLEN_SACK_PERBLOCK);
-		if (likely(opts->num_sack_blocks))
-			size += TCPOLEN_SACK_BASE_ALIGNED +
-				opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
+
+		size += TCPOLEN_SACK_BASE_ALIGNED +
+			opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
 	}
 
 	return size;
-- 
2.24.1

[MPTCP] [PATCH net-next v5 10/11] tcp: clean ext on tx recycle

[Mat Martineau]

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

From: Paolo Abeni <pabeni(a)redhat.com>

Otherwise we will find stray/unexpected/old extensions value on next
iteration.

On tcp_write_xmit() we can end-up splitting an already queued skb in two
parts, via tso_fragment(). The newly created skb can be allocated via
the tx cache and an upper layer will not be aware of it, so that upper
layer cannot set the ext properly.

Resetting the ext on recycle ensures that stale data is not propagated
in to packet headers or elsewhere.

An alternative would be add an additional hook in tso_fragment() or in
sk_stream_alloc_skb() to init the ext for upper layers that need it.

Co-developed-by: Florian Westphal <fw(a)strlen.de>
Signed-off-by: Florian Westphal <fw(a)strlen.de>
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/net/sock.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/sock.h b/include/net/sock.h
index b93cadba1a3b..23efed7f4e70 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1474,6 +1474,7 @@ static inline void sk_wmem_free_skb(struct sock *sk, struct sk_buff *skb)
 	sk_mem_uncharge(sk, skb->truesize);
 	if (static_branch_unlikely(&tcp_tx_skb_cache_key) &&
 	    !sk->sk_tx_skb_cache && !skb_cloned(skb)) {
+		skb_ext_reset(skb);
 		skb_zcopy_clear(skb, true);
 		sk->sk_tx_skb_cache = skb;
 		return;
-- 
2.24.1

[PATCH net-next v5 10/11] tcp: clean ext on tx recycle

[Mat Martineau]

From: Paolo Abeni <pabeni@redhat.com>

Otherwise we will find stray/unexpected/old extensions value on next
iteration.

On tcp_write_xmit() we can end-up splitting an already queued skb in two
parts, via tso_fragment(). The newly created skb can be allocated via
the tx cache and an upper layer will not be aware of it, so that upper
layer cannot set the ext properly.

Resetting the ext on recycle ensures that stale data is not propagated
in to packet headers or elsewhere.

An alternative would be add an additional hook in tso_fragment() or in
sk_stream_alloc_skb() to init the ext for upper layers that need it.

Co-developed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/net/sock.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/sock.h b/include/net/sock.h
index b93cadba1a3b..23efed7f4e70 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1474,6 +1474,7 @@ static inline void sk_wmem_free_skb(struct sock *sk, struct sk_buff *skb)
 	sk_mem_uncharge(sk, skb->truesize);
 	if (static_branch_unlikely(&tcp_tx_skb_cache_key) &&
 	    !sk->sk_tx_skb_cache && !skb_cloned(skb)) {
+		skb_ext_reset(skb);
 		skb_zcopy_clear(skb, true);
 		sk->sk_tx_skb_cache = skb;
 		return;
-- 
2.24.1

[MPTCP] [PATCH net-next v5 11/11] skb: add helpers to allocate ext independently from sk_buff

[Mat Martineau]

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

From: Paolo Abeni <pabeni(a)redhat.com>

Currently we can allocate the extension only after the skb,
this change allows the user to do the opposite, will simplify
allocation failure handling from MPTCP.

Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
---
 include/linux/skbuff.h |  3 +++
 net/core/skbuff.c      | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1a261c3ee074..af9b6cf79a65 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4115,6 +4115,9 @@ struct skb_ext {
 	char data[0] __aligned(8);
 };
 
+struct skb_ext *__skb_ext_alloc(void);
+void *__skb_ext_set(struct sk_buff *skb, enum skb_ext_id id,
+		    struct skb_ext *ext);
 void *skb_ext_add(struct sk_buff *skb, enum skb_ext_id id);
 void __skb_ext_del(struct sk_buff *skb, enum skb_ext_id id);
 void __skb_ext_put(struct skb_ext *ext);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index fa67036dd928..504e9bd5ebce 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5983,7 +5983,14 @@ static void *skb_ext_get_ptr(struct skb_ext *ext, enum skb_ext_id id)
 	return (void *)ext + (ext->offset[id] * SKB_EXT_ALIGN_VALUE);
 }
 
-static struct skb_ext *skb_ext_alloc(void)
+/**
+ * __skb_ext_alloc - allocate a new skb extensions storage
+ *
+ * Returns the newly allocated pointer. The pointer can later attached to a
+ * skb via __skb_ext_set().
+ * Note: caller must handle the skb_ext as an opaque data.
+ */
+struct skb_ext *__skb_ext_alloc(void)
 {
 	struct skb_ext *new = kmem_cache_alloc(skbuff_ext_cache, GFP_ATOMIC);
 
@@ -6023,6 +6030,30 @@ static struct skb_ext *skb_ext_maybe_cow(struct skb_ext *old,
 	return new;
 }
 
+/**
+ * __skb_ext_set - attach the specified extension storage to this skb
+ * @skb: buffer
+ * @id: extension id
+ * @ext: extension storage previously allocated via __skb_ext_alloc()
+ *
+ * Existing extensions, if any, are cleared.
+ *
+ * Returns the pointer to the extension.
+ */
+void *__skb_ext_set(struct sk_buff *skb, enum skb_ext_id id,
+		    struct skb_ext *ext)
+{
+	unsigned int newlen, newoff = SKB_EXT_CHUNKSIZEOF(*ext);
+
+	skb_ext_put(skb);
+	newlen = newoff + skb_ext_type_len[id];
+	ext->chunks = newlen;
+	ext->offset[id] = newoff;
+	skb->extensions = ext;
+	skb->active_extensions = 1 << id;
+	return skb_ext_get_ptr(ext, id);
+}
+
 /**
  * skb_ext_add - allocate space for given extension, COW if needed
  * @skb: buffer
@@ -6056,7 +6087,7 @@ void *skb_ext_add(struct sk_buff *skb, enum skb_ext_id id)
 	} else {
 		newoff = SKB_EXT_CHUNKSIZEOF(*new);
 
-		new = skb_ext_alloc();
+		new = __skb_ext_alloc();
 		if (!new)
 			return NULL;
 	}
-- 
2.24.1

[PATCH net-next v5 11/11] skb: add helpers to allocate ext independently from sk_buff

[Mat Martineau]

From: Paolo Abeni <pabeni@redhat.com>

Currently we can allocate the extension only after the skb,
this change allows the user to do the opposite, will simplify
allocation failure handling from MPTCP.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/linux/skbuff.h |  3 +++
 net/core/skbuff.c      | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1a261c3ee074..af9b6cf79a65 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4115,6 +4115,9 @@ struct skb_ext {
 	char data[0] __aligned(8);
 };
 
+struct skb_ext *__skb_ext_alloc(void);
+void *__skb_ext_set(struct sk_buff *skb, enum skb_ext_id id,
+		    struct skb_ext *ext);
 void *skb_ext_add(struct sk_buff *skb, enum skb_ext_id id);
 void __skb_ext_del(struct sk_buff *skb, enum skb_ext_id id);
 void __skb_ext_put(struct skb_ext *ext);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index fa67036dd928..504e9bd5ebce 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5983,7 +5983,14 @@ static void *skb_ext_get_ptr(struct skb_ext *ext, enum skb_ext_id id)
 	return (void *)ext + (ext->offset[id] * SKB_EXT_ALIGN_VALUE);
 }
 
-static struct skb_ext *skb_ext_alloc(void)
+/**
+ * __skb_ext_alloc - allocate a new skb extensions storage
+ *
+ * Returns the newly allocated pointer. The pointer can later attached to a
+ * skb via __skb_ext_set().
+ * Note: caller must handle the skb_ext as an opaque data.
+ */
+struct skb_ext *__skb_ext_alloc(void)
 {
 	struct skb_ext *new = kmem_cache_alloc(skbuff_ext_cache, GFP_ATOMIC);
 
@@ -6023,6 +6030,30 @@ static struct skb_ext *skb_ext_maybe_cow(struct skb_ext *old,
 	return new;
 }
 
+/**
+ * __skb_ext_set - attach the specified extension storage to this skb
+ * @skb: buffer
+ * @id: extension id
+ * @ext: extension storage previously allocated via __skb_ext_alloc()
+ *
+ * Existing extensions, if any, are cleared.
+ *
+ * Returns the pointer to the extension.
+ */
+void *__skb_ext_set(struct sk_buff *skb, enum skb_ext_id id,
+		    struct skb_ext *ext)
+{
+	unsigned int newlen, newoff = SKB_EXT_CHUNKSIZEOF(*ext);
+
+	skb_ext_put(skb);
+	newlen = newoff + skb_ext_type_len[id];
+	ext->chunks = newlen;
+	ext->offset[id] = newoff;
+	skb->extensions = ext;
+	skb->active_extensions = 1 << id;
+	return skb_ext_get_ptr(ext, id);
+}
+
 /**
  * skb_ext_add - allocate space for given extension, COW if needed
  * @skb: buffer
@@ -6056,7 +6087,7 @@ void *skb_ext_add(struct sk_buff *skb, enum skb_ext_id id)
 	} else {
 		newoff = SKB_EXT_CHUNKSIZEOF(*new);
 
-		new = skb_ext_alloc();
+		new = __skb_ext_alloc();
 		if (!new)
 			return NULL;
 	}
-- 
2.24.1

[MPTCP] Re: [PATCH net-next v5 07/11] tcp: coalesce/collapse must respect MPTCP extensions

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> Coalesce and collapse of packets carrying MPTCP extensions is allowed
> when the newer packet has no extension or the extensions carried by both
> packets are equal.
> 
> This allows merging of TSO packet trains and even cross-TSO packets, and
> does not require any additional action when moving data into existing
> SKBs.
> 
> v3 -> v4:
>  - allow collapsing, under mptcp_skb_can_collapse() constraint
> 
> Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---
>  include/net/mptcp.h   | 54 +++++++++++++++++++++++++++++++++++++++++++
>  include/net/tcp.h     |  8 +++++++
>  net/ipv4/tcp_input.c  | 11 ++++++---
>  net/ipv4/tcp_output.c |  2 +-
>  4 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index f9f668ac4339..8e27e33861ab 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -8,6 +8,7 @@
>  #ifndef __NET_MPTCP_H
>  #define __NET_MPTCP_H
>  
> +#include <linux/skbuff.h>
>  #include <linux/types.h>
>  
>  /* MPTCP sk_buff extension data */
> @@ -24,4 +25,57 @@ struct mptcp_ext {
>  			__unused:2;
>  };
>  
> +#ifdef CONFIG_MPTCP
> +
> +/* move the skb extension owership, with the assumption that 'to' is
> + * newly allocated
> + */
> +static inline void mptcp_skb_ext_move(struct sk_buff *to,
> +				      struct sk_buff *from)
> +{
> +	if (!skb_ext_exist(from, SKB_EXT_MPTCP))
> +		return;
> +
> +	if (WARN_ON_ONCE(to->active_extensions))
> +		skb_ext_put(to);
> +
> +	to->active_extensions = from->active_extensions;
> +	to->extensions = from->extensions;
> +	from->active_extensions = 0;
> +}
> +
> +static inline bool mptcp_ext_matches(const struct mptcp_ext *to_ext,
> +				     const struct mptcp_ext *from_ext)
> +{
> +	return !from_ext ||
> +	       (to_ext && from_ext &&
> +	        !memcmp(from_ext, to_ext, sizeof(struct mptcp_ext)));

There is a hole at the end of struct mptcp_ext

How is it properly cleared/initialized so that the memcmp() wont
spuriously fail ?

Thanks.

Re: [PATCH net-next v5 07/11] tcp: coalesce/collapse must respect MPTCP extensions

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> Coalesce and collapse of packets carrying MPTCP extensions is allowed
> when the newer packet has no extension or the extensions carried by both
> packets are equal.
> 
> This allows merging of TSO packet trains and even cross-TSO packets, and
> does not require any additional action when moving data into existing
> SKBs.
> 
> v3 -> v4:
>  - allow collapsing, under mptcp_skb_can_collapse() constraint
> 
> Co-developed-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  include/net/mptcp.h   | 54 +++++++++++++++++++++++++++++++++++++++++++
>  include/net/tcp.h     |  8 +++++++
>  net/ipv4/tcp_input.c  | 11 ++++++---
>  net/ipv4/tcp_output.c |  2 +-
>  4 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index f9f668ac4339..8e27e33861ab 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -8,6 +8,7 @@
>  #ifndef __NET_MPTCP_H
>  #define __NET_MPTCP_H
>  
> +#include <linux/skbuff.h>
>  #include <linux/types.h>
>  
>  /* MPTCP sk_buff extension data */
> @@ -24,4 +25,57 @@ struct mptcp_ext {
>  			__unused:2;
>  };
>  
> +#ifdef CONFIG_MPTCP
> +
> +/* move the skb extension owership, with the assumption that 'to' is
> + * newly allocated
> + */
> +static inline void mptcp_skb_ext_move(struct sk_buff *to,
> +				      struct sk_buff *from)
> +{
> +	if (!skb_ext_exist(from, SKB_EXT_MPTCP))
> +		return;
> +
> +	if (WARN_ON_ONCE(to->active_extensions))
> +		skb_ext_put(to);
> +
> +	to->active_extensions = from->active_extensions;
> +	to->extensions = from->extensions;
> +	from->active_extensions = 0;
> +}
> +
> +static inline bool mptcp_ext_matches(const struct mptcp_ext *to_ext,
> +				     const struct mptcp_ext *from_ext)
> +{
> +	return !from_ext ||
> +	       (to_ext && from_ext &&
> +	        !memcmp(from_ext, to_ext, sizeof(struct mptcp_ext)));

There is a hole at the end of struct mptcp_ext

How is it properly cleared/initialized so that the memcmp() wont
spuriously fail ?

Thanks.

[MPTCP] Re: [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
> copied when the listener is cloned. Sometimes the clone is immediately
> deleted, which will invoke the release op on the clone and likely
> corrupt the listening socket's icsk_ulp_data.
> 
> The clone operation is invoked immediately after the clone is copied and
> gives the ULP type an opportunity to set up the clone socket and its
> icsk_ulp_data.
> 

Since the method is void, this means no error can happen.

For example we do not intend to attempt a memory allocation ?

> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---
>  include/net/tcp.h               |  5 +++++
>  net/ipv4/inet_connection_sock.c |  2 ++
>  net/ipv4/tcp_ulp.c              | 12 ++++++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d4b6bf2c5d3c..c82b2f75d024 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2145,6 +2145,9 @@ struct tcp_ulp_ops {
>  	/* diagnostic */
>  	int (*get_info)(const struct sock *sk, struct sk_buff *skb);
>  	size_t (*get_info_size)(const struct sock *sk);
> +	/* clone ulp */
> +	void (*clone)(const struct request_sock *req, struct sock *newsk,
> +		      const gfp_t priority);
>  
>  	char		name[TCP_ULP_NAME_MAX];
>  	struct module	*owner;
> @@ -2155,6 +2158,8 @@ int tcp_set_ulp(struct sock *sk, const char *name);
>  void tcp_get_available_ulp(char *buf, size_t len);
>  void tcp_cleanup_ulp(struct sock *sk);
>  void tcp_update_ulp(struct sock *sk, struct proto *p);
> +void tcp_clone_ulp(const struct request_sock *req,
> +		   struct sock *newsk, const gfp_t priority);
>  
>  #define MODULE_ALIAS_TCP_ULP(name)				\
>  	__MODULE_INFO(alias, alias_userspace, name);		\
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index e4c6e8b40490..d667f2569f8e 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -810,6 +810,8 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
>  		/* Deinitialize accept_queue to trap illegal accesses. */
>  		memset(&newicsk->icsk_accept_queue, 0, sizeof(newicsk->icsk_accept_queue));
>  
> +		tcp_clone_ulp(req, newsk, priority);
> +
>  		security_inet_csk_clone(newsk, req);
>  	}
>  	return newsk;
> diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
> index 12ab5db2b71c..e7a2589d69ee 100644
> --- a/net/ipv4/tcp_ulp.c
> +++ b/net/ipv4/tcp_ulp.c
> @@ -130,6 +130,18 @@ void tcp_cleanup_ulp(struct sock *sk)
>  	icsk->icsk_ulp_ops = NULL;
>  }
>  
> +void tcp_clone_ulp(const struct request_sock *req, struct sock *newsk,
> +		   const gfp_t priority)
> +{
> +	struct inet_connection_sock *icsk = inet_csk(newsk);
> +
> +	if (!icsk->icsk_ulp_ops)
> +		return;
> +
> +	if (icsk->icsk_ulp_ops->clone)
> +		icsk->icsk_ulp_ops->clone(req, newsk, priority);
> +}
> +
>  static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
>  {
>  	struct inet_connection_sock *icsk = inet_csk(sk);
> 

Re: [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
> copied when the listener is cloned. Sometimes the clone is immediately
> deleted, which will invoke the release op on the clone and likely
> corrupt the listening socket's icsk_ulp_data.
> 
> The clone operation is invoked immediately after the clone is copied and
> gives the ULP type an opportunity to set up the clone socket and its
> icsk_ulp_data.
> 

Since the method is void, this means no error can happen.

For example we do not intend to attempt a memory allocation ?

> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  include/net/tcp.h               |  5 +++++
>  net/ipv4/inet_connection_sock.c |  2 ++
>  net/ipv4/tcp_ulp.c              | 12 ++++++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d4b6bf2c5d3c..c82b2f75d024 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2145,6 +2145,9 @@ struct tcp_ulp_ops {
>  	/* diagnostic */
>  	int (*get_info)(const struct sock *sk, struct sk_buff *skb);
>  	size_t (*get_info_size)(const struct sock *sk);
> +	/* clone ulp */
> +	void (*clone)(const struct request_sock *req, struct sock *newsk,
> +		      const gfp_t priority);
>  
>  	char		name[TCP_ULP_NAME_MAX];
>  	struct module	*owner;
> @@ -2155,6 +2158,8 @@ int tcp_set_ulp(struct sock *sk, const char *name);
>  void tcp_get_available_ulp(char *buf, size_t len);
>  void tcp_cleanup_ulp(struct sock *sk);
>  void tcp_update_ulp(struct sock *sk, struct proto *p);
> +void tcp_clone_ulp(const struct request_sock *req,
> +		   struct sock *newsk, const gfp_t priority);
>  
>  #define MODULE_ALIAS_TCP_ULP(name)				\
>  	__MODULE_INFO(alias, alias_userspace, name);		\
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index e4c6e8b40490..d667f2569f8e 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -810,6 +810,8 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
>  		/* Deinitialize accept_queue to trap illegal accesses. */
>  		memset(&newicsk->icsk_accept_queue, 0, sizeof(newicsk->icsk_accept_queue));
>  
> +		tcp_clone_ulp(req, newsk, priority);
> +
>  		security_inet_csk_clone(newsk, req);
>  	}
>  	return newsk;
> diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
> index 12ab5db2b71c..e7a2589d69ee 100644
> --- a/net/ipv4/tcp_ulp.c
> +++ b/net/ipv4/tcp_ulp.c
> @@ -130,6 +130,18 @@ void tcp_cleanup_ulp(struct sock *sk)
>  	icsk->icsk_ulp_ops = NULL;
>  }
>  
> +void tcp_clone_ulp(const struct request_sock *req, struct sock *newsk,
> +		   const gfp_t priority)
> +{
> +	struct inet_connection_sock *icsk = inet_csk(newsk);
> +
> +	if (!icsk->icsk_ulp_ops)
> +		return;
> +
> +	if (icsk->icsk_ulp_ops->clone)
> +		icsk->icsk_ulp_ops->clone(req, newsk, priority);
> +}
> +
>  static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
>  {
>  	struct inet_connection_sock *icsk = inet_csk(sk);
> 

[MPTCP] Re: [PATCH net-next v5 09/11] tcp: Check for filled TCP option space before SACK

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> Update the SACK check to work with zero option space available, a case
> that's possible with MPTCP but not MD5+TS. Maintained only one
> conditional branch for insufficient SACK space.
> 
> v1 -> v2:
> - Moves the check inside the SACK branch by taking recent SACK fix:
> 
>     9424e2e7ad93 (tcp: md5: fix potential overestimation of TCP option space)
> 
>   in to account, but modifies it to work in MPTCP scenarios beyond the
>   MD5+TS corner case.
> 
> Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---

Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 09/11] tcp: Check for filled TCP option space before SACK

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> Update the SACK check to work with zero option space available, a case
> that's possible with MPTCP but not MD5+TS. Maintained only one
> conditional branch for insufficient SACK space.
> 
> v1 -> v2:
> - Moves the check inside the SACK branch by taking recent SACK fix:
> 
>     9424e2e7ad93 (tcp: md5: fix potential overestimation of TCP option space)
> 
>   in to account, but modifies it to work in MPTCP scenarios beyond the
>   MD5+TS corner case.
> 
> Co-developed-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 10/11] tcp: clean ext on tx recycle

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> From: Paolo Abeni <pabeni(a)redhat.com>
> 
> Otherwise we will find stray/unexpected/old extensions value on next
> iteration.
> 
> On tcp_write_xmit() we can end-up splitting an already queued skb in two
> parts, via tso_fragment(). The newly created skb can be allocated via
> the tx cache and an upper layer will not be aware of it, so that upper
> layer cannot set the ext properly.
> 
> Resetting the ext on recycle ensures that stale data is not propagated
> in to packet headers or elsewhere.
> 
> An alternative would be add an additional hook in tso_fragment() or in
> sk_stream_alloc_skb() to init the ext for upper layers that need it.
> 
> Co-developed-by: Florian Westphal <fw(a)strlen.de>
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---

Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 10/11] tcp: clean ext on tx recycle

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> 
> Otherwise we will find stray/unexpected/old extensions value on next
> iteration.
> 
> On tcp_write_xmit() we can end-up splitting an already queued skb in two
> parts, via tso_fragment(). The newly created skb can be allocated via
> the tx cache and an upper layer will not be aware of it, so that upper
> layer cannot set the ext properly.
> 
> Resetting the ext on recycle ensures that stale data is not propagated
> in to packet headers or elsewhere.
> 
> An alternative would be add an additional hook in tso_fragment() or in
> sk_stream_alloc_skb() to init the ext for upper layers that need it.
> 
> Co-developed-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 08/11] tcp: Export TCP functions and ops struct

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> MPTCP will make use of tcp_send_mss() and tcp_push() when sending
> data to specific TCP subflows.
> 
> tcp_request_sock_ipvX_ops and ipvX_specific will be referenced
> during TCP subflow creation.
> 
> Co-developed-by: Peter Krystad <peter.krystad(a)linux.intel.com>
> Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---

Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 08/11] tcp: Export TCP functions and ops struct

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> MPTCP will make use of tcp_send_mss() and tcp_push() when sending
> data to specific TCP subflows.
> 
> tcp_request_sock_ipvX_ops and ipvX_specific will be referenced
> during TCP subflow creation.
> 
> Co-developed-by: Peter Krystad <peter.krystad@linux.intel.com>
> Signed-off-by: Peter Krystad <peter.krystad@linux.intel.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 06/11] mptcp: Add MPTCP to skb extensions

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> Add enum value for MPTCP and update config dependencies
> 
> Co-developed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>

...

> +/* MPTCP sk_buff extension data */
> +struct mptcp_ext {
> +	u64		data_ack;
> +	u64		data_seq;
> +	u32		subflow_seq;
> +	u16		data_len;
> +	u8		use_map:1,
> +			dsn64:1,
> +			data_fin:1,
> +			use_ack:1,
> +			ack64:1,
> +			__unused:2;
	__unused:3; 

Also worth noting there is one byte hole in this structure.

> +};
> +

Re: [PATCH net-next v5 06/11] mptcp: Add MPTCP to skb extensions

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> Add enum value for MPTCP and update config dependencies
> 
> Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

...

> +/* MPTCP sk_buff extension data */
> +struct mptcp_ext {
> +	u64		data_ack;
> +	u64		data_seq;
> +	u32		subflow_seq;
> +	u16		data_len;
> +	u8		use_map:1,
> +			dsn64:1,
> +			data_fin:1,
> +			use_ack:1,
> +			ack64:1,
> +			__unused:2;
	__unused:3; 

Also worth noting there is one byte hole in this structure.

> +};
> +

[MPTCP] Re: [PATCH net-next v5 01/11] net: Make sock protocol value checks more specific

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> SK_PROTOCOL_MAX is only used in two places, for DECNet and AX.25. The
> limits have more to do with the those protocol definitions than they do
> with the data type of sk_protocol, so remove SK_PROTOCOL_MAX and use
> U8_MAX directly.
> 
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>

Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 01/11] net: Make sock protocol value checks more specific

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> SK_PROTOCOL_MAX is only used in two places, for DECNet and AX.25. The
> limits have more to do with the those protocol definitions than they do
> with the data type of sk_protocol, so remove SK_PROTOCOL_MAX and use
> U8_MAX directly.
> 
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 02/11] sock: Make sk_protocol a 16-bit value

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> Match the 16-bit width of skbuff->protocol. Fills an 8-bit hole so
> sizeof(struct sock) does not change.
> 
> v2 -> v3:
>  - keep 'sk_type' 2 bytes aligned (Eric)
> v1 -> v2:
>  - preserve sk_pacing_shift as bit field (Eric)
> 
> Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> Co-developed-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> Signed-off-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---


Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 02/11] sock: Make sk_protocol a 16-bit value

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> Match the 16-bit width of skbuff->protocol. Fills an 8-bit hole so
> sizeof(struct sock) does not change.
> 
> v2 -> v3:
>  - keep 'sk_type' 2 bytes aligned (Eric)
> v1 -> v2:
>  - preserve sk_pacing_shift as bit field (Eric)
> 
> Co-developed-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---


Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 03/11] tcp: Define IPPROTO_MPTCP

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> To open a MPTCP socket with socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP),
> IPPROTO_MPTCP needs a value that differs from IPPROTO_TCP. The existing
> IPPROTO numbers mostly map directly to IANA-specified protocol numbers.
> MPTCP does not have a protocol number allocated because MPTCP packets
> use the TCP protocol number. Use private number not used OTA.
> 
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>

Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 03/11] tcp: Define IPPROTO_MPTCP

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> To open a MPTCP socket with socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP),
> IPPROTO_MPTCP needs a value that differs from IPPROTO_TCP. The existing
> IPPROTO numbers mostly map directly to IANA-specified protocol numbers.
> MPTCP does not have a protocol number allocated because MPTCP packets
> use the TCP protocol number. Use private number not used OTA.
> 
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 04/11] tcp: Add MPTCP option number

[Eric Dumazet]

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



On 12/19/19 2:34 PM, Mat Martineau wrote:
> TCP option 30 is allocated for MPTCP by the IANA.
> 
> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> ---
>  include/net/tcp.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 86b9a8766648..d4b6bf2c5d3c 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -182,6 +182,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
>  #define TCPOPT_SACK             5       /* SACK Block */
>  #define TCPOPT_TIMESTAMP	8	/* Better RTT estimations/PAWS */
>  #define TCPOPT_MD5SIG		19	/* MD5 Signature (RFC2385) */
> +#define TCPOPT_MPTCP		30	/* Multipath TCP (RFC6824) */
>  #define TCPOPT_FASTOPEN		34	/* Fast open (RFC7413) */
>  #define TCPOPT_EXP		254	/* Experimental */
>  /* Magic number to be after the option value for sharing TCP
> 

Reviewed-by: Eric Dumazet <edumazet(a)google.com>

Re: [PATCH net-next v5 04/11] tcp: Add MPTCP option number

[Eric Dumazet]

On 12/19/19 2:34 PM, Mat Martineau wrote:
> TCP option 30 is allocated for MPTCP by the IANA.
> 
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  include/net/tcp.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 86b9a8766648..d4b6bf2c5d3c 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -182,6 +182,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
>  #define TCPOPT_SACK             5       /* SACK Block */
>  #define TCPOPT_TIMESTAMP	8	/* Better RTT estimations/PAWS */
>  #define TCPOPT_MD5SIG		19	/* MD5 Signature (RFC2385) */
> +#define TCPOPT_MPTCP		30	/* Multipath TCP (RFC6824) */
>  #define TCPOPT_FASTOPEN		34	/* Fast open (RFC7413) */
>  #define TCPOPT_EXP		254	/* Experimental */
>  /* Magic number to be after the option value for sharing TCP
> 

Reviewed-by: Eric Dumazet <edumazet@google.com>

[MPTCP] Re: [PATCH net-next v5 07/11] tcp: coalesce/collapse must respect MPTCP extensions

[Mat Martineau]

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

On Fri, 20 Dec 2019, Eric Dumazet wrote:

>
>
> On 12/19/19 2:34 PM, Mat Martineau wrote:
>> Coalesce and collapse of packets carrying MPTCP extensions is allowed
>> when the newer packet has no extension or the extensions carried by both
>> packets are equal.
>>
>> This allows merging of TSO packet trains and even cross-TSO packets, and
>> does not require any additional action when moving data into existing
>> SKBs.
>>
>> v3 -> v4:
>>  - allow collapsing, under mptcp_skb_can_collapse() constraint
>>
>> Co-developed-by: Paolo Abeni <pabeni(a)redhat.com>
>> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
>> Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
>> ---
>>  include/net/mptcp.h   | 54 +++++++++++++++++++++++++++++++++++++++++++
>>  include/net/tcp.h     |  8 +++++++
>>  net/ipv4/tcp_input.c  | 11 ++++++---
>>  net/ipv4/tcp_output.c |  2 +-
>>  4 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>> index f9f668ac4339..8e27e33861ab 100644
>> --- a/include/net/mptcp.h
>> +++ b/include/net/mptcp.h
>> @@ -8,6 +8,7 @@
>>  #ifndef __NET_MPTCP_H
>>  #define __NET_MPTCP_H
>>
>> +#include <linux/skbuff.h>
>>  #include <linux/types.h>
>>
>>  /* MPTCP sk_buff extension data */
>> @@ -24,4 +25,57 @@ struct mptcp_ext {
>>  			__unused:2;
>>  };
>>
>> +#ifdef CONFIG_MPTCP
>> +
>> +/* move the skb extension owership, with the assumption that 'to' is
>> + * newly allocated
>> + */
>> +static inline void mptcp_skb_ext_move(struct sk_buff *to,
>> +				      struct sk_buff *from)
>> +{
>> +	if (!skb_ext_exist(from, SKB_EXT_MPTCP))
>> +		return;
>> +
>> +	if (WARN_ON_ONCE(to->active_extensions))
>> +		skb_ext_put(to);
>> +
>> +	to->active_extensions = from->active_extensions;
>> +	to->extensions = from->extensions;
>> +	from->active_extensions = 0;
>> +}
>> +
>> +static inline bool mptcp_ext_matches(const struct mptcp_ext *to_ext,
>> +				     const struct mptcp_ext *from_ext)
>> +{
>> +	return !from_ext ||
>> +	       (to_ext && from_ext &&
>> +	        !memcmp(from_ext, to_ext, sizeof(struct mptcp_ext)));
>
> There is a hole at the end of struct mptcp_ext
>
> How is it properly cleared/initialized so that the memcmp() wont
> spuriously fail ?
>

Hi Eric,

Yes, that's important - we have code in part 2 that initializes the full 
sizeof(struct mptcp_ext) for exactly this reason:

         mpext = skb_ext_add(skb, SKB_EXT_MPTCP);
         if (!mpext)
                 return;

         memset(mpext, 0, sizeof(*mpext));

(reference: 
https://github.com/multipath-tcp/mptcp_net-next/commit/0796b4a779d0c2a87e552fdec801cb2596c23a1f 
)

Thank you very much for your review!

--
Mat Martineau
Intel

Re: [PATCH net-next v5 07/11] tcp: coalesce/collapse must respect MPTCP extensions

[Mat Martineau]

On Fri, 20 Dec 2019, Eric Dumazet wrote:

>
>
> On 12/19/19 2:34 PM, Mat Martineau wrote:
>> Coalesce and collapse of packets carrying MPTCP extensions is allowed
>> when the newer packet has no extension or the extensions carried by both
>> packets are equal.
>>
>> This allows merging of TSO packet trains and even cross-TSO packets, and
>> does not require any additional action when moving data into existing
>> SKBs.
>>
>> v3 -> v4:
>>  - allow collapsing, under mptcp_skb_can_collapse() constraint
>>
>> Co-developed-by: Paolo Abeni <pabeni@redhat.com>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>> ---
>>  include/net/mptcp.h   | 54 +++++++++++++++++++++++++++++++++++++++++++
>>  include/net/tcp.h     |  8 +++++++
>>  net/ipv4/tcp_input.c  | 11 ++++++---
>>  net/ipv4/tcp_output.c |  2 +-
>>  4 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>> index f9f668ac4339..8e27e33861ab 100644
>> --- a/include/net/mptcp.h
>> +++ b/include/net/mptcp.h
>> @@ -8,6 +8,7 @@
>>  #ifndef __NET_MPTCP_H
>>  #define __NET_MPTCP_H
>>
>> +#include <linux/skbuff.h>
>>  #include <linux/types.h>
>>
>>  /* MPTCP sk_buff extension data */
>> @@ -24,4 +25,57 @@ struct mptcp_ext {
>>  			__unused:2;
>>  };
>>
>> +#ifdef CONFIG_MPTCP
>> +
>> +/* move the skb extension owership, with the assumption that 'to' is
>> + * newly allocated
>> + */
>> +static inline void mptcp_skb_ext_move(struct sk_buff *to,
>> +				      struct sk_buff *from)
>> +{
>> +	if (!skb_ext_exist(from, SKB_EXT_MPTCP))
>> +		return;
>> +
>> +	if (WARN_ON_ONCE(to->active_extensions))
>> +		skb_ext_put(to);
>> +
>> +	to->active_extensions = from->active_extensions;
>> +	to->extensions = from->extensions;
>> +	from->active_extensions = 0;
>> +}
>> +
>> +static inline bool mptcp_ext_matches(const struct mptcp_ext *to_ext,
>> +				     const struct mptcp_ext *from_ext)
>> +{
>> +	return !from_ext ||
>> +	       (to_ext && from_ext &&
>> +	        !memcmp(from_ext, to_ext, sizeof(struct mptcp_ext)));
>
> There is a hole at the end of struct mptcp_ext
>
> How is it properly cleared/initialized so that the memcmp() wont
> spuriously fail ?
>

Hi Eric,

Yes, that's important - we have code in part 2 that initializes the full 
sizeof(struct mptcp_ext) for exactly this reason:

         mpext = skb_ext_add(skb, SKB_EXT_MPTCP);
         if (!mpext)
                 return;

         memset(mpext, 0, sizeof(*mpext));

(reference: 
https://github.com/multipath-tcp/mptcp_net-next/commit/0796b4a779d0c2a87e552fdec801cb2596c23a1f 
)

Thank you very much for your review!

--
Mat Martineau
Intel

[MPTCP] Re: [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Paolo Abeni]

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

On Fri, 2019-12-20 at 07:26 -0800, Eric Dumazet wrote:
> 
> On 12/19/19 2:34 PM, Mat Martineau wrote:
> > If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
> > copied when the listener is cloned. Sometimes the clone is immediately
> > deleted, which will invoke the release op on the clone and likely
> > corrupt the listening socket's icsk_ulp_data.
> > 
> > The clone operation is invoked immediately after the clone is copied and
> > gives the ULP type an opportunity to set up the clone socket and its
> > icsk_ulp_data.
> > 
> 
> Since the method is void, this means no error can happen.
> 
> For example we do not intend to attempt a memory allocation ?

if the MPTCP ULP clone fails, we fallback to plain TCP (the 'is_mptcp'
flag cleared on the tcp 'struct sock'), so we don't have an error
return code there.

If we change the 'clone' signature, than the only in-kernel user will
always return 0, would that be ok?

Thank you!

Paolo

Re: [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Paolo Abeni]

On Fri, 2019-12-20 at 07:26 -0800, Eric Dumazet wrote:
> 
> On 12/19/19 2:34 PM, Mat Martineau wrote:
> > If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
> > copied when the listener is cloned. Sometimes the clone is immediately
> > deleted, which will invoke the release op on the clone and likely
> > corrupt the listening socket's icsk_ulp_data.
> > 
> > The clone operation is invoked immediately after the clone is copied and
> > gives the ULP type an opportunity to set up the clone socket and its
> > icsk_ulp_data.
> > 
> 
> Since the method is void, this means no error can happen.
> 
> For example we do not intend to attempt a memory allocation ?

if the MPTCP ULP clone fails, we fallback to plain TCP (the 'is_mptcp'
flag cleared on the tcp 'struct sock'), so we don't have an error
return code there.

If we change the 'clone' signature, than the only in-kernel user will
always return 0, would that be ok?

Thank you!

Paolo

[MPTCP] Re: [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Eric Dumazet]

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



On 12/20/19 8:21 AM, Paolo Abeni wrote:
> On Fri, 2019-12-20 at 07:26 -0800, Eric Dumazet wrote:
>>
>> On 12/19/19 2:34 PM, Mat Martineau wrote:
>>> If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
>>> copied when the listener is cloned. Sometimes the clone is immediately
>>> deleted, which will invoke the release op on the clone and likely
>>> corrupt the listening socket's icsk_ulp_data.
>>>
>>> The clone operation is invoked immediately after the clone is copied and
>>> gives the ULP type an opportunity to set up the clone socket and its
>>> icsk_ulp_data.
>>>
>>
>> Since the method is void, this means no error can happen.
>>
>> For example we do not intend to attempt a memory allocation ?
> 
> if the MPTCP ULP clone fails, we fallback to plain TCP (the 'is_mptcp'
> flag cleared on the tcp 'struct sock'), so we don't have an error
> return code there.
> 
> If we change the 'clone' signature, than the only in-kernel user will
> always return 0, would that be ok?

I guess you could simply add more comments in the changelog for the time being.

I was trying to make sure we would not have later another patch changing
the signature.

Re: [PATCH net-next v5 05/11] tcp, ulp: Add clone operation to tcp_ulp_ops

[Eric Dumazet]

On 12/20/19 8:21 AM, Paolo Abeni wrote:
> On Fri, 2019-12-20 at 07:26 -0800, Eric Dumazet wrote:
>>
>> On 12/19/19 2:34 PM, Mat Martineau wrote:
>>> If ULP is used on a listening socket, icsk_ulp_ops and icsk_ulp_data are
>>> copied when the listener is cloned. Sometimes the clone is immediately
>>> deleted, which will invoke the release op on the clone and likely
>>> corrupt the listening socket's icsk_ulp_data.
>>>
>>> The clone operation is invoked immediately after the clone is copied and
>>> gives the ULP type an opportunity to set up the clone socket and its
>>> icsk_ulp_data.
>>>
>>
>> Since the method is void, this means no error can happen.
>>
>> For example we do not intend to attempt a memory allocation ?
> 
> if the MPTCP ULP clone fails, we fallback to plain TCP (the 'is_mptcp'
> flag cleared on the tcp 'struct sock'), so we don't have an error
> return code there.
> 
> If we change the 'clone' signature, than the only in-kernel user will
> always return 0, would that be ok?

I guess you could simply add more comments in the changelog for the time being.

I was trying to make sure we would not have later another patch changing
the signature.