Hi Gabriel,

Yes, I checked the ordering and this leaves a gap in the check.
Software VLAN insertion moves skb->data back by VLAN_HLEN, which
can create a header crossing when the original data started near
the beginning of a 4 KB boundary. skb_cow_head() can also relocate
the buffer.

There are other later layout changes inside e1000e too: the payload-pull
workaround in e1000_xmit_frame() and skb_cow_head() in e1000_tso().
I need to account for those before relying on the checked address.

The posted testing covers the original untagged reproducer, not
these cases. I’ll address this in v2 and test the affected paths.

I think V2 can make the final decision inside e1000_xmit_frame():

 1. Complete VLAN insertion, payload pulling and any head copy-on-write.
 2. Check the final buffer address and header length.
 3. Use hardware TSO if safe; otherwise explicitly segment in software.

The check must precede TSO header rewriting and descriptor programming.
AFAICT software fallback also needs correct ring-space and queue handling,
not just clearing feature bits.

I will have Claude try to reproduce this on the same Proxmox hosts
with the v1 fix already applied, to isolate crashes to later packet growth.

André Brait


On Mon, Oct 19, 2026 at 3:36 PM Gabriel Goller <[email protected]> wrote:

> Hi Andre,
>
> On 18.10.2026 22:32, Andre Brait wrote:
> > [snip]
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> > index 834a5b888..6f95ed19a 100644
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/pagemap.h>
> >  #include <linux/delay.h>
> >  #include <linux/netdevice.h>
> > +#include <linux/sizes.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/tcp.h>
> >  #include <linux/ipv6.h>
> > @@ -7359,6 +7360,34 @@ static int e1000_set_features(struct net_device
> *netdev,
> >       return 1;
> >  }
> >
> > +/**
> > + * e1000_features_check - validate offloads for a packet about to be
> sent
> > + * @skb: packet to check
> > + * @netdev: network interface device structure
> > + * @features: offload features usable for this packet
> > + *
> > + * The transmit DMA can stop for good when a TSO packet's protocol
> headers
> > + * cross a 4 KB boundary in the buffer holding them: the descriptor is
> never
> > + * written back, the driver reports "Detected Hardware Unit Hang", and
> only a
> > + * reset brings the queue back. Seen on I219-LM/-V (PCH) with forwarded
> > + * guest traffic, which arrives as linear GSO skbs that can start
> anywhere in a
> > + * page. Let the stack segment those few packets in software instead.
> > + *
> > + * Returns: the features to use for @skb.
> > + */
> > +static netdev_features_t e1000_features_check(struct sk_buff *skb,
> > +                                           struct net_device *netdev,
> > +                                           netdev_features_t features)
> > +{
> > +     if (skb_is_gso(skb) &&
> > +         (skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
> &&
> > +         (offset_in_page(skb->data) & (SZ_4K - 1)) +
> > +         skb_tcp_all_headers(skb) > SZ_4K)
>
> Hmm, doesn't validate_xmit_skb call validate_xmit_vlan after the
> ndo_features_check, which means that the vlan header can be pushed into the
> packet? Meaning the packet can grow after this check.
>
> > +             features &= ~NETIF_F_GSO_MASK;
> > +
> > +     return features;
> > +}
> > +
> >  static const struct net_device_ops e1000e_netdev_ops = {
> >       .ndo_open               = e1000e_open,
> >       .ndo_stop               = e1000e_close,
> > @@ -7378,7 +7407,7 @@ static const struct net_device_ops
> e1000e_netdev_ops = {
> >  #endif
> >       .ndo_set_features       = e1000_set_features,
> >       .ndo_fix_features       = e1000_fix_features,
> > -     .ndo_features_check     = passthru_features_check,
> > +     .ndo_features_check     = e1000_features_check,
> >       .ndo_hwtstamp_get       = e1000e_hwtstamp_get,
> >       .ndo_hwtstamp_set       = e1000e_hwtstamp_set,
> >  };
> > --
> > 2.56.0
>
> Gabriel
>
>

Reply via email to