[PATCHv2 net-next 00/10] net: support ipv4 big tcp

[PATCHv2 net-next 00/10] net: support ipv4 big tcp

[Xin Long]

This is similar to the BIG TCP patchset added by Eric for IPv6:

  https://lwn.net/Articles/895398/

Different from IPv6, IPv4 tot_len is 16-bit long only, and IPv4 header
doesn't have exthdrs(options) for the BIG TCP packets' length. To make
it simple, as David and Paolo suggested, we set IPv4 tot_len to 0 to
indicate this might be a BIG TCP packet and use skb->len as the real
IPv4 total length.

This will work safely, as all BIG TCP packets are GSO/GRO packets and
processed on the same host as they were created; There is no padding
in GSO/GRO packets, and skb->len - network_offset is exactly the IPv4
packet total length; Also, before implementing the feature, all those
places that may get iph tot_len from BIG TCP packets are taken care
with some new APIs:

Patch 1 adds some APIs for iph tot_len setting and getting, which are
used in all these places where IPv4 BIG TCP packets may reach in Patch
2-8, and Patch 9 implements this feature and Patch 10 adds a selftest
for it.

Note that the similar change as in Patch 2-6 are also needed for IPv6
BIG TCP packets, and will be addressed in another patchset.

The similar performance test is done for IPv4 BIG TCP with 25Gbit NIC
and 1.5K MTU:

No BIG TCP:
for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
168          322          337          3776.49
143          236          277          4654.67
128          258          288          4772.83
171          229          278          4645.77
175          228          243          4678.93
149          239          279          4599.86
164          234          268          4606.94
155          276          289          4235.82
180          255          268          4418.95
168          241          249          4417.82

Enable BIG TCP:
ip link set dev ens1f0np0 gro_max_size 128000 gso_max_size 128000
for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
161          241          252          4821.73
174          205          217          5098.28
167          208          220          5001.43
164          228          249          4883.98
150          233          249          4914.90
180          233          244          4819.66
154          208          219          5004.92
157          209          247          4999.78
160          218          246          4842.31
174          206          217          5080.99

Thanks for the feedback from Eric and David Ahern.

v1->v2:
  - add GSO_TCP for tp_status in packet sockets to tell the af_packet
    users that this is a TCP GSO packet in Patch 8.
  - remove the fixes and the selftest for IPv6 BIG TCP, will do it in
    another patchset.
  - also check skb_is_gso() when checking if it's a GSO TCP packet in
    Patch 1.

Xin Long (10):
  net: add a couple of helpers for iph tot_len
  bridge: use skb_ip_totlen in br netfilter
  openvswitch: use skb_ip_totlen in conntrack
  net: sched: use skb_ip_totlen and iph_totlen
  netfilter: use skb_ip_totlen and iph_totlen
  cipso_ipv4: use iph_set_totlen in skbuff_setattr
  ipvlan: use skb_ip_totlen in ipvlan_get_L3_hdr
  packet: add TP_STATUS_GSO_TCP for tp_status
  net: add support for ipv4 big tcp
  selftests: add a selftest for ipv4 big tcp

 drivers/net/ipvlan/ipvlan_core.c           |   2 +-
 include/linux/ip.h                         |  21 ++++
 include/net/netfilter/nf_tables_ipv4.h     |   4 +-
 include/net/route.h                        |   3 -
 include/uapi/linux/if_packet.h             |   1 +
 net/bridge/br_netfilter_hooks.c            |   2 +-
 net/bridge/netfilter/nf_conntrack_bridge.c |   4 +-
 net/core/gro.c                             |   6 +-
 net/core/sock.c                            |  11 +-
 net/ipv4/af_inet.c                         |   7 +-
 net/ipv4/cipso_ipv4.c                      |   2 +-
 net/ipv4/ip_input.c                        |   2 +-
 net/ipv4/ip_output.c                       |   2 +-
 net/netfilter/ipvs/ip_vs_xmit.c            |   2 +-
 net/netfilter/nf_log_syslog.c              |   2 +-
 net/netfilter/xt_length.c                  |   2 +-
 net/openvswitch/conntrack.c                |   2 +-
 net/packet/af_packet.c                     |   4 +
 net/sched/act_ct.c                         |   2 +-
 net/sched/sch_cake.c                       |   2 +-
 tools/testing/selftests/net/Makefile       |   1 +
 tools/testing/selftests/net/big_tcp.sh     | 140 +++++++++++++++++++++
 22 files changed, 191 insertions(+), 33 deletions(-)
 create mode 100644 tools/testing/selftests/net/big_tcp.sh

-- 
2.31.1

[PATCHv2 net-next 01/10] net: add a couple of helpers for iph tot_len

[Xin Long]

This patch adds three APIs to replace the iph->tot_len setting
and getting in all places where IPv4 BIG TCP packets may reach,
they will be used in the following patches.

Note that iph_totlen() will be used when iph is not in linear
data of the skb.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/linux/ip.h  | 21 +++++++++++++++++++++
 include/net/route.h |  3 ---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/include/linux/ip.h b/include/linux/ip.h
index 3d9c6750af62..d11c25f5030a 100644
--- a/include/linux/ip.h
+++ b/include/linux/ip.h
@@ -35,4 +35,25 @@ static inline unsigned int ip_transport_len(const struct sk_buff *skb)
 {
 	return ntohs(ip_hdr(skb)->tot_len) - skb_network_header_len(skb);
 }
+
+static inline unsigned int iph_totlen(const struct sk_buff *skb, const struct iphdr *iph)
+{
+	u32 len = ntohs(iph->tot_len);
+
+	return (len || !skb_is_gso(skb) || !skb_is_gso_tcp(skb)) ?
+	       len : skb->len - skb_network_offset(skb);
+}
+
+static inline unsigned int skb_ip_totlen(const struct sk_buff *skb)
+{
+	return iph_totlen(skb, ip_hdr(skb));
+}
+
+/* IPv4 datagram length is stored into 16bit field (tot_len) */
+#define IP_MAX_MTU	0xFFFFU
+
+static inline void iph_set_totlen(struct iphdr *iph, unsigned int len)
+{
+	iph->tot_len = len <= IP_MAX_MTU ? htons(len) : 0;
+}
 #endif	/* _LINUX_IP_H */
diff --git a/include/net/route.h b/include/net/route.h
index 6e92dd5bcd61..fe00b0a2e475 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -35,9 +35,6 @@
 #include <linux/cache.h>
 #include <linux/security.h>
 
-/* IPv4 datagram length is stored into 16bit field (tot_len) */
-#define IP_MAX_MTU	0xFFFFU
-
 #define RTO_ONLINK	0x01
 
 #define RT_CONN_FLAGS(sk)   (RT_TOS(inet_sk(sk)->tos) | sock_flag(sk, SOCK_LOCALROUTE))
-- 
2.31.1

[PATCHv2 net-next 02/10] bridge: use skb_ip_totlen in br netfilter

[Xin Long]

These 3 places in bridge netfilter are called on RX path after GRO
and IPv4 TCP GSO packets may come through, so replace iph tot_len
accessing with skb_ip_totlen() in there.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/bridge/br_netfilter_hooks.c            | 2 +-
 net/bridge/netfilter/nf_conntrack_bridge.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index f20f4373ff40..b67c9c98effa 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -214,7 +214,7 @@ static int br_validate_ipv4(struct net *net, struct sk_buff *skb)
 	if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
 		goto csum_error;
 
-	len = ntohs(iph->tot_len);
+	len = skb_ip_totlen(skb);
 	if (skb->len < len) {
 		__IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
 		goto drop;
diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index 5c5dd437f1c2..71056ee84773 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -212,7 +212,7 @@ static int nf_ct_br_ip_check(const struct sk_buff *skb)
 	    iph->version != 4)
 		return -1;
 
-	len = ntohs(iph->tot_len);
+	len = skb_ip_totlen(skb);
 	if (skb->len < nhoff + len ||
 	    len < (iph->ihl * 4))
                 return -1;
@@ -256,7 +256,7 @@ static unsigned int nf_ct_bridge_pre(void *priv, struct sk_buff *skb,
 		if (!pskb_may_pull(skb, sizeof(struct iphdr)))
 			return NF_ACCEPT;
 
-		len = ntohs(ip_hdr(skb)->tot_len);
+		len = skb_ip_totlen(skb);
 		if (pskb_trim_rcsum(skb, len))
 			return NF_ACCEPT;
 
-- 
2.31.1

[PATCHv2 net-next 03/10] openvswitch: use skb_ip_totlen in conntrack

[Xin Long]

IPv4 GSO packets may get processed in ovs_skb_network_trim(),
and we need to use skb_ip_totlen() to get iph totlen.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/openvswitch/conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index c8b137649ca4..2172930b1f17 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1103,7 +1103,7 @@ static int ovs_skb_network_trim(struct sk_buff *skb)
 
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
-		len = ntohs(ip_hdr(skb)->tot_len);
+		len = skb_ip_totlen(skb);
 		break;
 	case htons(ETH_P_IPV6):
 		len = sizeof(struct ipv6hdr)
-- 
2.31.1

[PATCHv2 net-next 04/10] net: sched: use skb_ip_totlen and iph_totlen

[Xin Long]

There are 1 action and 1 qdisc that may process IPv4 TCP GSO packets
and access iph->tot_len, replace them with skb_ip_totlen() and
iph_totlen() accordingly.

Note that we don't need to replace the one in tcf_csum_ipv4(), as it
will return for TCP GSO packets in tcf_csum_ipv4_tcp().

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sched/act_ct.c   | 2 +-
 net/sched/sch_cake.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 0ca2bb8ed026..d68bb5dbf0dc 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -707,7 +707,7 @@ static int tcf_ct_skb_network_trim(struct sk_buff *skb, int family)
 
 	switch (family) {
 	case NFPROTO_IPV4:
-		len = ntohs(ip_hdr(skb)->tot_len);
+		len = skb_ip_totlen(skb);
 		break;
 	case NFPROTO_IPV6:
 		len = sizeof(struct ipv6hdr)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 3ed0c3342189..7970217b565a 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1209,7 +1209,7 @@ static struct sk_buff *cake_ack_filter(struct cake_sched_data *q,
 			    iph_check->daddr != iph->daddr)
 				continue;
 
-			seglen = ntohs(iph_check->tot_len) -
+			seglen = iph_totlen(skb, iph_check) -
 				       (4 * iph_check->ihl);
 		} else if (iph_check->version == 6) {
 			ipv6h = (struct ipv6hdr *)iph;
-- 
2.31.1

[PATCHv2 net-next 05/10] netfilter: use skb_ip_totlen and iph_totlen

[Xin Long]

There are also quite some places in netfilter that may process IPv4 TCP
GSO packets, we need to replace them too.

In length_mt(), we have to use u_int32_t/int to accept skb_ip_totlen()
return value, otherwise it may overflow and mismatch. This change will
also help us add selftest for IPv4 BIG TCP in the following patch.

Note that we don't need to replace the one in tcpmss_tg4(), as it will
return if there is data after tcphdr in tcpmss_mangle_packet(). The
same in mangle_contents() in nf_nat_helper.c, it returns false when
skb->len + extra > 65535 in enlarge_skb().

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/netfilter/nf_tables_ipv4.h | 4 ++--
 net/netfilter/ipvs/ip_vs_xmit.c        | 2 +-
 net/netfilter/nf_log_syslog.c          | 2 +-
 net/netfilter/xt_length.c              | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/netfilter/nf_tables_ipv4.h b/include/net/netfilter/nf_tables_ipv4.h
index 112708f7a6b4..947973623dc7 100644
--- a/include/net/netfilter/nf_tables_ipv4.h
+++ b/include/net/netfilter/nf_tables_ipv4.h
@@ -29,7 +29,7 @@ static inline int __nft_set_pktinfo_ipv4_validate(struct nft_pktinfo *pkt)
 	if (iph->ihl < 5 || iph->version != 4)
 		return -1;
 
-	len = ntohs(iph->tot_len);
+	len = iph_totlen(pkt->skb, iph);
 	thoff = iph->ihl * 4;
 	if (pkt->skb->len < len)
 		return -1;
@@ -64,7 +64,7 @@ static inline int nft_set_pktinfo_ipv4_ingress(struct nft_pktinfo *pkt)
 	if (iph->ihl < 5 || iph->version != 4)
 		goto inhdr_error;
 
-	len = ntohs(iph->tot_len);
+	len = iph_totlen(pkt->skb, iph);
 	thoff = iph->ihl * 4;
 	if (pkt->skb->len < len) {
 		__IP_INC_STATS(nft_net(pkt), IPSTATS_MIB_INTRUNCATEDPKTS);
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 029171379884..80448885c3d7 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -994,7 +994,7 @@ ip_vs_prepare_tunneled_skb(struct sk_buff *skb, int skb_af,
 		old_dsfield = ipv4_get_dsfield(old_iph);
 		*ttl = old_iph->ttl;
 		if (payload_len)
-			*payload_len = ntohs(old_iph->tot_len);
+			*payload_len = skb_ip_totlen(skb);
 	}
 
 	/* Implement full-functionality option for ECN encapsulation */
diff --git a/net/netfilter/nf_log_syslog.c b/net/netfilter/nf_log_syslog.c
index cb894f0d63e9..c66689ad2b49 100644
--- a/net/netfilter/nf_log_syslog.c
+++ b/net/netfilter/nf_log_syslog.c
@@ -322,7 +322,7 @@ dump_ipv4_packet(struct net *net, struct nf_log_buf *m,
 
 	/* Max length: 46 "LEN=65535 TOS=0xFF PREC=0xFF TTL=255 ID=65535 " */
 	nf_log_buf_add(m, "LEN=%u TOS=0x%02X PREC=0x%02X TTL=%u ID=%u ",
-		       ntohs(ih->tot_len), ih->tos & IPTOS_TOS_MASK,
+		       iph_totlen(skb, ih), ih->tos & IPTOS_TOS_MASK,
 		       ih->tos & IPTOS_PREC_MASK, ih->ttl, ntohs(ih->id));
 
 	/* Max length: 6 "CE DF MF " */
diff --git a/net/netfilter/xt_length.c b/net/netfilter/xt_length.c
index 1873da3a945a..b3d623a52885 100644
--- a/net/netfilter/xt_length.c
+++ b/net/netfilter/xt_length.c
@@ -21,7 +21,7 @@ static bool
 length_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
 	const struct xt_length_info *info = par->matchinfo;
-	u_int16_t pktlen = ntohs(ip_hdr(skb)->tot_len);
+	u32 pktlen = skb_ip_totlen(skb);
 
 	return (pktlen >= info->min && pktlen <= info->max) ^ info->invert;
 }
-- 
2.31.1

[PATCHv2 net-next 06/10] cipso_ipv4: use iph_set_totlen in skbuff_setattr

[Xin Long]

It may process IPv4 TCP GSO packets in cipso_v4_skbuff_setattr(), so
the iph->tot_len update should use iph_set_totlen().

Note that for these non GSO packets, the new iph tot_len with extra
iph option len added may become greater than 65535, the old process
will cast it and set iph->tot_len to it, which is a bug. In theory,
iph options shouldn't be added for these big packets in here, a fix
may be needed here in the future. For now this patch is only to set
iph->tot_len to 0 when it happens.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/ipv4/cipso_ipv4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 6cd3b6c559f0..79ae7204e8ed 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -2222,7 +2222,7 @@ int cipso_v4_skbuff_setattr(struct sk_buff *skb,
 		memset((char *)(iph + 1) + buf_len, 0, opt_len - buf_len);
 	if (len_delta != 0) {
 		iph->ihl = 5 + (opt_len >> 2);
-		iph->tot_len = htons(skb->len);
+		iph_set_totlen(iph, skb->len);
 	}
 	ip_send_check(iph);
 
-- 
2.31.1

[PATCHv2 net-next 07/10] ipvlan: use skb_ip_totlen in ipvlan_get_L3_hdr

[Xin Long]

ipvlan devices calls netif_inherit_tso_max() to get the tso_max_size/segs
from the lower device, so when lower device supports BIG TCP, the ipvlan
devices support it too. We also should consider its iph tot_len accessing.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 drivers/net/ipvlan/ipvlan_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index bb1c298c1e78..460b3d4f2245 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -157,7 +157,7 @@ void *ipvlan_get_L3_hdr(struct ipvl_port *port, struct sk_buff *skb, int *type)
 			return NULL;
 
 		ip4h = ip_hdr(skb);
-		pktlen = ntohs(ip4h->tot_len);
+		pktlen = skb_ip_totlen(skb);
 		if (ip4h->ihl < 5 || ip4h->version != 4)
 			return NULL;
 		if (skb->len < pktlen || pktlen < (ip4h->ihl * 4))
-- 
2.31.1

[PATCHv2 net-next 08/10] packet: add TP_STATUS_GSO_TCP for tp_status

[Xin Long]

Introduce TP_STATUS_GSO_TCP tp_status flag to tell the af_packet user
that this is a TCP GSO packet. When parsing IPv4 BIG TCP packets in
tcpdump/libpcap, it can use tp_len as the IPv4 packet len when this
flag is set, as iph tot_len is set to 0 for IPv4 BIG TCP packets.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/if_packet.h | 1 +
 net/packet/af_packet.c         | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h
index a8516b3594a4..78c981d6a9d4 100644
--- a/include/uapi/linux/if_packet.h
+++ b/include/uapi/linux/if_packet.h
@@ -115,6 +115,7 @@ struct tpacket_auxdata {
 #define TP_STATUS_BLK_TMO		(1 << 5)
 #define TP_STATUS_VLAN_TPID_VALID	(1 << 6) /* auxdata has valid tp_vlan_tpid */
 #define TP_STATUS_CSUM_VALID		(1 << 7)
+#define TP_STATUS_GSO_TCP		(1 << 8)
 
 /* Tx ring - header status */
 #define TP_STATUS_AVAILABLE	      0
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index b5ab98ca2511..8ffb19c643ab 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2296,6 +2296,8 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	else if (skb->pkt_type != PACKET_OUTGOING &&
 		 skb_csum_unnecessary(skb))
 		status |= TP_STATUS_CSUM_VALID;
+	if (skb_is_gso(skb) && skb_is_gso_tcp(skb))
+		status |= TP_STATUS_GSO_TCP;
 
 	if (snaplen > res)
 		snaplen = res;
@@ -3522,6 +3524,8 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		else if (skb->pkt_type != PACKET_OUTGOING &&
 			 skb_csum_unnecessary(skb))
 			aux.tp_status |= TP_STATUS_CSUM_VALID;
+		if (skb_is_gso(skb) && skb_is_gso_tcp(skb))
+			aux.tp_status |= TP_STATUS_GSO_TCP;
 
 		aux.tp_len = origlen;
 		aux.tp_snaplen = skb->len;
-- 
2.31.1

[PATCHv2 net-next 09/10] net: add support for ipv4 big tcp

[Xin Long]

Similar to Eric's IPv6 BIG TCP, this patch is to enable IPv4 BIG TCP.

Firstly, allow sk->sk_gso_max_size to be set to a value greater than
GSO_LEGACY_MAX_SIZE by not trimming gso_max_size in sk_trim_gso_size()
for IPv4 TCP sockets.

Then on TX path, set IP header tot_len to 0 when skb->len > IP_MAX_MTU
in __ip_local_out() to allow to send BIG TCP packets, and this implies
that skb->len is the length of a IPv4 packet; On RX path, use skb->len
as the length of the IPv4 packet when the IP header tot_len is 0 and
skb->len > IP_MAX_MTU in ip_rcv_core(). As the API iph_set_totlen() and
skb_ip_totlen() are used in __ip_local_out() and ip_rcv_core(), we only
need to update these APIs.

Also in GRO receive, add the check for ETH_P_IP/IPPROTO_TCP, and allows
the merged packet size >= GRO_LEGACY_MAX_SIZE in skb_gro_receive(). In
GRO complete, set IP header tot_len to 0 when the merged packet size
greater than IP_MAX_MTU in iph_set_totlen() so that it can be processed
on RX path.

Note that by checking skb_is_gso_tcp() in API iph_totlen(), it makes
this implementation safe to use iph->len == 0 indicates IPv4 BIG TCP
packets.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/core/gro.c       |  6 +++---
 net/core/sock.c      | 11 ++---------
 net/ipv4/af_inet.c   |  7 ++++---
 net/ipv4/ip_input.c  |  2 +-
 net/ipv4/ip_output.c |  2 +-
 5 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/net/core/gro.c b/net/core/gro.c
index 506f83d715f8..82656dc787f2 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -169,9 +169,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
 		return -E2BIG;
 
 	if (unlikely(p->len + len >= GRO_LEGACY_MAX_SIZE)) {
-		if (p->protocol != htons(ETH_P_IPV6) ||
-		    skb_headroom(p) < sizeof(struct hop_jumbo_hdr) ||
-		    ipv6_hdr(p)->nexthdr != IPPROTO_TCP ||
+		if (NAPI_GRO_CB(skb)->proto != IPPROTO_TCP ||
+		    (p->protocol == htons(ETH_P_IPV6) &&
+		     skb_headroom(p) < sizeof(struct hop_jumbo_hdr)) ||
 		    p->encapsulation)
 			return -E2BIG;
 	}
diff --git a/net/core/sock.c b/net/core/sock.c
index 7ba4891460ad..602c3d68ce19 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2375,15 +2375,8 @@ EXPORT_SYMBOL_GPL(sk_free_unlock_clone);
 
 static void sk_trim_gso_size(struct sock *sk)
 {
-	if (sk->sk_gso_max_size <= GSO_LEGACY_MAX_SIZE)
-		return;
-#if IS_ENABLED(CONFIG_IPV6)
-	if (sk->sk_family == AF_INET6 &&
-	    sk_is_tcp(sk) &&
-	    !ipv6_addr_v4mapped(&sk->sk_v6_rcv_saddr))
-		return;
-#endif
-	sk->sk_gso_max_size = GSO_LEGACY_MAX_SIZE;
+	if (sk->sk_gso_max_size > GSO_LEGACY_MAX_SIZE && !sk_is_tcp(sk))
+		sk->sk_gso_max_size = GSO_LEGACY_MAX_SIZE;
 }
 
 void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 6c0ec2789943..2f992a323b95 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1485,6 +1485,7 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
 	if (unlikely(ip_fast_csum((u8 *)iph, 5)))
 		goto out;
 
+	NAPI_GRO_CB(skb)->proto = proto;
 	id = ntohl(*(__be32 *)&iph->id);
 	flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (id & ~IP_DF));
 	id >>= 16;
@@ -1618,9 +1619,9 @@ int inet_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
 
 int inet_gro_complete(struct sk_buff *skb, int nhoff)
 {
-	__be16 newlen = htons(skb->len - nhoff);
 	struct iphdr *iph = (struct iphdr *)(skb->data + nhoff);
 	const struct net_offload *ops;
+	__be16 totlen = iph->tot_len;
 	int proto = iph->protocol;
 	int err = -ENOSYS;
 
@@ -1629,8 +1630,8 @@ int inet_gro_complete(struct sk_buff *skb, int nhoff)
 		skb_set_inner_network_header(skb, nhoff);
 	}
 
-	csum_replace2(&iph->check, iph->tot_len, newlen);
-	iph->tot_len = newlen;
+	iph_set_totlen(iph, skb->len - nhoff);
+	csum_replace2(&iph->check, totlen, iph->tot_len);
 
 	ops = rcu_dereference(inet_offloads[proto]);
 	if (WARN_ON(!ops || !ops->callbacks.gro_complete))
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index e880ce77322a..0aa8c49b4e1b 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -511,7 +511,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
 	if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
 		goto csum_error;
 
-	len = ntohs(iph->tot_len);
+	len = skb_ip_totlen(skb);
 	if (skb->len < len) {
 		drop_reason = SKB_DROP_REASON_PKT_TOO_SMALL;
 		__IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 922c87ef1ab5..4e4e308c3230 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -100,7 +100,7 @@ int __ip_local_out(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
 	struct iphdr *iph = ip_hdr(skb);
 
-	iph->tot_len = htons(skb->len);
+	iph_set_totlen(iph, skb->len);
 	ip_send_check(iph);
 
 	/* if egress device is enslaved to an L3 master device pass the
-- 
2.31.1

[PATCHv2 net-next 10/10] selftests: add a selftest for ipv4 big tcp

[Xin Long]

This test runs on the client-router-server topo, and monitors the traffic
on the RX devices of router and server while sending BIG TCP packets with
netperf from client to server. Meanwhile, it changes 'tso' on the TX devs
and 'gro' on the RX devs. Then it checks if any BIG TCP packets appears
on the RX devs with 'iptables -m length ! --length 0:65535' for each case.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 tools/testing/selftests/net/Makefile   |   1 +
 tools/testing/selftests/net/big_tcp.sh | 140 +++++++++++++++++++++++++
 2 files changed, 141 insertions(+)
 create mode 100644 tools/testing/selftests/net/big_tcp.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 47314f0b3006..2b26b1c20198 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -76,6 +76,7 @@ TEST_PROGS += sctp_vrf.sh
 TEST_GEN_FILES += sctp_hello
 TEST_GEN_FILES += csum
 TEST_GEN_FILES += nat6to4.o
+TEST_PROGS += big_tcp.sh
 
 TEST_FILES := settings
 
diff --git a/tools/testing/selftests/net/big_tcp.sh b/tools/testing/selftests/net/big_tcp.sh
new file mode 100644
index 000000000000..af1fd60de71d
--- /dev/null
+++ b/tools/testing/selftests/net/big_tcp.sh
@@ -0,0 +1,140 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Testing For BIG TCP.
+# TOPO: CLIENT_NS (link0)<--->(link1) ROUTER_NS (link2)<--->(link3) SERVER_NS
+
+CLIENT_NS=$(mktemp -u client-XXXXXXXX)
+CLIENT_IP4="198.51.100.1"
+
+SERVER_NS=$(mktemp -u server-XXXXXXXX)
+SERVER_IP4="203.0.113.1"
+
+ROUTER_NS=$(mktemp -u router-XXXXXXXX)
+SERVER_GW4="203.0.113.2"
+CLIENT_GW4="198.51.100.2"
+
+MAX_SIZE=128000
+CHK_SIZE=65535
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+setup() {
+	ip netns add $CLIENT_NS
+	ip netns add $SERVER_NS
+	ip netns add $ROUTER_NS
+	ip -net $ROUTER_NS link add link1 type veth peer name link0 netns $CLIENT_NS
+	ip -net $ROUTER_NS link add link2 type veth peer name link3 netns $SERVER_NS
+
+	ip -net $CLIENT_NS link set link0 up
+	ip -net $CLIENT_NS addr add $CLIENT_IP4/24 dev link0
+	ip -net $CLIENT_NS route add $SERVER_IP4 dev link0 via $CLIENT_GW4
+	ip -net $CLIENT_NS link set dev link0 gro_max_size $MAX_SIZE gso_max_size $MAX_SIZE
+	ip net exec $CLIENT_NS sysctl -wq net.ipv4.tcp_window_scaling=10
+
+	ip -net $ROUTER_NS link set link1 up
+	ip -net $ROUTER_NS link set link2 up
+	ip -net $ROUTER_NS addr add $CLIENT_GW4/24 dev link1
+	ip -net $ROUTER_NS addr add $SERVER_GW4/24 dev link2
+	ip -net $ROUTER_NS link set dev link1 gro_max_size $MAX_SIZE gso_max_size $MAX_SIZE
+	ip -net $ROUTER_NS link set dev link2 gro_max_size $MAX_SIZE gso_max_size $MAX_SIZE
+	ip net exec $ROUTER_NS sysctl -wq net.ipv4.ip_forward=1
+
+	ip -net $SERVER_NS link set link3 up
+	ip -net $SERVER_NS addr add $SERVER_IP4/24 dev link3
+	ip -net $SERVER_NS route add $CLIENT_IP4 dev link3 via $SERVER_GW4
+	ip -net $SERVER_NS link set dev link3 gro_max_size $MAX_SIZE gso_max_size $MAX_SIZE
+	ip net exec $SERVER_NS sysctl -wq net.ipv4.tcp_window_scaling=10
+	ip net exec $SERVER_NS netserver 2>&1 >/dev/null
+}
+
+cleanup() {
+	ip net exec $SERVER_NS pkill netserver
+	ip -net $ROUTER_NS link del link1
+	ip -net $ROUTER_NS link del link2
+	ip netns del "$CLIENT_NS"
+	ip netns del "$SERVER_NS"
+	ip netns del "$ROUTER_NS"
+}
+
+start_counter() {
+	local ipt="iptables"
+	local iface=$1
+	local netns=$2
+
+	ip net exec $netns $ipt -t raw -A PREROUTING -i $iface \
+		-m length ! --length 0:$CHK_SIZE -j ACCEPT
+}
+
+check_counter() {
+	local ipt="iptables"
+	local iface=$1
+	local netns=$2
+
+	test `ip net exec $netns $ipt -t raw -L -v |grep $iface | awk '{print $1}'` != "0"
+}
+
+stop_counter() {
+	local ipt="iptables"
+	local iface=$1
+	local netns=$2
+
+	ip net exec $netns $ipt -t raw -D PREROUTING -i $iface \
+		-m length ! --length 0:$CHK_SIZE -j ACCEPT
+}
+
+do_netperf() {
+	local serip=$SERVER_IP4
+	local netns=$1
+
+	ip net exec $netns netperf -$NF -t TCP_STREAM -H $serip 2>&1 >/dev/null
+}
+
+do_test() {
+	local cli_tso=$1
+	local gw_gro=$2
+	local gw_tso=$3
+	local ser_gro=$4
+	local ret="PASS"
+
+	ip net exec $CLIENT_NS ethtool -K link0 tso $cli_tso
+	ip net exec $ROUTER_NS ethtool -K link1 gro $gw_gro
+	ip net exec $ROUTER_NS ethtool -K link2 tso $gw_tso
+	ip net exec $SERVER_NS ethtool -K link3 gro $ser_gro
+
+	start_counter link1 $ROUTER_NS
+	start_counter link3 $SERVER_NS
+	do_netperf $CLIENT_NS
+
+	if check_counter link1 $ROUTER_NS; then
+		check_counter link3 $SERVER_NS || ret="FAIL_on_link3"
+	else
+		ret="FAIL_on_link1"
+	fi
+
+	stop_counter link1 $ROUTER_NS
+	stop_counter link3 $SERVER_NS
+	printf "%-9s %-8s %-8s %-8s: [%s]\n" \
+		$cli_tso $gw_gro $gw_tso $ser_gro $ret
+	test $ret = "PASS"
+}
+
+testup() {
+	echo "CLI GSO | GW GRO | GW GSO | SER GRO" && \
+	do_test "on"  "on"  "on"  "on"  && \
+	do_test "on"  "off" "on"  "off" && \
+	do_test "off" "on"  "on"  "on"  && \
+	do_test "on"  "on"  "off" "on"  && \
+	do_test "off" "on"  "off" "on"
+}
+
+if ! netperf -V &> /dev/null; then
+	echo "SKIP: Could not run test without netperf tool"
+	exit $ksft_skip
+fi
+
+trap cleanup EXIT
+setup && echo "Testing for BIG TCP:" && \
+NF=4 testup && echo "***v4 Tests Done***"
+exit $?
-- 
2.31.1

Re: [PATCHv2 net-next 00/10] net: support ipv4 big tcp

[Eric Dumazet]

On Tue, Jan 24, 2023 at 3:20 AM Xin Long <lucien.xin@gmail.com> wrote:
>
> This is similar to the BIG TCP patchset added by Eric for IPv6:
>
>   https://lwn.net/Articles/895398/
>
> Different from IPv6, IPv4 tot_len is 16-bit long only, and IPv4 header
> doesn't have exthdrs(options) for the BIG TCP packets' length. To make
> it simple, as David and Paolo suggested, we set IPv4 tot_len to 0 to
> indicate this might be a BIG TCP packet and use skb->len as the real
> IPv4 total length.
>
> This will work safely, as all BIG TCP packets are GSO/GRO packets and
> processed on the same host as they were created; There is no padding
> in GSO/GRO packets, and skb->len - network_offset is exactly the IPv4
> packet total length; Also, before implementing the feature, all those
> places that may get iph tot_len from BIG TCP packets are taken care
> with some new APIs:
>
> Patch 1 adds some APIs for iph tot_len setting and getting, which are
> used in all these places where IPv4 BIG TCP packets may reach in Patch
> 2-8, and Patch 9 implements this feature and Patch 10 adds a selftest
> for it.
>
> Note that the similar change as in Patch 2-6 are also needed for IPv6
> BIG TCP packets, and will be addressed in another patchset.
>
> The similar performance test is done for IPv4 BIG TCP with 25Gbit NIC
> and 1.5K MTU:
>
> No BIG TCP:
> for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
> 168          322          337          3776.49
> 143          236          277          4654.67
> 128          258          288          4772.83
> 171          229          278          4645.77
> 175          228          243          4678.93
> 149          239          279          4599.86
> 164          234          268          4606.94
> 155          276          289          4235.82
> 180          255          268          4418.95
> 168          241          249          4417.82
>

NACK again

You have not addressed my feedback.

Given the experimental nature of BIG TCP, we need separate netlink attributes,
so that we can selectively enable BIG TCP for IPV6, and not for IPV4.

Some of us do not have time to risk breaking IPV4, even if IPV4 for
our networks is less than 0.01 % of the traffic.

Re: [PATCHv2 net-next 00/10] net: support ipv4 big tcp

[Xin Long]

On Tue, Jan 24, 2023 at 3:27 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Jan 24, 2023 at 3:20 AM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > This is similar to the BIG TCP patchset added by Eric for IPv6:
> >
> >   https://lwn.net/Articles/895398/
> >
> > Different from IPv6, IPv4 tot_len is 16-bit long only, and IPv4 header
> > doesn't have exthdrs(options) for the BIG TCP packets' length. To make
> > it simple, as David and Paolo suggested, we set IPv4 tot_len to 0 to
> > indicate this might be a BIG TCP packet and use skb->len as the real
> > IPv4 total length.
> >
> > This will work safely, as all BIG TCP packets are GSO/GRO packets and
> > processed on the same host as they were created; There is no padding
> > in GSO/GRO packets, and skb->len - network_offset is exactly the IPv4
> > packet total length; Also, before implementing the feature, all those
> > places that may get iph tot_len from BIG TCP packets are taken care
> > with some new APIs:
> >
> > Patch 1 adds some APIs for iph tot_len setting and getting, which are
> > used in all these places where IPv4 BIG TCP packets may reach in Patch
> > 2-8, and Patch 9 implements this feature and Patch 10 adds a selftest
> > for it.
> >
> > Note that the similar change as in Patch 2-6 are also needed for IPv6
> > BIG TCP packets, and will be addressed in another patchset.
> >
> > The similar performance test is done for IPv4 BIG TCP with 25Gbit NIC
> > and 1.5K MTU:
> >
> > No BIG TCP:
> > for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
> > 168          322          337          3776.49
> > 143          236          277          4654.67
> > 128          258          288          4772.83
> > 171          229          278          4645.77
> > 175          228          243          4678.93
> > 149          239          279          4599.86
> > 164          234          268          4606.94
> > 155          276          289          4235.82
> > 180          255          268          4418.95
> > 168          241          249          4417.82
> >
>
> NACK again
>
> You have not addressed my feedback.
>
> Given the experimental nature of BIG TCP, we need separate netlink attributes,
> so that we can selectively enable BIG TCP for IPV6, and not for IPV4.
>
That will be some change, and I will try to work on it.

While at it, just try to be clearer, about the fixes for IPv6 BIG TCP
I mentioned in this patchset. Since skb->len is trustable for GSO TCP
packets. Are you still not okay with checking the skb_ipv6_pktlen()
API added to fix them in netfilter/tc/bridge/openvswitch?

Thanks.

Re: [PATCHv2 net-next 00/10] net: support ipv4 big tcp

[Eric Dumazet]

On Tue, Jan 24, 2023 at 4:51 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Tue, Jan 24, 2023 at 3:27 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Tue, Jan 24, 2023 at 3:20 AM Xin Long <lucien.xin@gmail.com> wrote:
> > >
> > > This is similar to the BIG TCP patchset added by Eric for IPv6:
> > >
> > >   https://lwn.net/Articles/895398/
> > >
> > > Different from IPv6, IPv4 tot_len is 16-bit long only, and IPv4 header
> > > doesn't have exthdrs(options) for the BIG TCP packets' length. To make
> > > it simple, as David and Paolo suggested, we set IPv4 tot_len to 0 to
> > > indicate this might be a BIG TCP packet and use skb->len as the real
> > > IPv4 total length.
> > >
> > > This will work safely, as all BIG TCP packets are GSO/GRO packets and
> > > processed on the same host as they were created; There is no padding
> > > in GSO/GRO packets, and skb->len - network_offset is exactly the IPv4
> > > packet total length; Also, before implementing the feature, all those
> > > places that may get iph tot_len from BIG TCP packets are taken care
> > > with some new APIs:
> > >
> > > Patch 1 adds some APIs for iph tot_len setting and getting, which are
> > > used in all these places where IPv4 BIG TCP packets may reach in Patch
> > > 2-8, and Patch 9 implements this feature and Patch 10 adds a selftest
> > > for it.
> > >
> > > Note that the similar change as in Patch 2-6 are also needed for IPv6
> > > BIG TCP packets, and will be addressed in another patchset.
> > >
> > > The similar performance test is done for IPv4 BIG TCP with 25Gbit NIC
> > > and 1.5K MTU:
> > >
> > > No BIG TCP:
> > > for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
> > > 168          322          337          3776.49
> > > 143          236          277          4654.67
> > > 128          258          288          4772.83
> > > 171          229          278          4645.77
> > > 175          228          243          4678.93
> > > 149          239          279          4599.86
> > > 164          234          268          4606.94
> > > 155          276          289          4235.82
> > > 180          255          268          4418.95
> > > 168          241          249          4417.82
> > >
> >
> > NACK again
> >
> > You have not addressed my feedback.
> >
> > Given the experimental nature of BIG TCP, we need separate netlink attributes,
> > so that we can selectively enable BIG TCP for IPV6, and not for IPV4.
> >
> That will be some change, and I will try to work on it.
>
> While at it, just try to be clearer, about the fixes for IPv6 BIG TCP
> I mentioned in this patchset. Since skb->len is trustable for GSO TCP
> packets. Are you still not okay with checking the skb_ipv6_pktlen()
> API added to fix them in netfilter/tc/bridge/openvswitch?
>

Are you speaking of length_mt6() ?

Quite frankly I do not think its implementation should care of GSO or anything.

Considering the definition of this thing clearly never thought of
having big packets,
and an overflow was already possible, I do not see how you can fix it
without some hack...

struct xt_length_info {
    __u16 min, max;
    __u8 invert;
};

Something like:

diff --git a/net/netfilter/xt_length.c b/net/netfilter/xt_length.c
index 1873da3a945abbc6e8849e4555b42acdd34cff2d..90eba619cbe1d11f0fdd394f6dfda2b03fa573cd
100644
--- a/net/netfilter/xt_length.c
+++ b/net/netfilter/xt_length.c
@@ -30,8 +30,7 @@ static bool
 length_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 {
        const struct xt_length_info *info = par->matchinfo;
-       const u_int16_t pktlen = ntohs(ipv6_hdr(skb)->payload_len) +
-                                sizeof(struct ipv6hdr);
+       u32 pktlen = min_t(u32, skb->len, 65535);

        return (pktlen >= info->min && pktlen <= info->max) ^ info->invert;
 }

Re: [PATCHv2 net-next 00/10] net: support ipv4 big tcp

[Xin Long]

On Tue, Jan 24, 2023 at 11:35 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Jan 24, 2023 at 4:51 PM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > On Tue, Jan 24, 2023 at 3:27 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Tue, Jan 24, 2023 at 3:20 AM Xin Long <lucien.xin@gmail.com> wrote:
> > > >
> > > > This is similar to the BIG TCP patchset added by Eric for IPv6:
> > > >
> > > >   https://lwn.net/Articles/895398/
> > > >
> > > > Different from IPv6, IPv4 tot_len is 16-bit long only, and IPv4 header
> > > > doesn't have exthdrs(options) for the BIG TCP packets' length. To make
> > > > it simple, as David and Paolo suggested, we set IPv4 tot_len to 0 to
> > > > indicate this might be a BIG TCP packet and use skb->len as the real
> > > > IPv4 total length.
> > > >
> > > > This will work safely, as all BIG TCP packets are GSO/GRO packets and
> > > > processed on the same host as they were created; There is no padding
> > > > in GSO/GRO packets, and skb->len - network_offset is exactly the IPv4
> > > > packet total length; Also, before implementing the feature, all those
> > > > places that may get iph tot_len from BIG TCP packets are taken care
> > > > with some new APIs:
> > > >
> > > > Patch 1 adds some APIs for iph tot_len setting and getting, which are
> > > > used in all these places where IPv4 BIG TCP packets may reach in Patch
> > > > 2-8, and Patch 9 implements this feature and Patch 10 adds a selftest
> > > > for it.
> > > >
> > > > Note that the similar change as in Patch 2-6 are also needed for IPv6
> > > > BIG TCP packets, and will be addressed in another patchset.
> > > >
> > > > The similar performance test is done for IPv4 BIG TCP with 25Gbit NIC
> > > > and 1.5K MTU:
> > > >
> > > > No BIG TCP:
> > > > for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
> > > > 168          322          337          3776.49
> > > > 143          236          277          4654.67
> > > > 128          258          288          4772.83
> > > > 171          229          278          4645.77
> > > > 175          228          243          4678.93
> > > > 149          239          279          4599.86
> > > > 164          234          268          4606.94
> > > > 155          276          289          4235.82
> > > > 180          255          268          4418.95
> > > > 168          241          249          4417.82
> > > >
> > >
> > > NACK again
> > >
> > > You have not addressed my feedback.
> > >
> > > Given the experimental nature of BIG TCP, we need separate netlink attributes,
> > > so that we can selectively enable BIG TCP for IPV6, and not for IPV4.
> > >
> > That will be some change, and I will try to work on it.
> >
> > While at it, just try to be clearer, about the fixes for IPv6 BIG TCP
> > I mentioned in this patchset. Since skb->len is trustable for GSO TCP
> > packets. Are you still not okay with checking the skb_ipv6_pktlen()
> > API added to fix them in netfilter/tc/bridge/openvswitch?
> >
>
> Are you speaking of length_mt6() ?
Yes, but not only there, see also:

[1]:
dump_ipv6_packet()
nf_ct_bridge_pre()
ovs_skb_network_trim()/tcf_ct_skb_network_trim()
cake_ack_filter()

These places get pktlen directly from ipv6_hdr(skb)->payload_len.

>
> Quite frankly I do not think its implementation should care of GSO or anything.
Agree, and IPv6 jumbo packets don't only include IPv6 BIG TCP.

But,
>
> Considering the definition of this thing clearly never thought of
> having big packets,
> and an overflow was already possible, I do not see how you can fix it
> without some hack...
>
> struct xt_length_info {
>     __u16 min, max;
>     __u8 invert;
> };
>
> Something like:
>
> diff --git a/net/netfilter/xt_length.c b/net/netfilter/xt_length.c
> index 1873da3a945abbc6e8849e4555b42acdd34cff2d..90eba619cbe1d11f0fdd394f6dfda2b03fa573cd
> 100644
> --- a/net/netfilter/xt_length.c
> +++ b/net/netfilter/xt_length.c
> @@ -30,8 +30,7 @@ static bool
>  length_mt6(const struct sk_buff *skb, struct xt_action_param *par)
>  {
>         const struct xt_length_info *info = par->matchinfo;
> -       const u_int16_t pktlen = ntohs(ipv6_hdr(skb)->payload_len) +
> -                                sizeof(struct ipv6hdr);
> +       u32 pktlen = min_t(u32, skb->len, 65535);
do you also expect this to be applied to other places in [1] above?

I was just thinking since skb->len is trustable for GSO TCP (not all jumbo
packets), we should use skb->len as the pktlen only when it's a GSO TCP
packet in such places.

Thanks.

Re: [PATCHv2 net-next 00/10] net: support ipv4 big tcp

[Eric Dumazet]

On Tue, Jan 24, 2023 at 6:14 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Tue, Jan 24, 2023 at 11:35 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Tue, Jan 24, 2023 at 4:51 PM Xin Long <lucien.xin@gmail.com> wrote:
> > >
> > > On Tue, Jan 24, 2023 at 3:27 AM Eric Dumazet <edumazet@google.com> wrote:
> > > >
> > > > On Tue, Jan 24, 2023 at 3:20 AM Xin Long <lucien.xin@gmail.com> wrote:
> > > > >
> > > > > This is similar to the BIG TCP patchset added by Eric for IPv6:
> > > > >
> > > > >   https://lwn.net/Articles/895398/
> > > > >
> > > > > Different from IPv6, IPv4 tot_len is 16-bit long only, and IPv4 header
> > > > > doesn't have exthdrs(options) for the BIG TCP packets' length. To make
> > > > > it simple, as David and Paolo suggested, we set IPv4 tot_len to 0 to
> > > > > indicate this might be a BIG TCP packet and use skb->len as the real
> > > > > IPv4 total length.
> > > > >
> > > > > This will work safely, as all BIG TCP packets are GSO/GRO packets and
> > > > > processed on the same host as they were created; There is no padding
> > > > > in GSO/GRO packets, and skb->len - network_offset is exactly the IPv4
> > > > > packet total length; Also, before implementing the feature, all those
> > > > > places that may get iph tot_len from BIG TCP packets are taken care
> > > > > with some new APIs:
> > > > >
> > > > > Patch 1 adds some APIs for iph tot_len setting and getting, which are
> > > > > used in all these places where IPv4 BIG TCP packets may reach in Patch
> > > > > 2-8, and Patch 9 implements this feature and Patch 10 adds a selftest
> > > > > for it.
> > > > >
> > > > > Note that the similar change as in Patch 2-6 are also needed for IPv6
> > > > > BIG TCP packets, and will be addressed in another patchset.
> > > > >
> > > > > The similar performance test is done for IPv4 BIG TCP with 25Gbit NIC
> > > > > and 1.5K MTU:
> > > > >
> > > > > No BIG TCP:
> > > > > for i in {1..10}; do netperf -t TCP_RR -H 192.168.100.1 -- -r80000,80000 -O MIN_LATENCY,P90_LATENCY,P99_LATENCY,THROUGHPUT|tail -1; done
> > > > > 168          322          337          3776.49
> > > > > 143          236          277          4654.67
> > > > > 128          258          288          4772.83
> > > > > 171          229          278          4645.77
> > > > > 175          228          243          4678.93
> > > > > 149          239          279          4599.86
> > > > > 164          234          268          4606.94
> > > > > 155          276          289          4235.82
> > > > > 180          255          268          4418.95
> > > > > 168          241          249          4417.82
> > > > >
> > > >
> > > > NACK again
> > > >
> > > > You have not addressed my feedback.
> > > >
> > > > Given the experimental nature of BIG TCP, we need separate netlink attributes,
> > > > so that we can selectively enable BIG TCP for IPV6, and not for IPV4.
> > > >
> > > That will be some change, and I will try to work on it.
> > >
> > > While at it, just try to be clearer, about the fixes for IPv6 BIG TCP
> > > I mentioned in this patchset. Since skb->len is trustable for GSO TCP
> > > packets. Are you still not okay with checking the skb_ipv6_pktlen()
> > > API added to fix them in netfilter/tc/bridge/openvswitch?
> > >
> >
> > Are you speaking of length_mt6() ?
> Yes, but not only there, see also:
>
> [1]:
> dump_ipv6_packet()
> nf_ct_bridge_pre()
> ovs_skb_network_trim()/tcf_ct_skb_network_trim()
> cake_ack_filter()
>
> These places get pktlen directly from ipv6_hdr(skb)->payload_len.

dump_ipv6_packet() is right if its intent is to 'dump header values'
If jumbo is not yet parsed, this should be added.

cake_ack_filter() depends on receiving non malicious packets, which is
not guaranteed.

Special casing GSO seems the wrong way to fix buggy spots like these.