netdev - RE: [PATCH net-next v2 2/5] net: phy: microchip_ptp : Add ptp library for Microchip phys

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:
 <CO1PR11MB47717F2BB311C7F40B5B388AE2592@CO1PR11MB4771.namprd11.prod.outlook.com>
Date: Tue, 12 Nov 2024 08:27:40 +0000
From: <Divya.Koppera@...rochip.com>
To: <vadim.fedorenko@...ux.dev>, <andrew@...n.ch>,
	<Arun.Ramadoss@...rochip.com>, <UNGLinuxDriver@...rochip.com>,
	<hkallweit1@...il.com>, <linux@...linux.org.uk>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<richardcochran@...il.com>
Subject: RE: [PATCH net-next v2 2/5] net: phy: microchip_ptp : Add ptp library
 for Microchip phys

Hi @Vadim Fedorenko,

> -----Original Message-----
> From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> Sent: Monday, November 11, 2024 7:12 PM
> To: Divya Koppera - I30481 <Divya.Koppera@...rochip.com>;
> andrew@...n.ch; Arun Ramadoss - I17769
> <Arun.Ramadoss@...rochip.com>; UNGLinuxDriver
> <UNGLinuxDriver@...rochip.com>; hkallweit1@...il.com;
> linux@...linux.org.uk; davem@...emloft.net; edumazet@...gle.com;
> kuba@...nel.org; pabeni@...hat.com; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; richardcochran@...il.com
> Subject: Re: [PATCH net-next v2 2/5] net: phy: microchip_ptp : Add ptp library
> for Microchip phys
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On 11/11/2024 12:58, Divya Koppera wrote:
> > Add ptp library for Microchip phys
> > 1-step and 2-step modes are supported, over Ethernet and UDP(ipv4,
> > ipv6)
> >
> > Signed-off-by: Divya Koppera <divya.koppera@...rochip.com>
> > ---
> > v1 -> v2
> > - Removed redundant memsets
> > - Moved to standard comparision than memcmp for u16
> > - Fixed sparse/smatch warnings reported by kernel test robot
> > - Added spinlock to shared code
> > - Moved redundant part of code out of spinlock protected area
> > ---
> >   drivers/net/phy/microchip_ptp.c | 998
> ++++++++++++++++++++++++++++++++
> >   1 file changed, 998 insertions(+)
> >   create mode 100644 drivers/net/phy/microchip_ptp.c
> 
> [..snip..]
> 
> > +static struct mchp_ptp_rx_ts *mchp_ptp_get_rx_ts(struct
> > +mchp_ptp_clock *ptp_clock) {
> > +     struct phy_device *phydev = ptp_clock->phydev;
> > +     struct mchp_ptp_rx_ts *rx_ts = NULL;
> > +     u32 sec, nsec;
> > +     int rc;
> > +
> > +     rc = phy_read_mmd(phydev, PTP_MMD(ptp_clock),
> > +                       MCHP_PTP_RX_INGRESS_NS_HI(BASE_PORT(ptp_clock)));
> > +     if (rc < 0)
> > +             goto error;
> > +     if (!(rc & MCHP_PTP_RX_INGRESS_NS_HI_TS_VALID)) {
> > +             phydev_err(phydev, "RX Timestamp is not valid!\n");
> > +             goto error;
> > +     }
> > +     nsec = (rc & GENMASK(13, 0)) << 16;
> > +
> > +     rc = phy_read_mmd(phydev, PTP_MMD(ptp_clock),
> > +                       MCHP_PTP_RX_INGRESS_NS_LO(BASE_PORT(ptp_clock)));
> > +     if (rc < 0)
> > +             goto error;
> > +     nsec |= rc;
> > +
> > +     rc = phy_read_mmd(phydev, PTP_MMD(ptp_clock),
> > +                       MCHP_PTP_RX_INGRESS_SEC_HI(BASE_PORT(ptp_clock)));
> > +     if (rc < 0)
> > +             goto error;
> > +     sec = rc << 16;
> > +
> > +     rc = phy_read_mmd(phydev, PTP_MMD(ptp_clock),
> > +                       MCHP_PTP_RX_INGRESS_SEC_LO(BASE_PORT(ptp_clock)));
> > +     if (rc < 0)
> > +             goto error;
> > +     sec |= rc;
> > +
> > +     rc = phy_read_mmd(phydev, PTP_MMD(ptp_clock),
> > +                       MCHP_PTP_RX_MSG_HEADER2(BASE_PORT(ptp_clock)));
> > +     if (rc < 0)
> > +             goto error;
> > +
> > +     rx_ts = kzalloc(sizeof(*rx_ts), GFP_KERNEL);
> 
> I think I've asked it already, but why zero out new allocation, which will be fully
> re-written by the next instructions? Did you find any problems?
> 

Yes, checked with different thing and faced issue. So reverted change. But now replaced with kmalloc and not seen any issues.
Will send new revision with this change.

Thanks,
Divya

> > +     if (!rx_ts)
> > +             return NULL;
> > +
> > +     rx_ts->seconds = sec;
> > +     rx_ts->nsec = nsec;
> > +     rx_ts->seq_id = rc;
> > +
> > +error:
> > +     return rx_ts;
> > +}
> > +
> > +static void mchp_ptp_process_rx_ts(struct mchp_ptp_clock *ptp_clock)
> > +{
> > +     struct phy_device *phydev = ptp_clock->phydev;
> > +     int caps;
> > +
> > +     do {
> > +             struct mchp_ptp_rx_ts *rx_ts;
> > +
> > +             rx_ts = mchp_ptp_get_rx_ts(ptp_clock);
> > +             if (rx_ts)
> > +                     mchp_ptp_match_rx_ts(ptp_clock, rx_ts);
> > +
> > +             caps = phy_read_mmd(phydev, PTP_MMD(ptp_clock),
> > +                                 MCHP_PTP_CAP_INFO(BASE_PORT(ptp_clock)));
> > +             if (caps < 0)
> > +                     return;
> > +     } while (MCHP_PTP_RX_TS_CNT(caps) > 0); }
> > +