* [PATCH iproute2 0/4] monitor command fixes
@ 2022-09-22 6:19 Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode Benjamin Poirier
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Benjamin Poirier @ 2022-09-22 6:19 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel
Fix problems in `bridge monitor` and `ip monitor` regarding the filtering
of object types and address families.
Benjamin Poirier (4):
bridge: Do not print stray prefixes in monitor mode
ip-monitor: Do not listen for nexthops by default when specifying
stats
ip-monitor: Include stats events in default and "all" cases
ip-monitor: Fix the selection of rtnl groups when listening for all
object types
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 ++
ip/ipmonitor.c | 115 +++++++++++++++++----------------------------
8 files changed, 71 insertions(+), 98 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode
2022-09-22 6:19 [PATCH iproute2 0/4] monitor command fixes Benjamin Poirier
@ 2022-09-22 6:19 ` Benjamin Poirier
2022-09-22 15:28 ` Stephen Hemminger
2022-09-22 6:19 ` [PATCH iproute2 2/4] ip-monitor: Do not listen for nexthops by default when specifying stats Benjamin Poirier
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Benjamin Poirier @ 2022-09-22 6:19 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel
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(-)
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)
--
2.37.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH iproute2 2/4] ip-monitor: Do not listen for nexthops by default when specifying stats
2022-09-22 6:19 [PATCH iproute2 0/4] monitor command fixes Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode Benjamin Poirier
@ 2022-09-22 6:19 ` Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 3/4] ip-monitor: Include stats events in default and "all" cases Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 4/4] ip-monitor: Fix the selection of rtnl groups when listening for all object types Benjamin Poirier
3 siblings, 0 replies; 11+ messages in thread
From: Benjamin Poirier @ 2022-09-22 6:19 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel
`ip monitor stats` listens for changes to nexthops and stats. It should
listen for stats only.
Fixes: a05a27c07cbf ("ipmonitor: Add monitoring support for stats events")
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
ip/ipmonitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index a4326d2a..380ab526 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -262,6 +262,7 @@ int do_ipmonitor(int argc, char **argv)
} else if (strcmp(*argv, "stats") == 0) {
lstats = 1;
groups = 0;
+ nh_set = 0;
} else if (strcmp(*argv, "all") == 0) {
prefix_banner = 1;
} else if (matches(*argv, "all-nsid") == 0) {
--
2.37.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH iproute2 3/4] ip-monitor: Include stats events in default and "all" cases
2022-09-22 6:19 [PATCH iproute2 0/4] monitor command fixes Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 2/4] ip-monitor: Do not listen for nexthops by default when specifying stats Benjamin Poirier
@ 2022-09-22 6:19 ` Benjamin Poirier
2022-10-25 15:51 ` Stephen Hemminger
2022-09-22 6:19 ` [PATCH iproute2 4/4] ip-monitor: Fix the selection of rtnl groups when listening for all object types Benjamin Poirier
3 siblings, 1 reply; 11+ messages in thread
From: Benjamin Poirier @ 2022-09-22 6:19 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel
It seems that stats were omitted from `ip monitor` and `ip monitor all`.
Since all other event types are included, include stats as well. Use the
same logic as for nexthops.
Fixes: a05a27c07cbf ("ipmonitor: Add monitoring support for stats events")
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
ip/ipmonitor.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 380ab526..cb2195d1 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -180,6 +180,7 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
int do_ipmonitor(int argc, char **argv)
{
+ int lstats = 0, stats_set = 1;
int lnexthop = 0, nh_set = 1;
char *file = NULL;
unsigned int groups = 0;
@@ -190,7 +191,6 @@ int do_ipmonitor(int argc, char **argv)
int lprefix = 0;
int lneigh = 0;
int lnetconf = 0;
- int lstats = 0;
int lrule = 0;
int lnsid = 0;
int ifindex = 0;
@@ -224,41 +224,51 @@ int do_ipmonitor(int argc, char **argv)
llink = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "address") == 0) {
laddr = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "route") == 0) {
lroute = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "mroute") == 0) {
lmroute = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "prefix") == 0) {
lprefix = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "neigh") == 0) {
lneigh = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "netconf") == 0) {
lnetconf = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "rule") == 0) {
lrule = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "nsid") == 0) {
lnsid = 1;
groups = 0;
nh_set = 0;
+ stats_set = 0;
} else if (matches(*argv, "nexthop") == 0) {
lnexthop = 1;
groups = 0;
+ stats_set = 0;
} else if (strcmp(*argv, "stats") == 0) {
lstats = 1;
groups = 0;
@@ -336,6 +346,8 @@ int do_ipmonitor(int argc, char **argv)
}
if (nh_set)
lnexthop = 1;
+ if (stats_set)
+ lstats = 1;
if (file) {
FILE *fp;
--
2.37.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH iproute2 4/4] ip-monitor: Fix the selection of rtnl groups when listening for all object types
2022-09-22 6:19 [PATCH iproute2 0/4] monitor command fixes Benjamin Poirier
` (2 preceding siblings ...)
2022-09-22 6:19 ` [PATCH iproute2 3/4] ip-monitor: Include stats events in default and "all" cases Benjamin Poirier
@ 2022-09-22 6:19 ` Benjamin Poirier
3 siblings, 0 replies; 11+ messages in thread
From: Benjamin Poirier @ 2022-09-22 6:19 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger, Ido Schimmel
Currently, when using `ip monitor`, family-specific rtnl multicast groups
(ex. RTNLGRP_IPV4_IFADDR) are used when specifying the '-family' option (or
one of its short forms) and an object type is specified (ex. `ip -4 monitor
addr`) but not when listening for changes to all object types (ex. `ip -4
monitor`). In that case, multicast groups for all families, regardless of
the '-family' option, are used. Depending on the object type, this leads to
ignoring the '-family' selection (MROUTE, ADDR, NETCONF), or printing stray
prefix headers with no event (ROUTE, RULE).
Rewrite the parameter parsing code so that per-family rtnl multicast groups
are selected in all cases.
The issue can be witnessed while running `ip -4 monitor label` at the same
time as the following command:
ip link add dummy0 address 02:00:00:00:00:01 up type dummy
The output includes:
[ROUTE][ROUTE][ADDR]9: dummy0 inet6 fe80::ff:fe00:1/64 scope link
valid_lft forever preferred_lft forever
Notice the stray "[ROUTE]" labels (related to filtered out ipv6 routes) and
the ipv6 ADDR entry. Those do not appear if using `ip -4 monitor label
route address`.
Fixes: aba5acdfdb34 ("(Logical change 1.3)")
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
ip/ipmonitor.c | 128 ++++++++++++++++---------------------------------
1 file changed, 42 insertions(+), 86 deletions(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index cb2195d1..8a72ea42 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -178,40 +178,26 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
return 0;
}
+#define IPMON_LLINK BIT(0)
+#define IPMON_LADDR BIT(1)
+#define IPMON_LROUTE BIT(2)
+#define IPMON_LMROUTE BIT(3)
+#define IPMON_LPREFIX BIT(4)
+#define IPMON_LNEIGH BIT(5)
+#define IPMON_LNETCONF BIT(6)
+#define IPMON_LSTATS BIT(7)
+#define IPMON_LRULE BIT(8)
+#define IPMON_LNSID BIT(9)
+#define IPMON_LNEXTHOP BIT(10)
+
+#define IPMON_L_ALL (~0)
+
int do_ipmonitor(int argc, char **argv)
{
- int lstats = 0, stats_set = 1;
- int lnexthop = 0, nh_set = 1;
+ unsigned int groups = 0, lmask = 0;
char *file = NULL;
- unsigned int groups = 0;
- int llink = 0;
- int laddr = 0;
- int lroute = 0;
- int lmroute = 0;
- int lprefix = 0;
- int lneigh = 0;
- int lnetconf = 0;
- int lrule = 0;
- int lnsid = 0;
int ifindex = 0;
- groups |= nl_mgrp(RTNLGRP_LINK);
- groups |= nl_mgrp(RTNLGRP_IPV4_IFADDR);
- groups |= nl_mgrp(RTNLGRP_IPV6_IFADDR);
- groups |= nl_mgrp(RTNLGRP_IPV4_ROUTE);
- groups |= nl_mgrp(RTNLGRP_IPV6_ROUTE);
- groups |= nl_mgrp(RTNLGRP_MPLS_ROUTE);
- groups |= nl_mgrp(RTNLGRP_IPV4_MROUTE);
- groups |= nl_mgrp(RTNLGRP_IPV6_MROUTE);
- groups |= nl_mgrp(RTNLGRP_IPV6_PREFIX);
- groups |= nl_mgrp(RTNLGRP_NEIGH);
- groups |= nl_mgrp(RTNLGRP_IPV4_NETCONF);
- groups |= nl_mgrp(RTNLGRP_IPV6_NETCONF);
- groups |= nl_mgrp(RTNLGRP_IPV4_RULE);
- groups |= nl_mgrp(RTNLGRP_IPV6_RULE);
- groups |= nl_mgrp(RTNLGRP_NSID);
- groups |= nl_mgrp(RTNLGRP_MPLS_NETCONF);
-
rtnl_close(&rth);
while (argc > 0) {
@@ -221,58 +207,27 @@ int do_ipmonitor(int argc, char **argv)
} else if (matches(*argv, "label") == 0) {
prefix_banner = 1;
} else if (matches(*argv, "link") == 0) {
- llink = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LLINK;
} else if (matches(*argv, "address") == 0) {
- laddr = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LADDR;
} else if (matches(*argv, "route") == 0) {
- lroute = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LROUTE;
} else if (matches(*argv, "mroute") == 0) {
- lmroute = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LMROUTE;
} else if (matches(*argv, "prefix") == 0) {
- lprefix = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LPREFIX;
} else if (matches(*argv, "neigh") == 0) {
- lneigh = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LNEIGH;
} else if (matches(*argv, "netconf") == 0) {
- lnetconf = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LNETCONF;
} else if (matches(*argv, "rule") == 0) {
- lrule = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LRULE;
} else if (matches(*argv, "nsid") == 0) {
- lnsid = 1;
- groups = 0;
- nh_set = 0;
- stats_set = 0;
+ lmask |= IPMON_LNSID;
} else if (matches(*argv, "nexthop") == 0) {
- lnexthop = 1;
- groups = 0;
- stats_set = 0;
+ lmask |= IPMON_LNEXTHOP;
} else if (strcmp(*argv, "stats") == 0) {
- lstats = 1;
- groups = 0;
- nh_set = 0;
+ lmask |= IPMON_LSTATS;
} else if (strcmp(*argv, "all") == 0) {
prefix_banner = 1;
} else if (matches(*argv, "all-nsid") == 0) {
@@ -298,15 +253,18 @@ int do_ipmonitor(int argc, char **argv)
ipneigh_reset_filter(ifindex);
ipnetconf_reset_filter(ifindex);
- if (llink)
+ if (!lmask)
+ lmask = IPMON_L_ALL;
+
+ if (lmask & IPMON_LLINK)
groups |= nl_mgrp(RTNLGRP_LINK);
- if (laddr) {
+ if (lmask & IPMON_LADDR) {
if (!preferred_family || preferred_family == AF_INET)
groups |= nl_mgrp(RTNLGRP_IPV4_IFADDR);
if (!preferred_family || preferred_family == AF_INET6)
groups |= nl_mgrp(RTNLGRP_IPV6_IFADDR);
}
- if (lroute) {
+ if (lmask & IPMON_LROUTE) {
if (!preferred_family || preferred_family == AF_INET)
groups |= nl_mgrp(RTNLGRP_IPV4_ROUTE);
if (!preferred_family || preferred_family == AF_INET6)
@@ -314,20 +272,20 @@ int do_ipmonitor(int argc, char **argv)
if (!preferred_family || preferred_family == AF_MPLS)
groups |= nl_mgrp(RTNLGRP_MPLS_ROUTE);
}
- if (lmroute) {
+ if (lmask & IPMON_LMROUTE) {
if (!preferred_family || preferred_family == AF_INET)
groups |= nl_mgrp(RTNLGRP_IPV4_MROUTE);
if (!preferred_family || preferred_family == AF_INET6)
groups |= nl_mgrp(RTNLGRP_IPV6_MROUTE);
}
- if (lprefix) {
+ if (lmask & IPMON_LPREFIX) {
if (!preferred_family || preferred_family == AF_INET6)
groups |= nl_mgrp(RTNLGRP_IPV6_PREFIX);
}
- if (lneigh) {
+ if (lmask & IPMON_LNEIGH) {
groups |= nl_mgrp(RTNLGRP_NEIGH);
}
- if (lnetconf) {
+ if (lmask & IPMON_LNETCONF) {
if (!preferred_family || preferred_family == AF_INET)
groups |= nl_mgrp(RTNLGRP_IPV4_NETCONF);
if (!preferred_family || preferred_family == AF_INET6)
@@ -335,19 +293,15 @@ int do_ipmonitor(int argc, char **argv)
if (!preferred_family || preferred_family == AF_MPLS)
groups |= nl_mgrp(RTNLGRP_MPLS_NETCONF);
}
- if (lrule) {
+ if (lmask & IPMON_LRULE) {
if (!preferred_family || preferred_family == AF_INET)
groups |= nl_mgrp(RTNLGRP_IPV4_RULE);
if (!preferred_family || preferred_family == AF_INET6)
groups |= nl_mgrp(RTNLGRP_IPV6_RULE);
}
- if (lnsid) {
+ if (lmask & IPMON_LNSID) {
groups |= nl_mgrp(RTNLGRP_NSID);
}
- if (nh_set)
- lnexthop = 1;
- if (stats_set)
- lstats = 1;
if (file) {
FILE *fp;
@@ -366,12 +320,14 @@ int do_ipmonitor(int argc, char **argv)
if (rtnl_open(&rth, groups) < 0)
exit(1);
- if (lnexthop && rtnl_add_nl_group(&rth, RTNLGRP_NEXTHOP) < 0) {
+ if (lmask & IPMON_LNEXTHOP &&
+ rtnl_add_nl_group(&rth, RTNLGRP_NEXTHOP) < 0) {
fprintf(stderr, "Failed to add nexthop group to list\n");
exit(1);
}
- if (lstats && rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
+ if (lmask & IPMON_LSTATS &&
+ rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
fprintf(stderr, "Failed to add stats group to list\n");
exit(1);
}
--
2.37.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode
2022-09-22 6:19 ` [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode Benjamin Poirier
@ 2022-09-22 15:28 ` Stephen Hemminger
2022-10-25 22:29 ` [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available Benjamin Poirier
0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2022-09-22 15:28 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, David Ahern, Ido Schimmel
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH iproute2 3/4] ip-monitor: Include stats events in default and "all" cases
2022-09-22 6:19 ` [PATCH iproute2 3/4] ip-monitor: Include stats events in default and "all" cases Benjamin Poirier
@ 2022-10-25 15:51 ` Stephen Hemminger
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2022-10-25 15:51 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, David Ahern, Ido Schimmel
On Thu, 22 Sep 2022 15:19:37 +0900
Benjamin Poirier <bpoirier@nvidia.com> wrote:
> It seems that stats were omitted from `ip monitor` and `ip monitor all`.
> Since all other event types are included, include stats as well. Use the
> same logic as for nexthops.
>
> Fixes: a05a27c07cbf ("ipmonitor: Add monitoring support for stats events")
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
This change has caused a regression. Simple ip monitor command now fails.
$ ip monitor
Failed to add stats group to list
The failure is from setsockopt() returning -EINVAL
Using git bisect this patch is identified as the cause.
Please fix ASAP or will revert from next release and maybe even make
a point release to remove it.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available
2022-09-22 15:28 ` Stephen Hemminger
@ 2022-10-25 22:29 ` Benjamin Poirier
2022-10-26 2:17 ` Stephen Hemminger
2022-10-26 3:20 ` Stephen Hemminger
0 siblings, 2 replies; 11+ messages in thread
From: Benjamin Poirier @ 2022-10-25 22:29 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Ahern, Ido Schimmel
Following commit 4e8a9914c4d4 ("ip-monitor: Include stats events in default
and "all" cases"), `ip monitor` fails to start on kernels which do not
contain linux.git commit 5fd0b838efac ("net: rtnetlink: Add UAPI toggle for
IFLA_OFFLOAD_XSTATS_L3_STATS") because the netlink group RTNLGRP_STATS
doesn't exist:
$ ip monitor
Failed to add stats group to list
When "stats" is not explicitly requested, change the error to a warning so
that `ip monitor` and `ip monitor all` continue to work on older kernels.
Note that the same change is not done for RTNLGRP_NEXTHOP because its value
is 32 and group numbers <= 32 are always supported; see the comment above
netlink_change_ngroups() in the kernel source. Therefore
NETLINK_ADD_MEMBERSHIP 32 does not error out even on kernels which do not
support RTNLGRP_NEXTHOP.
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Fixes: 4e8a9914c4d4 ("ip-monitor: Include stats events in default and "all" cases")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
ip/ipmonitor.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 8a72ea42..45e4e8f1 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -195,6 +195,8 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
int do_ipmonitor(int argc, char **argv)
{
unsigned int groups = 0, lmask = 0;
+ /* "needed" mask */
+ unsigned int nmask;
char *file = NULL;
int ifindex = 0;
@@ -253,6 +255,7 @@ int do_ipmonitor(int argc, char **argv)
ipneigh_reset_filter(ifindex);
ipnetconf_reset_filter(ifindex);
+ nmask = lmask;
if (!lmask)
lmask = IPMON_L_ALL;
@@ -328,8 +331,11 @@ int do_ipmonitor(int argc, char **argv)
if (lmask & IPMON_LSTATS &&
rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
+ if (!(nmask & IPMON_LSTATS))
+ fprintf(stderr, "Warning: ");
fprintf(stderr, "Failed to add stats group to list\n");
- exit(1);
+ if (nmask & IPMON_LSTATS)
+ exit(1);
}
if (listen_all_nsid && rtnl_listen_all_nsid(&rth) < 0)
--
2.37.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available
2022-10-25 22:29 ` [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available Benjamin Poirier
@ 2022-10-26 2:17 ` Stephen Hemminger
2022-10-26 3:20 ` Stephen Hemminger
1 sibling, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2022-10-26 2:17 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, David Ahern, Ido Schimmel
On Wed, 26 Oct 2022 07:29:09 +0900
Benjamin Poirier <bpoirier@nvidia.com> wrote:
> Following commit 4e8a9914c4d4 ("ip-monitor: Include stats events in default
> and "all" cases"), `ip monitor` fails to start on kernels which do not
> contain linux.git commit 5fd0b838efac ("net: rtnetlink: Add UAPI toggle for
> IFLA_OFFLOAD_XSTATS_L3_STATS") because the netlink group RTNLGRP_STATS
> doesn't exist:
>
> $ ip monitor
> Failed to add stats group to list
>
> When "stats" is not explicitly requested, change the error to a warning so
> that `ip monitor` and `ip monitor all` continue to work on older kernels.
>
> Note that the same change is not done for RTNLGRP_NEXTHOP because its value
> is 32 and group numbers <= 32 are always supported; see the comment above
> netlink_change_ngroups() in the kernel source. Therefore
> NETLINK_ADD_MEMBERSHIP 32 does not error out even on kernels which do not
> support RTNLGRP_NEXTHOP.
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Fixes: 4e8a9914c4d4 ("ip-monitor: Include stats events in default and "all" cases")
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> ---
> ip/ipmonitor.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
> index 8a72ea42..45e4e8f1 100644
> --- a/ip/ipmonitor.c
> +++ b/ip/ipmonitor.c
> @@ -195,6 +195,8 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
> int do_ipmonitor(int argc, char **argv)
> {
> unsigned int groups = 0, lmask = 0;
> + /* "needed" mask */
> + unsigned int nmask;
> char *file = NULL;
> int ifindex = 0;
>
> @@ -253,6 +255,7 @@ int do_ipmonitor(int argc, char **argv)
> ipneigh_reset_filter(ifindex);
> ipnetconf_reset_filter(ifindex);
>
> + nmask = lmask;
> if (!lmask)
> lmask = IPMON_L_ALL;
>
> @@ -328,8 +331,11 @@ int do_ipmonitor(int argc, char **argv)
>
> if (lmask & IPMON_LSTATS &&
> rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
> + if (!(nmask & IPMON_LSTATS))
> + fprintf(stderr, "Warning: ");
> fprintf(stderr, "Failed to add stats group to list\n");
> - exit(1);
> + if (nmask & IPMON_LSTATS)
> + exit(1);
> }
>
> if (listen_all_nsid && rtnl_listen_all_nsid(&rth) < 0)
You still end up warning on older kernels. My version is simpler.
All needs to not include lstats
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available
2022-10-25 22:29 ` [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available Benjamin Poirier
2022-10-26 2:17 ` Stephen Hemminger
@ 2022-10-26 3:20 ` Stephen Hemminger
2022-10-26 6:49 ` [PATCH iproute2 v2] " Benjamin Poirier
1 sibling, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2022-10-26 3:20 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, David Ahern, Ido Schimmel
On Wed, 26 Oct 2022 07:29:09 +0900
Benjamin Poirier <bpoirier@nvidia.com> wrote:
> Following commit 4e8a9914c4d4 ("ip-monitor: Include stats events in default
> and "all" cases"), `ip monitor` fails to start on kernels which do not
> contain linux.git commit 5fd0b838efac ("net: rtnetlink: Add UAPI toggle for
> IFLA_OFFLOAD_XSTATS_L3_STATS") because the netlink group RTNLGRP_STATS
> doesn't exist:
>
> $ ip monitor
> Failed to add stats group to list
>
> When "stats" is not explicitly requested, change the error to a warning so
> that `ip monitor` and `ip monitor all` continue to work on older kernels.
>
> Note that the same change is not done for RTNLGRP_NEXTHOP because its value
> is 32 and group numbers <= 32 are always supported; see the comment above
> netlink_change_ngroups() in the kernel source. Therefore
> NETLINK_ADD_MEMBERSHIP 32 does not error out even on kernels which do not
> support RTNLGRP_NEXTHOP.
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Fixes: 4e8a9914c4d4 ("ip-monitor: Include stats events in default and "all" cases")
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
There are two acceptable solutions:
1. Ignore the error, and never print any warning.
2. Don't ask for the stats feature with the default "ip monitor" and "ip monitor all"
Either way, it needs to be totally silent when built and run on older kernels.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH iproute2 v2] ip-monitor: Do not error out when RTNLGRP_STATS is not available
2022-10-26 3:20 ` Stephen Hemminger
@ 2022-10-26 6:49 ` Benjamin Poirier
0 siblings, 0 replies; 11+ messages in thread
From: Benjamin Poirier @ 2022-10-26 6:49 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David Ahern, Ido Schimmel
Following commit 4e8a9914c4d4 ("ip-monitor: Include stats events in default
and "all" cases"), `ip monitor` fails to start on kernels which do not
contain linux.git commit 5fd0b838efac ("net: rtnetlink: Add UAPI toggle for
IFLA_OFFLOAD_XSTATS_L3_STATS") because the netlink group RTNLGRP_STATS
doesn't exist:
$ ip monitor
Failed to add stats group to list
When "stats" is not explicitly requested, ignore the error so that `ip
monitor` and `ip monitor all` continue to work on older kernels.
Note that the same change is not done for RTNLGRP_NEXTHOP because its value
is 32 and group numbers <= 32 are always supported; see the comment above
netlink_change_ngroups() in the kernel source. Therefore
NETLINK_ADD_MEMBERSHIP 32 does not error out even on kernels which do not
support RTNLGRP_NEXTHOP.
v2:
* Silently ignore a failure to implicitly add the stats group, instead of
printing a warning.
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Fixes: 4e8a9914c4d4 ("ip-monitor: Include stats events in default and "all" cases")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
> There are two acceptable solutions:
> 1. Ignore the error, and never print any warning.
> 2. Don't ask for the stats feature with the default "ip monitor" and "ip monitor all"
>
> Either way, it needs to be totally silent when built and run on older kernels.
Strictly speaking, the patch below is solution 1*:
Ignore the error, and never print any warning ... when implicitly
adding the stats group.
Before 4e8a9914c4d4, `ip mon stats` used to error out if the stats group
could not be added. That behavior is preserved.
ip/ipmonitor.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 8a72ea42..d808369c 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -195,6 +195,8 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
int do_ipmonitor(int argc, char **argv)
{
unsigned int groups = 0, lmask = 0;
+ /* "needed" mask, failure to enable is an error */
+ unsigned int nmask;
char *file = NULL;
int ifindex = 0;
@@ -253,6 +255,7 @@ int do_ipmonitor(int argc, char **argv)
ipneigh_reset_filter(ifindex);
ipnetconf_reset_filter(ifindex);
+ nmask = lmask;
if (!lmask)
lmask = IPMON_L_ALL;
@@ -327,7 +330,8 @@ int do_ipmonitor(int argc, char **argv)
}
if (lmask & IPMON_LSTATS &&
- rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
+ rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0 &&
+ nmask & IPMON_LSTATS) {
fprintf(stderr, "Failed to add stats group to list\n");
exit(1);
}
--
2.37.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-10-26 6:50 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 6:19 [PATCH iproute2 0/4] monitor command fixes Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 1/4] bridge: Do not print stray prefixes in monitor mode Benjamin Poirier
2022-09-22 15:28 ` Stephen Hemminger
2022-10-25 22:29 ` [PATCH iproute2] ip-monitor: Do not error out when RTNLGRP_STATS is not available Benjamin Poirier
2022-10-26 2:17 ` Stephen Hemminger
2022-10-26 3:20 ` Stephen Hemminger
2022-10-26 6:49 ` [PATCH iproute2 v2] " Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 2/4] ip-monitor: Do not listen for nexthops by default when specifying stats Benjamin Poirier
2022-09-22 6:19 ` [PATCH iproute2 3/4] ip-monitor: Include stats events in default and "all" cases Benjamin Poirier
2022-10-25 15:51 ` Stephen Hemminger
2022-09-22 6:19 ` [PATCH iproute2 4/4] ip-monitor: Fix the selection of rtnl groups when listening for all object types Benjamin Poirier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.