[PATCH iproute2-next v4 1/1] police: Add support for json output

[PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

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,
                    "kind": "police",
                    "index": 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>
---

Notes:
    v2
    - fix json output to match correctly the other actions
      i.e. output the action name in key 'kind' and unsigned for the index
    
    v3
    - print errors to stderr.
    - return -1 on null key.
    
    v4
    - removed left over debug that was forgotten. sorry for that.

 tc/m_police.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/tc/m_police.c b/tc/m_police.c
index 9ef0e40b861b..2594c08979e0 100644
--- a/tc/m_police.c
+++ b/tc/m_police.c
@@ -278,18 +278,19 @@ static int print_police(struct action_util *a, FILE *f, struct rtattr *arg)
 	__u64 rate64, prate64;
 	__u64 pps64, ppsburst64;
 
+	print_string(PRINT_ANY, "kind", "%s", "police");
 	if (arg == NULL)
 		return 0;
 
 	parse_rtattr_nested(tb, TCA_POLICE_MAX, arg);
 
 	if (tb[TCA_POLICE_TBF] == NULL) {
-		fprintf(f, "[NULL police tbf]");
-		return 0;
+		fprintf(stderr, "[NULL police tbf]");
+		return -1;
 	}
 #ifndef STOOPID_8BYTE
 	if (RTA_PAYLOAD(tb[TCA_POLICE_TBF])  < sizeof(*p)) {
-		fprintf(f, "[truncated police tbf]");
+		fprintf(stderr, "[truncated police tbf]");
 		return -1;
 	}
 #endif
@@ -300,13 +301,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_uint(PRINT_ANY, "index", "\t index %u ", 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 +328,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 +338,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 +356,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;
-- 
2.21.0

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[David Ahern]

On 6/7/21 12:44 AM, Roi Dayan wrote:
> 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,
>                     "kind": "police",
>                     "index": 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>
> ---
> 
> Notes:
>     v2
>     - fix json output to match correctly the other actions
>       i.e. output the action name in key 'kind' and unsigned for the index
>     
>     v3
>     - print errors to stderr.
>     - return -1 on null key.
>     
>     v4
>     - removed left over debug that was forgotten. sorry for that.
> 
>  tc/m_police.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 

applied to iproute2-next

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Hangbin Liu]

On Mon, Jun 07, 2021 at 09:44:08AM +0300, Roi Dayan wrote:
> 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,
>                     "kind": "police",
>                     "index": 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>
> ---

[...]
> 
> @@ -300,13 +301,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_uint(PRINT_ANY, "index", "\t index %u ", p->index);

Hi everyone,

This update break all policy checking in kernel tc selftest actions/police.json.
As the new output would like 

total acts 1

        action order 0: police   index 1 rate 1Kbit burst 10Kb mtu 2Kb action reclassify overhead 0 ref 1 bind 0


And the current test checks like

	"matchPattern": "action order [0-9]*:  police 0x1 rate 1Kbit burst 10Kb"

I plan to update the kselftest to mach the new output. But I have a question.
Why need we add a "\t" before index output? Is it needed or could be removed?

Thanks
Hangbin

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Davide Caratti]

On Mon, Jul 05, 2021 at 06:41:37PM +0800, Hangbin Liu wrote:
> On Mon, Jun 07, 2021 at 09:44:08AM +0300, Roi Dayan wrote:
> > Change to use the print wrappers instead of fprintf().
> > 
> > Signed-off-by: Roi Dayan <roid@nvidia.com>
> > Reviewed-by: Paul Blakey <paulb@nvidia.com>
> > ---

hello Hangbin,
 
[...]
> > 
> > @@ -300,13 +301,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_uint(PRINT_ANY, "index", "\t index %u ", p->index);
> 
> Hi everyone,
> 
> This update break all policy checking in kernel tc selftest actions/police.json.
> As the new output would like 

thanks for catching this!

> total acts 1
> 
>         action order 0: police   index 1 rate 1Kbit burst 10Kb mtu 2Kb action reclassify overhead 0 ref 1 bind 0
> 
> 
> And the current test checks like
> 
> 	"matchPattern": "action order [0-9]*:  police 0x1 rate 1Kbit burst 10Kb"
> 
>  I plan to update the kselftest to mach the new output.

my 2 cents:

what about using PRINT_FP / PRINT_JSON, so we fix the JSON output only to show "index", and
preserve the human-readable printout iproute and kselftests? besides avoiding failures because
of mismatching kselftests / iproute, this would preserve functionality of scripts that
configure / dump the "police" action. WDYT?

thanks,
-- 
davide

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Hangbin Liu]

On Tue, Jul 06, 2021 at 10:27:34AM +0200, Davide Caratti wrote:
> my 2 cents:
> 
> what about using PRINT_FP / PRINT_JSON, so we fix the JSON output only to show "index", and
> preserve the human-readable printout iproute and kselftests? besides avoiding failures because
> of mismatching kselftests / iproute, this would preserve functionality of scripts that
> configure / dump the "police" action. WDYT?

+1

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

On 2021-07-07 9:53 AM, Hangbin Liu wrote:
> On Tue, Jul 06, 2021 at 10:27:34AM +0200, Davide Caratti wrote:
>> my 2 cents:
>>
>> what about using PRINT_FP / PRINT_JSON, so we fix the JSON output only to show "index", and
>> preserve the human-readable printout iproute and kselftests? besides avoiding failures because
>> of mismatching kselftests / iproute, this would preserve functionality of scripts that
>> configure / dump the "police" action. WDYT?
> 
> +1
> 


why not fix the kselftest to look for the correct output?

all actions output unsigned as the index.
though I did find an issue with the fp output that you pasted
that I missed.


         action order 0: police   index 1 rate 1Kbit burst 10Kb mtu 2Kb 
action reclassify overhead 0 ref 1 bind 0


You asked about the \t before index and actually there is a missing
print_nl() call before printing index and the rest as in the other
actions.

then the match should be something like this

	"matchPattern": "action order [0-9]*: police.*index 0x1 rate 1Kbit 
burst 10Kb"

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

On 2021-07-08 9:57 AM, Roi Dayan wrote:
> 
> 
> On 2021-07-07 9:53 AM, Hangbin Liu wrote:
>> On Tue, Jul 06, 2021 at 10:27:34AM +0200, Davide Caratti wrote:
>>> my 2 cents:
>>>
>>> what about using PRINT_FP / PRINT_JSON, so we fix the JSON output 
>>> only to show "index", and
>>> preserve the human-readable printout iproute and kselftests? besides 
>>> avoiding failures because
>>> of mismatching kselftests / iproute, this would preserve 
>>> functionality of scripts that
>>> configure / dump the "police" action. WDYT?
>>
>> +1
>>
> 
> 
> why not fix the kselftest to look for the correct output?
> 
> all actions output unsigned as the index.
> though I did find an issue with the fp output that you pasted
> that I missed.
> 
> 
>          action order 0: police   index 1 rate 1Kbit burst 10Kb mtu 2Kb 
> action reclassify overhead 0 ref 1 bind 0
> 
> 
> You asked about the \t before index and actually there is a missing
> print_nl() call before printing index and the rest as in the other
> actions.
> 
> then the match should be something like this
> 
>      "matchPattern": "action order [0-9]*: police.*index 0x1 rate 1Kbit 
> burst 10Kb"

actually its

    "matchPattern": "action order [0-9]*: police.*index 1 rate 1Kbit 
burst 10Kb"

also i found some selftest fail for "overhead" matching as i used
print_uint() instead of print_size().

let me send a fix for this with fix for the kselftest of police.json
i think its better to have the consistent output of all actions
instead of police using hex for the instance number.

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[David Ahern]

On 7/8/21 12:57 AM, Roi Dayan wrote:
> 
> 
> On 2021-07-07 9:53 AM, Hangbin Liu wrote:
>> On Tue, Jul 06, 2021 at 10:27:34AM +0200, Davide Caratti wrote:
>>> my 2 cents:
>>>
>>> what about using PRINT_FP / PRINT_JSON, so we fix the JSON output
>>> only to show "index", and
>>> preserve the human-readable printout iproute and kselftests? besides
>>> avoiding failures because
>>> of mismatching kselftests / iproute, this would preserve
>>> functionality of scripts that
>>> configure / dump the "police" action. WDYT?
>>
>> +1
>>
> 
> 
> why not fix the kselftest to look for the correct output?

That is but 1 user. The general rule is that you do not change the
output like you did.

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

On 2021-07-08 5:46 PM, David Ahern wrote:
> On 7/8/21 12:57 AM, Roi Dayan wrote:
>>
>>
>> On 2021-07-07 9:53 AM, Hangbin Liu wrote:
>>> On Tue, Jul 06, 2021 at 10:27:34AM +0200, Davide Caratti wrote:
>>>> my 2 cents:
>>>>
>>>> what about using PRINT_FP / PRINT_JSON, so we fix the JSON output
>>>> only to show "index", and
>>>> preserve the human-readable printout iproute and kselftests? besides
>>>> avoiding failures because
>>>> of mismatching kselftests / iproute, this would preserve
>>>> functionality of scripts that
>>>> configure / dump the "police" action. WDYT?
>>>
>>> +1
>>>
>>
>>
>> why not fix the kselftest to look for the correct output?
> 
> That is but 1 user. The general rule is that you do not change the
> output like you did.
> 

but the output was "broken" and not consistent with all actions.
we are not fixing this kind of thing?
so to continue with the suggestion to use print_fp and keep police
action output broken and print_json for the json output?
just to be sure before submitting change back to old output for fp.


...
         action order 1:  police 0x1 rate 1Mbit burst 20Kb mtu 2Kb 
action reclassify overhead 0b




-       print_string(PRINT_ANY, "kind", "%s", "police");
+       print_string(PRINT_JSON, "kind", "%s", "police");

-       print_uint(PRINT_ANY, "index", "\tindex %u ", p->index);
+       print_hex(PRINT_FP, NULL, " police 0x%x ", p->index);
+       print_uint(PRINT_JSON, "index", NULL, p->index);

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[David Ahern]

On 7/11/21 4:24 AM, Roi Dayan wrote:
> 
> 
> On 2021-07-08 5:46 PM, David Ahern wrote:
>> On 7/8/21 12:57 AM, Roi Dayan wrote:
>>>
>>>
>>> On 2021-07-07 9:53 AM, Hangbin Liu wrote:
>>>> On Tue, Jul 06, 2021 at 10:27:34AM +0200, Davide Caratti wrote:
>>>>> my 2 cents:
>>>>>
>>>>> what about using PRINT_FP / PRINT_JSON, so we fix the JSON output
>>>>> only to show "index", and
>>>>> preserve the human-readable printout iproute and kselftests? besides
>>>>> avoiding failures because
>>>>> of mismatching kselftests / iproute, this would preserve
>>>>> functionality of scripts that
>>>>> configure / dump the "police" action. WDYT?
>>>>
>>>> +1
>>>>
>>>
>>>
>>> why not fix the kselftest to look for the correct output?
>>
>> That is but 1 user. The general rule is that you do not change the
>> output like you did.
>>
> 
> but the output was "broken" and not consistent with all actions.
> we are not fixing this kind of thing?

It has been in hex since 2004, and you can not decide in 2021 that it is
'broken' and change it.

> so to continue with the suggestion to use print_fp and keep police
> action output broken and print_json for the json output?
> just to be sure before submitting change back to old output for fp.
> 
> 
> ...
>         action order 1:  police 0x1 rate 1Mbit burst 20Kb mtu 2Kb action
> reclassify overhead 0b
> 
> 
> 
> 
> -       print_string(PRINT_ANY, "kind", "%s", "police");
> +       print_string(PRINT_JSON, "kind", "%s", "police");
> 
> -       print_uint(PRINT_ANY, "index", "\tindex %u ", p->index);
> +       print_hex(PRINT_FP, NULL, " police 0x%x ", p->index);
> +       print_uint(PRINT_JSON, "index", NULL, p->index);
> 
> 

Jamal: opinions?

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Jamal Hadi Salim]

On 2021-07-11 12:00 p.m., David Ahern wrote:

>> ...
>>          action order 1:  police 0x1 rate 1Mbit burst 20Kb mtu 2Kb action
>> reclassify overhead 0b
>>
>>
>>
>>
>> -       print_string(PRINT_ANY, "kind", "%s", "police");
>> +       print_string(PRINT_JSON, "kind", "%s", "police");
>>
>> -       print_uint(PRINT_ANY, "index", "\tindex %u ", p->index);
>> +       print_hex(PRINT_FP, NULL, " police 0x%x ", p->index);
>> +       print_uint(PRINT_JSON, "index", NULL, p->index);
>>
>>
> 
> Jamal: opinions?

Looks good to me. Roi please run the kernel tests to make sure
nothing breaks.

cheers,
jamal

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

On 2021-07-12 2:02 PM, Jamal Hadi Salim wrote:
> On 2021-07-11 12:00 p.m., David Ahern wrote:
> 
>>> ...
>>>          action order 1:  police 0x1 rate 1Mbit burst 20Kb mtu 2Kb 
>>> action
>>> reclassify overhead 0b
>>>
>>>
>>>
>>>
>>> -       print_string(PRINT_ANY, "kind", "%s", "police");
>>> +       print_string(PRINT_JSON, "kind", "%s", "police");
>>>
>>> -       print_uint(PRINT_ANY, "index", "\tindex %u ", p->index);
>>> +       print_hex(PRINT_FP, NULL, " police 0x%x ", p->index);
>>> +       print_uint(PRINT_JSON, "index", NULL, p->index);
>>>
>>>
>>
>> Jamal: opinions?
> 
> Looks good to me. Roi please run the kernel tests to make sure
> nothing breaks.
> 
> cheers,
> jamal
> 

ok
I sent patch titled "police: Fix normal output back to what it was"
I ran the tdc tests and passed.

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Stephen Hemminger]

On Mon, 7 Jun 2021 09:44:08 +0300
Roi Dayan <roid@nvidia.com> wrote:

> -	fprintf(f, " police 0x%x ", p->index);
> +	print_uint(PRINT_ANY, "index", "\t index %u ", p->index);

Why did output format have to change here? Why not:

	print_hex(PRINT_ANY, "police", " police %#x", p->index);

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[David Ahern]

On 7/5/21 10:28 AM, Stephen Hemminger wrote:
> On Mon, 7 Jun 2021 09:44:08 +0300
> Roi Dayan <roid@nvidia.com> wrote:
> 
>> -	fprintf(f, " police 0x%x ", p->index);
>> +	print_uint(PRINT_ANY, "index", "\t index %u ", p->index);
> 
> Why did output format have to change here? Why not:
> 
> 	print_hex(PRINT_ANY, "police", " police %#x", p->index);
> 

it should not have. I caught it in the first version in a review
comment; missed it in v4 that was applied.

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

On 2021-07-05 7:30 PM, David Ahern wrote:
> On 7/5/21 10:28 AM, Stephen Hemminger wrote:
>> On Mon, 7 Jun 2021 09:44:08 +0300
>> Roi Dayan <roid@nvidia.com> wrote:
>>
>>> -	fprintf(f, " police 0x%x ", p->index);
>>> +	print_uint(PRINT_ANY, "index", "\t index %u ", p->index);
>>
>> Why did output format have to change here? Why not:
>>
>> 	print_hex(PRINT_ANY, "police", " police %#x", p->index);
>>
> 
> it should not have. I caught it in the first version in a review
> comment; missed it in v4 that was applied.
> 

Hi,

I replied to your review in v0 that I wanted to match all the other
actions output which output as unsigned.
Since I didn't get another reply I thought its ok to continue and sent
another version which other changes that were required.

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Roi Dayan]

On 2021-07-06 11:30 AM, Roi Dayan wrote:
> 
> 
> On 2021-07-05 7:30 PM, David Ahern wrote:
>> On 7/5/21 10:28 AM, Stephen Hemminger wrote:
>>> On Mon, 7 Jun 2021 09:44:08 +0300
>>> Roi Dayan <roid@nvidia.com> wrote:
>>>
>>>> -    fprintf(f, " police 0x%x ", p->index);
>>>> +    print_uint(PRINT_ANY, "index", "\t index %u ", p->index);
>>>
>>> Why did output format have to change here? Why not:
>>>
>>>     print_hex(PRINT_ANY, "police", " police %#x", p->index);
>>>
>>
>> it should not have. I caught it in the first version in a review
>> comment; missed it in v4 that was applied.
>>
> 
> Hi,
> 
> I replied to your review in v0 that I wanted to match all the other
> actions output which output as unsigned.
> Since I didn't get another reply I thought its ok to continue and sent
> another version which other changes that were required.
> 
> 

beside the unsigned the json output in all other actions use "index"
as far as I notice.
the action name, e.g. "police", being printed under "kind" also to
match the other actions.
So I wanted the json output for police to match with same keys as
the other actions. at least the keys "kind" and "index" which all
have and not each action use different key.

Re: [PATCH iproute2-next v4 1/1] police: Add support for json output

[Stephen Hemminger]

On Mon, 7 Jun 2021 09:44:08 +0300
Roi Dayan <roid@nvidia.com> wrote:

> -	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) {

This should add newline like original by using print_nl()