diff mbox series

[iproute2-next,v2,1/1] police: Add support for json output

Message ID 20210603073345.1479732-1-roid@nvidia.com (mailing list archive)
State Superseded
Delegated to: David Ahern
Headers show
Series [iproute2-next,v2,1/1] police: Add support for json output | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Roi Dayan June 3, 2021, 7:33 a.m. UTC 
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

 tc/m_police.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Comments

Stephen Hemminger June 3, 2021, 3:49 p.m. UTC | #1 
On Thu, 3 Jun 2021 10:33:45 +0300
Roi Dayan <roid@nvidia.com> wrote:

>  	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]");

These are errors, and you should just print them to stderr.
That way if program is using JSON they can see the output on stdout;
and look for non-structured errors on stderr.
Roi Dayan June 3, 2021, 4:14 p.m. UTC | #2 
On 2021-06-03 6:49 PM, Stephen Hemminger wrote:
> On Thu, 3 Jun 2021 10:33:45 +0300
> Roi Dayan <roid@nvidia.com> wrote:
> 
>>   	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]");
> 
> These are errors, and you should just print them to stderr.
> That way if program is using JSON they can see the output on stdout;
> and look for non-structured errors on stderr.
> 

right. thanks. i'll fix it.
I think I looked in m_ct.c example for the print and there it's using
print_string(). so i'll do a commit also to fix it there.

other actions use fprintf() but they print to stdout and not stderr.
i guess those should be fixed as well.

any reason also why the original return is 0? i think it should be -1
also in that case.
diff mbox series

Patch

diff --git a/tc/m_police.c b/tc/m_police.c
index 9ef0e40b861b..a78e96c9cf18 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]");
+		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 +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;