linux-kernel - Re: [PATCH net-next] net: micrel: Change to receive timestamp in the frame for lan8841

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]

Date:   Wed, 7 Jun 2023 22:51:52 -0700
From:   Richard Cochran <richardcochran@...il.com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com
Subject: Re: [PATCH net-next] net: micrel: Change to receive timestamp in the
 frame for lan8841

On Wed, Jun 07, 2023 at 10:47:16AM -0700, Richard Cochran wrote:
> On Wed, Jun 07, 2023 at 09:09:48AM +0200, Horatiu Vultur wrote:
> > +static long lan8841_ptp_do_aux_work(struct ptp_clock_info *ptp)
> > +{
> > +	struct kszphy_ptp_priv *ptp_priv = container_of(ptp, struct kszphy_ptp_priv,
> > +							ptp_clock_info);
> > +	struct skb_shared_hwtstamps *shhwtstamps;
> > +	struct timespec64 ts;
> > +	struct sk_buff *skb;
> > +	u32 ts_header;
> > +
> > +	while ((skb = skb_dequeue(&ptp_priv->rx_queue)) != NULL) {
> > +		lan8841_ptp_getseconds(ptp, &ts);
> 
> No need to call this once per frame.  It would be sufficent to call it
> once every 2 seconds and cache the result.

Okay, this is tricky.

- If you call lan8841_ptp_getseconds() after gathering the received
  frames, then the frame timestamps are clearly in the past WRT the
  call to getseconds.  That makes the wrap check simpler.  But the
  getseconds call really should be placed before the 'while' loop.

- If the Rx frame rate exceeds 1/second, then it would be more
  efficient to call getseconds every second, and cache the result.
  But then the wrap around check needs to account for the fact that
  the cached value may have occurred either before or after the frame
  timestamp.

I'll explain that second point when my brain wakes again...

Thanks,
Richard