Re: [PATCH iproute2-next 3/7] ip: add json support to ip rule

[David Ahern <dsahern@gmail.com>]

On 3/5/18 8:04 PM, Stephen Hemminger wrote:
> @@ -202,50 +202,66 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  	if (!filter_nlmsg(n, tb, host_len))
>  		return 0;
>  
> +	open_json_object(NULL);
>  	if (n->nlmsg_type == RTM_DELRULE)
> -		fprintf(fp, "Deleted ");
> +		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
>  
>  	if (tb[FRA_PRIORITY])
> -		fprintf(fp, "%u:\t",
> -			rta_getattr_u32(tb[FRA_PRIORITY]));
> -	else
> -		fprintf(fp, "0:\t");
> +		prio = rta_getattr_u32(tb[FRA_PRIORITY]);
> +
> +	print_uint(PRINT_ANY, "priority",
> +		   "%u:\t", prio);

This one has a lot of instances where the print_* can be put on one line.


>  
>  	if (frh->flags & FIB_RULE_INVERT)
> -		fprintf(fp, "not ");
> +		print_null(PRINT_ANY, "not", "not ", NULL);
> +
> +	if (!is_json_context())
> +		fprintf(fp, "from ");
>  
>  	if (tb[FRA_SRC]) {
> -		if (frh->src_len != host_len) {
> -			fprintf(fp, "from %s/%u ",
> -				rt_addr_n2a_rta(frh->family, tb[FRA_SRC]),
> -				frh->src_len);
> -		} else {
> -			fprintf(fp, "from %s ",
> -				format_host_rta(frh->family, tb[FRA_SRC]));
> -		}
> +		const char *src
> +			= rt_addr_n2a_rta(frh->family, tb[FRA_SRC]);
> +
> +		print_color_string(PRINT_ANY,
> +				   ifa_family_color(frh->family),
> +				   "src", "%s", src);
> +		if (frh->src_len != host_len)
> +			print_uint(PRINT_ANY, "srclen",
> +				   "/%u", frh->src_len);
>  	} else if (frh->src_len) {
> -		fprintf(fp, "from 0/%d ", frh->src_len);
> +		print_string(PRINT_ANY,
> +			     "src", "%s", "0");
> +		print_uint(PRINT_ANY,
> +			   "srclen", "/%u", frh->src_len);
>  	} else {
> -		fprintf(fp, "from all ");
> +		print_string(PRINT_ANY,
> +			     "src", "%s", "all");
>  	}
>  
> +	if (!is_json_context())
> +		fprintf(fp, " to ");
> +
>  	if (tb[FRA_DST]) {
> -		if (frh->dst_len != host_len) {
> -			fprintf(fp, "to %s/%u ",
> -				rt_addr_n2a_rta(frh->family, tb[FRA_DST]),
> -				frh->dst_len);
> -		} else {
> -			fprintf(fp, "to %s ",
> -				format_host_rta(frh->family, tb[FRA_DST]));
> -		}
> +		const char *dst
> +			= rt_addr_n2a_rta(frh->family, tb[FRA_DST]);
> +
> +		print_color_string(PRINT_ANY,
> +				   ifa_family_color(frh->family),
> +				   "dst", "%s", dst);
> +		if (frh->dst_len != host_len)
> +			print_uint(PRINT_ANY, "dstlen",
> +				   "/%u ", frh->dst_len);
>  	} else if (frh->dst_len) {
> -		fprintf(fp, "to 0/%d ", frh->dst_len);
> +		print_string(PRINT_ANY,
> +			     "dst", "%s", "0");
> +		print_uint(PRINT_ANY,
> +			   "dstlen", "/%u ", frh->dst_len);
>  	}
>  
>  	if (frh->tos) {
> -		SPRINT_BUF(b1);
> -		fprintf(fp, "tos %s ",
> -			rtnl_dsfield_n2a(frh->tos, b1, sizeof(b1)));
> +		print_string(PRINT_ANY, "tos",
> +			     "tos %s ",
> +			     rtnl_dsfield_n2a(frh->tos, b1, sizeof(b1)));
>  	}
>  
>  	if (tb[FRA_FWMARK] || tb[FRA_FWMASK]) {
> @@ -255,53 +271,82 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  			mark = rta_getattr_u32(tb[FRA_FWMARK]);
>  
>  		if (tb[FRA_FWMASK] &&
> -		    (mask = rta_getattr_u32(tb[FRA_FWMASK])) != 0xFFFFFFFF)
> -			fprintf(fp, "fwmark 0x%x/0x%x ", mark, mask);
> -		else
> -			fprintf(fp, "fwmark 0x%x ", mark);
> +		    (mask = rta_getattr_u32(tb[FRA_FWMASK])) != 0xFFFFFFFF) {
> +			print_0xhex(PRINT_ANY, "fwmark",
> +				    "fwmark 0x%x", mark);
> +			print_0xhex(PRINT_ANY, "fwmask",
> +				    "/0x%x ",  mask);
> +		} else {
> +			print_0xhex(PRINT_ANY, "fwmark",
> +				    "fwmark 0x%x ", mark);
> +		}
>  	}
>  
>  	if (tb[FRA_IFNAME]) {
> -		fprintf(fp, "iif %s ", rta_getattr_str(tb[FRA_IFNAME]));
> +		if (!is_json_context())
> +			fprintf(fp, "iif ");
> +		print_color_string(PRINT_ANY, COLOR_IFNAME,
> +				   "iif", "%s ",
> +				   rta_getattr_str(tb[FRA_IFNAME]));
> +
>  		if (frh->flags & FIB_RULE_IIF_DETACHED)
> -			fprintf(fp, "[detached] ");
> +			print_null(PRINT_ANY, "iif_detached",
> +				   "[detached] ", NULL);
>  	}
>  
>  	if (tb[FRA_OIFNAME]) {
> -		fprintf(fp, "oif %s ", rta_getattr_str(tb[FRA_OIFNAME]));
> +		if (!is_json_context())
> +			fprintf(fp, "oif ");
> +
> +		print_color_string(PRINT_ANY, COLOR_IFNAME,
> +				   "oif", "%s ",
> +				   rta_getattr_str(tb[FRA_OIFNAME]));
> +
>  		if (frh->flags & FIB_RULE_OIF_DETACHED)
> -			fprintf(fp, "[detached] ");
> +			print_null(PRINT_ANY, "oif_detached",
> +				   "[detached] ", NULL);

NULL for detached value seems weird.
[ {
        "priority": 998,
        "src": "all",
        "iif": "dummy2",
        "iif_detached": null,
        "table": "1"
    },

Is that normal for json for fields that only exist when a value is true?


>  	}
>  
>  	if (tb[FRA_L3MDEV]) {
> -		if (rta_getattr_u8(tb[FRA_L3MDEV]))
> -			fprintf(fp, "lookup [l3mdev-table] ");
> +		__u8 mdev = rta_getattr_u8(tb[FRA_L3MDEV]);
> +
> +		if (mdev)
> +			print_null(PRINT_ANY, "l3mdev",
> +				   "lookup [l3mdev-table] ", NULL);
>  	}
>  
>  	if (tb[FRA_UID_RANGE]) {
>  		struct fib_rule_uid_range *r = RTA_DATA(tb[FRA_UID_RANGE]);
>  
> -		fprintf(fp, "uidrange %u-%u ", r->start, r->end);
> +		print_uint(PRINT_ANY, "uid_start",
> +			   "uidrange %u", r->start);
> +		print_uint(PRINT_ANY, "uid_end",
> +			   "-%u ", r->end);
>  	}
>  
>  	table = frh_get_table(frh, tb);
>  	if (table) {
> -		fprintf(fp, "lookup %s ",
> -			rtnl_rttable_n2a(table, b1, sizeof(b1)));
> +		print_string(PRINT_ANY, "table",
> +			     "lookup %s ",
> +			     rtnl_rttable_n2a(table, b1, sizeof(b1)));

dropped a space in the non-json output:
# ip   ru ls
998:	from all to iif dummy2 [detached] lookup 1
998:	from all to 9.9.9.9lookup 2
999:	from all to 8.8.8.8lookup main