| Message ID | 060ba6e2de48763aec25df3ed87b64f86022f8b1.1576591746.git.Jose.Abreu@synopsys.com |
|---|---|
| State | Deferred |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] taprio: Add support for the SetAndHold and SetAndRelease commands | expand |
Hi Jose, Quoting Jose Abreu (2019-12-17 06:10:24) > Although this is already in kernel, currently the tool does not support > them. We need these commands for full TSN features which are currently > supported in Synopsys IPs such as QoS and XGMAC3. > > Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> > --- > Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com> > Cc: David Ahern <dsahern@gmail.com> > --- > tc/q_taprio.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tc/q_taprio.c b/tc/q_taprio.c > index b9954436b0f9..62ff860e80ae 100644 > --- a/tc/q_taprio.c > +++ b/tc/q_taprio.c > @@ -99,6 +99,10 @@ static const char *entry_cmd_to_str(__u8 cmd) > switch (cmd) { > case TC_TAPRIO_CMD_SET_GATES: > return "S"; > + case TC_TAPRIO_CMD_SET_AND_HOLD: > + return "H"; > + case TC_TAPRIO_CMD_SET_AND_RELEASE: > + return "R"; > default: > return "Invalid"; > } > @@ -108,6 +112,10 @@ static int str_to_entry_cmd(const char *str) > { > if (strcmp(str, "S") == 0) > return TC_TAPRIO_CMD_SET_GATES; > + if (strcmp(str, "H") == 0) > + return TC_TAPRIO_CMD_SET_AND_HOLD; > + if (strcmp(str, "R") == 0) > + return TC_TAPRIO_CMD_SET_AND_RELEASE; > > return -1; > } From the commit message, I had the impression that HOLD and RELEASE commands are supported by QoS and XGMAC3. However, in '[PATCH net-next v2 7/7] net: stmmac: Integrate EST with TAPRIO scheduler API' I see: + if (qopt->entries[i].command != TC_TAPRIO_CMD_SET_GATES) + return -EOPNOTSUPP; Am I missing something or these commands are indeed not supported by Synopsys IPs? Anyhow, this patch makes sense to me. Regards, Andre
From: Andre Guedes <andre.guedes@linux.intel.com> Date: Dec/17/2019, 21:25:36 (UTC+00:00) > From the commit message, I had the impression that HOLD and RELEASE commands > are supported by QoS and XGMAC3. However, in '[PATCH net-next v2 7/7] net: > stmmac: Integrate EST with TAPRIO scheduler API' I see: > > + if (qopt->entries[i].command != TC_TAPRIO_CMD_SET_GATES) > + return -EOPNOTSUPP; > > Am I missing something or these commands are indeed not supported by Synopsys > IPs? Its already supported internally but I just didn't add it in the series. I'll probably sent the EST and FP support in a different series today. > Anyhow, this patch makes sense to me. Thanks! --- Thanks, Jose Miguel Abreu
Hi Jose, Jose Abreu <Jose.Abreu@synopsys.com> writes: > Although this is already in kernel, currently the tool does not support > them. We need these commands for full TSN features which are currently > supported in Synopsys IPs such as QoS and XGMAC3. > > Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> This patch looks good in itself. However, I feel that this is incomplete. At least the way I understand things, without specifying which traffic classes are going to be preemptible (or it's dual concept, express), I don't see how this is going to be used in practice. Or does the hardware have a default configuration, that all traffic classes are preemptible, for example. What am I missing here? Cheers, -- Vinicius
From: Vinicius Costa Gomes <vinicius.gomes@intel.com> Date: Dec/18/2019, 23:05:13 (UTC+00:00) > Hi Jose, > > Jose Abreu <Jose.Abreu@synopsys.com> writes: > > > Although this is already in kernel, currently the tool does not support > > them. We need these commands for full TSN features which are currently > > supported in Synopsys IPs such as QoS and XGMAC3. > > > > Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> > > This patch looks good in itself. > > However, I feel that this is incomplete. At least the way I understand > things, without specifying which traffic classes are going to be > preemptible (or it's dual concept, express), I don't see how this is > going to be used in practice. Or does the hardware have a default > configuration, that all traffic classes are preemptible, for example. > > What am I missing here? On our IPs Queue 0 is by preemptible and all remaining ones are express by default. The way I tested it is quite easy: send traffic from queue 0 and at same time configure EST with SetAndHold for remaining queues. Which means queue 0 traffic will be blocked while remaining ones are sending. --- Thanks, Jose Miguel Abreu
On 12/18/19 4:05 PM, Vinicius Costa Gomes wrote: > Hi Jose, > > Jose Abreu <Jose.Abreu@synopsys.com> writes: > >> Although this is already in kernel, currently the tool does not support >> them. We need these commands for full TSN features which are currently >> supported in Synopsys IPs such as QoS and XGMAC3. >> >> Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> > > This patch looks good in itself. > > However, I feel that this is incomplete. At least the way I understand > things, without specifying which traffic classes are going to be > preemptible (or it's dual concept, express), I don't see how this is > going to be used in practice. Or does the hardware have a default > configuration, that all traffic classes are preemptible, for example. > > What am I missing here? > this patch has been lingering for a while. What's the status? good enough for commit or are changes needed?
Hi Jose, On 12/18/2019 06:08 PM, Jose Abreu wrote: > From: Vinicius Costa Gomes <vinicius.gomes@intel.com> > Date: Dec/18/2019, 23:05:13 (UTC+00:00) > >> Hi Jose, >> >> Jose Abreu <Jose.Abreu@synopsys.com> writes: >> >>> Although this is already in kernel, currently the tool does not support >>> them. We need these commands for full TSN features which are currently >>> supported in Synopsys IPs such as QoS and XGMAC3. >>> >>> Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> >> >> This patch looks good in itself. >> >> However, I feel that this is incomplete. At least the way I understand >> things, without specifying which traffic classes are going to be >> preemptible (or it's dual concept, express), I don't see how this is >> going to be used in practice. Or does the hardware have a default >> configuration, that all traffic classes are preemptible, for example. >> >> What am I missing here? > > On our IPs Queue 0 is by preemptible and all remaining ones are express > by default. > > The way I tested it is quite easy: send traffic from queue 0 and at same > time configure EST with SetAndHold for remaining queues. Which means > queue 0 traffic will be blocked while remaining ones are sending. > So you have one sched entry that specify SetAndHold for all remaining queues. So this means, queue 0 will never get sent. I guess you also support SetAndRelease so that a mix of SetAndHold followed by SetAndRelease can be sent to enable sending from Queue 0. Is that correct? Something like sched-entry H 02 300000 \ <=== 300 usec tx from Q1 sched-entry R 01 200000 <=== 300 usec tx from Q0 Just trying to understand how this is being used for real world application. Regards, Murali > --- > Thanks, > Jose Miguel Abreu >
Hi Jose, Quoting Jose Abreu (2019-12-18 15:08:45) > From: Vinicius Costa Gomes <vinicius.gomes@intel.com> > Date: Dec/18/2019, 23:05:13 (UTC+00:00) > > > However, I feel that this is incomplete. At least the way I understand > > things, without specifying which traffic classes are going to be > > preemptible (or it's dual concept, express), I don't see how this is > > going to be used in practice. Or does the hardware have a default > > configuration, that all traffic classes are preemptible, for example. > > > > What am I missing here? > > On our IPs Queue 0 is by preemptible and all remaining ones are express > by default. Is this configuration fixed in your IP or the user can control if a specific queue is preemptible or express? I'm trying to figure out how this discussion relates to the Qbu discussion we're having in "[v1,net-next, 1/2] ethtool: add setting frame preemption of traffic classes". Regards, Andre
From: David Ahern <dsahern@gmail.com> Date: Dec/31/2019, 04:30:54 (UTC+00:00) > this patch has been lingering for a while. What's the status? good > enough for commit or are changes needed? Thanks for asking David. There are some on-going discussions (I'll add you on cc), let's see how that goes. --- Thanks, Jose Miguel Abreu
From: Murali Karicheri <m-karicheri2@ti.com> Date: Jan/03/2020, 22:24:14 (UTC+00:00) > So you have one sched entry that specify SetAndHold for all remaining > queues. So this means, queue 0 will never get sent. I guess you also > support SetAndRelease so that a mix of SetAndHold followed by > SetAndRelease can be sent to enable sending from Queue 0. Is that > correct? > > Something like > sched-entry H 02 300000 \ <=== 300 usec tx from Q1 > sched-entry R 01 200000 <=== 300 usec tx from Q0 > > Just trying to understand how this is being used for real world > application. This is the command I use: # tc qdisc add dev $intf handle 100: parent root taprio \ num_tc 4 \ map 0 1 2 3 3 3 3 3 3 3 3 3 3 3 3 3 \ base-time $base \ cycle-time 1000000 \ sched-entry R 00 100000 \ sched-entry H 02 200000 \ sched-entry H 04 300000 \ sched-entry H 08 400000 \ flags 0x2 # sleep 2 # iperf3 -c <ip> -u -b 0 -t 15 & # sleep 5 # echo "Queue 3: Expected=40%, Queue 0 will now be preempted" # tperf -i <ethX> -p 3 This will basically preempt Queue 0 and flood Queue 3 with express traffic. You can find tperf utility here: https://github.com/joabreu/tperf --- Thanks, Jose Miguel Abreu
From: Andre Guedes <andre.guedes@linux.intel.com> Date: Jan/07/2020, 00:19:17 (UTC+00:00) > > On our IPs Queue 0 is by preemptible and all remaining ones are express > > by default. > > Is this configuration fixed in your IP or the user can control if a specific > queue is preemptible or express? It's configurable for all Queues except 0 which is fixed as preemptible. > I'm trying to figure out how this discussion relates to the Qbu discussion > we're having in "[v1,net-next, 1/2] ethtool: add setting frame preemption of > traffic classes". Hmmm. I think tc utility is the right way to do this, and not ethtool because EST and FP are highly tied ... Do you agree ? --- Thanks, Jose Miguel Abreu
Hi Jose, > > > On our IPs Queue 0 is by preemptible and all remaining ones are express > > > by default. > > > > Is this configuration fixed in your IP or the user can control if a specific > > queue is preemptible or express? > > It's configurable for all Queues except 0 which is fixed as preemptible. Thanks for the clarification. > > I'm trying to figure out how this discussion relates to the Qbu discussion > > we're having in "[v1,net-next, 1/2] ethtool: add setting frame preemption of > > traffic classes". > > Hmmm. > > I think tc utility is the right way to do this, and not ethtool because > EST and FP are highly tied ... Do you agree ? I'm still wrapping my head around this :) See my last reply on that other thread. (BTW, let's concentrate this 'Qbu enabling' discussion there) Thanks, Andre
Hi Jose, On 01/07/2020 04:27 AM, Jose Abreu wrote: > From: Murali Karicheri <m-karicheri2@ti.com> > Date: Jan/03/2020, 22:24:14 > (UTC+00:00) > >> So you have one sched entry that specify SetAndHold for all remaining >> queues. So this means, queue 0 will never get sent. I guess you also >> support SetAndRelease so that a mix of SetAndHold followed by >> SetAndRelease can be sent to enable sending from Queue 0. Is that >> correct? >> >> Something like >> sched-entry H 02 300000 \ <=== 300 usec tx from Q1 >> sched-entry R 01 200000 <=== 300 usec tx from Q0 >> >> Just trying to understand how this is being used for real world >> application. > > This is the command I use: > > # tc qdisc add dev $intf handle 100: parent root taprio \ > num_tc 4 \ > map 0 1 2 3 3 3 3 3 3 3 3 3 3 3 3 3 \ > base-time $base \ > cycle-time 1000000 \ > sched-entry R 00 100000 \ > sched-entry H 02 200000 \ > sched-entry H 04 300000 \ > sched-entry H 08 400000 \ > flags 0x2 > # sleep 2 > # iperf3 -c <ip> -u -b 0 -t 15 & > # sleep 5 > # echo "Queue 3: Expected=40%, Queue 0 will now be preempted" > # tperf -i <ethX> -p 3 > > This will basically preempt Queue 0 and flood Queue 3 with express > traffic. I see you don't include Q0 in your schedule. Why is that the case? Why is the entry with zero mask introduced (first entry)? Typo? I thought it should be like below. No? > # tc qdisc add dev $intf handle 100: parent root taprio \ > num_tc 4 \ > map 0 1 2 3 3 3 3 3 3 3 3 3 3 3 3 3 \ > base-time $base \ > cycle-time 1000000 \ > sched-entry R 01 100000 \ > sched-entry H 02 200000 \ > sched-entry H 04 300000 \ > sched-entry H 08 400000 \ > flags 0x2 Based on my understanding, if holdAdvance is t1 and releaseAdvance is t2, hold is asserted t1 nano second before Q1 slot (during first entry) begins and release is asserted t2 nano second before the start of Q0 slot (during fourth entry) so that pre-emptable frame start transmission during first entry. I thought R/H entries are a replacement for zero mask entry that are introduced in the schedule as a guard band before express queue slot when frame pre-emption not supported. Is my understanding correct? So the above make sense? Regards, Murali > > You can find tperf utility here: https://github.com/joabreu/tperf > > --- > Thanks, > Jose Miguel Abreu >
From: Murali Karicheri <m-karicheri2@ti.com> Date: Jan/17/2020, 22:18:07 (UTC+00:00) > Hi Jose, > > On 01/07/2020 04:27 AM, Jose Abreu wrote: > > From: Murali Karicheri <m-karicheri2@ti.com> > > Date: Jan/03/2020, 22:24:14 > > (UTC+00:00) > > > >> So you have one sched entry that specify SetAndHold for all remaining > >> queues. So this means, queue 0 will never get sent. I guess you also > >> support SetAndRelease so that a mix of SetAndHold followed by > >> SetAndRelease can be sent to enable sending from Queue 0. Is that > >> correct? > >> > >> Something like > >> sched-entry H 02 300000 \ <=== 300 usec tx from Q1 > >> sched-entry R 01 200000 <=== 300 usec tx from Q0 > >> > >> Just trying to understand how this is being used for real world > >> application. > > > > This is the command I use: > > > > # tc qdisc add dev $intf handle 100: parent root taprio \ > > num_tc 4 \ > > map 0 1 2 3 3 3 3 3 3 3 3 3 3 3 3 3 \ > > base-time $base \ > > cycle-time 1000000 \ > > sched-entry R 00 100000 \ > > sched-entry H 02 200000 \ > > sched-entry H 04 300000 \ > > sched-entry H 08 400000 \ > > flags 0x2 > > # sleep 2 > > # iperf3 -c <ip> -u -b 0 -t 15 & > > # sleep 5 > > # echo "Queue 3: Expected=40%, Queue 0 will now be preempted" > > # tperf -i <ethX> -p 3 > > > > This will basically preempt Queue 0 and flood Queue 3 with express > > traffic. > I see you don't include Q0 in your schedule. Why is that the case? > > Why is the entry with zero mask introduced (first entry)? Typo? > I thought it should be like below. No? > > > # tc qdisc add dev $intf handle 100: parent root taprio \ > > num_tc 4 \ > > map 0 1 2 3 3 3 3 3 3 3 3 3 3 3 3 3 \ > > base-time $base \ > > cycle-time 1000000 \ > > sched-entry R 01 100000 \ > > sched-entry H 02 200000 \ > > sched-entry H 04 300000 \ > > sched-entry H 08 400000 \ > > flags 0x2 > > Based on my understanding, if holdAdvance is t1 and releaseAdvance is > t2, hold is asserted t1 nano second before Q1 slot (during first > entry) begins and release is asserted t2 nano second before the start of > Q0 slot (during fourth entry) so that pre-emptable frame start > transmission during first entry. > > I thought R/H entries are a replacement for zero mask entry that > are introduced in the schedule as a guard band before express queue > slot when frame pre-emption not supported. Is my understanding correct? > > So the above make sense? You are right, my example was an old one, sorry. This is what I'm currently using: [...] cycle-time 1000000 \ sched-entry R 02 200000 \ sched-entry R 04 400000 \ sched-entry H 08 400000 \ [...] Notice I don't need to set Queue 0 on sched-entry because its always preemptable queue. Then I test it with: # echo "Queue 0: Exp=60%" # iperf3 -t 15 -c <ip> -Z -A <cpu> & # sleep 5 # echo "Queue 3: Exp=40%" # tperf -i <interface> -p 3 -c <cpu> This will result in: - iperf will have 60% of bandwidth because Queue 0-2 have 400 + 200 = 60% cycle time - tperf will have 40% of bandwidth because Queue 3 has 40% cycle time and is express Queue --- Thanks, Jose Miguel Abreu
diff --git a/tc/q_taprio.c b/tc/q_taprio.c index b9954436b0f9..62ff860e80ae 100644 --- a/tc/q_taprio.c +++ b/tc/q_taprio.c @@ -99,6 +99,10 @@ static const char *entry_cmd_to_str(__u8 cmd) switch (cmd) { case TC_TAPRIO_CMD_SET_GATES: return "S"; + case TC_TAPRIO_CMD_SET_AND_HOLD: + return "H"; + case TC_TAPRIO_CMD_SET_AND_RELEASE: + return "R"; default: return "Invalid"; } @@ -108,6 +112,10 @@ static int str_to_entry_cmd(const char *str) { if (strcmp(str, "S") == 0) return TC_TAPRIO_CMD_SET_GATES; + if (strcmp(str, "H") == 0) + return TC_TAPRIO_CMD_SET_AND_HOLD; + if (strcmp(str, "R") == 0) + return TC_TAPRIO_CMD_SET_AND_RELEASE; return -1; }
Although this is already in kernel, currently the tool does not support them. We need these commands for full TSN features which are currently supported in Synopsys IPs such as QoS and XGMAC3. Signed-off-by: Jose Abreu <Jose.Abreu@synopsys.com> --- Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com> Cc: David Ahern <dsahern@gmail.com> --- tc/q_taprio.c | 8 ++++++++ 1 file changed, 8 insertions(+)