lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 5 Oct 2022 12:27:30 +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 3/3] i40e: Add i40e_napi_poll tracepoint On Wed, Oct 05, 2022 at 01:31:43AM -0700, Joe Damato wrote: Hi Joe, > 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: > > [...snip...] > > 1029.268 :0/0 i40e:i40e_napi_poll(i40e_napi_poll on dev eth1 q > i40e-eth1-TxRx-30 irq 172 irq_mask > 00000000,00000000,00000000,00000010,00000000,00000000 curr_cpu 68 budget > 64 bpr 64 work_done 0 tx_work_done 2 clean_complete 1 tx_clean_complete > 1) > 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]) maybe you could also include how to configure this tracepoint for future readers? > > Signed-off-by: Joe Damato <jdamato@...tly.com> > --- > drivers/net/ethernet/intel/i40e/i40e_trace.h | 50 ++++++++++++++++++++++++++++ > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 ++ > 2 files changed, 53 insertions(+) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_trace.h b/drivers/net/ethernet/intel/i40e/i40e_trace.h > index b5b1229..779d046 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_trace.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_trace.h > @@ -55,6 +55,56 @@ > * 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, int work_done, int tx_work_done, bool clean_complete, > + bool tx_clean_complete), > + > + TP_ARGS(napi, q, budget, budget_per_ring, work_done, tx_work_done, > + clean_complete, tx_clean_complete), > + > + TP_STRUCT__entry( > + __field(int, budget) > + __field(int, budget_per_ring) > + __field(int, work_done) > + __field(int, tx_work_done) > + __field(int, 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->work_done = work_done; What if rx clean routines failed to do allocation of new rx bufs? then this would be misinterpreted. maybe we should change the API to static bool i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget, unsigned int *processed_pkts); so you would return failure and at the end do *processed_pkts = total_rx_packets; then also i would change the naming of tracepoint entry. I'm not a native english speaker but having 'done' within the variable name suggests to me that it is rather a boolean. what about something like 'rx_cleaned_pkts' instead? Generally I think this is useful, personally I was in need of tracing the next_to_clean and next_to_use ring indexes a lot, but that is probably out of the scope in here. > + __entry->tx_work_done = tx_work_done; > + __entry->clean_complete = 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 work_done %d tx_work_done %d " > + "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->work_done, > + __entry->tx_work_done, > + __entry->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 ed88309..8b72f1b 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > @@ -2743,6 +2743,9 @@ int i40e_napi_poll(struct napi_struct *napi, int budget) > clean_complete = false; > } > > + trace_i40e_napi_poll(napi, q_vector, budget, budget_per_ring, work_done, tx_wd, > + clean_complete, tx_clean_complete); > + > /* If work not completed, return budget and polling will return */ > if (!clean_complete || !tx_clean_complete) { > int cpu_id = smp_processor_id(); > -- > 2.7.4 >
Powered by blists - more mailing lists