* [PATCH net-next] net: bridge: add STP xstats
@ 2019-12-09 21:18 ` Vivien Didelot
0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 21:18 UTC (permalink / raw)
To: David S. Miller
Cc: Roopa Prabhu, Nikolay Aleksandrov, netdev, bridge, Vivien Didelot
This adds rx_bpdu, tx_bpdu, rx_tcn, tx_tcn, transition_blk,
transition_fwd xstats counters to the bridge ports copied over via
netlink, providing useful information for STP.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
include/uapi/linux/if_bridge.h | 10 ++++++++++
net/bridge/br_if.c | 8 ++++++++
net/bridge/br_netlink.c | 9 +++++++++
net/bridge/br_private.h | 9 +++++++++
net/bridge/br_stp.c | 25 +++++++++++++++++++++++++
net/bridge/br_stp_bpdu.c | 12 ++++++++++++
net/bridge/br_stp_if.c | 27 +++++++++++++++++++++++++++
7 files changed, 100 insertions(+)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 1b3c2b643a02..e7f2bb782006 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -156,6 +156,15 @@ struct bridge_vlan_xstats {
__u32 pad2;
};
+struct bridge_stp_xstats {
+ __u64 transition_blk;
+ __u64 transition_fwd;
+ __u64 rx_bpdu;
+ __u64 tx_bpdu;
+ __u64 rx_tcn;
+ __u64 tx_tcn;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -261,6 +270,7 @@ enum {
BRIDGE_XSTATS_UNSPEC,
BRIDGE_XSTATS_VLAN,
BRIDGE_XSTATS_MCAST,
+ BRIDGE_XSTATS_STP,
BRIDGE_XSTATS_PAD,
__BRIDGE_XSTATS_MAX
};
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 4fe30b182ee7..3eb214ef9763 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -250,6 +250,7 @@ static void release_nbp(struct kobject *kobj)
{
struct net_bridge_port *p
= container_of(kobj, struct net_bridge_port, kobj);
+ free_percpu(p->stp_stats);
kfree(p);
}
@@ -419,6 +420,12 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
if (p == NULL)
return ERR_PTR(-ENOMEM);
+ p->stp_stats = netdev_alloc_pcpu_stats(struct br_stp_stats);
+ if (!p->stp_stats) {
+ kfree(p);
+ return ERR_PTR(-ENOMEM);
+ }
+
p->br = br;
dev_hold(dev);
p->dev = dev;
@@ -432,6 +439,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
err = br_multicast_add_port(p);
if (err) {
dev_put(dev);
+ free_percpu(p->stp_stats);
kfree(p);
p = ERR_PTR(err);
}
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index a0a54482aabc..03aced1f862b 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1597,6 +1597,15 @@ static int br_fill_linkxstats(struct sk_buff *skb,
}
}
+ if (p) {
+ struct bridge_stp_xstats xstats;
+
+ br_stp_get_xstats(p, &xstats);
+
+ if (nla_put(skb, BRIDGE_XSTATS_STP, sizeof(xstats), &xstats))
+ goto nla_put_failure;
+ }
+
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
if (++vl_idx >= *prividx) {
nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_MCAST,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 36b0367ca1e0..af5f28f0f2ef 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -95,6 +95,11 @@ struct br_vlan_stats {
struct u64_stats_sync syncp;
};
+struct br_stp_stats {
+ struct bridge_stp_xstats xstats;
+ struct u64_stats_sync syncp;
+};
+
struct br_tunnel_info {
__be64 tunnel_id;
struct metadata_dst *tunnel_dst;
@@ -283,6 +288,8 @@ struct net_bridge_port {
#endif
u16 group_fwd_mask;
u16 backup_redirected_cnt;
+
+ struct br_stp_stats __percpu *stp_stats;
};
#define kobj_to_brport(obj) container_of(obj, struct net_bridge_port, kobj)
@@ -1146,6 +1153,8 @@ void br_stp_change_bridge_id(struct net_bridge *br, const unsigned char *a);
void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio);
int br_stp_set_port_priority(struct net_bridge_port *p, unsigned long newprio);
int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost);
+void br_stp_get_xstats(const struct net_bridge_port *p,
+ struct bridge_stp_xstats *xstats);
ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id);
/* br_stp_bpdu.c */
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 1f1410f8d312..8bcdab29442d 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -45,6 +45,18 @@ void br_set_state(struct net_bridge_port *p, unsigned int state)
br_info(p->br, "port %u(%s) entered %s state\n",
(unsigned int) p->port_no, p->dev->name,
br_port_state_names[p->state]);
+
+ if (p->br->stp_enabled == BR_KERNEL_STP) {
+ struct br_stp_stats *stats;
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ if (p->state == BR_STATE_BLOCKING)
+ stats->xstats.transition_blk++;
+ else if (p->state == BR_STATE_FORWARDING)
+ stats->xstats.transition_fwd++;
+ u64_stats_update_end(&stats->syncp);
+ }
}
/* called under bridge lock */
@@ -481,9 +493,15 @@ static void br_topology_change_acknowledge(struct net_bridge_port *p)
void br_received_config_bpdu(struct net_bridge_port *p,
const struct br_config_bpdu *bpdu)
{
+ struct br_stp_stats *stats;
struct net_bridge *br;
int was_root;
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.rx_bpdu++;
+ u64_stats_update_end(&stats->syncp);
+
br = p->br;
was_root = br_is_root_bridge(br);
@@ -517,6 +535,13 @@ void br_received_config_bpdu(struct net_bridge_port *p,
/* called under bridge lock */
void br_received_tcn_bpdu(struct net_bridge_port *p)
{
+ struct br_stp_stats *stats;
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.rx_tcn++;
+ u64_stats_update_end(&stats->syncp);
+
if (br_is_designated_port(p)) {
br_info(p->br, "port %u(%s) received tcn bpdu\n",
(unsigned int) p->port_no, p->dev->name);
diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
index 7796dd9d42d7..2dbd11e21f2a 100644
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -78,6 +78,7 @@ static inline int br_get_ticks(const unsigned char *src)
/* called under bridge lock */
void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
{
+ struct br_stp_stats *stats;
unsigned char buf[35];
if (p->br->stp_enabled != BR_KERNEL_STP)
@@ -118,11 +119,17 @@ void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
br_set_ticks(buf+33, bpdu->forward_delay);
br_send_bpdu(p, buf, 35);
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.tx_bpdu++;
+ u64_stats_update_end(&stats->syncp);
}
/* called under bridge lock */
void br_send_tcn_bpdu(struct net_bridge_port *p)
{
+ struct br_stp_stats *stats;
unsigned char buf[4];
if (p->br->stp_enabled != BR_KERNEL_STP)
@@ -133,6 +140,11 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
buf[2] = 0;
buf[3] = BPDU_TYPE_TCN;
br_send_bpdu(p, buf, 4);
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.tx_tcn++;
+ u64_stats_update_end(&stats->syncp);
}
/*
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index d174d3a566aa..cbce7d0e40b9 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -333,6 +333,33 @@ int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost)
return 0;
}
+void br_stp_get_xstats(const struct net_bridge_port *p,
+ struct bridge_stp_xstats *xstats)
+{
+ int i;
+
+ memset(xstats, 0, sizeof(*xstats));
+
+ for_each_possible_cpu(i) {
+ struct bridge_stp_xstats cpu_xstats;
+ struct br_stp_stats *stats;
+ unsigned int start;
+
+ stats = per_cpu_ptr(p->stp_stats, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&stats->syncp);
+ memcpy(&cpu_xstats, &stats->xstats, sizeof(cpu_xstats));
+ } while (u64_stats_fetch_retry_irq(&stats->syncp, start));
+
+ xstats->transition_blk += cpu_xstats.transition_blk;
+ xstats->transition_fwd += cpu_xstats.transition_fwd;
+ xstats->rx_bpdu += cpu_xstats.rx_bpdu;
+ xstats->tx_bpdu += cpu_xstats.tx_bpdu;
+ xstats->rx_tcn += cpu_xstats.rx_tcn;
+ xstats->tx_tcn += cpu_xstats.tx_tcn;
+ }
+}
+
ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id)
{
return sprintf(buf, "%.2x%.2x.%.2x%.2x%.2x%.2x%.2x%.2x\n",
--
2.24.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Bridge] [PATCH net-next] net: bridge: add STP xstats
@ 2019-12-09 21:18 ` Vivien Didelot
0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 21:18 UTC (permalink / raw)
To: David S. Miller
Cc: Nikolay Aleksandrov, netdev, Roopa Prabhu, bridge, Vivien Didelot
This adds rx_bpdu, tx_bpdu, rx_tcn, tx_tcn, transition_blk,
transition_fwd xstats counters to the bridge ports copied over via
netlink, providing useful information for STP.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
include/uapi/linux/if_bridge.h | 10 ++++++++++
net/bridge/br_if.c | 8 ++++++++
net/bridge/br_netlink.c | 9 +++++++++
net/bridge/br_private.h | 9 +++++++++
net/bridge/br_stp.c | 25 +++++++++++++++++++++++++
net/bridge/br_stp_bpdu.c | 12 ++++++++++++
net/bridge/br_stp_if.c | 27 +++++++++++++++++++++++++++
7 files changed, 100 insertions(+)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 1b3c2b643a02..e7f2bb782006 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -156,6 +156,15 @@ struct bridge_vlan_xstats {
__u32 pad2;
};
+struct bridge_stp_xstats {
+ __u64 transition_blk;
+ __u64 transition_fwd;
+ __u64 rx_bpdu;
+ __u64 tx_bpdu;
+ __u64 rx_tcn;
+ __u64 tx_tcn;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -261,6 +270,7 @@ enum {
BRIDGE_XSTATS_UNSPEC,
BRIDGE_XSTATS_VLAN,
BRIDGE_XSTATS_MCAST,
+ BRIDGE_XSTATS_STP,
BRIDGE_XSTATS_PAD,
__BRIDGE_XSTATS_MAX
};
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 4fe30b182ee7..3eb214ef9763 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -250,6 +250,7 @@ static void release_nbp(struct kobject *kobj)
{
struct net_bridge_port *p
= container_of(kobj, struct net_bridge_port, kobj);
+ free_percpu(p->stp_stats);
kfree(p);
}
@@ -419,6 +420,12 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
if (p == NULL)
return ERR_PTR(-ENOMEM);
+ p->stp_stats = netdev_alloc_pcpu_stats(struct br_stp_stats);
+ if (!p->stp_stats) {
+ kfree(p);
+ return ERR_PTR(-ENOMEM);
+ }
+
p->br = br;
dev_hold(dev);
p->dev = dev;
@@ -432,6 +439,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
err = br_multicast_add_port(p);
if (err) {
dev_put(dev);
+ free_percpu(p->stp_stats);
kfree(p);
p = ERR_PTR(err);
}
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index a0a54482aabc..03aced1f862b 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1597,6 +1597,15 @@ static int br_fill_linkxstats(struct sk_buff *skb,
}
}
+ if (p) {
+ struct bridge_stp_xstats xstats;
+
+ br_stp_get_xstats(p, &xstats);
+
+ if (nla_put(skb, BRIDGE_XSTATS_STP, sizeof(xstats), &xstats))
+ goto nla_put_failure;
+ }
+
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
if (++vl_idx >= *prividx) {
nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_MCAST,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 36b0367ca1e0..af5f28f0f2ef 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -95,6 +95,11 @@ struct br_vlan_stats {
struct u64_stats_sync syncp;
};
+struct br_stp_stats {
+ struct bridge_stp_xstats xstats;
+ struct u64_stats_sync syncp;
+};
+
struct br_tunnel_info {
__be64 tunnel_id;
struct metadata_dst *tunnel_dst;
@@ -283,6 +288,8 @@ struct net_bridge_port {
#endif
u16 group_fwd_mask;
u16 backup_redirected_cnt;
+
+ struct br_stp_stats __percpu *stp_stats;
};
#define kobj_to_brport(obj) container_of(obj, struct net_bridge_port, kobj)
@@ -1146,6 +1153,8 @@ void br_stp_change_bridge_id(struct net_bridge *br, const unsigned char *a);
void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio);
int br_stp_set_port_priority(struct net_bridge_port *p, unsigned long newprio);
int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost);
+void br_stp_get_xstats(const struct net_bridge_port *p,
+ struct bridge_stp_xstats *xstats);
ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id);
/* br_stp_bpdu.c */
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 1f1410f8d312..8bcdab29442d 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -45,6 +45,18 @@ void br_set_state(struct net_bridge_port *p, unsigned int state)
br_info(p->br, "port %u(%s) entered %s state\n",
(unsigned int) p->port_no, p->dev->name,
br_port_state_names[p->state]);
+
+ if (p->br->stp_enabled == BR_KERNEL_STP) {
+ struct br_stp_stats *stats;
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ if (p->state == BR_STATE_BLOCKING)
+ stats->xstats.transition_blk++;
+ else if (p->state == BR_STATE_FORWARDING)
+ stats->xstats.transition_fwd++;
+ u64_stats_update_end(&stats->syncp);
+ }
}
/* called under bridge lock */
@@ -481,9 +493,15 @@ static void br_topology_change_acknowledge(struct net_bridge_port *p)
void br_received_config_bpdu(struct net_bridge_port *p,
const struct br_config_bpdu *bpdu)
{
+ struct br_stp_stats *stats;
struct net_bridge *br;
int was_root;
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.rx_bpdu++;
+ u64_stats_update_end(&stats->syncp);
+
br = p->br;
was_root = br_is_root_bridge(br);
@@ -517,6 +535,13 @@ void br_received_config_bpdu(struct net_bridge_port *p,
/* called under bridge lock */
void br_received_tcn_bpdu(struct net_bridge_port *p)
{
+ struct br_stp_stats *stats;
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.rx_tcn++;
+ u64_stats_update_end(&stats->syncp);
+
if (br_is_designated_port(p)) {
br_info(p->br, "port %u(%s) received tcn bpdu\n",
(unsigned int) p->port_no, p->dev->name);
diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
index 7796dd9d42d7..2dbd11e21f2a 100644
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -78,6 +78,7 @@ static inline int br_get_ticks(const unsigned char *src)
/* called under bridge lock */
void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
{
+ struct br_stp_stats *stats;
unsigned char buf[35];
if (p->br->stp_enabled != BR_KERNEL_STP)
@@ -118,11 +119,17 @@ void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
br_set_ticks(buf+33, bpdu->forward_delay);
br_send_bpdu(p, buf, 35);
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.tx_bpdu++;
+ u64_stats_update_end(&stats->syncp);
}
/* called under bridge lock */
void br_send_tcn_bpdu(struct net_bridge_port *p)
{
+ struct br_stp_stats *stats;
unsigned char buf[4];
if (p->br->stp_enabled != BR_KERNEL_STP)
@@ -133,6 +140,11 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
buf[2] = 0;
buf[3] = BPDU_TYPE_TCN;
br_send_bpdu(p, buf, 4);
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.tx_tcn++;
+ u64_stats_update_end(&stats->syncp);
}
/*
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index d174d3a566aa..cbce7d0e40b9 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -333,6 +333,33 @@ int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost)
return 0;
}
+void br_stp_get_xstats(const struct net_bridge_port *p,
+ struct bridge_stp_xstats *xstats)
+{
+ int i;
+
+ memset(xstats, 0, sizeof(*xstats));
+
+ for_each_possible_cpu(i) {
+ struct bridge_stp_xstats cpu_xstats;
+ struct br_stp_stats *stats;
+ unsigned int start;
+
+ stats = per_cpu_ptr(p->stp_stats, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&stats->syncp);
+ memcpy(&cpu_xstats, &stats->xstats, sizeof(cpu_xstats));
+ } while (u64_stats_fetch_retry_irq(&stats->syncp, start));
+
+ xstats->transition_blk += cpu_xstats.transition_blk;
+ xstats->transition_fwd += cpu_xstats.transition_fwd;
+ xstats->rx_bpdu += cpu_xstats.rx_bpdu;
+ xstats->tx_bpdu += cpu_xstats.tx_bpdu;
+ xstats->rx_tcn += cpu_xstats.rx_tcn;
+ xstats->tx_tcn += cpu_xstats.tx_tcn;
+ }
+}
+
ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id)
{
return sprintf(buf, "%.2x%.2x.%.2x%.2x%.2x%.2x%.2x%.2x\n",
--
2.24.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH iproute2] iplink: add support for STP xstats
2019-12-09 21:18 ` [Bridge] " Vivien Didelot
@ 2019-12-09 21:18 ` Vivien Didelot
-1 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 21:18 UTC (permalink / raw)
To: David S. Miller
Cc: Roopa Prabhu, Nikolay Aleksandrov, netdev, bridge, Vivien Didelot
Add support for the BRIDGE_XSTATS_STP xstats, as follow:
# ip link xstats type bridge_slave dev lan5
STP BPDU:
RX: 0
TX: 39
STP TCN:
RX: 0
TX: 0
STP Transitions:
Blocked: 0
Forwarding: 1
IGMP queries:
RX: v1 0 v2 0 v3 0
TX: v1 0 v2 0 v3 0
...
Or below as JSON:
# ip -j -p link xstats type bridge_slave dev lan0 | head
[ {
"ifname": "lan0",
"stp": {
"rx_bpdu": 0,
"tx_bpdu": 500,
"rx_tcn": 0,
"tx_tcn": 0,
"transition_blk": 0,
"transition_fwd": 1
},
...
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
include/uapi/linux/if_bridge.h | 16 +++++++++++++---
ip/iplink_bridge.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 31fc51bd..e7f2bb78 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -11,8 +11,8 @@
* 2 of the License, or (at your option) any later version.
*/
-#ifndef _LINUX_IF_BRIDGE_H
-#define _LINUX_IF_BRIDGE_H
+#ifndef _UAPI_LINUX_IF_BRIDGE_H
+#define _UAPI_LINUX_IF_BRIDGE_H
#include <linux/types.h>
#include <linux/if_ether.h>
@@ -156,6 +156,15 @@ struct bridge_vlan_xstats {
__u32 pad2;
};
+struct bridge_stp_xstats {
+ __u64 transition_blk;
+ __u64 transition_fwd;
+ __u64 rx_bpdu;
+ __u64 tx_bpdu;
+ __u64 rx_tcn;
+ __u64 tx_tcn;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -261,6 +270,7 @@ enum {
BRIDGE_XSTATS_UNSPEC,
BRIDGE_XSTATS_VLAN,
BRIDGE_XSTATS_MCAST,
+ BRIDGE_XSTATS_STP,
BRIDGE_XSTATS_PAD,
__BRIDGE_XSTATS_MAX
};
@@ -314,4 +324,4 @@ struct br_boolopt_multi {
__u32 optval;
__u32 optmask;
};
-#endif /* _LINUX_IF_BRIDGE_H */
+#endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 06f736d4..92e051c8 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -688,6 +688,7 @@ static void bridge_print_xstats_help(struct link_util *lu, FILE *f)
static void bridge_print_stats_attr(struct rtattr *attr, int ifindex)
{
struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
+ struct bridge_stp_xstats *sstats;
struct br_mcast_stats *mstats;
struct rtattr *i, *list;
const char *ifname = "";
@@ -807,6 +808,35 @@ static void bridge_print_stats_attr(struct rtattr *attr, int ifindex)
mstats->mld_parse_errors);
close_json_object();
break;
+ case BRIDGE_XSTATS_STP:
+ sstats = RTA_DATA(i);
+ open_json_object("stp");
+ print_string(PRINT_FP, NULL,
+ "%-16s STP BPDU:\n", "");
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "rx_bpdu", "RX: %llu\n",
+ sstats->rx_bpdu);
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "tx_bpdu", "TX: %llu\n",
+ sstats->tx_bpdu);
+ print_string(PRINT_FP, NULL,
+ "%-16s STP TCN:\n", "");
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "rx_tcn", "RX: %llu\n",
+ sstats->rx_tcn);
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "tx_tcn", "TX: %llu\n",
+ sstats->tx_tcn);
+ print_string(PRINT_FP, NULL,
+ "%-16s STP Transitions:\n", "");
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "transition_blk", "Blocked: %llu\n",
+ sstats->transition_blk);
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "transition_fwd", "Forwarding: %llu\n",
+ sstats->transition_fwd);
+ close_json_object();
+ break;
}
}
close_json_object();
@@ -843,6 +873,8 @@ int bridge_parse_xstats(struct link_util *lu, int argc, char **argv)
while (argc > 0) {
if (strcmp(*argv, "igmp") == 0 || strcmp(*argv, "mcast") == 0) {
xstats_print_attr = BRIDGE_XSTATS_MCAST;
+ } else if (strcmp(*argv, "stp") == 0) {
+ xstats_print_attr = BRIDGE_XSTATS_STP;
} else if (strcmp(*argv, "dev") == 0) {
NEXT_ARG();
filter_index = ll_name_to_index(*argv);
--
2.24.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Bridge] [PATCH iproute2] iplink: add support for STP xstats
@ 2019-12-09 21:18 ` Vivien Didelot
0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 21:18 UTC (permalink / raw)
To: David S. Miller
Cc: Nikolay Aleksandrov, netdev, Roopa Prabhu, bridge, Vivien Didelot
Add support for the BRIDGE_XSTATS_STP xstats, as follow:
# ip link xstats type bridge_slave dev lan5
STP BPDU:
RX: 0
TX: 39
STP TCN:
RX: 0
TX: 0
STP Transitions:
Blocked: 0
Forwarding: 1
IGMP queries:
RX: v1 0 v2 0 v3 0
TX: v1 0 v2 0 v3 0
...
Or below as JSON:
# ip -j -p link xstats type bridge_slave dev lan0 | head
[ {
"ifname": "lan0",
"stp": {
"rx_bpdu": 0,
"tx_bpdu": 500,
"rx_tcn": 0,
"tx_tcn": 0,
"transition_blk": 0,
"transition_fwd": 1
},
...
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
include/uapi/linux/if_bridge.h | 16 +++++++++++++---
ip/iplink_bridge.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 31fc51bd..e7f2bb78 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -11,8 +11,8 @@
* 2 of the License, or (at your option) any later version.
*/
-#ifndef _LINUX_IF_BRIDGE_H
-#define _LINUX_IF_BRIDGE_H
+#ifndef _UAPI_LINUX_IF_BRIDGE_H
+#define _UAPI_LINUX_IF_BRIDGE_H
#include <linux/types.h>
#include <linux/if_ether.h>
@@ -156,6 +156,15 @@ struct bridge_vlan_xstats {
__u32 pad2;
};
+struct bridge_stp_xstats {
+ __u64 transition_blk;
+ __u64 transition_fwd;
+ __u64 rx_bpdu;
+ __u64 tx_bpdu;
+ __u64 rx_tcn;
+ __u64 tx_tcn;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -261,6 +270,7 @@ enum {
BRIDGE_XSTATS_UNSPEC,
BRIDGE_XSTATS_VLAN,
BRIDGE_XSTATS_MCAST,
+ BRIDGE_XSTATS_STP,
BRIDGE_XSTATS_PAD,
__BRIDGE_XSTATS_MAX
};
@@ -314,4 +324,4 @@ struct br_boolopt_multi {
__u32 optval;
__u32 optmask;
};
-#endif /* _LINUX_IF_BRIDGE_H */
+#endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 06f736d4..92e051c8 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -688,6 +688,7 @@ static void bridge_print_xstats_help(struct link_util *lu, FILE *f)
static void bridge_print_stats_attr(struct rtattr *attr, int ifindex)
{
struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
+ struct bridge_stp_xstats *sstats;
struct br_mcast_stats *mstats;
struct rtattr *i, *list;
const char *ifname = "";
@@ -807,6 +808,35 @@ static void bridge_print_stats_attr(struct rtattr *attr, int ifindex)
mstats->mld_parse_errors);
close_json_object();
break;
+ case BRIDGE_XSTATS_STP:
+ sstats = RTA_DATA(i);
+ open_json_object("stp");
+ print_string(PRINT_FP, NULL,
+ "%-16s STP BPDU:\n", "");
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "rx_bpdu", "RX: %llu\n",
+ sstats->rx_bpdu);
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "tx_bpdu", "TX: %llu\n",
+ sstats->tx_bpdu);
+ print_string(PRINT_FP, NULL,
+ "%-16s STP TCN:\n", "");
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "rx_tcn", "RX: %llu\n",
+ sstats->rx_tcn);
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "tx_tcn", "TX: %llu\n",
+ sstats->tx_tcn);
+ print_string(PRINT_FP, NULL,
+ "%-16s STP Transitions:\n", "");
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "transition_blk", "Blocked: %llu\n",
+ sstats->transition_blk);
+ print_string(PRINT_FP, NULL, "%-16s ", "");
+ print_u64(PRINT_ANY, "transition_fwd", "Forwarding: %llu\n",
+ sstats->transition_fwd);
+ close_json_object();
+ break;
}
}
close_json_object();
@@ -843,6 +873,8 @@ int bridge_parse_xstats(struct link_util *lu, int argc, char **argv)
while (argc > 0) {
if (strcmp(*argv, "igmp") == 0 || strcmp(*argv, "mcast") == 0) {
xstats_print_attr = BRIDGE_XSTATS_MCAST;
+ } else if (strcmp(*argv, "stp") == 0) {
+ xstats_print_attr = BRIDGE_XSTATS_STP;
} else if (strcmp(*argv, "dev") == 0) {
NEXT_ARG();
filter_index = ll_name_to_index(*argv);
--
2.24.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH iproute2] iplink: add support for STP xstats
2019-12-09 21:18 ` [Bridge] " Vivien Didelot
@ 2019-12-09 22:34 ` Stephen Hemminger
-1 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-12-09 22:34 UTC (permalink / raw)
To: Vivien Didelot
Cc: David S. Miller, Roopa Prabhu, Nikolay Aleksandrov, netdev, bridge
On Mon, 9 Dec 2019 16:18:41 -0500
Vivien Didelot <vivien.didelot@gmail.com> wrote:
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 31fc51bd..e7f2bb78 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
These headers are semi-automatically updated from the kernel.
Do not make changes here.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH iproute2] iplink: add support for STP xstats
@ 2019-12-09 22:34 ` Stephen Hemminger
0 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2019-12-09 22:34 UTC (permalink / raw)
To: Vivien Didelot
Cc: Nikolay Aleksandrov, netdev, Roopa Prabhu, bridge, David S. Miller
On Mon, 9 Dec 2019 16:18:41 -0500
Vivien Didelot <vivien.didelot@gmail.com> wrote:
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 31fc51bd..e7f2bb78 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
These headers are semi-automatically updated from the kernel.
Do not make changes here.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH iproute2] iplink: add support for STP xstats
2019-12-09 22:34 ` [Bridge] " Stephen Hemminger
@ 2019-12-09 23:15 ` Vivien Didelot
-1 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 23:15 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Roopa Prabhu, Nikolay Aleksandrov, netdev, bridge
On Mon, 9 Dec 2019 14:34:23 -0800, Stephen Hemminger <stephen@networkplumber.org> wrote:
> On Mon, 9 Dec 2019 16:18:41 -0500
> Vivien Didelot <vivien.didelot@gmail.com> wrote:
>
> > diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> > index 31fc51bd..e7f2bb78 100644
> > --- a/include/uapi/linux/if_bridge.h
> > +++ b/include/uapi/linux/if_bridge.h
>
> These headers are semi-automatically updated from the kernel.
> Do not make changes here.
OK, I respin the series with only the minimal changes required for iproute2
to compile correctly.
Thank you,
Vivien
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH iproute2] iplink: add support for STP xstats
@ 2019-12-09 23:15 ` Vivien Didelot
0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 23:15 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Nikolay Aleksandrov, netdev, Roopa Prabhu, bridge, David S. Miller
On Mon, 9 Dec 2019 14:34:23 -0800, Stephen Hemminger <stephen@networkplumber.org> wrote:
> On Mon, 9 Dec 2019 16:18:41 -0500
> Vivien Didelot <vivien.didelot@gmail.com> wrote:
>
> > diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> > index 31fc51bd..e7f2bb78 100644
> > --- a/include/uapi/linux/if_bridge.h
> > +++ b/include/uapi/linux/if_bridge.h
>
> These headers are semi-automatically updated from the kernel.
> Do not make changes here.
OK, I respin the series with only the minimal changes required for iproute2
to compile correctly.
Thank you,
Vivien
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 20:52 ` Nikolay Aleksandrov
@ 2019-12-10 21:08 ` Vivien Didelot
0 siblings, 0 replies; 18+ messages in thread
From: Vivien Didelot @ 2019-12-10 21:08 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
On Tue, 10 Dec 2019 22:52:59 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >>>>>> Why do you need percpu ? All of these seem to be incremented with the
> >>>>>> bridge lock held. A few more comments below.
> >>>>>
> >>>>> All other xstats are incremented percpu, I simply followed the pattern.
> >>>>>
> >>>>
> >>>> We have already a lock, we can use it and avoid the whole per-cpu memory handling.
> >>>> It seems to be acquired in all cases where these counters need to be changed.
> >>>
> >>> Since the other xstats counters are currently implemented this way, I prefer
> >>> to keep the code as is, until we eventually change them all if percpu is in
> >>> fact not needed anymore.
> >>>
> >>> The new series is ready and I can submit it now if there's no objection.
> >>
> >> There is a reason other counters use per-cpu - they're incremented without any locking from fast-path.
> >> The bridge STP code already has a lock which is acquired in all of these paths and we don't need
> >> this overhead and the per-cpu memory allocations. Unless you can find a STP codepath which actually
> >> needs per-cpu, I'd prefer you drop it.
> >
> > Ho ok I understand what you mean now. I'll drop the percpu attribute.
>
> Great, thanks again.
> I think it's clear, but I'll add just in case to avoid extra work - you can drop
> the dynamic memory allocation altogether and make the struct part of net_bridge_port.
Yup, that's what I've done and it makes the patch shamely small now ;)
Thanks,
Vivien
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 20:34 ` Vivien Didelot
@ 2019-12-10 20:52 ` Nikolay Aleksandrov
2019-12-10 21:08 ` Vivien Didelot
0 siblings, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-12-10 20:52 UTC (permalink / raw)
To: Vivien Didelot
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
On 10/12/2019 22:34, Vivien Didelot wrote:
> On Tue, 10 Dec 2019 22:15:26 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>>>>> Why do you need percpu ? All of these seem to be incremented with the
>>>>>> bridge lock held. A few more comments below.
>>>>>
>>>>> All other xstats are incremented percpu, I simply followed the pattern.
>>>>>
>>>>
>>>> We have already a lock, we can use it and avoid the whole per-cpu memory handling.
>>>> It seems to be acquired in all cases where these counters need to be changed.
>>>
>>> Since the other xstats counters are currently implemented this way, I prefer
>>> to keep the code as is, until we eventually change them all if percpu is in
>>> fact not needed anymore.
>>>
>>> The new series is ready and I can submit it now if there's no objection.
>>
>> There is a reason other counters use per-cpu - they're incremented without any locking from fast-path.
>> The bridge STP code already has a lock which is acquired in all of these paths and we don't need
>> this overhead and the per-cpu memory allocations. Unless you can find a STP codepath which actually
>> needs per-cpu, I'd prefer you drop it.
>
> Ho ok I understand what you mean now. I'll drop the percpu attribute.
>
>
> Thanks,
>
> Vivien
>
Great, thanks again.
I think it's clear, but I'll add just in case to avoid extra work - you can drop
the dynamic memory allocation altogether and make the struct part of net_bridge_port.
Cheers,
Nik
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 20:15 ` Nikolay Aleksandrov
@ 2019-12-10 20:34 ` Vivien Didelot
2019-12-10 20:52 ` Nikolay Aleksandrov
0 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2019-12-10 20:34 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
On Tue, 10 Dec 2019 22:15:26 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >>>> Why do you need percpu ? All of these seem to be incremented with the
> >>>> bridge lock held. A few more comments below.
> >>>
> >>> All other xstats are incremented percpu, I simply followed the pattern.
> >>>
> >>
> >> We have already a lock, we can use it and avoid the whole per-cpu memory handling.
> >> It seems to be acquired in all cases where these counters need to be changed.
> >
> > Since the other xstats counters are currently implemented this way, I prefer
> > to keep the code as is, until we eventually change them all if percpu is in
> > fact not needed anymore.
> >
> > The new series is ready and I can submit it now if there's no objection.
>
> There is a reason other counters use per-cpu - they're incremented without any locking from fast-path.
> The bridge STP code already has a lock which is acquired in all of these paths and we don't need
> this overhead and the per-cpu memory allocations. Unless you can find a STP codepath which actually
> needs per-cpu, I'd prefer you drop it.
Ho ok I understand what you mean now. I'll drop the percpu attribute.
Thanks,
Vivien
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 20:10 ` Vivien Didelot
@ 2019-12-10 20:15 ` Nikolay Aleksandrov
2019-12-10 20:34 ` Vivien Didelot
0 siblings, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-12-10 20:15 UTC (permalink / raw)
To: Vivien Didelot
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
On 10/12/2019 22:10, Vivien Didelot wrote:
> Hi Nikolay,
>
> On Tue, 10 Dec 2019 21:50:10 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>>> Why do you need percpu ? All of these seem to be incremented with the
>>>> bridge lock held. A few more comments below.
>>>
>>> All other xstats are incremented percpu, I simply followed the pattern.
>>>
>>
>> We have already a lock, we can use it and avoid the whole per-cpu memory handling.
>> It seems to be acquired in all cases where these counters need to be changed.
>
> Since the other xstats counters are currently implemented this way, I prefer
> to keep the code as is, until we eventually change them all if percpu is in
> fact not needed anymore.
>
> The new series is ready and I can submit it now if there's no objection.
>
>
> Thanks,
>
> Vivien
>
There is a reason other counters use per-cpu - they're incremented without any locking from fast-path.
The bridge STP code already has a lock which is acquired in all of these paths and we don't need
this overhead and the per-cpu memory allocations. Unless you can find a STP codepath which actually
needs per-cpu, I'd prefer you drop it.
Thank you,
Nik
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 19:50 ` Nikolay Aleksandrov
@ 2019-12-10 20:10 ` Vivien Didelot
2019-12-10 20:15 ` Nikolay Aleksandrov
0 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2019-12-10 20:10 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
Hi Nikolay,
On Tue, 10 Dec 2019 21:50:10 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >> Why do you need percpu ? All of these seem to be incremented with the
> >> bridge lock held. A few more comments below.
> >
> > All other xstats are incremented percpu, I simply followed the pattern.
> >
>
> We have already a lock, we can use it and avoid the whole per-cpu memory handling.
> It seems to be acquired in all cases where these counters need to be changed.
Since the other xstats counters are currently implemented this way, I prefer
to keep the code as is, until we eventually change them all if percpu is in
fact not needed anymore.
The new series is ready and I can submit it now if there's no objection.
Thanks,
Vivien
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 19:39 ` Vivien Didelot
@ 2019-12-10 19:50 ` Nikolay Aleksandrov
2019-12-10 20:10 ` Vivien Didelot
0 siblings, 1 reply; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-12-10 19:50 UTC (permalink / raw)
To: Vivien Didelot
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
On 10/12/2019 21:39, Vivien Didelot wrote:
> Hi Nikolay,
>
> On Tue, 10 Dec 2019 09:49:59 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>> Why did you send the bridge patch again ? Does it have any changes ?
>
> The second iproute2 patch does not include the include guards update, but
> I kept the bridge_stp_stats structure and the BRIDGE_XSTATS_STP definition
> otherwise iproute2 wouldn't compile.
> >>
>> Why do you need percpu ? All of these seem to be incremented with the
>> bridge lock held. A few more comments below.
>
> All other xstats are incremented percpu, I simply followed the pattern.
>
We have already a lock, we can use it and avoid the whole per-cpu memory handling.
It seems to be acquired in all cases where these counters need to be changed.
>>> struct net_bridge_port *p
>>> = container_of(kobj, struct net_bridge_port, kobj);
>>> + free_percpu(p->stp_stats);
>>
>> Please leave a new line between local var declaration and the code. I know
>> it was missing, but you can add it now. :)
>
> OK.
>
>>> + if (p) {
>>> + struct bridge_stp_xstats xstats;
>>
>> Please rename the local var here, using just xstats is misleading.
>> Maybe stp_xstats ?
>
> This isn't misleading to me since its scope is limited to the current block
> and not the entire function. The block above dumping the VLAN xstats is
> using a local "struct br_vlan_stats stats" variable for example.
>
Yep, as I answered to myself earlier, with the below change this goes away.
>>
>>> +
>>> + br_stp_get_xstats(p, &xstats);
>>> +
>>> + if (nla_put(skb, BRIDGE_XSTATS_STP, sizeof(xstats), &xstats))
>>> + goto nla_put_failure;
>>
>> Could you please follow how mcast xstats are dumped and do something similar ?
>> It'd be nice to have similar code to audit.
>
> Sure. I would also love to have easily auditable code in net/bridge. For
> the bridge STP xstats I followed the VLAN xstats code above, which does:
>
> if (nla_put(skb, BRIDGE_XSTATS_VLAN, sizeof(vxi), &vxi))
> goto nla_put_failure;
>
Yeah, we can move that to a vlan-specific helper too, but that is an unrelated change.
> But I can change the STP xstats code to the following:
>
> if (p) {
> nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_STP,
> sizeof(struct bridge_stp_xstats),
> BRIDGE_XSTATS_PAD);
> if (!nla)
> goto nla_put_failure;
>
> br_stp_get_xstats(p, nla_data(nla));
> }
>
> Would that be preferred?
>
>
Perfect, thanks!
> Thanks,
>
> Vivien
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 7:49 ` Nikolay Aleksandrov
2019-12-10 7:54 ` Nikolay Aleksandrov
@ 2019-12-10 19:39 ` Vivien Didelot
2019-12-10 19:50 ` Nikolay Aleksandrov
1 sibling, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2019-12-10 19:39 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: David S. Miller, Roopa Prabhu, netdev, bridge, Stephen Hemminger
Hi Nikolay,
On Tue, 10 Dec 2019 09:49:59 +0200, Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> Why did you send the bridge patch again ? Does it have any changes ?
The second iproute2 patch does not include the include guards update, but
I kept the bridge_stp_stats structure and the BRIDGE_XSTATS_STP definition
otherwise iproute2 wouldn't compile.
>
> Why do you need percpu ? All of these seem to be incremented with the
> bridge lock held. A few more comments below.
All other xstats are incremented percpu, I simply followed the pattern.
> > struct net_bridge_port *p
> > = container_of(kobj, struct net_bridge_port, kobj);
> > + free_percpu(p->stp_stats);
>
> Please leave a new line between local var declaration and the code. I know
> it was missing, but you can add it now. :)
OK.
> > + if (p) {
> > + struct bridge_stp_xstats xstats;
>
> Please rename the local var here, using just xstats is misleading.
> Maybe stp_xstats ?
This isn't misleading to me since its scope is limited to the current block
and not the entire function. The block above dumping the VLAN xstats is
using a local "struct br_vlan_stats stats" variable for example.
>
> > +
> > + br_stp_get_xstats(p, &xstats);
> > +
> > + if (nla_put(skb, BRIDGE_XSTATS_STP, sizeof(xstats), &xstats))
> > + goto nla_put_failure;
>
> Could you please follow how mcast xstats are dumped and do something similar ?
> It'd be nice to have similar code to audit.
Sure. I would also love to have easily auditable code in net/bridge. For
the bridge STP xstats I followed the VLAN xstats code above, which does:
if (nla_put(skb, BRIDGE_XSTATS_VLAN, sizeof(vxi), &vxi))
goto nla_put_failure;
But I can change the STP xstats code to the following:
if (p) {
nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_STP,
sizeof(struct bridge_stp_xstats),
BRIDGE_XSTATS_PAD);
if (!nla)
goto nla_put_failure;
br_stp_get_xstats(p, nla_data(nla));
}
Would that be preferred?
Thanks,
Vivien
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-10 7:49 ` Nikolay Aleksandrov
@ 2019-12-10 7:54 ` Nikolay Aleksandrov
2019-12-10 19:39 ` Vivien Didelot
1 sibling, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-12-10 7:54 UTC (permalink / raw)
To: Vivien Didelot, David S. Miller
Cc: Roopa Prabhu, netdev, bridge, Stephen Hemminger
On 10/12/2019 09:49, Nikolay Aleksandrov wrote:
> On 10/12/2019 01:05, Vivien Didelot wrote:
>> This adds rx_bpdu, tx_bpdu, rx_tcn, tx_tcn, transition_blk,
>> transition_fwd xstats counters to the bridge ports copied over via
>> netlink, providing useful information for STP.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
>> ---
>> include/uapi/linux/if_bridge.h | 10 ++++++++++
>> net/bridge/br_if.c | 8 ++++++++
>> net/bridge/br_netlink.c | 9 +++++++++
>> net/bridge/br_private.h | 9 +++++++++
>> net/bridge/br_stp.c | 25 +++++++++++++++++++++++++
>> net/bridge/br_stp_bpdu.c | 12 ++++++++++++
>> net/bridge/br_stp_if.c | 27 +++++++++++++++++++++++++++
>> 7 files changed, 100 insertions(+)
>>
[snip]
>> }
>>
>> @@ -419,6 +420,12 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
>> if (p == NULL)
>> return ERR_PTR(-ENOMEM);
>>
>> + p->stp_stats = netdev_alloc_pcpu_stats(struct br_stp_stats);
>> + if (!p->stp_stats) {
>> + kfree(p);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> p->br = br;
>> dev_hold(dev);
>> p->dev = dev;
>> @@ -432,6 +439,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
>> err = br_multicast_add_port(p);
>> if (err) {
>> dev_put(dev);
>> + free_percpu(p->stp_stats);
>> kfree(p);
>> p = ERR_PTR(err);
>> }
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index a0a54482aabc..03aced1f862b 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -1597,6 +1597,15 @@ static int br_fill_linkxstats(struct sk_buff *skb,
>> }
>> }
>>
>> + if (p) {
>> + struct bridge_stp_xstats xstats;
>
> Please rename the local var here, using just xstats is misleading.
> Maybe stp_xstats ?
>
Actually if this gets re-written to follow the mcast dump the local var
would disappear altogether.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next] net: bridge: add STP xstats
2019-12-09 23:05 [PATCH net-next] net: bridge: add " Vivien Didelot
@ 2019-12-10 7:49 ` Nikolay Aleksandrov
2019-12-10 7:54 ` Nikolay Aleksandrov
2019-12-10 19:39 ` Vivien Didelot
0 siblings, 2 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-12-10 7:49 UTC (permalink / raw)
To: Vivien Didelot, David S. Miller
Cc: Roopa Prabhu, netdev, bridge, Stephen Hemminger
On 10/12/2019 01:05, Vivien Didelot wrote:
> This adds rx_bpdu, tx_bpdu, rx_tcn, tx_tcn, transition_blk,
> transition_fwd xstats counters to the bridge ports copied over via
> netlink, providing useful information for STP.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> ---
> include/uapi/linux/if_bridge.h | 10 ++++++++++
> net/bridge/br_if.c | 8 ++++++++
> net/bridge/br_netlink.c | 9 +++++++++
> net/bridge/br_private.h | 9 +++++++++
> net/bridge/br_stp.c | 25 +++++++++++++++++++++++++
> net/bridge/br_stp_bpdu.c | 12 ++++++++++++
> net/bridge/br_stp_if.c | 27 +++++++++++++++++++++++++++
> 7 files changed, 100 insertions(+)
>
Hi Vivien,
Why did you send the bridge patch again ? Does it have any changes ?
Why do you need percpu ? All of these seem to be incremented with the
bridge lock held. A few more comments below.
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 1b3c2b643a02..e7f2bb782006 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -156,6 +156,15 @@ struct bridge_vlan_xstats {
> __u32 pad2;
> };
>
> +struct bridge_stp_xstats {
> + __u64 transition_blk;
> + __u64 transition_fwd;
> + __u64 rx_bpdu;
> + __u64 tx_bpdu;
> + __u64 rx_tcn;
> + __u64 tx_tcn;
> +};
> +
> /* Bridge multicast database attributes
> * [MDBA_MDB] = {
> * [MDBA_MDB_ENTRY] = {
> @@ -261,6 +270,7 @@ enum {
> BRIDGE_XSTATS_UNSPEC,
> BRIDGE_XSTATS_VLAN,
> BRIDGE_XSTATS_MCAST,
> + BRIDGE_XSTATS_STP,
> BRIDGE_XSTATS_PAD,
> __BRIDGE_XSTATS_MAX
> };
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 4fe30b182ee7..3eb214ef9763 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -250,6 +250,7 @@ static void release_nbp(struct kobject *kobj)
> {
> struct net_bridge_port *p
> = container_of(kobj, struct net_bridge_port, kobj);
> + free_percpu(p->stp_stats);
Please leave a new line between local var declaration and the code. I know
it was missing, but you can add it now. :)
> kfree(p);
> }
>
> @@ -419,6 +420,12 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
> if (p == NULL)
> return ERR_PTR(-ENOMEM);
>
> + p->stp_stats = netdev_alloc_pcpu_stats(struct br_stp_stats);
> + if (!p->stp_stats) {
> + kfree(p);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> p->br = br;
> dev_hold(dev);
> p->dev = dev;
> @@ -432,6 +439,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
> err = br_multicast_add_port(p);
> if (err) {
> dev_put(dev);
> + free_percpu(p->stp_stats);
> kfree(p);
> p = ERR_PTR(err);
> }
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index a0a54482aabc..03aced1f862b 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1597,6 +1597,15 @@ static int br_fill_linkxstats(struct sk_buff *skb,
> }
> }
>
> + if (p) {
> + struct bridge_stp_xstats xstats;
Please rename the local var here, using just xstats is misleading.
Maybe stp_xstats ?
> +
> + br_stp_get_xstats(p, &xstats);
> +
> + if (nla_put(skb, BRIDGE_XSTATS_STP, sizeof(xstats), &xstats))
> + goto nla_put_failure;
Could you please follow how mcast xstats are dumped and do something similar ?
It'd be nice to have similar code to audit.
> + }
> +
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> if (++vl_idx >= *prividx) {
> nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_MCAST,
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 36b0367ca1e0..af5f28f0f2ef 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -95,6 +95,11 @@ struct br_vlan_stats {
> struct u64_stats_sync syncp;
> };
>
> +struct br_stp_stats {
> + struct bridge_stp_xstats xstats;
> + struct u64_stats_sync syncp;
> +};
> +
> struct br_tunnel_info {
> __be64 tunnel_id;
> struct metadata_dst *tunnel_dst;
> @@ -283,6 +288,8 @@ struct net_bridge_port {
> #endif
> u16 group_fwd_mask;
> u16 backup_redirected_cnt;
> +
> + struct br_stp_stats __percpu *stp_stats;
> };
>
> #define kobj_to_brport(obj) container_of(obj, struct net_bridge_port, kobj)
> @@ -1146,6 +1153,8 @@ void br_stp_change_bridge_id(struct net_bridge *br, const unsigned char *a);
> void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio);
> int br_stp_set_port_priority(struct net_bridge_port *p, unsigned long newprio);
> int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost);
> +void br_stp_get_xstats(const struct net_bridge_port *p,
> + struct bridge_stp_xstats *xstats);
> ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id);
>
> /* br_stp_bpdu.c */
> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
> index 1f1410f8d312..8bcdab29442d 100644
> --- a/net/bridge/br_stp.c
> +++ b/net/bridge/br_stp.c
> @@ -45,6 +45,18 @@ void br_set_state(struct net_bridge_port *p, unsigned int state)
> br_info(p->br, "port %u(%s) entered %s state\n",
> (unsigned int) p->port_no, p->dev->name,
> br_port_state_names[p->state]);
> +
> + if (p->br->stp_enabled == BR_KERNEL_STP) {
> + struct br_stp_stats *stats;
> +
> + stats = this_cpu_ptr(p->stp_stats);
> + u64_stats_update_begin(&stats->syncp);
> + if (p->state == BR_STATE_BLOCKING)
> + stats->xstats.transition_blk++;
> + else if (p->state == BR_STATE_FORWARDING)
> + stats->xstats.transition_fwd++;
> + u64_stats_update_end(&stats->syncp);
> + }
> }
>
> /* called under bridge lock */
> @@ -481,9 +493,15 @@ static void br_topology_change_acknowledge(struct net_bridge_port *p)
> void br_received_config_bpdu(struct net_bridge_port *p,
> const struct br_config_bpdu *bpdu)
> {
> + struct br_stp_stats *stats;
> struct net_bridge *br;
> int was_root;
>
> + stats = this_cpu_ptr(p->stp_stats);
> + u64_stats_update_begin(&stats->syncp);
> + stats->xstats.rx_bpdu++;
> + u64_stats_update_end(&stats->syncp);
> +
> br = p->br;
> was_root = br_is_root_bridge(br);
>
> @@ -517,6 +535,13 @@ void br_received_config_bpdu(struct net_bridge_port *p,
> /* called under bridge lock */
> void br_received_tcn_bpdu(struct net_bridge_port *p)
> {
> + struct br_stp_stats *stats;
> +
> + stats = this_cpu_ptr(p->stp_stats);
> + u64_stats_update_begin(&stats->syncp);
> + stats->xstats.rx_tcn++;
> + u64_stats_update_end(&stats->syncp);
> +
> if (br_is_designated_port(p)) {
> br_info(p->br, "port %u(%s) received tcn bpdu\n",
> (unsigned int) p->port_no, p->dev->name);
> diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
> index 7796dd9d42d7..2dbd11e21f2a 100644
> --- a/net/bridge/br_stp_bpdu.c
> +++ b/net/bridge/br_stp_bpdu.c
> @@ -78,6 +78,7 @@ static inline int br_get_ticks(const unsigned char *src)
> /* called under bridge lock */
> void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
> {
> + struct br_stp_stats *stats;
> unsigned char buf[35];
>
> if (p->br->stp_enabled != BR_KERNEL_STP)
> @@ -118,11 +119,17 @@ void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
> br_set_ticks(buf+33, bpdu->forward_delay);
>
> br_send_bpdu(p, buf, 35);
> +
> + stats = this_cpu_ptr(p->stp_stats);
> + u64_stats_update_begin(&stats->syncp);
> + stats->xstats.tx_bpdu++;
> + u64_stats_update_end(&stats->syncp);
> }
>
> /* called under bridge lock */
> void br_send_tcn_bpdu(struct net_bridge_port *p)
> {
> + struct br_stp_stats *stats;
> unsigned char buf[4];
>
> if (p->br->stp_enabled != BR_KERNEL_STP)
> @@ -133,6 +140,11 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
> buf[2] = 0;
> buf[3] = BPDU_TYPE_TCN;
> br_send_bpdu(p, buf, 4);
> +
> + stats = this_cpu_ptr(p->stp_stats);
> + u64_stats_update_begin(&stats->syncp);
> + stats->xstats.tx_tcn++;
> + u64_stats_update_end(&stats->syncp);
> }
>
> /*
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index d174d3a566aa..cbce7d0e40b9 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -333,6 +333,33 @@ int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost)
> return 0;
> }
>
> +void br_stp_get_xstats(const struct net_bridge_port *p,
> + struct bridge_stp_xstats *xstats)
> +{
> + int i;
> +
> + memset(xstats, 0, sizeof(*xstats));
> +
> + for_each_possible_cpu(i) {
> + struct bridge_stp_xstats cpu_xstats;
> + struct br_stp_stats *stats;
> + unsigned int start;
> +
> + stats = per_cpu_ptr(p->stp_stats, i);
> + do {
> + start = u64_stats_fetch_begin_irq(&stats->syncp);
> + memcpy(&cpu_xstats, &stats->xstats, sizeof(cpu_xstats));
> + } while (u64_stats_fetch_retry_irq(&stats->syncp, start));
> +
> + xstats->transition_blk += cpu_xstats.transition_blk;
> + xstats->transition_fwd += cpu_xstats.transition_fwd;
> + xstats->rx_bpdu += cpu_xstats.rx_bpdu;
> + xstats->tx_bpdu += cpu_xstats.tx_bpdu;
> + xstats->rx_tcn += cpu_xstats.rx_tcn;
> + xstats->tx_tcn += cpu_xstats.tx_tcn;
> + }
> +}
> +
> ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id)
> {
> return sprintf(buf, "%.2x%.2x.%.2x%.2x%.2x%.2x%.2x%.2x\n",
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next] net: bridge: add STP xstats
@ 2019-12-09 23:05 Vivien Didelot
2019-12-10 7:49 ` Nikolay Aleksandrov
0 siblings, 1 reply; 18+ messages in thread
From: Vivien Didelot @ 2019-12-09 23:05 UTC (permalink / raw)
To: David S. Miller
Cc: Roopa Prabhu, Nikolay Aleksandrov, netdev, bridge,
Stephen Hemminger, Vivien Didelot
This adds rx_bpdu, tx_bpdu, rx_tcn, tx_tcn, transition_blk,
transition_fwd xstats counters to the bridge ports copied over via
netlink, providing useful information for STP.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
include/uapi/linux/if_bridge.h | 10 ++++++++++
net/bridge/br_if.c | 8 ++++++++
net/bridge/br_netlink.c | 9 +++++++++
net/bridge/br_private.h | 9 +++++++++
net/bridge/br_stp.c | 25 +++++++++++++++++++++++++
net/bridge/br_stp_bpdu.c | 12 ++++++++++++
net/bridge/br_stp_if.c | 27 +++++++++++++++++++++++++++
7 files changed, 100 insertions(+)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 1b3c2b643a02..e7f2bb782006 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -156,6 +156,15 @@ struct bridge_vlan_xstats {
__u32 pad2;
};
+struct bridge_stp_xstats {
+ __u64 transition_blk;
+ __u64 transition_fwd;
+ __u64 rx_bpdu;
+ __u64 tx_bpdu;
+ __u64 rx_tcn;
+ __u64 tx_tcn;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -261,6 +270,7 @@ enum {
BRIDGE_XSTATS_UNSPEC,
BRIDGE_XSTATS_VLAN,
BRIDGE_XSTATS_MCAST,
+ BRIDGE_XSTATS_STP,
BRIDGE_XSTATS_PAD,
__BRIDGE_XSTATS_MAX
};
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 4fe30b182ee7..3eb214ef9763 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -250,6 +250,7 @@ static void release_nbp(struct kobject *kobj)
{
struct net_bridge_port *p
= container_of(kobj, struct net_bridge_port, kobj);
+ free_percpu(p->stp_stats);
kfree(p);
}
@@ -419,6 +420,12 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
if (p == NULL)
return ERR_PTR(-ENOMEM);
+ p->stp_stats = netdev_alloc_pcpu_stats(struct br_stp_stats);
+ if (!p->stp_stats) {
+ kfree(p);
+ return ERR_PTR(-ENOMEM);
+ }
+
p->br = br;
dev_hold(dev);
p->dev = dev;
@@ -432,6 +439,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
err = br_multicast_add_port(p);
if (err) {
dev_put(dev);
+ free_percpu(p->stp_stats);
kfree(p);
p = ERR_PTR(err);
}
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index a0a54482aabc..03aced1f862b 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1597,6 +1597,15 @@ static int br_fill_linkxstats(struct sk_buff *skb,
}
}
+ if (p) {
+ struct bridge_stp_xstats xstats;
+
+ br_stp_get_xstats(p, &xstats);
+
+ if (nla_put(skb, BRIDGE_XSTATS_STP, sizeof(xstats), &xstats))
+ goto nla_put_failure;
+ }
+
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
if (++vl_idx >= *prividx) {
nla = nla_reserve_64bit(skb, BRIDGE_XSTATS_MCAST,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 36b0367ca1e0..af5f28f0f2ef 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -95,6 +95,11 @@ struct br_vlan_stats {
struct u64_stats_sync syncp;
};
+struct br_stp_stats {
+ struct bridge_stp_xstats xstats;
+ struct u64_stats_sync syncp;
+};
+
struct br_tunnel_info {
__be64 tunnel_id;
struct metadata_dst *tunnel_dst;
@@ -283,6 +288,8 @@ struct net_bridge_port {
#endif
u16 group_fwd_mask;
u16 backup_redirected_cnt;
+
+ struct br_stp_stats __percpu *stp_stats;
};
#define kobj_to_brport(obj) container_of(obj, struct net_bridge_port, kobj)
@@ -1146,6 +1153,8 @@ void br_stp_change_bridge_id(struct net_bridge *br, const unsigned char *a);
void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio);
int br_stp_set_port_priority(struct net_bridge_port *p, unsigned long newprio);
int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost);
+void br_stp_get_xstats(const struct net_bridge_port *p,
+ struct bridge_stp_xstats *xstats);
ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id);
/* br_stp_bpdu.c */
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 1f1410f8d312..8bcdab29442d 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -45,6 +45,18 @@ void br_set_state(struct net_bridge_port *p, unsigned int state)
br_info(p->br, "port %u(%s) entered %s state\n",
(unsigned int) p->port_no, p->dev->name,
br_port_state_names[p->state]);
+
+ if (p->br->stp_enabled == BR_KERNEL_STP) {
+ struct br_stp_stats *stats;
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ if (p->state == BR_STATE_BLOCKING)
+ stats->xstats.transition_blk++;
+ else if (p->state == BR_STATE_FORWARDING)
+ stats->xstats.transition_fwd++;
+ u64_stats_update_end(&stats->syncp);
+ }
}
/* called under bridge lock */
@@ -481,9 +493,15 @@ static void br_topology_change_acknowledge(struct net_bridge_port *p)
void br_received_config_bpdu(struct net_bridge_port *p,
const struct br_config_bpdu *bpdu)
{
+ struct br_stp_stats *stats;
struct net_bridge *br;
int was_root;
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.rx_bpdu++;
+ u64_stats_update_end(&stats->syncp);
+
br = p->br;
was_root = br_is_root_bridge(br);
@@ -517,6 +535,13 @@ void br_received_config_bpdu(struct net_bridge_port *p,
/* called under bridge lock */
void br_received_tcn_bpdu(struct net_bridge_port *p)
{
+ struct br_stp_stats *stats;
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.rx_tcn++;
+ u64_stats_update_end(&stats->syncp);
+
if (br_is_designated_port(p)) {
br_info(p->br, "port %u(%s) received tcn bpdu\n",
(unsigned int) p->port_no, p->dev->name);
diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
index 7796dd9d42d7..2dbd11e21f2a 100644
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -78,6 +78,7 @@ static inline int br_get_ticks(const unsigned char *src)
/* called under bridge lock */
void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
{
+ struct br_stp_stats *stats;
unsigned char buf[35];
if (p->br->stp_enabled != BR_KERNEL_STP)
@@ -118,11 +119,17 @@ void br_send_config_bpdu(struct net_bridge_port *p, struct br_config_bpdu *bpdu)
br_set_ticks(buf+33, bpdu->forward_delay);
br_send_bpdu(p, buf, 35);
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.tx_bpdu++;
+ u64_stats_update_end(&stats->syncp);
}
/* called under bridge lock */
void br_send_tcn_bpdu(struct net_bridge_port *p)
{
+ struct br_stp_stats *stats;
unsigned char buf[4];
if (p->br->stp_enabled != BR_KERNEL_STP)
@@ -133,6 +140,11 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
buf[2] = 0;
buf[3] = BPDU_TYPE_TCN;
br_send_bpdu(p, buf, 4);
+
+ stats = this_cpu_ptr(p->stp_stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->xstats.tx_tcn++;
+ u64_stats_update_end(&stats->syncp);
}
/*
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index d174d3a566aa..cbce7d0e40b9 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -333,6 +333,33 @@ int br_stp_set_path_cost(struct net_bridge_port *p, unsigned long path_cost)
return 0;
}
+void br_stp_get_xstats(const struct net_bridge_port *p,
+ struct bridge_stp_xstats *xstats)
+{
+ int i;
+
+ memset(xstats, 0, sizeof(*xstats));
+
+ for_each_possible_cpu(i) {
+ struct bridge_stp_xstats cpu_xstats;
+ struct br_stp_stats *stats;
+ unsigned int start;
+
+ stats = per_cpu_ptr(p->stp_stats, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&stats->syncp);
+ memcpy(&cpu_xstats, &stats->xstats, sizeof(cpu_xstats));
+ } while (u64_stats_fetch_retry_irq(&stats->syncp, start));
+
+ xstats->transition_blk += cpu_xstats.transition_blk;
+ xstats->transition_fwd += cpu_xstats.transition_fwd;
+ xstats->rx_bpdu += cpu_xstats.rx_bpdu;
+ xstats->tx_bpdu += cpu_xstats.tx_bpdu;
+ xstats->rx_tcn += cpu_xstats.rx_tcn;
+ xstats->tx_tcn += cpu_xstats.tx_tcn;
+ }
+}
+
ssize_t br_show_bridge_id(char *buf, const struct bridge_id *id)
{
return sprintf(buf, "%.2x%.2x.%.2x%.2x%.2x%.2x%.2x%.2x\n",
--
2.24.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-12-10 21:49 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 21:18 [PATCH net-next] net: bridge: add STP xstats Vivien Didelot
2019-12-09 21:18 ` [Bridge] " Vivien Didelot
2019-12-09 21:18 ` [PATCH iproute2] iplink: add support for " Vivien Didelot
2019-12-09 21:18 ` [Bridge] " Vivien Didelot
2019-12-09 22:34 ` Stephen Hemminger
2019-12-09 22:34 ` [Bridge] " Stephen Hemminger
2019-12-09 23:15 ` Vivien Didelot
2019-12-09 23:15 ` [Bridge] " Vivien Didelot
2019-12-09 23:05 [PATCH net-next] net: bridge: add " Vivien Didelot
2019-12-10 7:49 ` Nikolay Aleksandrov
2019-12-10 7:54 ` Nikolay Aleksandrov
2019-12-10 19:39 ` Vivien Didelot
2019-12-10 19:50 ` Nikolay Aleksandrov
2019-12-10 20:10 ` Vivien Didelot
2019-12-10 20:15 ` Nikolay Aleksandrov
2019-12-10 20:34 ` Vivien Didelot
2019-12-10 20:52 ` Nikolay Aleksandrov
2019-12-10 21:08 ` Vivien Didelot
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.