Re: [v2 PATCH -tip 3/6] net: sctp: Add SCTP ACK tracking trace event

2017-12-19 Thread kbuild test robot 
Hi Masami,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on v4.15-rc4 next-20171219]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/net-tcp-sctp-dccp-Replace-jprobe-usage-with-trace-events/20171220-081035
config: i386-randconfig-x011-201751 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/trace/define_trace.h:96:0,
from include/trace/events/sctp.h:96,
from net//sctp/sm_statefuns.c:63:
   include/trace/events/sctp.h: In function 
'trace_event_raw_event_sctp_probe_path':
>> include/trace/events/sctp.h:31:19: warning: cast from pointer to integer of 
>> different size [-Wpointer-to-int-cast]
  __entry->asoc = (__u64)asoc;
  ^
   include/trace/trace_events.h:719:4: note: in definition of macro 
'DECLARE_EVENT_CLASS'
 { assign; }   \
   ^~
   include/trace/trace_events.h:78:9: note: in expansion of macro 'PARAMS'
PARAMS(assign), \
^~
>> include/trace/events/sctp.h:11:1: note: in expansion of macro 'TRACE_EVENT'
TRACE_EVENT(sctp_probe_path,
^~~
>> include/trace/events/sctp.h:30:2: note: in expansion of macro 
>> 'TP_fast_assign'
 TP_fast_assign(
 ^~
   include/trace/events/sctp.h: In function 'trace_event_raw_event_sctp_probe':
   include/trace/events/sctp.h:72:19: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
  __entry->asoc = (__u64)asoc;
  ^
   include/trace/trace_events.h:719:4: note: in definition of macro 
'DECLARE_EVENT_CLASS'
 { assign; }   \
   ^~
   include/trace/trace_events.h:78:9: note: in expansion of macro 'PARAMS'
PARAMS(assign), \
^~
   include/trace/events/sctp.h:50:1: note: in expansion of macro 'TRACE_EVENT'
TRACE_EVENT(sctp_probe,
^~~
   include/trace/events/sctp.h:68:2: note: in expansion of macro 
'TP_fast_assign'
 TP_fast_assign(
 ^~
   In file included from include/trace/define_trace.h:97:0,
from include/trace/events/sctp.h:96,
from net//sctp/sm_statefuns.c:63:
   include/trace/events/sctp.h: In function 'perf_trace_sctp_probe_path':
>> include/trace/events/sctp.h:31:19: warning: cast from pointer to integer of 
>> different size [-Wpointer-to-int-cast]
  __entry->asoc = (__u64)asoc;
  ^
   include/trace/perf.h:66:4: note: in definition of macro 'DECLARE_EVENT_CLASS'
 { assign; }   \
   ^~
   include/trace/trace_events.h:78:9: note: in expansion of macro 'PARAMS'
PARAMS(assign), \
^~
>> include/trace/events/sctp.h:11:1: note: in expansion of macro 'TRACE_EVENT'
TRACE_EVENT(sctp_probe_path,
^~~
>> include/trace/events/sctp.h:30:2: note: in expansion of macro 
>> 'TP_fast_assign'
 TP_fast_assign(
 ^~
   include/trace/events/sctp.h: In function 'perf_trace_sctp_probe':
   include/trace/events/sctp.h:72:19: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-cast]
  __entry->asoc = (__u64)asoc;
  ^
   include/trace/perf.h:66:4: note: in definition of macro 'DECLARE_EVENT_CLASS'
 { assign; }   \
   ^~
   include/trace/trace_events.h:78:9: note: in expansion of macro 'PARAMS'
PARAMS(assign), \
^~
   include/trace/events/sctp.h:50:1: note: in expansion of macro 'TRACE_EVENT'
TRACE_EVENT(sctp_probe,
^~~
   include/trace/events/sctp.h:68:2: note: in expansion of macro 
'TP_fast_assign'
 TP_fast_assign(
 ^~

vim +31 include/trace/events/sctp.h

10  
  > 11  TRACE_EVENT(sctp_probe_path,
12  
13  TP_PROTO(struct sctp_transport *sp,
14   const struct sctp_association *asoc),
15  
16  TP_ARGS(sp, asoc),
17  
18  TP_STRUCT__entry(
19  __field(__u64, asoc)
20  __field(__u32, primary)
21  __array(__u8, ipaddr, sizeof(union sctp_addr))
22  __field(__u32, state)
23  __field(__u32, cwnd)
24  __field(__u32, ssthresh)
25  __field(__u32, flight_size)
26  __field(__u32, partial_bytes_acked)
27  __field(__u32, pathmtu)
28  ),
29  
  > 30  TP_fast_assign(
  > 31  __entry->asoc = (__u64)asoc;
32  __entry->primary = (sp == asoc->peer.primary_path);
33

Re: [v2 PATCH -tip 3/6] net: sctp: Add SCTP ACK tracking trace event

2017-12-18 Thread Masami Hiramatsu 
On Mon, 18 Dec 2017 12:05:16 -0500
Steven Rostedt  wrote:

> On Mon, 18 Dec 2017 17:12:15 +0900
> Masami Hiramatsu  wrote:
> 
> > Add SCTP ACK tracking trace event to trace the changes of SCTP
> > association state in response to incoming packets.
> > It is used for debugging SCTP congestion control algorithms,
> > and will replace sctp_probe module.
> > 
> > Note that this event a bit tricky. Since this consists of 2
> > events (sctp_probe and sctp_probe_path) so you have to enable
> > both events as below.
> > 
> >   # cd /sys/kernel/debug/tracing
> >   # echo 1 > events/sctp/sctp_probe/enable
> >   # echo 1 > events/sctp/sctp_probe_path/enable
> > 
> > Or, you can enable all the events under sctp.
> > 
> >   # echo 1 > events/sctp/enable
> > 
> > Since sctp_probe_path event is always invoked from sctp_probe
> > event, you can not see any output if you only enable
> > sctp_probe_path.
> 
> I have to ask, why did you do it this way?
> 
> 
> > +#include 
> > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> > index 8f8ccded13e4..c5f92b2cc5c3 100644
> > --- a/net/sctp/sm_statefuns.c
> > +++ b/net/sctp/sm_statefuns.c
> > @@ -59,6 +59,9 @@
> >  #include 
> >  #include 
> >  
> > +#define CREATE_TRACE_POINTS
> > +#include 
> > +
> >  static struct sctp_packet *sctp_abort_pkt_new(
> > struct net *net,
> > const struct sctp_endpoint *ep,
> > @@ -3219,6 +3222,8 @@ enum sctp_disposition sctp_sf_eat_sack_6_2(struct net 
> > *net,
> > struct sctp_sackhdr *sackh;
> > __u32 ctsn;
> >  
> > +   trace_sctp_probe(ep, asoc, chunk);
> 
> What about doing this right after this probe:
> 
>   if (trace_sctp_probe_path_enabled()) {
>   struct sctp_transport *sp;
> 
>   list_for_each_entry(sp, >peer.transpor_addr_list,
>   transports) {
>   trace_sctp_probe_path(sp, asoc);
>   }
>   }
> 
> The "trace_sctp_probe_path_enabled()" is a static branch, which means
> it's a nop just like a tracepoint is, and will not add any overhead if
> the trace_sctp_probe_path is not enabled.

That's a good idea! I'll update to use it :)

Thank you,

> 
> -- Steve
> 
> > +
> > if (!sctp_vtag_verify(chunk, asoc))
> > return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
> >  
> 


-- 
Masami Hiramatsu

Re: [v2 PATCH -tip 3/6] net: sctp: Add SCTP ACK tracking trace event

2017-12-18 Thread Steven Rostedt 
On Mon, 18 Dec 2017 17:12:15 +0900
Masami Hiramatsu  wrote:

> Add SCTP ACK tracking trace event to trace the changes of SCTP
> association state in response to incoming packets.
> It is used for debugging SCTP congestion control algorithms,
> and will replace sctp_probe module.
> 
> Note that this event a bit tricky. Since this consists of 2
> events (sctp_probe and sctp_probe_path) so you have to enable
> both events as below.
> 
>   # cd /sys/kernel/debug/tracing
>   # echo 1 > events/sctp/sctp_probe/enable
>   # echo 1 > events/sctp/sctp_probe_path/enable
> 
> Or, you can enable all the events under sctp.
> 
>   # echo 1 > events/sctp/enable
> 
> Since sctp_probe_path event is always invoked from sctp_probe
> event, you can not see any output if you only enable
> sctp_probe_path.

I have to ask, why did you do it this way?


> +#include 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 8f8ccded13e4..c5f92b2cc5c3 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -59,6 +59,9 @@
>  #include 
>  #include 
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  static struct sctp_packet *sctp_abort_pkt_new(
>   struct net *net,
>   const struct sctp_endpoint *ep,
> @@ -3219,6 +3222,8 @@ enum sctp_disposition sctp_sf_eat_sack_6_2(struct net 
> *net,
>   struct sctp_sackhdr *sackh;
>   __u32 ctsn;
>  
> + trace_sctp_probe(ep, asoc, chunk);

What about doing this right after this probe:

if (trace_sctp_probe_path_enabled()) {
struct sctp_transport *sp;

list_for_each_entry(sp, >peer.transpor_addr_list,
transports) {
trace_sctp_probe_path(sp, asoc);
}
}

The "trace_sctp_probe_path_enabled()" is a static branch, which means
it's a nop just like a tracepoint is, and will not add any overhead if
the trace_sctp_probe_path is not enabled.

-- Steve

> +
>   if (!sctp_vtag_verify(chunk, asoc))
>   return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
>

[v2 PATCH -tip 3/6] net: sctp: Add SCTP ACK tracking trace event

2017-12-18 Thread Masami Hiramatsu 
Add SCTP ACK tracking trace event to trace the changes of SCTP
association state in response to incoming packets.
It is used for debugging SCTP congestion control algorithms,
and will replace sctp_probe module.

Note that this event a bit tricky. Since this consists of 2
events (sctp_probe and sctp_probe_path) so you have to enable
both events as below.

  # cd /sys/kernel/debug/tracing
  # echo 1 > events/sctp/sctp_probe/enable
  # echo 1 > events/sctp/sctp_probe_path/enable

Or, you can enable all the events under sctp.

  # echo 1 > events/sctp/enable

Since sctp_probe_path event is always invoked from sctp_probe
event, you can not see any output if you only enable
sctp_probe_path.


Signed-off-by: Masami Hiramatsu 
---
 include/trace/events/sctp.h |   96 +++
 net/sctp/sm_statefuns.c |5 ++
 2 files changed, 101 insertions(+)
 create mode 100644 include/trace/events/sctp.h

diff --git a/include/trace/events/sctp.h b/include/trace/events/sctp.h
new file mode 100644
index ..32c2dc72311e
--- /dev/null
+++ b/include/trace/events/sctp.h
@@ -0,0 +1,96 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM sctp
+
+#if !defined(_TRACE_SCTP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SCTP_H
+
+#include 
+#include 
+
+TRACE_EVENT(sctp_probe_path,
+
+   TP_PROTO(struct sctp_transport *sp,
+const struct sctp_association *asoc),
+
+   TP_ARGS(sp, asoc),
+
+   TP_STRUCT__entry(
+   __field(__u64, asoc)
+   __field(__u32, primary)
+   __array(__u8, ipaddr, sizeof(union sctp_addr))
+   __field(__u32, state)
+   __field(__u32, cwnd)
+   __field(__u32, ssthresh)
+   __field(__u32, flight_size)
+   __field(__u32, partial_bytes_acked)
+   __field(__u32, pathmtu)
+   ),
+
+   TP_fast_assign(
+   __entry->asoc = (__u64)asoc;
+   __entry->primary = (sp == asoc->peer.primary_path);
+   memcpy(__entry->ipaddr, >ipaddr, sizeof(union sctp_addr));
+   __entry->state = sp->state;
+   __entry->cwnd = sp->cwnd;
+   __entry->ssthresh = sp->ssthresh;
+   __entry->flight_size = sp->flight_size;
+   __entry->partial_bytes_acked = sp->partial_bytes_acked;
+   __entry->pathmtu = sp->pathmtu;
+   ),
+
+   TP_printk("asoc=%#llx%s ipaddr=%pISpc state=%u cwnd=%u ssthresh=%u "
+ "flight_size=%u partial_bytes_acked=%u pathmtu=%u",
+ __entry->asoc, __entry->primary ? "(*)" : "",
+ __entry->ipaddr, __entry->state, __entry->cwnd,
+ __entry->ssthresh, __entry->flight_size,
+ __entry->partial_bytes_acked, __entry->pathmtu)
+);
+
+TRACE_EVENT(sctp_probe,
+
+   TP_PROTO(const struct sctp_endpoint *ep,
+const struct sctp_association *asoc,
+struct sctp_chunk *chunk),
+
+   TP_ARGS(ep, asoc, chunk),
+
+   TP_STRUCT__entry(
+   __field(__u64, asoc)
+   __field(__u32, mark)
+   __field(__u16, bind_port)
+   __field(__u16, peer_port)
+   __field(__u32, pathmtu)
+   __field(__u32, rwnd)
+   __field(__u16, unack_data)
+   ),
+
+   TP_fast_assign(
+   struct sctp_transport *sp;
+   struct sk_buff *skb = chunk->skb;
+
+   __entry->asoc = (__u64)asoc;
+   __entry->mark = skb->mark;
+   __entry->bind_port = ep->base.bind_addr.port;
+   __entry->peer_port = asoc->peer.port;
+   __entry->pathmtu = asoc->pathmtu;
+   __entry->rwnd = asoc->peer.rwnd;
+   __entry->unack_data = asoc->unack_data;
+
+   list_for_each_entry(sp, >peer.transport_addr_list,
+   transports) {
+   trace_sctp_probe_path(sp, asoc);
+   }
+   ),
+
+   TP_printk("asoc=%#llx mark=%#x bind_port=%d peer_port=%d pathmtu=%d "
+ "rwnd=%u unack_data=%d",
+ __entry->asoc, __entry->mark, __entry->bind_port,
+ __entry->peer_port, __entry->pathmtu, __entry->rwnd,
+ __entry->unack_data)
+);
+
+#endif /* _TRACE_SCTP_H */
+
+/* This part must be outside protection */
+#include 
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 8f8ccded13e4..c5f92b2cc5c3 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -59,6 +59,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 static struct sctp_packet *sctp_abort_pkt_new(
struct net *net,
const struct sctp_endpoint *ep,
@@ -3219,6 +3222,8 @@ enum sctp_disposition sctp_sf_eat_sack_6_2(struct net