[PATCH net 1/2] MAINTAINERS: add entry for ethtool

[PATCH net 1/2] MAINTAINERS: add entry for ethtool

[Jakub Kicinski]

Michal did an amazing job converting ethtool to Netlink, but never
added an entry to MAINTAINERS for himself. Create a formal entry
so that we can delegate (portions) of this code to folks.

Over the last 3 years majority of the reviews have been done by
Andrew and I. I suppose Michal didn't want to be on the receiving
end of the flood of patches.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: Michal Kubecek <mkubecek@suse.cz>

I emailed Michal a few days ago and didn't hear back.
Michal, please LMK if you'd like to be added as well!
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ce92c8a3e3ce..4e701b9a57e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16459,6 +16459,16 @@ F:	include/net/dsa.h
 F:	net/dsa/
 F:	tools/testing/selftests/drivers/net/dsa/
 
+NETWORKING [ETHTOOL]
+M:	Andrew Lunn <andrew@lunn.ch>
+M:	Jakub Kicinski <kuba@kernel.org>
+F:	Documentation/netlink/specs/ethtool.yaml
+F:	Documentation/networking/ethtool-netlink.rst
+F:	include/linux/ethtool*
+F:	include/uapi/linux/ethtool*
+F:	net/ethtool/
+F:	tools/testing/selftests/drivers/net/*/ethtool*
+
 NETWORKING [GENERAL]
 M:	"David S. Miller" <davem@davemloft.net>
 M:	Eric Dumazet <edumazet@google.com>
-- 
2.48.1

[PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Jakub Kicinski]

I feel like we don't do a good enough keeping authors of driver
APIs around. The ethtool code base was very nicely compartmentalized
by Michal. Establish a precedent of creating MAINTAINERS entries
for "sections" of the ethtool API. Use Andrew and cable test as
a sample entry. The entry should ideally cover 3 elements:
a core file, test(s), and keywords. The last one is important
because we intend the entries to cover core code *and* reviews
of drivers implementing given API!

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
This patch is a nop from process perspective, since Andrew already
is a maintainer and reviews all this code. Let's focus on discussing
merits of the "section entries" in abstract?
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4e701b9a57e4..9bf31ba720b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16469,6 +16469,12 @@ F:	include/uapi/linux/ethtool*
 F:	net/ethtool/
 F:	tools/testing/selftests/drivers/net/*/ethtool*
 
+NETWORKING [ETHTOOL CABLE TEST]
+M:	Andrew Lunn <andrew@lunn.ch>
+F:	net/ethtool/cabletest.c
+F:	tools/testing/selftests/drivers/net/*/ethtool*
+K:	start_cable_test
+
 NETWORKING [GENERAL]
 M:	"David S. Miller" <davem@davemloft.net>
 M:	Eric Dumazet <edumazet@google.com>
-- 
2.48.1

Re: [PATCH net 1/2] MAINTAINERS: add entry for ethtool

[Simon Horman]

On Sat, Feb 01, 2025 at 06:11:54PM -0800, Jakub Kicinski wrote:
> Michal did an amazing job converting ethtool to Netlink, but never
> added an entry to MAINTAINERS for himself. Create a formal entry
> so that we can delegate (portions) of this code to folks.
> 
> Over the last 3 years majority of the reviews have been done by
> Andrew and I. I suppose Michal didn't want to be on the receiving
> end of the flood of patches.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: Michal Kubecek <mkubecek@suse.cz>
> 
> I emailed Michal a few days ago and didn't hear back.
> Michal, please LMK if you'd like to be added as well!

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Simon Horman]

On Sat, Feb 01, 2025 at 06:11:55PM -0800, Jakub Kicinski wrote:
> I feel like we don't do a good enough keeping authors of driver
> APIs around. The ethtool code base was very nicely compartmentalized
> by Michal. Establish a precedent of creating MAINTAINERS entries
> for "sections" of the ethtool API. Use Andrew and cable test as
> a sample entry. The entry should ideally cover 3 elements:
> a core file, test(s), and keywords. The last one is important
> because we intend the entries to cover core code *and* reviews
> of drivers implementing given API!
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> This patch is a nop from process perspective, since Andrew already
> is a maintainer and reviews all this code. Let's focus on discussing
> merits of the "section entries" in abstract?

In the first instance this seems like a good direction to go in to me.
My only slight concern is that we might see an explosion in entries.
Do we think so? Do we mind if that happens?

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Andrew Lunn]

On Mon, Feb 03, 2025 at 10:56:47AM +0000, Simon Horman wrote:
> On Sat, Feb 01, 2025 at 06:11:55PM -0800, Jakub Kicinski wrote:
> > I feel like we don't do a good enough keeping authors of driver
> > APIs around. The ethtool code base was very nicely compartmentalized
> > by Michal. Establish a precedent of creating MAINTAINERS entries
> > for "sections" of the ethtool API. Use Andrew and cable test as
> > a sample entry. The entry should ideally cover 3 elements:
> > a core file, test(s), and keywords. The last one is important
> > because we intend the entries to cover core code *and* reviews
> > of drivers implementing given API!
> > 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> > This patch is a nop from process perspective, since Andrew already
> > is a maintainer and reviews all this code. Let's focus on discussing
> > merits of the "section entries" in abstract?
> 
> In the first instance this seems like a good direction to go in to me.
> My only slight concern is that we might see an explosion in entries.

I don't think that will happen. I don't think we really have many
sections of ethtool which people personally care about, always try to
review across all drivers.

Even if it does explode, so what. Is ./scripts/get_maintainer.pl the
bottleneck in any workflows?

	Andrew

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Jakub Kicinski]

On Mon, 3 Feb 2025 14:29:23 +0100 Andrew Lunn wrote:
> > In the first instance this seems like a good direction to go in to me.
> > My only slight concern is that we might see an explosion in entries.  
> 
> I don't think that will happen. I don't think we really have many
> sections of ethtool which people personally care about, always try to
> review across all drivers.

Agreed, FWIW.

> Even if it does explode, so what. Is ./scripts/get_maintainer.pl the
> bottleneck in any workflows?

Only concern there could be the keywords, we had issues with regexps
being too expensive in the past. There are plenty examples of how to
do them right now, tho.

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Paolo Abeni]

On 2/2/25 3:11 AM, Jakub Kicinski wrote:
> This patch is a nop from process perspective, since Andrew already
> is a maintainer and reviews all this code. Let's focus on discussing
> merits of the "section entries" in abstract?

Should the keyword be a little more generic, i.e. just 'cable_test'?
AFAICS the current one doesn't catch the device drivers,

I agree encouraging more driver API reviewer would be great, but I
personally have a slight preference to add/maintain entries only they
actually affect the process.

What about tying the creation of the entry to some specific contribution?

/P

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Simon Horman]

On Mon, Feb 03, 2025 at 02:29:23PM +0100, Andrew Lunn wrote:
> On Mon, Feb 03, 2025 at 10:56:47AM +0000, Simon Horman wrote:
> > On Sat, Feb 01, 2025 at 06:11:55PM -0800, Jakub Kicinski wrote:
> > > I feel like we don't do a good enough keeping authors of driver
> > > APIs around. The ethtool code base was very nicely compartmentalized
> > > by Michal. Establish a precedent of creating MAINTAINERS entries
> > > for "sections" of the ethtool API. Use Andrew and cable test as
> > > a sample entry. The entry should ideally cover 3 elements:
> > > a core file, test(s), and keywords. The last one is important
> > > because we intend the entries to cover core code *and* reviews
> > > of drivers implementing given API!
> > > 
> > > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > > ---
> > > This patch is a nop from process perspective, since Andrew already
> > > is a maintainer and reviews all this code. Let's focus on discussing
> > > merits of the "section entries" in abstract?
> > 
> > In the first instance this seems like a good direction to go in to me.
> > My only slight concern is that we might see an explosion in entries.
> 
> I don't think that will happen. I don't think we really have many
> sections of ethtool which people personally care about, always try to
> review across all drivers.
> 
> Even if it does explode, so what. Is ./scripts/get_maintainer.pl the
> bottleneck in any workflows?

Thanks Andrew,

I'm not overly concerned by the points I raised either, but I did think
they were worth raising.  And given that doing so didn't raise any alarm
bells (so far), I'm happy for this patch to proceed.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Jakub Kicinski]

On Tue, 4 Feb 2025 10:26:40 +0100 Paolo Abeni wrote:
> On 2/2/25 3:11 AM, Jakub Kicinski wrote:
> > This patch is a nop from process perspective, since Andrew already
> > is a maintainer and reviews all this code. Let's focus on discussing
> > merits of the "section entries" in abstract?  
> 
> Should the keyword be a little more generic, i.e. just 'cable_test'?
> AFAICS the current one doesn't catch the device drivers,
> 
> I agree encouraging more driver API reviewer would be great, but I
> personally have a slight preference to add/maintain entries only they
> actually affect the process.

You're right, I was going after the op name. Seems like a good default
keyword. But it appears that there are two layers of ops, one called
start_cable_test and the next cable_test_start, so this isn't catching
actual drivers.

> What about tying the creation of the entry to some specific contribution?

Sure. I'm adding this so that we have a commit to point people at 
as an example when they contribute what should be a new section.
Maybe I don't understand the question..

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Paolo Abeni]

On 2/4/25 4:37 PM, Jakub Kicinski wrote:
> On Tue, 4 Feb 2025 10:26:40 +0100 Paolo Abeni wrote:
>> What about tying the creation of the entry to some specific contribution?
> 
> Sure. I'm adding this so that we have a commit to point people at 
> as an example when they contribute what should be a new section.
> Maybe I don't understand the question..

I meant that this section could be added together with a new associated
name when a suitable person will pop-up.

Or course that would not help as an example, and I initially did see
there was such a goal. I'm fine with adding new entry even now.

/P

Re: [PATCH net 2/2] MAINTAINERS: add a sample ethtool section entry

[Andrew Lunn]

On Tue, Feb 04, 2025 at 07:37:59AM -0800, Jakub Kicinski wrote:
> On Tue, 4 Feb 2025 10:26:40 +0100 Paolo Abeni wrote:
> > On 2/2/25 3:11 AM, Jakub Kicinski wrote:
> > > This patch is a nop from process perspective, since Andrew already
> > > is a maintainer and reviews all this code. Let's focus on discussing
> > > merits of the "section entries" in abstract?  
> > 
> > Should the keyword be a little more generic, i.e. just 'cable_test'?
> > AFAICS the current one doesn't catch the device drivers,
> > 
> > I agree encouraging more driver API reviewer would be great, but I
> > personally have a slight preference to add/maintain entries only they
> > actually affect the process.
> 
> You're right, I was going after the op name. Seems like a good default
> keyword. But it appears that there are two layers of ops, one called
> start_cable_test and the next cable_test_start, so this isn't catching
> actual drivers.

https://elixir.bootlin.com/linux/v6.13.1/source/include/linux/phy.h#L1080

The op in the driver structure is cable_test_start. There is also
cable_test_tdr_start but so far only Marvell hardware supports raw TDR
data. At the moment, cable_test_start does not produce any false
positives anywhere else in the kernel.

	Andrew