Messages in this thread | | | Date | Sat, 02 Dec 2023 09:07:23 -0500 | From | Willem de Bruijn <> | Subject | Re: [PATCH v3 net-next] packet: add a generic drop reason for receive |
| |
Yan Zhai wrote: > 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;
Reverse xmas tree
> > 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; > +
This can be set in drop_n_account, rather than the common path.
Same in tpacket_rcv.
| |