| 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
| ||
|
Message-ID: <dbc2fcc9-5cd9-43c9-a9f9-9d5d16bb81b4@pengutronix.de>
Date: Mon, 23 Jun 2025 09:45:21 +0200
From: Fabian Pfitzner <f.pfitzner@...gutronix.de>
To: Nikolay Aleksandrov <razor@...ckwall.org>, netdev@...r.kernel.org
Cc: dsahern@...il.com, idosch@...dia.com, bridge@...ts.linux-foundation.org,
entwicklung@...gutronix.de
Subject: Re: [PATCH v3] bridge: dump mcast querier state per vlan
On 6/20/25 14:44, Nikolay Aleksandrov wrote:
> On 6/20/25 15:16, 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>
>> ---
>>
>> v1->v2
>> - refactor code
>> - link to v1: https://lore.kernel.org/netdev/20250604105322.1185872-1-f.pfitzner@pengutronix.de/
>>
>> v2->v3
>> - move code into a shared function
>> - use shared function in bridge and ip utility
>> - link to v2: https://lore.kernel.org/netdev/20250611121151.1660231-1-f.pfitzner@pengutronix.de/
>> ---
>> bridge/vlan.c | 3 +++
>> include/bridge.h | 3 +++
>> ip/iplink_bridge.c | 57 +---------------------------------------------
>> lib/bridge.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 63 insertions(+), 56 deletions(-)
>>
> Hi,
> The subject should contain the target for this patch which is iproute2-next,
> e.g. [PATCH iproute2-next v3]. Since there would be another version, I'd split
> it in 2 patches - 1 that moves the existing code to lib/bridge.c and the second
> which adds the vlan querier print code.
>
> Also a few comments below..
I'll split it into three commits then:
1. Move existing code into shared function
2. Call shared function in bridge/vlan.c as well
3. Refactor code according to Ido's code proposal from v1
>
>> diff --git a/bridge/vlan.c b/bridge/vlan.c
>> index 14b8475d..0233eaf6 100644
>> --- a/bridge/vlan.c
>> +++ b/bridge/vlan.c
>> @@ -852,6 +852,9 @@ 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]) {
>> + dump_mcast_querier_state(vtb[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE]);
> Add a local variable for the attribute and make this line shorter (<= 80chars).
>
>> + }
>> if (vtb[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION]) {
>> vattr = vtb[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION];
>> print_uint(PRINT_ANY, "mcast_igmp_version",
>> diff --git a/include/bridge.h b/include/bridge.h
>> index 8bcd1e38..9e9447c6 100644
>> --- a/include/bridge.h
>> +++ b/include/bridge.h
>> @@ -3,9 +3,12 @@
>> #define __BRIDGE_H__ 1
>>
>> #include <linux/if_bridge.h>
>> +#include <linux/rtnetlink.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);
>>
>> +void dump_mcast_querier_state(const struct rtattr* vtb);
>> +
>> #endif /* __BRIDGE_H__ */
>> diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
>> index 31e7cb5e..68a277ef 100644
>> --- a/ip/iplink_bridge.c
>> +++ b/ip/iplink_bridge.c
>> @@ -682,62 +682,7 @@ static void bridge_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>> rta_getattr_u8(tb[IFLA_BR_MCAST_QUERIER]));
>>
>> if (tb[IFLA_BR_MCAST_QUERIER_STATE]) {
>> - struct rtattr *bqtb[BRIDGE_QUERIER_MAX + 1];
>> - SPRINT_BUF(other_time);
>> -
>> - parse_rtattr_nested(bqtb, BRIDGE_QUERIER_MAX, tb[IFLA_BR_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");
>> - 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();
>> + dump_mcast_querier_state(tb[IFLA_BR_MCAST_QUERIER_STATE]);
>> }
>>
>> if (tb[IFLA_BR_MCAST_HASH_ELASTICITY])
>> diff --git a/lib/bridge.c b/lib/bridge.c
>> index a888a20e..13b0d633 100644
>> --- a/lib/bridge.c
>> +++ b/lib/bridge.c
>> @@ -45,3 +45,59 @@ void bridge_print_vlan_stats(const struct bridge_vlan_xstats *vstats)
>>
>> close_json_object();
>> }
>> +
>> +void dump_mcast_querier_state(const struct rtattr *vtb)
> Stay consistent with the rest of lib/bridge.c and add bridge_ in front of the name.
> And maybe bridge_print_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);
>> + 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");
> formatting is off, "mcast_querier_ipv4_addr" should start under PRINT_FP
> use tabs as much as possible (before going over) and if
> needed finish alignment with spaces, e.g.:
Hmm, for some reason my editor replaced the spaces with tabs when I
copied Ido's proposal.
Did not notice that... Sorry. I'll send a new patch.
>
> querier_ip = format_host_rta(AF_INET,
> bqtb[BRIDGE_QUERIER_IP_ADDRESS]);
> or
>
> print_string(PRINT_FP, NULL, "%s ",
> "mcast_querier_ipv4_addr");
>
>> + print_color_string(PRINT_ANY, COLOR_INET,
>> + "mcast_querier_ipv4_addr", "%s ",
>> + querier_ip);
> same about formatting
>
>> + }
>> + 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]));
> formatting
>
>> + 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));
> formatting
>
>> + }
>> + 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);
> formatting
>> + }
>> + 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]));
> formatting
>> + 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));
> formatting
>
>> + }
>> + close_json_object();
>> +}
>> --
>> 2.39.5
>>
>
--
Pengutronix e.K. | Fabian Pfitzner |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Powered by blists - more mailing lists