Re: [PATCH net v2] net: drop bad gso csum_start and offset in virtio_net_hdr

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

> > > > So I guess VIRTIO_NET_HDR_GSO_* without VIRTIO_NET_HDR_F_DATA_VALID
> > > > would be wrong on rx.
> > > >
> > > > But the new check
> > > >
> > > >         if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > > >
> > > >                 [...]
> > > >
> > > >                 case SKB_GSO_TCPV4:
> > > >                 case SKB_GSO_TCPV6:
> > > >                         if (skb->csum_offset != offsetof(struct tcphdr, check))
> > > >                                 return -EINVAL;
> > > >
> > > > should be limited to callers of virtio_net_hdr_to_skb on the tx/GSO path.
> > > >
> > > > Looking what the cleanest/minimal patch is to accomplish that.
> > > >
> > >
> > > virtio_net_hdr_to_skb() translates virtio-net header to skb metadata,
> > > so it's RX. For TX the helper should be virtio_net_hdr_from_skb()
> > > which translates skb metadata to virtio hdr.
> >
> > virtio_net_hdr_to_skb is used by PF_PACKET, tun and tap
> 
> Exactly.
> 
> > when injecting a packet into the egress path.
> 
> For tuntap it's still the RX path. For PF_PACEKT and macvtap, it's the tx.
> 
> Maybe a new parameter to virtio_net_hdr_to_skb()?

This is the most straightforward approach. But requires changse to all
callers, in a patch targeting all the stable branches.

I'd prefer if we can detect ingress vs egress directly.

Based on ip_summed, pkt_type, is_skb_wmem or so. But so far have not
found a suitable condition.

I noticed something else: as you point out TUN is ingress. Unlike
virtnet_receive, it does not set ip_summed to CHECKSUM_UNNECESSARY if
VIRTIO_NET_HDR_F_DATA_VALID. It probably should. GRO expects packets
to have had their integrity verified. CHECKSUM_NONE on ingress is not
correct for GRO.

And also related: no GRO should be generated by a device unless
VIRTIO_NET_HDR_F_DATA_VALID is also passed? I have to check the spec
if it says anything about this.