netdev - Re: [PATCH v3 net] enetc: Let the hardware auto-advance the taprio base-time of 0

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] [day] [month] [year] [list]

Message-ID: <20201125123803.02c1f508@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 25 Nov 2020 12:38:03 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Po Liu <po.liu@....com>
Cc:     Vladimir Oltean <vladimir.oltean@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Vinicius Costa Gomes <vinicius.gomes@...el.com>
Subject: Re: [PATCH v3 net] enetc: Let the hardware auto-advance the taprio
 base-time of 0

On Wed, 25 Nov 2020 02:28:27 +0000 Po Liu wrote:
> > The tc-taprio base time indicates the beginning of the tc-taprio schedule,
> > which is cyclic by definition (where the length of the cycle in nanoseconds
> > is called the cycle time). The base time is a 64-bit PTP time in the TAI
> > domain.
> > 
> > Logically, the base-time should be a future time. But that imposes some
> > restrictions to user space, which has to retrieve the current PTP time from
> > the NIC first, then calculate a base time that will still be larger than the
> > base time by the time the kernel driver programs this value into the
> > hardware. Actually ensuring that the programmed base time is in the
> > future is still a problem even if the kernel alone deals with this.
> > 
> > Luckily, the enetc hardware already advances a base-time that is in the
> > past into a congruent time in the immediate future, according to the same
> > formula that can be found in the software implementation of taprio (in
> > taprio_get_start_time):
> > 
> > 	/* Schedule the start time for the beginning of the next
> > 	 * cycle.
> > 	 */
> > 	n = div64_s64(ktime_sub_ns(now, base), cycle);
> > 	*start = ktime_add_ns(base, (n + 1) * cycle);
> > 
> > There's only one problem: the driver doesn't let the hardware do that.
> > It interferes with the base-time passed from user space, by special-casing
> > the situation when the base-time is zero, and replaces that with the
> > current PTP time. This changes the intended effective base-time of the
> > schedule, which will in the end have a different phase offset than if the
> > base-time of 0.000000000 was to be advanced by an integer multiple of
> > the cycle-time.
> > 
> > Fixes: 34c6adf1977b ("enetc: Configure the Time-Aware Scheduler via tc-
> > taprio offload")
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
>
> It makes sense to me for this patch. Thanks!

Applied, thanks!