Netdev Archive on lore.kernel.org
 help / color / Atom feed
* forwarded bridged packets enqueuing is broken
@ 2019-12-02 12:34 Marco Oliverio
  2019-12-02 16:16 ` Florian Westphal
  0 siblings, 1 reply; 2+ messages in thread
From: Marco Oliverio @ 2019-12-02 12:34 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Marco Oliverio, Rocco Folino, Florian Westphal, netdev


Hi,

We cannot enqueue userspace bridged forwarded packets (neither in the
forward chain nor in the postrouting one):

nft add table bridge t
nft add chain bridge t forward {type filter hook forward priority 0\;}
nft add rule bridge t forward queue

packets from machines other than localhost aren't enqueued at all.

(this is also true for the postrouting chain).

We think the root of the problem is the check introduced by
b60a77386b1d4868f72f6353d35dabe5fbe981f2 (net: make skb_dst_force
return true when dst is refcounted):

modified   net/netfilter/nf_queue.c
@@ -174,6 +174,11 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state,
 		goto err;
 	}
 
+	if (!skb_dst_force(skb) && state->hook != NF_INET_PRE_ROUTING) {
+		status = -ENETDOWN;
+		goto err;
+	}
+

AFAIU forwarded bridge packets have a null dst entry in the first
place, as they don't enter the ip stack, so skb_dst_force() returns
false. The very same commit suggested to check skb_dst() before
skb_dst_force(), doing that indeed fix the issue for us:

modified   net/netfilter/nf_queue.c
@@ -174,7 +174,7 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state,
 		goto err;
 	}
 
-	if (!skb_dst_force(skb) && state->hook != NF_INET_PRE_ROUTING) {
+	if (skb_dst(skb) && !skb_dst_force(skb)) {
 		status = -ENETDOWN;
 		goto err;
 	}

This assumes that we shouldn't enqueue the packet if skb_dst_force()
sets not-NULL skb->dst to NULL, but it is safe to do that if skb->dst
was NULL in the first place. It should also cover che PRE_ROUTING hook
case. Is this assumption correct? Are there any side effects we're
missing?

If it is correct and it helps we can send a patch on top of the
netfilter tree.

Greetins
Marco


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: forwarded bridged packets enqueuing is broken
  2019-12-02 12:34 forwarded bridged packets enqueuing is broken Marco Oliverio
@ 2019-12-02 16:16 ` Florian Westphal
  0 siblings, 0 replies; 2+ messages in thread
From: Florian Westphal @ 2019-12-02 16:16 UTC (permalink / raw)
  To: Marco Oliverio; +Cc: netfilter-devel, Rocco Folino, Florian Westphal, netdev

Marco Oliverio <marco.oliverio@tanaza.com> wrote:
> We cannot enqueue userspace bridged forwarded packets (neither in the
> forward chain nor in the postrouting one):

[..]

> AFAIU forwarded bridge packets have a null dst entry in the first
> place, as they don't enter the ip stack, so skb_dst_force() returns
> false. The very same commit suggested to check skb_dst() before
> skb_dst_force(), doing that indeed fix the issue for us:
> 
> modified   net/netfilter/nf_queue.c
> @@ -174,7 +174,7 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state,
>  		goto err;
>  	}
>  
> -	if (!skb_dst_force(skb) && state->hook != NF_INET_PRE_ROUTING) {
> +	if (skb_dst(skb) && !skb_dst_force(skb)) {

Looks fine to me, please submit this formally.  Thanks!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 12:34 forwarded bridged packets enqueuing is broken Marco Oliverio
2019-12-02 16:16 ` Florian Westphal

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git