Commit 2f57dd94 authored by Yan Zhai's avatar Yan Zhai Committed by Jakub Kicinski
Browse files

packet: add a generic drop reason for receive 

Commit da37845f ("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: da37845f ("packet: uses kfree_skb() for errors.")
Suggested-by: default avatarEric Dumazet <edumazet@google.com>
Suggested-by: default avatarWillem de Bruijn <willemdebruijn.kernel@gmail.com>
Signed-off-by: default avatarYan Zhai <yan@cloudflare.com>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Reviewed-by: default avatarWillem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/r/ZW4piNbx3IenYnuw@debian.debian


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 19b707c3

include/net/dropreason-core.h

+6 −0
Original line number Diff line number Diff line
@@ -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

net/packet/af_packet.c

+7 −13
Original line number Diff line number Diff line
@@ -2121,13 +2121,13 @@ static int packet_rcv_vnet(struct msghdr *msg, const struct sk_buff *skb, 
static int packet_rcv(struct sk_buff *skb, struct net_device *dev,

		      struct packet_type *pt, struct net_device *orig_dev)

{

	enum skb_drop_reason drop_reason = SKB_CONSUMED;

	struct sock *sk;

	struct sockaddr_ll *sll;

	struct packet_sock *po;

	u8 *skb_head = skb->data;

	int skb_len = skb->len;

	unsigned int snaplen, res;

	bool is_drop_n_account = false;



	if (skb->pkt_type == PACKET_LOOPBACK)

		goto drop;
@@ -2217,9 +2217,9 @@ 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);

	drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;



drop_n_restore:

	if (skb_head != skb->data && skb_shared(skb)) {
@@ -2227,16 +2227,14 @@ 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;

}



static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,

		       struct packet_type *pt, struct net_device *orig_dev)

{

	enum skb_drop_reason drop_reason = SKB_CONSUMED;

	struct sock *sk;

	struct packet_sock *po;

	struct sockaddr_ll *sll;
@@ -2250,7 +2248,6 @@ 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;
@@ -2498,19 +2495,16 @@ 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;

	drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;



	sk->sk_data_ready(sk);

	kfree_skb(copy_skb);

	kfree_skb_reason(copy_skb, drop_reason);

	goto drop_n_restore;

}