linux-kernel - Re: [PATCH net] net: phy: mscc: Stop clearing the the UDPv4 checksum for L2 frames

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: <20250526065445.o7pchn5tilq7izmx@DEN-DL-M31836.microchip.com>
Date: Mon, 26 May 2025 08:54:45 +0200
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <hkallweit1@...il.com>, <linux@...linux.org.uk>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<richardcochran@...il.com>, <kory.maincent@...tlin.com>,
	<wintera@...ux.ibm.com>, <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] net: phy: mscc: Stop clearing the the UDPv4 checksum
 for L2 frames

The 05/23/2025 14:59, Andrew Lunn wrote:

Hi Andrew,

> 
> On Fri, May 23, 2025 at 10:27:16AM +0200, Horatiu Vultur wrote:
> > We have noticed that when PHY timestamping is enabled, L2 frames seems
> > to be modified by changing two 2 bytes with a value of 0. The place were
> > these 2 bytes seems to be random(or I couldn't find a pattern).  In most
> > of the cases the userspace can ignore these frames but if for example
> > those 2 bytes are in the correction field there is nothing to do.  This
> > seems to happen when configuring the HW for IPv4 even that the flow is
> > not enabled.
> > These 2 bytes correspond to the UDPv4 checksum and once we don't enable
> > clearing the checksum when using L2 frames then the frame doesn't seem
> > to be changed anymore.
> >
> > Fixes: 7d272e63e0979d ("net: phy: mscc: timestamping and PHC support")
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> > ---
> >  drivers/net/phy/mscc/mscc_ptp.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
> > index 6f96f2679f0bf..6b800081eed52 100644
> > --- a/drivers/net/phy/mscc/mscc_ptp.c
> > +++ b/drivers/net/phy/mscc/mscc_ptp.c
> > @@ -946,7 +946,9 @@ static int vsc85xx_ip1_conf(struct phy_device *phydev, enum ts_blk blk,
> >       /* UDP checksum offset in IPv4 packet
> >        * according to: https://tools.ietf.org/html/rfc768
> >        */
> > -     val |= IP1_NXT_PROT_UDP_CHKSUM_OFF(26) | IP1_NXT_PROT_UDP_CHKSUM_CLEAR;
> > +     val |= IP1_NXT_PROT_UDP_CHKSUM_OFF(26);
> > +     if (enable)
> > +             val |= IP1_NXT_PROT_UDP_CHKSUM_CLEAR;
> 
> Is this towards the media, or received from the media?

It is when the vsc85xx PHY receives frames from the link partner.

>Have you tried sending packets with deliberately broken UDPv4 checksum?
>Does the PHYs PTP engine correctly ignore such packets?

No, I have not done that. What I don't understand is why should I send
UDPv4 frames when we enable to timestamp only L2 frames.

> 
> I suppose the opposite could also be true. Do you see it ignoring
> frames which are actually O.K? It could be looking in the wrong place
> for the checksum, so the checksum fails.

I have not seen any frames being ignored by HW. The HW is configured to
set the nanosecond part of the RX timestamp into the frame. And it is
always doing that, the problem is that sometimes on top of this change
it also replaces 2 bytes in the frame with 0. And it is the userspace
(ptp4l) who ignores those frames because the are corrupted.

> 
>         Andrew

-- 
/Horatiu