All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2-next] police: Add support for json output
@ 2021-05-27 13:07 Roi Dayan
  2021-06-02 14:29 ` David Ahern
  0 siblings, 1 reply; 5+ messages in thread
From: Roi Dayan @ 2021-05-27 13:07 UTC (permalink / raw)
  To: netdev; +Cc: Roi Dayan, Paul Blakey

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

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;
-- 
2.8.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2-next] police: Add support for json output
  2021-05-27 13:07 [PATCH iproute2-next] police: Add support for json output Roi Dayan
@ 2021-06-02 14:29 ` David Ahern
  2021-06-03  7:27   ` Roi Dayan
  0 siblings, 1 reply; 5+ messages in thread
From: David Ahern @ 2021-06-02 14:29 UTC (permalink / raw)
  To: Roi Dayan, netdev, Stephen Hemminger, Jamal Hadi Salim; +Cc: Paul Blakey

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] &&



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2-next] police: Add support for json output
  2021-06-02 14:29 ` David Ahern
@ 2021-06-03  7:27   ` Roi Dayan
  2021-06-03 15:40     ` Stephen Hemminger
  0 siblings, 1 reply; 5+ messages in thread
From: Roi Dayan @ 2021-06-03  7:27 UTC (permalink / raw)
  To: David Ahern, netdev, Stephen Hemminger, Jamal Hadi Salim; +Cc: Paul Blakey



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] &&
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2-next] police: Add support for json output
  2021-06-03  7:27   ` Roi Dayan
@ 2021-06-03 15:40     ` Stephen Hemminger
  2021-06-03 16:04       ` Roi Dayan
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2021-06-03 15:40 UTC (permalink / raw)
  To: Roi Dayan; +Cc: David Ahern, netdev, Jamal Hadi Salim, Paul Blakey

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2-next] police: Add support for json output
  2021-06-03 15:40     ` Stephen Hemminger
@ 2021-06-03 16:04       ` Roi Dayan
  0 siblings, 0 replies; 5+ messages in thread
From: Roi Dayan @ 2021-06-03 16:04 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Ahern, netdev, Jamal Hadi Salim, Paul Blakey



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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-03 16:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 13:07 [PATCH iproute2-next] police: Add support for json output Roi Dayan
2021-06-02 14:29 ` David Ahern
2021-06-03  7:27   ` Roi Dayan
2021-06-03 15:40     ` Stephen Hemminger
2021-06-03 16:04       ` Roi Dayan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.