RE: [PATCH net 2/2] net: tap: fix skb->protocol not updated after VLAN network header adjustment

[Wei Fang]

> > > --- a/drivers/net/tap.c
> > > +++ b/drivers/net/tap.c
> > > @@ -1081,9 +1081,15 @@ static int tap_get_user_xdp(struct tap_queue
> *q,
> > struct xdp_buff *xdp)
> > >  	}
> > >
> > >  	/* Move network header to the right position for VLAN tagged
> packets */
> > > -	if (eth_type_vlan(skb->protocol) &&
> > > -	    vlan_get_protocol_and_depth(skb, skb->protocol, &depth) != 0)
> > > -		skb_set_network_header(skb, depth);
> > > +	if (eth_type_vlan(skb->protocol)) {
> > > +		__be16 proto = vlan_get_protocol_and_depth(skb,
> skb->protocol,
> > > +							   &depth);
> > > +
> > > +		if (proto != 0) {
> > > +			skb_set_network_header(skb, depth);
> > > +			skb->protocol = proto;
> > > +		}
> > > +	}
> >
> > Does the same apply to the same call in tap_get_user?
> 
> The situation of tap_get_user() is different, skb_probe_transport_header()
> is called before the VLAN adjustment block. So I think transport_header
> should be correct. The only concern is whether skb->protocol needs to be
> updated after calling skb_set_network_header().
> 
> I'm not sure as I am not fairly familiar with the tap driver. I added this patch
> because Sashiko reported that the tap driver has the same issue as af_packet.
> 
> >
> > And in general to other callers of vlan_get_protocol_and_depth,
> > including through wrapper skb_network_protocol?
> 
> I don't think this issue exists elsewhere. The issue arises because
> skb_probe_transport_header() is called after skb_set_network_header(), and
> at this point, skb->protocol and network_header are not synchronized ( In
> __skb_flow_dissect(), nhoff = skb_network_offset(skb) but proto is
> ETH_P_8021Q or ETH_P_8021AD)), causing skb_probe_transport_header() to
> fail to set transport_header correctly.
> 
> Perhaps the correct approach would be to restore the original `skb->protocol`
> value after `skb_probe_transport_header()`, maintaining consistency with the
> previous behavior; otherwise, it might introduce new issues.

For AF_PACKET, it has been confirmed that skb->protocol does not need to be
restored to its initial value; otherwise, the egress tc flower for protocol ip will
not match the packet. A known issue is that the packets cannot match the egress
TC flower rules for protocol 802.1Q, but this issue exists before this series. This
is likely a limitation of using AF_PACKET to send packets.