netdev - Re: [PATCH net v1] net: taprio offload: enforce qdisc to netdev queue mapping

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 14 May 2021 14:01:54 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc:     Yannick Vignon <yannick.vignon@....nxp.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Joakim Zhang <qiangqing.zhang@....com>,
        sebastien.laveze@....nxp.com,
        Yannick Vignon <yannick.vignon@....com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>,
        Vladimir Oltean <olteanv@...il.com>,
        Vedang Patel <vedang.patel@...el.com>,
        Michael Walle <michael@...le.cc>
Subject: Re: [PATCH net v1] net: taprio offload: enforce qdisc to netdev
 queue mapping

On Fri, 14 May 2021 13:40:58 -0700 Vinicius Costa Gomes wrote:
> Jakub Kicinski <kuba@...nel.org> writes:
> > You haven't CCed anyone who worked on this Qdisc in the last 2 years :/
> > CCing them now. Comments, anyone?  
> 
> I guess I should suggest myself as maintainer, to reduce chances of this
> happening again.

Yes, please.

> > This looks like a very drastic change. Are you expecting the qdisc will
> > always be bypassed?  
> 
> Only when running in full offload mode it will be bypassed.
> 
> And it's kind of by design, in offload mode, the idea was: configure the
> netdev traffic class to queue mapping, send the schedule to the hardware
> and stay out of the way.
> 
> But as per Yannick's report, it seems that taprio doesn't stay enough
> out of the yay.
> 
> > After a 1 minute looks it seems like taprio is using device queues in
> > strict priority fashion. Maybe a different model is needed, but a qdisc
> > with:
> >
> > enqueue()
> > {
> > 	WARN_ONCE(1)
> > }
> >
> > really doesn't look right to me.  
> 
> This patch takes the "stay out of the way" to the extreme, I kind of
> like it/I am not opposed to it, if I had this idea a couple of years
> ago, perhaps I would have used this same approach.

Sorry for my ignorance, but for TXTIME is the hardware capable of
reordering or the user is supposed to know how to send packets?

My biggest problem with this patch is that unless the application is
very careful that WARN_ON_ONCE(1) will trigger. E.g. if softirq is
servicing the queue when the application sends - the qdisc will not 
be bypassed, right?

> I am now thinking if this idea locks us out of anything.
> 
> Anyway, a nicer alternative would exist if we had a way to tell the core
> "this qdisc should be bypassed" (i.e. don't call enqueue()/dequeue())
> after init() runs.

I don't think calling enqueue() and dequeue() is a problem. The problem
is that RT process does unrelated work.

> > Quoting the rest of the patch below for the benefit of those on CC.