netdev - Re: [PATCH net-next 5/8] ice: register 1588 PTP clock device object for E810 devices

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: <20210614134802.633be4c5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 14 Jun 2021 13:48:02 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     "Keller, Jacob E" <jacob.e.keller@...el.com>
Cc:     "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        Richard Cochran <richardcochran@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        "Brelinski, TonyX" <tonyx.brelinski@...el.com>
Subject: Re: [PATCH net-next 5/8] ice: register 1588 PTP clock device object
 for E810 devices

On Mon, 14 Jun 2021 19:50:23 +0000 Keller, Jacob E wrote:
> > > Hmmm.. I thought ppb was a s64, not an s32.
> > >
> > > In general, I believe max_adj is usually capped at 1 billion anyways,
> > > since it doesn't make sense to slow a clock by more than 1billioln ppb,
> > > and increasing it more than that isn't really useful either.  
> > 
> > Do you mean it's capped somewhere in the code to 1B?
> > 
> > I'm no time expert but this is not probability where 1 is a magic
> > value, adjusting clock by 1 - 1ppb vs 1 + 1ppb makes little difference,
> > no? Both mean something is super fishy with the nominal or expected
> > frequency, but the hardware can do that and more.
> > 
> > Flipping the question, if adjusting by large ppb values is not correct,
> > why not cap the adjustment at the value which would prevent the u64
> > overflow?  
> 
> Large ppb values are sometimes used when you want to slew a clock to
> bring it in sync when its a few milliseconds to seconds off, without
> performing a time jump (so that you maintain monotonic increasing
> time).

Ah, you're right, ptp4l will explicitly cap the freq adjustments
based on max_adj from sysfs, so setting max_adj too low could impact
the convergence time in strange scenarios.

> That being said, we are supposed to be checking max_adj, except that
> you're right the conversion to ppb could overflow, and there's no
> check prior to the conversion from scaled_ppm to ppb.