2018-01-05 06:43:01

by Yafang Shao

[permalink] [raw]
Subject: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state

sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments.
Then we can conveniently use these two arguments to do the filter.

Suggested-by: Brendan Gregg <[email protected]>
Signed-off-by: Yafang Shao <[email protected]>
---
include/trace/events/sock.h | 24 ++++++++++++++++++------
net/ipv4/af_inet.c | 6 ++++--
2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 3537c5f..c7df70f 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -11,7 +11,11 @@
#include <linux/ipv6.h>
#include <linux/tcp.h>

-/* The protocol traced by sock_set_state */
+#define family_names \
+ EM(AF_INET) \
+ EMe(AF_INET6)
+
+/* The protocol traced by inet_sock_set_state */
#define inet_protocol_names \
EM(IPPROTO_TCP) \
EM(IPPROTO_DCCP) \
@@ -37,6 +41,7 @@
#define EM(a) TRACE_DEFINE_ENUM(a);
#define EMe(a) TRACE_DEFINE_ENUM(a);

+family_names
inet_protocol_names
tcp_state_names

@@ -45,6 +50,9 @@
#define EM(a) { a, #a },
#define EMe(a) { a, #a }

+#define show_family_name(val) \
+ __print_symbolic(val, family_names)
+
#define show_inet_protocol_name(val) \
__print_symbolic(val, inet_protocol_names)

@@ -108,9 +116,10 @@

TRACE_EVENT(inet_sock_set_state,

- TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
+ TP_PROTO(const struct sock *sk, const int family, const int protocol,
+ const int oldstate, const int newstate),

- TP_ARGS(sk, oldstate, newstate),
+ TP_ARGS(sk, family, protocol, oldstate, newstate),

TP_STRUCT__entry(
__field(const void *, skaddr)
@@ -118,6 +127,7 @@
__field(int, newstate)
__field(__u16, sport)
__field(__u16, dport)
+ __field(__u16, family)
__field(__u8, protocol)
__array(__u8, saddr, 4)
__array(__u8, daddr, 4)
@@ -133,8 +143,9 @@
__entry->skaddr = sk;
__entry->oldstate = oldstate;
__entry->newstate = newstate;
+ __entry->family = family;
+ __entry->protocol = protocol;

- __entry->protocol = sk->sk_protocol;
__entry->sport = ntohs(inet->inet_sport);
__entry->dport = ntohs(inet->inet_dport);

@@ -145,7 +156,7 @@
*p32 = inet->inet_daddr;

#if IS_ENABLED(CONFIG_IPV6)
- if (sk->sk_family == AF_INET6) {
+ if (family == AF_INET6) {
pin6 = (struct in6_addr *)__entry->saddr_v6;
*pin6 = sk->sk_v6_rcv_saddr;
pin6 = (struct in6_addr *)__entry->daddr_v6;
@@ -160,7 +171,8 @@
}
),

- TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+ TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+ show_family_name(__entry->family),
show_inet_protocol_name(__entry->protocol),
__entry->sport, __entry->dport,
__entry->saddr, __entry->daddr,
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index bab98a4..1d52796 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk)

void inet_sk_set_state(struct sock *sk, int state)
{
- trace_inet_sock_set_state(sk, sk->sk_state, state);
+ trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
+ sk->sk_state, state);
sk->sk_state = state;
}
EXPORT_SYMBOL(inet_sk_set_state);

void inet_sk_state_store(struct sock *sk, int newstate)
{
- trace_inet_sock_set_state(sk, sk->sk_state, newstate);
+ trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
+ sk->sk_state, newstate);
smp_store_release(&sk->sk_state, newstate);
}

--
1.8.3.1


2018-01-05 07:21:53

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state


> On Jan 4, 2018, at 10:42 PM, Yafang Shao <[email protected]> wrote:
>
> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments.
> Then we can conveniently use these two arguments to do the filter.
>
> Suggested-by: Brendan Gregg <[email protected]>
> Signed-off-by: Yafang Shao <[email protected]>
> ---
> include/trace/events/sock.h | 24 ++++++++++++++++++------
> net/ipv4/af_inet.c | 6 ++++--
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
> index 3537c5f..c7df70f 100644
> --- a/include/trace/events/sock.h
> +++ b/include/trace/events/sock.h
> @@ -11,7 +11,11 @@
> #include <linux/ipv6.h>
> #include <linux/tcp.h>
>
> -/* The protocol traced by sock_set_state */
> +#define family_names \
> + EM(AF_INET) \
> + EMe(AF_INET6)
> +
> +/* The protocol traced by inet_sock_set_state */
> #define inet_protocol_names \
> EM(IPPROTO_TCP) \
> EM(IPPROTO_DCCP) \
> @@ -37,6 +41,7 @@
> #define EM(a) TRACE_DEFINE_ENUM(a);
> #define EMe(a) TRACE_DEFINE_ENUM(a);
>
> +family_names
> inet_protocol_names
> tcp_state_names
>
> @@ -45,6 +50,9 @@
> #define EM(a) { a, #a },
> #define EMe(a) { a, #a }
>
> +#define show_family_name(val) \
> + __print_symbolic(val, family_names)
> +
> #define show_inet_protocol_name(val) \
> __print_symbolic(val, inet_protocol_names)
>
> @@ -108,9 +116,10 @@
>
> TRACE_EVENT(inet_sock_set_state,
>
> - TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
> + TP_PROTO(const struct sock *sk, const int family, const int protocol,
> + const int oldstate, const int newstate),

Are there cases we need protocol and/or family that is different to
sk->sk_protocol/sk_family? If not, I think we don't need to change the
TP_PROTO.

Thanks,
Song

>
> - TP_ARGS(sk, oldstate, newstate),
> + TP_ARGS(sk, family, protocol, oldstate, newstate),
>
> TP_STRUCT__entry(
> __field(const void *, skaddr)
> @@ -118,6 +127,7 @@
> __field(int, newstate)
> __field(__u16, sport)
> __field(__u16, dport)
> + __field(__u16, family)
> __field(__u8, protocol)
> __array(__u8, saddr, 4)
> __array(__u8, daddr, 4)
> @@ -133,8 +143,9 @@
> __entry->skaddr = sk;
> __entry->oldstate = oldstate;
> __entry->newstate = newstate;
> + __entry->family = family;
> + __entry->protocol = protocol;
>
> - __entry->protocol = sk->sk_protocol;
> __entry->sport = ntohs(inet->inet_sport);
> __entry->dport = ntohs(inet->inet_dport);
>
> @@ -145,7 +156,7 @@
> *p32 = inet->inet_daddr;
>
> #if IS_ENABLED(CONFIG_IPV6)
> - if (sk->sk_family == AF_INET6) {
> + if (family == AF_INET6) {
> pin6 = (struct in6_addr *)__entry->saddr_v6;
> *pin6 = sk->sk_v6_rcv_saddr;
> pin6 = (struct in6_addr *)__entry->daddr_v6;
> @@ -160,7 +171,8 @@
> }
> ),
>
> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> + show_family_name(__entry->family),
> show_inet_protocol_name(__entry->protocol),
> __entry->sport, __entry->dport,
> __entry->saddr, __entry->daddr,
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index bab98a4..1d52796 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk)
>
> void inet_sk_set_state(struct sock *sk, int state)
> {
> - trace_inet_sock_set_state(sk, sk->sk_state, state);
> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
> + sk->sk_state, state);
> sk->sk_state = state;
> }
> EXPORT_SYMBOL(inet_sk_set_state);
>
> void inet_sk_state_store(struct sock *sk, int newstate)
> {
> - trace_inet_sock_set_state(sk, sk->sk_state, newstate);
> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
> + sk->sk_state, newstate);
> smp_store_release(&sk->sk_state, newstate);
> }
>
> --
> 1.8.3.1
>

2018-01-05 08:10:31

by Yafang Shao

[permalink] [raw]
Subject: Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state

On Fri, Jan 5, 2018 at 3:21 PM, Song Liu <[email protected]> wrote:
>
>> On Jan 4, 2018, at 10:42 PM, Yafang Shao <[email protected]> wrote:
>>
>> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments.
>> Then we can conveniently use these two arguments to do the filter.
>>
>> Suggested-by: Brendan Gregg <[email protected]>
>> Signed-off-by: Yafang Shao <[email protected]>
>> ---
>> include/trace/events/sock.h | 24 ++++++++++++++++++------
>> net/ipv4/af_inet.c | 6 ++++--
>> 2 files changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
>> index 3537c5f..c7df70f 100644
>> --- a/include/trace/events/sock.h
>> +++ b/include/trace/events/sock.h
>> @@ -11,7 +11,11 @@
>> #include <linux/ipv6.h>
>> #include <linux/tcp.h>
>>
>> -/* The protocol traced by sock_set_state */
>> +#define family_names \
>> + EM(AF_INET) \
>> + EMe(AF_INET6)
>> +
>> +/* The protocol traced by inet_sock_set_state */
>> #define inet_protocol_names \
>> EM(IPPROTO_TCP) \
>> EM(IPPROTO_DCCP) \
>> @@ -37,6 +41,7 @@
>> #define EM(a) TRACE_DEFINE_ENUM(a);
>> #define EMe(a) TRACE_DEFINE_ENUM(a);
>>
>> +family_names
>> inet_protocol_names
>> tcp_state_names
>>
>> @@ -45,6 +50,9 @@
>> #define EM(a) { a, #a },
>> #define EMe(a) { a, #a }
>>
>> +#define show_family_name(val) \
>> + __print_symbolic(val, family_names)
>> +
>> #define show_inet_protocol_name(val) \
>> __print_symbolic(val, inet_protocol_names)
>>
>> @@ -108,9 +116,10 @@
>>
>> TRACE_EVENT(inet_sock_set_state,
>>
>> - TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
>> + TP_PROTO(const struct sock *sk, const int family, const int protocol,
>> + const int oldstate, const int newstate),
>
> Are there cases we need protocol and/or family that is different to
> sk->sk_protocol/sk_family? If not, I think we don't need to change the
> TP_PROTO.
>
> Thanks,
> Song
>

As of now, there're two sk_family, which are AF_INET and AF_INET6, and
three sk_protocol which are IPPROTO_TCP, IPPROTO_SCTP and
IPPROTO_DCCP.

Thanks
Yafang

>>
>> - TP_ARGS(sk, oldstate, newstate),
>> + TP_ARGS(sk, family, protocol, oldstate, newstate),
>>
>> TP_STRUCT__entry(
>> __field(const void *, skaddr)
>> @@ -118,6 +127,7 @@
>> __field(int, newstate)
>> __field(__u16, sport)
>> __field(__u16, dport)
>> + __field(__u16, family)
>> __field(__u8, protocol)
>> __array(__u8, saddr, 4)
>> __array(__u8, daddr, 4)
>> @@ -133,8 +143,9 @@
>> __entry->skaddr = sk;
>> __entry->oldstate = oldstate;
>> __entry->newstate = newstate;
>> + __entry->family = family;
>> + __entry->protocol = protocol;
>>
>> - __entry->protocol = sk->sk_protocol;
>> __entry->sport = ntohs(inet->inet_sport);
>> __entry->dport = ntohs(inet->inet_dport);
>>
>> @@ -145,7 +156,7 @@
>> *p32 = inet->inet_daddr;
>>
>> #if IS_ENABLED(CONFIG_IPV6)
>> - if (sk->sk_family == AF_INET6) {
>> + if (family == AF_INET6) {
>> pin6 = (struct in6_addr *)__entry->saddr_v6;
>> *pin6 = sk->sk_v6_rcv_saddr;
>> pin6 = (struct in6_addr *)__entry->daddr_v6;
>> @@ -160,7 +171,8 @@
>> }
>> ),
>>
>> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>> + show_family_name(__entry->family),
>> show_inet_protocol_name(__entry->protocol),
>> __entry->sport, __entry->dport,
>> __entry->saddr, __entry->daddr,
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index bab98a4..1d52796 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk)
>>
>> void inet_sk_set_state(struct sock *sk, int state)
>> {
>> - trace_inet_sock_set_state(sk, sk->sk_state, state);
>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>> + sk->sk_state, state);
>> sk->sk_state = state;
>> }
>> EXPORT_SYMBOL(inet_sk_set_state);
>>
>> void inet_sk_state_store(struct sock *sk, int newstate)
>> {
>> - trace_inet_sock_set_state(sk, sk->sk_state, newstate);
>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>> + sk->sk_state, newstate);
>> smp_store_release(&sk->sk_state, newstate);
>> }
>>
>> --
>> 1.8.3.1
>>
>

2018-01-06 17:47:02

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state


> On Jan 5, 2018, at 12:09 AM, Yafang Shao <[email protected]> wrote:
>
> On Fri, Jan 5, 2018 at 3:21 PM, Song Liu <[email protected]> wrote:
>>
>>> On Jan 4, 2018, at 10:42 PM, Yafang Shao <[email protected]> wrote:
>>>
>>> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments.
>>> Then we can conveniently use these two arguments to do the filter.
>>>
>>> Suggested-by: Brendan Gregg <[email protected]>
>>> Signed-off-by: Yafang Shao <[email protected]>
>>> ---
>>> include/trace/events/sock.h | 24 ++++++++++++++++++------
>>> net/ipv4/af_inet.c | 6 ++++--
>>> 2 files changed, 22 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
>>> index 3537c5f..c7df70f 100644
>>> --- a/include/trace/events/sock.h
>>> +++ b/include/trace/events/sock.h
>>> @@ -11,7 +11,11 @@
>>> #include <linux/ipv6.h>
>>> #include <linux/tcp.h>
>>>
>>> -/* The protocol traced by sock_set_state */
>>> +#define family_names \
>>> + EM(AF_INET) \
>>> + EMe(AF_INET6)
>>> +
>>> +/* The protocol traced by inet_sock_set_state */
>>> #define inet_protocol_names \
>>> EM(IPPROTO_TCP) \
>>> EM(IPPROTO_DCCP) \
>>> @@ -37,6 +41,7 @@
>>> #define EM(a) TRACE_DEFINE_ENUM(a);
>>> #define EMe(a) TRACE_DEFINE_ENUM(a);
>>>
>>> +family_names
>>> inet_protocol_names
>>> tcp_state_names
>>>
>>> @@ -45,6 +50,9 @@
>>> #define EM(a) { a, #a },
>>> #define EMe(a) { a, #a }
>>>
>>> +#define show_family_name(val) \
>>> + __print_symbolic(val, family_names)
>>> +
>>> #define show_inet_protocol_name(val) \
>>> __print_symbolic(val, inet_protocol_names)
>>>
>>> @@ -108,9 +116,10 @@
>>>
>>> TRACE_EVENT(inet_sock_set_state,
>>>
>>> - TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
>>> + TP_PROTO(const struct sock *sk, const int family, const int protocol,
>>> + const int oldstate, const int newstate),
>>
>> Are there cases we need protocol and/or family that is different to
>> sk->sk_protocol/sk_family? If not, I think we don't need to change the
>> TP_PROTO.
>>
>> Thanks,
>> Song
>>
>
> As of now, there're two sk_family, which are AF_INET and AF_INET6, and
> three sk_protocol which are IPPROTO_TCP, IPPROTO_SCTP and
> IPPROTO_DCCP.
>
> Thanks
> Yafang

How about we do not change the TP_PROTO line? The patch will be like:

@@ -118,6 +127,7 @@
__field(int, newstate)
__field(__u16, sport)
__field(__u16, dport)
+ __field(__u16, family)
__field(__u8, protocol)
__array(__u8, saddr, 4)
__array(__u8, daddr, 4)
@@ -133,8 +143,9 @@
__entry->skaddr = sk;
__entry->oldstate = oldstate;
__entry->newstate = newstate;
+ __entry->family = sk->sk_family;
__entry->protocol = sk->sk_protocol;
__entry->sport = ntohs(inet->inet_sport);
__entry->dport = ntohs(inet->inet_dport);
@@ xxxxx
- TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+ TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+ show_family_name(__entry->family),

Thanks,
Song

>>>
>>> - TP_ARGS(sk, oldstate, newstate),
>>> + TP_ARGS(sk, family, protocol, oldstate, newstate),
>>>
>>> TP_STRUCT__entry(
>>> __field(const void *, skaddr)
>>> @@ -118,6 +127,7 @@
>>> __field(int, newstate)
>>> __field(__u16, sport)
>>> __field(__u16, dport)
>>> + __field(__u16, family)
>>> __field(__u8, protocol)
>>> __array(__u8, saddr, 4)
>>> __array(__u8, daddr, 4)
>>> @@ -133,8 +143,9 @@
>>> __entry->skaddr = sk;
>>> __entry->oldstate = oldstate;
>>> __entry->newstate = newstate;
>>> + __entry->family = family;
>>> + __entry->protocol = protocol;
>>>
>>> - __entry->protocol = sk->sk_protocol;
>>> __entry->sport = ntohs(inet->inet_sport);
>>> __entry->dport = ntohs(inet->inet_dport);
>>>
>>> @@ -145,7 +156,7 @@
>>> *p32 = inet->inet_daddr;
>>>
>>> #if IS_ENABLED(CONFIG_IPV6)
>>> - if (sk->sk_family == AF_INET6) {
>>> + if (family == AF_INET6) {
>>> pin6 = (struct in6_addr *)__entry->saddr_v6;
>>> *pin6 = sk->sk_v6_rcv_saddr;
>>> pin6 = (struct in6_addr *)__entry->daddr_v6;
>>> @@ -160,7 +171,8 @@
>>> }
>>> ),
>>>
>>> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>>> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>>> + show_family_name(__entry->family),
>>> show_inet_protocol_name(__entry->protocol),
>>> __entry->sport, __entry->dport,
>>> __entry->saddr, __entry->daddr,
>>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>>> index bab98a4..1d52796 100644
>>> --- a/net/ipv4/af_inet.c
>>> +++ b/net/ipv4/af_inet.c
>>> @@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk)
>>>
>>> void inet_sk_set_state(struct sock *sk, int state)
>>> {
>>> - trace_inet_sock_set_state(sk, sk->sk_state, state);
>>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>>> + sk->sk_state, state);
>>> sk->sk_state = state;
>>> }
>>> EXPORT_SYMBOL(inet_sk_set_state);
>>>
>>> void inet_sk_state_store(struct sock *sk, int newstate)
>>> {
>>> - trace_inet_sock_set_state(sk, sk->sk_state, newstate);
>>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>>> + sk->sk_state, newstate);
>>> smp_store_release(&sk->sk_state, newstate);
>>> }
>>>
>>> --
>>> 1.8.3.1

2018-01-07 05:39:09

by Yafang Shao

[permalink] [raw]
Subject: Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state

On Sun, Jan 7, 2018 at 1:46 AM, Song Liu <[email protected]> wrote:
>
>> On Jan 5, 2018, at 12:09 AM, Yafang Shao <[email protected]> wrote:
>>
>> On Fri, Jan 5, 2018 at 3:21 PM, Song Liu <[email protected]> wrote:
>>>
>>>> On Jan 4, 2018, at 10:42 PM, Yafang Shao <[email protected]> wrote:
>>>>
>>>> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments.
>>>> Then we can conveniently use these two arguments to do the filter.
>>>>
>>>> Suggested-by: Brendan Gregg <[email protected]>
>>>> Signed-off-by: Yafang Shao <[email protected]>
>>>> ---
>>>> include/trace/events/sock.h | 24 ++++++++++++++++++------
>>>> net/ipv4/af_inet.c | 6 ++++--
>>>> 2 files changed, 22 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
>>>> index 3537c5f..c7df70f 100644
>>>> --- a/include/trace/events/sock.h
>>>> +++ b/include/trace/events/sock.h
>>>> @@ -11,7 +11,11 @@
>>>> #include <linux/ipv6.h>
>>>> #include <linux/tcp.h>
>>>>
>>>> -/* The protocol traced by sock_set_state */
>>>> +#define family_names \
>>>> + EM(AF_INET) \
>>>> + EMe(AF_INET6)
>>>> +
>>>> +/* The protocol traced by inet_sock_set_state */
>>>> #define inet_protocol_names \
>>>> EM(IPPROTO_TCP) \
>>>> EM(IPPROTO_DCCP) \
>>>> @@ -37,6 +41,7 @@
>>>> #define EM(a) TRACE_DEFINE_ENUM(a);
>>>> #define EMe(a) TRACE_DEFINE_ENUM(a);
>>>>
>>>> +family_names
>>>> inet_protocol_names
>>>> tcp_state_names
>>>>
>>>> @@ -45,6 +50,9 @@
>>>> #define EM(a) { a, #a },
>>>> #define EMe(a) { a, #a }
>>>>
>>>> +#define show_family_name(val) \
>>>> + __print_symbolic(val, family_names)
>>>> +
>>>> #define show_inet_protocol_name(val) \
>>>> __print_symbolic(val, inet_protocol_names)
>>>>
>>>> @@ -108,9 +116,10 @@
>>>>
>>>> TRACE_EVENT(inet_sock_set_state,
>>>>
>>>> - TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
>>>> + TP_PROTO(const struct sock *sk, const int family, const int protocol,
>>>> + const int oldstate, const int newstate),
>>>
>>> Are there cases we need protocol and/or family that is different to
>>> sk->sk_protocol/sk_family? If not, I think we don't need to change the
>>> TP_PROTO.
>>>
>>> Thanks,
>>> Song
>>>
>>
>> As of now, there're two sk_family, which are AF_INET and AF_INET6, and
>> three sk_protocol which are IPPROTO_TCP, IPPROTO_SCTP and
>> IPPROTO_DCCP.
>>
>> Thanks
>> Yafang
>
> How about we do not change the TP_PROTO line? The patch will be like:
>
> @@ -118,6 +127,7 @@
> __field(int, newstate)
> __field(__u16, sport)
> __field(__u16, dport)
> + __field(__u16, family)
> __field(__u8, protocol)
> __array(__u8, saddr, 4)
> __array(__u8, daddr, 4)
> @@ -133,8 +143,9 @@
> __entry->skaddr = sk;
> __entry->oldstate = oldstate;
> __entry->newstate = newstate;
> + __entry->family = sk->sk_family;
> __entry->protocol = sk->sk_protocol;
> __entry->sport = ntohs(inet->inet_sport);
> __entry->dport = ntohs(inet->inet_dport);
> @@ xxxxx
> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> + show_family_name(__entry->family),
>

Agreed.
IMO that's a better solution, because we make littler change and could
use the printk message to do the filter as well.

I will submit a new patch.

Thanks
Yafang

>
>>>>
>>>> - TP_ARGS(sk, oldstate, newstate),
>>>> + TP_ARGS(sk, family, protocol, oldstate, newstate),
>>>>
>>>> TP_STRUCT__entry(
>>>> __field(const void *, skaddr)
>>>> @@ -118,6 +127,7 @@
>>>> __field(int, newstate)
>>>> __field(__u16, sport)
>>>> __field(__u16, dport)
>>>> + __field(__u16, family)
>>>> __field(__u8, protocol)
>>>> __array(__u8, saddr, 4)
>>>> __array(__u8, daddr, 4)
>>>> @@ -133,8 +143,9 @@
>>>> __entry->skaddr = sk;
>>>> __entry->oldstate = oldstate;
>>>> __entry->newstate = newstate;
>>>> + __entry->family = family;
>>>> + __entry->protocol = protocol;
>>>>
>>>> - __entry->protocol = sk->sk_protocol;
>>>> __entry->sport = ntohs(inet->inet_sport);
>>>> __entry->dport = ntohs(inet->inet_dport);
>>>>
>>>> @@ -145,7 +156,7 @@
>>>> *p32 = inet->inet_daddr;
>>>>
>>>> #if IS_ENABLED(CONFIG_IPV6)
>>>> - if (sk->sk_family == AF_INET6) {
>>>> + if (family == AF_INET6) {
>>>> pin6 = (struct in6_addr *)__entry->saddr_v6;
>>>> *pin6 = sk->sk_v6_rcv_saddr;
>>>> pin6 = (struct in6_addr *)__entry->daddr_v6;
>>>> @@ -160,7 +171,8 @@
>>>> }
>>>> ),
>>>>
>>>> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>>>> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>>>> + show_family_name(__entry->family),
>>>> show_inet_protocol_name(__entry->protocol),
>>>> __entry->sport, __entry->dport,
>>>> __entry->saddr, __entry->daddr,
>>>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>>>> index bab98a4..1d52796 100644
>>>> --- a/net/ipv4/af_inet.c
>>>> +++ b/net/ipv4/af_inet.c
>>>> @@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk)
>>>>
>>>> void inet_sk_set_state(struct sock *sk, int state)
>>>> {
>>>> - trace_inet_sock_set_state(sk, sk->sk_state, state);
>>>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>>>> + sk->sk_state, state);
>>>> sk->sk_state = state;
>>>> }
>>>> EXPORT_SYMBOL(inet_sk_set_state);
>>>>
>>>> void inet_sk_state_store(struct sock *sk, int newstate)
>>>> {
>>>> - trace_inet_sock_set_state(sk, sk->sk_state, newstate);
>>>> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol,
>>>> + sk->sk_state, newstate);
>>>> smp_store_release(&sk->sk_state, newstate);
>>>> }
>>>>
>>>> --
>>>> 1.8.3.1
>