mbox series

[RFC,next-queue,0/4] i40e: Add a non-XDP i40e_napi_poll tracepoint

Message ID 1665165447-1802-1-git-send-email-jdamato@fastly.com
Headers show
Series i40e: Add a non-XDP i40e_napi_poll tracepoint | expand

Message

Joe Damato Oct. 7, 2022, 5:57 p.m. UTC 
Greetings:

This is an RFC which is similar to the series up for review, except that
this implementation does not touch XDP at all and adds a conditional in
i40e_napi_poll to only fire the tracepoint when XDP is not enabled.

This should avoid the issues that Maciej has with the naming of out
parameters (since none of that code is touched in this series) and it
clears the way for Maciej, Sridhar, et al to implement the XDP tracepoint.

I am submitting this an alternative to what's already up for review.

If you prefer to accept this code, please let me know that you want the
non-XDP version and I'll submit it as the 'v4'.

Thanks,
Joe


Joe Damato (4):
  i40e: Store the irq number in i40e_q_vector
  i40e: Record number TXes cleaned during NAPI
  i40e: Record number of RXes cleaned during NAPI
  i40e: Add i40e_napi_poll tracepoint

 drivers/net/ethernet/intel/i40e/i40e.h       |  1 +
 drivers/net/ethernet/intel/i40e/i40e_main.c  |  1 +
 drivers/net/ethernet/intel/i40e/i40e_trace.h | 49 ++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_txrx.c  | 27 +++++++++++----
 4 files changed, 72 insertions(+), 6 deletions(-)

Comments

Jesse Brandeburg Oct. 7, 2022, 8:36 p.m. UTC | #1 
On 10/7/2022 10:57 AM, Joe Damato wrote:
> Greetings:
> 
> This is an RFC which is similar to the series up for review, except that
> this implementation does not touch XDP at all and adds a conditional in
> i40e_napi_poll to only fire the tracepoint when XDP is not enabled.
> 
> This should avoid the issues that Maciej has with the naming of out
> parameters (since none of that code is touched in this series) and it
> clears the way for Maciej, Sridhar, et al to implement the XDP tracepoint.
> 
> I am submitting this an alternative to what's already up for review.
> 
> If you prefer to accept this code, please let me know that you want the
> non-XDP version and I'll submit it as the 'v4'.

Given the discussion, this is the series I prefer. I'm very happy to see 
some more debugging helpers coming into the driver so thanks for your 
work on this Joe! As for the rest of the team they seem to be fine 
speaking for themselves, so I imagine they'll let you know :-)

For the series:
Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Best regards,
Jesse
Joe Damato Oct. 7, 2022, 8:55 p.m. UTC | #2 
On Fri, Oct 07, 2022 at 01:36:20PM -0700, Jesse Brandeburg wrote:
> On 10/7/2022 10:57 AM, Joe Damato wrote:
> >Greetings:
> >
> >This is an RFC which is similar to the series up for review, except that
> >this implementation does not touch XDP at all and adds a conditional in
> >i40e_napi_poll to only fire the tracepoint when XDP is not enabled.
> >
> >This should avoid the issues that Maciej has with the naming of out
> >parameters (since none of that code is touched in this series) and it
> >clears the way for Maciej, Sridhar, et al to implement the XDP tracepoint.
> >
> >I am submitting this an alternative to what's already up for review.
> >
> >If you prefer to accept this code, please let me know that you want the
> >non-XDP version and I'll submit it as the 'v4'.
> 
> Given the discussion, this is the series I prefer. I'm very happy to see
> some more debugging helpers coming into the driver so thanks for your work
> on this Joe! As for the rest of the team they seem to be fine speaking for
> themselves, so I imagine they'll let you know :-)
> 
> For the series:
> Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

OK, thanks!

There's a minor build failure for a format string (lu should be u) in the
tracepoint.

I'll fix that now in this series and re-send it as a v4 with a proper
cover letter.

Thanks again for your detailed feedback and review; I appreciate your time
and energy on this.

Thanks,
Joe
Samudrala, Sridhar Oct. 7, 2022, 9:01 p.m. UTC | #3 
On 10/7/2022 3:55 PM, Joe Damato wrote:
> On Fri, Oct 07, 2022 at 01:36:20PM -0700, Jesse Brandeburg wrote:
>> On 10/7/2022 10:57 AM, Joe Damato wrote:
>>> Greetings:
>>>
>>> This is an RFC which is similar to the series up for review, except that
>>> this implementation does not touch XDP at all and adds a conditional in
>>> i40e_napi_poll to only fire the tracepoint when XDP is not enabled.
>>>
>>> This should avoid the issues that Maciej has with the naming of out
>>> parameters (since none of that code is touched in this series) and it
>>> clears the way for Maciej, Sridhar, et al to implement the XDP tracepoint.
>>>
>>> I am submitting this an alternative to what's already up for review.
>>>
>>> If you prefer to accept this code, please let me know that you want the
>>> non-XDP version and I'll submit it as the 'v4'.
>> Given the discussion, this is the series I prefer. I'm very happy to see
>> some more debugging helpers coming into the driver so thanks for your work
>> on this Joe! As for the rest of the team they seem to be fine speaking for
>> themselves, so I imagine they'll let you know :-)
>>
>> For the series:
>> Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> OK, thanks!
>
> There's a minor build failure for a format string (lu should be u) in the
> tracepoint.
>
> I'll fix that now in this series and re-send it as a v4 with a proper
> cover letter.
>
> Thanks again for your detailed feedback and review; I appreciate your time
> and energy on this.
>
> Thanks,
> Joe

Sorry for all the back and forth on this series. The tracepoint itself is definitely
useful and this series looks good.
   Acked-by: Sridhar Samudrala <sridhar.samudrala@intel.com>