On Sun, Nov 03, 2024 at 02:29:27PM +0200, Gur Stavi wrote:

quoted

quoted

On Wed, 30 Oct 2024 14:25:47 +0200 Gur Stavi wrote:

quoted

 50 files changed, 18058 insertions(+)

4kLoC is the right ballpark to target for the initial submission.
Please cut this down and submit a minimal driver, then add the
features.

Ack.
There is indeed code which is not critical to basic Ethernet

functionality

quoted

that can be postponed to later.

Our HW management infrastructure is rather large and contains 2

separate

quoted

mechanisms (cmdq+mbox). While I hope we can trim the driver to a VF-

only

quoted

version with no ethtool support that will fit the 10KLoC ballpark, the

4KLoC

quoted

goal is probably unrealistic for a functional driver.

It is really all about making you code attractive to reviewers. No
reviewer is likely to have time to review a single 10KLoc patch. A

The minimal work for a functional Ethernet driver when responding to
probe is:
1. Initialize device management
2. Create IO queues using device management
3. Register interrupts
4. Create netdev on top of the io queues
5. Handle TX+RX

At the moment, just our TX+RX code is ~4KLoC.

reviewer is more likely to look at the code if it is broken up into 15
smaller patches, each one which can be reviewed in a coffee break

We can obviously break a larger submission into multiple patches if
requested.
But when researching we saw a comment from David that indicated that there
is no advantage in that for initial submissions.
https://lore.kernel.org/netdev/20170817.220343.905568389038615738.davem@dave
mloft.net/
Quote:
"And this is a fine way to add a huge new driver (although not my
personal preference)."

Breaking a 10KLoC submission into a few 4KLcC (or less) patches helps to
review specific patches (and ignore other patches) but all lines still need
to be approved at once so someone must review them.

Breaking 10KLoC into multiple submissions is easier to review and approve
(in parts), but merged code will be non-functional until the last
submission.
It will compile fine, do no harm, and nobody will pick it except for allyes
builds.

etc. Also, reviewers have interests. I personally have no interest in
mailbox APIs, actually moving frames around, etc. I want to easily
find the ethtool code, have you got pause wrong like nearly everybody
does, are the statistics correctly broken up into the standard groups,

To properly review (error prone) pause it would be better to remove it from
initial submission and add it in a later dedicated submission.

are you abusing debugfs? Having little patches with good subject lines
will draw me towards the patches i want to review.

10KLoC is still on the large size. Can you throw VF out, it is just a
plain borring single function device, like the good old e1000e?

VF driver is just like good old e1000e NIC. It is a driver that registers
A single PCI vid:did, "unaware" that its part of SRIOV, and performs the
initialization sequence described above when probed.

	Andrew