From: Ido <ido@wireplug.org>
Subject: Re: ether_input(): remove redundant etype assignment
To: David Gwynne <david@gwynne.id.au>
Cc: tech@openbsd.org
Date: Tue, 17 Mar 2026 15:34:08 -0400

Hi David,

I really appreciate the detailed explanation, thank you.

I wonder if this warrants a comment (proposal attached),
as this function is already generously documented.

Best,

---
Ido


On 2026-03-17, David Gwynne wrote:
> hey Ido,
> 
> On Mon, Mar 16, 2026 at 01:04:07PM -0400, Ido wrote:
> > etype is already assigned the same value early in the function and remains unchanged.
> 
> except that vlan_input() between the first assignment and this one can
> change the packet to make a different ethertype available.
> 
> if the packet has a vlan tag, but the hardware that received it does not
> do vlan tag offloads, then the first etype will be ETHERTYPE_VLAN. if
> the vlan id in that packet is 0, the tag exists to encode priority for
> the packet, but it should be delivered as if it did not have a vlan tag
> on it. in this situation vlan_input removes the vlan tag and returns the
> packet to ether_input for processing.
> 
> the second etype assignment you're trying to remove here will then be
> able to read the actual type of the packet rather than the vlan type.
> 
> cheers,
> dlg
> 
> > ---
> > Ido
> > 
> > diff --git sys/net/if_ethersubr.c sys/net/if_ethersubr.c
> > index d67e791e7fa..63c4a80d811 100644
> > --- sys/net/if_ethersubr.c
> > +++ sys/net/if_ethersubr.c
> > @@ -525,7 +525,6 @@ ether_input(struct ifnet *ifp, struct mbuf *m, struct netstack *ns)
> >  	 * At this point it is known that the packet is destined
> >  	 * for layer 3 protocol handling on the local port.
> >  	 */
> > -	etype = ntohs(eh->ether_type);
> >  
> >  	switch (etype) {
> >  	case ETHERTYPE_IP:
> > 
> 

diff --git sys/net/if_ethersubr.c sys/net/if_ethersubr.c
index d67e791e7fa..68ab7357ca1 100644
--- sys/net/if_ethersubr.c
+++ sys/net/if_ethersubr.c
@@ -524,6 +524,9 @@ ether_input(struct ifnet *ifp, struct mbuf *m, struct netstack *ns)
 	 *
 	 * At this point it is known that the packet is destined
 	 * for layer 3 protocol handling on the local port.
+	 *
+	 * Reassign etype since the packet's ether_type
+	 * may have been modified by vlan_input() in the second phase.
 	 */
 	etype = ntohs(eh->ether_type);