diff options
| author | Eric Dumazet <edumazet@google.com> | 2025-11-21 06:17:25 +0000 |
|---|---|---|
| committer | Jakub Kicinski <kuba@kernel.org> | 2025-11-24 19:27:31 -0800 |
| commit | ec1e48e97febbf7f87fc150793af2eda7223815c (patch) | |
| tree | b34dae58d71044b81587b6083c86fdb5d8cda3e5 /net/ethernet | |
| parent | e254c212cd9cabb9469a1282c4c91ccc6180bf51 (diff) | |
net: optimize eth_type_trans() vs CONFIG_STACKPROTECTOR_STRONG=y
Some platforms exhibit very high costs with CONFIG_STACKPROTECTOR_STRONG=y
when a function needs to pass the address of a local variable to external
functions.
eth_type_trans() (and its callers) is showing this anomaly on AMD EPYC 7B12
platforms (and maybe others).
We could :
1) inline eth_type_trans()
This would help if its callers also has the same issue, and the canary cost
would be paid by the callers already.
This is a bit cumbersome because netdev_uses_dsa() is pulling
whole <net/dsa.h> definitions.
2) Compile net/ethernet/eth.c with -fno-stack-protector
This would weaken security.
3) Hack eth_type_trans() to temporarily use skb->dev as a place holder
if skb_header_pointer() needs to pull 2 bytes not present in skb->head.
This patch implements 3), and brings a 5% improvement on TX/RX intensive
workload (tcp_rr 10,000 flows) on AMD EPYC 7B12.
Removing CONFIG_STACKPROTECTOR_STRONG on this platform can improve
performance by 25 %.
This means eth_type_trans() issue is not an isolated artifact.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Link: https://patch.msgid.link/20251121061725.206675-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Diffstat (limited to 'net/ethernet')
| -rw-r--r-- | net/ethernet/eth.c | 16 |
1 files changed, 8 insertions, 8 deletions
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index 43e211e611b1..13a63b48b7ee 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -154,9 +154,9 @@ EXPORT_SYMBOL(eth_get_headlen); */ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) { - unsigned short _service_access_point; const unsigned short *sap; const struct ethhdr *eth; + __be16 res; skb->dev = dev; skb_reset_mac_header(skb); @@ -181,15 +181,15 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) * the protocol design and runs IPX over 802.3 without an 802.2 LLC * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This * won't work for fault tolerant netware but does for the rest. + * We use skb->dev as temporary storage to not hit + * CONFIG_STACKPROTECTOR_STRONG=y costs on some platforms. */ - sap = skb_header_pointer(skb, 0, sizeof(*sap), &_service_access_point); - if (sap && *sap == 0xFFFF) - return htons(ETH_P_802_3); + sap = skb_header_pointer(skb, 0, sizeof(*sap), &skb->dev); + res = (sap && *sap == 0xFFFF) ? htons(ETH_P_802_3) : htons(ETH_P_802_2); - /* - * Real 802.2 LLC - */ - return htons(ETH_P_802_2); + /* restore skb->dev in case it was mangled by skb_header_pointer(). */ + skb->dev = dev; + return res; } EXPORT_SYMBOL(eth_type_trans); |