net: in virtio_net_hdr only add VLAN_HLEN to csum_start if payload holds vlan

Tun, tap, virtio, packet and uml vector all use struct virtio_net_hdr
to communicate packet metadata to userspace.

For skbuffs with vlan, the first two return the packet as it may have
existed on the wire, inserting the VLAN tag in the user buffer.  Then
virtio_net_hdr.csum_start needs to be adjusted by VLAN_HLEN bytes.

Commit f09e2249c4f5 ("macvtap: restore vlan header on user read")
added this feature to macvtap. Commit 3ce9b20f1971 ("macvtap: Fix
csum_start when VLAN tags are present") then fixed up csum_start.

Virtio, packet and uml do not insert the vlan header in the user
buffer.

When introducing virtio_net_hdr_from_skb to deduplicate filling in
the virtio_net_hdr, the variant from macvtap which adds VLAN_HLEN was
applied uniformly, breaking csum offset for packets with vlan on
virtio and packet.

Make insertion of VLAN_HLEN optional. Convert the callers to pass it
when needed.

Fixes: e858fae2b0b8f4 ("virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
Fixes: 1276f24eeef2 ("packet: use common code for virtio_net_hdr and skb GSO conversion")
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c
index 9065047f844baee5c19574f4c12e8f3bd6e280bc..77e4ebc206ae1da08ac341010300ed8a0526b000 100644 (file)
--- a/arch/um/drivers/vector_transports.c
+++ b/arch/um/drivers/vector_transports.c
@@ -120,7 +120,8 @@ static int raw_form_header(uint8_t *header,
                skb,
                vheader,
                virtio_legacy_is_little_endian(),
-               false
+               false,
+               0
        );
 
        return 0;

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 9b6cb780affec603f8200d258988785c5edf55a0..f0f7cd9776671fb6d3f3dd6498611afe593b41c3 100644 (file)
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -774,13 +774,16 @@ static ssize_t tap_put_user(struct tap_queue *q,
        int total;
 
        if (q->flags & IFF_VNET_HDR) {
+               int vlan_hlen = skb_vlan_tag_present(skb) ? VLAN_HLEN : 0;
                struct virtio_net_hdr vnet_hdr;
+
                vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);
                if (iov_iter_count(iter) < vnet_hdr_len)
                        return -EINVAL;
 
                if (virtio_net_hdr_from_skb(skb, &vnet_hdr,
-                                           tap_is_little_endian(q), true))
+                                           tap_is_little_endian(q), true,
+                                           vlan_hlen))
                        BUG();
 
                if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 85e14adf520747c3f284472b89e30048e4b35451..a192a017cc68878360505b93df151de3d0b9b730 100644 (file)
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2089,7 +2089,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
                        return -EINVAL;
 
                if (virtio_net_hdr_from_skb(skb, &gso,
-                                           tun_is_little_endian(tun), true)) {
+                                           tun_is_little_endian(tun), true,
+                                           vlan_hlen)) {
                        struct skb_shared_info *sinfo = skb_shinfo(skb);
                        pr_err("unexpected GSO type: "
                               "0x%x, gso_size %d, hdr_len %d\n",

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 2aaa18ec7d460fde599aa0508b485077a47ee3e5..1619ee3070b6213cb9b9cc65e8fb146a2894ebdf 100644 (file)
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1411,7 +1411,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
                hdr = skb_vnet_hdr(skb);
 
        if (virtio_net_hdr_from_skb(skb, &hdr->hdr,
-                                   virtio_is_little_endian(vi->vdev), false))
+                                   virtio_is_little_endian(vi->vdev), false,
+                                   0))
                BUG();
 
        if (vi->mergeable_rx_bufs)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index f144216febc642fd70512df9dddefe1a7f119478..9397628a196714dc2177552465fe91fd18b9627d 100644 (file)
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -58,7 +58,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
                                          struct virtio_net_hdr *hdr,
                                          bool little_endian,
-                                         bool has_data_valid)
+                                         bool has_data_valid,
+                                         int vlan_hlen)
 {
        memset(hdr, 0, sizeof(*hdr));   /* no info leak */
 
@@ -83,12 +84,8 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
 
        if (skb->ip_summed == CHECKSUM_PARTIAL) {
                hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-               if (skb_vlan_tag_present(skb))
-                       hdr->csum_start = __cpu_to_virtio16(little_endian,
-                               skb_checksum_start_offset(skb) + VLAN_HLEN);
-               else
-                       hdr->csum_start = __cpu_to_virtio16(little_endian,
-                               skb_checksum_start_offset(skb));
+               hdr->csum_start = __cpu_to_virtio16(little_endian,
+                       skb_checksum_start_offset(skb) + vlan_hlen);
                hdr->csum_offset = __cpu_to_virtio16(little_endian,
                                skb->csum_offset);
        } else if (has_data_valid &&

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 54ce66f68482852a9896afbcbb2cf30cd4c7edfd..ee018564b2b4749d55c2080ac7ccd34fcd805e9a 100644 (file)
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2005,7 +2005,7 @@ static int packet_rcv_vnet(struct msghdr *msg, const struct sk_buff *skb,
                return -EINVAL;
        *len -= sizeof(vnet_hdr);
 
-       if (virtio_net_hdr_from_skb(skb, &vnet_hdr, vio_le(), true))
+       if (virtio_net_hdr_from_skb(skb, &vnet_hdr, vio_le(), true, 0))
                return -EINVAL;
 
        return memcpy_to_msg(msg, (void *)&vnet_hdr, sizeof(vnet_hdr));
@@ -2272,7 +2272,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
        if (do_vnet) {
                if (virtio_net_hdr_from_skb(skb, h.raw + macoff -
                                            sizeof(struct virtio_net_hdr),
-                                           vio_le(), true)) {
+                                           vio_le(), true, 0)) {
                        spin_lock(&sk->sk_receive_queue.lock);
                        goto drop_n_account;
                }