netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "John Efstathiades" <john.efstathiades@pebblebay.com>
To: "'Jesse Brandeburg'" <jesse.brandeburg@intel.com>
Cc: <UNGLinuxDriver@microchip.com>, <davem@davemloft.net>,
	<netdev@vger.kernel.org>, <Woojung.Huh@microchip.com>
Subject: RE: [PATCH net-next 1/9] lan78xx: add NAPI interface support
Date: Wed, 10 Feb 2021 16:11:09 -0000	[thread overview]
Message-ID: <005101d6ffc7$6409d8b0$2c1d8a10$@pebblebay.com> (raw)
In-Reply-To: <20210204124314.00007907@intel.com>

Apologies for taking a while to respond.

> -----Original Message-----
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Sent: 04 February 2021 20:43
> 
> NB: I thought I'd have a close look at this since I thought I
> understand NAPI pretty well, but using NAPI to transmit frames as well
> as with a usb device has got me pretty confused. 

I'll try to add some more rationale in the next revision of the patch.
However, the short answer is that using NAPI for transmit under high load is
the most effective way of getting the frames into the device's internal
buffer RAM.

> Also, I suspect that
> you didn't try compiling this against the net-next kernel.

I thought I had but it appears not. I won't let that happen again.

> I'm stopping my review only partially completed, please address issues
> https://patchwork.kernel.org/project/netdevbpf/patch/20210204113121.29786-
> 2-john.efstathiades@pebblebay.com/

Thanks, will do, but it will take me a few weeks to sort everything out due
to my other commitments.

> It might make it easier for reviewers to split the "infrastructure"
> refactors this patch uses into separate pieces. I know it is more work
> and this is tested already by you, but this is a pretty complicated
> chunk of code to review.

I appreciate this is a complicated patch, a point made by another reviewer. 
Could you explain what you mean by "infrastructure" refactors, please?

I'll certainly look at how to split this patch into smaller chunks but that
might be quite hard to do.

If that turns out not to possible, do you have any suggestions on how I can
make the patch easier for reviewers to understand and review?

John


  reply	other threads:[~2021-02-10 16:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-04 11:31 [PATCH net-next 0/9] LAN7800 USB network interface driver NAPI support John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 1/9] lan78xx: add NAPI interface support John Efstathiades
2021-02-04 20:43   ` Jesse Brandeburg
2021-02-10 16:11     ` John Efstathiades [this message]
     [not found]   ` <YBv4VVhsswYtX6qc@lunn.ch>
2021-02-10 15:23     ` John Efstathiades
2021-02-10 16:31       ` Andrew Lunn
2021-02-04 11:31 ` [PATCH net-next 2/9] lan78xx: disable U1/U2 power state transitions John Efstathiades
     [not found]   ` <YBv5S3OVWnPX5Y+M@lunn.ch>
2021-02-10 15:27     ` John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 3/9] lan78xx: fix USB errors and packet loss on suspend/resume John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 4/9] lan78xx: disable MAC address filter before updating entry John Efstathiades
     [not found]   ` <YBv6tF1caI7L96sW@lunn.ch>
2021-02-10 15:33     ` John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 5/9] lan78xx: fix race condition in PHY handling causing kernel lock up John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 6/9] lan78xx: reduce number of register access failure warnings John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 7/9] lan78xx: set maximum MTU John Efstathiades
     [not found]   ` <YBv6/zr82VZcgGMG@lunn.ch>
2021-02-10 15:35     ` John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 8/9] lan78xx: fix exception on link speed change John Efstathiades
2021-02-04 11:31 ` [PATCH net-next 9/9] lan78xx: remove set but unused 'ret' variable John Efstathiades

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='005101d6ffc7$6409d8b0$2c1d8a10$@pebblebay.com' \
    --to=john.efstathiades@pebblebay.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=Woojung.Huh@microchip.com \
    --cc=davem@davemloft.net \
    --cc=jesse.brandeburg@intel.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).