[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aEFmoU9_ci0rqQTQ@shredder>
Date: Thu, 5 Jun 2025 12:42:57 +0300
From: Ido Schimmel <idosch@...dia.com>
To: Fabian Pfitzner <f.pfitzner@...gutronix.de>, razor@...ckwall.org
Cc: netdev@...r.kernel.org, dsahern@...il.com,
bridge@...ts.linux-foundation.org, entwicklung@...gutronix.de
Subject: Re: [PATCH] bridge: dump mcast querier state per vlan
+ Nik
On Wed, Jun 04, 2025 at 12:53:23PM +0200, Fabian Pfitzner wrote:
> Dump the multicast querier state per vlan.
> This commit is almost identical to [1].
>
> The querier state can be seen with:
>
> bridge -d vlan global
>
> The options for vlan filtering and vlan mcast snooping have to be enabled
> in order to see the output:
>
> ip link set [dev] type bridge mcast_vlan_snooping 1 vlan_filtering 1
>
> The querier state shows the following information for IPv4 and IPv6
> respectively:
>
> 1) The ip address of the current querier in the network. This could be
> ourselves or an external querier.
> 2) The port on which the querier was seen
> 3) Querier timeout in seconds
>
> [1] https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=16aa4494d7fc6543e5e92beb2ce01648b79f8fa2
>
> Signed-off-by: Fabian Pfitzner <f.pfitzner@...gutronix.de>
> ---
>
> This patch is a bit redundant compared to [1]. It makes sense to put it
> into a helper function, but i am not sure where to place this function.
> Maybe somewhere under /lib?
Not sure it's appropriate to put this in lib. Given this duplication is
not new (see ip/iplink_bridge_slave.c and bridge/link.c, for example)
and that the code isn't complex, I would keep it as-is.
>
> bridge/vlan.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
>
> diff --git a/bridge/vlan.c b/bridge/vlan.c
> index ea4aff93..b928c653 100644
> --- a/bridge/vlan.c
> +++ b/bridge/vlan.c
> @@ -892,6 +892,64 @@ static void print_vlan_global_opts(struct rtattr *a, int ifindex)
> print_uint(PRINT_ANY, "mcast_querier", "mcast_querier %u ",
> rta_getattr_u8(vattr));
> }
> + if (vtb[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE]) {
> + struct rtattr *bqtb[BRIDGE_QUERIER_MAX + 1];
> + SPRINT_BUF(other_time);
> +
> + parse_rtattr_nested(bqtb, BRIDGE_QUERIER_MAX, vtb[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE]);
> + memset(other_time, 0, sizeof(other_time));
> +
> + open_json_object("mcast_querier_state_ipv4");
> + if (bqtb[BRIDGE_QUERIER_IP_ADDRESS]) {
> + print_string(PRINT_FP,
> + NULL,
> + "%s ",
> + "mcast_querier_ipv4_addr");
Is there a reason for this misalignment and the overly long lines?
How about something like [1] instead (compile tested only)?
> + print_color_string(PRINT_ANY,
> + COLOR_INET,
> + "mcast_querier_ipv4_addr",
> + "%s ",
> + format_host_rta(AF_INET, bqtb[BRIDGE_QUERIER_IP_ADDRESS]));
> + }
> + if (bqtb[BRIDGE_QUERIER_IP_PORT])
> + print_uint(PRINT_ANY,
> + "mcast_querier_ipv4_port",
> + "mcast_querier_ipv4_port %u ",
> + rta_getattr_u32(bqtb[BRIDGE_QUERIER_IP_PORT]));
> + if (bqtb[BRIDGE_QUERIER_IP_OTHER_TIMER])
> + print_string(PRINT_ANY,
> + "mcast_querier_ipv4_other_timer",
> + "mcast_querier_ipv4_other_timer %s ",
> + sprint_time64(
> + rta_getattr_u64(bqtb[BRIDGE_QUERIER_IP_OTHER_TIMER]),
> + other_time));
> + close_json_object();
> + open_json_object("mcast_querier_state_ipv6");
> + if (bqtb[BRIDGE_QUERIER_IPV6_ADDRESS]) {
> + print_string(PRINT_FP,
> + NULL,
> + "%s ",
> + "mcast_querier_ipv6_addr");
> + print_color_string(PRINT_ANY,
> + COLOR_INET6,
> + "mcast_querier_ipv6_addr",
> + "%s ",
> + format_host_rta(AF_INET6, bqtb[BRIDGE_QUERIER_IPV6_ADDRESS]));
> + }
> + if (bqtb[BRIDGE_QUERIER_IPV6_PORT])
> + print_uint(PRINT_ANY,
> + "mcast_querier_ipv6_port",
> + "mcast_querier_ipv6_port %u ",
> + rta_getattr_u32(bqtb[BRIDGE_QUERIER_IPV6_PORT]));
> + if (bqtb[BRIDGE_QUERIER_IPV6_OTHER_TIMER])
> + print_string(PRINT_ANY,
> + "mcast_querier_ipv6_other_timer",
> + "mcast_querier_ipv6_other_timer %s ",
> + sprint_time64(
> + rta_getattr_u64(bqtb[BRIDGE_QUERIER_IPV6_OTHER_TIMER]),
> + other_time));
> + close_json_object();
> + }
> if (vtb[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION]) {
> vattr = vtb[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION];
> print_uint(PRINT_ANY, "mcast_igmp_version",
[1]
diff --git a/bridge/vlan.c b/bridge/vlan.c
index ea4aff931a22..2afdc7c72890 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -892,6 +892,61 @@ static void print_vlan_global_opts(struct rtattr *a, int ifindex)
print_uint(PRINT_ANY, "mcast_querier", "mcast_querier %u ",
rta_getattr_u8(vattr));
}
+ if (vtb[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE]) {
+ struct rtattr *bqtb[BRIDGE_QUERIER_MAX + 1];
+ const char *querier_ip;
+ SPRINT_BUF(other_time);
+ __u64 tval;
+
+ parse_rtattr_nested(bqtb, BRIDGE_QUERIER_MAX,
+ vtb[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE]);
+ memset(other_time, 0, sizeof(other_time));
+
+ open_json_object("mcast_querier_state_ipv4");
+ if (bqtb[BRIDGE_QUERIER_IP_ADDRESS]) {
+ querier_ip = format_host_rta(AF_INET,
+ bqtb[BRIDGE_QUERIER_IP_ADDRESS]);
+ print_string(PRINT_FP, NULL, "%s ",
+ "mcast_querier_ipv4_addr");
+ print_color_string(PRINT_ANY, COLOR_INET,
+ "mcast_querier_ipv4_addr", "%s ",
+ querier_ip);
+ }
+ if (bqtb[BRIDGE_QUERIER_IP_PORT])
+ print_uint(PRINT_ANY, "mcast_querier_ipv4_port",
+ "mcast_querier_ipv4_port %u ",
+ rta_getattr_u32(bqtb[BRIDGE_QUERIER_IP_PORT]));
+ if (bqtb[BRIDGE_QUERIER_IP_OTHER_TIMER]) {
+ tval = rta_getattr_u64(bqtb[BRIDGE_QUERIER_IP_OTHER_TIMER]);
+ print_string(PRINT_ANY,
+ "mcast_querier_ipv4_other_timer",
+ "mcast_querier_ipv4_other_timer %s ",
+ sprint_time64(tval, other_time));
+ }
+ close_json_object();
+ open_json_object("mcast_querier_state_ipv6");
+ if (bqtb[BRIDGE_QUERIER_IPV6_ADDRESS]) {
+ querier_ip = format_host_rta(AF_INET6,
+ bqtb[BRIDGE_QUERIER_IPV6_ADDRESS]);
+ print_string(PRINT_FP, NULL, "%s ",
+ "mcast_querier_ipv6_addr");
+ print_color_string(PRINT_ANY, COLOR_INET6,
+ "mcast_querier_ipv6_addr", "%s ",
+ querier_ip);
+ }
+ if (bqtb[BRIDGE_QUERIER_IPV6_PORT])
+ print_uint(PRINT_ANY, "mcast_querier_ipv6_port",
+ "mcast_querier_ipv6_port %u ",
+ rta_getattr_u32(bqtb[BRIDGE_QUERIER_IPV6_PORT]));
+ if (bqtb[BRIDGE_QUERIER_IPV6_OTHER_TIMER]) {
+ tval = rta_getattr_u64(bqtb[BRIDGE_QUERIER_IPV6_OTHER_TIMER]);
+ print_string(PRINT_ANY,
+ "mcast_querier_ipv6_other_timer",
+ "mcast_querier_ipv6_other_timer %s ",
+ sprint_time64(tval, other_time));
+ }
+ close_json_object();
+ }
if (vtb[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION]) {
vattr = vtb[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION];
print_uint(PRINT_ANY, "mcast_igmp_version",
Powered by blists - more mailing lists