netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
@ 2020-09-03  7:18 sundeep.lkml
  2020-09-03  7:18 ` [net-next PATCH 1/2] octeontx2-af: Introduce tracepoints for mailbox sundeep.lkml
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: sundeep.lkml @ 2020-09-03  7:18 UTC (permalink / raw)
  To: davem, kuba, netdev, sgoutham; +Cc: Subbaraya Sundeep

From: Subbaraya Sundeep <sbhatta@marvell.com>

This patchset adds tracepoints support for mailbox.
In Octeontx2, PFs and VFs need to communicate with AF
for allocating and freeing resources. Once all the
configuration is done by AF for a PF/VF then packet I/O
can happen on PF/VF queues. When an interface
is brought up many mailbox messages are sent
to AF for initializing queues. Say a VF is brought up
then each message is sent to PF and PF forwards to
AF and response also traverses from AF to PF and then VF.
To aid debugging, tracepoints are added at places where
messages are allocated, sent and message interrupts.
Below is the trace of one of the messages from VF to AF
and AF response back to VF:

~ # echo 1 > /sys/kernel/tracing/events/rvu/enable
~ # ifconfig eth20 up
[  279.379559] eth20 NIC Link is UP 10000 Mbps Full duplex
~ # cat /sys/kernel/tracing/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 880/880   #P:4
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
        ifconfig-171   [000] ....   275.753345: otx2_msg_alloc: [0002:02:00.1] msg:(0x400) size:40

        ifconfig-171   [000] ...1   275.753347: otx2_msg_send: [0002:02:00.1] sent 1 msg(s) of size:48

          <idle>-0     [001] dNh1   275.753356: otx2_msg_interrupt: [0002:02:00.0] mbox interrupt VF(s) to PF (0x1)

    kworker/u9:1-90    [001] ...1   275.753364: otx2_msg_send: [0002:02:00.0] sent 1 msg(s) of size:48

    kworker/u9:1-90    [001] d.h.   275.753367: otx2_msg_interrupt: [0002:01:00.0] mbox interrupt PF(s) to AF (0x2)

    kworker/u9:2-167   [002] ....   275.753535: otx2_msg_process: [0002:01:00.0] msg:(0x400) error:0

    kworker/u9:2-167   [002] ...1   275.753537: otx2_msg_send: [0002:01:00.0] sent 1 msg(s) of size:32

          <idle>-0     [003] d.h1   275.753543: otx2_msg_interrupt: [0002:02:00.0] mbox interrupt AF to PF (0x1)

          <idle>-0     [001] d.h2   275.754376: otx2_msg_interrupt: [0002:02:00.1] mbox interrupt PF to VF (0x1)


Subbaraya Sundeep (2):
  octeontx2-af: Introduce tracepoints for mailbox
  octeontx2-pf: Add tracepoints for PF/VF mailbox

 drivers/net/ethernet/marvell/octeontx2/af/Makefile |   3 +-
 drivers/net/ethernet/marvell/octeontx2/af/mbox.c   |  14 ++-
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c    |   7 ++
 .../net/ethernet/marvell/octeontx2/af/rvu_cgx.c    |   2 +
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.c  |  15 +++
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.h  | 115 +++++++++++++++++++++
 .../ethernet/marvell/octeontx2/nic/otx2_common.h   |   2 +
 .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c   |   6 ++
 .../net/ethernet/marvell/octeontx2/nic/otx2_vf.c   |   2 +
 9 files changed, 162 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.c
 create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.h

-- 
2.7.4


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

* [net-next PATCH 1/2] octeontx2-af: Introduce tracepoints for mailbox
  2020-09-03  7:18 [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 sundeep.lkml
@ 2020-09-03  7:18 ` sundeep.lkml
  2020-09-03  7:18 ` [net-next PATCH 2/2] octeontx2-pf: Add tracepoints for PF/VF mailbox sundeep.lkml
  2020-09-03 19:18 ` [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 Jakub Kicinski
  2 siblings, 0 replies; 19+ messages in thread
From: sundeep.lkml @ 2020-09-03  7:18 UTC (permalink / raw)
  To: davem, kuba, netdev, sgoutham; +Cc: Subbaraya Sundeep

From: Subbaraya Sundeep <sbhatta@marvell.com>

Added tracepoints in mailbox code so that
the mailbox operations like message allocation,
sending message and message interrupts are traced.
Also the mailbox errors occurred like timeout
or wrong responses are traced.
These will help in debugging mailbox issues.

Here's an example output showing one of the mailbox
messages sent by PF to AF and AF responding to it:

~# mount -t tracefs none /sys/kernel/tracing/
~# echo 1 > /sys/kernel/tracing/events/rvu/enable
~# ifconfig eth0 up
~# cat /sys/kernel/tracing/trace

~# cat /sys/kernel/tracing/trace
 tracer: nop

		      _-----=> irqs-off
		     / _----=> need-resched
		    | / _---=> hardirq/softirq
		    || / _--=> preempt-depth
		    ||| /     delay
   TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
      | |       |   ||||       |         |
ifconfig-2382  [002] ....   756.161892: otx2_msg_alloc: [0002:02:00.0] msg:(0x400) size:40

ifconfig-2382  [002] ...1   756.161895: otx2_msg_send: [0002:02:00.0] sent 1 msg(s) of size:48

 <idle>-0     [000] d.h1   756.161902: otx2_msg_interrupt: [0002:01:00.0] mbox interrupt PF(s) to AF (0x2)

kworker/u49:0-1165  [000] ....   756.162049: otx2_msg_process: [0002:01:00.0] msg:(0x400) error:0

kworker/u49:0-1165  [000] ...1   756.162051: otx2_msg_send: [0002:01:00.0] sent 1 msg(s) of size:32

kworker/u49:0-1165  [000] d.h.   756.162056: otx2_msg_interrupt: [0002:02:00.0] mbox interrupt AF to PF (0x1)

Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/Makefile |   3 +-
 drivers/net/ethernet/marvell/octeontx2/af/mbox.c   |  14 ++-
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c    |   7 ++
 .../net/ethernet/marvell/octeontx2/af/rvu_cgx.c    |   2 +
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.c  |  15 +++
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.h  | 115 +++++++++++++++++++++
 6 files changed, 152 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.c
 create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.h

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/Makefile b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
index 0bc2410..2f7a861 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/Makefile
+++ b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
@@ -3,9 +3,10 @@
 # Makefile for Marvell's OcteonTX2 RVU Admin Function driver
 #
 
+ccflags-y += -I$(src)
 obj-$(CONFIG_OCTEONTX2_MBOX) += octeontx2_mbox.o
 obj-$(CONFIG_OCTEONTX2_AF) += octeontx2_af.o
 
-octeontx2_mbox-y := mbox.o
+octeontx2_mbox-y := mbox.o rvu_trace.o
 octeontx2_af-y := cgx.o rvu.o rvu_cgx.o rvu_npa.o rvu_nix.o \
 		  rvu_reg.o rvu_npc.o rvu_debugfs.o ptp.o
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
index 387e33f..3e89060 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.c
@@ -14,6 +14,7 @@
 
 #include "rvu_reg.h"
 #include "mbox.h"
+#include "rvu_trace.h"
 
 static const u16 msgs_offset = ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
 
@@ -138,14 +139,13 @@ int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid)
 {
 	unsigned long timeout = jiffies + msecs_to_jiffies(MBOX_RSP_TIMEOUT);
 	struct otx2_mbox_dev *mdev = &mbox->dev[devid];
-	struct device *sender = &mbox->pdev->dev;
 
 	while (!time_after(jiffies, timeout)) {
 		if (mdev->num_msgs == mdev->msgs_acked)
 			return 0;
 		usleep_range(800, 1000);
 	}
-	dev_dbg(sender, "timed out while waiting for rsp\n");
+	trace_otx2_msg_err(mbox->pdev, "response timed out");
 	return -EIO;
 }
 EXPORT_SYMBOL(otx2_mbox_wait_for_rsp);
@@ -199,6 +199,9 @@ void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid)
 	 */
 	tx_hdr->num_msgs = mdev->num_msgs;
 	rx_hdr->num_msgs = 0;
+
+	trace_otx2_msg_send(mbox->pdev, tx_hdr->num_msgs, tx_hdr->msg_size);
+
 	spin_unlock(&mdev->mbox_lock);
 
 	/* The interrupt should be fired after num_msgs is written
@@ -295,10 +298,15 @@ int otx2_mbox_check_rsp_msgs(struct otx2_mbox *mbox, int devid)
 		struct mbox_msghdr *preq = mdev->mbase + ireq;
 		struct mbox_msghdr *prsp = mdev->mbase + irsp;
 
-		if (preq->id != prsp->id)
+		if (preq->id != prsp->id) {
+			trace_otx2_msg_check(mbox->pdev, preq->id,
+					     prsp->id, prsp->rc);
 			goto exit;
+		}
 		if (prsp->rc) {
 			rc = prsp->rc;
+			trace_otx2_msg_check(mbox->pdev, preq->id,
+					     prsp->id, prsp->rc);
 			goto exit;
 		}
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index c3ef73a..e1f9189 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -20,6 +20,8 @@
 #include "rvu_reg.h"
 #include "ptp.h"
 
+#include "rvu_trace.h"
+
 #define DRV_NAME	"octeontx2-af"
 #define DRV_STRING      "Marvell OcteonTX2 RVU Admin Function Driver"
 
@@ -1549,6 +1551,7 @@ static int rvu_process_mbox_msg(struct otx2_mbox *mbox, int devid,
 		if (rsp && err)						\
 			rsp->hdr.rc = err;				\
 									\
+		trace_otx2_msg_process(mbox->pdev, _id, err);		\
 		return rsp ? err : -ENOMEM;				\
 	}
 MBOX_MESSAGES
@@ -1881,6 +1884,8 @@ static irqreturn_t rvu_mbox_intr_handler(int irq, void *rvu_irq)
 	intr = rvu_read64(rvu, BLKADDR_RVUM, RVU_AF_PFAF_MBOX_INT);
 	/* Clear interrupts */
 	rvu_write64(rvu, BLKADDR_RVUM, RVU_AF_PFAF_MBOX_INT, intr);
+	if (intr)
+		trace_otx2_msg_interrupt(rvu->pdev, "PF(s) to AF", intr);
 
 	/* Sync with mbox memory region */
 	rmb();
@@ -1898,6 +1903,8 @@ static irqreturn_t rvu_mbox_intr_handler(int irq, void *rvu_irq)
 
 	intr = rvupf_read64(rvu, RVU_PF_VFPF_MBOX_INTX(0));
 	rvupf_write64(rvu, RVU_PF_VFPF_MBOX_INTX(0), intr);
+	if (intr)
+		trace_otx2_msg_interrupt(rvu->pdev, "VF(s) to AF", intr);
 
 	rvu_queue_work(&rvu->afvf_wq_info, 0, vfs, intr);
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index fe3389c..fa9152f 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -15,6 +15,7 @@
 #include "rvu.h"
 #include "cgx.h"
 #include "rvu_reg.h"
+#include "rvu_trace.h"
 
 struct cgx_evq_entry {
 	struct list_head evq_node;
@@ -34,6 +35,7 @@ static struct _req_type __maybe_unused					\
 		return NULL;						\
 	req->hdr.sig = OTX2_MBOX_REQ_SIG;				\
 	req->hdr.id = _id;						\
+	trace_otx2_msg_alloc(rvu->pdev, _id, sizeof(*req));		\
 	return req;							\
 }
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.c
new file mode 100644
index 0000000..b545c05
--- /dev/null
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Marvell OcteonTx2 RVU Admin Function driver tracepoints
+ *
+ * Copyright (C) 2020 Marvell International Ltd.
+ */
+
+#define CREATE_TRACE_POINTS
+#include "rvu_trace.h"
+
+EXPORT_TRACEPOINT_SYMBOL(otx2_msg_alloc);
+EXPORT_TRACEPOINT_SYMBOL(otx2_msg_send);
+EXPORT_TRACEPOINT_SYMBOL(otx2_msg_check);
+EXPORT_TRACEPOINT_SYMBOL(otx2_msg_interrupt);
+EXPORT_TRACEPOINT_SYMBOL(otx2_msg_process);
+EXPORT_TRACEPOINT_SYMBOL(otx2_msg_err);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.h
new file mode 100644
index 0000000..4526659
--- /dev/null
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_trace.h
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Marvell OcteonTx2 RVU Admin Function driver tracepoints
+ *
+ * Copyright (C) 2020 Marvell International Ltd.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM rvu
+
+#if !defined(__RVU_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define __RVU_TRACE_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+#include <linux/pci.h>
+
+TRACE_EVENT(otx2_msg_alloc,
+	    TP_PROTO(const struct pci_dev *pdev, u16 id, u64 size),
+	    TP_ARGS(pdev, id, size),
+	    TP_STRUCT__entry(__string(dev, pci_name(pdev))
+			     __field(u16, id)
+			     __field(u64, size)
+	    ),
+	    TP_fast_assign(__assign_str(dev, pci_name(pdev))
+			   __entry->id = id;
+			   __entry->size = size;
+	    ),
+	    TP_printk("[%s] msg:(0x%x) size:%lld\n", __get_str(dev),
+		      __entry->id, __entry->size)
+);
+
+TRACE_EVENT(otx2_msg_send,
+	    TP_PROTO(const struct pci_dev *pdev, u16 num_msgs, u64 msg_size),
+	    TP_ARGS(pdev, num_msgs, msg_size),
+	    TP_STRUCT__entry(__string(dev, pci_name(pdev))
+			     __field(u16, num_msgs)
+			     __field(u64, msg_size)
+	    ),
+	    TP_fast_assign(__assign_str(dev, pci_name(pdev))
+			   __entry->num_msgs = num_msgs;
+			   __entry->msg_size = msg_size;
+	    ),
+	    TP_printk("[%s] sent %d msg(s) of size:%lld\n", __get_str(dev),
+		      __entry->num_msgs, __entry->msg_size)
+);
+
+TRACE_EVENT(otx2_msg_check,
+	    TP_PROTO(const struct pci_dev *pdev, u16 reqid, u16 rspid, int rc),
+	    TP_ARGS(pdev, reqid, rspid, rc),
+	    TP_STRUCT__entry(__string(dev, pci_name(pdev))
+			     __field(u16, reqid)
+			     __field(u16, rspid)
+			     __field(int, rc)
+	    ),
+	    TP_fast_assign(__assign_str(dev, pci_name(pdev))
+			   __entry->reqid = reqid;
+			   __entry->rspid = rspid;
+			   __entry->rc = rc;
+	    ),
+	    TP_printk("[%s] req->id:0x%x rsp->id:0x%x resp_code:%d\n",
+		      __get_str(dev), __entry->reqid,
+		      __entry->rspid, __entry->rc)
+);
+
+TRACE_EVENT(otx2_msg_interrupt,
+	    TP_PROTO(const struct pci_dev *pdev, const char *msg, u64 intr),
+	    TP_ARGS(pdev, msg, intr),
+	    TP_STRUCT__entry(__string(dev, pci_name(pdev))
+			     __string(str, msg)
+			     __field(u64, intr)
+	    ),
+	    TP_fast_assign(__assign_str(dev, pci_name(pdev))
+			   __assign_str(str, msg)
+			   __entry->intr = intr;
+	    ),
+	    TP_printk("[%s] mbox interrupt %s (0x%llx)\n", __get_str(dev),
+		      __get_str(str), __entry->intr)
+);
+
+TRACE_EVENT(otx2_msg_process,
+	    TP_PROTO(const struct pci_dev *pdev, u16 id, int err),
+	    TP_ARGS(pdev, id, err),
+	    TP_STRUCT__entry(__string(dev, pci_name(pdev))
+			     __field(u16, id)
+			     __field(int, err)
+	    ),
+	    TP_fast_assign(__assign_str(dev, pci_name(pdev))
+			   __entry->id = id;
+			   __entry->err = err;
+	    ),
+	    TP_printk("[%s] msg:(0x%x) error:%d\n", __get_str(dev),
+		      __entry->id, __entry->err)
+);
+
+TRACE_EVENT(otx2_msg_err,
+	    TP_PROTO(const struct pci_dev *pdev, const char *err_msg),
+	    TP_ARGS(pdev, err_msg),
+	    TP_STRUCT__entry(__string(dev, pci_name(pdev))
+			     __string(str, err_msg)
+	    ),
+	    TP_fast_assign(__assign_str(dev, pci_name(pdev));
+			   __assign_str(str, err_msg);
+	    ),
+	    TP_printk("[%s] %s\n", __get_str(dev), __get_str(str))
+);
+
+#endif /* __RVU_TRACE_H */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE rvu_trace
+
+#include <trace/define_trace.h>
-- 
2.7.4


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

* [net-next PATCH 2/2] octeontx2-pf: Add tracepoints for PF/VF mailbox
  2020-09-03  7:18 [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 sundeep.lkml
  2020-09-03  7:18 ` [net-next PATCH 1/2] octeontx2-af: Introduce tracepoints for mailbox sundeep.lkml
@ 2020-09-03  7:18 ` sundeep.lkml
  2020-09-03 19:18 ` [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 Jakub Kicinski
  2 siblings, 0 replies; 19+ messages in thread
From: sundeep.lkml @ 2020-09-03  7:18 UTC (permalink / raw)
  To: davem, kuba, netdev, sgoutham; +Cc: Subbaraya Sundeep

From: Subbaraya Sundeep <sbhatta@marvell.com>

With tracepoints support present in the mailbox
code this patch adds tracepoints in PF and VF drivers
at places where mailbox messages are allocated,
sent and at message interrupts.

Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 2 ++
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c     | 6 ++++++
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c     | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index ac47762..d6253f2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -20,6 +20,7 @@
 #include <mbox.h>
 #include "otx2_reg.h"
 #include "otx2_txrx.h"
+#include <rvu_trace.h>
 
 /* PCI device IDs */
 #define PCI_DEVID_OCTEONTX2_RVU_PF              0xA063
@@ -523,6 +524,7 @@ static struct _req_type __maybe_unused					\
 		return NULL;						\
 	req->hdr.sig = OTX2_MBOX_REQ_SIG;				\
 	req->hdr.id = _id;						\
+	trace_otx2_msg_alloc(mbox->mbox.pdev, _id, sizeof(*req));	\
 	return req;							\
 }
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index aac2845..265e4d1 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -22,6 +22,7 @@
 #include "otx2_txrx.h"
 #include "otx2_struct.h"
 #include "otx2_ptp.h"
+#include <rvu_trace.h>
 
 #define DRV_NAME	"octeontx2-nicpf"
 #define DRV_STRING	"Marvell OcteonTX2 NIC Physical Function Driver"
@@ -558,6 +559,8 @@ static irqreturn_t otx2_pfvf_mbox_intr_handler(int irq, void *pf_irq)
 
 	otx2_queue_work(mbox, pf->mbox_pfvf_wq, 0, vfs, intr, TYPE_PFVF);
 
+	trace_otx2_msg_interrupt(mbox->mbox.pdev, "VF(s) to PF", intr);
+
 	return IRQ_HANDLED;
 }
 
@@ -940,6 +943,9 @@ static irqreturn_t otx2_pfaf_mbox_intr_handler(int irq, void *pf_irq)
 	otx2_write64(pf, RVU_PF_INT, BIT_ULL(0));
 
 	mbox = &pf->mbox;
+
+	trace_otx2_msg_interrupt(mbox->mbox.pdev, "AF to PF", BIT_ULL(0));
+
 	otx2_queue_work(mbox, pf->mbox_wq, 0, 1, 1, TYPE_PFAF);
 
 	return IRQ_HANDLED;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index 70e0d4c..32daa3e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -187,6 +187,8 @@ static irqreturn_t otx2vf_vfaf_mbox_intr_handler(int irq, void *vf_irq)
 	mdev = &mbox->dev[0];
 	otx2_sync_mbox_bbuf(mbox, 0);
 
+	trace_otx2_msg_interrupt(mbox->pdev, "PF to VF", BIT_ULL(0));
+
 	hdr = (struct mbox_hdr *)(mdev->mbase + mbox->rx_start);
 	if (hdr->num_msgs) {
 		vf->mbox.num_msgs = hdr->num_msgs;
-- 
2.7.4


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

* Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-03  7:18 [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 sundeep.lkml
  2020-09-03  7:18 ` [net-next PATCH 1/2] octeontx2-af: Introduce tracepoints for mailbox sundeep.lkml
  2020-09-03  7:18 ` [net-next PATCH 2/2] octeontx2-pf: Add tracepoints for PF/VF mailbox sundeep.lkml
@ 2020-09-03 19:18 ` Jakub Kicinski
  2020-09-04  5:39   ` [EXT] " Sunil Kovvuri Goutham
  2 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2020-09-03 19:18 UTC (permalink / raw)
  To: sundeep.lkml; +Cc: davem, netdev, sgoutham, Subbaraya Sundeep

On Thu,  3 Sep 2020 12:48:16 +0530 sundeep.lkml@gmail.com wrote:
> From: Subbaraya Sundeep <sbhatta@marvell.com>
> 
> This patchset adds tracepoints support for mailbox.
> In Octeontx2, PFs and VFs need to communicate with AF
> for allocating and freeing resources. Once all the
> configuration is done by AF for a PF/VF then packet I/O
> can happen on PF/VF queues. When an interface
> is brought up many mailbox messages are sent
> to AF for initializing queues. Say a VF is brought up
> then each message is sent to PF and PF forwards to
> AF and response also traverses from AF to PF and then VF.
> To aid debugging, tracepoints are added at places where
> messages are allocated, sent and message interrupts.
> Below is the trace of one of the messages from VF to AF
> and AF response back to VF:

Could you use the devlink tracepoint? trace_devlink_hwmsg() ?

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

* RE: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-03 19:18 ` [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 Jakub Kicinski
@ 2020-09-04  5:39   ` Sunil Kovvuri Goutham
  2020-09-04  8:37     ` Jiri Pirko
  0 siblings, 1 reply; 19+ messages in thread
From: Sunil Kovvuri Goutham @ 2020-09-04  5:39 UTC (permalink / raw)
  To: Jakub Kicinski, sundeep.lkml; +Cc: davem, netdev, Subbaraya Sundeep Bhatta



> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, September 4, 2020 12:48 AM
> To: sundeep.lkml@gmail.com
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
> <sbhatta@marvell.com>
> Subject: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for
> Octeontx2
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu,  3 Sep 2020 12:48:16 +0530 sundeep.lkml@gmail.com wrote:
> > From: Subbaraya Sundeep <sbhatta@marvell.com>
> >
> > This patchset adds tracepoints support for mailbox.
> > In Octeontx2, PFs and VFs need to communicate with AF for allocating
> > and freeing resources. Once all the configuration is done by AF for a
> > PF/VF then packet I/O can happen on PF/VF queues. When an interface is
> > brought up many mailbox messages are sent to AF for initializing
> > queues. Say a VF is brought up then each message is sent to PF and PF
> > forwards to AF and response also traverses from AF to PF and then VF.
> > To aid debugging, tracepoints are added at places where messages are
> > allocated, sent and message interrupts.
> > Below is the trace of one of the messages from VF to AF and AF
> > response back to VF:
> 
> Could you use the devlink tracepoint? trace_devlink_hwmsg() ?

Thanks for the suggestion.
In our case the mailbox is central to 3 different drivers and there would be a 4th one
once crypto driver is accepted. We cannot add devlink to all of them inorder to use
the devlink trace points.

Thanks,
Sunil.

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-04  5:39   ` [EXT] " Sunil Kovvuri Goutham
@ 2020-09-04  8:37     ` Jiri Pirko
  2020-09-04  8:49       ` Sunil Kovvuri Goutham
  0 siblings, 1 reply; 19+ messages in thread
From: Jiri Pirko @ 2020-09-04  8:37 UTC (permalink / raw)
  To: Sunil Kovvuri Goutham
  Cc: Jakub Kicinski, sundeep.lkml, davem, netdev, Subbaraya Sundeep Bhatta

Fri, Sep 04, 2020 at 07:39:54AM CEST, sgoutham@marvell.com wrote:
>
>
>> -----Original Message-----
>> From: Jakub Kicinski <kuba@kernel.org>
>> Sent: Friday, September 4, 2020 12:48 AM
>> To: sundeep.lkml@gmail.com
>> Cc: davem@davemloft.net; netdev@vger.kernel.org; Sunil Kovvuri Goutham
>> <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
>> <sbhatta@marvell.com>
>> Subject: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for
>> Octeontx2
>> 
>> External Email
>> 
>> ----------------------------------------------------------------------
>> On Thu,  3 Sep 2020 12:48:16 +0530 sundeep.lkml@gmail.com wrote:
>> > From: Subbaraya Sundeep <sbhatta@marvell.com>
>> >
>> > This patchset adds tracepoints support for mailbox.
>> > In Octeontx2, PFs and VFs need to communicate with AF for allocating
>> > and freeing resources. Once all the configuration is done by AF for a
>> > PF/VF then packet I/O can happen on PF/VF queues. When an interface is
>> > brought up many mailbox messages are sent to AF for initializing
>> > queues. Say a VF is brought up then each message is sent to PF and PF
>> > forwards to AF and response also traverses from AF to PF and then VF.
>> > To aid debugging, tracepoints are added at places where messages are
>> > allocated, sent and message interrupts.
>> > Below is the trace of one of the messages from VF to AF and AF
>> > response back to VF:
>> 
>> Could you use the devlink tracepoint? trace_devlink_hwmsg() ?
>
>Thanks for the suggestion.
>In our case the mailbox is central to 3 different drivers and there would be a 4th one
>once crypto driver is accepted. We cannot add devlink to all of them inorder to use
>the devlink trace points.

I guess you have 1 pci device, right? Devlink instance is created per
pci device.


>
>Thanks,
>Sunil.

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

* RE: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-04  8:37     ` Jiri Pirko
@ 2020-09-04  8:49       ` Sunil Kovvuri Goutham
  2020-09-04 12:11         ` Jiri Pirko
  0 siblings, 1 reply; 19+ messages in thread
From: Sunil Kovvuri Goutham @ 2020-09-04  8:49 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Jakub Kicinski, sundeep.lkml, davem, netdev, Subbaraya Sundeep Bhatta



> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Friday, September 4, 2020 2:07 PM
> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Cc: Jakub Kicinski <kuba@kernel.org>; sundeep.lkml@gmail.com;
> davem@davemloft.net; netdev@vger.kernel.org; Subbaraya Sundeep
> Bhatta <sbhatta@marvell.com>
> Subject: Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for
> Octeontx2
> 
> Fri, Sep 04, 2020 at 07:39:54AM CEST, sgoutham@marvell.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jakub Kicinski <kuba@kernel.org>
> >> Sent: Friday, September 4, 2020 12:48 AM
> >> To: sundeep.lkml@gmail.com
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Sunil Kovvuri
> >> Goutham <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
> >> <sbhatta@marvell.com>
> >> Subject: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints
> >> for
> >> Octeontx2
> >>
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> - On Thu,  3 Sep 2020 12:48:16 +0530 sundeep.lkml@gmail.com wrote:
> >> > From: Subbaraya Sundeep <sbhatta@marvell.com>
> >> >
> >> > This patchset adds tracepoints support for mailbox.
> >> > In Octeontx2, PFs and VFs need to communicate with AF for
> >> > allocating and freeing resources. Once all the configuration is
> >> > done by AF for a PF/VF then packet I/O can happen on PF/VF queues.
> >> > When an interface is brought up many mailbox messages are sent to
> >> > AF for initializing queues. Say a VF is brought up then each
> >> > message is sent to PF and PF forwards to AF and response also traverses
> from AF to PF and then VF.
> >> > To aid debugging, tracepoints are added at places where messages
> >> > are allocated, sent and message interrupts.
> >> > Below is the trace of one of the messages from VF to AF and AF
> >> > response back to VF:
> >>
> >> Could you use the devlink tracepoint? trace_devlink_hwmsg() ?
> >
> >Thanks for the suggestion.
> >In our case the mailbox is central to 3 different drivers and there
> >would be a 4th one once crypto driver is accepted. We cannot add
> >devlink to all of them inorder to use the devlink trace points.
> 
> I guess you have 1 pci device, right? Devlink instance is created per pci
> device.
> 

No, there are 3 drivers registering to 3 PCI device IDs and there can be many
instances of the same devices. So there can be 10's of instances of AF, PF and VFs.

Thanks,
Sunil.

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-04  8:49       ` Sunil Kovvuri Goutham
@ 2020-09-04 12:11         ` Jiri Pirko
  2020-09-04 12:29           ` Sunil Kovvuri Goutham
  0 siblings, 1 reply; 19+ messages in thread
From: Jiri Pirko @ 2020-09-04 12:11 UTC (permalink / raw)
  To: Sunil Kovvuri Goutham
  Cc: Jakub Kicinski, sundeep.lkml, davem, netdev, Subbaraya Sundeep Bhatta

Fri, Sep 04, 2020 at 10:49:45AM CEST, sgoutham@marvell.com wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko <jiri@resnulli.us>
>> Sent: Friday, September 4, 2020 2:07 PM
>> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>; sundeep.lkml@gmail.com;
>> davem@davemloft.net; netdev@vger.kernel.org; Subbaraya Sundeep
>> Bhatta <sbhatta@marvell.com>
>> Subject: Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for
>> Octeontx2
>> 
>> Fri, Sep 04, 2020 at 07:39:54AM CEST, sgoutham@marvell.com wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Jakub Kicinski <kuba@kernel.org>
>> >> Sent: Friday, September 4, 2020 12:48 AM
>> >> To: sundeep.lkml@gmail.com
>> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Sunil Kovvuri
>> >> Goutham <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
>> >> <sbhatta@marvell.com>
>> >> Subject: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints
>> >> for
>> >> Octeontx2
>> >>
>> >> External Email
>> >>
>> >> ---------------------------------------------------------------------
>> >> - On Thu,  3 Sep 2020 12:48:16 +0530 sundeep.lkml@gmail.com wrote:
>> >> > From: Subbaraya Sundeep <sbhatta@marvell.com>
>> >> >
>> >> > This patchset adds tracepoints support for mailbox.
>> >> > In Octeontx2, PFs and VFs need to communicate with AF for
>> >> > allocating and freeing resources. Once all the configuration is
>> >> > done by AF for a PF/VF then packet I/O can happen on PF/VF queues.
>> >> > When an interface is brought up many mailbox messages are sent to
>> >> > AF for initializing queues. Say a VF is brought up then each
>> >> > message is sent to PF and PF forwards to AF and response also traverses
>> from AF to PF and then VF.
>> >> > To aid debugging, tracepoints are added at places where messages
>> >> > are allocated, sent and message interrupts.
>> >> > Below is the trace of one of the messages from VF to AF and AF
>> >> > response back to VF:
>> >>
>> >> Could you use the devlink tracepoint? trace_devlink_hwmsg() ?
>> >
>> >Thanks for the suggestion.
>> >In our case the mailbox is central to 3 different drivers and there
>> >would be a 4th one once crypto driver is accepted. We cannot add
>> >devlink to all of them inorder to use the devlink trace points.
>> 
>> I guess you have 1 pci device, right? Devlink instance is created per pci
>> device.
>> 
>
>No, there are 3 drivers registering to 3 PCI device IDs and there can be many
>instances of the same devices. So there can be 10's of instances of AF, PF and VFs.

So you can still have per-pci device devlink instance and use the
tracepoint Jakub suggested.


>
>Thanks,
>Sunil.

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

* RE: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-04 12:11         ` Jiri Pirko
@ 2020-09-04 12:29           ` Sunil Kovvuri Goutham
  2020-09-04 20:37             ` Jakub Kicinski
  0 siblings, 1 reply; 19+ messages in thread
From: Sunil Kovvuri Goutham @ 2020-09-04 12:29 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Jakub Kicinski, sundeep.lkml, davem, netdev, Subbaraya Sundeep Bhatta



> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Friday, September 4, 2020 5:41 PM
> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Cc: Jakub Kicinski <kuba@kernel.org>; sundeep.lkml@gmail.com;
> davem@davemloft.net; netdev@vger.kernel.org; Subbaraya Sundeep
> Bhatta <sbhatta@marvell.com>
> Subject: Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for
> Octeontx2
> 
> Fri, Sep 04, 2020 at 10:49:45AM CEST, sgoutham@marvell.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jiri Pirko <jiri@resnulli.us>
> >> Sent: Friday, September 4, 2020 2:07 PM
> >> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>
> >> Cc: Jakub Kicinski <kuba@kernel.org>; sundeep.lkml@gmail.com;
> >> davem@davemloft.net; netdev@vger.kernel.org; Subbaraya Sundeep
> Bhatta
> >> <sbhatta@marvell.com>
> >> Subject: Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox
> >> tracepoints for
> >> Octeontx2
> >>
> >> Fri, Sep 04, 2020 at 07:39:54AM CEST, sgoutham@marvell.com wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Jakub Kicinski <kuba@kernel.org>
> >> >> Sent: Friday, September 4, 2020 12:48 AM
> >> >> To: sundeep.lkml@gmail.com
> >> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Sunil Kovvuri
> >> >> Goutham <sgoutham@marvell.com>; Subbaraya Sundeep Bhatta
> >> >> <sbhatta@marvell.com>
> >> >> Subject: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints
> >> >> for
> >> >> Octeontx2
> >> >>
> >> >> External Email
> >> >>
> >> >> ------------------------------------------------------------------
> >> >> ---
> >> >> - On Thu,  3 Sep 2020 12:48:16 +0530 sundeep.lkml@gmail.com wrote:
> >> >> > From: Subbaraya Sundeep <sbhatta@marvell.com>
> >> >> >
> >> >> > This patchset adds tracepoints support for mailbox.
> >> >> > In Octeontx2, PFs and VFs need to communicate with AF for
> >> >> > allocating and freeing resources. Once all the configuration is
> >> >> > done by AF for a PF/VF then packet I/O can happen on PF/VF
> queues.
> >> >> > When an interface is brought up many mailbox messages are sent
> >> >> > to AF for initializing queues. Say a VF is brought up then each
> >> >> > message is sent to PF and PF forwards to AF and response also
> >> >> > traverses
> >> from AF to PF and then VF.
> >> >> > To aid debugging, tracepoints are added at places where messages
> >> >> > are allocated, sent and message interrupts.
> >> >> > Below is the trace of one of the messages from VF to AF and AF
> >> >> > response back to VF:
> >> >>
> >> >> Could you use the devlink tracepoint? trace_devlink_hwmsg() ?
> >> >
> >> >Thanks for the suggestion.
> >> >In our case the mailbox is central to 3 different drivers and there
> >> >would be a 4th one once crypto driver is accepted. We cannot add
> >> >devlink to all of them inorder to use the devlink trace points.
> >>
> >> I guess you have 1 pci device, right? Devlink instance is created per
> >> pci device.
> >>
> >
> >No, there are 3 drivers registering to 3 PCI device IDs and there can
> >be many instances of the same devices. So there can be 10's of instances of
> AF, PF and VFs.
> 
> So you can still have per-pci device devlink instance and use the tracepoint
> Jakub suggested.
> 

Two things
- As I mentioned above, there is a Crypto driver which uses the same mbox APIs
  which is in the process of upstreaming. There also we would need trace points.
  Not sure registering to devlink just for the sake of tracepoint is proper. 

- The devlink trace message is like this

   TRACE_EVENT(devlink_hwmsg,
     . . .
        TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
                  __get_str(bus_name), __get_str(dev_name),
                  __get_str(driver_name), __entry->incoming, __entry->type,
                  (int) __entry->len, __get_dynamic_array(buf), __entry->len)
   );

   Whatever debug message we want as output doesn't fit into this.

Thanks,
Sunil. 

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-04 12:29           ` Sunil Kovvuri Goutham
@ 2020-09-04 20:37             ` Jakub Kicinski
  2020-09-07 10:59               ` sundeep subbaraya
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2020-09-04 20:37 UTC (permalink / raw)
  To: Sunil Kovvuri Goutham
  Cc: Jiri Pirko, sundeep.lkml, davem, netdev, Subbaraya Sundeep Bhatta

On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
> > >No, there are 3 drivers registering to 3 PCI device IDs and there can
> > >be many instances of the same devices. So there can be 10's of instances of  
> > AF, PF and VFs.
> > 
> > So you can still have per-pci device devlink instance and use the tracepoint
> > Jakub suggested.
> >   
> 
> Two things
> - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
>   which is in the process of upstreaming. There also we would need trace points.
>   Not sure registering to devlink just for the sake of tracepoint is proper. 
> 
> - The devlink trace message is like this
> 
>    TRACE_EVENT(devlink_hwmsg,
>      . . .
>         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
>                   __get_str(bus_name), __get_str(dev_name),
>                   __get_str(driver_name), __entry->incoming, __entry->type,
>                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
>    );
> 
>    Whatever debug message we want as output doesn't fit into this.

Make use of the standard devlink tracepoint wherever applicable, and you
can keep your extra ones if you want (as long as Jiri don't object).

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-04 20:37             ` Jakub Kicinski
@ 2020-09-07 10:59               ` sundeep subbaraya
  2020-09-15 15:52                 ` sundeep subbaraya
  2020-09-16 10:34                 ` Jiri Pirko
  0 siblings, 2 replies; 19+ messages in thread
From: sundeep subbaraya @ 2020-09-07 10:59 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Sunil Kovvuri Goutham, Jiri Pirko, davem, netdev,
	Subbaraya Sundeep Bhatta

Hi Jakub,

On Sat, Sep 5, 2020 at 2:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
> > > >No, there are 3 drivers registering to 3 PCI device IDs and there can
> > > >be many instances of the same devices. So there can be 10's of instances of
> > > AF, PF and VFs.
> > >
> > > So you can still have per-pci device devlink instance and use the tracepoint
> > > Jakub suggested.
> > >
> >
> > Two things
> > - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
> >   which is in the process of upstreaming. There also we would need trace points.
> >   Not sure registering to devlink just for the sake of tracepoint is proper.
> >
> > - The devlink trace message is like this
> >
> >    TRACE_EVENT(devlink_hwmsg,
> >      . . .
> >         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
> >                   __get_str(bus_name), __get_str(dev_name),
> >                   __get_str(driver_name), __entry->incoming, __entry->type,
> >                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
> >    );
> >
> >    Whatever debug message we want as output doesn't fit into this.
>
> Make use of the standard devlink tracepoint wherever applicable, and you
> can keep your extra ones if you want (as long as Jiri don't object).

Sure and noted. I have tried to use devlink tracepoints and since it
could not fit our purpose I used these.

Thanks,
Sundeep

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-07 10:59               ` sundeep subbaraya
@ 2020-09-15 15:52                 ` sundeep subbaraya
  2020-09-15 16:12                   ` Jakub Kicinski
  2020-09-16 10:34                 ` Jiri Pirko
  1 sibling, 1 reply; 19+ messages in thread
From: sundeep subbaraya @ 2020-09-15 15:52 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Sunil Kovvuri Goutham, Jiri Pirko, davem, netdev,
	Subbaraya Sundeep Bhatta

Hi Jiri,

On Mon, Sep 7, 2020 at 4:29 PM sundeep subbaraya <sundeep.lkml@gmail.com> wrote:
>
> Hi Jakub,
>
> On Sat, Sep 5, 2020 at 2:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
> > > > >No, there are 3 drivers registering to 3 PCI device IDs and there can
> > > > >be many instances of the same devices. So there can be 10's of instances of
> > > > AF, PF and VFs.
> > > >
> > > > So you can still have per-pci device devlink instance and use the tracepoint
> > > > Jakub suggested.
> > > >
> > >
> > > Two things
> > > - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
> > >   which is in the process of upstreaming. There also we would need trace points.
> > >   Not sure registering to devlink just for the sake of tracepoint is proper.
> > >
> > > - The devlink trace message is like this
> > >
> > >    TRACE_EVENT(devlink_hwmsg,
> > >      . . .
> > >         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
> > >                   __get_str(bus_name), __get_str(dev_name),
> > >                   __get_str(driver_name), __entry->incoming, __entry->type,
> > >                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
> > >    );
> > >
> > >    Whatever debug message we want as output doesn't fit into this.
> >
> > Make use of the standard devlink tracepoint wherever applicable, and you
> > can keep your extra ones if you want (as long as Jiri don't object).
>
> Sure and noted. I have tried to use devlink tracepoints and since it
> could not fit our purpose I used these.
>
Can you please comment.

> Thanks,
> Sundeep

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-15 15:52                 ` sundeep subbaraya
@ 2020-09-15 16:12                   ` Jakub Kicinski
  2020-09-15 16:36                     ` sundeep subbaraya
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2020-09-15 16:12 UTC (permalink / raw)
  To: sundeep subbaraya
  Cc: Sunil Kovvuri Goutham, Jiri Pirko, davem, netdev,
	Subbaraya Sundeep Bhatta

On Tue, 15 Sep 2020 21:22:21 +0530 sundeep subbaraya wrote:
> > > Make use of the standard devlink tracepoint wherever applicable, and you
> > > can keep your extra ones if you want (as long as Jiri don't object).  
> >
> > Sure and noted. I have tried to use devlink tracepoints and since it
> > could not fit our purpose I used these.
>
> Can you please comment.

Comment on what? Restate what I already said? Add the standard
tracepoint, you can add extra ones where needed.

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-15 16:12                   ` Jakub Kicinski
@ 2020-09-15 16:36                     ` sundeep subbaraya
  2020-09-15 16:44                       ` Jakub Kicinski
  0 siblings, 1 reply; 19+ messages in thread
From: sundeep subbaraya @ 2020-09-15 16:36 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Sunil Kovvuri Goutham, Jiri Pirko, davem, netdev,
	Subbaraya Sundeep Bhatta

On Tue, Sep 15, 2020 at 9:42 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 15 Sep 2020 21:22:21 +0530 sundeep subbaraya wrote:
> > > > Make use of the standard devlink tracepoint wherever applicable, and you
> > > > can keep your extra ones if you want (as long as Jiri don't object).
> > >
> > > Sure and noted. I have tried to use devlink tracepoints and since it
> > > could not fit our purpose I used these.
> >
> > Can you please comment.
>
> Comment on what? Restate what I already said? Add the standard
> tracepoint, you can add extra ones where needed.

We did look at using the devlink tracepoint for our purpose and found
it not suitable for our current requirement.
As and when we want to add new tracepoints we will keep this in mind
to see if we can use the devlink one.

So was just checking if Jiri is okay with this.

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-15 16:36                     ` sundeep subbaraya
@ 2020-09-15 16:44                       ` Jakub Kicinski
  0 siblings, 0 replies; 19+ messages in thread
From: Jakub Kicinski @ 2020-09-15 16:44 UTC (permalink / raw)
  To: sundeep subbaraya
  Cc: Sunil Kovvuri Goutham, Jiri Pirko, davem, netdev,
	Subbaraya Sundeep Bhatta

On Tue, 15 Sep 2020 22:06:45 +0530 sundeep subbaraya wrote:
> On Tue, Sep 15, 2020 at 9:42 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Tue, 15 Sep 2020 21:22:21 +0530 sundeep subbaraya wrote:  
> > > > > Make use of the standard devlink tracepoint wherever applicable, and you
> > > > > can keep your extra ones if you want (as long as Jiri don't object).  
> > > >
> > > > Sure and noted. I have tried to use devlink tracepoints and since it
> > > > could not fit our purpose I used these.  
> > >
> > > Can you please comment.  
> >
> > Comment on what? Restate what I already said? Add the standard
> > tracepoint, you can add extra ones where needed.  
> 
> We did look at using the devlink tracepoint for our purpose and found
> it not suitable for our current requirement.
> As and when we want to add new tracepoints we will keep this in mind
> to see if we can use the devlink one.
> 
> So was just checking if Jiri is okay with this.

Please make sure you adjust the To: field of the email to the person
you're asking your question.

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-07 10:59               ` sundeep subbaraya
  2020-09-15 15:52                 ` sundeep subbaraya
@ 2020-09-16 10:34                 ` Jiri Pirko
  2020-09-16 17:19                   ` sundeep subbaraya
  1 sibling, 1 reply; 19+ messages in thread
From: Jiri Pirko @ 2020-09-16 10:34 UTC (permalink / raw)
  To: sundeep subbaraya
  Cc: Jakub Kicinski, Sunil Kovvuri Goutham, davem, netdev,
	Subbaraya Sundeep Bhatta

Mon, Sep 07, 2020 at 12:59:45PM CEST, sundeep.lkml@gmail.com wrote:
>Hi Jakub,
>
>On Sat, Sep 5, 2020 at 2:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
>>
>> On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
>> > > >No, there are 3 drivers registering to 3 PCI device IDs and there can
>> > > >be many instances of the same devices. So there can be 10's of instances of
>> > > AF, PF and VFs.
>> > >
>> > > So you can still have per-pci device devlink instance and use the tracepoint
>> > > Jakub suggested.
>> > >
>> >
>> > Two things
>> > - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
>> >   which is in the process of upstreaming. There also we would need trace points.
>> >   Not sure registering to devlink just for the sake of tracepoint is proper.
>> >
>> > - The devlink trace message is like this
>> >
>> >    TRACE_EVENT(devlink_hwmsg,
>> >      . . .
>> >         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
>> >                   __get_str(bus_name), __get_str(dev_name),
>> >                   __get_str(driver_name), __entry->incoming, __entry->type,
>> >                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
>> >    );
>> >
>> >    Whatever debug message we want as output doesn't fit into this.
>>
>> Make use of the standard devlink tracepoint wherever applicable, and you
>> can keep your extra ones if you want (as long as Jiri don't object).
>
>Sure and noted. I have tried to use devlink tracepoints and since it
>could not fit our purpose I used these.

Why exactly the existing TP didn't fit your need?

>
>Thanks,
>Sundeep

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-16 10:34                 ` Jiri Pirko
@ 2020-09-16 17:19                   ` sundeep subbaraya
  2020-09-17  6:04                     ` Jiri Pirko
  0 siblings, 1 reply; 19+ messages in thread
From: sundeep subbaraya @ 2020-09-16 17:19 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Jakub Kicinski, Sunil Kovvuri Goutham, davem, netdev,
	Subbaraya Sundeep Bhatta

On Wed, Sep 16, 2020 at 4:04 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Mon, Sep 07, 2020 at 12:59:45PM CEST, sundeep.lkml@gmail.com wrote:
> >Hi Jakub,
> >
> >On Sat, Sep 5, 2020 at 2:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >>
> >> On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
> >> > > >No, there are 3 drivers registering to 3 PCI device IDs and there can
> >> > > >be many instances of the same devices. So there can be 10's of instances of
> >> > > AF, PF and VFs.
> >> > >
> >> > > So you can still have per-pci device devlink instance and use the tracepoint
> >> > > Jakub suggested.
> >> > >
> >> >
> >> > Two things
> >> > - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
> >> >   which is in the process of upstreaming. There also we would need trace points.
> >> >   Not sure registering to devlink just for the sake of tracepoint is proper.
> >> >
> >> > - The devlink trace message is like this
> >> >
> >> >    TRACE_EVENT(devlink_hwmsg,
> >> >      . . .
> >> >         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
> >> >                   __get_str(bus_name), __get_str(dev_name),
> >> >                   __get_str(driver_name), __entry->incoming, __entry->type,
> >> >                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
> >> >    );
> >> >
> >> >    Whatever debug message we want as output doesn't fit into this.
> >>
> >> Make use of the standard devlink tracepoint wherever applicable, and you
> >> can keep your extra ones if you want (as long as Jiri don't object).
> >
> >Sure and noted. I have tried to use devlink tracepoints and since it
> >could not fit our purpose I used these.
>
> Why exactly the existing TP didn't fit your need?
>
Existing TP has provision to dump skb and trace error strings with
error code but
we are trying to trace the entire mailbox flow of the AF/PF and VF
drivers. In particular
we trace the below:
    message allocation with message id and size at initiator.
    number of messages sent and total size.
    check message requester id, response id and response code after
reply is received.
    interrupts happened on behalf of mailboxes in the entire process
with source and receiver of interrupt along with isr status.
    error like initiator timeout waiting for response.
  All the above are relevant and are required for Octeontx2 only hence
used own tracepoints.

Thanks,
Sundeep

> >
> >Thanks,
> >Sundeep

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-16 17:19                   ` sundeep subbaraya
@ 2020-09-17  6:04                     ` Jiri Pirko
  2020-09-17 10:30                       ` sundeep subbaraya
  0 siblings, 1 reply; 19+ messages in thread
From: Jiri Pirko @ 2020-09-17  6:04 UTC (permalink / raw)
  To: sundeep subbaraya
  Cc: Jakub Kicinski, Sunil Kovvuri Goutham, davem, netdev,
	Subbaraya Sundeep Bhatta

Wed, Sep 16, 2020 at 07:19:36PM CEST, sundeep.lkml@gmail.com wrote:
>On Wed, Sep 16, 2020 at 4:04 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Mon, Sep 07, 2020 at 12:59:45PM CEST, sundeep.lkml@gmail.com wrote:
>> >Hi Jakub,
>> >
>> >On Sat, Sep 5, 2020 at 2:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
>> >>
>> >> On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
>> >> > > >No, there are 3 drivers registering to 3 PCI device IDs and there can
>> >> > > >be many instances of the same devices. So there can be 10's of instances of
>> >> > > AF, PF and VFs.
>> >> > >
>> >> > > So you can still have per-pci device devlink instance and use the tracepoint
>> >> > > Jakub suggested.
>> >> > >
>> >> >
>> >> > Two things
>> >> > - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
>> >> >   which is in the process of upstreaming. There also we would need trace points.
>> >> >   Not sure registering to devlink just for the sake of tracepoint is proper.
>> >> >
>> >> > - The devlink trace message is like this
>> >> >
>> >> >    TRACE_EVENT(devlink_hwmsg,
>> >> >      . . .
>> >> >         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
>> >> >                   __get_str(bus_name), __get_str(dev_name),
>> >> >                   __get_str(driver_name), __entry->incoming, __entry->type,
>> >> >                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
>> >> >    );
>> >> >
>> >> >    Whatever debug message we want as output doesn't fit into this.
>> >>
>> >> Make use of the standard devlink tracepoint wherever applicable, and you
>> >> can keep your extra ones if you want (as long as Jiri don't object).
>> >
>> >Sure and noted. I have tried to use devlink tracepoints and since it
>> >could not fit our purpose I used these.
>>
>> Why exactly the existing TP didn't fit your need?
>>
>Existing TP has provision to dump skb and trace error strings with
>error code but
>we are trying to trace the entire mailbox flow of the AF/PF and VF
>drivers. In particular
>we trace the below:
>    message allocation with message id and size at initiator.
>    number of messages sent and total size.
>    check message requester id, response id and response code after
>reply is received.
>    interrupts happened on behalf of mailboxes in the entire process
>with source and receiver of interrupt along with isr status.
>    error like initiator timeout waiting for response.
>  All the above are relevant and are required for Octeontx2 only hence
>used own tracepoints.

You can still use devlink_hwmsg for the actual data exchanged between
the driver and hw. For the rest, you can have driver-specific TPs.


>
>Thanks,
>Sundeep
>
>> >
>> >Thanks,
>> >Sundeep

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

* Re: [EXT] Re: [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2
  2020-09-17  6:04                     ` Jiri Pirko
@ 2020-09-17 10:30                       ` sundeep subbaraya
  0 siblings, 0 replies; 19+ messages in thread
From: sundeep subbaraya @ 2020-09-17 10:30 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Jakub Kicinski, Sunil Kovvuri Goutham, davem, netdev,
	Subbaraya Sundeep Bhatta

On Thu, Sep 17, 2020 at 11:34 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Wed, Sep 16, 2020 at 07:19:36PM CEST, sundeep.lkml@gmail.com wrote:
> >On Wed, Sep 16, 2020 at 4:04 PM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> Mon, Sep 07, 2020 at 12:59:45PM CEST, sundeep.lkml@gmail.com wrote:
> >> >Hi Jakub,
> >> >
> >> >On Sat, Sep 5, 2020 at 2:07 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >> >>
> >> >> On Fri, 4 Sep 2020 12:29:04 +0000 Sunil Kovvuri Goutham wrote:
> >> >> > > >No, there are 3 drivers registering to 3 PCI device IDs and there can
> >> >> > > >be many instances of the same devices. So there can be 10's of instances of
> >> >> > > AF, PF and VFs.
> >> >> > >
> >> >> > > So you can still have per-pci device devlink instance and use the tracepoint
> >> >> > > Jakub suggested.
> >> >> > >
> >> >> >
> >> >> > Two things
> >> >> > - As I mentioned above, there is a Crypto driver which uses the same mbox APIs
> >> >> >   which is in the process of upstreaming. There also we would need trace points.
> >> >> >   Not sure registering to devlink just for the sake of tracepoint is proper.
> >> >> >
> >> >> > - The devlink trace message is like this
> >> >> >
> >> >> >    TRACE_EVENT(devlink_hwmsg,
> >> >> >      . . .
> >> >> >         TP_printk("bus_name=%s dev_name=%s driver_name=%s incoming=%d type=%lu buf=0x[%*phD] len=%zu",
> >> >> >                   __get_str(bus_name), __get_str(dev_name),
> >> >> >                   __get_str(driver_name), __entry->incoming, __entry->type,
> >> >> >                   (int) __entry->len, __get_dynamic_array(buf), __entry->len)
> >> >> >    );
> >> >> >
> >> >> >    Whatever debug message we want as output doesn't fit into this.
> >> >>
> >> >> Make use of the standard devlink tracepoint wherever applicable, and you
> >> >> can keep your extra ones if you want (as long as Jiri don't object).
> >> >
> >> >Sure and noted. I have tried to use devlink tracepoints and since it
> >> >could not fit our purpose I used these.
> >>
> >> Why exactly the existing TP didn't fit your need?
> >>
> >Existing TP has provision to dump skb and trace error strings with
> >error code but
> >we are trying to trace the entire mailbox flow of the AF/PF and VF
> >drivers. In particular
> >we trace the below:
> >    message allocation with message id and size at initiator.
> >    number of messages sent and total size.
> >    check message requester id, response id and response code after
> >reply is received.
> >    interrupts happened on behalf of mailboxes in the entire process
> >with source and receiver of interrupt along with isr status.
> >    error like initiator timeout waiting for response.
> >  All the above are relevant and are required for Octeontx2 only hence
> >used own tracepoints.
>
> You can still use devlink_hwmsg for the actual data exchanged between
> the driver and hw. For the rest, you can have driver-specific TPs.
>
>
I totally got your point and adding devlink to our drivers is work in progress
since we got a similar comment from Jakub for a patch previously:
https://www.mail-archive.com/netdev@vger.kernel.org/msg341414.html
All the errors in the drivers will be turned to devlink TP in future.
This patchset is a bit different since it traces mailbox messages state machine
at low level and does not even trace message data exchanged between
driver and hw.

Thanks,
Sundeep

> >
> >Thanks,
> >Sundeep
> >
> >> >
> >> >Thanks,
> >> >Sundeep

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

end of thread, other threads:[~2020-09-17 10:30 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03  7:18 [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 sundeep.lkml
2020-09-03  7:18 ` [net-next PATCH 1/2] octeontx2-af: Introduce tracepoints for mailbox sundeep.lkml
2020-09-03  7:18 ` [net-next PATCH 2/2] octeontx2-pf: Add tracepoints for PF/VF mailbox sundeep.lkml
2020-09-03 19:18 ` [net-next PATCH 0/2] Introduce mbox tracepoints for Octeontx2 Jakub Kicinski
2020-09-04  5:39   ` [EXT] " Sunil Kovvuri Goutham
2020-09-04  8:37     ` Jiri Pirko
2020-09-04  8:49       ` Sunil Kovvuri Goutham
2020-09-04 12:11         ` Jiri Pirko
2020-09-04 12:29           ` Sunil Kovvuri Goutham
2020-09-04 20:37             ` Jakub Kicinski
2020-09-07 10:59               ` sundeep subbaraya
2020-09-15 15:52                 ` sundeep subbaraya
2020-09-15 16:12                   ` Jakub Kicinski
2020-09-15 16:36                     ` sundeep subbaraya
2020-09-15 16:44                       ` Jakub Kicinski
2020-09-16 10:34                 ` Jiri Pirko
2020-09-16 17:19                   ` sundeep subbaraya
2020-09-17  6:04                     ` Jiri Pirko
2020-09-17 10:30                       ` sundeep subbaraya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).