| Message ID | 20210527130742.1070668-1-roid@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] police: Add support for json output | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 5/27/21 7:07 AM, Roi Dayan wrote: > @@ -300,13 +300,13 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) > RTA_PAYLOAD(tb[TCA_POLICE_RATE64]) >= sizeof(rate64)) > rate64 = rta_getattr_u64(tb[TCA_POLICE_RATE64]); > > - fprintf(f, " police 0x%x ", p->index); > + print_int(PRINT_ANY, "police", "police %d ", p->index); this changes the output format from hex to decimal. > tc_print_rate(PRINT_FP, NULL, "rate %s ", rate64); > buffer = tc_calc_xmitsize(rate64, p->burst); > print_size(PRINT_FP, NULL, "burst %s ", buffer); > print_size(PRINT_FP, NULL, "mtu %s ", p->mtu); > if (show_raw) > - fprintf(f, "[%08x] ", p->burst); > + print_hex(PRINT_FP, NULL, "[%08x] ", p->burst); > > prate64 = p->peakrate.rate; > if (tb[TCA_POLICE_PEAKRATE64] &&
On 2021-06-02 5:29 PM, David Ahern wrote: > On 5/27/21 7:07 AM, Roi Dayan wrote: >> @@ -300,13 +300,13 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) >> RTA_PAYLOAD(tb[TCA_POLICE_RATE64]) >= sizeof(rate64)) >> rate64 = rta_getattr_u64(tb[TCA_POLICE_RATE64]); >> >> - fprintf(f, " police 0x%x ", p->index); >> + print_int(PRINT_ANY, "police", "police %d ", p->index); > > this changes the output format from hex to decimal. > hmm thanks for the review. actually I see another issue with this. I missed this but this should actually be split into "kind" and "index". And index should be unsigned as the other actions. so we should have kind printed at the top of the function even if arg is null and index here. print_string(PRINT_ANY, "kind", "%s", "police"); if (arg == NULL) return 0; ... print_uint(PRINT_ANY, "index", "\t index %u ", p->index); then the json output should be this "actions": [ { "order": 1, "kind": "police", "index": 1, i'll send v2. > >> tc_print_rate(PRINT_FP, NULL, "rate %s ", rate64); >> buffer = tc_calc_xmitsize(rate64, p->burst); >> print_size(PRINT_FP, NULL, "burst %s ", buffer); >> print_size(PRINT_FP, NULL, "mtu %s ", p->mtu); >> if (show_raw) >> - fprintf(f, "[%08x] ", p->burst); >> + print_hex(PRINT_FP, NULL, "[%08x] ", p->burst); >> >> prate64 = p->peakrate.rate; >> if (tb[TCA_POLICE_PEAKRATE64] && > >
On Thu, 3 Jun 2021 10:27:55 +0300 Roi Dayan <roid@nvidia.com> wrote: > On 2021-06-02 5:29 PM, David Ahern wrote: > > On 5/27/21 7:07 AM, Roi Dayan wrote: > >> @@ -300,13 +300,13 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) > >> RTA_PAYLOAD(tb[TCA_POLICE_RATE64]) >= sizeof(rate64)) > >> rate64 = rta_getattr_u64(tb[TCA_POLICE_RATE64]); > >> > >> - fprintf(f, " police 0x%x ", p->index); > >> + print_int(PRINT_ANY, "police", "police %d ", p->index); > > > > this changes the output format from hex to decimal. > > > > hmm thanks for the review. actually I see another issue with this. > I missed this but this should actually be split into "kind" and "index". > And index should be unsigned as the other actions. > so we should have kind printed at the top of the function even if arg > is null and index here. > > print_string(PRINT_ANY, "kind", "%s", "police"); > > if (arg == NULL) > > return 0; > ... > print_uint(PRINT_ANY, "index", "\t index %u ", p->index); > > > > then the json output should be this > > "actions": [ { > "order": 1, > "kind": "police", > "index": 1, > > > i'll send v2. > > > > > >> tc_print_rate(PRINT_FP, NULL, "rate %s ", rate64); > >> buffer = tc_calc_xmitsize(rate64, p->burst); > >> print_size(PRINT_FP, NULL, "burst %s ", buffer); > >> print_size(PRINT_FP, NULL, "mtu %s ", p->mtu); > >> if (show_raw) > >> - fprintf(f, "[%08x] ", p->burst); > >> + print_hex(PRINT_FP, NULL, "[%08x] ", p->burst); > >> > >> prate64 = p->peakrate.rate; > >> if (tb[TCA_POLICE_PEAKRATE64] && > > > > One useful check is to run your JSON output into python parser to be sure it is valid
On 2021-06-03 6:40 PM, Stephen Hemminger wrote: > On Thu, 3 Jun 2021 10:27:55 +0300 > Roi Dayan <roid@nvidia.com> wrote: > >> On 2021-06-02 5:29 PM, David Ahern wrote: >>> On 5/27/21 7:07 AM, Roi Dayan wrote: >>>> @@ -300,13 +300,13 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) >>>> RTA_PAYLOAD(tb[TCA_POLICE_RATE64]) >= sizeof(rate64)) >>>> rate64 = rta_getattr_u64(tb[TCA_POLICE_RATE64]); >>>> >>>> - fprintf(f, " police 0x%x ", p->index); >>>> + print_int(PRINT_ANY, "police", "police %d ", p->index); >>> >>> this changes the output format from hex to decimal. >>> >> >> hmm thanks for the review. actually I see another issue with this. >> I missed this but this should actually be split into "kind" and "index". >> And index should be unsigned as the other actions. >> so we should have kind printed at the top of the function even if arg >> is null and index here. >> >> print_string(PRINT_ANY, "kind", "%s", "police"); >> >> if (arg == NULL) >> >> return 0; >> ... >> print_uint(PRINT_ANY, "index", "\t index %u ", p->index); >> >> >> >> then the json output should be this >> >> "actions": [ { >> "order": 1, >> "kind": "police", >> "index": 1, >> >> >> i'll send v2. >> >> >>> >>>> tc_print_rate(PRINT_FP, NULL, "rate %s ", rate64); >>>> buffer = tc_calc_xmitsize(rate64, p->burst); >>>> print_size(PRINT_FP, NULL, "burst %s ", buffer); >>>> print_size(PRINT_FP, NULL, "mtu %s ", p->mtu); >>>> if (show_raw) >>>> - fprintf(f, "[%08x] ", p->burst); >>>> + print_hex(PRINT_FP, NULL, "[%08x] ", p->burst); >>>> >>>> prate64 = p->peakrate.rate; >>>> if (tb[TCA_POLICE_PEAKRATE64] && >>> >>> > > One useful check is to run your JSON output into python parser to be sure it is valid > I used jq to parse the output from tc. jq gives an error on the original output, example: parse error: Invalid numeric literal at line 15, column 38 and after the fix jq works and gives a nice colored output. the fix i pointed out here is to match the dump like in other actions. so instead of key police with value index it should be key kind with value police and key index with the value index.
diff --git a/tc/m_police.c b/tc/m_police.c index 9ef0e40b861b..23ec973bee26 100644 --- a/tc/m_police.c +++ b/tc/m_police.c @@ -284,12 +284,12 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) parse_rtattr_nested(tb, TCA_POLICE_MAX, arg); if (tb[TCA_POLICE_TBF] == NULL) { - fprintf(f, "[NULL police tbf]"); + print_string(PRINT_FP, NULL, "%s", "[NULL police tbf]"); return 0; } #ifndef STOOPID_8BYTE if (RTA_PAYLOAD(tb[TCA_POLICE_TBF]) < sizeof(*p)) { - fprintf(f, "[truncated police tbf]"); + print_string(PRINT_FP, NULL, "%s", "[truncated police tbf]"); return -1; } #endif @@ -300,13 +300,13 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) RTA_PAYLOAD(tb[TCA_POLICE_RATE64]) >= sizeof(rate64)) rate64 = rta_getattr_u64(tb[TCA_POLICE_RATE64]); - fprintf(f, " police 0x%x ", p->index); + print_int(PRINT_ANY, "police", "police %d ", p->index); tc_print_rate(PRINT_FP, NULL, "rate %s ", rate64); buffer = tc_calc_xmitsize(rate64, p->burst); print_size(PRINT_FP, NULL, "burst %s ", buffer); print_size(PRINT_FP, NULL, "mtu %s ", p->mtu); if (show_raw) - fprintf(f, "[%08x] ", p->burst); + print_hex(PRINT_FP, NULL, "[%08x] ", p->burst); prate64 = p->peakrate.rate; if (tb[TCA_POLICE_PEAKRATE64] && @@ -327,8 +327,8 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) pps64 = rta_getattr_u64(tb[TCA_POLICE_PKTRATE64]); ppsburst64 = rta_getattr_u64(tb[TCA_POLICE_PKTBURST64]); ppsburst64 = tc_calc_xmitsize(pps64, ppsburst64); - fprintf(f, "pkts_rate %llu ", pps64); - fprintf(f, "pkts_burst %llu ", ppsburst64); + print_u64(PRINT_ANY, "pkts_rate", "pkts_rate %llu ", pps64); + print_u64(PRINT_ANY, "pkts_burst", "pkts_burst %llu ", ppsburst64); } print_action_control(f, "action ", p->action, ""); @@ -337,14 +337,17 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) __u32 action = rta_getattr_u32(tb[TCA_POLICE_RESULT]); print_action_control(f, "/", action, " "); - } else - fprintf(f, " "); + } else { + print_string(PRINT_FP, NULL, " ", NULL); + } - fprintf(f, "overhead %ub ", p->rate.overhead); + print_uint(PRINT_ANY, "overhead", "overhead %u ", p->rate.overhead); linklayer = (p->rate.linklayer & TC_LINKLAYER_MASK); if (linklayer > TC_LINKLAYER_ETHERNET || show_details) - fprintf(f, "linklayer %s ", sprint_linklayer(linklayer, b2)); - fprintf(f, "\n\tref %d bind %d", p->refcnt, p->bindcnt); + print_string(PRINT_ANY, "linklayer", "linklayer %s ", + sprint_linklayer(linklayer, b2)); + print_int(PRINT_ANY, "ref", "ref %d ", p->refcnt); + print_int(PRINT_ANY, "bind", "bind %d ", p->bindcnt); if (show_stats) { if (tb[TCA_POLICE_TM]) { struct tcf_t *tm = RTA_DATA(tb[TCA_POLICE_TM]); @@ -352,7 +355,7 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg) print_tm(f, tm); } } - fprintf(f, "\n"); + print_nl(); return 0;
Change to use the print wrappers instead of fprintf(). This is example output of the options part before this commit: "options": { "handle": 1, "in_hw": true, "actions": [ { "order": 1 police 0x2 , "control_action": { "type": "drop" }, "control_action": { "type": "continue" }overhead 0b linklayer unspec ref 1 bind 1 , "used_hw_stats": [ "delayed" ] } ] } This is the output of the same dump with this commit: "options": { "handle": 1, "in_hw": true, "actions": [ { "order": 1, "police": 2, "control_action": { "type": "drop" }, "control_action": { "type": "continue" }, "overhead": 0, "linklayer": "unspec", "ref": 1, "bind": 1, "used_hw_stats": [ "delayed" ] } ] } Signed-off-by: Roi Dayan <roid@nvidia.com> Reviewed-by: Paul Blakey <paulb@nvidia.com> --- tc/m_police.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)