tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'net-bridge-add-skb-drop-reasons-to-the-most-common-drop-points'

Radu Rendec says:

====================
net/bridge: Add skb drop reasons to the most common drop points

The bridge input code may drop frames for various reasons and at various
points in the ingress handling logic. Currently kfree_skb() is used
everywhere, and therefore no drop reason is specified. Add drop reasons
to the most common drop points.

The purpose of this series is to address the most common drop points on
the bridge ingress path. It does not exhaustively add drop reasons to
the entire bridge code. The intention here is to incrementally add drop
reasons to the rest of the bridge code in follow up patches.

Most of the skb drop points that are addressed in this series can be
easily tested by sending crafted packets. The diagram below shows a
simple test configuration, and some examples using `packit`(*) are
also included. The bridge is set up with STP disabled.
(*) https://github.com/resurrecting-open-source-projects/packit

The following changes were *not* tested:
* SKB_DROP_REASON_NOMEM in br_flood(). It's not easy to trigger an OOM
condition for testing purposes, while everything else works correctly.
* All drop reasons in br_multicast_flood(). I could not find an easy way
to make a crafted packet get there.
* SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE in br_handle_frame_finish()
when the port state is BR_STATE_DISABLED, because in that case the
frame is already dropped in the switch/case block at the end of
br_handle_frame().

+-------+
| br0 |
+---+---+
|
+---+---+ veth pair +-------+
| veth0 +-------------+ xeth0 |
+-------+ +-------+

SKB_DROP_REASON_MAC_INVALID_SOURCE - br_handle_frame()
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
-e 01:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
-p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL - br_handle_frame()
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
-e 02:22:33:44:55:66 -E 01:80:c2:00:00:01 -c 1 \
-p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE - br_handle_frame()
bridge link set dev veth0 state 0 # disabled
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
-e 02:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
-p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE - br_handle_frame_finish()
bridge link set dev veth0 state 2 # learning
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
-e 02:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
-p '0x de ad be ef' -i xeth0

SKB_DROP_REASON_NO_TX_TARGET - br_flood()
packit -t UDP -s 192.168.0.1 -d 192.168.0.2 -S 8000 -D 8000 \
-e 02:22:33:44:55:66 -E aa:bb:cc:dd:ee:ff -c 1 \
-p '0x de ad be ef' -i xeth0
====================

Link: https://patch.msgid.link/20241219163606.717758-1-rrendec@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Jakub Kicinski a6ac6674 6ad7f719

+45 -15
+2 -2
drivers/net/vxlan/vxlan_core.c
··· 2798 2798 dev_dstats_tx_dropped(dev); 2799 2799 vxlan_vnifilter_count(vxlan, vni, NULL, 2800 2800 VXLAN_VNI_STATS_TX_DROPS, 0); 2801 - kfree_skb_reason(skb, SKB_DROP_REASON_VXLAN_NO_REMOTE); 2801 + kfree_skb_reason(skb, SKB_DROP_REASON_NO_TX_TARGET); 2802 2802 return NETDEV_TX_OK; 2803 2803 } 2804 2804 } ··· 2821 2821 if (fdst) 2822 2822 vxlan_xmit_one(skb, dev, vni, fdst, did_rsc); 2823 2823 else 2824 - kfree_skb_reason(skb, SKB_DROP_REASON_VXLAN_NO_REMOTE); 2824 + kfree_skb_reason(skb, SKB_DROP_REASON_NO_TX_TARGET); 2825 2825 } 2826 2826 2827 2827 return NETDEV_TX_OK;
+1 -1
drivers/net/vxlan/vxlan_mdb.c
··· 1712 1712 vxlan_xmit_one(skb, vxlan->dev, src_vni, 1713 1713 rcu_dereference(fremote->rd), false); 1714 1714 else 1715 - kfree_skb_reason(skb, SKB_DROP_REASON_VXLAN_NO_REMOTE); 1715 + kfree_skb_reason(skb, SKB_DROP_REASON_NO_TX_TARGET); 1716 1716 1717 1717 return NETDEV_TX_OK; 1718 1718 }
+15 -3
include/net/dropreason-core.h
··· 106 106 FN(VXLAN_VNI_NOT_FOUND) \ 107 107 FN(MAC_INVALID_SOURCE) \ 108 108 FN(VXLAN_ENTRY_EXISTS) \ 109 - FN(VXLAN_NO_REMOTE) \ 109 + FN(NO_TX_TARGET) \ 110 110 FN(IP_TUNNEL_ECN) \ 111 111 FN(TUNNEL_TXINFO) \ 112 112 FN(LOCAL_MAC) \ 113 113 FN(ARP_PVLAN_DISABLE) \ 114 + FN(MAC_IEEE_MAC_CONTROL) \ 115 + FN(BRIDGE_INGRESS_STP_STATE) \ 114 116 FNe(MAX) 115 117 116 118 /** ··· 499 497 * entry or an entry pointing to a nexthop. 500 498 */ 501 499 SKB_DROP_REASON_VXLAN_ENTRY_EXISTS, 502 - /** @SKB_DROP_REASON_VXLAN_NO_REMOTE: no remote found for xmit */ 503 - SKB_DROP_REASON_VXLAN_NO_REMOTE, 500 + /** @SKB_DROP_REASON_NO_TX_TARGET: no target found for xmit */ 501 + SKB_DROP_REASON_NO_TX_TARGET, 504 502 /** 505 503 * @SKB_DROP_REASON_IP_TUNNEL_ECN: skb is dropped according to 506 504 * RFC 6040 4.2, see __INET_ECN_decapsulate() for detail. ··· 522 520 * enabled. 523 521 */ 524 522 SKB_DROP_REASON_ARP_PVLAN_DISABLE, 523 + /** 524 + * @SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL: the destination MAC address 525 + * is an IEEE MAC Control address. 526 + */ 527 + SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL, 528 + /** 529 + * @SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE: the STP state of the 530 + * ingress bridge port does not allow frames to be forwarded. 531 + */ 532 + SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE, 525 533 /** 526 534 * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which 527 535 * shouldn't be used as a real 'reason' - only for tracing code gen
+12 -4
net/bridge/br_forward.c
··· 201 201 enum br_pkt_type pkt_type, bool local_rcv, bool local_orig, 202 202 u16 vid) 203 203 { 204 + enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET; 204 205 struct net_bridge_port *prev = NULL; 205 206 struct net_bridge_port *p; 206 207 ··· 235 234 continue; 236 235 237 236 prev = maybe_deliver(prev, p, skb, local_orig); 238 - if (IS_ERR(prev)) 237 + if (IS_ERR(prev)) { 238 + reason = PTR_ERR(prev) == -ENOMEM ? SKB_DROP_REASON_NOMEM : 239 + SKB_DROP_REASON_NOT_SPECIFIED; 239 240 goto out; 241 + } 240 242 } 241 243 242 244 if (!prev) ··· 253 249 254 250 out: 255 251 if (!local_rcv) 256 - kfree_skb(skb); 252 + kfree_skb_reason(skb, reason); 257 253 } 258 254 259 255 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING ··· 293 289 struct net_bridge_mcast *brmctx, 294 290 bool local_rcv, bool local_orig) 295 291 { 292 + enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET; 296 293 struct net_bridge_port *prev = NULL; 297 294 struct net_bridge_port_group *p; 298 295 bool allow_mode_include = true; ··· 334 329 } 335 330 336 331 prev = maybe_deliver(prev, port, skb, local_orig); 337 - if (IS_ERR(prev)) 332 + if (IS_ERR(prev)) { 333 + reason = PTR_ERR(prev) == -ENOMEM ? SKB_DROP_REASON_NOMEM : 334 + SKB_DROP_REASON_NOT_SPECIFIED; 338 335 goto out; 336 + } 339 337 delivered: 340 338 if ((unsigned long)lport >= (unsigned long)port) 341 339 p = rcu_dereference(p->next); ··· 357 349 358 350 out: 359 351 if (!local_rcv) 360 - kfree_skb(skb); 352 + kfree_skb_reason(skb, reason); 361 353 } 362 354 #endif
+15 -5
net/bridge/br_input.c
··· 75 75 /* note: already called with rcu_read_lock */ 76 76 int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb) 77 77 { 78 + enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED; 78 79 struct net_bridge_port *p = br_port_get_rcu(skb->dev); 79 80 enum br_pkt_type pkt_type = BR_PKT_UNICAST; 80 81 struct net_bridge_fdb_entry *dst = NULL; ··· 97 96 if (br_mst_is_enabled(br)) { 98 97 state = BR_STATE_FORWARDING; 99 98 } else { 100 - if (p->state == BR_STATE_DISABLED) 99 + if (p->state == BR_STATE_DISABLED) { 100 + reason = SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE; 101 101 goto drop; 102 + } 102 103 103 104 state = p->state; 104 105 } ··· 158 155 } 159 156 } 160 157 161 - if (state == BR_STATE_LEARNING) 158 + if (state == BR_STATE_LEARNING) { 159 + reason = SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE; 162 160 goto drop; 161 + } 163 162 164 163 BR_INPUT_SKB_CB(skb)->brdev = br->dev; 165 164 BR_INPUT_SKB_CB(skb)->src_port_isolated = !!(p->flags & BR_ISOLATED); ··· 228 223 out: 229 224 return 0; 230 225 drop: 231 - kfree_skb(skb); 226 + kfree_skb_reason(skb, reason); 232 227 goto out; 233 228 } 234 229 EXPORT_SYMBOL_GPL(br_handle_frame_finish); ··· 329 324 */ 330 325 static rx_handler_result_t br_handle_frame(struct sk_buff **pskb) 331 326 { 327 + enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED; 332 328 struct net_bridge_port *p; 333 329 struct sk_buff *skb = *pskb; 334 330 const unsigned char *dest = eth_hdr(skb)->h_dest; ··· 337 331 if (unlikely(skb->pkt_type == PACKET_LOOPBACK)) 338 332 return RX_HANDLER_PASS; 339 333 340 - if (!is_valid_ether_addr(eth_hdr(skb)->h_source)) 334 + if (!is_valid_ether_addr(eth_hdr(skb)->h_source)) { 335 + reason = SKB_DROP_REASON_MAC_INVALID_SOURCE; 341 336 goto drop; 337 + } 342 338 343 339 skb = skb_share_check(skb, GFP_ATOMIC); 344 340 if (!skb) ··· 382 374 return RX_HANDLER_PASS; 383 375 384 376 case 0x01: /* IEEE MAC (Pause) */ 377 + reason = SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL; 385 378 goto drop; 386 379 387 380 case 0x0E: /* 802.1AB LLDP */ ··· 432 423 433 424 return nf_hook_bridge_pre(skb, pskb); 434 425 default: 426 + reason = SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE; 435 427 drop: 436 - kfree_skb(skb); 428 + kfree_skb_reason(skb, reason); 437 429 } 438 430 return RX_HANDLER_CONSUMED; 439 431 }