Re: Re: Re: Subject: [PATCH net-next v4] net/ipv4: add tracepoint for icmp_send
>> >> >[...]
>> >> >> >I think my understanding based on what Eric depicted differs from =
>you:
>> >> >> >we're supposed to filter out those many invalid cases and only tra=
>ce
>> >> >> >the valid action of sending a icmp, so where to add a new tracepoi=
>nt
>> >> >> >is important instead of adding more checks in the tracepoint itsel=
>f.
>> >> >> >Please refer to what trace_tcp_retransmit_skb() does :)
>> >> >> >
>> >> >> >Thanks,
>> >> >> >Jason
>> >> >> Okay, thank you for your suggestion. In order to avoid filtering ou=
>t
>> >> >> those many invalid cases and only tracing the valid action of sendi=
>ng
>> >> >> a icmp, the next patch will add udd_fail_no_port trancepoint to the
>> >> >> include/trace/events/udp.h. This will solve the problem you mention=
>ed
>> >> >> very well. At this point, only UDP protocol exceptions will be trac=
>ked,
>> >> >> without the need to track them in icmp_send.
>> >> >
>> >> >I'm not against what you did (tracing all the icmp_send() for UDP) in
>> >> >your original patch. I was suggesting that you could put
>> >> >trace_icmp_send() in the right place, then you don't have to check th=
>e
>> >> >possible error condition (like if the skb->head is valid or not, ...)
>> >> >in your trace function.
>> >> >
>> >> >One example that can avoid various checks existing in the
>> >> >__icmp_send() function:
>> >> >diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> >> >index e63a3bf99617..2c9f7364de45 100644
>> >> >--- a/net/ipv4/icmp.c
>> >> >+++ b/net/ipv4/icmp.c
>> >> >@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type=
>,
>> >> >int code, __be32 info,
>> >> >if (!fl4.saddr)
>> >> >fl4.saddr =3D htonl(INADDR_DUMMY);
>> >> >
>> >> >+ trace_icmp_send(skb_in, type, code);
>> >> >icmp_push_reply(sk, _param, , , );
>> >> > ende
>> >> >ip_rt_put(rt);
>> >> >
>> >> >If we go here, it means we are ready to send the ICMP skb because
>> >> >we're done extracting the right information in the 'struct sk_buff
>> >> >skb_in'. Simpler and easier, right?
>> >> >
>> >> >Thanks,
>> >> >Jason
>> >>
>> >> I may not fully agree with this viewpoint. When trace_icmp_send is pla=
>ced
>> >> in this position, it cannot guarantee that all skbs in icmp are UDP pr=
>otocols
>> >> (UDP needs to be distinguished based on the proto_4!=3DIPPROTO_UDP con=
>dition),
>> >> nor can it guarantee the legitimacy of udphdr (*uh legitimacy check is=
> required).
>> >
>> >Of course, the UDP test statement is absolutely needed! Eric
>> >previously pointed this out in the V1 patch thread. I'm not referring
>> >to this one but like skb->head check something like this which exists
>> >in __icmp_send() function. You can see there are so many checks in it
>> >before sending.
>> >
>> >So only keeping the UDP check is enough, I think.
>>
>> The __icmp_send function only checks the IP header, but does not check
>> the UDP header, as shown in the following code snippet:
>>
>> if ((u8 *)iph < skb_in->head ||
>> (skb_network_header(skb_in) + sizeof(*iph)) >
>> skb_tail_pointer(skb_in))
>> goto out;
>>
>> There is no problem with the IP header check, which does not mean that
>> the UDP header is correct. Therefore, I believe that it is essential to
>> include a legitimacy judgment for the UDP header.
>>
>> Here is an explanation of this code:
>> Firstly, the UDP header (*uh) is extracted from the skb.
>> Then, if the current protocol of the skb is not UDP, or if the address of
>> uh is outside the range of the skb, the source port and destination port
>> will not be resolved, and 0 will be filled in directly.Otherwise,
>> the source port and destination port of the UDP header will be resolved.
>>
>> + struct udphdr *uh =3D udp_hdr(skb);
>> + if (proto_4 !=3D IPPROTO_UDP || (u8 *)uh < skb->head ||
>> + (u8 *)uh + sizeof(struct udphdr) > skb_tail_pointer(skb)) {
>
>>From the beginning, I always agree with the UDP check. I was saying if
>you can put the trace_icmp_send() just before icmp_push_reply()[1],
>you could avoid those kinds of checks.
>As I said in the previous email, "only keeping the UDP check is
>enough". So you are right.
>
>[1]
>diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>index e63a3bf99617..2c9f7364de45 100644
>--- a/net/ipv4/icmp.c
>+++ b/net/ipv4/icmp.c
>@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type,
>int code, __be32 info,
>if (!fl4.saddr)
>fl4.saddr =3D htonl(INADDR_DUMMY);
>
>+ trace_icmp_send(skb_in, type, code);
>icmp_push_reply(sk, _param, , , );
> ende:
>ip_rt_put(rt);
>
>If we're doing this, trace_icmp_send() can reflect the real action of
>sending an ICMP like trace_tcp_retransmit_skb(). Or else, the trace
>could print some messages but no real ICMP is sent (see those error
>checks). WDYT?
>
>Thanks,
>Jasoin
Yeah, placing trace_icmp_send() before icmp_push_reply() will ensure
that tracking starts when ICMP
Re: Re: Re: Subject: [PATCH net-next v4] net/ipv4: add tracepoint for icmp_send
On Thu, Apr 11, 2024 at 12:57 PM Peilin He wrote:
>
> >> >[...]
> >> >> >I think my understanding based on what Eric depicted differs from you:
> >> >> >we're supposed to filter out those many invalid cases and only trace
> >> >> >the valid action of sending a icmp, so where to add a new tracepoint
> >> >> >is important instead of adding more checks in the tracepoint itself.
> >> >> >Please refer to what trace_tcp_retransmit_skb() does :)
> >> >> >
> >> >> >Thanks,
> >> >> >Jason
> >> >> Okay, thank you for your suggestion. In order to avoid filtering out
> >> >> those many invalid cases and only tracing the valid action of sending
> >> >> a icmp, the next patch will add udd_fail_no_port trancepoint to the
> >> >> include/trace/events/udp.h. This will solve the problem you mentioned
> >> >> very well. At this point, only UDP protocol exceptions will be tracked,
> >> >> without the need to track them in icmp_send.
> >> >
> >> >I'm not against what you did (tracing all the icmp_send() for UDP) in
> >> >your original patch. I was suggesting that you could put
> >> >trace_icmp_send() in the right place, then you don't have to check the
> >> >possible error condition (like if the skb->head is valid or not, ...)
> >> >in your trace function.
> >> >
> >> >One example that can avoid various checks existing in the
> >> >__icmp_send() function:
> >> >diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> >> >index e63a3bf99617..2c9f7364de45 100644
> >> >--- a/net/ipv4/icmp.c
> >> >+++ b/net/ipv4/icmp.c
> >> >@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type,
> >> >int code, __be32 info,
> >> >if (!fl4.saddr)
> >> >fl4.saddr = htonl(INADDR_DUMMY);
> >> >
> >> >+ trace_icmp_send(skb_in, type, code);
> >> >icmp_push_reply(sk, _param, , , );
> >> > ende
> >> >ip_rt_put(rt);
> >> >
> >> >If we go here, it means we are ready to send the ICMP skb because
> >> >we're done extracting the right information in the 'struct sk_buff
> >> >skb_in'. Simpler and easier, right?
> >> >
> >> >Thanks,
> >> >Jason
> >>
> >> I may not fully agree with this viewpoint. When trace_icmp_send is placed
> >> in this position, it cannot guarantee that all skbs in icmp are UDP
> >> protocols
> >> (UDP needs to be distinguished based on the proto_4!=IPPROTO_UDP
> >> condition),
> >> nor can it guarantee the legitimacy of udphdr (*uh legitimacy check is
> >> required).
> >
> >Of course, the UDP test statement is absolutely needed! Eric
> >previously pointed this out in the V1 patch thread. I'm not referring
> >to this one but like skb->head check something like this which exists
> >in __icmp_send() function. You can see there are so many checks in it
> >before sending.
> >
> >So only keeping the UDP check is enough, I think.
>
> The __icmp_send function only checks the IP header, but does not check
> the UDP header, as shown in the following code snippet:
>
> if ((u8 *)iph < skb_in->head ||
> (skb_network_header(skb_in) + sizeof(*iph)) >
> skb_tail_pointer(skb_in))
> goto out;
>
> There is no problem with the IP header check, which does not mean that
> the UDP header is correct. Therefore, I believe that it is essential to
> include a legitimacy judgment for the UDP header.
>
> Here is an explanation of this code:
> Firstly, the UDP header (*uh) is extracted from the skb.
> Then, if the current protocol of the skb is not UDP, or if the address of
> uh is outside the range of the skb, the source port and destination port
> will not be resolved, and 0 will be filled in directly.Otherwise,
> the source port and destination port of the UDP header will be resolved.
>
> + struct udphdr *uh = udp_hdr(skb);
> + if (proto_4 != IPPROTO_UDP || (u8 *)uh < skb->head ||
> + (u8 *)uh + sizeof(struct udphdr) > skb_tail_pointer(skb)) {
>From the beginning, I always agree with the UDP check. I was saying if
you can put the trace_icmp_send() just before icmp_push_reply()[1],
you could avoid those kinds of checks.
As I said in the previous email, "only keeping the UDP check is
enough". So you are right.
[1]
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index e63a3bf99617..2c9f7364de45 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type,
int code, __be32 info,
if (!fl4.saddr)
fl4.saddr = htonl(INADDR_DUMMY);
+ trace_icmp_send(skb_in, type, code);
icmp_push_reply(sk, _param, , , );
ende:
ip_rt_put(rt);
If we're doing this, trace_icmp_send() can reflect the real action of
sending an ICMP like trace_tcp_retransmit_skb(). Or else, the trace
could print some messages but no real ICMP is sent (see those error
checks). WDYT?
Thanks,
Jason
>
> With best wishes
> Peilin He
>
> >Thanks,
> >Jason
> >
> >>
> >> With best wishes
> >> Peilin He
> >>
> >> >>
> >> >> >> 2.Target this patch for net-next.
> >> >> >>
> >> >> >> v2->v3:
> >>
Re: Re: Re: Subject: [PATCH net-next v4] net/ipv4: add tracepoint for icmp_send
>> >[...]
>> >> >I think my understanding based on what Eric depicted differs from you:
>> >> >we're supposed to filter out those many invalid cases and only trace
>> >> >the valid action of sending a icmp, so where to add a new tracepoint
>> >> >is important instead of adding more checks in the tracepoint itself.
>> >> >Please refer to what trace_tcp_retransmit_skb() does :)
>> >> >
>> >> >Thanks,
>> >> >Jason
>> >> Okay, thank you for your suggestion. In order to avoid filtering out
>> >> those many invalid cases and only tracing the valid action of sending
>> >> a icmp, the next patch will add udd_fail_no_port trancepoint to the
>> >> include/trace/events/udp.h. This will solve the problem you mentioned
>> >> very well. At this point, only UDP protocol exceptions will be tracked,
>> >> without the need to track them in icmp_send.
>> >
>> >I'm not against what you did (tracing all the icmp_send() for UDP) in
>> >your original patch. I was suggesting that you could put
>> >trace_icmp_send() in the right place, then you don't have to check the
>> >possible error condition (like if the skb->head is valid or not, ...)
>> >in your trace function.
>> >
>> >One example that can avoid various checks existing in the
>> >__icmp_send() function:
>> >diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> >index e63a3bf99617..2c9f7364de45 100644
>> >--- a/net/ipv4/icmp.c
>> >+++ b/net/ipv4/icmp.c
>> >@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type,
>> >int code, __be32 info,
>> >if (!fl4.saddr)
>> >fl4.saddr = htonl(INADDR_DUMMY);
>> >
>> >+ trace_icmp_send(skb_in, type, code);
>> >icmp_push_reply(sk, _param, , , );
>> > ende
>> >ip_rt_put(rt);
>> >
>> >If we go here, it means we are ready to send the ICMP skb because
>> >we're done extracting the right information in the 'struct sk_buff
>> >skb_in'. Simpler and easier, right?
>> >
>> >Thanks,
>> >Jason
>>
>> I may not fully agree with this viewpoint. When trace_icmp_send is placed
>> in this position, it cannot guarantee that all skbs in icmp are UDP protocols
>> (UDP needs to be distinguished based on the proto_4!=IPPROTO_UDP condition),
>> nor can it guarantee the legitimacy of udphdr (*uh legitimacy check is
>> required).
>
>Of course, the UDP test statement is absolutely needed! Eric
>previously pointed this out in the V1 patch thread. I'm not referring
>to this one but like skb->head check something like this which exists
>in __icmp_send() function. You can see there are so many checks in it
>before sending.
>
>So only keeping the UDP check is enough, I think.
The __icmp_send function only checks the IP header, but does not check
the UDP header, as shown in the following code snippet:
if ((u8 *)iph < skb_in->head ||
(skb_network_header(skb_in) + sizeof(*iph)) >
skb_tail_pointer(skb_in))
goto out;
There is no problem with the IP header check, which does not mean that
the UDP header is correct. Therefore, I believe that it is essential to
include a legitimacy judgment for the UDP header.
Here is an explanation of this code:
Firstly, the UDP header (*uh) is extracted from the skb.
Then, if the current protocol of the skb is not UDP, or if the address of
uh is outside the range of the skb, the source port and destination port
will not be resolved, and 0 will be filled in directly.Otherwise,
the source port and destination port of the UDP header will be resolved.
+ struct udphdr *uh = udp_hdr(skb);
+ if (proto_4 != IPPROTO_UDP || (u8 *)uh < skb->head ||
+ (u8 *)uh + sizeof(struct udphdr) > skb_tail_pointer(skb)) {
With best wishes
Peilin He
>Thanks,
>Jason
>
>>
>> With best wishes
>> Peilin He
>>
>> >>
>> >> >> 2.Target this patch for net-next.
>> >> >>
>> >> >> v2->v3:
>> >> >> Some fixes according to
>> >> >> https://lore.kernel.org/all/20240319102549.7f7f6...@gandalf.local.home/
>> >> >> 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=3DsZtRnKRu_tnUwqHuFQTJvJsv=
>> >> >-nz1x...@mail.gmail.com/
>> >> >> 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
>> >> >> Reviewed-by: xu xin
>> >> >> Reviewed-by: Yunkai Zhang
>> >> >> Cc: Yang Yang
>> >> >> Cc: Liu Chun
>> >> >> Cc: Xuexin Jiang
>> >> >> ---
>> >> >> include/trace/events/icmp.h | 65 +
>> >> >> net/ipv4/icmp.c | 4 +++
>> >> >> 2 files changed, 69 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 ..7d5190f48a28
>> >>
Re: Re: Re: Subject: [PATCH net-next v4] net/ipv4: add tracepoint for icmp_send
On Thu, Apr 11, 2024 at 10:34 AM Peilin He wrote: > > >[...] > >> >I think my understanding based on what Eric depicted differs from you: > >> >we're supposed to filter out those many invalid cases and only trace > >> >the valid action of sending a icmp, so where to add a new tracepoint > >> >is important instead of adding more checks in the tracepoint itself. > >> >Please refer to what trace_tcp_retransmit_skb() does :) > >> > > >> >Thanks, > >> >Jason > >> Okay, thank you for your suggestion. In order to avoid filtering out > >> those many invalid cases and only tracing the valid action of sending > >> a icmp, the next patch will add udd_fail_no_port trancepoint to the > >> include/trace/events/udp.h. This will solve the problem you mentioned > >> very well. At this point, only UDP protocol exceptions will be tracked, > >> without the need to track them in icmp_send. > > > >I'm not against what you did (tracing all the icmp_send() for UDP) in > >your original patch. I was suggesting that you could put > >trace_icmp_send() in the right place, then you don't have to check the > >possible error condition (like if the skb->head is valid or not, ...) > >in your trace function. > > > >One example that can avoid various checks existing in the > >__icmp_send() function: > >diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c > >index e63a3bf99617..2c9f7364de45 100644 > >--- a/net/ipv4/icmp.c > >+++ b/net/ipv4/icmp.c > >@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type, > >int code, __be32 info, > >if (!fl4.saddr) > >fl4.saddr = htonl(INADDR_DUMMY); > > > >+ trace_icmp_send(skb_in, type, code); > >icmp_push_reply(sk, _param, , , ); > > ende > >ip_rt_put(rt); > > > >If we go here, it means we are ready to send the ICMP skb because > >we're done extracting the right information in the 'struct sk_buff > >skb_in'. Simpler and easier, right? > > > >Thanks, > >Jason > > I may not fully agree with this viewpoint. When trace_icmp_send is placed > in this position, it cannot guarantee that all skbs in icmp are UDP protocols > (UDP needs to be distinguished based on the proto_4!=IPPROTO_UDP condition), > nor can it guarantee the legitimacy of udphdr (*uh legitimacy check is > required). Of course, the UDP test statement is absolutely needed! Eric previously pointed this out in the V1 patch thread. I'm not referring to this one but like skb->head check something like this which exists in __icmp_send() function. You can see there are so many checks in it before sending. So only keeping the UDP check is enough, I think. Thanks, Jason > > With best wishes > Peilin He > > >> > >> >> 2.Target this patch for net-next. > >> >> > >> >> v2->v3: > >> >> Some fixes according to > >> >> https://lore.kernel.org/all/20240319102549.7f7f6...@gandalf.local.home/ > >> >> 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=3DsZtRnKRu_tnUwqHuFQTJvJsv= > >> >-nz1x...@mail.gmail.com/ > >> >> 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 > >> >> Reviewed-by: xu xin > >> >> Reviewed-by: Yunkai Zhang > >> >> Cc: Yang Yang > >> >> Cc: Liu Chun > >> >> Cc: Xuexin Jiang > >> >> --- > >> >> include/trace/events/icmp.h | 65 + > >> >> net/ipv4/icmp.c | 4 +++ > >> >> 2 files changed, 69 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 ..7d5190f48a28 > >> >> --- /dev/null > >> >> +++ b/include/trace/events/icmp.h > >> >> @@ -0,0 +1,65 @@ > >> >> +/* 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 > >> >> +#include > >> >> + > >> >> +TRACE_EVENT(icmp_send, > >> >> + > >> >> + TP_PROTO(const struct sk_buff *skb, int type, int code), > >> >> + > >> >> + 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) > >> >> + ), > >> >> + > >> >> +
Re: Re: Re: Subject: [PATCH net-next v4] net/ipv4: add tracepoint for icmp_send
>[...]
>> >I think my understanding based on what Eric depicted differs from you:
>> >we're supposed to filter out those many invalid cases and only trace
>> >the valid action of sending a icmp, so where to add a new tracepoint
>> >is important instead of adding more checks in the tracepoint itself.
>> >Please refer to what trace_tcp_retransmit_skb() does :)
>> >
>> >Thanks,
>> >Jason
>> Okay, thank you for your suggestion. In order to avoid filtering out
>> those many invalid cases and only tracing the valid action of sending
>> a icmp, the next patch will add udd_fail_no_port trancepoint to the
>> include/trace/events/udp.h. This will solve the problem you mentioned
>> very well. At this point, only UDP protocol exceptions will be tracked,
>> without the need to track them in icmp_send.
>
>I'm not against what you did (tracing all the icmp_send() for UDP) in
>your original patch. I was suggesting that you could put
>trace_icmp_send() in the right place, then you don't have to check the
>possible error condition (like if the skb->head is valid or not, ...)
>in your trace function.
>
>One example that can avoid various checks existing in the
>__icmp_send() function:
>diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>index e63a3bf99617..2c9f7364de45 100644
>--- a/net/ipv4/icmp.c
>+++ b/net/ipv4/icmp.c
>@@ -767,6 +767,7 @@ void __icmp_send(struct sk_buff *skb_in, int type,
>int code, __be32 info,
>if (!fl4.saddr)
>fl4.saddr = htonl(INADDR_DUMMY);
>
>+ trace_icmp_send(skb_in, type, code);
>icmp_push_reply(sk, _param, , , );
> ende:
>ip_rt_put(rt);
>
>If we go here, it means we are ready to send the ICMP skb because
>we're done extracting the right information in the 'struct sk_buff
>skb_in'. Simpler and easier, right?
>
>Thanks,
>Jason
I may not fully agree with this viewpoint. When trace_icmp_send is placed
in this position, it cannot guarantee that all skbs in icmp are UDP protocols
(UDP needs to be distinguished based on the proto_4!=IPPROTO_UDP condition),
nor can it guarantee the legitimacy of udphdr (*uh legitimacy check is
required).
With best wishes
Peilin He
>>
>> >> 2.Target this patch for net-next.
>> >>
>> >> v2->v3:
>> >> Some fixes according to
>> >> https://lore.kernel.org/all/20240319102549.7f7f6...@gandalf.local.home/
>> >> 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=3DsZtRnKRu_tnUwqHuFQTJvJsv=
>> >-nz1x...@mail.gmail.com/
>> >> 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
>> >> Reviewed-by: xu xin
>> >> Reviewed-by: Yunkai Zhang
>> >> Cc: Yang Yang
>> >> Cc: Liu Chun
>> >> Cc: Xuexin Jiang
>> >> ---
>> >> include/trace/events/icmp.h | 65 +
>> >> net/ipv4/icmp.c | 4 +++
>> >> 2 files changed, 69 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 ..7d5190f48a28
>> >> --- /dev/null
>> >> +++ b/include/trace/events/icmp.h
>> >> @@ -0,0 +1,65 @@
>> >> +/* 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
>> >> +#include
>> >> +
>> >> +TRACE_EVENT(icmp_send,
>> >> +
>> >> + TP_PROTO(const struct sk_buff *skb, int type, int code),
>> >> +
>> >> + 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 =3D ip_hdr(skb);
>> >> + int proto_4 =3D iph->protocol;
>> >> + __be32 *p32;
>> >> +
>> >> + __entry->skbaddr =3D skb;
>> >> + __entry->type =3D type;
>> >> + __entry->code =3D code;
>> >> +
>> >> + struct udphdr *uh =3D udp_hdr(skb);
>> >> + if (proto_4 !=3D IPPROTO_UDP || (u8 *)uh < skb->h=
>> >ead ||
>> >> + (u8 *)uh + sizeof(struct udphdr) > skb_ta=
>> >il_pointer(skb)) {
>> >> +
