Commit d2438c16 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'don-t-take-hw-uso-path-when-packets-can-t-be-checksummed-by-device' 

Jakub Sitnicki says:

====================
Don't take HW USO path when packets can't be checksummed by device

This series addresses a recent regression report from syzbot [1].

After enabling UDP_SEGMENT for egress devices which don't support checksum
offload [2], we need to tighten down the checks which let packets take the
HW USO path.

The fix consists of two parts:

1. don't let devices offer USO without checksum offload, and
2. force software USO fallback in presence of IPv6 extension headers.

[1] https://lore.kernel.org/all/000000000000e1609a061d5330ce@google.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=10154dbded6d6a2fecaebdfda206609de0f121a9

v3: https://lore.kernel.org/r/20240807-udp-gso-egress-from-tunnel-v3-0-8828d93c5b45@cloudflare.com
v2: https://lore.kernel.org/r/20240801-udp-gso-egress-from-tunnel-v2-0-9a2af2f15d8d@cloudflare.com
v1: https://lore.kernel.org/r/20240725-udp-gso-egress-from-tunnel-v1-0-5e5530ead524@cloudflare.com
====================

Link: https://patch.msgid.link/20240808-udp-gso-egress-from-tunnel-v4-0-f5c5b4149ab9@cloudflare.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 3a3be7ff 1d2c46c1

net/core/dev.c

+17 −9
Original line number Diff line number Diff line
@@ -9912,6 +9912,15 @@ static void netdev_sync_lower_features(struct net_device *upper, 
	}

}



static bool netdev_has_ip_or_hw_csum(netdev_features_t features)

{

	netdev_features_t ip_csum_mask = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;

	bool ip_csum = (features & ip_csum_mask) == ip_csum_mask;

	bool hw_csum = features & NETIF_F_HW_CSUM;



	return ip_csum || hw_csum;

}



static netdev_features_t netdev_fix_features(struct net_device *dev,

	netdev_features_t features)

{
@@ -9993,22 +10002,21 @@ static netdev_features_t netdev_fix_features(struct net_device *dev, 
		features &= ~NETIF_F_LRO;

	}



	if (features & NETIF_F_HW_TLS_TX) {

		bool ip_csum = (features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)) ==

			(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);

		bool hw_csum = features & NETIF_F_HW_CSUM;



		if (!ip_csum && !hw_csum) {

	if ((features & NETIF_F_HW_TLS_TX) && !netdev_has_ip_or_hw_csum(features)) {

		netdev_dbg(dev, "Dropping TLS TX HW offload feature since no CSUM feature.\n");

		features &= ~NETIF_F_HW_TLS_TX;

	}

	}



	if ((features & NETIF_F_HW_TLS_RX) && !(features & NETIF_F_RXCSUM)) {

		netdev_dbg(dev, "Dropping TLS RX HW offload feature since no RXCSUM feature.\n");

		features &= ~NETIF_F_HW_TLS_RX;

	}



	if ((features & NETIF_F_GSO_UDP_L4) && !netdev_has_ip_or_hw_csum(features)) {

		netdev_dbg(dev, "Dropping USO feature since no CSUM feature.\n");

		features &= ~NETIF_F_GSO_UDP_L4;

	}



	return features;

}

net/ipv4/udp_offload.c

+6 −0
Original line number Diff line number Diff line
@@ -282,6 +282,12 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, 
		     skb_transport_header(gso_skb)))

		return ERR_PTR(-EINVAL);



	/* We don't know if egress device can segment and checksum the packet

	 * when IPv6 extension headers are present. Fall back to software GSO.

	 */

	if (gso_skb->ip_summed != CHECKSUM_PARTIAL)

		features &= ~(NETIF_F_GSO_UDP_L4 | NETIF_F_CSUM_MASK);



	if (skb_gso_ok(gso_skb, features | NETIF_F_GSO_ROBUST)) {

		/* Packet is from an untrusted source, reset gso_segs. */

		skb_shinfo(gso_skb)->gso_segs = DIV_ROUND_UP(gso_skb->len - sizeof(*uh),

tools/testing/selftests/net/udpgso.c

+24 −1
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ struct testcase { 
	int gso_len;		/* mss after applying gso */

	int r_num_mss;		/* recv(): number of calls of full mss */

	int r_len_last;		/* recv(): size of last non-mss dgram, if any */

	bool v6_ext_hdr;	/* send() dgrams with IPv6 extension headers */

};



const struct in6_addr addr6 = {
@@ -77,6 +78,8 @@ const struct in_addr addr4 = { 
	__constant_htonl(0x0a000001), /* 10.0.0.1 */

};



static const char ipv6_hopopts_pad1[8] = { 0 };



struct testcase testcases_v4[] = {

	{

		/* no GSO: send a single byte */
@@ -255,6 +258,13 @@ struct testcase testcases_v6[] = { 
		.gso_len = 1,

		.r_num_mss = 2,

	},

	{

		/* send 2 1B segments with extension headers */

		.tlen = 2,

		.gso_len = 1,

		.r_num_mss = 2,

		.v6_ext_hdr = true,

	},

	{

		/* send 2B + 2B + 1B segments */

		.tlen = 5,
@@ -396,11 +406,18 @@ static void run_one(struct testcase *test, int fdt, int fdr, 
	int i, ret, val, mss;

	bool sent;



	fprintf(stderr, "ipv%d tx:%d gso:%d %s\n",

	fprintf(stderr, "ipv%d tx:%d gso:%d %s%s\n",

			addr->sa_family == AF_INET ? 4 : 6,

			test->tlen, test->gso_len,

			test->v6_ext_hdr ? "ext-hdr " : "",

			test->tfail ? "(fail)" : "");



	if (test->v6_ext_hdr) {

		if (setsockopt(fdt, IPPROTO_IPV6, IPV6_HOPOPTS,

			       ipv6_hopopts_pad1, sizeof(ipv6_hopopts_pad1)))

			error(1, errno, "setsockopt ipv6 hopopts");

	}



	val = test->gso_len;

	if (cfg_do_setsockopt) {

		if (setsockopt(fdt, SOL_UDP, UDP_SEGMENT, &val, sizeof(val)))
@@ -412,6 +429,12 @@ static void run_one(struct testcase *test, int fdt, int fdr, 
		error(1, 0, "send succeeded while expecting failure");

	if (!sent && !test->tfail)

		error(1, 0, "send failed while expecting success");



	if (test->v6_ext_hdr) {

		if (setsockopt(fdt, IPPROTO_IPV6, IPV6_HOPOPTS, NULL, 0))

			error(1, errno, "setsockopt ipv6 hopopts clear");

	}



	if (!sent)

		return;