[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Yz/g4W9rnhLcBsrd@boxer>
Date: Fri, 7 Oct 2022 10:18:41 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Joe Damato <jdamato@...tly.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
<kuba@...nel.org>, <davem@...emloft.net>,
<anthony.l.nguyen@...el.com>, <jesse.brandeburg@...el.com>
Subject: Re: [next-queue v3 4/4] i40e: Add i40e_napi_poll tracepoint
On Thu, Oct 06, 2022 at 04:43:58PM -0700, Joe Damato wrote:
> Add a tracepoint for i40e_napi_poll that allows users to get detailed
> information about the amount of work done. This information can help users
> better tune the correct NAPI parameters (like weight and budget), as well
> as debug NIC settings like rx-usecs and tx-usecs, etc.
>
> An example of the output from this tracepoint:
>
> $ sudo perf trace -e i40e:i40e_napi_poll -a --call-graph=fp --libtraceevent_print
>
> [..snip..]
>
> 388.258 :0/0 i40e:i40e_napi_poll(i40e_napi_poll on dev eth2 q
> i40e-eth2-TxRx-9 irq 346 irq_mask
> 00000000,00000000,00000000,00000000,00000000,00800000 curr_cpu 23 budget
> 64 bpr 64 rx_cleaned 28 tx_cleaned 0 rx_clean_complete 1
> tx_clean_complete 1)
So from AF_XDP POV I won't be using it as I would need some other
information. As I said, we don't work on NAPI budget but rather with the
free ring space and I don't get it here. tx_cleaned is also quite
incorrect name to me for count of produced descriptors to Tx ring. I feel
like it would be better to stub it for AF_XDP.
As Jesse said previously we probably can followup with AF_XDP specific
tracepoint with tx cleaned/tx transmitted/NAPI budget/AF_XDP budget (free
ring space) if we find the need for it.
That's my 0.02$, I'm not going to hold this set or whatever, I'll leave the
decision to Sridhar & Jesse.
> i40e_napi_poll ([i40e])
> i40e_napi_poll ([i40e])
> __napi_poll ([kernel.kallsyms])
> net_rx_action ([kernel.kallsyms])
> __do_softirq ([kernel.kallsyms])
> common_interrupt ([kernel.kallsyms])
> asm_common_interrupt ([kernel.kallsyms])
> intel_idle_irq ([kernel.kallsyms])
> cpuidle_enter_state ([kernel.kallsyms])
> cpuidle_enter ([kernel.kallsyms])
> do_idle ([kernel.kallsyms])
> cpu_startup_entry ([kernel.kallsyms])
> [0x243fd8] ([kernel.kallsyms])
> secondary_startup_64_no_verify ([kernel.kallsyms])
>
> Signed-off-by: Joe Damato <jdamato@...tly.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_trace.h | 49 ++++++++++++++++++++++++++++
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 ++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_trace.h b/drivers/net/ethernet/intel/i40e/i40e_trace.h
> index b5b1229..7d7c161 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_trace.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_trace.h
> @@ -55,6 +55,55 @@
> * being built from shared code.
> */
>
> +#define NO_DEV "(i40e no_device)"
> +
> +TRACE_EVENT(i40e_napi_poll,
> +
> + TP_PROTO(struct napi_struct *napi, struct i40e_q_vector *q, int budget,
> + int budget_per_ring, unsigned int rx_cleaned, unsigned int tx_cleaned,
> + bool rx_clean_complete, bool tx_clean_complete),
> +
> + TP_ARGS(napi, q, budget, budget_per_ring, rx_cleaned, tx_cleaned,
> + rx_clean_complete, tx_clean_complete),
> +
> + TP_STRUCT__entry(
> + __field(int, budget)
> + __field(int, budget_per_ring)
> + __field(unsigned int, rx_cleaned)
> + __field(unsigned int, tx_cleaned)
> + __field(int, rx_clean_complete)
> + __field(int, tx_clean_complete)
> + __field(int, irq_num)
> + __field(int, curr_cpu)
> + __string(qname, q->name)
> + __string(dev_name, napi->dev ? napi->dev->name : NO_DEV)
> + __bitmask(irq_affinity, nr_cpumask_bits)
> + ),
> +
> + TP_fast_assign(
> + __entry->budget = budget;
> + __entry->budget_per_ring = budget_per_ring;
> + __entry->rx_cleaned = rx_cleaned;
> + __entry->tx_cleaned = tx_cleaned;
> + __entry->rx_clean_complete = rx_clean_complete;
> + __entry->tx_clean_complete = tx_clean_complete;
> + __entry->irq_num = q->irq_num;
> + __entry->curr_cpu = get_cpu();
> + __assign_str(qname, q->name);
> + __assign_str(dev_name, napi->dev ? napi->dev->name : NO_DEV);
> + __assign_bitmask(irq_affinity, cpumask_bits(&q->affinity_mask),
> + nr_cpumask_bits);
> + ),
> +
> + TP_printk("i40e_napi_poll on dev %s q %s irq %d irq_mask %s curr_cpu %d "
> + "budget %d bpr %d rx_cleaned %lu tx_cleaned %lu "
> + "rx_clean_complete %d tx_clean_complete %d",
> + __get_str(dev_name), __get_str(qname), __entry->irq_num,
> + __get_bitmask(irq_affinity), __entry->curr_cpu, __entry->budget,
> + __entry->budget_per_ring, __entry->rx_cleaned, __entry->tx_cleaned,
> + __entry->rx_clean_complete, __entry->tx_clean_complete)
> +);
> +
> /* Events related to a vsi & ring */
> DECLARE_EVENT_CLASS(
> i40e_tx_template,
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index adf133b..fb9add8 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2753,6 +2753,9 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
> clean_complete = rx_clean_complete = false;
> }
>
> + trace_i40e_napi_poll(napi, q_vector, budget, budget_per_ring, rx_cleaned,
> + tx_cleaned, rx_clean_complete, tx_clean_complete);
> +
> /* If work not completed, return budget and polling will return */
> if (!clean_complete) {
> int cpu_id = smp_processor_id();
> --
> 2.7.4
>
Powered by blists - more mailing lists