netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Paolo Abeni <pabeni@redhat.com>,
	Jakub Kicinski <kuba@kernel.org>,
	David Miller <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next] ethtool: check for unsupported modes in EEE advertisement
Date: Thu, 15 Feb 2024 15:53:40 +0000	[thread overview]
Message-ID: <Zc4zhPSceYVlYnWc@shell.armlinux.org.uk> (raw)
In-Reply-To: <c02d4d86-6e65-4270-bc46-70acb6eb2d4a@gmail.com>

On Thu, Feb 15, 2024 at 02:05:54PM +0100, Heiner Kallweit wrote:
> Let the core check whether userspace returned unsupported modes in the
> EEE advertisement bitmap. This allows to remove these checks from
> drivers.

Why is this a good thing to implement?

Concerns:
1) This is a change of behaviour for those drivers that do not
implement this behaviour.

2) This behaviour is different from ksettings_set() which silently
trims the advertisement down to the modes that are supported

3) This check is broken. Userspace is at liberty to pass in ~0 for
the supported mask and the advertising mask which subverts this
check.

So... I think overall, it's a NAK to this from me - I don't think
it's something that anyone should implement. Restricting the
advertisement to the modes that are supported (where the supported
mask is pulled from the network driver and not userspace) would
be acceptable, but is that actually necessary?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  parent reply	other threads:[~2024-02-15 15:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-15 13:05 [PATCH net-next] ethtool: check for unsupported modes in EEE advertisement Heiner Kallweit
2024-02-15 14:44 ` Andrew Lunn
2024-02-15 21:13   ` Heiner Kallweit
2024-02-15 15:53 ` Russell King (Oracle) [this message]
2024-02-15 20:27   ` Heiner Kallweit
2024-02-15 21:08     ` Heiner Kallweit
2024-02-15 23:33   ` Andrew Lunn
2024-02-16 11:21     ` Heiner Kallweit

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=Zc4zhPSceYVlYnWc@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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).