net: configurable extra TX headroom for local egress

A program attached at TC egress commonly pushes additional headers onto a packet, for example to encapsulate it. Locally generated UDP, raw and AF_PACKET traffic is allocated with exactly LL_RESERVED_SPACE() of headroom, which covers the link-layer header plus the driver's needed_headroom and alignment, leaving no room for headers an egress program pushes, so the push reallocates the skb head on every packet.

An XDP program on a veth peer has the same problem. A local UDP, raw or AF_PACKET socket, or AF_XDP in copy mode, sends a packet on a veth device. veth_xmit() passes the skb to the peer, where the XDP program runs. The skb reaches the peer with headroom smaller than XDP_PACKET_HEADROOM (256 bytes), which XDP guarantees to its programs. With native XDP, the peer therefore has to copy the packet in veth_xdp_rcv_skb() to get the headroom required. Generic XDP on veth and other devices like macvlan and ipvlan also copies the packet in netif_receive_generic_xdp().

Add a per-device tx_extra_headroom, configured through a new IFLA_TX_EXTRA_HEADROOM rtnetlink attribute. It is a separate value from
needed_headroom: LL_ALLOC_SPACE() adds it at the allocation sites only, it can be set and cleared freely without recreating the device, it is reported through its own attribute so it is never confused with the driver's requirement, and it is not inherited by stacked devices. needed_headroom keeps its current meaning and remains the threshold for deciding whether an existing packet must be expanded, so the extra reservation never forces a reallocation on its own.

The value is limited to NETDEV_TX_EXTRA_HEADROOM_MAX (512 bytes). This is enough for several stacked encapsulations, for example VXLAN, GRE or IPIP, and keeps the growth of per-packet allocations modest. The netlink policy rejects a larger value.

The reservation applies to the device the packet is allocated against (the one the route selects) at skb-allocation time. A GSO super-skb is therefore allocated once with the headroom in place; software segmentation copies that headroom into each segment, so the push lands in reserved space for every segment without a per-segment realloc. net/xdp/xsk.c is included among the converted paths: AF_XDP copy mode open-codes its own headroom calculation from needed_headroom, so it uses netdev_tx_alloc_headroom() to pick up the extra reservation consistently with the socket paths.

Because the reservation is applied only at the allocation sites, TCP is left alone: TCP segments are built with MAX_TCP_HEADER already reserved and are checked against LL_RESERVED_SPACE(), not LL_ALLOC_SPACE(), so raising the knob does not reallocate them. Covering the TCP allocation path is deliberately not part of this series.

With this series, generic XDP no longer copies packets that a local socket sends on veth. Native XDP on veth still copies them. Besides enough headroom, it also needs a head that is a page fragment. A local socket allocates the head with kmalloc, so it does not qualify. A follow-up veth patch will move this check and the copy after the program, so that XDP_PASS and XDP_DROP do not copy.

One option is to raise needed_headroom instead. Setting it from user space needs driver support [1], which veth does not have. Where a driver does support it, the value is fixed at creation time. Moreover, raising it regresses current data paths, as measured below. tx_extra_headroom avoids these problems. It needs no driver support. It only affects new allocations. It can be cleared at any time.

Another option, suggested in [2], is to declare the needed headroom in the BPF program itself, for example with a BTF decl tag. This works for a program at TC egress. An XDP program, however, runs on the receive side. The skb is already allocated elsewhere before it reaches the program.

Auto-tuning, as suggested in [3], is out of scope for this series, but it is possible future work. ip_tunnel_adj_headroom() already does this for needed_headroom. A tuned value would also need a field that does not cause the regressions described below, and tx_extra_headroom can be that field.

- Patch 1 adds the field and the LL_ALLOC_SPACE() macros.
- Patch 2 switches the local-egress allocation sites to them (UDP, raw, AF_PACKET and AF_XDP).
- Patch 3 exposes the attribute through rtnetlink, with documentation and the rt-link spec entry.
- Patch 4 tests the attribute itself through YNL.
- Patch 5 tests whether a push at TC egress reallocates the skb.
- Patch 6 adds a flag to xdp_helper that sends one frame.
- Patch 7 tests whether generic XDP on a veth peer copies packets from UDP and raw sockets (IPv4 and IPv6), AF_PACKET and AF_XDP.
- Patch 8 tests that ip can set tx_extra_headroom on both ends of a new veth pair.
- The iproute2 series "iplink: add tx_extra_headroom" is sent separately, at the same time as this series.

The following case shows how raising needed_headroom regresses current data paths. Take a veth pair with veth0 in the host and veth1 in a network namespace, and an XDP program on veth1. The ideal needed_headroom for veth0 would be XDP_PACKET_HEADROOM (256 bytes). A packet allocated on veth0 then reaches the XDP program on veth1 without a copy. However, this regresses data paths that reach ip_finish_output2() or ip_forward():

a. TCP sent between the host and the namespace. TCP reserves
MAX_TCP_HEADER of headroom for all its headers. After the TCP and IP headers, less headroom is left in front of the IP header than LL_RESERVED_SPACE() (272 bytes). ip_finish_output2() therefore expands the head of every TCP skb, ACKs included.

b. A packet received by the host and forwarded into the namespace. mlx5
and ice running XDP leave 256 bytes in front of the Ethernet header, so today the packet reaches the XDP program on veth1 without a copy. In front of the IP header it has 270 bytes, 2 short of LL_RESERVED_SPACE() (272 bytes). ip_forward() now expands the head of every such packet. The new head is not a page fragment, so veth1 also copies the packet before XDP runs. bnxt copies the headers into a new skb with 64 bytes of headroom, so veth1 already copies its packets, and the expansion comes on top.

We used patch 4/4 of Jesper Dangaard Brouer's veth reallocation RFC [4] to raise the needed_headroom of both veth0 and veth1 to 256 while XDP is attached. We then measured the throughput of cases a and b on a Xeon E5-2640 v4 with a ConnectX-4 Lx NIC, on net-next at 734bef548a96. Each value is the median of 9 rounds, and the scripts are in [5]:

test                          tool     net-next       256 (4/4 in [4])
local TCP, host to netns (a)  iperf3   5.75 Gbit/s    5.59 (-2.8%)
local TCP, netns to host (a)  iperf3   39.14 Gbit/s   37.70 (-3.7%)
TCP_RR, 1 byte (a)            netperf  91582 trans/s  86296 (-5.8%)
forwarded TCP (b)             iperf3   7.04 Gbit/s    6.76 (-4.0%)
forwarded 64B UDP (b)         pktgen   873822 pps     692155 (-20.8%)

bpftrace confirms that the head of every packet in these tests is expanded in the 256 column.

The next table shows that tx_extra_headroom removes the reallocation caused by a push at TC egress. netperf sends 500000 UDP datagrams of 64 bytes on a veth device, and a program at TC egress pushes 20 bytes after the Ethernet header with bpf_skb_adjust_room(). Only tx_extra_headroom varies. The machine and base are the same as above, with this series applied:

tx_extra_headroom  tc push  pskb_expand_head() calls
0                  yes      500003
64                 yes      0
128                yes      0
256                yes      0
256                no       0

All 5 rounds give the same counts. 500003 includes 3 datagrams that netperf sends at the end of the test. With no program pushing headers, a set value only costs the reserved bytes per allocation. It causes no reallocation or copy.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] https://lore.kernel.org/all/169272716651.1975370.10514711233878278884.stgit@firesoul/
[5] https://github.com/tgao-cf/tx-extra-headroom-bench/tree/main

---
Changes since RFC:
- Patch 1 also removes LL_RESERVED_SPACE_EXTRA(), which has no user since commit 6d5431555de0 ("caif: remove CAIF NETWORK LAYER").
- Patch 2 adds a paragraph on why ip_do_fragment() and ip6_fragment() are left alone. It also no longer says that AF_XDP copy mode uses the same calculation: xsk.c has its own, from needed_headroom.
- Patch 3 changes IFLA_TX_EXTRA_HEADROOM from u16 to u32.
- Patch 3 fixes the description of LL_ALLOC_SPACE() in netdevices.rst. It adds tx_extra_headroom before the HH_DATA_MOD alignment, not after.
- Patch 3 names the paths that use the setting in rt-link.yaml. TCP and forwarded packets are not covered.
- Patch 4 renames set_() to set_hr() and d0 to dummy0, and adds docstrings.
- Patch 4 checks that the VLAN headroom is exactly 4 and that tx_extra_headroom does not change needed_headroom.
- Patch 4 skips every case on a kernel without the attribute.
- RFC patches 5 and 6 are mostly dropped. Patch 5 is a new test. One tc program in selftests/net compares skb->head before and after the push. It follows test_dst_clear.c in selftests/bpf. The test setup of tx_extra_headroom_dataplane.py is moved into tx_extra_headroom.py.
- Patches 6-8 are new: a flag to send a frame in xdp_helper, a test of generic XDP on the veth peer, and a test that sets both ends of a veth pair at creation.
- The cover letter adds the veth XDP case, the regressions from raising needed_headroom and answers to the BTF annotation and auto-tuning ideas. The tx_extra_headroom measurements are repeated on net-next 734bef548a96. The selftests run on net-next fa5196392691. The cover letter drops the RFC question, the sentence that there is no in-tree way to ask for more headroom, and the sentence about a concurrent TCP stream, which no test covers.

RFC: https://lore.kernel.org/all/[email protected]/

David Keogh suggested that I post v1, with patches 1-4 from him:
https://lore.kernel.org/all/au4j2nQdOwr90ffh@dev-dsk-dkeogh-ai2-36d3b9a7.us-west-2.amazon.com/

David Keogh has a test that tx_extra_headroom does not change the headroom of TCP packets. He will post it as a reply to this series.

David Keogh (4):
net: add tx_extra_headroom and LL_ALLOC_SPACE()
net: reserve extra TX headroom for local egress
rtnetlink: add IFLA_TX_EXTRA_HEADROOM
selftests: net: add tx_extra_headroom test

Tianyi Gao (4):
selftests: net: tx_extra_headroom: test a push at TC egress
selftests: net: add flag to send a frame in xdp_helper
selftests: net: tx_extra_headroom: test generic XDP on the veth peer
selftests: net: tx_extra_headroom: test veth peer at creation

 Documentation/netlink/specs/rt-link.yaml      |  12 +
 .../networking/net_cachelines/net_device.rst  |   1 +
 Documentation/networking/netdevices.rst       |  35 ++
 include/linux/netdevice.h                     |  36 +-
 include/uapi/linux/if_link.h                  |   1 +
 net/core/dev.c                                |   2 +
 net/core/rtnetlink.c                          |  19 +-
 net/ipv4/ip_output.c                          |   2 +-
 net/ipv4/raw.c                                |   2 +-
 net/ipv6/ip6_output.c                         |   2 +-
 net/ipv6/raw.c                                |   2 +-
 net/packet/af_packet.c                        |   6 +-
 net/xdp/xsk.c                                 |   6 +-
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/config            |   1 +
 tools/testing/selftests/net/lib/xdp_helper.c  | 113 +++++-
 .../selftests/net/tx_extra_headroom.bpf.c     |  94 +++++
 .../selftests/net/tx_extra_headroom.py        | 330 ++++++++++++++++++
 18 files changed, 645 insertions(+), 20 deletions(-)
 create mode 100644 tools/testing/selftests/net/tx_extra_headroom.bpf.c
 create mode 100755 tools/testing/selftests/net/tx_extra_headroom.py

base-commit: fa5196392691051963726787944db872420dfa75

2.56.0

From: David Keogh <[email protected]>

Programs attached at TC egress commonly push additional headers onto a packet, for example to encapsulate it. When the packet does not carry enough headroom the BPF helpers reallocate the head, which costs an allocation and a copy per packet.

Locally generated UDP, raw and AF_PACKET traffic is allocated with exactly LL_RESERVED_SPACE() of headroom, which only covers the link-layer header and the driver's needed_headroom. An administrator can only ask for more headroom at allocation time if the driver supports it, as netkit does with IFLA_NETKIT_HEADROOM. veth does not.

Add a tx_extra_headroom field to struct net_device for that purpose, together with LL_ALLOC_SPACE() and LL_ALLOC_SPACE_EX(). These add the extra reservation to the header length before the HH_DATA_MOD alignment, so the alignment of the link-layer header does not change.

The new macros are intended only for sites that allocate new packets. LL_RESERVED_SPACE() keeps its current meaning and remains the threshold used when checking whether an existing packet must be expanded, so the extra reservation never forces a reallocation on its own: packets that already have enough room for the headers being pushed, such as TCP segments built with MAX_TCP_HEADER, are left alone.

Keep the extra reservation separate from needed_headroom so that driver updates of their own requirement do not clobber it and so that it can be cleared independently. Place the field in the padding that follows needed_headroom, which keeps it in the TX read-mostly cacheline group.

The field is not set anywhere yet. A later patch exposes it through rtnetlink.

Also remove LL_RESERVED_SPACE_EXTRA(). It has had no user since commit 6d5431555de0 ("caif: remove CAIF NETWORK LAYER"). It could add the extra headroom, but it reads dev->hard_header_len itself. AF_PACKET must use the value that it read once, see commit 03390aa32e66 ("packet:
use consistent hard_header_len in non-ring send paths"). LL_ALLOC_SPACE_EX() takes the header length as an argument instead.

---
checkpatch --strict reports "Macro argument reuse 'dev'" for LL_ALLOC_SPACE_EX() and LL_ALLOC_SPACE(). It reports the same for LL_RESERVED_SPACE().

---
 .../networking/net_cachelines/net_device.rst  |  1 +
 include/linux/netdevice.h                     | 36 +++++++++++++++++--
 net/core/dev.c                                |  2 ++
 3 files changed, 36 insertions(+), 3 deletions(-)

index 8eceaa80b686..a7e311fb7875 100644
@@ -35,6 +35,7 @@ unsigned_short                      gflags
 unsigned_short                      hard_header_len             read_mostly         read_mostly         ip6_xmit(tx);gro_list_prepare(rx)
 unsigned_int                        mtu                         read_mostly                             ip_finish_output2
 unsigned_short                      needed_headroom             read_mostly                             LL_RESERVED_SPACE/ip_finish_output2
+unsigned_short                      tx_extra_headroom           read_mostly                             LL_ALLOC_SPACE/__ip_append_data
 unsigned_short                      needed_tailroom
 netdev_features_t                   features                    read_mostly         read_mostly         HARD_TX_LOCK,netif_skb_features,sk_setup_caps(tx);netif_elide_gro(rx)
 netdev_features_t                   hw_features

index 435be7880d7e..98d1501d6fee 100644
@@ -311,9 +311,24 @@ struct hh_cache {
 	  & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
 #define LL_RESERVED_SPACE(dev) \
 	LL_RESERVED_SPACE_EX(dev, (dev)->hard_header_len)
-#define LL_RESERVED_SPACE_EXTRA(dev,extra) \
-	((((dev)->hard_header_len + READ_ONCE((dev)->needed_headroom) + (extra)) \
-	  & ~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
+
+/*
+ * Headroom to reserve when allocating a locally generated packet for
+ * transmission through a device. On top of LL_RESERVED_SPACE() this adds
+ * the administrator-configured tx_extra_headroom, folded in before the
+ * alignment so that the link-layer header placement does not change.
+ *
+ * Only use this for new allocations. Code that checks whether an existing
+ * packet has enough headroom must keep using LL_RESERVED_SPACE(), so that
+ * the extra reservation can never force a reallocation by itself.
+ */
+#define LL_ALLOC_SPACE_EX(dev, hlen) \
+	LL_RESERVED_SPACE_EX(dev, (hlen) + READ_ONCE((dev)->tx_extra_headroom))
+#define LL_ALLOC_SPACE(dev) \
+	LL_ALLOC_SPACE_EX(dev, (dev)->hard_header_len)
+
+/* Upper bound for tx_extra_headroom, in bytes. */
+#define NETDEV_TX_EXTRA_HEADROOM_MAX	512
 
 struct header_ops {
 	int	(*create) (struct sk_buff *skb, struct net_device *dev,

@@ -1934,6 +1949,10 @@ enum netdev_reg_state {
  *
  *	@needed_headroom: Extra headroom the hardware may need, but not in all
  *			  cases can this be guaranteed
+ *	@tx_extra_headroom: Additional headroom reserved when allocating
+ *			    locally generated packets for transmission,
+ *			    configured by the administrator. Not a driver
+ *			    requirement; see LL_ALLOC_SPACE()
  *	@needed_tailroom: Extra tailroom the hardware may need, but not in all
  *			  cases can this be guaranteed. Some cases also use
  *			  LL_MAX_HEADER instead to allocate the skb

@@ -2196,6 +2215,7 @@ struct net_device {
 	 */
 	unsigned int		mtu;
 	unsigned short		needed_headroom;
+	unsigned short		tx_extra_headroom;
 	struct netdev_tc_txq	tc_to_txq[TC_MAX_QUEUE];
 #ifdef CONFIG_XPS
 	struct xps_dev_maps __rcu *xps_maps[XPS_MAPS_MAX];

@@ -2772,6 +2792,16 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
 					 struct sk_buff *skb,
 					 struct net_device *sb_dev);
 
+/* Same as LL_ALLOC_SPACE() without link-layer header and alignment, for
+ * callers that size the headroom themselves.
+ */
+static inline unsigned int
+netdev_tx_alloc_headroom(const struct net_device *dev)
+{
+	return READ_ONCE(dev->needed_headroom) +
+	       READ_ONCE(dev->tx_extra_headroom);
+}
+
 /* returns the headroom that the master device needs to take in account
  * when forwarding to this dev
  */

index 3b60b15e2175..95483a132a73 100644
@@ -13551,6 +13551,8 @@ static void __init net_dev_struct_check(void)
 	CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_tx, num_tc);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_tx, mtu);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_tx, needed_headroom);
+	CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_tx,
+				      tx_extra_headroom);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_tx, tc_to_txq);
 #ifdef CONFIG_XPS
 	CACHELINE_ASSERT_GROUP_MEMBER(struct net_device, net_device_read_tx, xps_maps);

2.56.0

Main high level concern is just adding an extra field. What's next, do we need to add a field extra_extra_headroom?

Since this already changes callers from LL_RESERVED_SPACE to LL_ALLOC_SPACE, could this additional use case be integrated more in the struct fields and helpers that are already there?

Admittedly, I did not spend much time looking at that deeply, and you likely did, so the answer is no. But I want to poke a little at that before we add yet another optional length field.

Thanks for the review.

A user needs more room either in every packet or only in new skbs. needed_headroom covers the first case and tx_extra_headroom the second. I do not see a third case.

needed_headroom does not only set the headroom of new skbs. It is also the minimum that ip_finish_output2() and ip_forward() require of existing skbs. Raising it for new skbs also raises that minimum. With XDP on a veth peer and needed_headroom raised to 256, throughput drops by 2.8% to 5.8% for local TCP and by 4.0% to 20.8% for forwarded traffic. The cover letter has the detailed analysis. veth also cannot set needed_headroom from user space today.

pahole on x86_64 (defconfig, net/core/dev.o) shows that the field fills the 2-byte hole after needed_headroom, so struct net_device does not grow:

before:

	unsigned short             needed_headroom;      /*    60     2 */
	/* XXX 2 bytes hole, try to pack */
	/* size: 2688, cachelines: 42, members: 187 */

after:

	unsigned short             needed_headroom;      /*    60     2 */
	unsigned short             tx_extra_headroom;    /*    62     2 */
	/* size: 2688, cachelines: 42, members: 188 */

arm64 defconfig gives the same offsets, and the size stays 2752 bytes.

Changing only the helpers does not help. Without a new field, the helpers can only add a fixed amount, which every allocation on every device then pays for. Or they can derive it from needed_headroom, which brings back the regressions above.

hard_header_len cannot carry it, because AF_PACKET and header_ops use it as the link-layer header length.

Agreed. v2 will make the new helpers static inline functions.

Thanks.

Pushing back on another design choice: the UAPI change. That both cannot be undone, and is fragile and laborious, requiring manual admin arithmetic.

The RFC patch mentioned two other approaches: explicit headroom requirement labeling for tc and BPF, and dynamic adjustment of this field whenever a realloc in the stack is encountered.

They are left for future work. But should those instead be implemented first? As said, this manual hackery cannot be undone. While in-kernel mechanisms can be refined and built upon.

It seems to be that this series is trying to address a few related situations and argues that no single solution fixes all except for manual adjustment. But likely a combination of the above (and more) will.

This does leave the veth peer to solve, but that does not seem fundamentally intractable.

Thanks for the valuable feedback.

On fragile: a wrong value never changes the packet. If it is too small, the push reallocates, as it does today. If it is too large, locally generated skbs on that device use more memory. So if the value drifts from what the programs push, only performance changes. The software that loads the encapsulating program knows the size and can set the value at the same time.

On manual arithmetic: user space already does this for netkit. Commit b9ed315d3c4c ("netkit: Allow for configuring needed_{head,tail}room") lets the Cilium CNI plugin compute the headroom and set it on the device.

On "cannot be undone": the IFLA_TX_EXTRA_HEADROOM UAPI does not block in-kernel mechanisms. They can write the same field later, and the UAPI stays a bounded, explicit value.

Regarding labeling for tc and BPF: it is also a new UAPI, and a label increases the value of the device that the program is attached to. That is often not the device that the skb was allocated on, as in the cases below. If we have to add a new UAPI anyway, I would prefer one that covers all of them.

I looked at what dynamic adjustment would take. The kernel can find the device to increase in these cases:

- XDP on a veth peer: veth_xdp_set() and generic_xdp_install() can increase the headroom of the peer to XDP_PACKET_HEADROOM on attach and reset it on detach.
- A TC egress program on the device that the skb was allocated on: a check around the tc egress hook catches a head reallocation, and the kernel increases the headroom of skb->dev.
- Stacked devices: a vlan leaves its tag in skb->vlan_tci, so the lower device can find the vlan with __vlan_find_dev_deep_rcu(). macvlan, ipvlan and team keep the dst.

It cannot in this case, which we have at Cloudflare:

- A service in a network namespace sends through a veth pair. The host forwards the traffic to the uplink, and a TC egress program there encapsulates it. ip_rcv_core() has already orphaned the skb, so the kernel can only guess that the veth peer of skb_iif allocated it.

Even where the kernel finds the right device, auto-tuning has problems:

- It grows too easily. Background traffic counts the same as the main traffic, so a few encapsulated health-check probes increase the headroom of every skb on the device. A threshold to filter them out would be one more policy value to choose.
- That growth can move skbs into a larger kmalloc size without anyone deciding it. On x86_64, for 1500-byte MTU UDP over IPv4 on an Ethernet device, 194 bytes or more moves the skb head from kmalloc-2048 to kmalloc-4096. That adds 2048 bytes to the memory and the truesize of each such skb. The IFLA_TX_EXTRA_HEADROOM UAPI can do the same, but then the operator chose to pay that cost.
- It only grows, unless it is reset on detach. A reset on detach must find every device that the auto-tuning increased, which can be a veth peer in another namespace.

So I prefer to keep the IFLA_TX_EXTRA_HEADROOM UAPI. If we need auto-tuning on top of it, we can discuss it when someone actually needs it.

Thanks again for pushing on this. If you see a simpler way to do this, I am happy to change direction after discussing it with David Keogh, who posted the initial RFC.

This reminds me that I have previously proposed changing both the generic XDP and veth headroom check to use 192 bytes instead of 256 bytes. This makes sense because Intel driver for native XDP already set this to 192 bytes. Exactly due to 1500 MTU packets will fit into a 2048 alignment. e.g. 2048 -192 -320 (shared_info) = 1536 bytes left.

Reading about, this would also translate into a performance benefit?

--Jesper

I agree with Willem. This is heading the wrong direction.

The more wordy AI analysis is from the iproute2 patch side:

Network configuration should not require pre-guessing this value. To set it correctly an administrator must know which TC egress programs are attached, how many bytes each pushes, and whether XDP is attached on the veth peer. That is a kernel-internal sizing decision being pushed out to user space, where the information is not reliably available. Worse, it goes stale silently: attach another encap action or move the peer to a different netns, and the value that was right at "ip link add" time is now wrong with nothing reporting it.

The reply that a stale value only costs performance is true but does not rescue the design. The entire purpose of the field is performance. A setting that quietly stops delivering the benefit it exists for is one operators carry indefinitely without knowing whether it still helps. Failing quietly in the only dimension the knob was added for is the cost, not a mitigation.

Since the motivating case is XDP, the mechanism should let the XDP program ask for the padding it needs, rather than making the operator translate the program's requirement into a device setting by hand. XDP already guarantees XDP_PACKET_HEADROOM to its programs. The requirement is known to the kernel, in the right units, at the moment of attach. Propagating it from the attach point beats an administrator's arithmetic and cannot go stale, because detach undoes it.

Your own reply already works this out:

That is the mechanism, and it needs no uapi, no operator arithmetic, and no stale state. I would rather see it implemented for the cases it covers than add a permanent uapi that removes the incentive to do so.

To be clear on what I am not asking for: this is not auto-tuning. Your objections to auto-tuning are good ones. The kmalloc-2048 to kmalloc-4096 cliff being crossed without anyone deciding to is a real problem, and the reset-on-detach bookkeeping across namespaces is real work. Attach-time propagation is a different thing: a known requirement, declared at a known point, with a natural reset.

On the argument that program-side declaration is itself new uapi, and that the labeled device is often not the device the skb was allocated on:
granted for the general TC egress case. It does not apply to the attach-time propagation above, which is in-kernel plumbing rather than uapi.

Which leaves the genuinely hard case, the one you describe at Cloudflare: a service in a netns sends through a veth pair, the host forwards to the uplink, and a TC egress program there encapsulates, after ip_rcv_core() has orphaned the skb so the allocating device cannot be identified. If that is the real residue, make it the problem being solved and argue for the narrowest mechanism that solves it. As posted, the series leads with cases that in-kernel propagation handles and uses them to justify an interface sized for the case it does not.

Hi Stephen,

Thanks for the invaluable and detailed review.

The series started from David Keogh's TC egress case, as described in the RFC [1] and in the first paragraph of the v1 cover letter. The veth XDP case came later, when I found that the same field also avoids the copy for generic XDP on veth.

Agreed. For this case, I plan to move to this in v2. When an XDP program is attached to a veth device, tx_extra_headroom of the peer is set to XDP_PACKET_HEADROOM.

For TC egress, the kernel does not know at attach time how many bytes the program pushes. Only the program, or the software that loads it, knows. Attach-time propagation works for XDP because XDP_PACKET_HEADROOM is a fixed value.

Since we both agree that auto-tuning has real problems, a narrow UAPI, such as a BPF-side declaration, seems unavoidable for TC egress. The program declares the headroom that it needs through this UAPI. The initial plan is a BTF decl_tag, as Dragos Tatulea suggested on the RFC [2]. On attach, the kernel increases tx_extra_headroom of the device that the program is attached to.

This case only came up after I started to look at in-kernel approaches, because the general IFLA_TX_EXTRA_HEADROOM UAPI already solves it. It is not a new case, and it is not uncommon. Cilium, Flannel and Calico use a tunnel by default [3][4][5], so the device that encapsulates is not the device that allocated the skb.

netkit already has a similar UAPI. Commit b9ed315d3c4c ("netkit: Allow for configuring needed_{head,tail}room") [6] by Daniel Borkmann added IFLA_NETKIT_HEADROOM for Cilium in VXLAN or Geneve tunneling mode. As you point out, the value can go stale. But the CNI plugin sets it when it creates the device, as Cilium does for netkit.

As far as I can see, a narrow UAPI, such as a veth attribute VETH_INFO_TX_EXTRA_HEADROOM, is unavoidable for this case.

Based on what I have learned from both of you, this is my plan for v2:

- Drop IFLA_TX_EXTRA_HEADROOM.
- Keep 1/8 and 2/8, i.e. tx_extra_headroom, as an in-kernel field. It is needed to avoid a perf regression if we just use needed_headroom. It is needed in all three cases below.
- veth XDP: set on attach as described above.
- TC egress on the device that allocates the skb: the BTF decl_tag UAPI described above.
- Cross-device encapsulation: the veth attribute VETH_INFO_TX_EXTRA_HEADROOM UAPI described above. I will rework the existing iproute2 series [7] to set this attribute, and post it together with v2.

This would introduce two narrow UAPIs instead of one generic UAPI. Please let me know if everyone is happy with this new plan. If there are no objections, I will proceed with v2 following this plan. I am happy to change it if there is a better solution for these cases.

To David Keogh: this plan drops the generic device attribute UAPI that your RFC proposed. Your TC egress use case stays in v2, with a BPF-side declaration instead of the device attribute. Before I proceed, please let me know if you are unhappy with this plan.

Thanks,
Tianyi

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] Cilium, veth with VXLAN by default:
https://github.com/cilium/cilium/blob/main/pkg/defaults/defaults.go
[4] Flannel, VXLAN by default:
https://github.com/flannel-io/flannel/blob/master/Documentation/kube-flannel.yml
[5] Calico, IPIP by default:
https://github.com/projectcalico/calico/blob/master/manifests/calico.yaml
[6] https://git.kernel.org/torvalds/c/b9ed315d3c4c
[7] https://lore.kernel.org/all/[email protected]/

I have some hope that if there is sufficient reason, this backlinking to the originating netdev (or socket?) can eventually be addressed as well. Rather than work around it.

The skb_orphan and iif issues are well know. But not set in stone.

Hi Willem,

Thank you for the suggestion. I looked at what backlinking would take.

For the socket, ip_rcv_core() orphans the skb, and after that skb->sk holds the receiving socket, for tproxy and early demux. Keeping the sender's socket there would break that assumption.

For the netdev, skb_iif is the last receiving device, not the allocating one. So keeping the origin needs new state in the skb, such as an skb extension.

Even with a backlink, the kernel still needs to know when to adjust tx_extra_headroom of the originating netdev. This brings back auto-tuning, with the problems from my earlier reply.

Taking a step back, IMHO it is more natural to set the extra headroom indication on the netdev that the skb is allocated for, than to have other devices raise it through a backlink, which can get messy.

For v2, I would prefer the narrow VETH_INFO_TX_EXTRA_HEADROOM UAPI, following what has been done for IFLA_NETKIT_HEADROOM. I will put it in the last two patches, so the rest of the series can be taken without it if you prefer.

Thanks,
Tianyi

As said, the problem with that is that an admin has to do it, each time. Whereas the kernel has this information.

Ok. I don't expect this to be solved in this series.

From: David Keogh <[email protected]>

Use LL_ALLOC_SPACE() and LL_ALLOC_SPACE_EX() instead of LL_RESERVED_SPACE() and LL_RESERVED_SPACE_EX() where the stack allocates packets on behalf of a local sender and sizes them for the output device:

- __ip_append_data() and __ip6_append_data(), covering UDP, ICMP, ping and raw sockets without IP_HDRINCL;
- raw_send_hdrinc() and rawv6_send_hdrinc();
- the three AF_PACKET transmit paths;
- AF_XDP copy mode, which open-codes its own calculation from needed_headroom and now uses netdev_tx_alloc_headroom().

These are the main paths where a packet's headroom is decided by the device's advertised requirement and nothing else. Adding the administrator's extra reservation here means such packets reach the TC egress hook with room for additional headers already in place.

Paths that check an existing packet against LL_RESERVED_SPACE() and expand it on demand, such as ip_finish_output2() and the tunnel transmit functions, are deliberately left alone. Using the larger value there would force a reallocation of every packet that lacks the full extra reservation, including TCP segments that already have enough room for the headers an egress program actually pushes.

ip_do_fragment() and ip6_fragment() are also left alone. They use the same value to check the headroom of existing fragments and to size new ones. A larger value would send more packets to the slow path, which copies them. A local sender's fragments normally come from __ip_append_data() or __ip6_append_data().

---
 net/ipv4/ip_output.c   | 2 +-
 net/ipv4/raw.c         | 2 +-
 net/ipv6/ip6_output.c  | 2 +-
 net/ipv6/raw.c         | 2 +-
 net/packet/af_packet.c | 6 +++---
 net/xdp/xsk.c          | 6 ++++--
 6 files changed, 11 insertions(+), 9 deletions(-)

index 3396f177a18d..bf718b8f6f1d 100644
@@ -988,7 +988,7 @@ static int __ip_append_data(struct sock *sk,
 	mtu = cork->gso_size ? IP_MAX_MTU : cork->fragsize;
 	paged = !!cork->gso_size;
 
-	hh_len = LL_RESERVED_SPACE(rt->dst.dev);
+	hh_len = LL_ALLOC_SPACE(rt->dst.dev);
 
 	fragheaderlen = sizeof(struct iphdr) + (opt ? opt->optlen : 0);
 	maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;

index 9d01006d8189..a1e72e634d0e 100644
@@ -355,7 +355,7 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
 	if (flags&MSG_PROBE)
 		goto out;
 
-	hlen = LL_RESERVED_SPACE(rt->dst.dev);
+	hlen = LL_ALLOC_SPACE(rt->dst.dev);
 	tlen = rt->dst.dev->needed_tailroom;
 	skb = sock_alloc_send_skb(sk,
 				  length + hlen + tlen + 15,

index 25fe0ba98b1a..a3626d5f0d11 100644
@@ -1490,7 +1490,7 @@ static int __ip6_append_data(struct sock *sk,
 	mtu = cork->gso_size ? IP6_MAX_MTU : cork->fragsize;
 	orig_mtu = mtu;
 
-	hh_len = LL_RESERVED_SPACE(rt->dst.dev);
+	hh_len = LL_ALLOC_SPACE(rt->dst.dev);
 
 	fragheaderlen = sizeof(struct ipv6hdr) + rt->rt6i_nfheader_len +
 			(opt ? opt->opt_nflen : 0);

index bdea825434ea..06459685ec61 100644
@@ -601,7 +601,7 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length,
 	struct sk_buff *skb;
 	int err;
 	struct rt6_info *rt = dst_rt6_info(*dstp);
-	int hlen = LL_RESERVED_SPACE(rt->dst.dev);
+	int hlen = LL_ALLOC_SPACE(rt->dst.dev);
 	int tlen = rt->dst.dev->needed_tailroom;
 
 	if (length > rt->dst.dev->mtu) {

index 8250e6a791f6..8800da612880 100644
@@ -2032,7 +2032,7 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
 		goto out_unlock;
 
 	if (!skb) {
-		size_t reserved = LL_RESERVED_SPACE_EX(dev, hard_header_len);
+		size_t reserved = LL_ALLOC_SPACE_EX(dev, hard_header_len);
 		int tlen = dev->needed_tailroom;
 		unsigned int hhlen = dev->header_ops ? hard_header_len : 0;
 

@@ -2860,7 +2860,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
 			goto tpacket_error;
 
 		status = TP_STATUS_SEND_REQUEST;
-		hlen = LL_RESERVED_SPACE_EX(dev, hard_header_len);
+		hlen = LL_ALLOC_SPACE_EX(dev, hard_header_len);
 		tlen = dev->needed_tailroom;
 		if (vnet_hdr_sz) {
 			data += vnet_hdr_sz;

@@ -3087,7 +3087,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
 		goto out_unlock;
 
 	err = -ENOBUFS;
-	hlen = LL_RESERVED_SPACE_EX(dev, hard_header_len);
+	hlen = LL_ALLOC_SPACE_EX(dev, hard_header_len);
 	tlen = dev->needed_tailroom;
 	linear = __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len);
 	linear = max(linear, min_t(int, len, hard_header_len));

index 781ba46eb152..3ee8ad3fd155 100644
@@ -1045,7 +1045,8 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
 	buffer = xsk_buff_raw_get_data(pool, addr);
 
 	if (!skb) {
-		hr = max(NET_SKB_PAD, L1_CACHE_ALIGN(xs->dev->needed_headroom));
+		hr = max(NET_SKB_PAD,
+			 L1_CACHE_ALIGN(netdev_tx_alloc_headroom(xs->dev)));
 
 		skb = sock_alloc_send_skb(&xs->sk, hr, 1, &err);
 		if (unlikely(!skb))

@@ -1128,7 +1129,8 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
 		len = desc->len;
 
 		if (!skb) {
-			hr = max(NET_SKB_PAD, L1_CACHE_ALIGN(dev->needed_headroom));
+			hr = max(NET_SKB_PAD,
+				 L1_CACHE_ALIGN(netdev_tx_alloc_headroom(dev)));
 			tr = dev->needed_tailroom;
 			skb = sock_alloc_send_skb(&xs->sk, hr + len + tr, 1, &err);
 			if (unlikely(!skb))

2.56.0

[ ... ]

This is a pre-existing issue, not introduced by this series.

Jason already planned to fix it [2][3], but it has been five months.

Jason, are you still planning to send the fix? If not, I can send it to net.

[2] https://lore.kernel.org/all/CAL+tcoCp19F2rNxYL_LoGsY5MuBZ6cGKSZsj6D7kvQZcYp0wJg@mail.gmail.com/
[3] https://lore.kernel.org/all/CAL+tcoCgXG5MdPVzXavwMwk=jmPXgcB5aKpLK+TNXMVrsGmS9w@mail.gmail.com/

From: David Keogh <[email protected]>

Expose tx_extra_headroom through a new IFLA_TX_EXTRA_HEADROOM u32 attribute. The value is bounded to NETDEV_TX_EXTRA_HEADROOM_MAX (512 bytes) by the netlink policy, which is plenty for several layers of encapsulation while keeping the growth of per-packet allocations modest.

The attribute can be given when creating a link and when changing one. It is included in link dumps and a change triggers the usual link notification when the device is up. Zero is the default and resets the reservation.

IFLA_HEADROOM keeps reporting needed_headroom, the driver's own requirement, so userspace can tell the two apart.

The setting applies to the device the packet is allocated for, that is the device selected by the route, and is not inherited by stacked devices. Document that together with the allocation-only semantics in netdevices.rst and add the attribute to the rt-link spec.

---
IFLA_TX_EXTRA_HEADROOM is a u32 attribute, although the maximum value (NETDEV_TX_EXTRA_HEADROOM_MAX) is 512. Documentation/userspace-api/netlink/specs.rst says to avoid types smaller than 32 bits. Netlink aligns each attribute to 4 bytes, so a smaller type does not save memory. The dev->tx_extra_headroom field stays unsigned short. IFLA_GSO_MAX_SEGS and the u16 dev->gso_max_segs do the same.

---
 Documentation/netlink/specs/rt-link.yaml | 12 ++++++++
 Documentation/networking/netdevices.rst  | 35 ++++++++++++++++++++++++
 include/uapi/linux/if_link.h             |  1 +
 net/core/rtnetlink.c                     | 19 ++++++++++++-
 4 files changed, 66 insertions(+), 1 deletion(-)

index 81b58633911d..fb76410abd5b 100644
@@ -1098,6 +1098,16 @@ attribute-sets:
         name: pacing-offload
         type: u32
         doc: Enable EDT pacing offload (0 - disabled, 1 - enabled).
+      -
+        name: tx-extra-headroom
+        type: u32
+        doc: |
+          Additional headroom, in bytes, reserved on top of the driver's
+          requirement when the UDP, raw, AF_PACKET and AF_XDP copy mode transmit
+          paths allocate a packet for this device. TCP and forwarded packets are
+          not covered. Zero (the default) disables the extra reservation.
+        checks:
+          max: 512
   -
     name: prop-list-link-attrs
     subset-of: link-attrs

@@ -2471,6 +2481,7 @@ operations:
             - gso-ipv4-max-size
             - gro-ipv4-max-size
             - af-spec
+            - tx-extra-headroom
     -
       name: newlink-ntf
       doc: Notify that a link has been created

@@ -2567,6 +2578,7 @@ operations:
             - gso-ipv4-max-size
             - gro-ipv4-max-size
             - pacing-offload
+            - tx-extra-headroom
       dump:
         request:
           value: 18

index 6a87f09e7015..da3556e56cc2 100644
@@ -441,6 +441,41 @@ from a notifier.
 
 Drivers **must not** generate nested notifications of the ops-locked types.
 
+TX headroom reservation
+=======================
+
+``needed_headroom`` is the driver's own additional header-space requirement.
+It is written by the driver, or by a stacked device inheriting the value of
+the device below it, and is reported read-only through ``IFLA_HEADROOM``.
+
+``tx_extra_headroom`` is a separate reservation configured by the
+administrator through the ``IFLA_TX_EXTRA_HEADROOM`` rtnetlink attribute, a
+byte count from 0 to 512 that defaults to 0. It can be supplied when a link
+is created or changed, is reported in link dumps and change notifications,
+and setting it back to 0 removes it.
+
+The extra reservation is only applied when the stack allocates a locally
+generated packet for transmission through the device. ``LL_ALLOC_SPACE()``
+adds ``tx_extra_headroom`` before ``LL_RESERVED_SPACE()`` aligns the sum to
+``HH_DATA_MOD``, and is used by the UDP, raw and AF_PACKET transmit paths.
+AF_XDP copy mode adds it through ``netdev_tx_alloc_headroom()``. Its purpose
+is to give programs attached at TC egress room to push additional headers
+without having to reallocate the packet first.
+
+The setting is deliberately not part of ``LL_RESERVED_SPACE()``, which
+remains the threshold for deciding whether an already existing packet must
+be expanded. Packets that reach the device with less than the extra
+reservation, such as forwarded packets or TCP segments, are therefore not
+reallocated because of it. Consumers must keep their usual headroom and
+clone checks.
+
+The setting is not inherited through stacked devices. Packets are allocated
+against the device selected by the route, so configure it on the VLAN,
+bond or bridge device the route points at rather than on the device
+beneath it. It survives link down/up and network namespace moves and is
+independent of any TC or BPF attachment. It does not change the MTU or
+the bytes on the wire.
+
 NETDEV_INTERNAL symbol namespace
 ================================
 

index 245b36204525..fb3a6b382a78 100644
@@ -382,6 +382,7 @@ enum {
 	IFLA_HEADROOM,
 	IFLA_TAILROOM,
 	IFLA_PACING_OFFLOAD,
+	IFLA_TX_EXTRA_HEADROOM,
 	__IFLA_MAX
 };
 

index e141340172ce..16ee1a874107 100644
@@ -1409,6 +1409,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(4)  /* IFLA_PACING_OFFLOAD */
 	       + nla_total_size(2)  /* IFLA_HEADROOM */
 	       + nla_total_size(2)  /* IFLA_TAILROOM */
+	       + nla_total_size(4)  /* IFLA_TX_EXTRA_HEADROOM */
 	       + rtnl_dev_parent_size(dev)
 	       + 0;
 

@@ -2206,7 +2207,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
 	    nla_put_u16(skb, IFLA_HEADROOM,
 			READ_ONCE(dev->needed_headroom)) ||
 	    nla_put_u16(skb, IFLA_TAILROOM,
-			READ_ONCE(dev->needed_tailroom)))
+			READ_ONCE(dev->needed_tailroom)) ||
+	    nla_put_u32(skb, IFLA_TX_EXTRA_HEADROOM,
+			READ_ONCE(dev->tx_extra_headroom)))
 		goto nla_put_failure;
 
 	if (rtnl_fill_proto_down(skb, dev))

@@ -2367,6 +2370,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_HEADROOM]		= { .type = NLA_REJECT },
 	[IFLA_TAILROOM]		= { .type = NLA_REJECT },
 	[IFLA_PACING_OFFLOAD]	= NLA_POLICY_MAX(NLA_U32, 1),
+	[IFLA_TX_EXTRA_HEADROOM] =
+		NLA_POLICY_MAX(NLA_U32, NETDEV_TX_EXTRA_HEADROOM_MAX),
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {

@@ -3374,6 +3379,15 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
 		}
 	}
 
+	if (tb[IFLA_TX_EXTRA_HEADROOM]) {
+		u32 val = nla_get_u32(tb[IFLA_TX_EXTRA_HEADROOM]);
+
+		if (dev->tx_extra_headroom != val) {
+			WRITE_ONCE(dev->tx_extra_headroom, val);
+			status |= DO_SETLINK_NOTIFY;
+		}
+	}
+
 	if (tb[IFLA_OPERSTATE])
 		set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
 

@@ -3865,6 +3879,9 @@ struct net_device *rtnl_create_link(struct net *net, const char *ifname,
 		netif_set_gso_ipv4_max_size(dev, nla_get_u32(tb[IFLA_GSO_IPV4_MAX_SIZE]));
 	if (tb[IFLA_GRO_IPV4_MAX_SIZE])
 		netif_set_gro_ipv4_max_size(dev, nla_get_u32(tb[IFLA_GRO_IPV4_MAX_SIZE]));
+	if (tb[IFLA_TX_EXTRA_HEADROOM])
+		dev->tx_extra_headroom =
+			nla_get_u32(tb[IFLA_TX_EXTRA_HEADROOM]);
 
 	return dev;
 }

2.56.0

From: David Keogh <[email protected]>

Exercise the IFLA_TX_EXTRA_HEADROOM attribute through YNL: the default of zero, setting values up to the maximum and back to zero, rejection of a value above the maximum, supplying the attribute at link creation, the change notification, persistence across link down/up and across a network namespace move, and that a VLAN created on top of a configured device does not inherit the setting. It also checks that setting the attribute does not change needed_headroom. On a kernel without the attribute, every case is skipped.

---
 tools/testing/selftests/net/Makefile          |   1 +
 .../selftests/net/tx_extra_headroom.py        | 147 ++++++++++++++++++
 2 files changed, 148 insertions(+)
 create mode 100755 tools/testing/selftests/net/tx_extra_headroom.py

index 9180b9bec515..4a931c010762 100644
@@ -121,6 +121,7 @@ TEST_PROGS := \
 	test_vxlan_vnifiltering.sh \
 	tfo_passive.sh \
 	traceroute.sh \
+	tx_extra_headroom.py \
 	txtimestamp.sh \
 	udpgro.sh \
 	udpgro_bench.sh \

2.56.0

The tx_extra_headroom test only checks the IFLA_TX_EXTRA_HEADROOM attribute itself. Add test cases for its effect on UDP over IPv4 packets.

tx_extra_headroom.bpf.c is a TC egress program. It matches the UDP test packet by destination port and records its headroom. It then pushes 20 bytes after the MAC header with bpf_skb_adjust_room(). It records whether skb->head changed. A changed skb->head means that pskb_expand_head() reallocated the packet.

tx_extra_headroom.py is extended to create a veth pair. veth0 sends, and its peer is in a second netns. The program runs at the TC egress of veth0. Each case sets tx_extra_headroom on veth0, sends one UDP packet and checks:

tx_extra_headroom  headroom  push reallocates
0                  2         yes
17                 18        yes
18                 34        no
256                258       no

The headroom is LL_ALLOC_SPACE() minus the 14-byte Ethernet header. LL_ALLOC_SPACE() aligns to 16 bytes. So the headroom steps from 18 to 34 when tx_extra_headroom goes from 17 to 18. The test checks these exact values. HH_DATA_MOD and the rounding in LL_RESERVED_SPACE() have not changed since the beginning of git history (Linux 2.6.12-rc2, 2005).

---
The tc program follows tools/testing/selftests/bpf/progs/test_dst_clear.c.

---
 .../selftests/net/tx_extra_headroom.bpf.c     | 70 ++++++++++++++++++
 .../selftests/net/tx_extra_headroom.py        | 74 ++++++++++++++++++-
 2 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/tx_extra_headroom.bpf.c

new file mode 100644
index 000000000000..e74f44afbe56
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Record the headroom of a test packet and whether a push reallocates it. */
+#include <stdbool.h>
+#include <linux/bpf.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/pkt_cls.h>
+#include <linux/udp.h>
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+
+#define UDP_TEST_PORT 9999
+#define PUSH_LEN 20
+
+struct sk_buff {
+	unsigned char *head;
+	unsigned char *data;
+} __attribute__((preserve_access_index));
+
+void *bpf_cast_to_kern_ctx(void *obj) __ksym;
+
+__u32 count;
+__u32 headroom;
+int err;
+bool reallocated;
+
+static bool is_test_packet(struct __sk_buff *skb)
+{
+	struct udphdr udph;
+	struct iphdr iph;
+
+	if (skb->protocol != bpf_htons(ETH_P_IP))
+		return false;
+
+	if (bpf_skb_load_bytes(skb, ETH_HLEN, &iph, sizeof(iph)))
+		return false;
+
+	if (iph.protocol != IPPROTO_UDP)
+		return false;
+
+	if (bpf_skb_load_bytes(skb, ETH_HLEN + sizeof(iph), &udph,
+			       sizeof(udph)))
+		return false;
+
+	return udph.dest == bpf_htons(UDP_TEST_PORT);
+}
+
+SEC("tc/egress")
+int push(struct __sk_buff *skb)
+{
+	struct sk_buff *kskb;
+	unsigned char *head;
+
+	if (!is_test_packet(skb))
+		return TC_ACT_OK;
+
+	kskb = bpf_cast_to_kern_ctx(skb);
+	count++;
+	headroom = kskb->data - kskb->head;
+
+	/* pskb_expand_head() always allocates a new skb->head. */
+	head = kskb->head;
+	err = bpf_skb_adjust_room(skb, PUSH_LEN, BPF_ADJ_ROOM_MAC, 0);
+	reallocated = kskb->head != head;
+
+	return TC_ACT_SHOT;
+}
+
+char __license[] SEC("license") = "GPL";

2.56.0

xdp_helper only binds an XDP socket and waits. Tests in selftests/net have no way to send a frame through the AF_XDP TX path.

Add a -t option that takes one Ethernet frame in hex. With -t, the helper also creates a TX ring and sends the frame. It then waits for the frame on the completion ring and exits. It returns 1 if sendto() fails or no completion arrives within one second. Without -t, nothing changes.

The test passes the whole frame in hex, so xdp_helper does not need to know about IP or UDP. veth has no AF_XDP zerocopy support, so the socket runs in copy mode. In copy mode the kernel allocates an skb for the frame in xsk_build_skb(), which uses the extra headroom.

xdp_helper now maps the completion ring at XDP_UMEM_PGOFF_COMPLETION_RING (0x180000000). This offset does not fit in a 32-bit off_t, so define _FILE_OFFSET_BITS to 64.

---
 tools/testing/selftests/net/lib/xdp_helper.c | 113 +++++++++++++++++--
 1 file changed, 106 insertions(+), 7 deletions(-)

index eb025a9f35b1..5f4dec4408d9 100644
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#define _FILE_OFFSET_BITS 64
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>

@@ -19,15 +20,91 @@
 
 static void print_usage(const char *bin)
 {
-	fprintf(stderr, "Usage: %s ifindex queue_id [-z]\n\n"
-		"where:\n\t-z: force zerocopy mode", bin);
+	fprintf(stderr, "Usage: %s ifindex queue_id [-z] [-t frame_hex]\n\n"
+		"where:\n\t-z: force zerocopy mode\n"
+		"\t-t: send one frame, given in hex, and exit\n", bin);
+}
+
+static int parse_frame(unsigned char *buf, const char *hex)
+{
+	size_t len = strlen(hex) / 2;
+	size_t i;
+
+	if (!len || len > 2048 || strlen(hex) % 2 ||
+	    strspn(hex, "0123456789abcdefABCDEF") != strlen(hex))
+		return -1;
+	for (i = 0; i < len; i++)
+		if (sscanf(hex + 2 * i, "%2hhx", &buf[i]) != 1)
+			return -1;
+	return len;
+}
+
+/* Send the frame at UMEM address 0 and wait until its descriptor shows up
+ * on the completion ring. In copy mode this happens when the skb is freed.
+ */
+static int send_frame(int sock_fd, __u32 len)
+{
+	struct xdp_mmap_offsets off;
+	socklen_t optlen = sizeof(off);
+	struct xdp_desc *desc;
+	size_t tx_sz, cq_sz;
+	char *tx, *cq;
+	int ret = 1;
+	int i;
+
+	if (getsockopt(sock_fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen)) {
+		perror("getsockopt failed");
+		return 1;
+	}
+
+	tx_sz = off.tx.desc + NUM_DESC * sizeof(*desc);
+	tx = mmap(NULL, tx_sz, PROT_READ | PROT_WRITE, MAP_SHARED, sock_fd,
+		  XDP_PGOFF_TX_RING);
+	if (tx == MAP_FAILED) {
+		perror("TX ring mmap failed");
+		return 1;
+	}
+
+	cq_sz = off.cr.desc + NUM_DESC * sizeof(__u64);
+	cq = mmap(NULL, cq_sz, PROT_READ | PROT_WRITE, MAP_SHARED, sock_fd,
+		  XDP_UMEM_PGOFF_COMPLETION_RING);
+	if (cq == MAP_FAILED) {
+		perror("completion ring mmap failed");
+		goto out_tx;
+	}
+
+	desc = (struct xdp_desc *)(tx + off.tx.desc);
+	desc->addr = 0;
+	desc->len = len;
+	__atomic_store_n((__u32 *)(tx + off.tx.producer), 1, __ATOMIC_RELEASE);
+
+	if (sendto(sock_fd, NULL, 0, MSG_DONTWAIT, NULL, 0) < 0) {
+		perror("sendto failed");
+		goto out;
+	}
+
+	for (i = 0; i < 100; i++) {
+		usleep(10000);
+		if (__atomic_load_n((__u32 *)(cq + off.cr.producer),
+				    __ATOMIC_ACQUIRE)) {
+			ret = 0;
+			goto out;
+		}
+	}
+
+	fprintf(stderr, "no TX completion\n");
+out:
+	munmap(cq, cq_sz);
+out_tx:
+	munmap(tx, tx_sz);
+	return ret;
 }
 
 /* this is a simple helper program that creates an XDP socket and does the
  * minimum necessary to get bind() to succeed.
  *
- * this test program is not intended to actually process packets, but could be
- * extended in the future if that is actually needed.
+ * without -t it does not process packets. with -t it sends one frame
+ * through the TX ring and exits once the frame completes.
  *
  * it is used by queues.py to ensure the xsk netlinux attribute is set
  * correctly.

@@ -37,13 +114,16 @@ int main(int argc, char **argv)
 	struct xdp_umem_reg umem_reg = { 0 };
 	struct sockaddr_xdp sxdp = { 0 };
 	int num_desc = NUM_DESC;
+	int frame_len = 0;
 	void *umem_area;
 	int retry = 0;
 	int ifindex;
 	int sock_fd;
 	int queue;
+	int ret;
+	int i;
 
-	if (argc != 3 && argc != 4) {
+	if (argc < 3) {
 		print_usage(argv[0]);
 		return 1;
 	}

@@ -94,9 +174,21 @@ int main(int argc, char **argv)
 	sxdp.sxdp_queue_id = queue;
 	sxdp.sxdp_flags = 0;
 
-	if (argc > 3) {
-		if (!strcmp(argv[3], "-z")) {
+	for (i = 3; i < argc; i++) {
+		if (!strcmp(argv[i], "-z")) {
 			sxdp.sxdp_flags = XDP_ZEROCOPY;
+		} else if (!strcmp(argv[i], "-t") && i + 1 < argc &&
+			   !frame_len) {
+			frame_len = parse_frame(umem_area, argv[++i]);
+			if (frame_len < 0) {
+				print_usage(argv[0]);
+				return 1;
+			}
+			if (setsockopt(sock_fd, SOL_XDP, XDP_TX_RING, &num_desc,
+				       sizeof(num_desc))) {
+				perror("setsockopt XDP_TX_RING failed");
+				return 1;
+			}
 		} else {
 			print_usage(argv[0]);
 			return 1;

@@ -119,6 +211,13 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (frame_len) {
+		ret = send_frame(sock_fd, frame_len);
+		munmap(umem_area, UMEM_SZ);
+		close(sock_fd);
+		return ret;
+	}
+
 	ksft_ready();
 	ksft_wait();
 

2.56.0

Generic XDP copies an skb with less than XDP_PACKET_HEADROOM (256 bytes) of headroom. With tx_extra_headroom set to 256 on a veth device, a locally generated packet has enough headroom, and generic XDP on the peer no longer copies it.

Test this for six senders. Each one allocates the skb in a different
place: UDP over IPv4 and IPv6, raw sockets with IPPROTO_RAW over IPv4 and IPv6, AF_PACKET and AF_XDP in copy mode. xdp_helper -t sends the AF_XDP frame. The peer runs xdp_dummy in generic mode.

Extend tx_extra_headroom.bpf.c with a TC ingress program for the peer. It records the headroom of the test packet. On the receive path, it runs after generic XDP.

Each case sends one packet without xdp_dummy on the peer. Then it attaches xdp_dummy and sends a second packet. Both packets are allocated with the same headroom.

If the second packet has a different headroom from the first, it means generic XDP copied it. With tx_extra_headroom 0, the two packets have different headroom for all six senders. With tx_extra_headroom 256, they have the same headroom.

---
Native XDP on veth also copies an skb whose head is not a page fragment, so it still copies these packets with 256. This test does not cover it.

tpacket_snd() and packet_sendmsg_spkt() get the same change as packet_snd(), and xsk_build_skb_zerocopy() the same as xsk_build_skb(). They are not tested.

---
 tools/testing/selftests/net/config            |   1 +
 .../selftests/net/tx_extra_headroom.bpf.c     |  44 +++++--
 .../selftests/net/tx_extra_headroom.py        | 107 +++++++++++++++++-
 3 files changed, 138 insertions(+), 14 deletions(-)

index 23b425c12db5..1c13d028edff 100644
@@ -143,5 +143,6 @@ CONFIG_USER_NS=y
 CONFIG_VETH=y
 CONFIG_VLAN_8021Q=y
 CONFIG_VXLAN=m
+CONFIG_XDP_SOCKETS=y
 CONFIG_XFRM_INTERFACE=m
 CONFIG_XFRM_USER=m

2.56.0

veth_newlink() parses the attributes of the peer from VETH_INFO_PEER and passes them to rtnl_create_link(). So one request can set IFLA_TX_EXTRA_HEADROOM on both ends of a veth pair. test_newlink() only covers a dummy device.

Create a veth pair with ip and give each end a different value. Then read both values back with YNL. With a different value on each end, the test fails if one end gets the value of the other end. The YNL rt-link spec does not describe veth, so ip creates the pair. Skip the test if ip does not support tx_extra_headroom.

---
The test needs the iproute2 patch "iplink: add tx_extra_headroom attribute", which is sent separately to iproute2-next. With an older ip, the test is skipped.

---
 tools/testing/selftests/net/tx_extra_headroom.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

index f94eb29a25d9..8a2284f1b343 100755
@@ -141,6 +141,19 @@ def test_not_inherited() -> None:
                 4, "vlan needed_headroom")
 
 
+def test_veth_peer_at_creation() -> None:
+    """ip can set a different value on each end of a new veth pair."""
+    if "tx_extra_headroom" not in cmd("ip link help", fail=False).stderr:
+        raise KsftSkipEx("iproute2 does not support tx_extra_headroom")
+
+    with NetNS() as ns, NetNSEnter(str(ns)):
+        ip("link add veth0 tx_extra_headroom 256 type veth "
+           "peer name veth1 tx_extra_headroom 128")
+        rtnl = RtnlFamily()
+        ksft_eq(get(rtnl, ifindex("veth0")), 256)
+        ksft_eq(get(rtnl, ifindex("veth1")), 128)
+
+
 def setup_veth(peer_ns):
     """Create veth0 in the current netns and its peer veth1 in peer_ns."""
     ip("link add veth0 type veth peer name veth1")

@@ -307,6 +320,7 @@ def main() -> None:
               test_survives_down_up,
               test_survives_netns_move,
               test_not_inherited,
+              test_veth_peer_at_creation,
               test_push,
               test_peer_xdp])
     ksft_exit()

2.56.0

Hi Jakub,

Thank you for the comment. I will use Python YNL for v2, with no dependency on iproute2.

Cheers,
Tianyi