Re: [PATCH net v3 5/8] net: resolve uncommon partial checksums at ingress

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

Michael S. Tsirkin wrote:
> On Tue, Oct 20, 2026 at 03:44:35PM -0400, Willem de Bruijn wrote:
> > > +/* Walk the L3 header at @nhoff: where does its transport header
> > > + * start, and which protocol is it?  Returns false if the header
> > > + * is truncated, malformed or fragmented.
> > > + */
> > > +static inline bool
> > > +virtio_net_l4_start(struct sk_buff *skb, unsigned int nhoff, __be16 proto,
> > > +		    unsigned int *thoff, int *ip_proto)
> > > +{
> > > +	if (proto == htons(ETH_P_IP)) {
> > > +		const struct iphdr *iph = virtio_net_ip4_parse(skb, nhoff, thoff);
> > > +
> > > +		if (!iph || ip_is_fragment(iph))
> > > +			return false;
> > > +		*ip_proto = iph->protocol;
> > > +		return true;
> > > +	}
> > > +
> > > +	if (proto == htons(ETH_P_IPV6)) {
> > > +		const struct ipv6hdr *iph;
> > > +		int flags = 0;
> > > +
> > > +		if (!pskb_may_pull(skb, nhoff + sizeof(*iph)))
> > > +			return false;
> > > +		iph = (const void *)(skb->data + nhoff);
> > > +		if (iph->version != 6)
> > > +			return false;
> > > +		*thoff = nhoff + sizeof(*iph);
> > > +		*ip_proto = iph->nexthdr;
> > > +		if (*ip_proto != IPPROTO_TCP && *ip_proto != IPPROTO_UDP) {
> > > +			*thoff = nhoff;
> > > +			*ip_proto = ipv6_find_hdr(skb, thoff, -1, NULL,
> > > +						  &flags);
> > > +			if (*ip_proto < 0 || flags & IP6_FH_F_FRAG)
> > > +				return false;
> > > +		}
> > > +		return true;
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > 
> > for all this parsing: could we use flow dissection, like the
> > !VIRTIO_NET_HDR_F_NEEDS_CSUM case?
> 
> Hmm I don't see how.  That one trusts the caller's protocol and network
> header. We could parse L2 ourselves and use the dissector for L3+. But
> it stops at whatever it does not know, and every miss would turn into a
> drop or a software checksum for a packet the stack delivers fine.  Our
> parser deliberately covers everything the stack can deliver.

You're right, the protocol and network offset need to be parsed
manually. After that it could be passed to the flow dissector.

Totally untested:

    static inline bool virtio_net_setup_l3_input(struct sk_buff *skb, int *nhoff, __be16 *proto)
    {
        const struct net_device *dev = skb->dev;

        /* Devices with Ethernet framing: ARPHRD_ETHER, loopback, macvlan, etc. */
        if (dev->header_ops == &eth_header_ops || dev->type == ARPHRD_ETHER) {
            if (!pskb_may_pull(skb, ETH_HLEN))
                return false;
            *nhoff = ETH_HLEN;
            *proto = eth_hdr(skb)->h_proto;
        } else if (dev->type == ARPHRD_NONE) {
            /* Raw L3 devices (e.g. tun in IFF_TUN mode) */
            *nhoff = 0;
            *proto = skb->protocol;
            if (!*proto) {
                if (!pskb_may_pull(skb, 1))
                    return false;
                if ((skb->data[0] >> 4) == 4)
                    *proto = htons(ETH_P_IP);
                else if ((skb->data[0] >> 4) == 6)
                    *proto = htons(ETH_P_IPV6);
                else
                    return false;
            }
        } else {
            /* Fallback for other netdevs using caller's network header */
            *nhoff = skb_network_offset(skb);
            if (*nhoff < 0 || (unsigned int)*nhoff >= skb->len)
                return false;
            *proto = skb->protocol;
        }

        return true;
    }

    static inline bool virtio_net_csum_partial_ok(struct sk_buff *skb, u8 hdr_gso_type)
    {
        struct flow_keys_basic keys;
        __be16 orig_proto = skb->protocol;
        u16 orig_net_hdr = skb->network_header;
        __be16 proto;
        int nhoff;
        u16 csum_offset;
        size_t min_thlen;
  
        if (!virtio_net_setup_l3_input(skb, &nhoff, &proto))
            return false;
  
        skb_set_network_header(skb, nhoff);
        skb->protocol = proto;
  
        if (!skb_flow_dissect_flow_keys_basic(dev_net(skb->dev), skb, &keys,
                              NULL, 0, 0, 0, 0))
            goto restore_fail;
  
        /* Disallow fragments: partial checksums cannot be retained across frags */
        if (keys.control.flags & FLOW_DIS_IS_FRAGMENT)
            goto restore_fail;
  
        /* Validate protocol matching */
        switch (keys.basic.ip_proto) {
        case IPPROTO_TCP:
            if (!virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type))
                goto restore_fail;
            min_thlen = sizeof(struct tcphdr);
            csum_offset = offsetof(struct tcphdr, check);
            break;
        case IPPROTO_UDP:
            if (!virtio_net_hdr_match_proto(keys.basic.n_proto, hdr_gso_type))
                goto restore_fail;
            min_thlen = sizeof(struct udphdr);
            csum_offset = offsetof(struct udphdr, check);
            break;
        default:
            goto restore_fail;
        }

        /* csum_start must match the dissected transport header offset */
        if (skb_transport_offset(skb) != keys.control.thoff)
            goto restore_fail;

        /* csum_offset must point to the transport checksum field */
        if (skb->csum_offset != csum_offset)
            goto restore_fail;

        /* Ensure the transport header is linear */
        if (!pskb_may_pull(skb, keys.control.thoff + min_thlen))
            goto restore_fail;

        return true;

    restore_fail:
        skb->protocol = orig_proto;
        skb->network_header = orig_net_hdr;
        return false;
    }