On 11/06/2024 22.11, Yan Zhai wrote:
skb does not include enough information to find out receiving
sockets/services and netns/containers on packet drops. In theory
skb->dev tells about netns, but it can get cleared/reused, e.g. by TCP
stack for OOO packet lookup. Similarly, skb->sk often identifies a local
sender, and tells nothing about a receiver.
Allow passing an extra receiving socket to the tracepoint to improve
the visibility on receiving drops.
Signed-off-by: Yan Zhai<[email protected]>
---
v3->v4: adjusted the TP_STRUCT field order to be consistent
v2->v3: fixed drop_monitor function prototype
---
include/trace/events/skb.h | 11 +++++++----
net/core/dev.c | 2 +-
net/core/drop_monitor.c | 9 ++++++---
net/core/skbuff.c | 2 +-
4 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 07e0715628ec..3e9ea1cca6f2 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -24,13 +24,14 @@ DEFINE_DROP_REASON(FN, FN)
TRACE_EVENT(kfree_skb,
TP_PROTO(struct sk_buff *skb, void *location,
- enum skb_drop_reason reason),
+ enum skb_drop_reason reason, struct sock *rx_sk),
- TP_ARGS(skb, location, reason),
+ TP_ARGS(skb, location, reason, rx_sk),
TP_STRUCT__entry(
__field(void *, skbaddr)
__field(void *, location)
+ __field(void *, rx_skaddr)
Is there any reason for appending the "addr" part to "rx_sk" ?
It makes it harder to read this is the sk (socket).
AFAICR the skbaddr naming is a legacy thing.
__field(unsigned short, protocol)
__field(enum skb_drop_reason, reason)
),
@@ -38,12 +39,14 @@ TRACE_EVENT(kfree_skb,
TP_fast_assign(
__entry->skbaddr = skb;
__entry->location = location;
+ __entry->rx_skaddr = rx_sk;
__entry->protocol = ntohs(skb->protocol);
__entry->reason = reason;
),
- TP_printk("skbaddr=%p protocol=%u location=%pS reason: %s",
- __entry->skbaddr, __entry->protocol, __entry->location,
+ TP_printk("skbaddr=%p rx_skaddr=%p protocol=%u location=%pS reason: %s",
^^^^^^^^^
I find it hard to visually tell skbaddr and rx_skaddr apart.
And especially noticing the "skb" vs "sk" part of the string.
+ __entry->skbaddr, __entry->rx_skaddr, __entry->protocol,
+ __entry->location,
__print_symbolic(__entry->reason,
DEFINE_DROP_REASON(FN, FNe)))
);
--Jesper
