Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 21 Jun 2023 06:50:47 -0700
From:   Richard Cochran <richardcochran@gmail.com>
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     Horatiu Vultur <horatiu.vultur@microchip.com>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
        davem@davemloft.net, edumazet@google.com, pabeni@redhat.com
Subject: Re: [PATCH net-next v3] net: micrel: Change to receive timestamp in
 the frame for lan8841
Message-ID: <ZJMAN88XYA6drwfe@hoboy.vegasvil.org>
References: <20230615094740.627051-1-horatiu.vultur@microchip.com>
 <20230620113021.01d90f90@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230620113021.01d90f90@kernel.org>
Precedence: bulk

On Tue, Jun 20, 2023 at 11:30:21AM -0700, Jakub Kicinski wrote:
> Richard, looks good?

Yes.
 
> I'm not sure if the settime handling is sufficient, some packets may
> still sneak thru both getting stamped with new value of time and use
> old upper bits and get stamped with old bits and use new upper.
> Can we disable timestamping, drain the queue, then change the time
> and enable stamping again?

This can happen with any driver.  The user space stack combines four
time stamps to calculate the PTP client/server offset.  When setting
the clock, some skbs might be in the kernel's Rx path with the "old"
timestamp.

The kernel doesn't track Rx skbs that have HW timestamps, and stopping
the MAC and then flushing _everything_ seems heavy handed to me.

In the user space PTP stack, setting the clock happens immediately
after Sync message reception.  For configurations with a very high
Sync rate, a linuxptp client has an option to skip a given number of
messages after setting the clock.  In this way you can avoid stale
timestamps by choosing a duration that is longer than the longest
frame residence time from MAC to kernel.

Thanks,
Richard