quoted

quoted

quoted

---------
v2->v3:
Some fixes according to
https://lore.kernel.org/all/20240319102549.7f7f6f53@gandalf.local.home=

/

quoted

quoted

quoted

1. Change the tracking directory to/sys/kernel/tracking.
2. Adjust the layout of the TP-STRUCT_entry parameter structure.

v1->v2:
Some fixes according to
https://lore.kernel.org/all/CANn89iL-y9e_VFpdw=3D3DsZtRnKRu_tnUwqHuFQT=

JvJsv=3D

quoted

quoted

-nz1xPDw@mail.gmail.com/

quoted

1. adjust the trace_icmp_send() to more protocols than UDP.
2. move the calling of trace_icmp_send after sanity checks
in __icmp_send().

Signed-off-by: Peilin He<redacted>
Reviewed-by: xu xin <xu.xin16@zte.com.cn>
Reviewed-by: Yunkai Zhang <redacted>
Cc: Yang Yang <yang.yang29@zte.com.cn>
Cc: Liu Chun <redacted>
Cc: Xuexin Jiang <redacted>

I think it would be better to target net-next tree since it's not a
fix or something else important.

OK. I would target it for net-next.

quoted

quoted

---
 include/trace/events/icmp.h | 64 ++++++++++++++++++++++++++++++++++++=

+

quoted

quoted

quoted

 net/ipv4/icmp.c             |  4 +++
 2 files changed, 68 insertions(+)
 create mode 100644 include/trace/events/icmp.h

diff --git a/include/trace/events/icmp.h b/include/trace/events/icmp.h
new file mode 100644
index 000000000000..2098d4b1b12e
--- /dev/null
+++ b/include/trace/events/icmp.h

@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM icmp
+
+#if !defined(_TRACE_ICMP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ICMP_H
+
+#include <linux/icmp.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(icmp_send,
+
+               TP_PROTO(const struct sk_buff *skb, int type, int code=

),

quoted

quoted

quoted

+
+               TP_ARGS(skb, type, code),
+
+               TP_STRUCT__entry(
+                       __field(const void *, skbaddr)
+                       __field(int, type)
+                       __field(int, code)
+                       __array(__u8, saddr, 4)
+                       __array(__u8, daddr, 4)
+                       __field(__u16, sport)
+                       __field(__u16, dport)
+                       __field(unsigned short, ulen)
+               ),
+
+               TP_fast_assign(
+                       struct iphdr *iph =3D3D ip_hdr(skb);
+                       int proto_4 =3D3D iph->protocol;
+                       __be32 *p32;
+
+                       __entry->skbaddr =3D3D skb;
+                       __entry->type =3D3D type;
+                       __entry->code =3D3D code;
+
+                       if (proto_4 =3D3D=3D3D IPPROTO_UDP) {
+                               struct udphdr *uh =3D3D udp_hdr(skb);
+                               __entry->sport =3D3D ntohs(uh->source)=

;

quoted

quoted

quoted

+                               __entry->dport =3D3D ntohs(uh->dest);
+                               __entry->ulen =3D3D ntohs(uh->len);
+                       } else {
+                               __entry->sport =3D3D 0;
+                               __entry->dport =3D3D 0;
+                               __entry->ulen =3D3D 0;
+                       }

What about using the TP_STORE_ADDR_PORTS_SKB macro to record the sport
and dport like the patch[1] did through extending the use of header
for TCP and UDP?

I believe patch[1] is a good idea as it moves the TCP protocol parsing
previously done inside the TP_STORE_ADDR_PORTS_SKB macro to TP_fast_assig=

n,

quoted

and extracts the TP_STORE_ADDR_PORTS_SKB macro into a common file,
enabling support for both UDP and TCP protocol parsing simultaneously.

However, patch[1] only extracts the source and destination addresses of
the packet, but does not extract the source port and destination port,
which limits the significance of my submitted patch.

No, please take a look at TP_STORE_ADDR_PORTS_SKB() macro again. It
records 4-tuples of the flow.

Thanks,
Jason

Okay, after patch [1] is merged, we will propose an optimization patch based on it.

quoted

Perhaps the patch[1] could be referenced for integration after it is merg=

ed.

quoted

quoted

And, I wonder what the use of tracing ulen of that skb?

The tracking of ulen is primarily aimed at ensuring the legality of recei=

ved

quoted

UDP packets and providing developers with more detailed information
on exceptions. See net/ipv4/udp.c:2494-2501.

quoted

[1]: https://lore.kernel.org/all/1c7156a3f164eb33ef3a25b8432e359f0bb60a8=

e.1=3D

quoted

quoted

710866188.git.balazs.scheidler@axoflow.com/

Thanks,
Jason

quoted

+
+                       p32 =3D3D (__be32 *) __entry->saddr;
+                       *p32 =3D3D iph->saddr;
+
+                       p32 =3D3D (__be32 *) __entry->daddr;
+                       *p32 =3D3D iph->daddr;
+               ),
+
+               TP_printk("icmp_send: type=3D3D%d, code=3D3D%d. From %=

pI4:%u =3D

quoted

quoted

to %pI4:%u ulen=3D3D%d skbaddr=3D3D%p",

quoted

+                       __entry->type, __entry->code,
+                       __entry->saddr, __entry->sport, __entry->daddr=

,

quoted

quoted

quoted

+                       __entry->dport, __entry->ulen, __entry->skbadd=

r)

quoted

quoted

quoted

+);
+
+#endif /* _TRACE_ICMP_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
\ No newline at end of file

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index e63a3bf99617..21fb41257fe9 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c

@@ -92,6 +92,8 @@
 #include <net/inet_common.h>
 #include <net/ip_fib.h>
 #include <net/l3mdev.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/icmp.h>

 /*
  *     Build xmit assembly blocks

@@ -672,6 +674,8 @@ void __icmp_send(struct sk_buff *skb_in, int type,=

in=3D

quoted

quoted

t code, __be32 info,

quoted

                }
        }

+       trace_icmp_send(skb_in, type, code);
+
        /* Needed by both icmp_global_allow and icmp_xmit_lock */
        local_bh_disable();

--
2.44.0