[PATCH net] net: skb_scrub_packet(): Scrub offload_fwd_mark

[PATCH net] net: skb_scrub_packet(): Scrub offload_fwd_mark

[Petr Machata]

When a packet is trapped and the corresponding SKB marked as
already-forwarded, it retains this marking even after it is forwarded
across veth links into another bridge. There, since it ingresses the
bridge over veth, which doesn't have offload_fwd_mark, it triggers a
warning in nbp_switchdev_frame_mark().

Then nbp_switchdev_allowed_egress() decides not to allow egress from
this bridge through another veth, because the SKB is already marked, and
the mark (of 0) of course matches. Thus the packet is incorrectly
blocked.

Solve by resetting offload_fwd_mark() in skb_scrub_packet(). That
function is called from tunnels and also from veth, and thus catches the
cases where traffic is forwarded between bridges and transformed in a
way that invalidates the marking.

Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
Fixes: abf4bb6b63d0 ("skbuff: Add the offload_mr_fwd_mark field")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Suggested-by: Ido Schimmel <idosch@mellanox.com>
---
 net/core/skbuff.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b4ee5c8b928f..a8217e221e19 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4854,6 +4854,11 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 	nf_reset(skb);
 	nf_reset_trace(skb);
 
+#ifdef CONFIG_NET_SWITCHDEV
+	skb->offload_fwd_mark = 0;
+	skb->offload_mr_fwd_mark = 0;
+#endif
+
 	if (!xnet)
 		return;
 
-- 
2.4.11

Re: [PATCH net] net: skb_scrub_packet(): Scrub offload_fwd_mark

[Jiri Pirko]

Tue, Nov 20, 2018 at 12:39:56PM CET, petrm@mellanox.com wrote:
>When a packet is trapped and the corresponding SKB marked as
>already-forwarded, it retains this marking even after it is forwarded
>across veth links into another bridge. There, since it ingresses the
>bridge over veth, which doesn't have offload_fwd_mark, it triggers a
>warning in nbp_switchdev_frame_mark().
>
>Then nbp_switchdev_allowed_egress() decides not to allow egress from
>this bridge through another veth, because the SKB is already marked, and
>the mark (of 0) of course matches. Thus the packet is incorrectly
>blocked.
>
>Solve by resetting offload_fwd_mark() in skb_scrub_packet(). That
>function is called from tunnels and also from veth, and thus catches the
>cases where traffic is forwarded between bridges and transformed in a
>way that invalidates the marking.
>
>Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
>Fixes: abf4bb6b63d0 ("skbuff: Add the offload_mr_fwd_mark field")
>Signed-off-by: Petr Machata <petrm@mellanox.com>
>Suggested-by: Ido Schimmel <idosch@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

Re: [PATCH net] net: skb_scrub_packet(): Scrub offload_fwd_mark

[David Miller]

From: Petr Machata <petrm@mellanox.com>
Date: Tue, 20 Nov 2018 11:39:56 +0000

> When a packet is trapped and the corresponding SKB marked as
> already-forwarded, it retains this marking even after it is forwarded
> across veth links into another bridge. There, since it ingresses the
> bridge over veth, which doesn't have offload_fwd_mark, it triggers a
> warning in nbp_switchdev_frame_mark().
> 
> Then nbp_switchdev_allowed_egress() decides not to allow egress from
> this bridge through another veth, because the SKB is already marked, and
> the mark (of 0) of course matches. Thus the packet is incorrectly
> blocked.
> 
> Solve by resetting offload_fwd_mark() in skb_scrub_packet(). That
> function is called from tunnels and also from veth, and thus catches the
> cases where traffic is forwarded between bridges and transformed in a
> way that invalidates the marking.
> 
> Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
> Fixes: abf4bb6b63d0 ("skbuff: Add the offload_mr_fwd_mark field")
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Suggested-by: Ido Schimmel <idosch@mellanox.com>

Applied and queued up for -stable, thanks.