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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <874kf111nj.fsf@vcostago-mobl2.amr.corp.intel.com>
Date:   Mon, 17 May 2021 14:38:40 -0700
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Michael Walle <michael@...le.cc>,
        Yannick Vignon <yannick.vignon@....nxp.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        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>
Subject: Re: [PATCH net v1] net: taprio offload: enforce qdisc to netdev
 queue mapping

Michael Walle <michael@...le.cc> writes:

>>> 
>>> At least the hardware that I am familiar with doesn't reorder packets.
>>> 
>>> For TXTIME, we have ETF (the qdisc) that re-order packets. The way
>>> things work when taprio and ETF are used together is something like
>>> this: taprio only has enough knowledge about TXTIME to drop packets 
>>> that
>>> would be transmitted outside their "transmission window" (e.g. for
>>> traffic class 0 the transmission window is only for 10 to 50, the 
>>> TXTIME
>>> for a packet is 60, this packet is "invalid" and is dropped). And then
>>> when the packet is enqueued to the "child" ETF, it's re-ordered and 
>>> then
>>> sent to the driver.
>>> 
>>> And this is something that this patch breaks, the ability of dropping
>>> those invalid packets (I really wouldn't like to do this verification
>>> inside our drivers). Thanks for noticing this.
>
> Is this really how the taprio should behave? I mean, should the frame
> really be dropped by taprio if TXTIME is outside of the window? Why
> would taprio bother with TXTIME at all?

Yeah, I understand what you are saying, I also didn't like the idea of
having TXTIME knowledge inside taprio. This trade off was made because
the HW we have is very... particular about the launchtime, if the
launchtime for a packet ends after the "close" time for that queue, that
queue can end up stuck (for multiple seconds in some extreme cases).

The idea is that other vendors could be as particular.

Also, what helped convince me was that txtime is a feature of the
socket/skb, and if taprio/whatever can use it to make the driver life's
easier, then let's use it, that is

>
>> Hmm, indeed, I missed that check (we don't use ETF currently). I'm not
>> sure of the best way forward, but here are a few thoughts:
>> . The problem only arises for full-offload taprio, not for the
>> software or TxTime-assisted cases.
>> . I'm not sure mixing taprio(full-offload) with etf(no-offload) is
>> very useful, at least with small gate intervals: it's likely you will
>> miss your window when trying to send a packet at exactly the right
>> time in software (I am usually testing taprio with a 2ms period and a
>> 4µs interval for the RT stream).
>> . That leaves the case of taprio(full-offload) with etf(offload).
>> Right now with the current stmmac driver config, a packet whose tstamp
>> is outside its gate interval will be sent on the next interval (and
>> block the queue).
>
> If both are offloaded, are the h/w queues reordered if there is a
> new frame with an earlier TXTIME? Or will the queue be blocked by a
> frame with a later transmission time?

Even if offloaded ETF will re-order packets. In bit more detail, ETF
will re-order packets if their launchtime is more than "delta"
nanoseconds in the future.

>
> TBH I've never understood why the ETF qdisc will drop frames with
> TXTIME in the past. I mean it makes sense with hardware offloading. But
> without it, how can you make sure the kernel will wake up the queue
> at just the right time to be able to send it. You can juggle the delta
> parameter but on you don't want to send to too early, but on the other
> hand the frame will likely be dropped if delta is too small. What am
> I misssing here?

I don't think you are missing anything.

Just some background, ETF was written thinking mostly about it being
offloaded, the non-offloaded/"software" mode was a best-effort
implementation of that idea.

That is, If you have use cases for non-offloaded ETF and it work better
if it didn't drop "late" packets, I would be happy hear more.


Cheers,
-- 
Vinicius