| 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
| ||
|
Message-ID: <SJ1PR11MB62975DA6963EA9DE24ABD9EB9B8DA@SJ1PR11MB6297.namprd11.prod.outlook.com>
Date: Fri, 16 Jan 2026 18:31:43 +0000
From: "Salin, Samuel" <samuel.salin@...el.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, "Keller, Jacob E"
<jacob.e.keller@...el.com>, "Kitszel, Przemyslaw"
<przemyslaw.kitszel@...el.com>, Mina Almasry <almasrymina@...gle.com>
CC: "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, Andrew Lunn
<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, Eric Dumazet
<edumazet@...gle.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, "Rizzo,
Luigi" <lrizzo@...gle.com>, "namangulati@...gle.com"
<namangulati@...gle.com>, "willemb@...gle.com" <willemb@...gle.com>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"Olech, Milena" <milena.olech@...el.com>, Shachar Raindel
<shacharr@...gle.com>
Subject: RE: [Intel-wired-lan] [PATCH net v1] idpf: read lower clock bits
inside the time sandwich
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> Loktionov, Aleksandr
> Sent: Friday, December 12, 2025 1:08 PM
> To: Keller, Jacob E <jacob.e.keller@...el.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@...el.com>; Mina Almasry <almasrymina@...gle.com>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Andrew Lunn
> <andrew+netdev@...n.ch>; David S. Miller <davem@...emloft.net>;
> netdev@...r.kernel.org; Eric Dumazet <edumazet@...gle.com>; linux-
> kernel@...r.kernel.org; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> <pabeni@...hat.com>; Richard Cochran <richardcochran@...il.com>; Rizzo,
> Luigi <lrizzo@...gle.com>; namangulati@...gle.com; willemb@...gle.com;
> intel-wired-lan@...ts.osuosl.org; Olech, Milena <milena.olech@...el.com>;
> Shachar Raindel <shacharr@...gle.com>
> Subject: Re: [Intel-wired-lan] [PATCH net v1] idpf: read lower clock bits inside
> the time sandwich
>
>
>
> > -----Original Message-----
> > From: Keller, Jacob E <jacob.e.keller@...el.com>
> > Sent: Friday, December 12, 2025 8:43 PM
> > To: Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>; Loktionov,
> > Aleksandr <aleksandr.loktionov@...el.com>; Mina Almasry
> > <almasrymina@...gle.com>
> > Cc: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Andrew Lunn
> > <andrew+netdev@...n.ch>; David S. Miller <davem@...emloft.net>;
> > netdev@...r.kernel.org; Eric Dumazet <edumazet@...gle.com>; linux-
> > kernel@...r.kernel.org; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> > <pabeni@...hat.com>; Richard Cochran <richardcochran@...il.com>;
> > Rizzo, Luigi <lrizzo@...gle.com>; namangulati@...gle.com;
> > willemb@...gle.com; intel-wired-lan@...ts.osuosl.org; Olech, Milena
> > <milena.olech@...el.com>; Shachar Raindel <shacharr@...gle.com>
> > Subject: Re: [Intel-wired-lan] [PATCH net v1] idpf: read lower clock
> > bits inside the time sandwich
> >
> >
> >
> > On 12/11/2025 11:57 PM, Przemek Kitszel wrote:
> > > On 12/11/25 23:06, Jacob Keller wrote:
> > >>
> > >>
> > >> On 12/11/2025 2:37 AM, Loktionov, Aleksandr wrote:
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On
> > >>>> Behalf Of Mina Almasry
> > >>>> Sent: Thursday, December 11, 2025 11:19 AM
> > >>>> To: netdev@...r.kernel.org; linux-kernel@...r.kernel.org
> > >>>> Cc: Mina Almasry <almasrymina@...gle.com>; Nguyen, Anthony L
> > >>>> <anthony.l.nguyen@...el.com>; Kitszel, Przemyslaw
> > >>>> <przemyslaw.kitszel@...el.com>; Andrew Lunn
> > >>>> <andrew+netdev@...n.ch>; David S. Miller <davem@...emloft.net>;
> > >>>> Eric Dumazet <edumazet@...gle.com>; Jakub Kicinski
> > >>>> <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Richard
> > Cochran
> > >>>> <richardcochran@...il.com>; Rizzo, Luigi <lrizzo@...gle.com>;
> > >>>> namangulati@...gle.com; willemb@...gle.com;
> > >>>> intel-wired-lan@...ts.osuosl.org; Olech, Milena
> > >>>> <milena.olech@...el.com>; Keller, Jacob E
> > >>>> <jacob.e.keller@...el.com>; Shachar Raindel <shacharr@...gle.com>
> > >>>> Subject: [Intel-wired-lan] [PATCH net v1] idpf: read lower clock
> > >>>> bits inside the time sandwich
> > >>>>
> > >>>> PCIe reads need to be done inside the time sandwich because PCIe
> > >>>> writes may get buffered in the PCIe fabric and posted to the
> > device
> > >>>> after the _postts completes. Doing the PCIe read inside the time
> > >>>> sandwich guarantees that the write gets flushed before the
> > _postts
> > >>>> timestamp is taken.
> > >>>>
> > >>>> Cc: lrizzo@...gle.com
> > >>>> Cc: namangulati@...gle.com
> > >>>> Cc: willemb@...gle.com
> > >>>> Cc: intel-wired-lan@...ts.osuosl.org
> > >>>> Cc: milena.olech@...el.com
> > >>>> Cc: jacob.e.keller@...el.com
> > >>>>
> > >>>> Fixes: 5cb8805d2366 ("idpf: negotiate PTP capabilities and get
> > PTP
> > >>>> clock")
> > >>>> Suggested-by: Shachar Raindel <shacharr@...gle.com>
> > >>>> Signed-off-by: Mina Almasry <almasrymina@...gle.com>
> > >>>> ---
> > >>>> drivers/net/ethernet/intel/idpf/idpf_ptp.c | 2 +-
> > >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> > >>>> b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> > >>>> index 3e1052d070cf..0a8b50350b86 100644
> > >>>> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> > >>>> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> > >>>> @@ -108,11 +108,11 @@ static u64
> > >>>> idpf_ptp_read_src_clk_reg_direct(struct idpf_adapter *adapter,
> > >>>> ptp_read_system_prets(sts);
> > >>>>
> > >>>> idpf_ptp_enable_shtime(adapter);
> > >>>> + lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
> > >>> The high 32 bits (hi) are still read outside the time sandwich
> > >>> (after ptp_read_system_postts()), which defeats the stated purpose
> > of ensuring PCIe write flush before timestamp capture.
> > >>> /* I think he "time sandwich" is defined by the region between
> > ptp_read_system_prets(sts) and ptp_read_system_postts(sts) */ Isn't
> > it?
> > >>>
> > >>>
> > >>
> > >> Any read will cause writes to flush, so we don't need to move both
> > >> registers.
> > >>
> > >> The point here is that we write to the shadow register to snapshot
> > >> time, and it won't guarantee to be flushed to the device until a
> > >> read. By moving a single read in side the time sandwhich, we ensure
> > >> that its actually complete before the time snapshot is taken. We
> > >> don't need to wait for both registers because of the snapshot
> > behavior.
> > >
> > > very nice explanation Jake, thank you
> > >
> > > I don't know if it should be considered "basic common knowledge", or
> > > warrants an entry in commit message/code comment For sure we don't
> > > want anyone not knowing that to touch the code, so barrier to entry
> > is
> > > also a good thing ;)
> > >
> > >>
> > >> I think the patch is fine-as-is.
> > >
> > > given the scope of the function, agree
> > >
> > Reading both registers would take longer, and would delay post
> > timestamp, which would lower the accuracy of the clock comparison
> > mechanisms that use the pre+post timestamps. We *must* read one of the
> > values because we need to ensure the PHC timestamp is snapshot between
> > pre+post, but we should do as little work as necessary, so only
> > reading
> > the low register is the most optimal.
> >
> > This could be put into the commit message, but at least to me as a
> > domain expert the original commit message was sufficient, so I'm not
> > sure that I'm a good judge for what is necessary for others to
> > understand the logic.
>
> Thank you for the clarification!
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>
>
>
Tested-by: Samuel Salin <Samuel.salin@...el.com>
Powered by blists - more mailing lists