* [PATCH net-next 0/2] bridge: mdb: add support for extended attributes
@ 2016-02-16 11:46 Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 1/2] bridge: mdb: reduce the indentation level in br_mdb_fill_info Nikolay Aleksandrov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-16 11:46 UTC (permalink / raw)
To: netdev; +Cc: roopa, stephen, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Hi,
This small set allows to extend the per mdb entry exported attributes,
before this set we had only a structure exported which couldn't be changed
because we would've broken user-space, after this we extend the attribute
that was used for the structure and add per-mdb entry attributes after the
struct has been added (see patch 02 for more details). Note that the reason
we can't simply add an attribute after MDBA_MDB_ENTRY_INFO is that current
users (e.g. iproute2) walk over the attribute list directly without
checking for the attribute type.
Patch 01 is a simple change to reduce one indentation level in order to
avoid over 80 char lines.
Cheers,
Nik
Nikolay Aleksandrov (2):
bridge: mdb: reduce the indentation level in br_mdb_fill_info
bridge: mdb: add support for more attributes and export timer
include/uapi/linux/if_bridge.h | 13 +++++++++++-
net/bridge/br_mdb.c | 47 ++++++++++++++++++++++++++++--------------
2 files changed, 43 insertions(+), 17 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] bridge: mdb: reduce the indentation level in br_mdb_fill_info
2016-02-16 11:46 [PATCH net-next 0/2] bridge: mdb: add support for extended attributes Nikolay Aleksandrov
@ 2016-02-16 11:46 ` Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 2/2] bridge: mdb: add support for more attributes and export timer Nikolay Aleksandrov
2016-02-18 20:37 ` [PATCH net-next 0/2] bridge: mdb: add support for extended attributes David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-16 11:46 UTC (permalink / raw)
To: netdev; +Cc: roopa, stephen, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Switch the port check and skip if it's null, this allows us to reduce one
indentation level.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_mdb.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index ac089286526e..e66619171386 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -88,25 +88,26 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
for (pp = &mp->ports;
(p = rcu_dereference(*pp)) != NULL;
pp = &p->next) {
+ struct br_mdb_entry e;
+
port = p->port;
- if (port) {
- struct br_mdb_entry e;
- memset(&e, 0, sizeof(e));
- e.ifindex = port->dev->ifindex;
- e.vid = p->addr.vid;
- __mdb_entry_fill_flags(&e, p->flags);
- if (p->addr.proto == htons(ETH_P_IP))
- e.addr.u.ip4 = p->addr.u.ip4;
+ if (!port)
+ continue;
+ memset(&e, 0, sizeof(e));
+ e.ifindex = port->dev->ifindex;
+ e.vid = p->addr.vid;
+ __mdb_entry_fill_flags(&e, p->flags);
+ if (p->addr.proto == htons(ETH_P_IP))
+ e.addr.u.ip4 = p->addr.u.ip4;
#if IS_ENABLED(CONFIG_IPV6)
- if (p->addr.proto == htons(ETH_P_IPV6))
- e.addr.u.ip6 = p->addr.u.ip6;
+ if (p->addr.proto == htons(ETH_P_IPV6))
+ e.addr.u.ip6 = p->addr.u.ip6;
#endif
- e.addr.proto = p->addr.proto;
- if (nla_put(skb, MDBA_MDB_ENTRY_INFO, sizeof(e), &e)) {
- nla_nest_cancel(skb, nest2);
- err = -EMSGSIZE;
- goto out;
- }
+ e.addr.proto = p->addr.proto;
+ if (nla_put(skb, MDBA_MDB_ENTRY_INFO, sizeof(e), &e)) {
+ nla_nest_cancel(skb, nest2);
+ err = -EMSGSIZE;
+ goto out;
}
}
nla_nest_end(skb, nest2);
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] bridge: mdb: add support for more attributes and export timer
2016-02-16 11:46 [PATCH net-next 0/2] bridge: mdb: add support for extended attributes Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 1/2] bridge: mdb: reduce the indentation level in br_mdb_fill_info Nikolay Aleksandrov
@ 2016-02-16 11:46 ` Nikolay Aleksandrov
2016-02-18 20:37 ` [PATCH net-next 0/2] bridge: mdb: add support for extended attributes David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-16 11:46 UTC (permalink / raw)
To: netdev; +Cc: roopa, stephen, Nikolay Aleksandrov
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Currently mdb entries are exported directly as a structure inside
MDBA_MDB_ENTRY_INFO attribute, we can't really extend it without
breaking user-space. In order to export new mdb fields, I've converted
the MDBA_MDB_ENTRY_INFO into a nested attribute which starts like before
with struct br_mdb_entry (without header, as it's casted directly in
iproute2) and continues with MDBA_MDB_EATTR_ attributes. This way we
keep compatibility with older users and can export new data.
I've tested this with iproute2, both with and without support for the
added attribute and it works fine.
So basically we again have MDBA_MDB_ENTRY_INFO with struct br_mdb_entry
inside but it may contain also some additional MDBA_MDB_EATTR_ attributes
such as MDBA_MDB_EATTR_TIMER which can be parsed by user-space.
So the new structure is:
[MDBA_MDB] = {
[MDBA_MDB_ENTRY] = {
[MDBA_MDB_ENTRY_INFO]
[MDBA_MDB_ENTRY_INFO] { <- Nested attribute
struct br_mdb_entry <- nla_put_nohdr()
[MDBA_MDB_ENTRY attributes] <- normal netlink attributes
}
}
}
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
include/uapi/linux/if_bridge.h | 13 ++++++++++++-
net/bridge/br_mdb.c | 16 +++++++++++++++-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index ec3547234998..0890b217580d 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -137,7 +137,10 @@ struct bridge_vlan_info {
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
- * [MDBA_MDB_ENTRY_INFO]
+ * [MDBA_MDB_ENTRY_INFO] {
+ * struct br_mdb_entry
+ * [MDBA_MDB_EATTR attributes]
+ * }
* }
* }
* [MDBA_ROUTER] = {
@@ -166,6 +169,14 @@ enum {
};
#define MDBA_MDB_ENTRY_MAX (__MDBA_MDB_ENTRY_MAX - 1)
+/* per mdb entry additional attributes */
+enum {
+ MDBA_MDB_EATTR_UNSPEC,
+ MDBA_MDB_EATTR_TIMER,
+ __MDBA_MDB_EATTR_MAX
+};
+#define MDBA_MDB_EATTR_MAX (__MDBA_MDB_EATTR_MAX - 1)
+
enum {
MDBA_ROUTER_UNSPEC,
MDBA_ROUTER_PORT,
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index e66619171386..cf51b7bcb5d5 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -88,11 +88,13 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
for (pp = &mp->ports;
(p = rcu_dereference(*pp)) != NULL;
pp = &p->next) {
+ struct nlattr *nest_ent;
struct br_mdb_entry e;
port = p->port;
if (!port)
continue;
+
memset(&e, 0, sizeof(e));
e.ifindex = port->dev->ifindex;
e.vid = p->addr.vid;
@@ -104,11 +106,23 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
e.addr.u.ip6 = p->addr.u.ip6;
#endif
e.addr.proto = p->addr.proto;
- if (nla_put(skb, MDBA_MDB_ENTRY_INFO, sizeof(e), &e)) {
+ nest_ent = nla_nest_start(skb,
+ MDBA_MDB_ENTRY_INFO);
+ if (!nest_ent) {
+ nla_nest_cancel(skb, nest2);
+ err = -EMSGSIZE;
+ goto out;
+ }
+ if (nla_put_nohdr(skb, sizeof(e), &e) ||
+ nla_put_u32(skb,
+ MDBA_MDB_EATTR_TIMER,
+ br_timer_value(&p->timer))) {
+ nla_nest_cancel(skb, nest_ent);
nla_nest_cancel(skb, nest2);
err = -EMSGSIZE;
goto out;
}
+ nla_nest_end(skb, nest_ent);
}
nla_nest_end(skb, nest2);
skip:
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] bridge: mdb: add support for extended attributes
2016-02-16 11:46 [PATCH net-next 0/2] bridge: mdb: add support for extended attributes Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 1/2] bridge: mdb: reduce the indentation level in br_mdb_fill_info Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 2/2] bridge: mdb: add support for more attributes and export timer Nikolay Aleksandrov
@ 2016-02-18 20:37 ` David Miller
2016-02-18 20:51 ` Nikolay Aleksandrov
2 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2016-02-18 20:37 UTC (permalink / raw)
To: razor; +Cc: netdev, roopa, stephen, nikolay
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: Tue, 16 Feb 2016 12:46:52 +0100
> Note that the reason we can't simply add an attribute after
> MDBA_MDB_ENTRY_INFO is that current users (e.g. iproute2) walk over
> the attribute list directly without checking for the attribute type.
Honestly that sounds like a bug in iproute2 to me...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] bridge: mdb: add support for extended attributes
2016-02-18 20:37 ` [PATCH net-next 0/2] bridge: mdb: add support for extended attributes David Miller
@ 2016-02-18 20:51 ` Nikolay Aleksandrov
2016-02-19 20:26 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2016-02-18 20:51 UTC (permalink / raw)
To: David Miller, razor; +Cc: netdev, roopa, stephen
On 02/18/2016 09:37 PM, David Miller wrote:
> From: Nikolay Aleksandrov <razor@blackwall.org>
> Date: Tue, 16 Feb 2016 12:46:52 +0100
>
>> Note that the reason we can't simply add an attribute after
>> MDBA_MDB_ENTRY_INFO is that current users (e.g. iproute2) walk over
>> the attribute list directly without checking for the attribute type.
>
> Honestly that sounds like a bug in iproute2 to me...
>
I agree, but changing this in the kernel would make older iproute2 versions
incompatible with newer kernels, possibly outputting garbage from the additional
attributes, besides we still will have to turn MDBA_MDB_ENTRY_INFO into a nested
attribute and insert the struct with a header as that's the per-mdb entry attribute
currently.
Alternatively I have a version that uses a request flag in the dump request and
sends back an alternative/"extended" version of the mdbs where every field is
a netlink attribute and is extensible, thus keeping the old format in place
and offering extended attribute support to anyone who requests it.
I just thought this version is a middle ground between the two solutions and
still doesn't break user-space while being extensible.
There're no more holes in the mdb entry struct to reuse.. :-)
Cheers,
Nik
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] bridge: mdb: add support for extended attributes
2016-02-18 20:51 ` Nikolay Aleksandrov
@ 2016-02-19 20:26 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2016-02-19 20:26 UTC (permalink / raw)
To: nikolay; +Cc: razor, netdev, roopa, stephen
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 18 Feb 2016 21:51:26 +0100
> I just thought this version is a middle ground between the two solutions and
> still doesn't break user-space while being extensible.
Ok, I'll apply this series, thanks for taking me through your thought
process.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-19 20:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-16 11:46 [PATCH net-next 0/2] bridge: mdb: add support for extended attributes Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 1/2] bridge: mdb: reduce the indentation level in br_mdb_fill_info Nikolay Aleksandrov
2016-02-16 11:46 ` [PATCH net-next 2/2] bridge: mdb: add support for more attributes and export timer Nikolay Aleksandrov
2016-02-18 20:37 ` [PATCH net-next 0/2] bridge: mdb: add support for extended attributes David Miller
2016-02-18 20:51 ` Nikolay Aleksandrov
2016-02-19 20:26 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).