diff mbox series

[net] net/packet: tpacket_rcv: do not increment ring index on drop

Message ID 20200309153435.32109-1-willemdebruijn.kernel@gmail.com
State New
Headers show
Series [net] net/packet: tpacket_rcv: do not increment ring index on drop | expand

Commit Message

Willem de Bruijn March 9, 2020, 3:34 p.m. UTC 
From: Willem de Bruijn <willemb@google.com>

In one error case, tpacket_rcv drops packets after incrementing the
ring producer index.

If this happens, it does not update tp_status to TP_STATUS_USER and
thus the reader is stalled for an iteration of the ring, causing out
of order arrival.

The only such error path is when virtio_net_hdr_from_skb fails due
to encountering an unknown GSO type.

Signed-off-by: Willem de Bruijn <willemb@google.com>

---

I wonder whether it should drop packets with unknown GSO types at all.
This consistently blinds the reader to certain packets, including
recent UDP and SCTP GSO types.

The peer function virtio_net_hdr_to_skb already drops any packets with
unknown types, so it should be fine to add an SKB_GSO_UNKNOWN type and
let the peer at least be aware of failure.

And possibly add SKB_GSO_UDP_L4 and SKB_GSO_SCTP types to virtio too.
---
 net/packet/af_packet.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 30c6879d6774..e5b0986215d2 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2274,6 +2274,13 @@  static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 					TP_STATUS_KERNEL, (macoff+snaplen));
 	if (!h.raw)
 		goto drop_n_account;
+
+	if (do_vnet &&
+	    virtio_net_hdr_from_skb(skb, h.raw + macoff -
+				    sizeof(struct virtio_net_hdr),
+				    vio_le(), true, 0))
+		goto drop_n_account;
+
 	if (po->tp_version <= TPACKET_V2) {
 		packet_increment_rx_head(po, &po->rx_ring);
 	/*
@@ -2286,12 +2293,6 @@  static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 			status |= TP_STATUS_LOSING;
 	}
 
-	if (do_vnet &&
-	    virtio_net_hdr_from_skb(skb, h.raw + macoff -
-				    sizeof(struct virtio_net_hdr),
-				    vio_le(), true, 0))
-		goto drop_n_account;
-
 	po->stats.stats1.tp_packets++;
 	if (copy_skb) {
 		status |= TP_STATUS_COPY;