[PATCH net-next 0/3] Tracepoints for SMC

[PATCH net-next 0/3] Tracepoints for SMC

[Tony Lu]

This patch set introduces tracepoints for SMC, including the tracepoints
basic code. The tracepoitns would help us to track SMC's behaviors by
automatic tools, or other BPF tools, and zero overhead if not enabled.

Compared with kprobe and other dymatic tools, the tracepoints are
considered as stable API, and less overhead for tracing with easy-to-use
API.

Tony Lu (3):
  net/smc: Introduce tracepoint for fallback
  net/smc: Introduce tracepoints for tx and rx msg
  net/smc: Introduce tracepoint for smcr link down

 net/smc/Makefile         |   2 +
 net/smc/af_smc.c         |   2 +
 net/smc/smc_core.c       |   9 ++-
 net/smc/smc_rx.c         |   3 +
 net/smc/smc_tracepoint.c |   9 +++
 net/smc/smc_tracepoint.h | 116 +++++++++++++++++++++++++++++++++++++++
 net/smc/smc_tx.c         |   3 +
 7 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 net/smc/smc_tracepoint.c
 create mode 100644 net/smc/smc_tracepoint.h

-- 
2.19.1.6.gb485710b

[PATCH net-next 1/3] net/smc: Introduce tracepoint for fallback

[Tony Lu]

This introduces tracepoint for smc fallback to TCP, so that we can track
which connection and why it fallbacks, and map the clcsocks' pointer with
/proc/net/tcp to find more details about TCP connections. Compared with
kprobe or other dynamic tracing, tracepoints are stable and easy to use.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/Makefile         |  2 ++
 net/smc/af_smc.c         |  2 ++
 net/smc/smc_tracepoint.c |  6 +++++
 net/smc/smc_tracepoint.h | 49 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+)
 create mode 100644 net/smc/smc_tracepoint.c
 create mode 100644 net/smc/smc_tracepoint.h

diff --git a/net/smc/Makefile b/net/smc/Makefile
index 99a0186cba5b..196fb6f01b14 100644
--- a/net/smc/Makefile
+++ b/net/smc/Makefile
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
+ccflags-y += -I$(src)
 obj-$(CONFIG_SMC)	+= smc.o
 obj-$(CONFIG_SMC_DIAG)	+= smc_diag.o
 smc-y := af_smc.o smc_pnet.o smc_ib.o smc_clc.o smc_core.o smc_wr.o smc_llc.o
 smc-y += smc_cdc.o smc_tx.o smc_rx.o smc_close.o smc_ism.o smc_netlink.o smc_stats.o
+smc-y += smc_tracepoint.o
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 8dc34388b2c1..0cf7ed2f5d41 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -50,6 +50,7 @@
 #include "smc_rx.h"
 #include "smc_close.h"
 #include "smc_stats.h"
+#include "smc_tracepoint.h"
 
 static DEFINE_MUTEX(smc_server_lgr_pending);	/* serialize link group
 						 * creation on server
@@ -564,6 +565,7 @@ static void smc_switch_to_fallback(struct smc_sock *smc, int reason_code)
 	smc->use_fallback = true;
 	smc->fallback_rsn = reason_code;
 	smc_stat_fallback(smc);
+	trace_smc_switch_to_fallback(smc, reason_code);
 	if (smc->sk.sk_socket && smc->sk.sk_socket->file) {
 		smc->clcsock->file = smc->sk.sk_socket->file;
 		smc->clcsock->file->private_data = smc->clcsock;
diff --git a/net/smc/smc_tracepoint.c b/net/smc/smc_tracepoint.c
new file mode 100644
index 000000000000..861a41644971
--- /dev/null
+++ b/net/smc/smc_tracepoint.c
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define CREATE_TRACE_POINTS
+#include "smc_tracepoint.h"
+
+EXPORT_TRACEPOINT_SYMBOL(smc_switch_to_fallback);
diff --git a/net/smc/smc_tracepoint.h b/net/smc/smc_tracepoint.h
new file mode 100644
index 000000000000..3bc97f5f2134
--- /dev/null
+++ b/net/smc/smc_tracepoint.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM smc
+
+#if !defined(_TRACE_SMC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SMC_H
+
+#include <linux/ipv6.h>
+#include <linux/tcp.h>
+#include <linux/tracepoint.h>
+#include <net/ipv6.h>
+#include "smc.h"
+#include "smc_core.h"
+
+TRACE_EVENT(smc_switch_to_fallback,
+
+	    TP_PROTO(const struct smc_sock *smc, int fallback_rsn),
+
+	    TP_ARGS(smc, fallback_rsn),
+
+	    TP_STRUCT__entry(
+			     __field(const void *, sk)
+			     __field(const void *, clcsk)
+			     __field(int, fallback_rsn)
+	    ),
+
+	    TP_fast_assign(
+			   const struct sock *sk = &smc->sk;
+			   const struct sock *clcsk = smc->clcsock->sk;
+
+			   __entry->sk = sk;
+			   __entry->clcsk = clcsk;
+			   __entry->fallback_rsn = fallback_rsn;
+	    ),
+
+	    TP_printk("sk=%p clcsk=%p fallback_rsn=%d",
+		      __entry->sk, __entry->clcsk, __entry->fallback_rsn)
+);
+
+#endif /* _TRACE_SMC_H */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE smc_tracepoint
+
+#include <trace/define_trace.h>
-- 
2.19.1.6.gb485710b

[PATCH net-next 2/3] net/smc: Introduce tracepoints for tx and rx msg

[Tony Lu]

This introduce two tracepoints for smc tx and rx msg to help us
diagnosis issues of data path. These two tracepoitns don't cover the
path of CORK or MSG_MORE in tx, just the top half of data path.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_rx.c         |  3 +++
 net/smc/smc_tracepoint.c |  2 ++
 net/smc/smc_tracepoint.h | 37 +++++++++++++++++++++++++++++++++++++
 net/smc/smc_tx.c         |  3 +++
 4 files changed, 45 insertions(+)

diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index 170b733bc736..51e8eb2933ff 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -22,6 +22,7 @@
 #include "smc_tx.h" /* smc_tx_consumer_update() */
 #include "smc_rx.h"
 #include "smc_stats.h"
+#include "smc_tracepoint.h"
 
 /* callback implementation to wakeup consumers blocked with smc_rx_wait().
  * indirectly called by smc_cdc_msg_recv_action().
@@ -438,6 +439,8 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg,
 			if (msg && smc_rx_update_consumer(smc, cons, copylen))
 				goto out;
 		}
+
+		trace_smc_rx_recvmsg(smc, copylen);
 	} while (read_remaining);
 out:
 	return read_done;
diff --git a/net/smc/smc_tracepoint.c b/net/smc/smc_tracepoint.c
index 861a41644971..af031811ddb3 100644
--- a/net/smc/smc_tracepoint.c
+++ b/net/smc/smc_tracepoint.c
@@ -4,3 +4,5 @@
 #include "smc_tracepoint.h"
 
 EXPORT_TRACEPOINT_SYMBOL(smc_switch_to_fallback);
+EXPORT_TRACEPOINT_SYMBOL(smc_tx_sendmsg);
+EXPORT_TRACEPOINT_SYMBOL(smc_rx_recvmsg);
diff --git a/net/smc/smc_tracepoint.h b/net/smc/smc_tracepoint.h
index 3bc97f5f2134..eced1546afae 100644
--- a/net/smc/smc_tracepoint.h
+++ b/net/smc/smc_tracepoint.h
@@ -38,6 +38,43 @@ TRACE_EVENT(smc_switch_to_fallback,
 		      __entry->sk, __entry->clcsk, __entry->fallback_rsn)
 );
 
+DECLARE_EVENT_CLASS(smc_msg_event,
+
+		    TP_PROTO(const struct smc_sock *smc, size_t len),
+
+		    TP_ARGS(smc, len),
+
+		    TP_STRUCT__entry(
+				     __field(const void *, smc)
+				     __field(size_t, len)
+				     __string(name, smc->conn.lnk->ibname)
+		    ),
+
+		    TP_fast_assign(
+				   __entry->smc = smc;
+				   __entry->len = len;
+				   __assign_str(name, smc->conn.lnk->ibname);
+		    ),
+
+		    TP_printk("smc=%p len=%zu dev=%s",
+			      __entry->smc, __entry->len,
+			      __get_str(name))
+);
+
+DEFINE_EVENT(smc_msg_event, smc_tx_sendmsg,
+
+	     TP_PROTO(const struct smc_sock *smc, size_t len),
+
+	     TP_ARGS(smc, len)
+);
+
+DEFINE_EVENT(smc_msg_event, smc_rx_recvmsg,
+
+	     TP_PROTO(const struct smc_sock *smc, size_t len),
+
+	     TP_ARGS(smc, len)
+);
+
 #endif /* _TRACE_SMC_H */
 
 #undef TRACE_INCLUDE_PATH
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 738a4a99c827..be241d53020f 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -28,6 +28,7 @@
 #include "smc_ism.h"
 #include "smc_tx.h"
 #include "smc_stats.h"
+#include "smc_tracepoint.h"
 
 #define SMC_TX_WORK_DELAY	0
 #define SMC_TX_CORK_DELAY	(HZ >> 2)	/* 250 ms */
@@ -245,6 +246,8 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
 					   SMC_TX_CORK_DELAY);
 		else
 			smc_tx_sndbuf_nonempty(conn);
+
+		trace_smc_tx_sendmsg(smc, copylen);
 	} /* while (msg_data_left(msg)) */
 
 	return send_done;
-- 
2.19.1.6.gb485710b

[PATCH net-next 3/3] net/smc: Introduce tracepoint for smcr link down

[Tony Lu]

SMC-R link down event is important to help us find links' issues, we
should track this event, especially in the single nic mode, which means
upper layer connection would be shut down. Then find out the direct
link-down reason in time, not only increased the counter, also the
location of the code who triggered this event.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_core.c       |  9 +++++++--
 net/smc/smc_tracepoint.c |  1 +
 net/smc/smc_tracepoint.h | 30 ++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 8e642f8f334f..49b8ba3bb683 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -34,6 +34,7 @@
 #include "smc_ism.h"
 #include "smc_netlink.h"
 #include "smc_stats.h"
+#include "smc_tracepoint.h"
 
 #define SMC_LGR_NUM_INCR		256
 #define SMC_LGR_FREE_DELAY_SERV		(600 * HZ)
@@ -1620,15 +1621,19 @@ static void smcr_link_down(struct smc_link *lnk)
 /* must be called under lgr->llc_conf_mutex lock */
 void smcr_link_down_cond(struct smc_link *lnk)
 {
-	if (smc_link_downing(&lnk->state))
+	if (smc_link_downing(&lnk->state)) {
+		trace_smcr_link_down(lnk, __builtin_return_address(0));
 		smcr_link_down(lnk);
+	}
 }
 
 /* will get the lgr->llc_conf_mutex lock */
 void smcr_link_down_cond_sched(struct smc_link *lnk)
 {
-	if (smc_link_downing(&lnk->state))
+	if (smc_link_downing(&lnk->state)) {
+		trace_smcr_link_down(lnk, __builtin_return_address(0));
 		schedule_work(&lnk->link_down_wrk);
+	}
 }
 
 void smcr_port_err(struct smc_ib_device *smcibdev, u8 ibport)
diff --git a/net/smc/smc_tracepoint.c b/net/smc/smc_tracepoint.c
index af031811ddb3..8d47ced5a492 100644
--- a/net/smc/smc_tracepoint.c
+++ b/net/smc/smc_tracepoint.c
@@ -6,3 +6,4 @@
 EXPORT_TRACEPOINT_SYMBOL(smc_switch_to_fallback);
 EXPORT_TRACEPOINT_SYMBOL(smc_tx_sendmsg);
 EXPORT_TRACEPOINT_SYMBOL(smc_rx_recvmsg);
+EXPORT_TRACEPOINT_SYMBOL(smcr_link_down);
diff --git a/net/smc/smc_tracepoint.h b/net/smc/smc_tracepoint.h
index eced1546afae..b4c36795a928 100644
--- a/net/smc/smc_tracepoint.h
+++ b/net/smc/smc_tracepoint.h
@@ -75,6 +75,36 @@ DEFINE_EVENT(smc_msg_event, smc_rx_recvmsg,
 	     TP_ARGS(smc, len)
 );
 
+TRACE_EVENT(smcr_link_down,
+
+	    TP_PROTO(const struct smc_link *lnk, void *location),
+
+	    TP_ARGS(lnk, location),
+
+	    TP_STRUCT__entry(
+			     __field(const void *, lnk)
+			     __field(const void *, lgr)
+			     __field(int, state)
+			     __string(name, lnk->ibname)
+			     __field(void *, location)
+	    ),
+
+	    TP_fast_assign(
+			   const struct smc_link_group *lgr = lnk->lgr;
+
+			   __entry->lnk = lnk;
+			   __entry->lgr = lgr;
+			   __entry->state = lnk->state;
+			   __assign_str(name, lnk->ibname);
+			   __entry->location = location;
+	    ),
+
+	    TP_printk("lnk=%p lgr=%p state=%d dev=%s location=%p",
+		      __entry->lnk, __entry->lgr,
+		      __entry->state, __get_str(name),
+		      __entry->location)
+);
+
 #endif /* _TRACE_SMC_H */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.19.1.6.gb485710b

Re: [PATCH net-next 0/3] Tracepoints for SMC

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon,  1 Nov 2021 15:39:10 +0800 you wrote:
> This patch set introduces tracepoints for SMC, including the tracepoints
> basic code. The tracepoitns would help us to track SMC's behaviors by
> automatic tools, or other BPF tools, and zero overhead if not enabled.
> 
> Compared with kprobe and other dymatic tools, the tracepoints are
> considered as stable API, and less overhead for tracing with easy-to-use
> API.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] net/smc: Introduce tracepoint for fallback
    https://git.kernel.org/netdev/net-next/c/482626086820
  - [net-next,2/3] net/smc: Introduce tracepoints for tx and rx msg
    https://git.kernel.org/netdev/net-next/c/aff3083f10bf
  - [net-next,3/3] net/smc: Introduce tracepoint for smcr link down
    https://git.kernel.org/netdev/net-next/c/a3a0e81b6fd5

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 3/3] net/smc: Introduce tracepoint for smcr link down

[Karsten Graul]

On 01/11/2021 08:39, Tony Lu wrote:
> +
> +	    TP_printk("lnk=%p lgr=%p state=%d dev=%s location=%p",
> +		      __entry->lnk, __entry->lgr,
> +		      __entry->state, __get_str(name),
> +		      __entry->location)

The location is printed as pointer (which might even be randomized?),
is it possible to print the function name of the caller, as described
here: https://stackoverflow.com/questions/4141324/function-caller-in-linux-kernel

  printk("Caller is %pS\n", __builtin_return_address(0));

Not sure if this is possible with the trace points, but it would be
easier to use. You plan to use a dump to find out about the function caller?

Re: [PATCH net-next 3/3] net/smc: Introduce tracepoint for smcr link down

[Tony Lu]

On Tue, Nov 02, 2021 at 10:30:22AM +0100, Karsten Graul wrote:
> On 01/11/2021 08:39, Tony Lu wrote:
> > +
> > +	    TP_printk("lnk=%p lgr=%p state=%d dev=%s location=%p",
> > +		      __entry->lnk, __entry->lgr,
> > +		      __entry->state, __get_str(name),
> > +		      __entry->location)
> 
> The location is printed as pointer (which might even be randomized?),
> is it possible to print the function name of the caller, as described
> here: https://stackoverflow.com/questions/4141324/function-caller-in-linux-kernel
> 
>   printk("Caller is %pS\n", __builtin_return_address(0));
> 
> Not sure if this is possible with the trace points, but it would be
> easier to use. You plan to use a dump to find out about the function caller?

Yes, I am going to find out where caused the SMC-R link down. In our
test environment, the tracepoint for link down help me to debug the root
cause of link termination, without eBPF or systemtap.

By using "%pS", it makes the trace log easy to show the function caller
name without additional translating.

  <idle>-0       [000] ..s.    69.087164: smcr_link_down: lnk=00000000dab41cdc lgr=000000007d5d8e24 state=0 rc=1 dev=mlx5_0 location=smc_wr_tx_tasklet_fn+0x5ef/0x6f0 [smc]

I will improve it in the next patch.

Cheers,
Tony Lu