* [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel
@ 2023-08-07 16:08 Vladimir Oltean
2023-08-07 16:08 ` [PATCH iproute2 2/2] tc/taprio: fix JSON output when TCA_TAPRIO_ATTR_ADMIN_SCHED is present Vladimir Oltean
2023-08-07 20:15 ` [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vinicius Costa Gomes
0 siblings, 2 replies; 4+ messages in thread
From: Vladimir Oltean @ 2023-08-07 16:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Vinicius Costa Gomes
When an admin schedule is pending and hasn't yet become operational, the
kernel will report only the parameters of the admin schedule in a nested
TCA_TAPRIO_ATTR_ADMIN_SCHED attribute.
However, we default to printing zeroes even for the parameters of the
operational base time, when that doesn't exist.
Link: https://lore.kernel.org/netdev/87il9w0xx7.fsf@intel.com/
Fixes: 0dd16449356f ("tc: Add support for configuring the taprio scheduler")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
tc/q_taprio.c | 91 ++++++++++++++++++++++++++++++---------------------
1 file changed, 53 insertions(+), 38 deletions(-)
diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index 913197f68caa..795c013c1c2a 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -416,14 +416,11 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
return 0;
}
-static int print_sched_list(FILE *f, struct rtattr *list)
+static void print_sched_list(FILE *f, struct rtattr *list)
{
- struct rtattr *item;
+ struct rtattr *item, *nla;
int rem;
- if (list == NULL)
- return 0;
-
rem = RTA_PAYLOAD(list);
open_json_array(PRINT_JSON, "schedule");
@@ -432,60 +429,78 @@ static int print_sched_list(FILE *f, struct rtattr *list)
for (item = RTA_DATA(list); RTA_OK(item, rem); item = RTA_NEXT(item, rem)) {
struct rtattr *tb[TCA_TAPRIO_SCHED_ENTRY_MAX + 1];
- __u32 index = 0, gatemask = 0, interval = 0;
- __u8 command = 0;
+ __u32 index, gatemask, interval;
+ __u8 command;
parse_rtattr_nested(tb, TCA_TAPRIO_SCHED_ENTRY_MAX, item);
- if (tb[TCA_TAPRIO_SCHED_ENTRY_INDEX])
- index = rta_getattr_u32(tb[TCA_TAPRIO_SCHED_ENTRY_INDEX]);
+ open_json_object(NULL);
+
+ nla = tb[TCA_TAPRIO_SCHED_ENTRY_INDEX];
+ if (nla) {
+ index = rta_getattr_u32(nla);
+ print_uint(PRINT_ANY, "index", "\tindex %u", index);
+ }
- if (tb[TCA_TAPRIO_SCHED_ENTRY_CMD])
- command = rta_getattr_u8(tb[TCA_TAPRIO_SCHED_ENTRY_CMD]);
+ nla = tb[TCA_TAPRIO_SCHED_ENTRY_CMD];
+ if (nla) {
+ command = rta_getattr_u8(nla);
+ print_string(PRINT_ANY, "cmd", " cmd %s",
+ entry_cmd_to_str(command));
+ }
- if (tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK])
- gatemask = rta_getattr_u32(tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK]);
+ nla = tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK];
+ if (nla) {
+ gatemask = rta_getattr_u32(nla);
+ print_0xhex(PRINT_ANY, "gatemask", " gatemask %#llx",
+ gatemask);
+ }
- if (tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL])
- interval = rta_getattr_u32(tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL]);
+ nla = tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL];
+ if (nla) {
+ interval = rta_getattr_u32(nla);
+ print_uint(PRINT_ANY, "interval", " interval %u",
+ interval);
+ }
- open_json_object(NULL);
- print_uint(PRINT_ANY, "index", "\tindex %u", index);
- print_string(PRINT_ANY, "cmd", " cmd %s", entry_cmd_to_str(command));
- print_0xhex(PRINT_ANY, "gatemask", " gatemask %#llx", gatemask);
- print_uint(PRINT_ANY, "interval", " interval %u", interval);
close_json_object();
print_nl();
}
close_json_array(PRINT_ANY, "");
-
- return 0;
}
static int print_schedule(FILE *f, struct rtattr **tb)
{
- int64_t base_time = 0, cycle_time = 0, cycle_time_extension = 0;
-
- if (tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME])
- base_time = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME]);
+ int64_t base_time, cycle_time, cycle_time_extension;
+ struct rtattr *nla;
+
+ nla = tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME];
+ if (nla) {
+ base_time = rta_getattr_s64(nla);
+ print_lluint(PRINT_ANY, "base_time", "\tbase-time %lld",
+ base_time);
+ }
- if (tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME])
+ nla = tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME];
+ if (nla) {
cycle_time = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME]);
+ print_lluint(PRINT_ANY, "cycle_time", " cycle-time %lld",
+ cycle_time);
+ }
- if (tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME_EXTENSION])
- cycle_time_extension = rta_getattr_s64(
- tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME_EXTENSION]);
-
- print_lluint(PRINT_ANY, "base_time", "\tbase-time %lld", base_time);
-
- print_lluint(PRINT_ANY, "cycle_time", " cycle-time %lld", cycle_time);
-
- print_lluint(PRINT_ANY, "cycle_time_extension",
- " cycle-time-extension %lld", cycle_time_extension);
+ nla = tb[TCA_TAPRIO_ATTR_SCHED_CYCLE_TIME_EXTENSION];
+ if (nla) {
+ cycle_time_extension = rta_getattr_s64(nla);
+ print_lluint(PRINT_ANY, "cycle_time_extension",
+ " cycle-time-extension %lld",
+ cycle_time_extension);
+ }
- print_sched_list(f, tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST]);
+ nla = tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST];
+ if (nla)
+ print_sched_list(f, nla);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH iproute2 2/2] tc/taprio: fix JSON output when TCA_TAPRIO_ATTR_ADMIN_SCHED is present
2023-08-07 16:08 [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vladimir Oltean
@ 2023-08-07 16:08 ` Vladimir Oltean
2023-08-07 20:15 ` [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vinicius Costa Gomes
1 sibling, 0 replies; 4+ messages in thread
From: Vladimir Oltean @ 2023-08-07 16:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Vinicius Costa Gomes
When the kernel reports that a configuration change is pending
(and that the schedule is still in the administrative state and
not yet operational), we (tc -j -p qdisc show) produce the following
output:
[ {
"kind": "taprio",
"handle": "8001:",
"root": true,
"refcnt": 9,
"options": {
"tc": 8,
"map": [ 0,1,2,3,4,5,6,7,0,0,0,0,0,0,0,0 ],
"queues": [ {
"offset": 0,
"count": 1
},{
"offset": 1,
"count": 1
},{
"offset": 2,
"count": 1
},{
"offset": 3,
"count": 1
},{
"offset": 4,
"count": 1
},{
"offset": 5,
"count": 1
},{
"offset": 6,
"count": 1
},{
"offset": 7,
"count": 1
} ],
"clockid": "TAI",
"base_time": 0,
"cycle_time": 20000000,
"cycle_time_extension": 0,
"schedule": [ {
"index": 0,
"cmd": "S",
"gatemask": "0xff",
"interval": 20000000
} ],{
"base_time": 1691160103110424418,
"cycle_time": 20000000,
"cycle_time_extension": 0,
"schedule": [ {
"index": 0,
"cmd": "S",
"gatemask": "0xff",
"interval": 20000000
} ]
},
"max-sdu": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ],
"fp": [ "E","E","E","E","E","E","E","E","E","E","E","E","E","E","E","E" ]
}
} ]
which is invalid json, because the second group of "base_time",
"cycle_time", etc etc is placed in an unlabeled sub-object. If we pipe
it into jq, it complains:
parse error: Objects must consist of key:value pairs at line 53, column 14
Since it represents the administrative schedule, give this unnamed JSON
object the "admin" name. We now print valid JSON which looks like this:
[ {
"kind": "taprio",
"handle": "8001:",
"root": true,
"refcnt": 9,
"options": {
"tc": 8,
"map": [ 0,1,2,3,4,5,6,7,0,0,0,0,0,0,0,0 ],
"queues": [ {
"offset": 0,
"count": 1
},{
"offset": 1,
"count": 1
},{
"offset": 2,
"count": 1
},{
"offset": 3,
"count": 1
},{
"offset": 4,
"count": 1
},{
"offset": 5,
"count": 1
},{
"offset": 6,
"count": 1
},{
"offset": 7,
"count": 1
} ],
"clockid": "TAI",
"base_time": 0,
"cycle_time": 20000000,
"cycle_time_extension": 0,
"schedule": [ {
"index": 0,
"cmd": "S",
"gatemask": "0xff",
"interval": 20000000
} ],
"admin": {
"base_time": 1691160511783528178,
"cycle_time": 20000000,
"cycle_time_extension": 0,
"schedule": [ {
"index": 0,
"cmd": "S",
"gatemask": "0xff",
"interval": 20000000
} ]
},
"max-sdu": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ],
"fp": [ "E","E","E","E","E","E","E","E","E","E","E","E","E","E","E","E" ]
}
} ]
Fixes: 602fae856d80 ("taprio: Add support for changing schedules")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
tc/q_taprio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/q_taprio.c b/tc/q_taprio.c
index 795c013c1c2a..16a4992d70d2 100644
--- a/tc/q_taprio.c
+++ b/tc/q_taprio.c
@@ -650,7 +650,7 @@ static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
parse_rtattr_nested(t, TCA_TAPRIO_ATTR_MAX,
tb[TCA_TAPRIO_ATTR_ADMIN_SCHED]);
- open_json_object(NULL);
+ open_json_object("admin");
print_schedule(f, t);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel
2023-08-07 16:08 [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vladimir Oltean
2023-08-07 16:08 ` [PATCH iproute2 2/2] tc/taprio: fix JSON output when TCA_TAPRIO_ATTR_ADMIN_SCHED is present Vladimir Oltean
@ 2023-08-07 20:15 ` Vinicius Costa Gomes
2023-08-07 22:00 ` Vladimir Oltean
1 sibling, 1 reply; 4+ messages in thread
From: Vinicius Costa Gomes @ 2023-08-07 20:15 UTC (permalink / raw)
To: Vladimir Oltean, netdev; +Cc: David Ahern, Stephen Hemminger
Vladimir Oltean <vladimir.oltean@nxp.com> writes:
> When an admin schedule is pending and hasn't yet become operational, the
> kernel will report only the parameters of the admin schedule in a nested
> TCA_TAPRIO_ATTR_ADMIN_SCHED attribute.
>
> However, we default to printing zeroes even for the parameters of the
> operational base time, when that doesn't exist.
>
> Link: https://lore.kernel.org/netdev/87il9w0xx7.fsf@intel.com/
> Fixes: 0dd16449356f ("tc: Add support for configuring the taprio scheduler")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> tc/q_taprio.c | 91 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 53 insertions(+), 38 deletions(-)
>
> diff --git a/tc/q_taprio.c b/tc/q_taprio.c
> index 913197f68caa..795c013c1c2a 100644
> --- a/tc/q_taprio.c
> +++ b/tc/q_taprio.c
> @@ -416,14 +416,11 @@ static int taprio_parse_opt(struct qdisc_util *qu, int argc,
> return 0;
> }
>
> -static int print_sched_list(FILE *f, struct rtattr *list)
> +static void print_sched_list(FILE *f, struct rtattr *list)
> {
> - struct rtattr *item;
> + struct rtattr *item, *nla;
> int rem;
>
> - if (list == NULL)
> - return 0;
> -
> rem = RTA_PAYLOAD(list);
>
> open_json_array(PRINT_JSON, "schedule");
> @@ -432,60 +429,78 @@ static int print_sched_list(FILE *f, struct rtattr *list)
>
> for (item = RTA_DATA(list); RTA_OK(item, rem); item = RTA_NEXT(item, rem)) {
> struct rtattr *tb[TCA_TAPRIO_SCHED_ENTRY_MAX + 1];
> - __u32 index = 0, gatemask = 0, interval = 0;
> - __u8 command = 0;
> + __u32 index, gatemask, interval;
> + __u8 command;
nitpick, optional: as you are already opening blocks for each of the
fields, you could move these declarations there. (same comment for the
next patch)
For the series:
Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel
2023-08-07 20:15 ` [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vinicius Costa Gomes
@ 2023-08-07 22:00 ` Vladimir Oltean
0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Oltean @ 2023-08-07 22:00 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: netdev, David Ahern, Stephen Hemminger
On Mon, Aug 07, 2023 at 01:15:45PM -0700, Vinicius Costa Gomes wrote:
> nitpick, optional: as you are already opening blocks for each of the
> fields, you could move these declarations there. (same comment for the
> next patch)
>
> For the series:
>
> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Thanks for the review. I will submit a v2 with your comment addressed
(all variables defined within the smallest block in which they are used).
But as for "same comment for the next patch": the next patch is a single
line change which keeps the control flow the same.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-07 22:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-07 16:08 [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vladimir Oltean
2023-08-07 16:08 ` [PATCH iproute2 2/2] tc/taprio: fix JSON output when TCA_TAPRIO_ATTR_ADMIN_SCHED is present Vladimir Oltean
2023-08-07 20:15 ` [PATCH iproute2 1/2] tc/taprio: don't print netlink attributes which weren't reported by the kernel Vinicius Costa Gomes
2023-08-07 22:00 ` Vladimir Oltean
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).