Re: [PATCH v2 nf] netfilter: nf_flow_table_ip: Introduce nf_flow_vlan_push()

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Apr 14, 2026 at 01:21:20PM +0200, Eric Woudstra wrote:
> Calling skb_reset_mac_header() before calling skb_vlan_push() does
> remove the error:
> 
> "skb_vlan_push got skb with skb->data not at mac header (offset 18)"
> 
> But the inner vlan tag is still not inserted correctly.
> 
> skb_vlan_push() uses __vlan_insert_inner_tag() to insert the tag
> at offset ETH_HLEN. But the inner tag should only be pushed, without
> offset, similar to nf_flow_pppoe_push().

It is doubled-tagged-vlan that is broken, right? I observed this once
but I have been burdened into a few things.

> Fixes: c653d5a78f34 ("netfilter: flowtable: inline vlan encapsulation in xmit path")
> Fixes: a3aca98aec9a ("netfilter: nf_flow_table_ip: reset mac header before vlan push")
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> 
> ---
> 
>  net/netfilter/nf_flow_table_ip.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index fd56d663cb5b..0086f8a1a0d6 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c
> @@ -544,6 +544,26 @@ static int nf_flow_offload_forward(struct nf_flowtable_ctx *ctx,
>  	return 1;
>  }
>  
> +static int nf_flow_vlan_push(struct sk_buff *skb, __be16 proto, u16 id)
> +{
> +	if (skb_vlan_tag_present(skb)) {
> +		struct vlan_hdr *vhdr;
> +
> +		if (skb_cow_head(skb, VLAN_HLEN))
> +			return -1;
> +
> +		__skb_push(skb, VLAN_HLEN);
> +		skb_reset_network_header(skb);
> +		vhdr = (struct vlan_hdr *)(skb->data);
> +		vhdr->h_vlan_TCI = htons(id);
> +		vhdr->h_vlan_encapsulated_proto = skb->protocol;
> +		skb->protocol = proto;
> +	} else {
> +		__vlan_hwaccel_put_tag(skb, proto, id);
> +	}
> +	return 0;
> +}
> +
>  static int nf_flow_pppoe_push(struct sk_buff *skb, u16 id)
>  {
>  	int data_len = skb->len + sizeof(__be16);
> @@ -738,9 +758,8 @@ static int nf_flow_encap_push(struct sk_buff *skb,
>  		switch (tuple->encap[i].proto) {
>  		case htons(ETH_P_8021Q):
>  		case htons(ETH_P_8021AD):
> -			skb_reset_mac_header(skb);
> -			if (skb_vlan_push(skb, tuple->encap[i].proto,
> -					  tuple->encap[i].id) < 0)
> +			if (nf_flow_vlan_push(skb, tuple->encap[i].proto,
> +					      tuple->encap[i].id) < 0)
>  				return -1;
>  			break;
>  		case htons(ETH_P_PPP_SES):
> -- 
> 2.53.0
>