Michael S. Tsirkin wrote:
> On Tue, Oct 20, 2026 at 08:05:29PM -0400, Willem de Bruijn wrote:
> > 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.
>
> I posted v4 with the custom parser, since i had it tested. Let's do
> cleanups on top? I spent too much time on this, to be frank.
Yes, as a matter of fact, on investigating this approach in more
detail, I agree that flow dissection is a worse choice.
> Hmm, an interesting trick, but I don't see how this works for tunnels?
> We need to dissect at inner_nh, and on success we can not leave
> network_header there - the packet continues to outer IP processing.
> I fear making all this work will be more code than the parsing it
> replaces.
On the upsides, quoting an LLM:
"
Using the flow dissector replaces all of virtio_net_ip4_parse(),
virtio_net_l3_start(), virtio_net_l4_start(), and
virtio_net_csum_partial_ok_at_offset() with two compact functions:
Function | Lines of Code
| Description
------------------------------------------|------------------------------------------|-----------------------------------------------------
virtio_net_setup_l3_input() | ~28 lines
| Anchors L2 framing (eth_header_ops vs raw L3)
virtio_net_csum_partial_ok() | ~42 lines
| Runs flow dissector and validates thoff/csum_offset
virtio_net_tnl_offsets_ok() | ~8 lines
| Reuses outer dissection keys for tunnel validation
Total dedicated parsing code | ~78 lines
|
──────
### Summary of Differences
• Net Code Reduction: ~103 fewer lines of C code added to virtio_net.h (~57%
reduction in new parsing code).
• Eliminated Series Dependencies: Patch 7/8 (net: add ip4_parse()) is
completely eliminated, reducing the patch series by an entire patch.
• Deduplication: The existing non-checksum GSO validation in
__virtio_net_hdr_to_skb() (lines 120–146, 27 lines) already calls
skb_flow_dissect_flow_keys_basic(); sharing virtio_net_setup_l3_input()
consolidates both paths onto a single ingress dissection mechanism.
"
I particularly like that we use the same for NEEDS_CSUM and !NEEDS_CSUM.
Also the lack of code duplication.
But, the goal of this patch is to minimize overhead in the hot path of
standard TCP/IP and UDP code. And flow dissection is significantly more
expensive that open coding those specific checks.
