[PATCH iproute2-next 0/4] ip: Support bridge VLAN stats in `ip stats'

[PATCH iproute2-next 0/4] ip: Support bridge VLAN stats in `ip stats'

[Petr Machata]

ip stats displays bridge-related multicast and STP stats, but not VLAN
stats. There is code for requesting, decoding and formatting these stats
accessible through `bridge -s vlan', but the `ip stats' suite lacks it. In
this patchset, extract the `bridge vlan' code to a generally accessible
place and extend `ip stats' to use it.

This reuses the existing display and JSON format, and plugs it into the
existing `ip stats' hierarchy:

 # ip stats show dev v2 group xstats_slave subgroup bridge suite vlan
 2: v2: group xstats_slave subgroup bridge suite vlan
                   10
                     RX: 776 bytes 10 packets
                     TX: 224 bytes 4 packets

                   20
                     RX: 684 bytes 7 packets
                     TX: 0 bytes 0 packets

 # ip -j -p stats show dev v2 group xstats_slave subgroup bridge suite vlan
 [ {
         "ifindex": 2,
         "ifname": "v2",
         "group": "xstats_slave",
         "subgroup": "bridge",
         "suite": "vlan",
         "vlans": [ {
                 "vid": 10,
                 "rx_bytes": 552,
                 "rx_packets": 6,
                 "tx_bytes": 0,
                 "tx_packets": 0
             },{
                 "vid": 20,
                 "rx_bytes": 684,
                 "rx_packets": 7,
                 "tx_bytes": 0,
                 "tx_packets": 0
             } ]
     } ]

Petr Machata (4):
  ip: ipstats: Iterate all xstats attributes
  ip: ip_common: Drop ipstats_stat_desc_xstats::inner_max
  lib: bridge: Add a module for bridge-related helpers
  ip: iplink_bridge: Support bridge VLAN stats in `ip stats'

 bridge/vlan.c      | 50 +++++-----------------------------------------
 include/bridge.h   | 11 ++++++++++
 ip/ip_common.h     |  1 -
 ip/iplink_bond.c   |  2 --
 ip/iplink_bridge.c | 40 +++++++++++++++++++++++++++++++++----
 ip/ipstats.c       | 18 ++++++++---------
 lib/Makefile       |  3 ++-
 lib/bridge.c       | 47 +++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 110 insertions(+), 62 deletions(-)
 create mode 100644 include/bridge.h
 create mode 100644 lib/bridge.c

-- 
2.49.0

[PATCH iproute2-next 1/4] ip: ipstats: Iterate all xstats attributes

[Petr Machata]

ipstats_stat_desc_show_xstats() operates by first parsing the attribute
stream into a type-indexed table, and then accessing the right attribute.
But bridge VLAN stats are given as several BRIDGE_XSTATS_VLAN attributes,
one per VLAN. With the above approach to parsing, only one of these
attributes would be shown. Instead, iterate the stream of attributes and
call the show_cb for each one with a matching type.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 ip/ipstats.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index cb9d9cbb..6f7c59fd 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -590,7 +590,8 @@ int ipstats_stat_desc_show_xstats(struct ipstats_stat_show_attrs *attrs,
 {
 	struct ipstats_stat_desc_xstats *xdesc;
 	const struct rtattr *at;
-	struct rtattr **tb;
+	const struct rtattr *i;
+	int rem;
 	int err;
 
 	xdesc = container_of(desc, struct ipstats_stat_desc_xstats, desc);
@@ -600,15 +601,14 @@ int ipstats_stat_desc_show_xstats(struct ipstats_stat_show_attrs *attrs,
 	if (at == NULL)
 		return err;
 
-	tb = alloca(sizeof(*tb) * (xdesc->inner_max + 1));
-	err = parse_rtattr_nested(tb, xdesc->inner_max, at);
-	if (err != 0)
-		return err;
-
-	if (tb[xdesc->inner_at] != NULL) {
-		print_nl();
-		xdesc->show_cb(tb[xdesc->inner_at]);
+	rem = RTA_PAYLOAD(at);
+	for (i = RTA_DATA(at); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
+		if (i->rta_type == xdesc->inner_at) {
+			print_nl();
+			xdesc->show_cb(i);
+		}
 	}
+
 	return 0;
 }
 
-- 
2.49.0

[PATCH iproute2-next 2/4] ip: ip_common: Drop ipstats_stat_desc_xstats::inner_max

[Petr Machata]

After the previous patch, this field is not read anymore. Drop it.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 ip/ip_common.h     | 1 -
 ip/iplink_bond.c   | 2 --
 ip/iplink_bridge.c | 4 ----
 3 files changed, 7 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index 37de09d4..3f55ea33 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -195,7 +195,6 @@ struct ipstats_stat_desc_xstats {
 	const struct ipstats_stat_desc desc;
 	int xstats_at;
 	int link_type_at;
-	int inner_max;
 	int inner_at;
 	void (*show_cb)(const struct rtattr *at);
 };
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 19af67d0..a964f547 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -940,7 +940,6 @@ ipstats_stat_desc_xstats_bond_lacp = {
 	.desc = IPSTATS_STAT_DESC_XSTATS_LEAF("802.3ad"),
 	.xstats_at = IFLA_STATS_LINK_XSTATS,
 	.link_type_at = LINK_XSTATS_TYPE_BOND,
-	.inner_max = BOND_XSTATS_MAX,
 	.inner_at = BOND_XSTATS_3AD,
 	.show_cb = &bond_print_3ad_stats,
 };
@@ -962,7 +961,6 @@ ipstats_stat_desc_xstats_slave_bond_lacp = {
 	.desc = IPSTATS_STAT_DESC_XSTATS_LEAF("802.3ad"),
 	.xstats_at = IFLA_STATS_LINK_XSTATS_SLAVE,
 	.link_type_at = LINK_XSTATS_TYPE_BOND,
-	.inner_max = BOND_XSTATS_MAX,
 	.inner_at = BOND_XSTATS_3AD,
 	.show_cb = &bond_print_3ad_stats,
 };
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index d98bfa5a..3d54e203 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -1075,7 +1075,6 @@ ipstats_stat_desc_xstats_bridge_stp = {
 	.desc = IPSTATS_STAT_DESC_XSTATS_LEAF("stp"),
 	.xstats_at = IFLA_STATS_LINK_XSTATS,
 	.link_type_at = LINK_XSTATS_TYPE_BRIDGE,
-	.inner_max = BRIDGE_XSTATS_MAX,
 	.inner_at = BRIDGE_XSTATS_STP,
 	.show_cb = &bridge_print_stats_stp,
 };
@@ -1085,7 +1084,6 @@ ipstats_stat_desc_xstats_bridge_mcast = {
 	.desc = IPSTATS_STAT_DESC_XSTATS_LEAF("mcast"),
 	.xstats_at = IFLA_STATS_LINK_XSTATS,
 	.link_type_at = LINK_XSTATS_TYPE_BRIDGE,
-	.inner_max = BRIDGE_XSTATS_MAX,
 	.inner_at = BRIDGE_XSTATS_MCAST,
 	.show_cb = &bridge_print_stats_mcast,
 };
@@ -1108,7 +1106,6 @@ ipstats_stat_desc_xstats_slave_bridge_stp = {
 	.desc = IPSTATS_STAT_DESC_XSTATS_LEAF("stp"),
 	.xstats_at = IFLA_STATS_LINK_XSTATS_SLAVE,
 	.link_type_at = LINK_XSTATS_TYPE_BRIDGE,
-	.inner_max = BRIDGE_XSTATS_MAX,
 	.inner_at = BRIDGE_XSTATS_STP,
 	.show_cb = &bridge_print_stats_stp,
 };
@@ -1118,7 +1115,6 @@ ipstats_stat_desc_xstats_slave_bridge_mcast = {
 	.desc = IPSTATS_STAT_DESC_XSTATS_LEAF("mcast"),
 	.xstats_at = IFLA_STATS_LINK_XSTATS_SLAVE,
 	.link_type_at = LINK_XSTATS_TYPE_BRIDGE,
-	.inner_max = BRIDGE_XSTATS_MAX,
 	.inner_at = BRIDGE_XSTATS_MCAST,
 	.show_cb = &bridge_print_stats_mcast,
 };
-- 
2.49.0

[PATCH iproute2-next 3/4] lib: bridge: Add a module for bridge-related helpers

[Petr Machata]

`ip stats' displays a range of bridge_slave-related statistics, but not
the VLAN stats. `bridge vlan' actually has code to show these. Extract the
code to libutil so that it can be reused between the bridge and ip stats
tools.

Rename them reasonably so as not to litter the global namespace.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 bridge/vlan.c    | 50 +++++-------------------------------------------
 include/bridge.h | 11 +++++++++++
 lib/Makefile     |  3 ++-
 lib/bridge.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 46 deletions(-)
 create mode 100644 include/bridge.h
 create mode 100644 lib/bridge.c

diff --git a/bridge/vlan.c b/bridge/vlan.c
index ea4aff93..14b8475d 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -15,6 +15,7 @@
 #include "json_print.h"
 #include "libnetlink.h"
 #include "br_common.h"
+#include "bridge.h"
 #include "utils.h"
 
 static unsigned int filter_index, filter_vlan;
@@ -705,47 +706,6 @@ static int print_vlan(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
-static void print_vlan_flags(__u16 flags)
-{
-	if (flags == 0)
-		return;
-
-	open_json_array(PRINT_JSON, "flags");
-	if (flags & BRIDGE_VLAN_INFO_PVID)
-		print_string(PRINT_ANY, NULL, " %s", "PVID");
-
-	if (flags & BRIDGE_VLAN_INFO_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)
-{
-	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
-	print_lluint(PRINT_ANY, "rx_bytes", "RX: %llu bytes",
-		     vstats->rx_bytes);
-	print_lluint(PRINT_ANY, "rx_packets", " %llu packets\n",
-		     vstats->rx_packets);
-
-	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
-	print_lluint(PRINT_ANY, "tx_bytes", "TX: %llu bytes",
-		     vstats->tx_bytes);
-	print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n",
-		     vstats->tx_packets);
-}
-
-static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats)
-{
-	open_json_object(NULL);
-
-	print_hu(PRINT_ANY, "vid", "%hu", vstats->vid);
-	print_vlan_flags(vstats->flags);
-	print_nl();
-	__print_one_vlan_stats(vstats);
-
-	close_json_object();
-}
-
 static void print_vlan_stats_attr(struct rtattr *attr, int ifindex)
 {
 	struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
@@ -783,7 +743,7 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex)
 			print_string(PRINT_FP, NULL,
 				     "%-" textify(IFNAMSIZ) "s  ", "");
 		}
-		print_one_vlan_stats(vstats);
+		bridge_print_vlan_stats(vstats);
 	}
 
 	/* vlan_port is opened only if there are any vlan stats */
@@ -1025,7 +985,7 @@ static void print_vlan_opts(struct rtattr *a, int ifindex)
 		print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s  ", "");
 	}
 	print_range("vlan", vinfo->vid, vrange);
-	print_vlan_flags(vinfo->flags);
+	bridge_print_vlan_flags(vinfo->flags);
 	print_nl();
 	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
 	print_stp_state(state);
@@ -1051,7 +1011,7 @@ static void print_vlan_opts(struct rtattr *a, int ifindex)
 	}
 	print_nl();
 	if (show_stats)
-		__print_one_vlan_stats(&vstats);
+		bridge_print_vlan_stats_only(&vstats);
 	close_json_object();
 }
 
@@ -1334,7 +1294,7 @@ static void print_vlan_info(struct rtattr *tb, int ifindex)
 		open_json_object(NULL);
 		print_range("vlan", last_vid_start, vinfo->vid);
 
-		print_vlan_flags(vinfo->flags);
+		bridge_print_vlan_flags(vinfo->flags);
 		close_json_object();
 		print_nl();
 	}
diff --git a/include/bridge.h b/include/bridge.h
new file mode 100644
index 00000000..8bcd1e38
--- /dev/null
+++ b/include/bridge.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __BRIDGE_H__
+#define __BRIDGE_H__ 1
+
+#include <linux/if_bridge.h>
+
+void bridge_print_vlan_flags(__u16 flags);
+void bridge_print_vlan_stats_only(const struct bridge_vlan_xstats *vstats);
+void bridge_print_vlan_stats(const struct bridge_vlan_xstats *vstats);
+
+#endif /* __BRIDGE_H__ */
diff --git a/lib/Makefile b/lib/Makefile
index aa7bbd2e..0ba62942 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -5,7 +5,8 @@ CFLAGS += -fPIC
 
 UTILOBJ = utils.o utils_math.o rt_names.o ll_map.o ll_types.o ll_proto.o ll_addr.o \
 	inet_proto.o namespace.o json_writer.o json_print.o json_print_math.o \
-	names.o color.o bpf_legacy.o bpf_glue.o exec.o fs.o cg_map.o ppp_proto.o
+	names.o color.o bpf_legacy.o bpf_glue.o exec.o fs.o cg_map.o \
+	ppp_proto.o bridge.o
 
 ifeq ($(HAVE_ELF),y)
 ifeq ($(HAVE_LIBBPF),y)
diff --git a/lib/bridge.c b/lib/bridge.c
new file mode 100644
index 00000000..0a46b6a2
--- /dev/null
+++ b/lib/bridge.c
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <net/if.h>
+
+#include "bridge.h"
+#include "utils.h"
+
+void bridge_print_vlan_flags(__u16 flags)
+{
+	if (flags == 0)
+		return;
+
+	open_json_array(PRINT_JSON, "flags");
+	if (flags & BRIDGE_VLAN_INFO_PVID)
+		print_string(PRINT_ANY, NULL, " %s", "PVID");
+
+	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+		print_string(PRINT_ANY, NULL, " %s", "Egress Untagged");
+	close_json_array(PRINT_JSON, NULL);
+}
+
+void bridge_print_vlan_stats_only(const struct bridge_vlan_xstats *vstats)
+{
+	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
+	print_lluint(PRINT_ANY, "rx_bytes", "RX: %llu bytes",
+		     vstats->rx_bytes);
+	print_lluint(PRINT_ANY, "rx_packets", " %llu packets\n",
+		     vstats->rx_packets);
+
+	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
+	print_lluint(PRINT_ANY, "tx_bytes", "TX: %llu bytes",
+		     vstats->tx_bytes);
+	print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n",
+		     vstats->tx_packets);
+}
+
+void bridge_print_vlan_stats(const struct bridge_vlan_xstats *vstats)
+{
+	open_json_object(NULL);
+
+	print_hu(PRINT_ANY, "vid", "%hu", vstats->vid);
+	bridge_print_vlan_flags(vstats->flags);
+	print_nl();
+	bridge_print_vlan_stats_only(vstats);
+
+	close_json_object();
+}
-- 
2.49.0

[PATCH iproute2-next 4/4] ip: iplink_bridge: Support bridge VLAN stats in `ip stats'

[Petr Machata]

Add support for displaying bridge VLAN statistics in `ip stats'.
Reuse the existing `bridge vlan' display and JSON format:

 # ip stats show dev v2 group xstats_slave subgroup bridge suite vlan
 2: v2: group xstats_slave subgroup bridge suite vlan
                   10
                     RX: 776 bytes 10 packets
                     TX: 224 bytes 4 packets

                   20
                     RX: 684 bytes 7 packets
                     TX: 0 bytes 0 packets

 # ip -j -p stats show dev v2 group xstats_slave subgroup bridge suite vlan
 [ {
         "ifindex": 2,
         "ifname": "v2",
         "group": "xstats_slave",
         "subgroup": "bridge",
         "suite": "vlan",
         "vlans": [ {
                 "vid": 10,
                 "rx_bytes": 552,
                 "rx_packets": 6,
                 "tx_bytes": 0,
                 "tx_packets": 0
             },{
                 "vid": 20,
                 "rx_bytes": 684,
                 "rx_packets": 7,
                 "tx_bytes": 0,
                 "tx_packets": 0
             } ]
     } ]

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 ip/iplink_bridge.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 3d54e203..531c495d 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -14,6 +14,7 @@
 #include <linux/if_bridge.h>
 #include <net/if.h>
 
+#include "bridge.h"
 #include "rt_names.h"
 #include "utils.h"
 #include "ip_common.h"
@@ -978,6 +979,26 @@ static void bridge_print_stats_stp(const struct rtattr *attr)
 	close_json_object();
 }
 
+static void bridge_print_stats_vlan(const struct rtattr *attr)
+{
+	const struct bridge_vlan_xstats *vstats = RTA_DATA(attr);
+
+	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s  ", "");
+	bridge_print_vlan_stats(vstats);
+}
+
+static int bridge_stat_desc_show_xstats(struct ipstats_stat_show_attrs *attrs,
+					const struct ipstats_stat_desc *desc)
+{
+	int ret;
+
+	open_json_array(PRINT_JSON, "vlans");
+	ret = ipstats_stat_desc_show_xstats(attrs, desc);
+	close_json_array(PRINT_JSON, "vlans");
+
+	return ret;
+}
+
 static void bridge_print_stats_attr(struct rtattr *attr, int ifindex)
 {
 	struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
@@ -1119,10 +1140,25 @@ ipstats_stat_desc_xstats_slave_bridge_mcast = {
 	.show_cb = &bridge_print_stats_mcast,
 };
 
+static const struct ipstats_stat_desc_xstats
+ipstats_stat_desc_xstats_slave_bridge_vlan = {
+	.desc = {
+		.name = "vlan",
+		.kind = IPSTATS_STAT_DESC_KIND_LEAF,
+		.show = &bridge_stat_desc_show_xstats,
+		.pack = &ipstats_stat_desc_pack_xstats,
+	},
+	.xstats_at = IFLA_STATS_LINK_XSTATS_SLAVE,
+	.link_type_at = LINK_XSTATS_TYPE_BRIDGE,
+	.inner_at = BRIDGE_XSTATS_VLAN,
+	.show_cb = &bridge_print_stats_vlan,
+};
+
 static const struct ipstats_stat_desc *
 ipstats_stat_desc_xstats_slave_bridge_subs[] = {
 	&ipstats_stat_desc_xstats_slave_bridge_stp.desc,
 	&ipstats_stat_desc_xstats_slave_bridge_mcast.desc,
+	&ipstats_stat_desc_xstats_slave_bridge_vlan.desc,
 };
 
 const struct ipstats_stat_desc ipstats_stat_desc_xstats_slave_bridge_group = {
-- 
2.49.0

Re: [PATCH iproute2-next 3/4] lib: bridge: Add a module for bridge-related helpers

[Nikolay Aleksandrov]

On 6/6/25 18:04, Petr Machata wrote:
> `ip stats' displays a range of bridge_slave-related statistics, but not
> the VLAN stats. `bridge vlan' actually has code to show these. Extract the
> code to libutil so that it can be reused between the bridge and ip stats
> tools.
> 
> Rename them reasonably so as not to litter the global namespace.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>   bridge/vlan.c    | 50 +++++-------------------------------------------
>   include/bridge.h | 11 +++++++++++
>   lib/Makefile     |  3 ++-
>   lib/bridge.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 65 insertions(+), 46 deletions(-)
>   create mode 100644 include/bridge.h
>   create mode 100644 lib/bridge.c
> 

lol, Ido didn't we just have a discussion in another thread about adding
a bridge lib? :-)

https://www.spinics.net/lists/netdev/msg1096403.html

I'm not strictly against adding a lib, I think this approach might
actually save us from the tools going out of sync and we can avoid
repeating to people they have to update both places when they forget.

Cheers,
  Nik

> diff --git a/bridge/vlan.c b/bridge/vlan.c
> index ea4aff93..14b8475d 100644
> --- a/bridge/vlan.c
> +++ b/bridge/vlan.c
> @@ -15,6 +15,7 @@
>   #include "json_print.h"
>   #include "libnetlink.h"
>   #include "br_common.h"
> +#include "bridge.h"
>   #include "utils.h"
>   
>   static unsigned int filter_index, filter_vlan;
> @@ -705,47 +706,6 @@ static int print_vlan(struct nlmsghdr *n, void *arg)
>   	return 0;
>   }
>   
> -static void print_vlan_flags(__u16 flags)
> -{
> -	if (flags == 0)
> -		return;
> -
> -	open_json_array(PRINT_JSON, "flags");
> -	if (flags & BRIDGE_VLAN_INFO_PVID)
> -		print_string(PRINT_ANY, NULL, " %s", "PVID");
> -
> -	if (flags & BRIDGE_VLAN_INFO_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)
> -{
> -	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
> -	print_lluint(PRINT_ANY, "rx_bytes", "RX: %llu bytes",
> -		     vstats->rx_bytes);
> -	print_lluint(PRINT_ANY, "rx_packets", " %llu packets\n",
> -		     vstats->rx_packets);
> -
> -	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
> -	print_lluint(PRINT_ANY, "tx_bytes", "TX: %llu bytes",
> -		     vstats->tx_bytes);
> -	print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n",
> -		     vstats->tx_packets);
> -}
> -
> -static void print_one_vlan_stats(const struct bridge_vlan_xstats *vstats)
> -{
> -	open_json_object(NULL);
> -
> -	print_hu(PRINT_ANY, "vid", "%hu", vstats->vid);
> -	print_vlan_flags(vstats->flags);
> -	print_nl();
> -	__print_one_vlan_stats(vstats);
> -
> -	close_json_object();
> -}
> -
>   static void print_vlan_stats_attr(struct rtattr *attr, int ifindex)
>   {
>   	struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
> @@ -783,7 +743,7 @@ static void print_vlan_stats_attr(struct rtattr *attr, int ifindex)
>   			print_string(PRINT_FP, NULL,
>   				     "%-" textify(IFNAMSIZ) "s  ", "");
>   		}
> -		print_one_vlan_stats(vstats);
> +		bridge_print_vlan_stats(vstats);
>   	}
>   
>   	/* vlan_port is opened only if there are any vlan stats */
> @@ -1025,7 +985,7 @@ static void print_vlan_opts(struct rtattr *a, int ifindex)
>   		print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s  ", "");
>   	}
>   	print_range("vlan", vinfo->vid, vrange);
> -	print_vlan_flags(vinfo->flags);
> +	bridge_print_vlan_flags(vinfo->flags);
>   	print_nl();
>   	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
>   	print_stp_state(state);
> @@ -1051,7 +1011,7 @@ static void print_vlan_opts(struct rtattr *a, int ifindex)
>   	}
>   	print_nl();
>   	if (show_stats)
> -		__print_one_vlan_stats(&vstats);
> +		bridge_print_vlan_stats_only(&vstats);
>   	close_json_object();
>   }
>   
> @@ -1334,7 +1294,7 @@ static void print_vlan_info(struct rtattr *tb, int ifindex)
>   		open_json_object(NULL);
>   		print_range("vlan", last_vid_start, vinfo->vid);
>   
> -		print_vlan_flags(vinfo->flags);
> +		bridge_print_vlan_flags(vinfo->flags);
>   		close_json_object();
>   		print_nl();
>   	}
> diff --git a/include/bridge.h b/include/bridge.h
> new file mode 100644
> index 00000000..8bcd1e38
> --- /dev/null
> +++ b/include/bridge.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __BRIDGE_H__
> +#define __BRIDGE_H__ 1
> +
> +#include <linux/if_bridge.h>
> +
> +void bridge_print_vlan_flags(__u16 flags);
> +void bridge_print_vlan_stats_only(const struct bridge_vlan_xstats *vstats);
> +void bridge_print_vlan_stats(const struct bridge_vlan_xstats *vstats);
> +
> +#endif /* __BRIDGE_H__ */
> diff --git a/lib/Makefile b/lib/Makefile
> index aa7bbd2e..0ba62942 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -5,7 +5,8 @@ CFLAGS += -fPIC
>   
>   UTILOBJ = utils.o utils_math.o rt_names.o ll_map.o ll_types.o ll_proto.o ll_addr.o \
>   	inet_proto.o namespace.o json_writer.o json_print.o json_print_math.o \
> -	names.o color.o bpf_legacy.o bpf_glue.o exec.o fs.o cg_map.o ppp_proto.o
> +	names.o color.o bpf_legacy.o bpf_glue.o exec.o fs.o cg_map.o \
> +	ppp_proto.o bridge.o
>   
>   ifeq ($(HAVE_ELF),y)
>   ifeq ($(HAVE_LIBBPF),y)
> diff --git a/lib/bridge.c b/lib/bridge.c
> new file mode 100644
> index 00000000..0a46b6a2
> --- /dev/null
> +++ b/lib/bridge.c
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <net/if.h>
> +
> +#include "bridge.h"
> +#include "utils.h"
> +
> +void bridge_print_vlan_flags(__u16 flags)
> +{
> +	if (flags == 0)
> +		return;
> +
> +	open_json_array(PRINT_JSON, "flags");
> +	if (flags & BRIDGE_VLAN_INFO_PVID)
> +		print_string(PRINT_ANY, NULL, " %s", "PVID");
> +
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		print_string(PRINT_ANY, NULL, " %s", "Egress Untagged");
> +	close_json_array(PRINT_JSON, NULL);
> +}
> +
> +void bridge_print_vlan_stats_only(const struct bridge_vlan_xstats *vstats)
> +{
> +	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
> +	print_lluint(PRINT_ANY, "rx_bytes", "RX: %llu bytes",
> +		     vstats->rx_bytes);
> +	print_lluint(PRINT_ANY, "rx_packets", " %llu packets\n",
> +		     vstats->rx_packets);
> +
> +	print_string(PRINT_FP, NULL, "%-" textify(IFNAMSIZ) "s    ", "");
> +	print_lluint(PRINT_ANY, "tx_bytes", "TX: %llu bytes",
> +		     vstats->tx_bytes);
> +	print_lluint(PRINT_ANY, "tx_packets", " %llu packets\n",
> +		     vstats->tx_packets);
> +}
> +
> +void bridge_print_vlan_stats(const struct bridge_vlan_xstats *vstats)
> +{
> +	open_json_object(NULL);
> +
> +	print_hu(PRINT_ANY, "vid", "%hu", vstats->vid);
> +	bridge_print_vlan_flags(vstats->flags);
> +	print_nl();
> +	bridge_print_vlan_stats_only(vstats);
> +
> +	close_json_object();
> +}

Re: [PATCH iproute2-next 1/4] ip: ipstats: Iterate all xstats attributes

[Ido Schimmel]

On Fri, Jun 06, 2025 at 05:04:50PM +0200, Petr Machata wrote:
> @@ -600,15 +601,14 @@ int ipstats_stat_desc_show_xstats(struct ipstats_stat_show_attrs *attrs,
>  	if (at == NULL)
>  		return err;
>  
> -	tb = alloca(sizeof(*tb) * (xdesc->inner_max + 1));
> -	err = parse_rtattr_nested(tb, xdesc->inner_max, at);
> -	if (err != 0)
> -		return err;
> -
> -	if (tb[xdesc->inner_at] != NULL) {
> -		print_nl();
> -		xdesc->show_cb(tb[xdesc->inner_at]);
> +	rem = RTA_PAYLOAD(at);
> +	for (i = RTA_DATA(at); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {

Use rtattr_for_each_nested() ?

> +		if (i->rta_type == xdesc->inner_at) {
> +			print_nl();
> +			xdesc->show_cb(i);
> +		}
>  	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.49.0
> 

Re: [PATCH iproute2-next 2/4] ip: ip_common: Drop ipstats_stat_desc_xstats::inner_max

[Ido Schimmel]

On Fri, Jun 06, 2025 at 05:04:51PM +0200, Petr Machata wrote:
> After the previous patch, this field is not read anymore. Drop it.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH iproute2-next 3/4] lib: bridge: Add a module for bridge-related helpers

[Ido Schimmel]

On Fri, Jun 06, 2025 at 05:04:52PM +0200, Petr Machata wrote:
> `ip stats' displays a range of bridge_slave-related statistics, but not
> the VLAN stats. `bridge vlan' actually has code to show these. Extract the
> code to libutil so that it can be reused between the bridge and ip stats
> tools.
> 
> Rename them reasonably so as not to litter the global namespace.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>  bridge/vlan.c    | 50 +++++-------------------------------------------
>  include/bridge.h | 11 +++++++++++
>  lib/Makefile     |  3 ++-
>  lib/bridge.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 65 insertions(+), 46 deletions(-)
>  create mode 100644 include/bridge.h
>  create mode 100644 lib/bridge.c

Add file entry to MAINTAINERS?

Re: [PATCH iproute2-next 4/4] ip: iplink_bridge: Support bridge VLAN stats in `ip stats'

[Ido Schimmel]

On Fri, Jun 06, 2025 at 05:04:53PM +0200, Petr Machata wrote:
> +static const struct ipstats_stat_desc_xstats
> +ipstats_stat_desc_xstats_slave_bridge_vlan = {
> +	.desc = {
> +		.name = "vlan",
> +		.kind = IPSTATS_STAT_DESC_KIND_LEAF,
> +		.show = &bridge_stat_desc_show_xstats,
> +		.pack = &ipstats_stat_desc_pack_xstats,
> +	},
> +	.xstats_at = IFLA_STATS_LINK_XSTATS_SLAVE,

This will only show VLAN stats for bridge ports, but they also exist for
the bridge itself inside the IFLA_STATS_LINK_XSTATS nest

> +	.link_type_at = LINK_XSTATS_TYPE_BRIDGE,
> +	.inner_at = BRIDGE_XSTATS_VLAN,
> +	.show_cb = &bridge_print_stats_vlan,
> +};

Re: [PATCH iproute2-next 3/4] lib: bridge: Add a module for bridge-related helpers

[Petr Machata]

Nikolay Aleksandrov <razor@blackwall.org> writes:

> On 6/6/25 18:04, Petr Machata wrote:
>> `ip stats' displays a range of bridge_slave-related statistics, but not
>> the VLAN stats. `bridge vlan' actually has code to show these. Extract the
>> code to libutil so that it can be reused between the bridge and ip stats
>> tools.
>> Rename them reasonably so as not to litter the global namespace.
>> Signed-off-by: Petr Machata <petrm@nvidia.com>
>> ---
>>   bridge/vlan.c    | 50 +++++-------------------------------------------
>>   include/bridge.h | 11 +++++++++++
>>   lib/Makefile     |  3 ++-
>>   lib/bridge.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 65 insertions(+), 46 deletions(-)
>>   create mode 100644 include/bridge.h
>>   create mode 100644 lib/bridge.c
>> 
>
> lol, Ido didn't we just have a discussion in another thread about adding
> a bridge lib? :-)
>
> https://www.spinics.net/lists/netdev/msg1096403.html
>
> I'm not strictly against adding a lib, I think this approach might
> actually save us from the tools going out of sync and we can avoid
> repeating to people they have to update both places when they forget.

Fun. I really think reuse is better in this case. Or, you know, by and
large. But particularly having both bridge and ip stats give the data
with the same structure is valuable. But y'all's call.

Re: [PATCH iproute2-next 1/4] ip: ipstats: Iterate all xstats attributes

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> On Fri, Jun 06, 2025 at 05:04:50PM +0200, Petr Machata wrote:
>> @@ -600,15 +601,14 @@ int ipstats_stat_desc_show_xstats(struct ipstats_stat_show_attrs *attrs,
>>  	if (at == NULL)
>>  		return err;
>>  
>> -	tb = alloca(sizeof(*tb) * (xdesc->inner_max + 1));
>> -	err = parse_rtattr_nested(tb, xdesc->inner_max, at);
>> -	if (err != 0)
>> -		return err;
>> -
>> -	if (tb[xdesc->inner_at] != NULL) {
>> -		print_nl();
>> -		xdesc->show_cb(tb[xdesc->inner_at]);
>> +	rem = RTA_PAYLOAD(at);
>> +	for (i = RTA_DATA(at); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
>
> Use rtattr_for_each_nested() ?

Better.

Re: [PATCH iproute2-next 4/4] ip: iplink_bridge: Support bridge VLAN stats in `ip stats'

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> On Fri, Jun 06, 2025 at 05:04:53PM +0200, Petr Machata wrote:
>> +static const struct ipstats_stat_desc_xstats
>> +ipstats_stat_desc_xstats_slave_bridge_vlan = {
>> +	.desc = {
>> +		.name = "vlan",
>> +		.kind = IPSTATS_STAT_DESC_KIND_LEAF,
>> +		.show = &bridge_stat_desc_show_xstats,
>> +		.pack = &ipstats_stat_desc_pack_xstats,
>> +	},
>> +	.xstats_at = IFLA_STATS_LINK_XSTATS_SLAVE,
>
> This will only show VLAN stats for bridge ports, but they also exist for
> the bridge itself inside the IFLA_STATS_LINK_XSTATS nest

I got confused by bridge -s vlan show dev br1 not actually showing
anything for VLAN-aware bridges. Seems like the corresponding data is
not even arriving from the kernel. Anyway, when I add an entry for
XSTATS (sans SLAVE) to ipstats it does actually work.

Re: [PATCH iproute2-next 3/4] lib: bridge: Add a module for bridge-related helpers

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> On Fri, Jun 06, 2025 at 05:04:52PM +0200, Petr Machata wrote:
>> `ip stats' displays a range of bridge_slave-related statistics, but not
>> the VLAN stats. `bridge vlan' actually has code to show these. Extract the
>> code to libutil so that it can be reused between the bridge and ip stats
>> tools.
>> 
>> Rename them reasonably so as not to litter the global namespace.
>> 
>> Signed-off-by: Petr Machata <petrm@nvidia.com>
>> ---
>>  bridge/vlan.c    | 50 +++++-------------------------------------------
>>  include/bridge.h | 11 +++++++++++
>>  lib/Makefile     |  3 ++-
>>  lib/bridge.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 65 insertions(+), 46 deletions(-)
>>  create mode 100644 include/bridge.h
>>  create mode 100644 lib/bridge.c
>
> Add file entry to MAINTAINERS?

OK