[PATCH nf] netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

[PATCH nf] netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

[Andrea Parri]

Bridged IPv6 packets can lose their VLAN tag or acquire an unrelated tag
when conntrack-reassembled packets are refragmented. On a VLAN-aware
bridge, this can send fragments with a different VLAN tag from the one
selected for forwarding.

br_nf_push_frag_xmit() restores the VLAN tag from per-CPU storage, but
only the IPv4 branch of br_nf_dev_queue_xmit() saves it. The IPv6 branch
leaves the saved tag from the previous IPv4 refragmentation on that CPU.
Newly allocated fragments do not inherit the tag through
ip6_copy_metadata().

Commit d7b597421519 ("netfilter: bridge: restore vlan tag when
refragmenting") added VLAN tag restoration for IPv4 only, shortly after
IPv6 refragmentation was introduced.

Move saving the L2 header and VLAN tag into br_nf_save_frag_data() and
call it from both branches, so IPv6 fragments use the current packet's
VLAN information.

Fixes: efb6de9b4ba0 ("netfilter: bridge: forward IPv6 fragmented packets")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 net/bridge/br_netfilter_hooks.c | 45 +++++++++++++++------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 0a394e5f43916..ec69d6254e340 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -832,6 +832,25 @@ static unsigned int nf_bridge_mtu_reduction(const struct sk_buff *skb)
 	return 0;
 }
 
+/* Fragments may not inherit the MAC header or VLAN tag. */
+static void br_nf_save_frag_data(struct sk_buff *skb)
+{
+	struct brnf_frag_data *data = this_cpu_ptr(&brnf_frag_data_storage);
+
+	if (skb_vlan_tag_present(skb)) {
+		data->vlan_tci = skb->vlan_tci;
+		data->vlan_proto = skb->vlan_proto;
+	} else {
+		data->vlan_proto = 0;
+	}
+
+	data->encap_size = nf_bridge_encap_header_len(skb);
+	data->size = ETH_HLEN + data->encap_size;
+
+	skb_copy_from_linear_data_offset(skb, -data->size, data->mac,
+					 data->size);
+}
+
 static int br_nf_dev_queue_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
 	struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
@@ -866,28 +885,13 @@ static int br_nf_dev_queue_xmit(struct net *net, struct sock *sk, struct sk_buff
 	 */
 	if (IS_ENABLED(CONFIG_NF_DEFRAG_IPV4) &&
 	    skb->protocol == htons(ETH_P_IP)) {
-		struct brnf_frag_data *data;
-
 		if (br_validate_ipv4(net, skb))
 			goto drop;
 
 		IPCB(skb)->frag_max_size = nf_bridge->frag_max_size;
 
 		local_lock_nested_bh(&brnf_frag_data_storage.bh_lock);
-		data = this_cpu_ptr(&brnf_frag_data_storage);
-
-		if (skb_vlan_tag_present(skb)) {
-			data->vlan_tci = skb->vlan_tci;
-			data->vlan_proto = skb->vlan_proto;
-		} else {
-			data->vlan_proto = 0;
-		}
-
-		data->encap_size = nf_bridge_encap_header_len(skb);
-		data->size = ETH_HLEN + data->encap_size;
-
-		skb_copy_from_linear_data_offset(skb, -data->size, data->mac,
-						 data->size);
+		br_nf_save_frag_data(skb);
 
 		ret = br_nf_ip_fragment(net, sk, skb, br_nf_push_frag_xmit);
 		local_unlock_nested_bh(&brnf_frag_data_storage.bh_lock);
@@ -895,20 +899,13 @@ static int br_nf_dev_queue_xmit(struct net *net, struct sock *sk, struct sk_buff
 	}
 	if (IS_ENABLED(CONFIG_NF_DEFRAG_IPV6) &&
 	    skb->protocol == htons(ETH_P_IPV6)) {
-		struct brnf_frag_data *data;
-
 		if (br_validate_ipv6(net, skb))
 			goto drop;
 
 		IP6CB(skb)->frag_max_size = nf_bridge->frag_max_size;
 
 		local_lock_nested_bh(&brnf_frag_data_storage.bh_lock);
-		data = this_cpu_ptr(&brnf_frag_data_storage);
-		data->encap_size = nf_bridge_encap_header_len(skb);
-		data->size = ETH_HLEN + data->encap_size;
-
-		skb_copy_from_linear_data_offset(skb, -data->size, data->mac,
-						 data->size);
+		br_nf_save_frag_data(skb);
 
 		ret = ip6_fragment(net, sk, skb, br_nf_push_frag_xmit);
 		local_unlock_nested_bh(&brnf_frag_data_storage.bh_lock);
-- 
2.53.0

Re: [PATCH nf] netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH nf] netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

[Andrea Parri]

[ Resending with the full Cc list, which I dropped by mistake in my
  previous reply. ]

On Mon, Sep 28, 2026 at 04:19:47PM +0000, netdev-bot+sinfo@kernel.org wrote:
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.

The issue was triggered, not only found by code inspection. I
reproduced it under virt-ng with a local test on a VLAN-aware bridge
with br_netfilter and IPv6 conntrack defrag enabled. The test sends a
fragmented IPv4 packet on one VLAN, then a fragmented IPv6 packet on
another VLAN that the bridge floods to several ports. The test is
available on request.

Symptoms: with the patch not applied, the refragmented IPv6 packets
leave the bridge with the wrong VLAN tag. This is the tag of the earlier
IPv4 flow when both refragmentations run on the same CPU. Otherwise the
packets go out untagged. There is no crash or warning; the only effect
is the wrong or missing tag on the wire. With the patch applied, all the
IPv6 fragments keep the correct VLAN tag.

Thanks,
  Andrea

Re: [PATCH nf] netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.

--

netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

This commit moves saving the L2 header and VLAN tag into a new helper,
br_nf_save_frag_data(). Both the IPv4 and IPv6 refragmentation branches of
br_nf_dev_queue_xmit() now call it. Before this change, the IPv6 branch
reused whatever VLAN tag the last IPv4 refragmentation on that CPU had left
in per-CPU storage.

> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> index 0a394e5f4391..ec69d6254e34 100644
> --- a/net/bridge/br_netfilter_hooks.c
> +++ b/net/bridge/br_netfilter_hooks.c
> @@ -832,6 +832,25 @@ static unsigned int nf_bridge_mtu_reduction(const struct sk_buff *skb)
>  	return 0;
>  }
>  
> +/* Fragments may not inherit the MAC header or VLAN tag. */
> +static void br_nf_save_frag_data(struct sk_buff *skb)
> +{
> +	struct brnf_frag_data *data = this_cpu_ptr(&brnf_frag_data_storage);
> +
> +	if (skb_vlan_tag_present(skb)) {
> +		data->vlan_tci = skb->vlan_tci;
> +		data->vlan_proto = skb->vlan_proto;
> +	} else {
> +		data->vlan_proto = 0;
> +	}

[Severity: High]
What happens when the head skb goes out untagged but the fragments still
carry a tag?

For an untagged packet this helper saves vlan_proto = 0. The consumer only
ever sets a tag and never clears one:

net/bridge/br_netfilter_hooks.c:br_nf_push_frag_xmit() {
    ...
	if (data->vlan_proto)
		__vlan_hwaccel_put_tag(skb, data->vlan_proto, data->vlan_tci);
    ...
}

For IPv6, nf_ct_frag6_reasm() calls inet_frag_reasm_finish(..., false).
Each original fragment stays as its own skb on the head's frag_list and
keeps its own ingress vlan_all.

On egress to an untagged VLAN (or a BR_VLAN_TUNNEL port), br_handle_vlan()
clears the tag on the head skb only:

	if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED &&
	    !br_switchdev_frame_uses_tx_fwd_offload(skb))
		__vlan_hwaccel_clear_tag(skb);

A unicast-forwarded reassembled skb normally takes the skb_has_frag_list()
fast path in ip6_fragment(). On that path,
ip6_fraglist_prepare()->ip6_copy_metadata() does not copy or reset vlan_all.

So would fragments 2..N reach br_nf_push_frag_xmit()->br_dev_queue_push_xmit()
with their ingress 802.1Q/802.1ad tag still set, while the first fragment
goes out untagged?

The conntrack bridge restore path handles this case:

net/bridge/netfilter/nf_conntrack_bridge.c:nf_ct_bridge_frag_restore() {
    ...
	if (data->vlan_present)
		__vlan_hwaccel_put_tag(skb, data->vlan_proto, data->vlan_tci);
	else if (skb_vlan_tag_present(skb))
		__vlan_hwaccel_clear_tag(skb);
    ...
}

Without a similar else branch in br_nf_push_frag_xmit(), does the commit
message's claim that IPv6 fragments now "use the current packet's VLAN
information" still hold? The same branch would also cover the IPv4 path,
since ip_copy_metadata() does not touch vlan_all either.

Also, the comment "Fragments may not inherit the MAC header or VLAN tag"
does not seem accurate for reused frag_list skbs, which keep their own tag.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928161830.351199-1-parri.andrea%40gmail.com

Re: [PATCH nf] netfilter: br_netfilter: restore VLAN tag on refragmented IPv6 packets

[Andrea Parri]

On Thu, Oct 01, 2026 at 10:19:55AM +0000, netdev-bot+sashiko@kernel.org wrote:
> For an untagged packet this helper saves vlan_proto = 0. The consumer only
> ever sets a tag and never clears one:
[...]
> So would fragments 2..N reach br_nf_push_frag_xmit()->br_dev_queue_push_xmit()
> with their ingress 802.1Q/802.1ad tag still set, while the first fragment
> goes out untagged?

Yes. Conntrack defrag keeps the original fragments on frag_list for both
IPv6 and IPv4 here (ip_frag_coalesce_ok() is true only for local
delivery), each with its ingress tag, and br_handle_vlan() clears the
tag of the head skb only. On the fast path those skbs are sent as the
fragments.

I extended my reproducer with a port that is untagged in VLAN 10 and a
static FDB entry, so the packet is not cloned. With this patch applied,
6/9 IPv6 and 6/9 IPv4 fragments left that port with "vlan 10"; only the
first fragment of each packet was untagged. This is not introduced by
this patch, but it means the changelog claim was too broad.

v2 will add a second patch that clears the tag in br_nf_push_frag_xmit()
when none was saved, as nf_ct_bridge_frag_restore() does, and narrow the
changelog of this one to the fragments ip6_fragment() builds.

The BR_VLAN_TUNNEL case does not reach the refragmentation code:
br_handle_egress_vlan_tunnel() attaches a metadata dst, so
br_nf_dev_queue_xmit() drops the packet at the !skb_valid_dst() check.

> Also, the comment "Fragments may not inherit the MAC header or VLAN tag"
> does not seem accurate for reused frag_list skbs, which keep their own tag.

Agreed, v2 rewords it.

pw-bot: cr

Thanks,
  Andrea