diff mbox series

[iproute2,1/4] bridge: Do not print stray prefixes in monitor mode

Message ID 20220922061938.202705-2-bpoirier@nvidia.com (mailing list archive)
State Accepted
Delegated to: Stephen Hemminger
Headers show
Series monitor command fixes | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Benjamin Poirier Sept. 22, 2022, 6:19 a.m. UTC
When using `bridge monitor` with the '-timestamp' option or the "all"
parameter, prefixes are printed before the actual event descriptions.
Currently, those prefixes are printed for each netlink message that's
received. However, some netlink messages do not lead to an event
description being printed. That's usually because a message is not related
to AF_BRIDGE. This results in stray prefixes being printed.

Restructure accept_msg() and its callees such that prefixes are only
printed after a message has been checked for eligibility.

The issue can be witnessed using the following commands:
	ip link add dummy0 type dummy
	# Start `bridge monitor all` now in another terminal.
	# Cause a stray "[LINK]" to be printed (family 10).
	# It does not appear yet because the output is line buffered.
	ip link set dev dummy0 up
	# Cause a stray "[NEIGH]" to be printed (family 2).
	ip neigh add 10.0.0.1 lladdr 02:00:00:00:00:01 dev dummy0
	# Cause a genuine entry to be printed, which flushes the previous
	# output.
	bridge fdb add 02:00:00:00:00:01 dev dummy0
	# We now see:
	# [LINK][NEIGH][NEIGH]02:00:00:00:00:01 dev dummy0 self permanent

Fixes: d04bc300c3e3 ("Add bridge command")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
 bridge/br_common.h |  1 +
 bridge/fdb.c       |  3 +++
 bridge/link.c      |  4 ++++
 bridge/mdb.c       |  3 +++
 bridge/monitor.c   | 35 ++++++++++-------------------------
 bridge/vlan.c      |  4 ++++
 bridge/vni.c       |  4 ++++
 7 files changed, 29 insertions(+), 25 deletions(-)

Comments

Stephen Hemminger Sept. 22, 2022, 3:28 p.m. UTC | #1
On Thu, 22 Sep 2022 15:19:35 +0900
Benjamin Poirier <bpoirier@nvidia.com> wrote:

> When using `bridge monitor` with the '-timestamp' option or the "all"
> parameter, prefixes are printed before the actual event descriptions.
> Currently, those prefixes are printed for each netlink message that's
> received. However, some netlink messages do not lead to an event
> description being printed. That's usually because a message is not related
> to AF_BRIDGE. This results in stray prefixes being printed.
> 
> Restructure accept_msg() and its callees such that prefixes are only
> printed after a message has been checked for eligibility.
> 
> The issue can be witnessed using the following commands:
> 	ip link add dummy0 type dummy
> 	# Start `bridge monitor all` now in another terminal.
> 	# Cause a stray "[LINK]" to be printed (family 10).
> 	# It does not appear yet because the output is line buffered.
> 	ip link set dev dummy0 up
> 	# Cause a stray "[NEIGH]" to be printed (family 2).
> 	ip neigh add 10.0.0.1 lladdr 02:00:00:00:00:01 dev dummy0
> 	# Cause a genuine entry to be printed, which flushes the previous
> 	# output.
> 	bridge fdb add 02:00:00:00:00:01 dev dummy0
> 	# We now see:
> 	# [LINK][NEIGH][NEIGH]02:00:00:00:00:01 dev dummy0 self permanent
> 
> Fixes: d04bc300c3e3 ("Add bridge command")
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> ---

Looks good, reminds me that the bridge command need to be converted
to use json_print.
diff mbox series

Patch

diff --git a/bridge/br_common.h b/bridge/br_common.h
index 841f0594..da677df8 100644
--- a/bridge/br_common.h
+++ b/bridge/br_common.h
@@ -16,6 +16,7 @@  int print_vlan_rtm(struct nlmsghdr *n, void *arg, bool monitor,
 		   bool global_only);
 int print_vnifilter_rtm(struct nlmsghdr *n, void *arg, bool monitor);
 void br_print_router_port_stats(struct rtattr *pattr);
+void print_headers(FILE *fp, const char *label);
 
 int do_fdb(int argc, char **argv);
 int do_mdb(int argc, char **argv);
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 5f71bde0..775feb12 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -179,6 +179,8 @@  int print_fdb(struct nlmsghdr *n, void *arg)
 	if (filter_dynamic && (r->ndm_state & NUD_PERMANENT))
 		return 0;
 
+	print_headers(fp, "[NEIGH]");
+
 	open_json_object(NULL);
 	if (n->nlmsg_type == RTM_DELNEIGH)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
@@ -810,6 +812,7 @@  static int fdb_flush(int argc, char **argv)
 int do_fdb(int argc, char **argv)
 {
 	ll_init_map(&rth);
+	timestamp = 0;
 
 	if (argc > 0) {
 		if (matches(*argv, "add") == 0)
diff --git a/bridge/link.c b/bridge/link.c
index 3810fa04..fef3a9ef 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -230,6 +230,8 @@  int print_linkinfo(struct nlmsghdr *n, void *arg)
 	if (!name)
 		return -1;
 
+	print_headers(fp, "[LINK]");
+
 	open_json_object(NULL);
 	if (n->nlmsg_type == RTM_DELLINK)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
@@ -588,6 +590,8 @@  static int brlink_show(int argc, char **argv)
 int do_link(int argc, char **argv)
 {
 	ll_init_map(&rth);
+	timestamp = 0;
+
 	if (argc > 0) {
 		if (matches(*argv, "set") == 0 ||
 		    matches(*argv, "change") == 0)
diff --git a/bridge/mdb.c b/bridge/mdb.c
index 7b5863d3..d3afc900 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -380,6 +380,8 @@  int print_mdb_mon(struct nlmsghdr *n, void *arg)
 	if (ret != 1)
 		return ret;
 
+	print_headers(fp, "[MDB]");
+
 	if (n->nlmsg_type == RTM_DELMDB)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
@@ -564,6 +566,7 @@  static int mdb_modify(int cmd, int flags, int argc, char **argv)
 int do_mdb(int argc, char **argv)
 {
 	ll_init_map(&rth);
+	timestamp = 0;
 
 	if (argc > 0) {
 		if (matches(*argv, "add") == 0)
diff --git a/bridge/monitor.c b/bridge/monitor.c
index f17c1906..e321516a 100644
--- a/bridge/monitor.c
+++ b/bridge/monitor.c
@@ -35,42 +35,22 @@  static void usage(void)
 	exit(-1);
 }
 
-static int print_tunnel_rtm(struct nlmsghdr *n, void *arg, bool monitor)
-{
-	struct tunnel_msg *tmsg = NLMSG_DATA(n);
-
-	if (tmsg->family == PF_BRIDGE)
-		return print_vnifilter_rtm(n, arg, monitor);
-
-	return 0;
-}
-
 static int accept_msg(struct rtnl_ctrl_data *ctrl,
 		      struct nlmsghdr *n, void *arg)
 {
 	FILE *fp = arg;
 
-	if (timestamp)
-		print_timestamp(fp);
-
 	switch (n->nlmsg_type) {
 	case RTM_NEWLINK:
 	case RTM_DELLINK:
-		if (prefix_banner)
-			fprintf(fp, "[LINK]");
-
 		return print_linkinfo(n, arg);
 
 	case RTM_NEWNEIGH:
 	case RTM_DELNEIGH:
-		if (prefix_banner)
-			fprintf(fp, "[NEIGH]");
 		return print_fdb(n, arg);
 
 	case RTM_NEWMDB:
 	case RTM_DELMDB:
-		if (prefix_banner)
-			fprintf(fp, "[MDB]");
 		return print_mdb_mon(n, arg);
 
 	case NLMSG_TSTAMP:
@@ -79,21 +59,26 @@  static int accept_msg(struct rtnl_ctrl_data *ctrl,
 
 	case RTM_NEWVLAN:
 	case RTM_DELVLAN:
-		if (prefix_banner)
-			fprintf(fp, "[VLAN]");
 		return print_vlan_rtm(n, arg, true, false);
 
 	case RTM_NEWTUNNEL:
 	case RTM_DELTUNNEL:
-		if (prefix_banner)
-			fprintf(fp, "[TUNNEL]");
-		return print_tunnel_rtm(n, arg, true);
+		return print_vnifilter_rtm(n, arg, true);
 
 	default:
 		return 0;
 	}
 }
 
+void print_headers(FILE *fp, const char *label)
+{
+	if (timestamp)
+		print_timestamp(fp);
+
+	if (prefix_banner)
+		fprintf(fp, "%s", label);
+}
+
 int do_monitor(int argc, char **argv)
 {
 	char *file = NULL;
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 390a2037..13df1e84 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -1032,6 +1032,7 @@  int print_vlan_rtm(struct nlmsghdr *n, void *arg, bool monitor, bool global_only
 	struct br_vlan_msg *bvm = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *a;
+	FILE *fp = arg;
 	int rem;
 
 	if (n->nlmsg_type != RTM_NEWVLAN && n->nlmsg_type != RTM_DELVLAN &&
@@ -1053,6 +1054,8 @@  int print_vlan_rtm(struct nlmsghdr *n, void *arg, bool monitor, bool global_only
 	if (filter_index && filter_index != bvm->ifindex)
 		return 0;
 
+	print_headers(fp, "[VLAN]");
+
 	if (n->nlmsg_type == RTM_DELVLAN)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
@@ -1333,6 +1336,7 @@  static int vlan_global(int argc, char **argv)
 int do_vlan(int argc, char **argv)
 {
 	ll_init_map(&rth);
+	timestamp = 0;
 
 	if (argc > 0) {
 		if (matches(*argv, "add") == 0)
diff --git a/bridge/vni.c b/bridge/vni.c
index a0c2792c..e776797a 100644
--- a/bridge/vni.c
+++ b/bridge/vni.c
@@ -309,6 +309,7 @@  int print_vnifilter_rtm(struct nlmsghdr *n, void *arg, bool monitor)
 	int len = n->nlmsg_len;
 	bool first = true;
 	struct rtattr *t;
+	FILE *fp = arg;
 	int rem;
 
 	if (n->nlmsg_type != RTM_NEWTUNNEL &&
@@ -331,6 +332,8 @@  int print_vnifilter_rtm(struct nlmsghdr *n, void *arg, bool monitor)
 	if (filter_index && filter_index != tmsg->ifindex)
 		return 0;
 
+	print_headers(fp, "[TUNNEL]");
+
 	if (n->nlmsg_type == RTM_DELTUNNEL)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
@@ -418,6 +421,7 @@  static int vni_show(int argc, char **argv)
 int do_vni(int argc, char **argv)
 {
 	ll_init_map(&rth);
+	timestamp = 0;
 
 	if (argc > 0) {
 		if (strcmp(*argv, "add") == 0)