netdev - Re: [PATCH net-next v2 01/12] flow_dissector: Parse ETH_P

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <66c4d5b72c3ec_b14fa2943a@willemb.c.googlers.com.notmuch>
Date: Tue, 20 Aug 2024 13:43:19 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Tom Herbert <tom@...bertland.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: davem@...emloft.net, 
 kuba@...nel.org, 
 edumazet@...gle.com, 
 netdev@...r.kernel.org, 
 felipe@...anda.io
Subject: Re: [PATCH net-next v2 01/12] flow_dissector: Parse ETH_P_TEB and
 move out of GRE

> > > > > +             /* Cap headers that we access via pointers at the
> > > > > +              * end of the Ethernet header as our maximum alignment
> > > > > +              * at that point is only 2 bytes.
> > > > > +              */
> > > > > +             if (NET_IP_ALIGN)
> > > > > +                     hlen = nhoff;
> > > >
> > > > I wonder why this exists. But besides the point of this move.
> > >
> > > Willem,
> > >
> > > Ethernet header breaks 4-byte alignment of encapsulated protocols
> > > since it's 14 bytes, so the NET_IP_ALIGN can be used on architectures
> > > that don't like unaligned loads.
> >
> > I understand how NET_IP_ALIGN is used by drivers.
> >
> > I don't understand its use here in the flow dissector. Why is hlen
> > capped if it is set?
> 
> Willem,
> 
> For the real Ethernet header the receive skbuf is offset by two so
> that device places the packet such that the Ethernet payload, i.e. IP
> header, is aligned to four bytes (14+2=16 which will be offset of IP
> header). When a packets contains an encapsulated Ethernet header, the
> offset of the header is aligned to four bytes which means the payload
> of that Ethernet header, i.e. an encapsulated IP header, is not four
> byte aligned and neither are any subsequent headers (TCP, UDP, etc.).

> On some architectures, performing unaligned loads is expensive
> compared to aligned loads, so hlen is being capped here to avoid
> having flow dissector do that on unaligned headers after the Ethernet
> header. It's a tradeoff between performance and deeper flow
> dissection.

Thanks Tom. That explains.

So flow dissector behavior differs in this subtle way depending on
platform.

Maybe this is a good opportunity to add a comment. This thread alone
also already documents it to some extent.