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:   Thu, 05 May 2022 10:25:44 -0700
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Yannick Vignon <yannick.vignon@....com>,
        Michael Walle <michael@...le.cc>
Subject: Re: [RFC PATCH net] net/sched: taprio: account for L1 overhead when
 calculating transmit time

Hi Vladimir,

Vladimir Oltean <vladimir.oltean@....com> writes:

> The taprio scheduler underestimates the packet transmission time, which
> means that packets can be scheduled for transmission in time slots in
> which they are never going to fit.
>
> When this function was added in commit 4cfd5779bd6e ("taprio: Add
> support for txtime-assist mode"), the only implication was that time
> triggered packets would overrun its time slot and eat from the next one,
> because with txtime-assist there isn't really any emulation of a "gate
> close" event that would stop a packet from being transmitted.
>
> However, commit b5b73b26b3ca ("taprio: Fix allowing too small
> intervals") started using this function too, in all modes of operation
> (software, txtime-assist and full offload). So we now accept time slots
> which we know we won't be ever able to fulfill.
>
> It's difficult to say which issue is more pressing, I'd say both are
> visible with testing, even though the second would be more obvious
> because of a black&white result (trying to send small packets in an
> insufficiently large window blocks the queue).
>
> Issue found through code inspection, the code was not even compile
> tested.
>
> The L1 overhead chosen here is an approximation, because various network
> equipment has configurable IFG, however I don't think Linux is aware of
> this.

When testing CBS, I remember using tc-stab: 

https://man7.org/linux/man-pages/man8/tc-stab.8.html

To set the 'overhead' to some value.

That value should be used in the calculation.

I agree that it's not ideal, in the ideal world we would have a way to
retrieve the link overhead from the netdevice. But I would think that it
gets complicated really quickly when using netdevices that are not
Ethernet-based.

>
> Fixes: 4cfd5779bd6e ("taprio: Add support for txtime-assist mode")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  net/sched/sch_taprio.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index b9c71a304d39..8c8681c37d4f 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -176,7 +176,10 @@ static ktime_t get_interval_end_time(struct sched_gate_list *sched,
>  
>  static int length_to_duration(struct taprio_sched *q, int len)
>  {
> -	return div_u64(len * atomic64_read(&q->picos_per_byte), 1000);
> +	/* The duration of frame transmission should account for L1 overhead
> +	 * (12 octets IFG, 7 octets of preamble, 1 octet SFD, 4 octets FCS)
> +	 */
> +	return div_u64((24 + len) * atomic64_read(&q->picos_per_byte), 1000);
>  }
>  
>  /* Returns the entry corresponding to next available interval. If
> -- 
> 2.25.1
>

-- 
Vinicius

Powered by blists - more mailing lists