From: Menglong Dong <[email protected]>
In this series patches, kfree_skb() is replaced with kfree_skb_reason()
during ipv4 and udp4 packet receiving path, and following drop reasons
are introduced:
SKB_DROP_REASON_SOCKET_FILTER
SKB_DROP_REASON_NETFILTER_DROP
SKB_DROP_REASON_OTHERHOST
SKB_DROP_REASON_IP_CSUM
SKB_DROP_REASON_IP_INHDR
SKB_DROP_REASON_IP_RPFILTER
SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST
SKB_DROP_REASON_XFRM_POLICY
SKB_DROP_REASON_IP_NOPROTO
SKB_DROP_REASON_SOCKET_RCVBUFF
SKB_DROP_REASON_PROTO_MEM
TCP is more complex, so I left it in the next series.
I just figure out how __print_symbolic() works. It doesn't base on the
array index, but searching for symbols by loop. So I'm a little afraid
it's performance.
Changes since v2:
- use SKB_DROP_REASON_PKT_TOO_SMALL for a path in ip_rcv_core()
Changes since v1:
- add document for all drop reasons, as David advised
- remove unreleated cleanup
- remove EARLY_DEMUX and IP_ROUTE_INPUT drop reason
- replace {UDP, TCP}_FILTER with SOCKET_FILTER
Menglong Dong (7):
net: skb_drop_reason: add document for drop reasons
net: netfilter: use kfree_drop_reason() for NF_DROP
net: ipv4: use kfree_skb_reason() in ip_rcv_core()
net: ipv4: use kfree_skb_reason() in ip_rcv_finish_core()
net: ipv4: use kfree_skb_reason() in ip_protocol_deliver_rcu()
net: udp: use kfree_skb_reason() in udp_queue_rcv_one_skb()
net: udp: use kfree_skb_reason() in __udp_queue_rcv_skb()
include/linux/skbuff.h | 38 ++++++++++++++++++++++++++++++++------
include/trace/events/skb.h | 11 +++++++++++
net/ipv4/ip_input.c | 31 +++++++++++++++++++++++--------
net/ipv4/udp.c | 22 ++++++++++++++++------
net/netfilter/core.c | 3 ++-
5 files changed, 84 insertions(+), 21 deletions(-)
--
2.27.0
From: Menglong Dong <[email protected]>
Add document for following existing drop reasons:
SKB_DROP_REASON_NOT_SPECIFIED
SKB_DROP_REASON_NO_SOCKET
SKB_DROP_REASON_PKT_TOO_SMALL
SKB_DROP_REASON_TCP_CSUM
SKB_DROP_REASON_SOCKET_FILTER
SKB_DROP_REASON_UDP_CSUM
Signed-off-by: Menglong Dong <[email protected]>
---
include/linux/skbuff.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8a636e678902..5c5615a487e7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -314,12 +314,12 @@ struct sk_buff;
* used to translate the reason to string.
*/
enum skb_drop_reason {
- SKB_DROP_REASON_NOT_SPECIFIED,
- SKB_DROP_REASON_NO_SOCKET,
- SKB_DROP_REASON_PKT_TOO_SMALL,
- SKB_DROP_REASON_TCP_CSUM,
- SKB_DROP_REASON_SOCKET_FILTER,
- SKB_DROP_REASON_UDP_CSUM,
+ SKB_DROP_REASON_NOT_SPECIFIED, /* drop reason is not specified */
+ SKB_DROP_REASON_NO_SOCKET, /* socket not found */
+ SKB_DROP_REASON_PKT_TOO_SMALL, /* packet size is too small */
+ SKB_DROP_REASON_TCP_CSUM, /* TCP checksum error */
+ SKB_DROP_REASON_SOCKET_FILTER, /* dropped by socket filter */
+ SKB_DROP_REASON_UDP_CSUM, /* UDP checksum error */
SKB_DROP_REASON_MAX,
};
--
2.34.1
From: Menglong Dong <[email protected]>
Replace kfree_skb() with kfree_skb_reason() in nf_hook_slow() when
skb is dropped by reason of NF_DROP. Following new drop reasons
are introduced:
SKB_DROP_REASON_NETFILTER_DROP
Signed-off-by: Menglong Dong <[email protected]>
---
v2:
- add document for SKB_DROP_REASON_NETFILTER_DROP
---
include/linux/skbuff.h | 1 +
include/trace/events/skb.h | 1 +
net/netfilter/core.c | 3 ++-
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5c5615a487e7..786ea2c2334e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -320,6 +320,7 @@ enum skb_drop_reason {
SKB_DROP_REASON_TCP_CSUM, /* TCP checksum error */
SKB_DROP_REASON_SOCKET_FILTER, /* dropped by socket filter */
SKB_DROP_REASON_UDP_CSUM, /* UDP checksum error */
+ SKB_DROP_REASON_NETFILTER_DROP, /* dropped by netfilter */
SKB_DROP_REASON_MAX,
};
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index a8a64b97504d..3d89f7b09a43 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -16,6 +16,7 @@
EM(SKB_DROP_REASON_TCP_CSUM, TCP_CSUM) \
EM(SKB_DROP_REASON_SOCKET_FILTER, SOCKET_FILTER) \
EM(SKB_DROP_REASON_UDP_CSUM, UDP_CSUM) \
+ EM(SKB_DROP_REASON_NETFILTER_DROP, NETFILTER_DROP) \
EMe(SKB_DROP_REASON_MAX, MAX)
#undef EM
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 354cb472f386..d1c9dfbb11fa 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -621,7 +621,8 @@ int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state,
case NF_ACCEPT:
break;
case NF_DROP:
- kfree_skb(skb);
+ kfree_skb_reason(skb,
+ SKB_DROP_REASON_NETFILTER_DROP);
ret = NF_DROP_GETERR(verdict);
if (ret == 0)
ret = -EPERM;
--
2.34.1
From: Menglong Dong <[email protected]>
Replace kfree_skb() with kfree_skb_reason() in ip_rcv_core(). Three new
drop reasons are introduced:
SKB_DROP_REASON_OTHERHOST
SKB_DROP_REASON_IP_CSUM
SKB_DROP_REASON_IP_INHDR
Signed-off-by: Menglong Dong <[email protected]>
---
v3:
- add a path to SKB_DROP_REASON_PKT_TOO_SMALL
v2:
- remove unrelated cleanup
- add document for introduced drop reasons
---
include/linux/skbuff.h | 9 +++++++++
include/trace/events/skb.h | 3 +++
net/ipv4/ip_input.c | 12 ++++++++++--
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 786ea2c2334e..2e87da91424f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -321,6 +321,15 @@ enum skb_drop_reason {
SKB_DROP_REASON_SOCKET_FILTER, /* dropped by socket filter */
SKB_DROP_REASON_UDP_CSUM, /* UDP checksum error */
SKB_DROP_REASON_NETFILTER_DROP, /* dropped by netfilter */
+ SKB_DROP_REASON_OTHERHOST, /* packet don't belong to current
+ * host (interface is in promisc
+ * mode)
+ */
+ SKB_DROP_REASON_IP_CSUM, /* IP checksum error */
+ SKB_DROP_REASON_IP_INHDR, /* there is something wrong with
+ * IP header (see
+ * IPSTATS_MIB_INHDRERRORS)
+ */
SKB_DROP_REASON_MAX,
};
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 3d89f7b09a43..f2b1778485f0 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -17,6 +17,9 @@
EM(SKB_DROP_REASON_SOCKET_FILTER, SOCKET_FILTER) \
EM(SKB_DROP_REASON_UDP_CSUM, UDP_CSUM) \
EM(SKB_DROP_REASON_NETFILTER_DROP, NETFILTER_DROP) \
+ EM(SKB_DROP_REASON_OTHERHOST, OTHERHOST) \
+ EM(SKB_DROP_REASON_IP_CSUM, IP_CSUM) \
+ EM(SKB_DROP_REASON_IP_INHDR, IP_INHDR) \
EMe(SKB_DROP_REASON_MAX, MAX)
#undef EM
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 3a025c011971..627fad437593 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -436,13 +436,18 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
{
const struct iphdr *iph;
+ int drop_reason;
u32 len;
+ drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
+
/* When the interface is in promisc. mode, drop all the crap
* that it receives, do not try to analyse it.
*/
- if (skb->pkt_type == PACKET_OTHERHOST)
+ if (skb->pkt_type == PACKET_OTHERHOST) {
+ drop_reason = SKB_DROP_REASON_OTHERHOST;
goto drop;
+ }
__IP_UPD_PO_STATS(net, IPSTATS_MIB_IN, skb->len);
@@ -488,6 +493,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
len = ntohs(iph->tot_len);
if (skb->len < len) {
+ drop_reason = SKB_DROP_REASON_PKT_TOO_SMALL;
__IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
goto drop;
} else if (len < (iph->ihl*4))
@@ -516,11 +522,13 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
return skb;
csum_error:
+ drop_reason = SKB_DROP_REASON_IP_CSUM;
__IP_INC_STATS(net, IPSTATS_MIB_CSUMERRORS);
inhdr_error:
+ drop_reason = drop_reason ?: SKB_DROP_REASON_IP_INHDR;
__IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS);
drop:
- kfree_skb(skb);
+ kfree_skb_reason(skb, drop_reason);
out:
return NULL;
}
--
2.34.1
From: Menglong Dong <[email protected]>
Replace kfree_skb() with kfree_skb_reason() in ip_rcv_finish_core(),
following drop reasons are introduced:
SKB_DROP_REASON_IP_RPFILTER
SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST
Signed-off-by: Menglong Dong <[email protected]>
---
v2:
- remove SKB_DROP_REASON_EARLY_DEMUX and SKB_DROP_REASON_IP_ROUTE_INPUT
- add document for SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST and
SKB_DROP_REASON_IP_RPFILTER
---
include/linux/skbuff.h | 9 +++++++++
include/trace/events/skb.h | 3 +++
net/ipv4/ip_input.c | 14 ++++++++++----
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e87da91424f..2d712459d564 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -330,6 +330,15 @@ enum skb_drop_reason {
* IP header (see
* IPSTATS_MIB_INHDRERRORS)
*/
+ SKB_DROP_REASON_IP_RPFILTER, /* IP rpfilter validate failed.
+ * see the document for rp_filter
+ * in ip-sysctl.rst for more
+ * information
+ */
+ SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST, /* destination address of L2
+ * is multicast, but L3 is
+ * unicast.
+ */
SKB_DROP_REASON_MAX,
};
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index f2b1778485f0..485a1d3034a4 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -20,6 +20,9 @@
EM(SKB_DROP_REASON_OTHERHOST, OTHERHOST) \
EM(SKB_DROP_REASON_IP_CSUM, IP_CSUM) \
EM(SKB_DROP_REASON_IP_INHDR, IP_INHDR) \
+ EM(SKB_DROP_REASON_IP_RPFILTER, IP_RPFILTER) \
+ EM(SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST, \
+ UNICAST_IN_L2_MULTICAST) \
EMe(SKB_DROP_REASON_MAX, MAX)
#undef EM
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 7f64c5432cba..184decb1c8eb 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -318,8 +318,10 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
{
const struct iphdr *iph = ip_hdr(skb);
int (*edemux)(struct sk_buff *skb);
+ int err, drop_reason;
struct rtable *rt;
- int err;
+
+ drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
if (ip_can_use_hint(skb, iph, hint)) {
err = ip_route_use_hint(skb, iph->daddr, iph->saddr, iph->tos,
@@ -396,19 +398,23 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
* so-called "hole-196" attack) so do it for both.
*/
if (in_dev &&
- IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST))
+ IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
+ drop_reason = SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
goto drop;
+ }
}
return NET_RX_SUCCESS;
drop:
- kfree_skb(skb);
+ kfree_skb_reason(skb, drop_reason);
return NET_RX_DROP;
drop_error:
- if (err == -EXDEV)
+ if (err == -EXDEV) {
+ drop_reason = SKB_DROP_REASON_IP_RPFILTER;
__NET_INC_STATS(net, LINUX_MIB_IPRPFILTER);
+ }
goto drop;
}
--
2.34.1
From: Menglong Dong <[email protected]>
Replace kfree_skb() with kfree_skb_reason() in ip_protocol_deliver_rcu().
Following new drop reasons are introduced:
SKB_DROP_REASON_XFRM_POLICY
SKB_DROP_REASON_IP_NOPROTO
Signed-off-by: Menglong Dong <[email protected]>
---
v2:
- add document for the introduced drop reasons
---
include/linux/skbuff.h | 2 ++
include/trace/events/skb.h | 2 ++
net/ipv4/ip_input.c | 5 +++--
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2d712459d564..4e55321e2fc2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -339,6 +339,8 @@ enum skb_drop_reason {
* is multicast, but L3 is
* unicast.
*/
+ SKB_DROP_REASON_XFRM_POLICY, /* xfrm policy check failed */
+ SKB_DROP_REASON_IP_NOPROTO, /* no support for IP protocol */
SKB_DROP_REASON_MAX,
};
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 485a1d3034a4..985e481c092d 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -23,6 +23,8 @@
EM(SKB_DROP_REASON_IP_RPFILTER, IP_RPFILTER) \
EM(SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST, \
UNICAST_IN_L2_MULTICAST) \
+ EM(SKB_DROP_REASON_XFRM_POLICY, XFRM_POLICY) \
+ EM(SKB_DROP_REASON_IP_NOPROTO, IP_NOPROTO) \
EMe(SKB_DROP_REASON_MAX, MAX)
#undef EM
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 184decb1c8eb..74c090f6eb9d 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -196,7 +196,8 @@ void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int protocol)
if (ipprot) {
if (!ipprot->no_policy) {
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
- kfree_skb(skb);
+ kfree_skb_reason(skb,
+ SKB_DROP_REASON_XFRM_POLICY);
return;
}
nf_reset_ct(skb);
@@ -215,7 +216,7 @@ void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int protocol)
icmp_send(skb, ICMP_DEST_UNREACH,
ICMP_PROT_UNREACH, 0);
}
- kfree_skb(skb);
+ kfree_skb_reason(skb, SKB_DROP_REASON_IP_NOPROTO);
} else {
__IP_INC_STATS(net, IPSTATS_MIB_INDELIVERS);
consume_skb(skb);
--
2.34.1
From: Menglong Dong <[email protected]>
Replace kfree_skb() with kfree_skb_reason() in udp_queue_rcv_one_skb().
Signed-off-by: Menglong Dong <[email protected]>
---
v2:
- use SKB_DROP_REASON_SOCKET_FILTER instead of
SKB_DROP_REASON_UDP_FILTER
---
net/ipv4/udp.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 464590ea922e..9e28e76e95b8 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2120,14 +2120,17 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
*/
static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
{
+ int drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
struct udp_sock *up = udp_sk(sk);
int is_udplite = IS_UDPLITE(sk);
/*
* Charge it to the socket, dropping if the queue is full.
*/
- if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
+ if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb)) {
+ drop_reason = SKB_DROP_REASON_XFRM_POLICY;
goto drop;
+ }
nf_reset_ct(skb);
if (static_branch_unlikely(&udp_encap_needed_key) && up->encap_type) {
@@ -2204,8 +2207,10 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
udp_lib_checksum_complete(skb))
goto csum_error;
- if (sk_filter_trim_cap(sk, skb, sizeof(struct udphdr)))
+ if (sk_filter_trim_cap(sk, skb, sizeof(struct udphdr))) {
+ drop_reason = SKB_DROP_REASON_SOCKET_FILTER;
goto drop;
+ }
udp_csum_pull_header(skb);
@@ -2213,11 +2218,12 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
return __udp_queue_rcv_skb(sk, skb);
csum_error:
+ drop_reason = SKB_DROP_REASON_UDP_CSUM;
__UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
drop:
__UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
atomic_inc(&sk->sk_drops);
- kfree_skb(skb);
+ kfree_skb_reason(skb, drop_reason);
return -1;
}
--
2.34.1
From: Menglong Dong <[email protected]>
Replace kfree_skb() with kfree_skb_reason() in __udp_queue_rcv_skb().
Following new drop reasons are introduced:
SKB_DROP_REASON_SOCKET_RCVBUFF
SKB_DROP_REASON_PROTO_MEM
Signed-off-by: Menglong Dong <[email protected]>
---
include/linux/skbuff.h | 5 +++++
include/trace/events/skb.h | 2 ++
net/ipv4/udp.c | 10 +++++++---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 4e55321e2fc2..2390f6e230fb 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -341,6 +341,11 @@ enum skb_drop_reason {
*/
SKB_DROP_REASON_XFRM_POLICY, /* xfrm policy check failed */
SKB_DROP_REASON_IP_NOPROTO, /* no support for IP protocol */
+ SKB_DROP_REASON_SOCKET_RCVBUFF, /* socket receive buff is full */
+ SKB_DROP_REASON_PROTO_MEM, /* proto memory limition, such as
+ * udp packet drop out of
+ * udp_memory_allocated.
+ */
SKB_DROP_REASON_MAX,
};
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 985e481c092d..cfcfd26399f7 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -25,6 +25,8 @@
UNICAST_IN_L2_MULTICAST) \
EM(SKB_DROP_REASON_XFRM_POLICY, XFRM_POLICY) \
EM(SKB_DROP_REASON_IP_NOPROTO, IP_NOPROTO) \
+ EM(SKB_DROP_REASON_SOCKET_RCVBUFF, SOCKET_RCVBUFF) \
+ EM(SKB_DROP_REASON_PROTO_MEM, PROTO_MEM) \
EMe(SKB_DROP_REASON_MAX, MAX)
#undef EM
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e295f7f38398..1f756bb0bb1f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2093,16 +2093,20 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
rc = __udp_enqueue_schedule_skb(sk, skb);
if (rc < 0) {
int is_udplite = IS_UDPLITE(sk);
+ int drop_reason;
/* Note that an ENOMEM error is charged twice */
- if (rc == -ENOMEM)
+ if (rc == -ENOMEM) {
UDP_INC_STATS(sock_net(sk), UDP_MIB_RCVBUFERRORS,
is_udplite);
- else
+ drop_reason = SKB_DROP_REASON_SOCKET_RCVBUFF;
+ } else {
UDP_INC_STATS(sock_net(sk), UDP_MIB_MEMERRORS,
is_udplite);
+ drop_reason = SKB_DROP_REASON_PROTO_MEM;
+ }
UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
- kfree_skb(skb);
+ kfree_skb_reason(skb, drop_reason);
trace_udp_fail_queue_rcv_skb(rc, sk);
return -1;
}
--
2.34.1
On 1/28/22 12:33 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Add document for following existing drop reasons:
>
> SKB_DROP_REASON_NOT_SPECIFIED
> SKB_DROP_REASON_NO_SOCKET
> SKB_DROP_REASON_PKT_TOO_SMALL
> SKB_DROP_REASON_TCP_CSUM
> SKB_DROP_REASON_SOCKET_FILTER
> SKB_DROP_REASON_UDP_CSUM
>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> include/linux/skbuff.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
Reviewed-by: David Ahern <[email protected]>
On 1/28/22 12:33 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Replace kfree_skb() with kfree_skb_reason() in ip_protocol_deliver_rcu().
> Following new drop reasons are introduced:
>
> SKB_DROP_REASON_XFRM_POLICY
> SKB_DROP_REASON_IP_NOPROTO
>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> v2:
> - add document for the introduced drop reasons
> ---
> include/linux/skbuff.h | 2 ++
> include/trace/events/skb.h | 2 ++
> net/ipv4/ip_input.c | 5 +++--
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
Reviewed-by: David Ahern <[email protected]>
On 1/28/22 12:33 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Replace kfree_skb() with kfree_skb_reason() in udp_queue_rcv_one_skb().
>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> v2:
> - use SKB_DROP_REASON_SOCKET_FILTER instead of
> SKB_DROP_REASON_UDP_FILTER
> ---
> net/ipv4/udp.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
Reviewed-by: David Ahern <[email protected]>
On 1/28/22 12:33 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Replace kfree_skb() with kfree_skb_reason() in __udp_queue_rcv_skb().
> Following new drop reasons are introduced:
>
> SKB_DROP_REASON_SOCKET_RCVBUFF
> SKB_DROP_REASON_PROTO_MEM
>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> include/linux/skbuff.h | 5 +++++
> include/trace/events/skb.h | 2 ++
> net/ipv4/udp.c | 10 +++++++---
> 3 files changed, 14 insertions(+), 3 deletions(-)
>
Reviewed-by: David Ahern <[email protected]>
On 1/28/22 12:33 AM, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> Replace kfree_skb() with kfree_skb_reason() in ip_rcv_finish_core(),
> following drop reasons are introduced:
>
> SKB_DROP_REASON_IP_RPFILTER
> SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST
>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> v2:
> - remove SKB_DROP_REASON_EARLY_DEMUX and SKB_DROP_REASON_IP_ROUTE_INPUT
> - add document for SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST and
> SKB_DROP_REASON_IP_RPFILTER
> ---
> include/linux/skbuff.h | 9 +++++++++
> include/trace/events/skb.h | 3 +++
> net/ipv4/ip_input.c | 14 ++++++++++----
> 3 files changed, 22 insertions(+), 4 deletions(-)
>
Reviewed-by: David Ahern <[email protected]>
On 1/28/22 12:33 AM, [email protected] wrote:
\> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index 3a025c011971..627fad437593 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -436,13 +436,18 @@ static int ip_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
> static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
> {
> const struct iphdr *iph;
> + int drop_reason;
> u32 len;
>
> + drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
move this line down, right before:
if (!pskb_may_pull(skb, sizeof(struct iphdr)))
goto inhdr_error;
> +
> /* When the interface is in promisc. mode, drop all the crap
> * that it receives, do not try to analyse it.
> */
> - if (skb->pkt_type == PACKET_OTHERHOST)
> + if (skb->pkt_type == PACKET_OTHERHOST) {
> + drop_reason = SKB_DROP_REASON_OTHERHOST;
> goto drop;
> + }
>
> __IP_UPD_PO_STATS(net, IPSTATS_MIB_IN, skb->len);
>
> @@ -488,6 +493,7 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
>
> len = ntohs(iph->tot_len);
> if (skb->len < len) {
> + drop_reason = SKB_DROP_REASON_PKT_TOO_SMALL;
> __IP_INC_STATS(net, IPSTATS_MIB_INTRUNCATEDPKTS);
> goto drop;
> } else if (len < (iph->ihl*4))
> @@ -516,11 +522,13 @@ static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
> return skb;
>
> csum_error:
> + drop_reason = SKB_DROP_REASON_IP_CSUM;
> __IP_INC_STATS(net, IPSTATS_MIB_CSUMERRORS);
> inhdr_error:
> + drop_reason = drop_reason ?: SKB_DROP_REASON_IP_INHDR;
That makes assumptions about the value of SKB_DROP_REASON_NOT_SPECIFIED.
Make that line:
if (drop_reason != SKB_DROP_REASON_NOT_SPECIFIED)
drop_reason = SKB_DROP_REASON_IP_INHDR;
> __IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS);
> drop:
> - kfree_skb(skb);
> + kfree_skb_reason(skb, drop_reason);
> out:
> return NULL;
> }
Hello!
On Tue, Feb 1, 2022 at 1:14 AM David Ahern <[email protected]> wrote:
>
> On 1/28/22 12:33 AM, [email protected] wrote:
> > From: Menglong Dong <[email protected]>
> >
> > Add document for following existing drop reasons:
> >
> > SKB_DROP_REASON_NOT_SPECIFIED
> > SKB_DROP_REASON_NO_SOCKET
> > SKB_DROP_REASON_PKT_TOO_SMALL
> > SKB_DROP_REASON_TCP_CSUM
> > SKB_DROP_REASON_SOCKET_FILTER
> > SKB_DROP_REASON_UDP_CSUM
> >
> > Signed-off-by: Menglong Dong <[email protected]>
> > ---
> > include/linux/skbuff.h | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
>
> Reviewed-by: David Ahern <[email protected]>
>
>
I'm doing the job of using kfree_skb_reason() for the TCP layer,
and I have some puzzles.
When collecting drop reason for tcp_v4_inbound_md5_hash() in
tcp_v4_rcv(), I come up with 2 ways:
First way: pass the address of reason to tcp_v4_inbound_md5_hash()
like this:
static bool tcp_v4_inbound_md5_hash(const struct sock *sk,
const struct sk_buff *skb,
- int dif, int sdif)
+ int dif, int sdif,
+ enum skb_drop_reason *reason)
This can work, but many functions like tcp_v4_inbound_md5_hash()
need to do such a change.
Second way: introduce a 'drop_reason' field to 'struct sk_buff'. Therefore,
drop reason can be set by 'skb->drop_reason = SKB_DROP_REASON_XXX'
anywhere.
For TCP, there are many cases where you can't get a drop reason in
the place where skb is freed, so I think there needs to be a way to
deeply collect drop reasons. The second can resolve this problem
easily, but extra fields may have performance problems.
Do you have some better ideas?
Thanks!
Menglong Dong
On Thu, 10 Feb 2022 11:19:49 +0800 Menglong Dong wrote:
> I'm doing the job of using kfree_skb_reason() for the TCP layer,
> and I have some puzzles.
>
> When collecting drop reason for tcp_v4_inbound_md5_hash() in
> tcp_v4_rcv(), I come up with 2 ways:
>
> First way: pass the address of reason to tcp_v4_inbound_md5_hash()
> like this:
>
> static bool tcp_v4_inbound_md5_hash(const struct sock *sk,
> const struct sk_buff *skb,
> - int dif, int sdif)
> + int dif, int sdif,
> + enum skb_drop_reason *reason)
>
> This can work, but many functions like tcp_v4_inbound_md5_hash()
> need to do such a change.
>
> Second way: introduce a 'drop_reason' field to 'struct sk_buff'. Therefore,
> drop reason can be set by 'skb->drop_reason = SKB_DROP_REASON_XXX'
> anywhere.
>
> For TCP, there are many cases where you can't get a drop reason in
> the place where skb is freed, so I think there needs to be a way to
> deeply collect drop reasons. The second can resolve this problem
> easily, but extra fields may have performance problems.
>
> Do you have some better ideas?
On a quick look tcp_v4_inbound_md5_hash() returns a drop / no drop
decision, so you could just change the return type to enum
skb_drop_reason. SKB_DROP_REASON_NOT_SPECIFIED is 0 is false,
so if (reason) goto drop; logic will hold up.
On Thu, 10 Feb 2022 21:42:14 +0800 Menglong Dong wrote:
> How about introducing a field to 'struct sock' for drop reasons? As sk is
> locked during the packet process in tcp_v4_do_rcv(), this seems to work.
I find adding temporary storage to persistent data structures awkward.
You can put a structure on the stack and pass it thru the call chain,
that's just my subjective preference, tho, others may have better ideas.
On Fri, Feb 11, 2022 at 12:13 AM Jakub Kicinski <[email protected]> wrote:
>
> On Thu, 10 Feb 2022 21:42:14 +0800 Menglong Dong wrote:
> > How about introducing a field to 'struct sock' for drop reasons? As sk is
> > locked during the packet process in tcp_v4_do_rcv(), this seems to work.
>
> I find adding temporary storage to persistent data structures awkward.
> You can put a structure on the stack and pass it thru the call chain,
> that's just my subjective preference, tho, others may have better ideas.
Yes, I also feel it is awkward. I'll try to do this job by passing drop reasons
through the call chain. Thanks for your help :)
On Fri, Feb 11, 2022 at 12:29 AM Eric Dumazet <[email protected]> wrote:
>
> On Thu, Feb 10, 2022 at 8:13 AM Jakub Kicinski <[email protected]> wrote:
> >
> > On Thu, 10 Feb 2022 21:42:14 +0800 Menglong Dong wrote:
> > > How about introducing a field to 'struct sock' for drop reasons? As sk is
> > > locked during the packet process in tcp_v4_do_rcv(), this seems to work.
> >
> > I find adding temporary storage to persistent data structures awkward.
> > You can put a structure on the stack and pass it thru the call chain,
> > that's just my subjective preference, tho, others may have better ideas.
>
> I had a similar TODO item, because stuff like 'waking up task' or free
> one skb (or list of skb) could be performed outside of socket lock.
May I ask what it's like? Is it used to solve this kind of problem?
Thanks!
Menglong Dong
On Thu, Feb 10, 2022 at 8:13 AM Jakub Kicinski <[email protected]> wrote:
>
> On Thu, 10 Feb 2022 21:42:14 +0800 Menglong Dong wrote:
> > How about introducing a field to 'struct sock' for drop reasons? As sk is
> > locked during the packet process in tcp_v4_do_rcv(), this seems to work.
>
> I find adding temporary storage to persistent data structures awkward.
> You can put a structure on the stack and pass it thru the call chain,
> that's just my subjective preference, tho, others may have better ideas.
I had a similar TODO item, because stuff like 'waking up task' or free
one skb (or list of skb) could be performed outside of socket lock.
On Thu, Feb 10, 2022 at 1:12 PM Jakub Kicinski <[email protected]> wrote:
>
> On Thu, 10 Feb 2022 11:19:49 +0800 Menglong Dong wrote:
> > I'm doing the job of using kfree_skb_reason() for the TCP layer,
> > and I have some puzzles.
> >
> > When collecting drop reason for tcp_v4_inbound_md5_hash() in
> > tcp_v4_rcv(), I come up with 2 ways:
> >
> > First way: pass the address of reason to tcp_v4_inbound_md5_hash()
> > like this:
> >
> > static bool tcp_v4_inbound_md5_hash(const struct sock *sk,
> > const struct sk_buff *skb,
> > - int dif, int sdif)
> > + int dif, int sdif,
> > + enum skb_drop_reason *reason)
> >
> > This can work, but many functions like tcp_v4_inbound_md5_hash()
> > need to do such a change.
> >
> > Second way: introduce a 'drop_reason' field to 'struct sk_buff'. Therefore,
> > drop reason can be set by 'skb->drop_reason = SKB_DROP_REASON_XXX'
> > anywhere.
> >
> > For TCP, there are many cases where you can't get a drop reason in
> > the place where skb is freed, so I think there needs to be a way to
> > deeply collect drop reasons. The second can resolve this problem
> > easily, but extra fields may have performance problems.
> >
> > Do you have some better ideas?
>
> On a quick look tcp_v4_inbound_md5_hash() returns a drop / no drop
> decision, so you could just change the return type to enum
> skb_drop_reason. SKB_DROP_REASON_NOT_SPECIFIED is 0 is false,
> so if (reason) goto drop; logic will hold up.
Yeah, that's an idea. But some functions are more complex, such as
tcp_rcv_state_process() and tcp_rcv_state_process()->tcp_v4_conn_request().
The return value of tcp_rcv_state_process() can't be reused, and it's hard
to add a function param of type 'enum skb_drop_reason *' to
tcp_v4_conn_request().
There are some nice drop reasons in tcp_v4_conn_request(), it's a pity to
give up them.
How about introducing a field to 'struct sock' for drop reasons? As sk is
locked during the packet process in tcp_v4_do_rcv(), this seems to work.
Thanks!
Menglong Dong