Skip to content

Commit

Permalink
net: in virtio_net_hdr only add VLAN_HLEN to csum_start if payload ho…
Browse files Browse the repository at this point in the history
…lds 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 f09e224 ("macvtap: restore vlan header on user read")
added this feature to macvtap. Commit 3ce9b20 ("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: e858fae ("virtio_net: use common code for virtio_net_hdr and skb GSO conversion")
Fixes: 1276f24 ("packet: use common code for virtio_net_hdr and skb GSO conversion")
Signed-off-by: Willem de Bruijn <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
wdebruij authored and davem330 committed Jun 7, 2018
1 parent 7eced5a commit fd3a886
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 13 deletions.
3 changes: 2 additions & 1 deletion arch/um/drivers/vector_transports.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ static int raw_form_header(uint8_t *header,
skb,
vheader,
virtio_legacy_is_little_endian(),
false
false,
0
);

return 0;
Expand Down
5 changes: 4 additions & 1 deletion drivers/net/tap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) !=
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/tun.c
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion drivers/net/virtio_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 4 additions & 7 deletions include/linux/virtio_net.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand All @@ -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 &&
Expand Down
4 changes: 2 additions & 2 deletions net/packet/af_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit fd3a886

Please sign in to comment.