Tianyi Gao wrote:
From: David Keogh <redacted>
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,
Main high level concern is just adding an extra field. What's next,
do we need to add a field extra_extra_headroom?
together with LL_ALLOC_SPACE() and LL_ALLOC_SPACE_EX(). These add the
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.
quoted hunk ↗ jump to hunk
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.
Signed-off-by: David Keogh <redacted>
[tianyi: remove LL_RESERVED_SPACE_EXTRA()]
Signed-off-by: Tianyi Gao <redacted>
---
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(-)
diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index 8eceaa80b686..a7e311fb7875 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -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
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 435be7880d7e..98d1501d6fee 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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)
+
If having to modify/add these helpers, it's probably a good time to
switch to actual, type safe and readable, functions instead.