lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 10 Sep 2018 13:52:06 +0200
From:   Tobias Jungel <tobias.jungel@...dn.de>
To:     Stephen Hemminger <stephen@...workplumber.org>,
        netdev@...r.kernel.org
Subject: Re: [PATCH iproute2 3/3] bridge: fix vlan show formatting

Hi Stephen,

I just have seen this patch, hence please ignore the "bridge: Correct
json output" I sent. This one solves the issue in a way more elegant
manor.

I tested the JSON output in this series and it works as intended.

Thanks


Reviewed-by: Tobias Jungel <tobias.jungel@...dn.de>


On Thu, 2018-09-06 at 16:30 +0100, Stephen Hemminger wrote:
> The output of vlan show was broken previous change to use json_print.
> Clean the code up and return to original format.
> 
> Note: the JSON syntax has changed to make the bridge vlan
> show more like other outputs (e.g. ip -j li show).
> 
> Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
> ---
>  bridge/br_common.h |  2 +-
>  bridge/link.c      |  6 ++---
>  bridge/vlan.c      | 61 +++++++++++++++++++++++++++-----------------
> --
>  3 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/bridge/br_common.h b/bridge/br_common.h
> index 2f1cb8fd9f3d..69665fde32b6 100644
> --- a/bridge/br_common.h
> +++ b/bridge/br_common.h
> @@ -6,7 +6,7 @@
>  #define MDB_RTR_RTA(r) \
>  		((struct rtattr *)(((char *)(r)) +
> RTA_ALIGN(sizeof(__u32))))
>  
> -extern void print_vlan_info(FILE *fp, struct rtattr *tb);
> +extern void print_vlan_info(struct rtattr *tb, int ifindex);

nitpick, this does not apply to master.

>  extern int print_linkinfo(const struct sockaddr_nl *who,
>  			  struct nlmsghdr *n,
>  			  void *arg);
> diff --git a/bridge/link.c b/bridge/link.c
> index 8d89aca2e638..a5ee9a5c58e6 100644
> --- a/bridge/link.c
> +++ b/bridge/link.c
> @@ -161,7 +161,7 @@ static void print_protinfo(FILE *fp, struct
> rtattr *attr)
>   * This is reported by HW devices that have some bridging
>   * capabilities.
>   */
> -static void print_af_spec(FILE *fp, struct rtattr *attr)
> +static void print_af_spec(struct rtattr *attr, int ifindex)
>  {
>  	struct rtattr *aftb[IFLA_BRIDGE_MAX+1];
>  
> @@ -174,7 +174,7 @@ static void print_af_spec(FILE *fp, struct rtattr
> *attr)
>  		return;
>  
>  	if (aftb[IFLA_BRIDGE_VLAN_INFO])
> -		print_vlan_info(fp, aftb[IFLA_BRIDGE_VLAN_INFO]);
> +		print_vlan_info(aftb[IFLA_BRIDGE_VLAN_INFO], ifindex);
>  }
>  
>  int print_linkinfo(const struct sockaddr_nl *who,
> @@ -229,7 +229,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
>  		print_protinfo(fp, tb[IFLA_PROTINFO]);
>  
>  	if (tb[IFLA_AF_SPEC])
> -		print_af_spec(fp, tb[IFLA_AF_SPEC]);
> +		print_af_spec(tb[IFLA_AF_SPEC], ifi->ifi_index);
>  
>  	print_string(PRINT_FP, NULL, "%s", "\n");
>  	close_json_object();
> diff --git a/bridge/vlan.c b/bridge/vlan.c
> index 19a36b804069..bdce55ae4e14 100644
> --- a/bridge/vlan.c
> +++ b/bridge/vlan.c
> @@ -252,10 +252,18 @@ static int filter_vlan_check(__u16 vid, __u16
> flags)
>  	return 1;
>  }
>  
> -static void print_vlan_port(FILE *fp, int ifi_index)
> +static void open_vlan_port(int ifi_index)
>  {
> -	print_string(PRINT_ANY, NULL, "%s",
> +	open_json_object(NULL);
> +	print_string(PRINT_ANY, "ifname", "%s",
>  		     ll_index_to_name(ifi_index));
> +	open_json_array(PRINT_JSON, "vlans");
> +}
> +
> +static void close_vlan_port(void)
> +{
> +	close_json_array(PRINT_JSON, NULL);
> +	close_json_object();
>  }
>  
>  static void print_range(const char *name, __u16 start, __u16 id)
> @@ -278,7 +286,7 @@ static void print_vlan_tunnel_info(FILE *fp,
> struct rtattr *tb, int ifindex)
>  	__u32 last_tunid_start = 0;
>  
>  	if (!filter_vlan)
> -		print_vlan_port(fp, ifindex);
> +		open_vlan_port(ifindex);
>  
>  	open_json_array(PRINT_JSON, "tunnel");
>  	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> {
> @@ -323,18 +331,20 @@ static void print_vlan_tunnel_info(FILE *fp,
> struct rtattr *tb, int ifindex)
>  			continue;
>  
>  		if (filter_vlan)
> -			print_vlan_port(fp, ifindex);
> +			open_vlan_port(ifindex);
>  
>  		open_json_object(NULL);
>  		print_range("vlan", last_vid_start, tunnel_vid);
>  		print_range("tunid", last_tunid_start, tunnel_id);
>  		close_json_object();
>  
> -		if (!is_json_context())
> -			fprintf(fp, "\n");
> -
> +		print_string(PRINT_FP, NULL, "%s", _SL_);
> +		if (filter_vlan)
> +			close_vlan_port();
>  	}
> -	close_json_array(PRINT_JSON, NULL);
> +
> +	if (!filter_vlan)
> +		close_vlan_port();
>  }
>  
>  static int print_vlan_tunnel(const struct sockaddr_nl *who,
> @@ -421,8 +431,8 @@ static int print_vlan(const struct sockaddr_nl
> *who,
>  		return 0;
>  	}
>  
> -	print_vlan_port(fp, ifm->ifi_index);
> -	print_vlan_info(fp, tb[IFLA_AF_SPEC]);
> +	print_vlan_info(tb[IFLA_AF_SPEC], ifm->ifi_index);
> +	print_string(PRINT_FP, NULL, "%s", _SL_);
>  
>  	fflush(fp);
>  	return 0;
> @@ -430,11 +440,16 @@ static int print_vlan(const struct sockaddr_nl
> *who,
>  
>  static void print_vlan_flags(__u16 flags)
>  {
> +	if (flags == 0)
> +		return;
> +
> +	open_json_array(PRINT_JSON, "flags");
>  	if (flags & BRIDGE_VLAN_INFO_PVID)
> -		print_null(PRINT_ANY, "pvid", " %s", "PVID");
> +		print_string(PRINT_ANY, NULL, " %s", "PVID");
>  
>  	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> -		print_null(PRINT_ANY, "untagged", " %s", "untagged");
> +		print_string(PRINT_ANY, NULL, " %s", "Egress
> Untagged");
> +	close_json_array(PRINT_JSON, NULL);
>  }
>  
>  static void print_one_vlan_stats(const struct bridge_vlan_xstats
> *vstats)
> @@ -461,6 +476,7 @@ static void print_vlan_stats_attr(struct rtattr
> *attr, int ifindex)
>  {
>  	struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
>  	struct rtattr *i, *list;
> +	const char *ifname;
>  	int rem;
>  
>  	parse_rtattr(brtb, LINK_XSTATS_TYPE_MAX, RTA_DATA(attr),
> @@ -471,13 +487,12 @@ static void print_vlan_stats_attr(struct rtattr
> *attr, int ifindex)
>  	list = brtb[LINK_XSTATS_TYPE_BRIDGE];
>  	rem = RTA_PAYLOAD(list);
>  
> -	open_json_object(NULL);
> +	ifname = ll_index_to_name(ifindex);
> +	open_json_object(ifname);
>  
> -	print_color_string(PRINT_ANY, COLOR_IFNAME,
> -			   "dev", "%-16s",
> -			   ll_index_to_name(ifindex));
> +	print_color_string(PRINT_FP, COLOR_IFNAME,
> +			   NULL, "%-16s", ifname);
>  
> -	open_json_array(PRINT_JSON, "xstats");
>  	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> {
>  		const struct bridge_vlan_xstats *vstats = RTA_DATA(i);
>  
> @@ -494,7 +509,6 @@ static void print_vlan_stats_attr(struct rtattr
> *attr, int ifindex)
>  
>  		print_one_vlan_stats(vstats);
>  	}
> -	close_json_array(PRINT_ANY, "\n");
>  	close_json_object();
>  
>  }
> @@ -623,16 +637,13 @@ static int vlan_show(int argc, char **argv)
>  	return 0;
>  }
>  
> -void print_vlan_info(FILE *fp, struct rtattr *tb)
> +void print_vlan_info(struct rtattr *tb, int ifindex)
>  {
>  	struct rtattr *i, *list = tb;
>  	int rem = RTA_PAYLOAD(list);
>  	__u16 last_vid_start = 0;
>  
> -	if (!is_json_context())
> -		fprintf(fp, "%s", _SL_);
> -
> -	open_json_array(PRINT_JSON, "vlan");
> +	open_vlan_port(ifindex);
>  
>  	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> {
>  		struct bridge_vlan_info *vinfo;
> @@ -656,9 +667,9 @@ void print_vlan_info(FILE *fp, struct rtattr *tb)
>  
>  		print_vlan_flags(vinfo->flags);
>  		close_json_object();
> +		print_string(PRINT_FP, NULL, "%s", _SL_);
>  	}
> -
> -	close_json_array(PRINT_ANY, "\n");
> +	close_vlan_port();
>  }
>  
>  int do_vlan(int argc, char **argv)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ