Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
To:     Michael Walle <michael@walle.cc>,
        Vladimir Oltean <vladimir.oltean@nxp.com>
CC:     Vladimir Oltean <olteanv@gmail.com>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        "alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
        "allan.nielsen@microchip.com" <allan.nielsen@microchip.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "idosch@mellanox.com" <idosch@mellanox.com>,
        "joergen.andreasen@microchip.com" <joergen.andreasen@microchip.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Po Liu <po.liu@nxp.com>,
        "vinicius.gomes@intel.com" <vinicius.gomes@intel.com>
Subject: RE: [EXT] Re: [net-next] net: dsa: felix: disable always guard band
 bit for TAS config
Thread-Topic: [EXT] Re: [net-next] net: dsa: felix: disable always guard band
 bit for TAS config
Thread-Index: AQHXQS0PuJYBaI7R9kW6JDEEKsgTEqrWdByAgAEnPZA=
Date:   Fri, 7 May 2021 07:16:16 +0000
Message-ID: <DB8PR04MB5785A6A773FEA4F3E0E77698F0579@DB8PR04MB5785.eurprd04.prod.outlook.com>
References: <20210419102530.20361-1-xiaoliang.yang_1@nxp.com>
 <20210504170514.10729-1-michael@walle.cc>
 <20210504181833.w2pecbp2qpuiactv@skbuf>
 <c7618025da6723418c56a54fe4683bd7@walle.cc>
 <20210504185040.ftkub3ropuacmyel@skbuf>
 <ccb40b7fd18b51ecfc3f849a47378c54@walle.cc>
 <20210504191739.73oejybqb6z7dlxr@skbuf>
 <d933eef300cb1e1db7d36ca2cb876ef6@walle.cc>
 <20210504213259.l5rbnyhxrrbkykyg@skbuf>
 <efe5ac03ceddc8ff472144b5fe9fd046@walle.cc>
In-Reply-To: <efe5ac03ceddc8ff472144b5fe9fd046@walle.cc>
Accept-Language: zh-CN, en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6c8ce14d-db9b-49a5-db89-08d911280180
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 May 2021 07:16:17.0182
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: nh35EXFs551pqmmCXn/Vz7FtsTALsIEoT4UJQRxaRqvEgcSGSl+4lG4dlGFMPMciZvFaL4Vc09KlcuTjjYv+LzHQ14f8OyMhVYtX1biqqVQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB5193
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Michael,

On 2021-05-06 21:25, Michael Walle <michael@walle.cc> wrote:
> Am 2021-05-04 23:33, schrieb Vladimir Oltean:
> > [ trimmed the CC list, as this is most likely spam for most people ]
> >
> > On Tue, May 04, 2021 at 10:23:11PM +0200, Michael Walle wrote:
> >> Am 2021-05-04 21:17, schrieb Vladimir Oltean:
> >> > On Tue, May 04, 2021 at 09:08:00PM +0200, Michael Walle wrote:
> >> > > > > > > As explained in another mail in this thread, all queues
> >> > > > > > > are marked as scheduled. So this is actually a no-op,
> >> > > > > > > correct? It doesn't matter if it set or not set for now. D=
unno why
> we even care for this bit then.
> >> > > > > >
> >> > > > > > It matters because ALWAYS_GUARD_BAND_SCH_Q reduces the
> >> > > > > > available throughput when set.
> >> > > > >
> >> > > > > Ahh, I see now. All queues are "scheduled" but the guard band
> >> > > > > only applies for "non-scheduled" -> "scheduled" transitions.
> >> > > > > So the guard band is never applied, right? Is that really
> >> > > > > what we want?
> >> > > >
> >> > > > Xiaoliang explained that yes, this is what we want. If the end
> >> > > > user wants a guard band they can explicitly add a "sched-entry
> >> > > > 00" in the tc-taprio config.
> >> > >
> >> > > You're disabling the guard band, then. I figured, but isn't that
> >> > > suprising for the user? Who else implements taprio? Do they do it
> >> > > in the same way? I mean this behavior is passed right to the
> >> > > userspace and have a direct impact on how it is configured. Of
> >> > > course a user can add it manually, but I'm not sure that is what
> >> > > we want here. At least it needs to be documented somewhere. Or
> >> > > maybe it should be a switchable option.
> >> > >
> >> > > Consider the following:
> >> > > sched-entry S 01 25000
> >> > > sched-entry S fe 175000
> >> > > basetime 0
> >> > >
> >> > > Doesn't guarantee, that queue 0 is available at the beginning of
> >> > > the cycle, in the worst case it takes up to <begin of cycle> +
> >> > > ~12.5us until the frame makes it through (given gigabit and 1518b
> >> > > frames).
> >> > >
> >> > > Btw. there are also other implementations which don't need a
> >> > > guard band (because they are store-and-forward and cound the
> >> > > remaining bytes). So yes, using a guard band and scheduling is
> >> > > degrading the performance.
> >> >
> >> > What is surprising for the user, and I mentioned this already in
> >> > another thread on this patch, is that the Felix switch overruns the
> >> > time gate (a packet taking 2 us to transmit will start transmission
> >> > even if there is only 1 us left of its time slot, delaying the
> >> > packets from the next time slot by 1 us). I guess that this is why
> >> > the ALWAYS_GUARD_BAND_SCH_Q bit exists, as a way to avoid these
> >> > overruns, but it is a bit of a poor tool for that job. Anyway,
> >> > right now we disable it and live with the overruns.
> >>
> >> We are talking about the same thing here. Why is that a poor tool?
> >
> > It is a poor tool because it revolves around the idea of "scheduled
> > queues" and "non-scheduled queues".
> >
> > Consider the following tc-taprio schedule:
> >
> >       sched-entry S 81 2000 # TC 7 and 0 open, all others closed
> >       sched-entry S 82 2000 # TC 7 and 1 open, all others closed
> >       sched-entry S 84 2000 # TC 7 and 2 open, all others closed
> >       sched-entry S 88 2000 # TC 7 and 3 open, all others closed
> >       sched-entry S 90 2000 # TC 7 and 4 open, all others closed
> >       sched-entry S a0 2000 # TC 7 and 5 open, all others closed
> >       sched-entry S c0 2000 # TC 7 and 6 open, all others closed
> >
> > Otherwise said, traffic class 7 should be able to send any time it
> > wishes.
>=20
> What is the use case behind that? TC7 (with the highest priority) may alw=
ays
> take precedence of the other TCs, thus what is the point of having a dedi=
cated
> window for the others.
>=20
> Anyway, I've tried it and there are no hiccups. I've meassured the delta
> between the start of successive packets and they are always ~12370ns for =
a
> 1518b frame. TC7 is open all the time, which makes sense. It only happens=
 if
> you actually close the gate, eg. you have a sched-entry where a TC7 bit i=
s not
> set. In this case, I can see a difference between ALWAYS_GUARD_BAND_SCH_Q
> set and not set. If it is set, there is up to a ~12.5us delay added (of c=
ourse it
> depends on when the former frame was scheduled).
>=20
> It seems that also needs to be 1->0 transition.
>=20
> You've already mentioned that the switch violates the Qbv standard.
> What makes you think so? IMHO before that patch, it wasn't violated.
> Now it likely is (still have to confirm that). How can this be reasonable=
?
>=20
> If you have a look at the initial commit message, it is about making it p=
ossible
> to have a smaller gate window, but that is not possible because of the cu=
rrent
> guard band of ~12.5us. It seems to be a shortcut for not having the MAXSD=
U
> (and thus the length of the guard band) configurable. Yes (static) guard =
bands
> will have a performance impact, also described in [1]. You are trading th=
e
> correctness of the TAS for performance. And it is the sole purpose of Qbv=
 to
> have a determisitc way (in terms of timing) of sending the frames.
>=20
> And telling the user, hey, we know we violate the Qbv standard, please in=
sert
> the guard bands yourself if you really need them is not a real solution. =
As
> already mentioned, (1) it is not documented anywhere, (2) can't be shared
> among other switches (unless they do the same workaround) and (3) what am
> I supposed to do for TSN compliance testing. Modifying the schedule that =
is
> about to be checked (and thus given by the compliance suite)?
>
I disable the always guard band bit because each gate control list needs to=
 reserve a guard band slot, which affects performance. The user does not ne=
ed to set a guard band for each queue transmission. For example, "sched-ent=
ry S 01 2000 sched-entry S fe 98000". Queue 0 is protected traffic and has =
smaller frames, so there is no need to reserve a guard band during the open=
 time of queue 0. The user can add the following guard band before protecte=
d traffic: "sched-entry S 00 25000 sched-entry S 01 2000 sched-entry S fe 7=
3000"

I checked other devices such as ENETC and it can calculate how long the fra=
me transmission will take and determine whether to transmit before the gate=
 is closed. The VSC9959 device does not have this hardware function. If we =
implement guard bands on each queue, we need to reserve a 12.5us guard band=
 for each GCL, even if it only needs to send a small packet. This confuses =
customers.

actually, I'm not sure if this will violate the Qbv standard. I looked up t=
he Qbv standard spec, and found it only introduce the guard band before pro=
tected window (Annex Q (informative)Traffic scheduling). It allows to desig=
n the schedule to accommodate the intended pattern of transmission without =
overrunning the next gate-close event for the traffic classes concerned.

Thanks,
Xiaoliang