linux-kernel - Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250818143732.q5eymo65iywz44ci@skbuf>
Date: Mon, 18 Aug 2025 17:37:32 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc: andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, richardcochran@...il.com,
	rmk+kernel@...linux.org.uk, rosenp@...il.com,
	christophe.jaillet@...adoo.fr, viro@...iv.linux.org.uk,
	quentin.schulz@...tlin.com, atenart@...nel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v4] phy: mscc: Fix timestamping for vsc8584

On Mon, Aug 18, 2025 at 04:19:25PM +0200, Horatiu Vultur wrote:
> Nothing prevents me for looking at this issue. I just need to alocate
> some time for this.
> 
> > The two problems are introduced by the same commit, and fixes will be
> > backported to all the same stable kernels. I don't exactly understand
> > why you'd add some code to the PHY's remove() method, but not enough in
> > order for it to work.
> 
> Yes, I understand that but the fix for ptp_clock_unregister will fix a
> different issue that this patch is trying to fix. That is the reason why
> I prefer not to add that fix now, just to make things more clear.

Not sure "clear" for whom. One of the rules from Documentation/process/stable-kernel-rules.rst
is "It must be obviously correct and tested.", which to me makes it confusing
why you wouldn't fix that issue first (within the same patch set), and then
test this patch during unbind/bind to confirm that it achieves what it intends.

I think the current state of the art is that unbinding a PHY that the
MAC hasn't connected to will work, whereas unbinding a connected PHY,
where the state machine is running, will crash the kernel. To be
perfectly clear, the request is just for the case that is supposed to
work given current phylib implementation, aka with the MAC unconnected
(put administratively down or also unbound, depending on whether it
connects to the PHY at probe time or ndo_open() time).

I don't see where the reluctance comes from - is it that there are going
to be 2 patches instead of 1? My reluctance as a reviewer comes from the
fact that I'm analyzing the change in the larger context and not seeing
how the remove() method you introduced makes any practical difference.
Not sure what I'm supposed to say.