Nikolay Aleksandrov
2019-Nov-01 12:38 UTC
[Bridge] [PATCH net-next 0/3] net: bridge: minor followup optimizations
Hi,
After the converted flags to bitops we can take advantage of the flags
assignment and remove one test and three atomic bitops from the learning
paths (patch 01 and patch 02), patch 03 restores the unlikely() when taking
over HW learned entries.
Thanks,
Nik
Nikolay Aleksandrov (3):
net: bridge: fdb: br_fdb_update can take flags directly
net: bridge: fdb: avoid two atomic bitops in
br_fdb_external_learn_add()
net: bridge: fdb: restore unlikely() when taking over externally added
entries
include/trace/events/bridge.h | 12 ++++++------
net/bridge/br_fdb.c | 30 +++++++++++++++---------------
net/bridge/br_input.c | 4 ++--
net/bridge/br_private.h | 2 +-
4 files changed, 24 insertions(+), 24 deletions(-)
--
2.21.0
Nikolay Aleksandrov
2019-Nov-01 12:38 UTC
[Bridge] [PATCH net-next 1/3] net: bridge: fdb: br_fdb_update can take flags directly
If we modify br_fdb_update() to take flags directly we can get rid of
one test and one atomic bitop in the learning path.
Signed-off-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
---
The trace line was over 80 chars before, now it's very close and
doesn't
look good IMO when split.
include/trace/events/bridge.h | 12 ++++++------
net/bridge/br_fdb.c | 15 ++++++---------
net/bridge/br_input.c | 4 ++--
net/bridge/br_private.h | 2 +-
4 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
index 8ea966448b58..6b200059c2c5 100644
--- a/include/trace/events/bridge.h
+++ b/include/trace/events/bridge.h
@@ -95,16 +95,16 @@ TRACE_EVENT(fdb_delete,
TRACE_EVENT(br_fdb_update,
TP_PROTO(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr, u16 vid, bool added_by_user),
+ const unsigned char *addr, u16 vid, unsigned long flags),
- TP_ARGS(br, source, addr, vid, added_by_user),
+ TP_ARGS(br, source, addr, vid, flags),
TP_STRUCT__entry(
__string(br_dev, br->dev->name)
__string(dev, source->dev->name)
__array(unsigned char, addr, ETH_ALEN)
__field(u16, vid)
- __field(bool, added_by_user)
+ __field(unsigned long, flags)
),
TP_fast_assign(
@@ -112,14 +112,14 @@ TRACE_EVENT(br_fdb_update,
__assign_str(dev, source->dev->name);
memcpy(__entry->addr, addr, ETH_ALEN);
__entry->vid = vid;
- __entry->added_by_user = added_by_user;
+ __entry->flags = flags;
),
- TP_printk("br_dev %s source %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u
added_by_user %d",
+ TP_printk("br_dev %s source %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u
flags 0x%lx",
__get_str(br_dev), __get_str(dev), __entry->addr[0],
__entry->addr[1], __entry->addr[2], __entry->addr[3],
__entry->addr[4], __entry->addr[5], __entry->vid,
- __entry->added_by_user)
+ __entry->flags)
);
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index f244f2ac7156..b37e0f4c1b2b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -557,7 +557,7 @@ int br_fdb_insert(struct net_bridge *br, struct
net_bridge_port *source,
}
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr, u16 vid, bool added_by_user)
+ const unsigned char *addr, u16 vid, unsigned long flags)
{
struct net_bridge_fdb_entry *fdb;
bool fdb_modified = false;
@@ -592,21 +592,18 @@ void br_fdb_update(struct net_bridge *br, struct
net_bridge_port *source,
}
if (now != fdb->updated)
fdb->updated = now;
- if (unlikely(added_by_user))
+ if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags)))
set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
if (unlikely(fdb_modified)) {
- trace_br_fdb_update(br, source, addr, vid, added_by_user);
+ trace_br_fdb_update(br, source, addr, vid, flags);
fdb_notify(br, fdb, RTM_NEWNEIGH, true);
}
}
} else {
spin_lock(&br->hash_lock);
- fdb = fdb_create(br, source, addr, vid, 0);
+ fdb = fdb_create(br, source, addr, vid, flags);
if (fdb) {
- if (unlikely(added_by_user))
- set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
- trace_br_fdb_update(br, source, addr, vid,
- added_by_user);
+ trace_br_fdb_update(br, source, addr, vid, flags);
fdb_notify(br, fdb, RTM_NEWNEIGH, true);
}
/* else we lose race and someone else inserts
@@ -889,7 +886,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge
*br,
}
local_bh_disable();
rcu_read_lock();
- br_fdb_update(br, p, addr, vid, true);
+ br_fdb_update(br, p, addr, vid, BIT(BR_FDB_ADDED_BY_USER));
rcu_read_unlock();
local_bh_enable();
} else if (ndm->ndm_flags & NTF_EXT_LEARNED) {
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 7f5f646dba6e..f37b05090f45 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -88,7 +88,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk,
struct sk_buff *skb
/* insert into forwarding database after filtering to avoid spoofing */
br = p->br;
if (p->flags & BR_LEARNING)
- br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, false);
+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
local_rcv = !!(br->dev->flags & IFF_PROMISC);
if (is_multicast_ether_addr(eth_hdr(skb)->h_dest)) {
@@ -184,7 +184,7 @@ static void __br_handle_local_finish(struct sk_buff *skb)
if ((p->flags & BR_LEARNING) &&
!br_opt_get(p->br, BROPT_NO_LL_LEARN) &&
br_should_learn(p, skb, &vid))
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, false);
+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, 0);
}
/* note: already called with rcu_read_lock */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f4754bf7f4bd..08742bff9bf0 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -571,7 +571,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
unsigned long count,
int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid);
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr, u16 vid, bool added_by_user);
+ const unsigned char *addr, u16 vid, unsigned long flags);
int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev, const unsigned char *addr, u16 vid);
--
2.21.0
Nikolay Aleksandrov
2019-Nov-01 12:38 UTC
[Bridge] [PATCH net-next 2/3] net: bridge: fdb: avoid one atomic bitop in br_fdb_external_learn_add()
If we pass the BR_FDB_ADDED_BY_EXT_LEARN flag directly to fdb_create()
we can avoid one unconditional atomic bitop when learning a new entry.
Signed-off-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
---
net/bridge/br_fdb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index b37e0f4c1b2b..7db09410679b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1113,14 +1113,14 @@ int br_fdb_external_learn_add(struct net_bridge *br,
struct net_bridge_port *p,
fdb = br_fdb_find(br, addr, vid);
if (!fdb) {
- fdb = fdb_create(br, p, addr, vid, 0);
+ fdb = fdb_create(br, p, addr, vid,
+ BIT(BR_FDB_ADDED_BY_EXT_LEARN));
if (!fdb) {
err = -ENOMEM;
goto err_unlock;
}
if (swdev_notify)
set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
- set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags);
fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify);
} else {
fdb->updated = jiffies;
--
2.21.0
Nikolay Aleksandrov
2019-Nov-01 12:38 UTC
[Bridge] [PATCH net-next 2/3] net: bridge: fdb: avoid two atomic bitops in br_fdb_external_learn_add()
If we setup the fdb flags prior to calling fdb_create() we can avoid
two atomic bitops when learning a new entry.
Signed-off-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
---
net/bridge/br_fdb.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index b37e0f4c1b2b..7500c84fc675 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1113,14 +1113,15 @@ int br_fdb_external_learn_add(struct net_bridge *br,
struct net_bridge_port *p,
fdb = br_fdb_find(br, addr, vid);
if (!fdb) {
- fdb = fdb_create(br, p, addr, vid, 0);
+ unsigned long flags = BIT(BR_FDB_ADDED_BY_EXT_LEARN);
+
+ if (swdev_notify)
+ flags |= BIT(BR_FDB_ADDED_BY_USER);
+ fdb = fdb_create(br, p, addr, vid, flags);
if (!fdb) {
err = -ENOMEM;
goto err_unlock;
}
- if (swdev_notify)
- set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
- set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags);
fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify);
} else {
fdb->updated = jiffies;
--
2.21.0
Nikolay Aleksandrov
2019-Nov-01 12:38 UTC
[Bridge] [PATCH net-next 3/3] net: bridge: fdb: restore unlikely() when taking over externally added entries
Taking over hw-learned entries is not a likely scenario so restore the unlikely() use for the case of SW taking over externally learned entries. Signed-off-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com> --- net/bridge/br_fdb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index 7500c84fc675..284b3662d234 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -587,8 +587,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, fdb->dst = source; fdb_modified = true; /* Take over HW learned entry */ - test_and_clear_bit(BR_FDB_ADDED_BY_EXT_LEARN, - &fdb->flags); + if (unlikely(test_bit(BR_FDB_ADDED_BY_EXT_LEARN, + &fdb->flags))) + clear_bit(BR_FDB_ADDED_BY_EXT_LEARN, + &fdb->flags); } if (now != fdb->updated) fdb->updated = now; -- 2.21.0
Nikolay Aleksandrov
2019-Nov-01 12:40 UTC
[Bridge] [PATCH net-next 0/3] net: bridge: minor followup optimizations
On 01/11/2019 14:38, Nikolay Aleksandrov wrote:> Hi, > After the converted flags to bitops we can take advantage of the flags > assignment and remove one test and three atomic bitops from the learning > paths (patch 01 and patch 02), patch 03 restores the unlikely() when taking > over HW learned entries. > > Thanks, > Nik > > > Nikolay Aleksandrov (3): > net: bridge: fdb: br_fdb_update can take flags directly > net: bridge: fdb: avoid two atomic bitops in > br_fdb_external_learn_add() > net: bridge: fdb: restore unlikely() when taking over externally added > entries > > include/trace/events/bridge.h | 12 ++++++------ > net/bridge/br_fdb.c | 30 +++++++++++++++--------------- > net/bridge/br_input.c | 4 ++-- > net/bridge/br_private.h | 2 +- > 4 files changed, 24 insertions(+), 24 deletions(-) >Aaargh.. apologies for the noise, the script caught an older set export. Will re-post a proper v2 in a minute.