Re: [Bridge] [PATCH net-next] net: bridge: add STP xstats

2019-12-09 Thread Nikolay Aleksandrov
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 
>> ---
>>  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.




Re: [Bridge] [PATCH net-next] net: bridge: add STP xstats

2019-12-09 Thread Nikolay Aleksandrov
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 
> ---
>  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 bridg

Re: [Bridge] [PATCH iproute2 v2] iplink: add support for STP xstats

2019-12-09 Thread Stephen Hemminger
On Mon,  9 Dec 2019 18:05:22 -0500
Vivien Didelot  wrote:

> 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
> ...

Might I suggest a more concise format:
STP BPDU:  RX: 0 TX: 39
STP TCN:   RX: 0 TX:0
STP Transitions: Blocked: 0 Forwarding: 1
...


Re: [Bridge] [PATCH iproute2] iplink: add support for STP xstats

2019-12-09 Thread Vivien Didelot
On Mon, 9 Dec 2019 14:34:23 -0800, Stephen Hemminger 
 wrote:
> On Mon,  9 Dec 2019 16:18:41 -0500
> Vivien Didelot  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


[Bridge] [PATCH iproute2 v2] iplink: add support for STP xstats

2019-12-09 Thread 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 
---
 include/uapi/linux/if_bridge.h | 10 ++
 ip/iplink_bridge.c | 32 
 2 files changed, 42 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 31fc51bd..42f76697 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/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,
+"%-16sSTP 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,
+"%-16sSTP 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,
+"%-16sSTP 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



[Bridge] [PATCH net-next] net: bridge: add STP xstats

2019-12-09 Thread 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 
---
 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_updat

Re: [Bridge] [PATCH iproute2] iplink: add support for STP xstats

2019-12-09 Thread Stephen Hemminger
On Mon,  9 Dec 2019 16:18:41 -0500
Vivien Didelot  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.


[Bridge] [PATCH iproute2] iplink: add support for STP xstats

2019-12-09 Thread 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 
---
 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 
 #include 
@@ -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,
+"%-16sSTP 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,
+"%-16sSTP 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,
+"%-16sSTP 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(*

[Bridge] [PATCH net-next] net: bridge: add STP xstats

2019-12-09 Thread 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 
---
 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_updat