Messages in this thread | ![/](/images/icornerl.gif) | | From | Willem de Bruijn <> | Date | Thu, 12 Jan 2023 11:04:58 -0500 | Subject | Re: [PATCH net 2/2] net/af_packet: fix tx skb network header on SOCK_RAW sockets over VLAN device |
| |
On Tue, Jan 10, 2023 at 2:38 PM Hervé Boisse <admin@netgeek.ovh> wrote: > > When an application sends a packet on a SOCK_RAW socket over a VLAN device, > there is a possibility that the skb network header is incorrectly set. > > The issue happens when the device used to send the packet is a VLAN device > whose underlying device has no VLAN tx hardware offloading support. > In that case, the VLAN driver reports a LL header size increased by 4 bytes > to take into account the tag that will be added in software. > > However, the socket user has no clue about that and still provides a normal > LL header without tag.
This is arguably a mistake.
A process using PF_PACKET to write to a device must know the link layer type. SOCK_RAW should prepare a header equivalent to dev_hard_header (which prepares it for SOCK_DGRAM). vlan_dev_hard_header clearly adds both the Ethernet header and VLAN tag.
If applications assume virtual VLAN device exposes an Ethernet link layer, then net/8021q/vlan_dev.c needs to expose that, including that hard_header_len.
> This results in the network header of the skb being shifted 4 bytes too far > in the packet. This shift makes tc classifiers fail as they point to > incorrect data. > > Move network header right after underlying VLAN device LL header size > without tag, regardless of hardware offloading support. That is, the > expected LL header size from socket user. > > Signed-off-by: Hervé Boisse <admin@netgeek.ovh> > --- > net/packet/af_packet.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index c18274f65b17..be892fd498a6 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -1933,6 +1933,18 @@ static void packet_parse_headers(struct sk_buff *skb, struct socket *sock) > skb->protocol = dev_parse_header_protocol(skb); > } > > + /* VLAN device may report bigger LL header size due to reserved room for > + * tag on devices without hardware offloading support > + */ > + if (is_vlan_dev(skb->dev) && > + (sock->type == SOCK_RAW || sock->type == SOCK_PACKET)) {
Let's also try very hard to avoid adding branches in the hot path for cases this rare.
> + struct net_device *real_dev = vlan_dev_real_dev(skb->dev); > + > + depth = real_dev->hard_header_len; > + if (pskb_may_pull(skb, depth)) > + skb_set_network_header(skb, depth); > + } > + > /* Move network header to the right position for VLAN tagged packets */ > if (likely(skb->dev->type == ARPHRD_ETHER) && > eth_type_vlan(skb->protocol) && > -- > 2.38.2 >
| ![\](/images/icornerr.gif) |