All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, mst@redhat.com,
	Willem de Bruijn <willemb@google.com>
Subject: [PATCH net] net/packet: tpacket_rcv: do not increment ring index on drop
Date: Mon,  9 Mar 2020 11:34:35 -0400	[thread overview]
Message-ID: <20200309153435.32109-1-willemdebruijn.kernel@gmail.com> (raw)

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 --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;
-- 
2.25.1.481.gfbce0eb801-goog


             reply	other threads:[~2020-03-09 15:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09 15:34 Willem de Bruijn [this message]
2020-03-09 15:42 ` [PATCH net] net/packet: tpacket_rcv: do not increment ring index on drop Willem de Bruijn
2020-03-09 15:50   ` Willem de Bruijn
2020-03-10  6:46     ` Michael S. Tsirkin
2020-03-10  6:43 ` Michael S. Tsirkin
2020-03-10 12:49   ` Willem de Bruijn
2020-03-10 12:59     ` Michael S. Tsirkin
2020-03-10 14:16       ` Willem de Bruijn
2020-03-10 14:43         ` Michael S. Tsirkin
2020-03-10 15:38           ` Willem de Bruijn
2020-03-10 16:14             ` Willem de Bruijn
2020-03-10 21:29             ` Michael S. Tsirkin
2020-03-10 21:35               ` Willem de Bruijn
2020-03-10 21:57                 ` Michael S. Tsirkin
2020-03-10 23:13                   ` Willem de Bruijn
2020-03-11  7:56                     ` Michael S. Tsirkin
2020-03-11 14:31                       ` Willem de Bruijn
2020-03-11 21:25                         ` Michael S. Tsirkin
2020-03-11 21:49                           ` Willem de Bruijn
2020-03-12  6:13 ` David Miller
2020-03-12  6:27   ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200309153435.32109-1-willemdebruijn.kernel@gmail.com \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=davem@davemloft.net \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.