* [PATCH net-next v2 1/5] net: rtnetlink: allow rtnl_fill_statsinfo to save private state counter
2016-04-28 15:52 [PATCH net-next v2 0/5] bridge: per-vlan stats Nikolay Aleksandrov
@ 2016-04-28 15:52 ` Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 2/5] net: rtnetlink: add linkxstats callbacks and attribute Nikolay Aleksandrov
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
The new prividx argument allows the current dumping device to save a
private state counter which would enable it to continue dumping from
where it left off. And the idxattr is used to save the current idx user
so multiple prividx using attributes can be requested at the same time
as suggested by Roopa Prabhu.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: improve the error check in rtnl_fill_statsinfo, rename lidx to
prividx, squash patch 2 into this one and save the current idx user
instead of restricting only one
net/core/rtnetlink.c | 44 +++++++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5503dfe6a050..de529a20cd18 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3444,13 +3444,21 @@ out:
return err;
}
+static bool stats_attr_valid(unsigned int mask, int attrid, int idxattr)
+{
+ return (mask & IFLA_STATS_FILTER_BIT(attrid)) &&
+ (!idxattr || idxattr == attrid);
+}
+
static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
- unsigned int flags, unsigned int filter_mask)
+ unsigned int flags, unsigned int filter_mask,
+ int *idxattr, int *prividx)
{
struct if_stats_msg *ifsm;
struct nlmsghdr *nlh;
struct nlattr *attr;
+ int s_prividx = *prividx;
ASSERT_RTNL();
@@ -3462,7 +3470,7 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
ifsm->ifindex = dev->ifindex;
ifsm->filter_mask = filter_mask;
- if (filter_mask & IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_64)) {
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, *idxattr)) {
struct rtnl_link_stats64 *sp;
attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_64,
@@ -3480,7 +3488,11 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
return 0;
nla_put_failure:
- nlmsg_cancel(skb, nlh);
+ /* not a multi message or no progress mean a real error */
+ if (!(flags & NLM_F_MULTI) || s_prividx == *prividx)
+ nlmsg_cancel(skb, nlh);
+ else
+ nlmsg_end(skb, nlh);
return -EMSGSIZE;
}
@@ -3494,7 +3506,7 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
{
size_t size = 0;
- if (filter_mask & IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_64))
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
return size;
@@ -3503,8 +3515,9 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
- struct if_stats_msg *ifsm;
struct net_device *dev = NULL;
+ int idxattr = 0, prividx = 0;
+ struct if_stats_msg *ifsm;
struct sk_buff *nskb;
u32 filter_mask;
int err;
@@ -3528,7 +3541,7 @@ static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh)
err = rtnl_fill_statsinfo(nskb, dev, RTM_NEWSTATS,
NETLINK_CB(skb).portid, nlh->nlmsg_seq, 0,
- 0, filter_mask);
+ 0, filter_mask, &idxattr, &prividx);
if (err < 0) {
/* -EMSGSIZE implies BUG in if_nlmsg_stats_size */
WARN_ON(err == -EMSGSIZE);
@@ -3542,18 +3555,19 @@ static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh)
static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
{
+ int h, s_h, err, s_idx, s_idxattr, s_prividx;
struct net *net = sock_net(skb->sk);
+ unsigned int flags = NLM_F_MULTI;
struct if_stats_msg *ifsm;
- int h, s_h;
- int idx = 0, s_idx;
- struct net_device *dev;
struct hlist_head *head;
- unsigned int flags = NLM_F_MULTI;
+ struct net_device *dev;
u32 filter_mask = 0;
- int err;
+ int idx = 0;
s_h = cb->args[0];
s_idx = cb->args[1];
+ s_idxattr = cb->args[2];
+ s_prividx = cb->args[3];
cb->seq = net->dev_base_seq;
@@ -3571,7 +3585,8 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
err = rtnl_fill_statsinfo(skb, dev, RTM_NEWSTATS,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, 0,
- flags, filter_mask);
+ flags, filter_mask,
+ &s_idxattr, &s_prividx);
/* If we ran out of room on the first message,
* we're in trouble
*/
@@ -3579,13 +3594,16 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
if (err < 0)
goto out;
-
+ s_prividx = 0;
+ s_idxattr = 0;
nl_dump_check_consistent(cb, nlmsg_hdr(skb));
cont:
idx++;
}
}
out:
+ cb->args[3] = s_prividx;
+ cb->args[2] = s_idxattr;
cb->args[1] = idx;
cb->args[0] = h;
--
2.4.11
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next v2 2/5] net: rtnetlink: add linkxstats callbacks and attribute
2016-04-28 15:52 [PATCH net-next v2 0/5] bridge: per-vlan stats Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 1/5] net: rtnetlink: allow rtnl_fill_statsinfo to save private state counter Nikolay Aleksandrov
@ 2016-04-28 15:52 ` Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 3/5] bridge: vlan: RCUify pvid Nikolay Aleksandrov
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
Add callbacks to calculate the size and fill link extended statistics
which can be split into multiple messages and are dumped via the new
rtnl stats API (RTM_GETSTATS) with the IFLA_STATS_LINK_XSTATS attribute.
Also add that attribute to the idx mask check since it is expected to
be able to save state and resume dumping (e.g. future bridge per-vlan
stats will be dumped via this attribute and callbacks).
Each link type should nest its private attributes under the per-link type
attribute. This allows to have any number of separated private attributes
and to avoid one call to get the dev link type.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: add callback descriptions and make the size calculation more
accurate, change the netlink xstats message structure with one more
level for each rtnl link type which allows for private link type attributes
and also allows us to avoid 1 call to get the dev link type.
include/net/rtnetlink.h | 7 +++++++
include/uapi/linux/if_link.h | 12 ++++++++++++
net/core/rtnetlink.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 2f87c1ba13de..006a7b81d758 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -47,6 +47,9 @@ static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
* @get_num_rx_queues: Function to determine number of receive queues
* to create when creating a new device.
* @get_link_net: Function to get the i/o netns of the device
+ * @get_linkxstats_size: Function to calculate the required room for
+ * dumping device-specific extended link stats
+ * @fill_linkxstats: Function to dump device-specific extended link stats
*/
struct rtnl_link_ops {
struct list_head list;
@@ -95,6 +98,10 @@ struct rtnl_link_ops {
const struct net_device *dev,
const struct net_device *slave_dev);
struct net *(*get_link_net)(const struct net_device *dev);
+ size_t (*get_linkxstats_size)(const struct net_device *dev);
+ int (*fill_linkxstats)(struct sk_buff *skb,
+ const struct net_device *dev,
+ int *prividx);
};
int __rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d82de331bb6b..8577c0e4116f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -802,6 +802,7 @@ struct if_stats_msg {
enum {
IFLA_STATS_UNSPEC, /* also used as 64bit pad attribute */
IFLA_STATS_LINK_64,
+ IFLA_STATS_LINK_XSTATS,
__IFLA_STATS_MAX,
};
@@ -809,4 +810,15 @@ enum {
#define IFLA_STATS_FILTER_BIT(ATTR) (1 << (ATTR - 1))
+/* These are embedded into IFLA_STATS_LINK_XSTATS:
+ * [IFLA_STATS_LINK_XSTATS]
+ * -> [LINK_XSTATS_TYPE_xxx]
+ * -> [rtnl link type specific attributes]
+ */
+enum {
+ LINK_XSTATS_TYPE_UNSPEC,
+ __LINK_XSTATS_TYPE_MAX
+};
+#define LINK_XSTATS_TYPE_MAX (__LINK_XSTATS_TYPE_MAX - 1)
+
#endif /* _UAPI_LINUX_IF_LINK_H */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index de529a20cd18..d471f097c739 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3483,6 +3483,26 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
dev_get_stats(dev, sp);
}
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, *idxattr)) {
+ const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
+
+ if (ops && ops->fill_linkxstats) {
+ int err;
+
+ *idxattr = IFLA_STATS_LINK_XSTATS;
+ attr = nla_nest_start(skb,
+ IFLA_STATS_LINK_XSTATS);
+ if (!attr)
+ goto nla_put_failure;
+
+ err = ops->fill_linkxstats(skb, dev, prividx);
+ nla_nest_end(skb, attr);
+ if (err)
+ goto nla_put_failure;
+ *idxattr = 0;
+ }
+ }
+
nlmsg_end(skb, nlh);
return 0;
@@ -3509,6 +3529,16 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, 0)) {
+ const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
+
+ if (ops && ops->get_linkxstats_size) {
+ size += nla_total_size(ops->get_linkxstats_size(dev));
+ /* for IFLA_STATS_LINK_XSTATS */
+ size += nla_total_size(0);
+ }
+ }
+
return size;
}
--
2.4.11
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next v2 3/5] bridge: vlan: RCUify pvid
2016-04-28 15:52 [PATCH net-next v2 0/5] bridge: per-vlan stats Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 1/5] net: rtnetlink: allow rtnl_fill_statsinfo to save private state counter Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 2/5] net: rtnetlink: add linkxstats callbacks and attribute Nikolay Aleksandrov
@ 2016-04-28 15:52 ` Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 4/5] bridge: vlan: learn to count Nikolay Aleksandrov
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
Make pvid a pointer to a vlan struct and RCUify the access to it. Vlans
are already RCU-protected so the pvid vlan entry cannot disappear
without being initialized to NULL and going through a grace period first.
This change is necessary for the upcoming vlan counters and also would
serve to later move to vlan passing via a pointer instead of id.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: no change
net/bridge/br_netlink.c | 23 ++++++++++-----------
net/bridge/br_private.h | 16 +--------------
net/bridge/br_vlan.c | 54 +++++++++++++++++++++++--------------------------
3 files changed, 37 insertions(+), 56 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 6bae1125e36d..9c74fa438a8c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -24,22 +24,22 @@
static int __get_num_vlan_infos(struct net_bridge_vlan_group *vg,
u32 filter_mask)
{
- struct net_bridge_vlan *v;
+ struct net_bridge_vlan *v, *pvid;
u16 vid_range_start = 0, vid_range_end = 0, vid_range_flags = 0;
- u16 flags, pvid;
int num_vlans = 0;
+ u16 flags;
if (!(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED))
return 0;
- pvid = br_get_pvid(vg);
+ pvid = rcu_dereference(vg->pvid);
/* Count number of vlan infos */
list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
flags = 0;
/* only a context, bridge vlan not activated */
if (!br_vlan_should_use(v))
continue;
- if (v->vid == pvid)
+ if (v == pvid)
flags |= BRIDGE_VLAN_INFO_PVID;
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
@@ -246,21 +246,21 @@ nla_put_failure:
static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
struct net_bridge_vlan_group *vg)
{
- struct net_bridge_vlan *v;
+ struct net_bridge_vlan *v, *pvid;
u16 vid_range_start = 0, vid_range_end = 0, vid_range_flags = 0;
- u16 flags, pvid;
int err = 0;
+ u16 flags;
/* Pack IFLA_BRIDGE_VLAN_INFO's for every vlan
* and mark vlan info with begin and end flags
* if vlaninfo represents a range
*/
- pvid = br_get_pvid(vg);
+ pvid = rcu_dereference(vg->pvid);
list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
flags = 0;
if (!br_vlan_should_use(v))
continue;
- if (v->vid == pvid)
+ if (v == pvid)
flags |= BRIDGE_VLAN_INFO_PVID;
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
@@ -301,18 +301,17 @@ initvars:
static int br_fill_ifvlaninfo(struct sk_buff *skb,
struct net_bridge_vlan_group *vg)
{
+ struct net_bridge_vlan *v, *pvid;
struct bridge_vlan_info vinfo;
- struct net_bridge_vlan *v;
- u16 pvid;
- pvid = br_get_pvid(vg);
+ pvid = rcu_dereference(vg->pvid);
list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
if (!br_vlan_should_use(v))
continue;
vinfo.vid = v->vid;
vinfo.flags = 0;
- if (v->vid == pvid)
+ if (v == pvid)
vinfo.flags |= BRIDGE_VLAN_INFO_PVID;
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1b5d145dfcbf..50d70b5eb307 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -130,8 +130,8 @@ struct net_bridge_vlan {
struct net_bridge_vlan_group {
struct rhashtable vlan_hash;
struct list_head vlan_list;
+ struct net_bridge_vlan __rcu *pvid;
u16 num_vlans;
- u16 pvid;
};
struct net_bridge_fdb_entry
@@ -741,15 +741,6 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
return err;
}
-static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
-{
- if (!vg)
- return 0;
-
- smp_rmb();
- return vg->pvid;
-}
-
static inline int br_vlan_enabled(struct net_bridge *br)
{
return br->vlan_enabled;
@@ -835,11 +826,6 @@ static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
return 0;
}
-static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
-{
- return 0;
-}
-
static inline int br_vlan_enabled(struct net_bridge *br)
{
return 0;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index e001152d6ad1..4fab7665df8c 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -31,22 +31,11 @@ static struct net_bridge_vlan *br_vlan_lookup(struct rhashtable *tbl, u16 vid)
return rhashtable_lookup_fast(tbl, &vid, br_vlan_rht_params);
}
-static void __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+/* __vlan_delete_pvid is just __vlan_set_pvid(vg, NULL) */
+static void __vlan_set_pvid(struct net_bridge_vlan_group *vg,
+ struct net_bridge_vlan *v)
{
- if (vg->pvid == vid)
- return;
-
- smp_wmb();
- vg->pvid = vid;
-}
-
-static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
-{
- if (vg->pvid != vid)
- return;
-
- smp_wmb();
- vg->pvid = 0;
+ rcu_assign_pointer(vg->pvid, v);
}
static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
@@ -59,9 +48,9 @@ static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
vg = nbp_vlan_group(v->port);
if (flags & BRIDGE_VLAN_INFO_PVID)
- __vlan_add_pvid(vg, v->vid);
- else
- __vlan_delete_pvid(vg, v->vid);
+ __vlan_set_pvid(vg, v);
+ else if (rtnl_dereference(vg->pvid) == v)
+ __vlan_set_pvid(vg, NULL);
if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
@@ -285,7 +274,9 @@ static int __vlan_del(struct net_bridge_vlan *v)
masterv = v->brvlan;
}
- __vlan_delete_pvid(vg, v->vid);
+ if (rtnl_dereference(vg->pvid) == v)
+ __vlan_set_pvid(vg, NULL);
+
if (p) {
err = __vlan_vid_del(p->dev, p->br, v->vid);
if (err)
@@ -320,7 +311,7 @@ static void __vlan_flush(struct net_bridge_vlan_group *vg)
{
struct net_bridge_vlan *vlan, *tmp;
- __vlan_delete_pvid(vg, vg->pvid);
+ __vlan_set_pvid(vg, NULL);
list_for_each_entry_safe(vlan, tmp, &vg->vlan_list, vlist)
__vlan_del(vlan);
}
@@ -404,29 +395,29 @@ static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
}
if (!*vid) {
- u16 pvid = br_get_pvid(vg);
+ v = rcu_dereference(vg->pvid);
/* Frame had a tag with VID 0 or did not have a tag.
* See if pvid is set on this port. That tells us which
* vlan untagged or priority-tagged traffic belongs to.
*/
- if (!pvid)
+ if (!v)
goto drop;
/* PVID is set on this port. Any untagged or priority-tagged
* ingress frame is considered to belong to this vlan.
*/
- *vid = pvid;
+ *vid = v->vid;
if (likely(!tagged))
/* Untagged Frame. */
- __vlan_hwaccel_put_tag(skb, proto, pvid);
+ __vlan_hwaccel_put_tag(skb, proto, v->vid);
else
/* Priority-tagged Frame.
* At this point, We know that skb->vlan_tci had
* VLAN_TAG_PRESENT bit and its VID field was 0x000.
* We update only VID field and preserve PCP field.
*/
- skb->vlan_tci |= pvid;
+ skb->vlan_tci |= v->vid;
return true;
}
@@ -451,6 +442,9 @@ bool br_allowed_ingress(const struct net_bridge *br,
BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
return true;
}
+ /* if there's no vlan_group, there's nothing to match against */
+ if (!vg)
+ return false;
return __allowed_ingress(vg, br->vlan_proto, skb, vid);
}
@@ -492,9 +486,11 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
*vid = 0;
if (!*vid) {
- *vid = br_get_pvid(vg);
- if (!*vid)
+ struct net_bridge_vlan *v = rcu_dereference(vg->pvid);
+
+ if (!v)
return false;
+ *vid = v->vid;
return true;
}
@@ -713,9 +709,9 @@ int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
static bool vlan_default_pvid(struct net_bridge_vlan_group *vg, u16 vid)
{
- struct net_bridge_vlan *v;
+ struct net_bridge_vlan *v = rtnl_dereference(vg->pvid);
- if (vid != vg->pvid)
+ if (v && vid != v->vid)
return false;
v = br_vlan_lookup(&vg->vlan_hash, vid);
--
2.4.11
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next v2 4/5] bridge: vlan: learn to count
2016-04-28 15:52 [PATCH net-next v2 0/5] bridge: per-vlan stats Nikolay Aleksandrov
` (2 preceding siblings ...)
2016-04-28 15:52 ` [PATCH net-next v2 3/5] bridge: vlan: RCUify pvid Nikolay Aleksandrov
@ 2016-04-28 15:52 ` Nikolay Aleksandrov
2016-04-28 15:52 ` [PATCH net-next v2 5/5] bridge: netlink: export per-vlan stats Nikolay Aleksandrov
2016-04-29 19:33 ` [PATCH net-next v2 0/5] bridge: " David Miller
5 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
Add support for per-VLAN Tx/Rx statistics. Every global vlan context gets
allocated a per-cpu stats which is then set in each per-port vlan context
for quick access. The br_allowed_ingress() common function is used to
account for Rx packets and the br_handle_vlan() common function is used
to account for Tx packets.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: no change
net/bridge/br_private.h | 11 +++++++++-
net/bridge/br_vlan.c | 53 +++++++++++++++++++++++++++++++++++++++----------
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 50d70b5eb307..f6876ed718a5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -77,12 +77,21 @@ struct bridge_mcast_querier {
};
#endif
+struct br_vlan_stats {
+ u64 rx_bytes;
+ u64 rx_packets;
+ u64 tx_bytes;
+ u64 tx_packets;
+ struct u64_stats_sync syncp;
+};
+
/**
* struct net_bridge_vlan - per-vlan entry
*
* @vnode: rhashtable member
* @vid: VLAN id
* @flags: bridge vlan flags
+ * @stats: per-cpu VLAN statistics
* @br: if MASTER flag set, this points to a bridge struct
* @port: if MASTER flag unset, this points to a port struct
* @refcnt: if MASTER flag set, this is bumped for each port referencing it
@@ -100,6 +109,7 @@ struct net_bridge_vlan {
struct rhash_head vnode;
u16 vid;
u16 flags;
+ struct br_vlan_stats __percpu *stats;
union {
struct net_bridge *br;
struct net_bridge_port *port;
@@ -866,7 +876,6 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
{
return NULL;
}
-
#endif
struct nf_br_ops {
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 4fab7665df8c..d7a70c2ea3ec 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -151,6 +151,17 @@ static struct net_bridge_vlan *br_vlan_get_master(struct net_bridge *br, u16 vid
return masterv;
}
+static void br_master_vlan_rcu_free(struct rcu_head *rcu)
+{
+ struct net_bridge_vlan *v;
+
+ v = container_of(rcu, struct net_bridge_vlan, rcu);
+ WARN_ON(!br_vlan_is_master(v));
+ free_percpu(v->stats);
+ v->stats = NULL;
+ kfree(v);
+}
+
static void br_vlan_put_master(struct net_bridge_vlan *masterv)
{
struct net_bridge_vlan_group *vg;
@@ -163,7 +174,7 @@ static void br_vlan_put_master(struct net_bridge_vlan *masterv)
rhashtable_remove_fast(&vg->vlan_hash,
&masterv->vnode, br_vlan_rht_params);
__vlan_del_list(masterv);
- kfree_rcu(masterv, rcu);
+ call_rcu(&masterv->rcu, br_master_vlan_rcu_free);
}
}
@@ -219,6 +230,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
if (!masterv)
goto out_filt;
v->brvlan = masterv;
+ v->stats = masterv->stats;
}
/* Add the dev mac and count the vlan only if it's usable */
@@ -320,6 +332,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
struct net_bridge_vlan_group *vg,
struct sk_buff *skb)
{
+ struct br_vlan_stats *stats;
struct net_bridge_vlan *v;
u16 vid;
@@ -346,9 +359,14 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
return NULL;
}
}
+ stats = this_cpu_ptr(v->stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->tx_bytes += skb->len;
+ stats->tx_packets++;
+ u64_stats_update_end(&stats->syncp);
+
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
skb->vlan_tci = 0;
-
out:
return skb;
}
@@ -357,7 +375,8 @@ out:
static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
struct sk_buff *skb, u16 *vid)
{
- const struct net_bridge_vlan *v;
+ struct br_vlan_stats *stats;
+ struct net_bridge_vlan *v;
bool tagged;
BR_INPUT_SKB_CB(skb)->vlan_filtered = true;
@@ -418,14 +437,21 @@ static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
* We update only VID field and preserve PCP field.
*/
skb->vlan_tci |= v->vid;
-
- return true;
+ } else {
+ /* Frame had a valid vlan tag. See if vlan is allowed */
+ v = br_vlan_find(vg, *vid);
}
+ if (!v || !br_vlan_should_use(v))
+ goto drop;
+
+ stats = this_cpu_ptr(v->stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->rx_bytes += skb->len;
+ stats->rx_packets++;
+ u64_stats_update_end(&stats->syncp);
+
+ return true;
- /* Frame had a valid vlan tag. See if vlan is allowed */
- v = br_vlan_find(vg, *vid);
- if (v && br_vlan_should_use(v))
- return true;
drop:
kfree_skb(skb);
return false;
@@ -538,6 +564,11 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
if (!vlan)
return -ENOMEM;
+ vlan->stats = netdev_alloc_pcpu_stats(struct br_vlan_stats);
+ if (!vlan->stats) {
+ kfree(vlan);
+ return -ENOMEM;
+ }
vlan->vid = vid;
vlan->flags = flags | BRIDGE_VLAN_INFO_MASTER;
vlan->flags &= ~BRIDGE_VLAN_INFO_PVID;
@@ -545,8 +576,10 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
if (flags & BRIDGE_VLAN_INFO_BRENTRY)
atomic_set(&vlan->refcnt, 1);
ret = __vlan_add(vlan, flags);
- if (ret)
+ if (ret) {
+ free_percpu(vlan->stats);
kfree(vlan);
+ }
return ret;
}
--
2.4.11
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next v2 5/5] bridge: netlink: export per-vlan stats
2016-04-28 15:52 [PATCH net-next v2 0/5] bridge: per-vlan stats Nikolay Aleksandrov
` (3 preceding siblings ...)
2016-04-28 15:52 ` [PATCH net-next v2 4/5] bridge: vlan: learn to count Nikolay Aleksandrov
@ 2016-04-28 15:52 ` Nikolay Aleksandrov
2016-04-29 19:33 ` [PATCH net-next v2 0/5] bridge: " David Miller
5 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
Add a new LINK_XSTATS_TYPE_BRIDGE attribute and implement the
RTM_GETSTATS callbacks for IFLA_STATS_LINK_XSTATS (fill_linkxstats and
get_linkxstats_size) in order to export the per-vlan stats.
The paddings were added because soon these fields will be needed for
per-port per-vlan stats (or something else if someone beats me to it) so
avoiding at least a few more netlink attributes.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: remove unused pvid pointer, fix the case where bridge has 0 vlans
but there're global contexts and move to rtnl link type private
attributes nested into a LINK_XSTATS_TYPE_ attribute. The paddings were
added because soon these fields will be needed for per-port per-vlan
stats (or something else if someone beats me to it) so avoiding at least
a few more netlink attributes.
include/uapi/linux/if_bridge.h | 18 ++++++++++++
include/uapi/linux/if_link.h | 1 +
net/bridge/br_netlink.c | 65 ++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_private.h | 7 +++++
net/bridge/br_vlan.c | 27 ++++++++++++++++++
5 files changed, 118 insertions(+)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 0536eefff9bf..397d503fdedb 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -134,6 +134,16 @@ struct bridge_vlan_info {
__u16 vid;
};
+struct bridge_vlan_xstats {
+ __u64 rx_bytes;
+ __u64 rx_packets;
+ __u64 tx_bytes;
+ __u64 tx_packets;
+ __u16 vid;
+ __u16 pad1;
+ __u32 pad2;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -233,4 +243,12 @@ enum {
};
#define MDBA_SET_ENTRY_MAX (__MDBA_SET_ENTRY_MAX - 1)
+/* Embedded inside LINK_XSTATS_TYPE_BRIDGE */
+enum {
+ BRIDGE_XSTATS_UNSPEC,
+ BRIDGE_XSTATS_VLAN,
+ __BRIDGE_XSTATS_MAX
+};
+#define BRIDGE_XSTATS_MAX (__BRIDGE_XSTATS_MAX - 1)
+
#endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 8577c0e4116f..fd0621ec9222 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -817,6 +817,7 @@ enum {
*/
enum {
LINK_XSTATS_TYPE_UNSPEC,
+ LINK_XSTATS_TYPE_BRIDGE,
__LINK_XSTATS_TYPE_MAX
};
#define LINK_XSTATS_TYPE_MAX (__LINK_XSTATS_TYPE_MAX - 1)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9c74fa438a8c..c36bafcad569 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1222,6 +1222,69 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
return 0;
}
+static size_t br_get_linkxstats_size(const struct net_device *dev)
+{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_vlan *v;
+ int numvls = 0;
+
+ vg = br_vlan_group(br);
+ if (!vg)
+ return 0;
+
+ /* we need to count all, even placeholder entries */
+ list_for_each_entry(v, &vg->vlan_list, vlist)
+ numvls++;
+
+ /* account for the vlans and the link xstats type nest attribute */
+ return numvls * nla_total_size(sizeof(struct bridge_vlan_xstats)) +
+ nla_total_size(0);
+}
+
+static int br_fill_linkxstats(struct sk_buff *skb, const struct net_device *dev,
+ int *prividx)
+{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_vlan *v;
+ struct nlattr *nest;
+ int vl_idx = 0;
+
+ vg = br_vlan_group(br);
+ if (!vg)
+ goto out;
+ nest = nla_nest_start(skb, LINK_XSTATS_TYPE_BRIDGE);
+ if (!nest)
+ return -EMSGSIZE;
+ list_for_each_entry(v, &vg->vlan_list, vlist) {
+ struct bridge_vlan_xstats vxi;
+ struct br_vlan_stats stats;
+
+ if (vl_idx++ < *prividx)
+ continue;
+ memset(&vxi, 0, sizeof(vxi));
+ vxi.vid = v->vid;
+ br_vlan_get_stats(v, &stats);
+ vxi.rx_bytes = stats.rx_bytes;
+ vxi.rx_packets = stats.rx_packets;
+ vxi.tx_bytes = stats.tx_bytes;
+ vxi.tx_packets = stats.tx_packets;
+
+ if (nla_put(skb, BRIDGE_XSTATS_VLAN, sizeof(vxi), &vxi))
+ goto nla_put_failure;
+ }
+ nla_nest_end(skb, nest);
+ *prividx = 0;
+out:
+ return 0;
+
+nla_put_failure:
+ nla_nest_end(skb, nest);
+ *prividx = vl_idx;
+
+ return -EMSGSIZE;
+}
static struct rtnl_af_ops br_af_ops __read_mostly = {
.family = AF_BRIDGE,
@@ -1240,6 +1303,8 @@ struct rtnl_link_ops br_link_ops __read_mostly = {
.dellink = br_dev_delete,
.get_size = br_get_size,
.fill_info = br_fill_info,
+ .fill_linkxstats = br_fill_linkxstats,
+ .get_linkxstats_size = br_get_linkxstats_size,
.slave_maxtype = IFLA_BRPORT_MAX,
.slave_policy = br_port_policy,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f6876ed718a5..a10f7ed26f3b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -709,6 +709,8 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
void nbp_vlan_flush(struct net_bridge_port *port);
int nbp_vlan_init(struct net_bridge_port *port);
int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask);
+void br_vlan_get_stats(const struct net_bridge_vlan *v,
+ struct br_vlan_stats *stats);
static inline struct net_bridge_vlan_group *br_vlan_group(
const struct net_bridge *br)
@@ -876,6 +878,11 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
{
return NULL;
}
+
+static inline void br_vlan_get_stats(const struct net_bridge_vlan *v,
+ struct br_vlan_stats *stats)
+{
+}
#endif
struct nf_br_ops {
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index d7a70c2ea3ec..b39d9f5761d9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1029,3 +1029,30 @@ void nbp_vlan_flush(struct net_bridge_port *port)
synchronize_rcu();
__vlan_group_free(vg);
}
+
+void br_vlan_get_stats(const struct net_bridge_vlan *v,
+ struct br_vlan_stats *stats)
+{
+ int i;
+
+ memset(stats, 0, sizeof(*stats));
+ for_each_possible_cpu(i) {
+ u64 rxpackets, rxbytes, txpackets, txbytes;
+ struct br_vlan_stats *cpu_stats;
+ unsigned int start;
+
+ cpu_stats = per_cpu_ptr(v->stats, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
+ rxpackets = cpu_stats->rx_packets;
+ rxbytes = cpu_stats->rx_bytes;
+ txbytes = cpu_stats->tx_bytes;
+ txpackets = cpu_stats->tx_packets;
+ } while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
+
+ stats->rx_packets += rxpackets;
+ stats->rx_bytes += rxbytes;
+ stats->tx_bytes += txbytes;
+ stats->tx_packets += txpackets;
+ }
+}
--
2.4.11
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
2016-04-28 15:52 [PATCH net-next v2 0/5] bridge: per-vlan stats Nikolay Aleksandrov
` (4 preceding siblings ...)
2016-04-28 15:52 ` [PATCH net-next v2 5/5] bridge: netlink: export per-vlan stats Nikolay Aleksandrov
@ 2016-04-29 19:33 ` David Miller
2016-04-29 19:49 ` Nikolay Aleksandrov
5 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2016-04-29 19:33 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen, jhs
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 28 Apr 2016 17:52:46 +0200
> This set adds support for bridge per-vlan statistics.
Between the counter bumps in fast paths and new levels of pointer
indirection in order to RCU things, I have to agree with Stephen
that this new overhead is really pushing it.
All of this new overhead contributes to the transactional overhead
for every single packet.
Sorry I'm not going to apply this for now, unless you can come up
with something significantly cheaper.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
2016-04-29 19:33 ` [PATCH net-next v2 0/5] bridge: " David Miller
@ 2016-04-29 19:49 ` Nikolay Aleksandrov
2016-04-29 20:12 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-29 19:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, roopa, stephen, jhs
On 04/29/2016 09:33 PM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Thu, 28 Apr 2016 17:52:46 +0200
>
>> This set adds support for bridge per-vlan statistics.
>
> Between the counter bumps in fast paths and new levels of pointer
> indirection in order to RCU things, I have to agree with Stephen
> that this new overhead is really pushing it.
>
> All of this new overhead contributes to the transactional overhead
> for every single packet.
>
> Sorry I'm not going to apply this for now, unless you can come up
> with something significantly cheaper.
>
> Thanks.
>
Okay, thanks for the feedback. Is this about the RCUfication of the pvid ?
Because that is not needed for the per-vlan stats to work, I did to unify the paths
and simplify the pvid code but I can easily drop it and revert back to using
the direct pvid id.
The only fetch will be the stats per-cpu pointer then. Would that be acceptable ?
Cheers,
Nik
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
2016-04-29 19:49 ` Nikolay Aleksandrov
@ 2016-04-29 20:12 ` David Miller
2016-04-29 20:19 ` Nikolay Aleksandrov
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2016-04-29 20:12 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen, jhs
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri, 29 Apr 2016 21:49:17 +0200
> Because that is not needed for the per-vlan stats to work, I did to
> unify the paths and simplify the pvid code but I can easily drop it
> and revert back to using the direct pvid id. The only fetch will be
> the stats per-cpu pointer then. Would that be acceptable ?
It would be a step in the right direction, for sure.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v2 0/5] bridge: per-vlan stats
2016-04-29 20:12 ` David Miller
@ 2016-04-29 20:19 ` Nikolay Aleksandrov
0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-04-29 20:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev, roopa, stephen, jhs
On 04/29/2016 10:12 PM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Fri, 29 Apr 2016 21:49:17 +0200
>
>> Because that is not needed for the per-vlan stats to work, I did to
>> unify the paths and simplify the pvid code but I can easily drop it
>> and revert back to using the direct pvid id. The only fetch will be
>> the stats per-cpu pointer then. Would that be acceptable ?
>
> It would be a step in the right direction, for sure.
>
Okay, just one more thing I forgot to mention - please note that my code swaps
an unconditional smp_rmb() (in br_get_pvid()) for a pointer fetch, I'm not sure
the pointer fetch is slower as it's probably already in the cache if that vlan
is used.
Anyway, I will resubmit without that patch.
Thanks,
Nik
^ permalink raw reply [flat|nested] 10+ messages in thread