[lttng-dev] Fwd: [PATCH lttng-modules] Add UDP and ICMP packet header information to the tracepoint:

[lttng-dev] Fwd: [PATCH lttng-modules] Add UDP and ICMP packet header information to the tracepoint:

[Florian Walbroel]

Hi,

I sent in this patch last month and did not hear about any updates since 
then.

There were a few formatting issues that have been addressed. Is there 
anything more that should be done?

Best regards,
Florian Walbroel

-------- Forwarded Message --------
Subject: 	[PATCH lttng-modules] Add UDP and ICMP packet header 
information to the tracepoint:
Date: 	Wed, 13 Nov 2019 17:38:14 +0100
From: 	Florian Walbroel <walbroel@silexica.com>
To: 	lttng-dev at lists.lttng.org



* UDP transport header
* ICMP transport header

(Correct indentation for switch/case)

Signed-off-by: Florian Walbroel <walbroel at silexica.com>
---
instrumentation/events/lttng-module/net.h | 166 ++++++++++++++++++++--
1 file changed, 153 insertions(+), 13 deletions(-)

diff --git a/instrumentation/events/lttng-module/net.h 
b/instrumentation/events/lttng-module/net.h
index bfa14fc..ad1892a 100644
--- a/instrumentation/events/lttng-module/net.h
+++ b/instrumentation/events/lttng-module/net.h
@@ -11,6 +11,8 @@
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/tcp.h>
+#include <linux/udp.h>
+#include <linux/icmp.h>
#include <linux/version.h>
#include <lttng-endian.h>
#include <net/sock.h>
@@ -85,6 +87,53 @@ static struct lttng_event_field tcpfields[] = {
},
};
+static struct lttng_event_field udpfields[] = {
+ [0] = {
+ .name = "source_port",
+ .type = __type_integer(uint16_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+ [1] = {
+ .name = "dest_port",
+ .type = __type_integer(uint16_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+ [2] = {
+ .name = "len",
+ .type = __type_integer(uint16_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+ [3] = {
+ .name = "check",
+ .type = __type_integer(uint16_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+};
+
+static struct lttng_event_field icmpfields[] = {
+ [0] = {
+ .name = "type",
+ .type = __type_integer(uint8_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+ [1] = {
+ .name = "code",
+ .type = __type_integer(uint8_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+ [2] = {
+ .name = "checksum",
+ .type = __type_integer(uint16_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+ [3] = {
+ .name = "gateway",
+ .type = __type_integer(uint32_t, 0, 0, 0,
+ __BIG_ENDIAN, 10, none),
+ },
+};
+
+
static struct lttng_event_field transport_fields[] = {
[0] = {
.name = "unknown",
@@ -102,13 +151,57 @@ static struct lttng_event_field transport_fields[] = {
.u._struct.fields = tcpfields,
},
},
+ [2] = {
+ .name = "udp",
+ .type = {
+ .atype = atype_struct,
+ .u._struct.nr_fields = ARRAY_SIZE(udpfields),
+ .u._struct.fields = udpfields,
+ },
+ },
+ [3] = {
+ .name = "icmp",
+ .type = {
+ .atype = atype_struct,
+ .u._struct.nr_fields = ARRAY_SIZE(icmpfields),
+ .u._struct.fields = icmpfields,
+ },
+ },
};
enum transport_header_types {
TH_NONE = 0,
TH_TCP = 1,
+ TH_UDP = 2,
+ TH_ICMP = 3,
};
+static inline enum transport_header_types 
__get_transport_header_type_ip(struct sk_buff *skb)
+{
+ switch(ip_hdr(skb)->protocol) {
+ case IPPROTO_TCP:
+ return TH_TCP;
+ case IPPROTO_UDP:
+ return TH_UDP;
+ case IPPROTO_ICMP:
+ return TH_ICMP;
+ }
+ return TH_NONE;
+}
+
+static inline enum transport_header_types 
__get_transport_header_type_ipv6(struct sk_buff *skb)
+{
+ switch(ipv6_hdr(skb)->nexthdr) {
+ case IPPROTO_TCP:
+ return TH_TCP;
+ case IPPROTO_UDP:
+ return TH_UDP;
+ case IPPROTO_ICMP:
+ return TH_ICMP;
+ }
+ return TH_NONE;
+}
+
static inline enum transport_header_types 
__get_transport_header_type(struct sk_buff *skb)
{
if (__has_network_hdr(skb)) {
@@ -123,13 +216,13 @@ static inline enum transport_header_types 
__get_transport_header_type(struct sk_
* header's data. This method works both for
* sent and received packets.
*/
- if ((skb->protocol == htons(ETH_P_IP) &&
- ip_hdr(skb)->protocol == IPPROTO_TCP) ||
- (skb->protocol == htons(ETH_P_IPV6) &&
- ipv6_hdr(skb)->nexthdr == IPPROTO_TCP))
- return TH_TCP;
+ if (skb->protocol == htons(ETH_P_IP)) {
+ return __get_transport_header_type_ip(skb);
+ } else if(skb->protocol == htons(ETH_P_IPV6)) {
+ return __get_transport_header_type_ipv6(skb);
+ }
}
- /* Fallthrough for other cases where header is not TCP. */
+ /* Fallthrough for other cases where header is not recognized. */
}
return TH_NONE;
}
@@ -137,16 +230,36 @@ static inline enum transport_header_types 
__get_transport_header_type(struct sk_
static struct lttng_enum_entry proto_transport_enum_entries[] = {
[0] = {
.start = { .value = 0, .signedness = 0, },
- .end = { .value = IPPROTO_TCP - 1, .signedness = 0, },
+ .end = { .value = IPPROTO_ICMP - 1, .signedness = 0, },
.string = "_unknown",
},
[1] = {
+ .start = { .value = IPPROTO_ICMP, .signedness = 0, },
+ .end = { .value = IPPROTO_ICMP, .signedness = 0, },
+ .string = "_icmp",
+ },
+ [2] = {
+ .start = { .value = IPPROTO_ICMP + 1, .signedness = 0, },
+ .end = { .value = IPPROTO_TCP - 1, .signedness = 0, },
+ .string = "_unknown",
+ },
+ [3] = {
.start = { .value = IPPROTO_TCP, .signedness = 0, },
.end = { .value = IPPROTO_TCP, .signedness = 0, },
.string = "_tcp",
},
- [2] = {
+ [4] = {
.start = { .value = IPPROTO_TCP + 1, .signedness = 0, },
+ .end = { .value = IPPROTO_UDP - 1, .signedness = 0, },
+ .string = "_unknown",
+ },
+ [5] = {
+ .start = { .value = IPPROTO_UDP, .signedness = 0, },
+ .end = { .value = IPPROTO_UDP, .signedness = 0, },
+ .string = "_udp",
+ },
+ [6] = {
+ .start = { .value = IPPROTO_UDP + 1, .signedness = 0, },
.end = { .value = 255, .signedness = 0, },
.string = "_unknown",
},
@@ -169,6 +282,16 @@ static struct lttng_enum_entry 
transport_enum_entries[] = {
.end = { .value = TH_TCP, .signedness = 0, },
.string = "_tcp",
},
+ [2] = {
+ .start = { .value = TH_UDP, .signedness = 0, },
+ .end = { .value = TH_UDP, .signedness = 0, },
+ .string = "_udp",
+ },
+ [3] = {
+ .start = { .value = TH_ICMP, .signedness = 0, },
+ .end = { .value = TH_ICMP, .signedness = 0, },
+ .string = "_icmp",
+ },
};
static const struct lttng_enum_desc transport_header_type = {
@@ -510,15 +633,32 @@ LTTNG_TRACEPOINT_EVENT_CLASS(net_dev_template,
ctf_integer_type(unsigned char, th_type)
/* Copy the transport header. */
- if (th_type == TH_TCP) {
+ switch (th_type) {
+ case TH_TCP: {
ctf_align(uint32_t)
ctf_array_type(uint8_t, tcp_hdr(skb),
sizeof(struct tcphdr))
+ break;
+ }
+ case TH_UDP: {
+ ctf_align(uint32_t)
+ ctf_array_type(uint8_t, udp_hdr(skb),
+ sizeof(struct udphdr))
+ break;
+ }
+ case TH_ICMP: {
+ ctf_align(uint32_t)
+ ctf_array_type(uint8_t, icmp_hdr(skb),
+ sizeof(struct udphdr))
+ break;
+ }
+ default:
+ /*
+ * For any other transport header type,
+ * there is nothing to do.
+ */
+ break;
}
- /*
- * For any other transport header type,
- * there is nothing to do.
- */
}
)
)

-- 
2.17.1

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.lttng.org/pipermail/lttng-dev/attachments/20191217/48b23948/attachment.html>

[lttng-dev] Fwd: [PATCH lttng-modules] Add UDP and ICMP packet header information to the tracepoint:

[Mathieu Desnoyers]

----- On Dec 17, 2019, at 3:56 AM, Florian Walbroel <walbroel at silexica.com> wrote: 

> Hi,

> I sent in this patch last month and did not hear about any updates since then.

> There were a few formatting issues that have been addressed. Is there anything
> more that should be done?
Hi, 

I was expecting review from Genevi?ve and Julien on this patch, but never heard back from 
them. 

Genevi?ve, Julien, Michael, can you review this patch please ? 

Thanks, 

Mathieu 

> Best regards,
> Florian Walbroel

> -------- Forwarded Message -------- Subject: 	[PATCH lttng-modules] Add UDP and
> ICMP packet header information to the tracepoint:
> Date: 	Wed, 13 Nov 2019 17:38:14 +0100
> From: 	Florian Walbroel [ mailto:walbroel@silexica.com | <walbroel@silexica.com>
> ]

> To: 	[ mailto:lttng-dev at lists.lttng.org | lttng-dev at lists.lttng.org ]

> * UDP transport header
> * ICMP transport header

> (Correct indentation for switch/case)

> Signed-off-by: Florian Walbroel [ mailto:walbroel at silexica.com |
> <walbroel at silexica.com> ]
> ---
> instrumentation/events/lttng-module/net.h | 166 ++++++++++++++++++++--
> 1 file changed, 153 insertions(+), 13 deletions(-)

> diff --git a/instrumentation/events/lttng-module/net.h
> b/instrumentation/events/lttng-module/net.h
> index bfa14fc..ad1892a 100644
> --- a/instrumentation/events/lttng-module/net.h
> +++ b/instrumentation/events/lttng-module/net.h
> @@ -11,6 +11,8 @@
> #include <linux/ip.h>
> #include <linux/ipv6.h>
> #include <linux/tcp.h>
> +#include <linux/udp.h>
> +#include <linux/icmp.h>
> #include <linux/version.h>
> #include <lttng-endian.h>
> #include <net/sock.h>
> @@ -85,6 +87,53 @@ static struct lttng_event_field tcpfields[] = {
> },
> };
> +static struct lttng_event_field udpfields[] = {
> + [0] = {
> + .name = "source_port",
> + .type = __type_integer(uint16_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> + [1] = {
> + .name = "dest_port",
> + .type = __type_integer(uint16_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> + [2] = {
> + .name = "len",
> + .type = __type_integer(uint16_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> + [3] = {
> + .name = "check",
> + .type = __type_integer(uint16_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> +};
> +
> +static struct lttng_event_field icmpfields[] = {
> + [0] = {
> + .name = "type",
> + .type = __type_integer(uint8_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> + [1] = {
> + .name = "code",
> + .type = __type_integer(uint8_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> + [2] = {
> + .name = "checksum",
> + .type = __type_integer(uint16_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> + [3] = {
> + .name = "gateway",
> + .type = __type_integer(uint32_t, 0, 0, 0,
> + __BIG_ENDIAN, 10, none),
> + },
> +};
> +
> +
> static struct lttng_event_field transport_fields[] = {
> [0] = {
> .name = "unknown",
> @@ -102,13 +151,57 @@ static struct lttng_event_field transport_fields[] = {
> .u._struct.fields = tcpfields,
> },
> },
> + [2] = {
> + .name = "udp",
> + .type = {
> + .atype = atype_struct,
> + .u._struct.nr_fields = ARRAY_SIZE(udpfields),
> + .u._struct.fields = udpfields,
> + },
> + },
> + [3] = {
> + .name = "icmp",
> + .type = {
> + .atype = atype_struct,
> + .u._struct.nr_fields = ARRAY_SIZE(icmpfields),
> + .u._struct.fields = icmpfields,
> + },
> + },
> };
> enum transport_header_types {
> TH_NONE = 0,
> TH_TCP = 1,
> + TH_UDP = 2,
> + TH_ICMP = 3,
> };
> +static inline enum transport_header_types __get_transport_header_type_ip(struct
> sk_buff *skb)
> +{
> + switch(ip_hdr(skb)->protocol) {
> + case IPPROTO_TCP:
> + return TH_TCP;
> + case IPPROTO_UDP:
> + return TH_UDP;
> + case IPPROTO_ICMP:
> + return TH_ICMP;
> + }
> + return TH_NONE;
> +}
> +
> +static inline enum transport_header_types
> __get_transport_header_type_ipv6(struct sk_buff *skb)
> +{
> + switch(ipv6_hdr(skb)->nexthdr) {
> + case IPPROTO_TCP:
> + return TH_TCP;
> + case IPPROTO_UDP:
> + return TH_UDP;
> + case IPPROTO_ICMP:
> + return TH_ICMP;
> + }
> + return TH_NONE;
> +}
> +
> static inline enum transport_header_types __get_transport_header_type(struct
> sk_buff *skb)
> {
> if (__has_network_hdr(skb)) {
> @@ -123,13 +216,13 @@ static inline enum transport_header_types
> __get_transport_header_type(struct sk_
> * header's data. This method works both for
> * sent and received packets.
> */
> - if ((skb->protocol == htons(ETH_P_IP) &&
> - ip_hdr(skb)->protocol == IPPROTO_TCP) ||
> - (skb->protocol == htons(ETH_P_IPV6) &&
> - ipv6_hdr(skb)->nexthdr == IPPROTO_TCP))
> - return TH_TCP;
> + if (skb->protocol == htons(ETH_P_IP)) {
> + return __get_transport_header_type_ip(skb);
> + } else if(skb->protocol == htons(ETH_P_IPV6)) {
> + return __get_transport_header_type_ipv6(skb);
> + }
> }
> - /* Fallthrough for other cases where header is not TCP. */
> + /* Fallthrough for other cases where header is not recognized. */
> }
> return TH_NONE;
> }
> @@ -137,16 +230,36 @@ static inline enum transport_header_types
> __get_transport_header_type(struct sk_
> static struct lttng_enum_entry proto_transport_enum_entries[] = {
> [0] = {
> .start = { .value = 0, .signedness = 0, },
> - .end = { .value = IPPROTO_TCP - 1, .signedness = 0, },
> + .end = { .value = IPPROTO_ICMP - 1, .signedness = 0, },
> .string = "_unknown",
> },
> [1] = {
> + .start = { .value = IPPROTO_ICMP, .signedness = 0, },
> + .end = { .value = IPPROTO_ICMP, .signedness = 0, },
> + .string = "_icmp",
> + },
> + [2] = {
> + .start = { .value = IPPROTO_ICMP + 1, .signedness = 0, },
> + .end = { .value = IPPROTO_TCP - 1, .signedness = 0, },
> + .string = "_unknown",
> + },
> + [3] = {
> .start = { .value = IPPROTO_TCP, .signedness = 0, },
> .end = { .value = IPPROTO_TCP, .signedness = 0, },
> .string = "_tcp",
> },
> - [2] = {
> + [4] = {
> .start = { .value = IPPROTO_TCP + 1, .signedness = 0, },
> + .end = { .value = IPPROTO_UDP - 1, .signedness = 0, },
> + .string = "_unknown",
> + },
> + [5] = {
> + .start = { .value = IPPROTO_UDP, .signedness = 0, },
> + .end = { .value = IPPROTO_UDP, .signedness = 0, },
> + .string = "_udp",
> + },
> + [6] = {
> + .start = { .value = IPPROTO_UDP + 1, .signedness = 0, },
> .end = { .value = 255, .signedness = 0, },
> .string = "_unknown",
> },
> @@ -169,6 +282,16 @@ static struct lttng_enum_entry transport_enum_entries[] = {
> .end = { .value = TH_TCP, .signedness = 0, },
> .string = "_tcp",
> },
> + [2] = {
> + .start = { .value = TH_UDP, .signedness = 0, },
> + .end = { .value = TH_UDP, .signedness = 0, },
> + .string = "_udp",
> + },
> + [3] = {
> + .start = { .value = TH_ICMP, .signedness = 0, },
> + .end = { .value = TH_ICMP, .signedness = 0, },
> + .string = "_icmp",
> + },
> };
> static const struct lttng_enum_desc transport_header_type = {
> @@ -510,15 +633,32 @@ LTTNG_TRACEPOINT_EVENT_CLASS(net_dev_template,
> ctf_integer_type(unsigned char, th_type)
> /* Copy the transport header. */
> - if (th_type == TH_TCP) {
> + switch (th_type) {
> + case TH_TCP: {
> ctf_align(uint32_t)
> ctf_array_type(uint8_t, tcp_hdr(skb),
> sizeof(struct tcphdr))
> + break;
> + }
> + case TH_UDP: {
> + ctf_align(uint32_t)
> + ctf_array_type(uint8_t, udp_hdr(skb),
> + sizeof(struct udphdr))
> + break;
> + }
> + case TH_ICMP: {
> + ctf_align(uint32_t)
> + ctf_array_type(uint8_t, icmp_hdr(skb),
> + sizeof(struct udphdr))
> + break;
> + }
> + default:
> + /*
> + * For any other transport header type,
> + * there is nothing to do.
> + */
> + break;
> }
> - /*
> - * For any other transport header type,
> - * there is nothing to do.
> - */
> }
> )
> )
> --
> 2.17.1

> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers 
EfficiOS Inc. 
http://www.efficios.com 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.lttng.org/pipermail/lttng-dev/attachments/20191217/cf30fdf4/attachment-0001.html>

[lttng-dev] Fwd: [PATCH lttng-modules] Add UDP and ICMP packet header information to the tracepoint:

[Michael Jeanson]

On 2019-12-17 9:39 a.m., Mathieu Desnoyers wrote:
> I was expecting review from Genevi?ve and Julien on this patch, but
> never heard back from
> them.
> 
> Genevi?ve, Julien, Michael, can you review this patch please ?
> 
> Thanks,
> 
> Mathieu

It builds on all the latest tags of the stable kernel branches we
support and the patch itself looks good to me but Genevi?ve's or
Julien's feedback would be appreciated.

Michael