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

net/sched: act_mirred: use the backlog for mirred ingress

The test Davide added in commit ca22da2fbd69 ("act_mirred: use the backlog
for nested calls to mirred ingress") hangs our testing VMs every 10 or so
runs, with the familiar tcp_v4_rcv -> tcp_v4_rcv deadlock reported by
lockdep.

The problem as previously described by Davide (see Link) is that
if we reverse flow of traffic with the redirect (egress -> ingress)
we may reach the same socket which generated the packet. And we may
still be holding its socket lock. The common solution to such deadlocks
is to put the packet in the Rx backlog, rather than run the Rx path
inline. Do that for all egress -> ingress reversals, not just once
we started to nest mirred calls.

In the past there was a concern that the backlog indirection will
lead to loss of error reporting / less accurate stats. But the current
workaround does not seem to address the issue.

Fixes: 53592b364001 ("net/sched: act_mirred: Implement ingress actions")
Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Suggested-by: Davide Caratti <dcaratti@redhat.com>
Link: https://lore.kernel.org/netdev/33dc43f587ec1388ba456b4915c75f02a8aae226.1663945716.git.dcaratti@redhat.com/
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Jakub Kicinski and committed by
David S. Miller
52f671db a9f80df4

+5 -12
+5 -9
net/sched/act_mirred.c
··· 232 232 return err; 233 233 } 234 234 235 - static bool is_mirred_nested(void) 236 - { 237 - return unlikely(__this_cpu_read(mirred_nest_level) > 1); 238 - } 239 - 240 - static int tcf_mirred_forward(bool want_ingress, struct sk_buff *skb) 235 + static int 236 + tcf_mirred_forward(bool at_ingress, bool want_ingress, struct sk_buff *skb) 241 237 { 242 238 int err; 243 239 244 240 if (!want_ingress) 245 241 err = tcf_dev_queue_xmit(skb, dev_queue_xmit); 246 - else if (is_mirred_nested()) 242 + else if (!at_ingress) 247 243 err = netif_rx(skb); 248 244 else 249 245 err = netif_receive_skb(skb); ··· 315 319 316 320 skb_set_redirected(skb_to_send, skb_to_send->tc_at_ingress); 317 321 318 - err = tcf_mirred_forward(want_ingress, skb_to_send); 322 + err = tcf_mirred_forward(at_ingress, want_ingress, skb_to_send); 319 323 } else { 320 - err = tcf_mirred_forward(want_ingress, skb_to_send); 324 + err = tcf_mirred_forward(at_ingress, want_ingress, skb_to_send); 321 325 } 322 326 323 327 if (err) {
-3
tools/testing/selftests/net/forwarding/tc_actions.sh
··· 235 235 check_err $? "didn't mirred redirect ICMP" 236 236 tc_check_packets "dev $h1 ingress" 102 10 237 237 check_err $? "didn't drop mirred ICMP" 238 - local overlimits=$(tc_rule_stats_get ${h1} 101 egress .overlimits) 239 - test ${overlimits} = 10 240 - check_err $? "wrong overlimits, expected 10 got ${overlimits}" 241 238 242 239 tc filter del dev $h1 egress protocol ip pref 100 handle 100 flower 243 240 tc filter del dev $h1 egress protocol ip pref 101 handle 101 flower