All of lore.kernel.org
 help / color / mirror / Atom feed
* [MPTCP][PATCH v2 mptcp-next 0/5] add tracepoints
@ 2021-03-24  8:51 Geliang Tang
  2021-03-24  8:51 ` [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Geliang Tang
  0 siblings, 1 reply; 10+ messages in thread
From: Geliang Tang @ 2021-03-24  8:51 UTC (permalink / raw)
  To: mptcp, mptcp; +Cc: Geliang Tang

v2:
 - add two new patches.
 - update the patch set as Paolo suggested.

This patchset addressed issues 131, replace some/most pr_debug with trace
events. As Paolo suggested, we should add the tracepoints at "the packet
scheduler and the mapping status" first.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/131

Geliang Tang (5):
  mptcp: export mptcp_subflow_active
  mptcp: add tracepoint in mptcp_subflow_get_send
  mptcp: add tracepoint in get_mapping_status
  mptcp: add tracepoint in ack_update_msk
  mptcp: add tracepoint in subflow_check_data_avail

 include/trace/events/mptcp.h | 151 +++++++++++++++++++++++++++++++++++
 net/mptcp/options.c          |   3 +
 net/mptcp/protocol.c         |   6 +-
 net/mptcp/protocol.h         |   1 +
 net/mptcp/subflow.c          |  10 +--
 5 files changed, 164 insertions(+), 7 deletions(-)
 create mode 100644 include/trace/events/mptcp.h

-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active
  2021-03-24  8:51 [MPTCP][PATCH v2 mptcp-next 0/5] add tracepoints Geliang Tang
@ 2021-03-24  8:51 ` Geliang Tang
  2021-03-24  8:51   ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Geliang Tang
  2021-03-29 13:52   ` [MPTCP] [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Paolo Abeni
  0 siblings, 2 replies; 10+ messages in thread
From: Geliang Tang @ 2021-03-24  8:51 UTC (permalink / raw)
  To: mptcp, mptcp; +Cc: Geliang Tang

This patch exported the static function mptcp_subflow_active.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/protocol.c | 2 +-
 net/mptcp/protocol.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9d7e7e13fba8..3ad236689480 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -399,7 +399,7 @@ static void mptcp_set_timeout(const struct sock *sk, const struct sock *ssk)
 	mptcp_sk(sk)->timer_ival = tout > 0 ? tout : TCP_RTO_MIN;
 }
 
-static bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
+bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
 {
 	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 14f0114be17a..20770fe39ea5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -538,6 +538,7 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
 void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
 			 struct sockaddr_storage *addr,
 			 unsigned short family);
+bool mptcp_subflow_active(struct mptcp_subflow_context *subflow);
 
 static inline void mptcp_subflow_tcp_fallback(struct sock *sk,
 					      struct mptcp_subflow_context *ctx)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send
  2021-03-24  8:51 ` [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Geliang Tang
@ 2021-03-24  8:51   ` Geliang Tang
  2021-03-24  8:51     ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Geliang Tang
  2021-03-29 13:52     ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Paolo Abeni
  2021-03-29 13:52   ` [MPTCP] [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Paolo Abeni
  1 sibling, 2 replies; 10+ messages in thread
From: Geliang Tang @ 2021-03-24  8:51 UTC (permalink / raw)
  To: mptcp, mptcp; +Cc: Geliang Tang, Paolo Abeni

This patch added a tracepoint in the packet scheduler function
mptcp_subflow_get_send().

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 include/trace/events/mptcp.h | 43 ++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.c         |  4 ++++
 2 files changed, 47 insertions(+)
 create mode 100644 include/trace/events/mptcp.h

diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
new file mode 100644
index 000000000000..06908ea6afb4
--- /dev/null
+++ b/include/trace/events/mptcp.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM mptcp
+
+#if !defined(_TRACE_MPTCP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_MPTCP_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(mptcp_subflow_get_send,
+
+	TP_PROTO(struct mptcp_subflow_context *subflow, struct sock *ssk),
+
+	TP_ARGS(subflow, ssk),
+
+	TP_STRUCT__entry(
+		__field(bool, active)
+		__field(bool, free)
+		__field(u32, snd_wnd)
+		__field(u32, pace)
+		__field(u8, backup)
+		__field(u64, ratio)
+	),
+
+	TP_fast_assign(
+		__entry->active = mptcp_subflow_active(subflow);
+		__entry->free = sk_stream_memory_free(subflow->tcp_sock);
+		__entry->snd_wnd = tcp_sk(ssk)->snd_wnd;
+		__entry->pace = ssk->sk_pacing_rate;
+		__entry->backup = subflow->backup;
+		__entry->ratio = div_u64((u64)ssk->sk_wmem_queued << 32, __entry->pace);
+	),
+
+	TP_printk("active=%d free=%d snd_wnd=%u pace=%u backup=%u ratio=%llu",
+		  __entry->active, __entry->free,
+		  __entry->snd_wnd, __entry->pace,
+		  __entry->backup, __entry->ratio)
+);
+
+#endif /* _TRACE_MPTCP_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3ad236689480..0265b8be0f84 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -25,6 +25,9 @@
 #include "protocol.h"
 #include "mib.h"
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/mptcp.h>
+
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 struct mptcp6_sock {
 	struct mptcp_sock msk;
@@ -1394,6 +1397,7 @@ static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
 	}
 	mptcp_for_each_subflow(msk, subflow) {
 		ssk =  mptcp_subflow_tcp_sock(subflow);
+		trace_mptcp_subflow_get_send(subflow, ssk);
 		if (!mptcp_subflow_active(subflow))
 			continue;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status
  2021-03-24  8:51   ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Geliang Tang
@ 2021-03-24  8:51     ` Geliang Tang
  2021-03-24  8:51       ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Geliang Tang
  2021-03-29 14:00       ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Paolo Abeni
  2021-03-29 13:52     ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Paolo Abeni
  1 sibling, 2 replies; 10+ messages in thread
From: Geliang Tang @ 2021-03-24  8:51 UTC (permalink / raw)
  To: mptcp, mptcp; +Cc: Geliang Tang, Paolo Abeni

This patch added a tracepoint in the mapping status function
get_mapping_status() to dump every mpext field.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 include/trace/events/mptcp.h | 54 ++++++++++++++++++++++++++++++++++++
 net/mptcp/subflow.c          |  6 ++--
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
index 06908ea6afb4..503a6098909d 100644
--- a/include/trace/events/mptcp.h
+++ b/include/trace/events/mptcp.h
@@ -37,6 +37,60 @@ TRACE_EVENT(mptcp_subflow_get_send,
 		  __entry->backup, __entry->ratio)
 );
 
+DECLARE_EVENT_CLASS(mptcp_dump_mpext,
+
+	TP_PROTO(struct mptcp_ext *mpext),
+
+	TP_ARGS(mpext),
+
+	TP_STRUCT__entry(
+		__field(u64, data_ack)
+		__field(u32, data_ack32)
+		__field(u64, data_seq)
+		__field(u32, subflow_seq)
+		__field(u16, data_len)
+		__field(u8, use_map)
+		__field(u8, dsn64)
+		__field(u8, data_fin)
+		__field(u8, use_ack)
+		__field(u8, ack64)
+		__field(u8, mpc_map)
+		__field(u8, frozen)
+		__field(u8, reset_transient)
+		__field(u8, reset_reason)
+	),
+
+	TP_fast_assign(
+		__entry->data_ack = mpext->data_ack;
+		__entry->data_ack32 = mpext->data_ack32;
+		__entry->data_seq = mpext->data_seq;
+		__entry->subflow_seq = mpext->subflow_seq;
+		__entry->data_len = mpext->data_len;
+		__entry->use_map = mpext->use_map;
+		__entry->dsn64 = mpext->dsn64;
+		__entry->data_fin = mpext->data_fin;
+		__entry->use_ack = mpext->use_ack;
+		__entry->ack64 = mpext->ack64;
+		__entry->mpc_map = mpext->mpc_map;
+		__entry->frozen = mpext->frozen;
+		__entry->reset_transient = mpext->reset_transient;
+		__entry->reset_reason = mpext->reset_reason;
+	),
+
+	TP_printk("data_ack=%llu data_ack32=%u data_seq=%llu subflow_seq=%u data_len=%u use_map=%u dsn64=%u data_fin=%u use_ack=%u ack64=%u mpc_map=%u frozen=%u reset_transient=%u reset_reason=%u",
+		  __entry->data_ack, __entry->data_ack32,
+		  __entry->data_seq, __entry->subflow_seq,
+		  __entry->data_len, __entry->use_map,
+		  __entry->dsn64, __entry->data_fin,
+		  __entry->use_ack, __entry->ack64,
+		  __entry->mpc_map, __entry->frozen,
+		  __entry->reset_transient, __entry->reset_reason)
+);
+
+DEFINE_EVENT(mptcp_dump_mpext, get_mapping_status,
+	TP_PROTO(struct mptcp_ext *mpext),
+	TP_ARGS(mpext));
+
 #endif /* _TRACE_MPTCP_H */
 
 /* This part must be outside protection */
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 5fc3cada11dd..dc08e5de807e 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -25,6 +25,8 @@
 #include "protocol.h"
 #include "mib.h"
 
+#include <trace/events/mptcp.h>
+
 static void mptcp_subflow_ops_undo_override(struct sock *ssk);
 
 static void SUBFLOW_REQ_INC_STATS(struct request_sock *req,
@@ -853,9 +855,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 		goto validate_seq;
 	}
 
-	pr_debug("seq=%llu is64=%d ssn=%u data_len=%u data_fin=%d",
-		 mpext->data_seq, mpext->dsn64, mpext->subflow_seq,
-		 mpext->data_len, mpext->data_fin);
+	trace_get_mapping_status(mpext);
 
 	data_len = mpext->data_len;
 	if (data_len == 0) {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk
  2021-03-24  8:51     ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Geliang Tang
@ 2021-03-24  8:51       ` Geliang Tang
  2021-03-24  8:51         ` [MPTCP][PATCH v2 mptcp-next 5/5] mptcp: add tracepoint in subflow_check_data_avail Geliang Tang
  2021-03-29 14:03         ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Paolo Abeni
  2021-03-29 14:00       ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Paolo Abeni
  1 sibling, 2 replies; 10+ messages in thread
From: Geliang Tang @ 2021-03-24  8:51 UTC (permalink / raw)
  To: mptcp, mptcp; +Cc: Geliang Tang, Paolo Abeni

This patch added a tracepoint in ack_update_msk() to track the
incoming data_ack and window/snd_una updates.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 include/trace/events/mptcp.h | 25 +++++++++++++++++++++++++
 net/mptcp/options.c          |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
index 503a6098909d..2b8ae5e81cd1 100644
--- a/include/trace/events/mptcp.h
+++ b/include/trace/events/mptcp.h
@@ -91,6 +91,31 @@ DEFINE_EVENT(mptcp_dump_mpext, get_mapping_status,
 	TP_PROTO(struct mptcp_ext *mpext),
 	TP_ARGS(mpext));
 
+TRACE_EVENT(ack_update_msk,
+
+	TP_PROTO(u64 data_ack, u64 old_snd_una, u64 new_snd_una, u64 new_wnd_end),
+
+	TP_ARGS(data_ack, old_snd_una, new_snd_una, new_wnd_end),
+
+	TP_STRUCT__entry(
+		__field(u64, data_ack)
+		__field(u64, old_snd_una)
+		__field(u64, new_snd_una)
+		__field(u64, new_wnd_end)
+	),
+
+	TP_fast_assign(
+		__entry->data_ack = data_ack;
+		__entry->old_snd_una = old_snd_una;
+		__entry->new_snd_una = new_snd_una;
+		__entry->new_wnd_end = new_wnd_end;
+	),
+
+	TP_printk("data_ack=%llu old_snd_una=%llu new_snd_una=%llu new_wnd_end=%llu",
+		  __entry->data_ack, __entry->old_snd_una,
+		  __entry->new_snd_una, __entry->new_wnd_end)
+);
+
 #endif /* _TRACE_MPTCP_H */
 
 /* This part must be outside protection */
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index c7eb61d0564c..87bd1bfde964 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -13,6 +13,8 @@
 #include "protocol.h"
 #include "mib.h"
 
+#include <trace/events/mptcp.h>
+
 static bool mptcp_cap_flag_sha256(u8 flags)
 {
 	return (flags & MPTCP_CAP_FLAG_MASK) == MPTCP_CAP_HMAC_SHA256;
@@ -942,6 +944,7 @@ static void ack_update_msk(struct mptcp_sock *msk,
 		__mptcp_data_acked(sk);
 	}
 	mptcp_data_unlock(sk);
+	trace_ack_update_msk(mp_opt->data_ack, old_snd_una, new_snd_una, new_wnd_end);
 }
 
 bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq, bool use_64bit)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [MPTCP][PATCH v2 mptcp-next 5/5] mptcp: add tracepoint in subflow_check_data_avail
  2021-03-24  8:51       ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Geliang Tang
@ 2021-03-24  8:51         ` Geliang Tang
  2021-03-29 14:03         ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Paolo Abeni
  1 sibling, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2021-03-24  8:51 UTC (permalink / raw)
  To: mptcp, mptcp; +Cc: Geliang Tang, Paolo Abeni

This patch added a tracepoint in subflow_check_data_avail() to show the
mapping status.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 include/trace/events/mptcp.h | 29 +++++++++++++++++++++++++++++
 net/mptcp/subflow.c          |  4 +---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
index 2b8ae5e81cd1..5571c45c61f2 100644
--- a/include/trace/events/mptcp.h
+++ b/include/trace/events/mptcp.h
@@ -7,6 +7,14 @@
 
 #include <linux/tracepoint.h>
 
+#define show_mapping_status(status)					\
+	__print_symbolic(status,					\
+		{ 0, "MAPPING_OK" },					\
+		{ 1, "MAPPING_INVALID" },				\
+		{ 2, "MAPPING_EMPTY" },					\
+		{ 3, "MAPPING_DATA_FIN" },				\
+		{ 4, "MAPPING_DUMMY" })
+
 TRACE_EVENT(mptcp_subflow_get_send,
 
 	TP_PROTO(struct mptcp_subflow_context *subflow, struct sock *ssk),
@@ -116,6 +124,27 @@ TRACE_EVENT(ack_update_msk,
 		  __entry->new_snd_una, __entry->new_wnd_end)
 );
 
+TRACE_EVENT(subflow_check_data_avail,
+
+	TP_PROTO(__u8 status, struct sk_buff *skb),
+
+	TP_ARGS(status, skb),
+
+	TP_STRUCT__entry(
+		__field(u8, status)
+		__field(const void *, skb)
+	),
+
+	TP_fast_assign(
+		__entry->status = status;
+		__entry->skb = skb;
+	),
+
+	TP_printk("mapping_status=%s, skb=%p",
+		  show_mapping_status(__entry->status),
+		  __entry->skb)
+);
+
 #endif /* _TRACE_MPTCP_H */
 
 /* This part must be outside protection */
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index dc08e5de807e..de8f16006fce 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -993,8 +993,6 @@ static bool subflow_check_data_avail(struct sock *ssk)
 	struct mptcp_sock *msk;
 	struct sk_buff *skb;
 
-	pr_debug("msk=%p ssk=%p data_avail=%d skb=%p", subflow->conn, ssk,
-		 subflow->data_avail, skb_peek(&ssk->sk_receive_queue));
 	if (!skb_peek(&ssk->sk_receive_queue))
 		subflow->data_avail = 0;
 	if (subflow->data_avail)
@@ -1006,7 +1004,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
 		u64 old_ack;
 
 		status = get_mapping_status(ssk, msk);
-		pr_debug("msk=%p ssk=%p status=%d", msk, ssk, status);
+		trace_subflow_check_data_avail(status, skb_peek(&ssk->sk_receive_queue));
 		if (status == MAPPING_INVALID) {
 			ssk->sk_err = EBADMSG;
 			goto fatal;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send
  2021-03-24  8:51   ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Geliang Tang
  2021-03-24  8:51     ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Geliang Tang
@ 2021-03-29 13:52     ` Paolo Abeni
  1 sibling, 0 replies; 10+ messages in thread
From: Paolo Abeni @ 2021-03-29 13:52 UTC (permalink / raw)
  To: Geliang Tang, mptcp, mptcp

On Wed, 2021-03-24 at 16:51 +0800, Geliang Tang wrote:
> This patch added a tracepoint in the packet scheduler function
> mptcp_subflow_get_send().
> 
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>  include/trace/events/mptcp.h | 43 ++++++++++++++++++++++++++++++++++++
>  net/mptcp/protocol.c         |  4 ++++
>  2 files changed, 47 insertions(+)
>  create mode 100644 include/trace/events/mptcp.h
> 
> diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
> new file mode 100644
> index 000000000000..06908ea6afb4
> --- /dev/null
> +++ b/include/trace/events/mptcp.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM mptcp
> +
> +#if !defined(_TRACE_MPTCP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_MPTCP_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(mptcp_subflow_get_send,
> +
> +	TP_PROTO(struct mptcp_subflow_context *subflow, struct sock *ssk),
> +
> +	TP_ARGS(subflow, ssk),
> +
> +	TP_STRUCT__entry(
> +		__field(bool, active)
> +		__field(bool, free)
> +		__field(u32, snd_wnd)
> +		__field(u32, pace)
> +		__field(u8, backup)
> +		__field(u64, ratio)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->active = mptcp_subflow_active(subflow);
> +		__entry->free = sk_stream_memory_free(subflow->tcp_sock);

I'm sorry for not noticing the thing before, but subflow->tcp_sock can
be potentially a TIME_WAIT socket. We need to do:

	sk = sk_fullsock(subflow->tcp_sock);

and checking for sk being != NULL before accessing any tcp_sock or sock
fields.

Note that the caller don't need such thing explicitly, because it
accesses such fields only for mptcp_subflow_active() sockets - which
are NOT TIME_WAIT ones.

> +		__entry->snd_wnd = tcp_sk(ssk)->snd_wnd;
> +		__entry->pace = ssk->sk_pacing_rate;
> +		__entry->backup = subflow->backup;
> +		__entry->ratio = div_u64((u64)ssk->sk_wmem_queued << 32, __entry->pace);

Here we need to check for !__entry->pace, to avoid possible division by
0.

Thanks!

Paolo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [MPTCP] [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active
  2021-03-24  8:51 ` [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Geliang Tang
  2021-03-24  8:51   ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Geliang Tang
@ 2021-03-29 13:52   ` Paolo Abeni
  1 sibling, 0 replies; 10+ messages in thread
From: Paolo Abeni @ 2021-03-29 13:52 UTC (permalink / raw)
  To: Geliang Tang, mptcp, mptcp

On Wed, 2021-03-24 at 16:51 +0800, Geliang Tang wrote:
> This patch exported the static function mptcp_subflow_active.
> 
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>  net/mptcp/protocol.c | 2 +-
>  net/mptcp/protocol.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 9d7e7e13fba8..3ad236689480 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -399,7 +399,7 @@ static void mptcp_set_timeout(const struct sock *sk, const struct sock *ssk)
>  	mptcp_sk(sk)->timer_ival = tout > 0 ? tout : TCP_RTO_MIN;
>  }
>  
> -static bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
> +bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
>  {
>  	struct sock *ssk = mptcp_subflow_tcp_sock(subflow);

possibly is better to simply move the function definition to the
protocol.h header, as 'static inline' one.

Thanks!

Paolo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status
  2021-03-24  8:51     ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Geliang Tang
  2021-03-24  8:51       ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Geliang Tang
@ 2021-03-29 14:00       ` Paolo Abeni
  1 sibling, 0 replies; 10+ messages in thread
From: Paolo Abeni @ 2021-03-29 14:00 UTC (permalink / raw)
  To: Geliang Tang, mptcp, mptcp

On Wed, 2021-03-24 at 16:51 +0800, Geliang Tang wrote:
> This patch added a tracepoint in the mapping status function
> get_mapping_status() to dump every mpext field.
> 
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>  include/trace/events/mptcp.h | 54 ++++++++++++++++++++++++++++++++++++
>  net/mptcp/subflow.c          |  6 ++--
>  2 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
> index 06908ea6afb4..503a6098909d 100644
> --- a/include/trace/events/mptcp.h
> +++ b/include/trace/events/mptcp.h
> @@ -37,6 +37,60 @@ TRACE_EVENT(mptcp_subflow_get_send,
>  		  __entry->backup, __entry->ratio)
>  );
>  
> +DECLARE_EVENT_CLASS(mptcp_dump_mpext,
> +
> +	TP_PROTO(struct mptcp_ext *mpext),
> +
> +	TP_ARGS(mpext),
> +
> +	TP_STRUCT__entry(
> +		__field(u64, data_ack)
> +		__field(u32, data_ack32)
> +		__field(u64, data_seq)
> +		__field(u32, subflow_seq)
> +		__field(u16, data_len)
> +		__field(u8, use_map)
> +		__field(u8, dsn64)
> +		__field(u8, data_fin)
> +		__field(u8, use_ack)
> +		__field(u8, ack64)
> +		__field(u8, mpc_map)
> +		__field(u8, frozen)
> +		__field(u8, reset_transient)
> +		__field(u8, reset_reason)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->data_ack = mpext->data_ack;
> +		__entry->data_ack32 = mpext->data_ack32;

Here it's probaly better dump only one of __entry->data_ack and 
and mpext->data_ack32:

		__entry->data_ack = mpext->ack64 ? mpext->data_ack : mpext->data_ack32;

Thanks!

Paolo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk
  2021-03-24  8:51       ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Geliang Tang
  2021-03-24  8:51         ` [MPTCP][PATCH v2 mptcp-next 5/5] mptcp: add tracepoint in subflow_check_data_avail Geliang Tang
@ 2021-03-29 14:03         ` Paolo Abeni
  1 sibling, 0 replies; 10+ messages in thread
From: Paolo Abeni @ 2021-03-29 14:03 UTC (permalink / raw)
  To: Geliang Tang, mptcp, mptcp

On Wed, 2021-03-24 at 16:51 +0800, Geliang Tang wrote:
> This patch added a tracepoint in ack_update_msk() to track the
> incoming data_ack and window/snd_una updates.
> 
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>  include/trace/events/mptcp.h | 25 +++++++++++++++++++++++++
>  net/mptcp/options.c          |  3 +++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/trace/events/mptcp.h b/include/trace/events/mptcp.h
> index 503a6098909d..2b8ae5e81cd1 100644
> --- a/include/trace/events/mptcp.h
> +++ b/include/trace/events/mptcp.h
> @@ -91,6 +91,31 @@ DEFINE_EVENT(mptcp_dump_mpext, get_mapping_status,
>  	TP_PROTO(struct mptcp_ext *mpext),
>  	TP_ARGS(mpext));
>  
> +TRACE_EVENT(ack_update_msk,
> +
> +	TP_PROTO(u64 data_ack, u64 old_snd_una, u64 new_snd_una, u64 new_wnd_end),
> +
> +	TP_ARGS(data_ack, old_snd_una, new_snd_una, new_wnd_end),
> +
> +	TP_STRUCT__entry(
> +		__field(u64, data_ack)
> +		__field(u64, old_snd_una)
> +		__field(u64, new_snd_una)
> +		__field(u64, new_wnd_end)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->data_ack = data_ack;
> +		__entry->old_snd_una = old_snd_una;
> +		__entry->new_snd_una = new_snd_una;
> +		__entry->new_wnd_end = new_wnd_end;
> +	),
> +
> +	TP_printk("data_ack=%llu old_snd_una=%llu new_snd_una=%llu new_wnd_end=%llu",
> +		  __entry->data_ack, __entry->old_snd_una,
> +		  __entry->new_snd_una, __entry->new_wnd_end)
> +);
> +
>  #endif /* _TRACE_MPTCP_H */
>  
>  /* This part must be outside protection */
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index c7eb61d0564c..87bd1bfde964 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -13,6 +13,8 @@
>  #include "protocol.h"
>  #include "mib.h"
>  
> +#include <trace/events/mptcp.h>
> +
>  static bool mptcp_cap_flag_sha256(u8 flags)
>  {
>  	return (flags & MPTCP_CAP_FLAG_MASK) == MPTCP_CAP_HMAC_SHA256;
> @@ -942,6 +944,7 @@ static void ack_update_msk(struct mptcp_sock *msk,
>  		__mptcp_data_acked(sk);
>  	}
>  	mptcp_data_unlock(sk);
> +	trace_ack_update_msk(mp_opt->data_ack, old_snd_una, new_snd_una, new_wnd_end);

Here it would be nice add also 'msk->wnd_end' as an argument and dump
it in the trace event, so that trace point could tell us always the new
windows end value.

Thanks!

Paolo


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-03-29 14:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24  8:51 [MPTCP][PATCH v2 mptcp-next 0/5] add tracepoints Geliang Tang
2021-03-24  8:51 ` [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Geliang Tang
2021-03-24  8:51   ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Geliang Tang
2021-03-24  8:51     ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Geliang Tang
2021-03-24  8:51       ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Geliang Tang
2021-03-24  8:51         ` [MPTCP][PATCH v2 mptcp-next 5/5] mptcp: add tracepoint in subflow_check_data_avail Geliang Tang
2021-03-29 14:03         ` [MPTCP][PATCH v2 mptcp-next 4/5] mptcp: add tracepoint in ack_update_msk Paolo Abeni
2021-03-29 14:00       ` [MPTCP][PATCH v2 mptcp-next 3/5] mptcp: add tracepoint in get_mapping_status Paolo Abeni
2021-03-29 13:52     ` [MPTCP][PATCH v2 mptcp-next 2/5] mptcp: add tracepoint in mptcp_subflow_get_send Paolo Abeni
2021-03-29 13:52   ` [MPTCP] [MPTCP][PATCH v2 mptcp-next 1/5] mptcp: export mptcp_subflow_active Paolo Abeni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.