[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CANn89iL5TbT24Xy_=9SrqE=QJ-aF2V+jiuUY37KBnjK-qPcefQ@mail.gmail.com>
Date: Mon, 12 Jan 2026 10:03:03 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Chen Zhen <chenzhen126@...wei.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org, huyizhen2@...wei.com,
gaoxingwang1@...wei.com
Subject: Re: [PATCH v3 net 1/2] net: vlan: set header_ops to match
hard_header_len when hw offload is toggled
On Mon, Jan 12, 2026 at 8:49 AM Chen Zhen <chenzhen126@...wei.com> wrote:
>
> When tx-vlan-hw-insert is toggled to on, vlan device hard_header_len
> will be reduced to dev->hard_header_len since commit 029f5fc31cdb
> ("8021q: set hard_header_len when VLAN offload features are toggled"),
> but the header_ops remains unchanged, ndisc skb will be allocated
> with this len and filled in vlan hdr in vlan_dev_hard_header(), but
> with reorder_hdr off, the skb room is not enough so it triggers
> skb_panic() as below:
>
> skbuff: skb_under_panic: text:ffffffffa0535126 len:90 put:14
> head:ffff916c04232ec0 data:ffff916c04232ebe tail:0x58 end:0x180 dev:veth0.10
> ------------[ cut here ]------------
> kernel BUG at net/core/skbuff.c:197!
> <TASK>
> skb_push+0x39/0x40 net/core/skbuff.c:207
> eth_header+0x26/0xb0 net/ethernet/eth.c:90
> vlan_dev_hard_header+0x58/0x130 net/8021q/vlan_dev.c:85 [8021q]
> neigh_connected_output+0xae/0x100 net/core/neighbour.c:1589
> ip6_finish_output2+0x2cc/0x650 net/ipv6/ip6_output.c:213
> ip6_finish_output+0x27/0xd0 net/ipv6/ip6_output.c:246
> ndisc_send_skb+0x1d0/0x370 net/ipv6/ndisc.c:516
> ndisc_send_ns+0x5a/0xb0 net/ipv6/ndisc.c:672
> addrconf_dad_work+0x2b5/0x380 net/ipv6/addrconf.c:4258
> process_one_work+0x17f/0x320 kernel/workqueue.c:2743
>
> Fix this by also setting header_ops of vlan dev when offload feature
> is toggled.
>
> Fixes: 029f5fc31cdb ("8021q: set hard_header_len when VLAN offload features are toggled")
> Signed-off-by: Chen Zhen <chenzhen126@...wei.com>
> ---
> net/8021q/vlan.c | 5 +----
> net/8021q/vlan.h | 3 +++
> net/8021q/vlan_dev.c | 22 ++++++++++++++--------
> 3 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index 2b74ed56eb16..84b3a3f67996 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -323,10 +323,7 @@ static void vlan_transfer_features(struct net_device *dev,
>
> netif_inherit_tso_max(vlandev, dev);
>
> - if (vlan_hw_offload_capable(dev->features, vlan->vlan_proto))
> - vlandev->hard_header_len = dev->hard_header_len;
> - else
> - vlandev->hard_header_len = dev->hard_header_len + VLAN_HLEN;
> + vlan_dev_set_header_attributes(dev, vlandev, vlan->vlan_proto);
>
> #if IS_ENABLED(CONFIG_FCOE)
> vlandev->fcoe_ddp_xid = dev->fcoe_ddp_xid;
> diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
> index c7ffe591d593..1d837814e061 100644
> --- a/net/8021q/vlan.h
> +++ b/net/8021q/vlan.h
> @@ -143,6 +143,9 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack);
> void unregister_vlan_dev(struct net_device *dev, struct list_head *head);
> bool vlan_dev_inherit_address(struct net_device *dev,
> struct net_device *real_dev);
> +void vlan_dev_set_header_attributes(struct net_device *dev,
> + struct net_device *vlan_dev,
> + __be16 proto);
>
> static inline u32 vlan_get_ingress_priority(struct net_device *dev,
> u16 vlan_tci)
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index fbf296137b09..1fe171748711 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -519,6 +519,19 @@ static const struct device_type vlan_type = {
>
> static const struct net_device_ops vlan_netdev_ops;
>
> +void vlan_dev_set_header_attributes(struct net_device *dev,
> + struct net_device *vlan_dev,
> + __be16 proto)
> +{
> + if (vlan_hw_offload_capable(dev->features, proto)) {
> + vlan_dev->header_ops = &vlan_passthru_header_ops;
> + vlan_dev->hard_header_len = dev->hard_header_len;
> + } else {
> + vlan_dev->header_ops = &vlan_header_ops;
> + vlan_dev->hard_header_len = dev->hard_header_len + VLAN_HLEN;
> + }
> +}
> +
> static int vlan_dev_init(struct net_device *dev)
> {
> struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> @@ -572,14 +585,7 @@ static int vlan_dev_init(struct net_device *dev)
> #endif
>
> dev->needed_headroom = real_dev->needed_headroom;
> - if (vlan_hw_offload_capable(real_dev->features, vlan->vlan_proto)) {
> - dev->header_ops = &vlan_passthru_header_ops;
> - dev->hard_header_len = real_dev->hard_header_len;
> - } else {
> - dev->header_ops = &vlan_header_ops;
> - dev->hard_header_len = real_dev->hard_header_len + VLAN_HLEN;
> - }
> -
> + vlan_dev_set_header_attributes(real_dev, dev, vlan->vlan_proto);
> dev->netdev_ops = &vlan_netdev_ops;
>
> SET_NETDEV_DEVTYPE(dev, &vlan_type);
> --
> 2.33.0
>
While nice to have, I think a race is still possible.
Some callers might see different hard_header_len values along their
path and crash.
Look at
commit db5b4e39c4e63700c68a7e65fc4e1f1375273476
Author: Eric Dumazet <edumazet@...gle.com>
Date: Thu Dec 11 17:35:50 2025 +0000
ip6_gre: make ip6gre_header() robust
My suggestion would be to always add VLAN_HLEN in hard_header_len,
even if the 'current' operational mode would not request it.
Powered by blists - more mailing lists