From: Yan Zhai <yan@cloudflare.com>
To: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
Weongyo Jeong <weongyo.linux@gmail.com>,
linux-kernel@vger.kernel.org, kernel-team@cloudflare.com,
Jesper Brouer <jesper@cloudflare.com>
Subject: [PATCH v3 net-next] packet: add a generic drop reason for receive
Date: Fri, 1 Dec 2023 10:32:08 -0800 [thread overview]
Message-ID: <ZWomqO8m4vVcW+ro@debian.debian> (raw)
Commit da37845fdce2 ("packet: uses kfree_skb() for errors.") switches
from consume_skb to kfree_skb to improve error handling. However, this
could bring a lot of noises when we monitor real packet drops in
kfree_skb[1], because in tpacket_rcv or packet_rcv only packet clones
can be freed, not actual packets.
Adding a generic drop reason to allow distinguish these "clone drops".
[1]: https://lore.kernel.org/netdev/CABWYdi00L+O30Q=Zah28QwZ_5RU-xcxLFUK2Zj08A8MrLk9jzg@mail.gmail.com/
Fixes: da37845fdce2 ("packet: uses kfree_skb() for errors.")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Yan Zhai <yan@cloudflare.com>
---
v2->v3: removed an unused variable
v1->v2: fixups suggested by Eric Dumazet
v2: https://lore.kernel.org/netdev/ZWobMUp22oTpP3FW@debian.debian/
v1: https://lore.kernel.org/netdev/ZU3EZKQ3dyLE6T8z@debian.debian/
---
include/net/dropreason-core.h | 6 ++++++
net/packet/af_packet.c | 26 +++++++++++++-------------
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 3c70ad53a49c..278e4c7d465c 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -86,6 +86,7 @@
FN(IPV6_NDISC_NS_OTHERHOST) \
FN(QUEUE_PURGE) \
FN(TC_ERROR) \
+ FN(PACKET_SOCK_ERROR) \
FNe(MAX)
/**
@@ -378,6 +379,11 @@ enum skb_drop_reason {
SKB_DROP_REASON_QUEUE_PURGE,
/** @SKB_DROP_REASON_TC_ERROR: generic internal tc error. */
SKB_DROP_REASON_TC_ERROR,
+ /**
+ * @SKB_DROP_REASON_PACKET_SOCK_ERROR: generic packet socket errors
+ * after its filter matches an incoming packet.
+ */
+ SKB_DROP_REASON_PACKET_SOCK_ERROR,
/**
* @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which
* shouldn't be used as a real 'reason' - only for tracing code gen
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index a84e00b5904b..933fdfaacc44 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2127,7 +2127,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
u8 *skb_head = skb->data;
int skb_len = skb->len;
unsigned int snaplen, res;
- bool is_drop_n_account = false;
+ enum skb_drop_reason drop_reason = SKB_CONSUMED;
if (skb->pkt_type == PACKET_LOOPBACK)
goto drop;
@@ -2161,6 +2161,10 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
res = run_filter(skb, sk, snaplen);
if (!res)
goto drop_n_restore;
+
+ /* skb will only be "consumed" not "dropped" before this */
+ drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;
+
if (snaplen > res)
snaplen = res;
@@ -2217,7 +2221,6 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
return 0;
drop_n_acct:
- is_drop_n_account = true;
atomic_inc(&po->tp_drops);
atomic_inc(&sk->sk_drops);
@@ -2227,10 +2230,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
skb->len = skb_len;
}
drop:
- if (!is_drop_n_account)
- consume_skb(skb);
- else
- kfree_skb(skb);
+ kfree_skb_reason(skb, drop_reason);
return 0;
}
@@ -2250,9 +2250,9 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
struct sk_buff *copy_skb = NULL;
struct timespec64 ts;
__u32 ts_status;
- bool is_drop_n_account = false;
unsigned int slot_id = 0;
int vnet_hdr_sz = 0;
+ enum skb_drop_reason drop_reason = SKB_CONSUMED;
/* struct tpacket{2,3}_hdr is aligned to a multiple of TPACKET_ALIGNMENT.
* We may add members to them until current aligned size without forcing
@@ -2355,6 +2355,10 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
vnet_hdr_sz = 0;
}
}
+
+ /* skb will only be "consumed" not "dropped" before this */
+ drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;
+
spin_lock(&sk->sk_receive_queue.lock);
h.raw = packet_current_rx_frame(po, skb,
TP_STATUS_KERNEL, (macoff+snaplen));
@@ -2498,19 +2502,15 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
skb->len = skb_len;
}
drop:
- if (!is_drop_n_account)
- consume_skb(skb);
- else
- kfree_skb(skb);
+ kfree_skb_reason(skb, drop_reason);
return 0;
drop_n_account:
spin_unlock(&sk->sk_receive_queue.lock);
atomic_inc(&po->tp_drops);
- is_drop_n_account = true;
sk->sk_data_ready(sk);
- kfree_skb(copy_skb);
+ kfree_skb_reason(copy_skb, drop_reason);
goto drop_n_restore;
}
--
2.30.2
next reply other threads:[~2023-12-01 18:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-01 18:32 Yan Zhai [this message]
2023-12-02 10:39 ` [PATCH v3 net-next] packet: add a generic drop reason for receive Eric Dumazet
2023-12-02 14:07 ` Willem de Bruijn
2023-12-03 2:50 ` Yan Zhai
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=ZWomqO8m4vVcW+ro@debian.debian \
--to=yan@cloudflare.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jesper@cloudflare.com \
--cc=kernel-team@cloudflare.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=weongyo.linux@gmail.com \
--cc=willemdebruijn.kernel@gmail.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.