Skip to content

Commit 094adad

Browse files
idoschPaolo Abeni
idosch
authored and
Paolo Abeni
committed
vxlan: Use a single lock to protect the FDB table
Currently, the VXLAN driver stores FDB entries in a hash table with a fixed number of buckets (256). Subsequent patches are going to convert this table to rhashtable with a linked list for entry traversal, as rhashtable is more scalable. In preparation for this conversion, move from a per-bucket spin lock to a single spin lock that protects the entire FDB table. The per-bucket spin locks were introduced by commit fe1e071 ("vxlan: Use FDB_HASH_SIZE hash_locks to reduce contention") citing "huge contention when inserting/deleting vxlan_fdbs into the fdb_head". It is not clear from the commit message which code path was holding the spin lock for long periods of time, but the obvious suspect is the FDB cleanup routine (vxlan_cleanup()) that periodically traverses the entire table in order to delete aged-out entries. This will be solved by subsequent patches that will convert the FDB cleanup routine to traverse the linked list of FDB entries using RCU, only acquiring the spin lock when deleting an aged-out entry. The change reduces the size of the VXLAN device structure from 3600 bytes to 2576 bytes. Reviewed-by: Petr Machata <[email protected]> Signed-off-by: Ido Schimmel <[email protected]> Link: https://patch.msgid.link/[email protected] Reviewed-by: Nikolay Aleksandrov <[email protected]> Signed-off-by: Paolo Abeni <[email protected]>
1 parent 6ba480c commit 094adad

File tree

3 files changed

+34
-58
lines changed

3 files changed

+34
-58
lines changed

‎drivers/net/vxlan/vxlan_core.c‎

Lines changed: 30 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@ int vxlan_fdb_replay(const struct net_device *dev, __be32 vni,
524524
return -EINVAL;
525525
vxlan = netdev_priv(dev);
526526

527+
spin_lock_bh(&vxlan->hash_lock);
527528
for (h = 0; h < FDB_HASH_SIZE; ++h) {
528-
spin_lock_bh(&vxlan->hash_lock[h]);
529529
hlist_for_each_entry(f, &vxlan->fdb_head[h], hlist) {
530530
if (f->vni == vni) {
531531
list_for_each_entry(rdst, &f->remotes, list) {
@@ -537,12 +537,12 @@ int vxlan_fdb_replay(const struct net_device *dev, __be32 vni,
537537
}
538538
}
539539
}
540-
spin_unlock_bh(&vxlan->hash_lock[h]);
541540
}
541+
spin_unlock_bh(&vxlan->hash_lock);
542542
return 0;
543543

544544
unlock:
545-
spin_unlock_bh(&vxlan->hash_lock[h]);
545+
spin_unlock_bh(&vxlan->hash_lock);
546546
return rc;
547547
}
548548
EXPORT_SYMBOL_GPL(vxlan_fdb_replay);
@@ -558,14 +558,14 @@ void vxlan_fdb_clear_offload(const struct net_device *dev, __be32 vni)
558558
return;
559559
vxlan = netdev_priv(dev);
560560

561+
spin_lock_bh(&vxlan->hash_lock);
561562
for (h = 0; h < FDB_HASH_SIZE; ++h) {
562-
spin_lock_bh(&vxlan->hash_lock[h]);
563563
hlist_for_each_entry(f, &vxlan->fdb_head[h], hlist)
564564
if (f->vni == vni)
565565
list_for_each_entry(rdst, &f->remotes, list)
566566
rdst->offloaded = false;
567-
spin_unlock_bh(&vxlan->hash_lock[h]);
568567
}
568+
spin_unlock_bh(&vxlan->hash_lock);
569569

570570
}
571571
EXPORT_SYMBOL_GPL(vxlan_fdb_clear_offload);
@@ -1248,7 +1248,6 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
12481248
__be16 port;
12491249
__be32 src_vni, vni;
12501250
u32 ifindex, nhid;
1251-
u32 hash_index;
12521251
int err;
12531252

12541253
if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_REACHABLE))) {
@@ -1268,13 +1267,12 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
12681267
if (vxlan->default_dst.remote_ip.sa.sa_family != ip.sa.sa_family)
12691268
return -EAFNOSUPPORT;
12701269

1271-
hash_index = fdb_head_index(vxlan, addr, src_vni);
1272-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
1270+
spin_lock_bh(&vxlan->hash_lock);
12731271
err = vxlan_fdb_update(vxlan, addr, &ip, ndm->ndm_state, flags,
12741272
port, src_vni, vni, ifindex,
12751273
ndm->ndm_flags | NTF_VXLAN_ADDED_BY_USER,
12761274
nhid, true, extack);
1277-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
1275+
spin_unlock_bh(&vxlan->hash_lock);
12781276

12791277
if (!err)
12801278
*notified = true;
@@ -1325,7 +1323,6 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
13251323
union vxlan_addr ip;
13261324
__be32 src_vni, vni;
13271325
u32 ifindex, nhid;
1328-
u32 hash_index;
13291326
__be16 port;
13301327
int err;
13311328

@@ -1334,11 +1331,10 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
13341331
if (err)
13351332
return err;
13361333

1337-
hash_index = fdb_head_index(vxlan, addr, src_vni);
1338-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
1334+
spin_lock_bh(&vxlan->hash_lock);
13391335
err = __vxlan_fdb_delete(vxlan, addr, ip, port, src_vni, vni, ifindex,
13401336
true);
1341-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
1337+
spin_unlock_bh(&vxlan->hash_lock);
13421338

13431339
if (!err)
13441340
*notified = true;
@@ -1486,10 +1482,8 @@ static enum skb_drop_reason vxlan_snoop(struct net_device *dev,
14861482
rdst->remote_ip = *src_ip;
14871483
vxlan_fdb_notify(vxlan, f, rdst, RTM_NEWNEIGH, true, NULL);
14881484
} else {
1489-
u32 hash_index = fdb_head_index(vxlan, src_mac, vni);
1490-
14911485
/* learned new entry */
1492-
spin_lock(&vxlan->hash_lock[hash_index]);
1486+
spin_lock(&vxlan->hash_lock);
14931487

14941488
/* close off race between vxlan_flush and incoming packets */
14951489
if (netif_running(dev))
@@ -1500,7 +1494,7 @@ static enum skb_drop_reason vxlan_snoop(struct net_device *dev,
15001494
vni,
15011495
vxlan->default_dst.remote_vni,
15021496
ifindex, NTF_SELF, 0, true, NULL);
1503-
spin_unlock(&vxlan->hash_lock[hash_index]);
1497+
spin_unlock(&vxlan->hash_lock);
15041498
}
15051499

15061500
return SKB_NOT_DROPPED_YET;
@@ -2839,10 +2833,10 @@ static void vxlan_cleanup(struct timer_list *t)
28392833
if (!netif_running(vxlan->dev))
28402834
return;
28412835

2836+
spin_lock(&vxlan->hash_lock);
28422837
for (h = 0; h < FDB_HASH_SIZE; ++h) {
28432838
struct hlist_node *p, *n;
28442839

2845-
spin_lock(&vxlan->hash_lock[h]);
28462840
hlist_for_each_safe(p, n, &vxlan->fdb_head[h]) {
28472841
struct vxlan_fdb *f
28482842
= container_of(p, struct vxlan_fdb, hlist);
@@ -2864,8 +2858,8 @@ static void vxlan_cleanup(struct timer_list *t)
28642858
} else if (time_before(timeout, next_timer))
28652859
next_timer = timeout;
28662860
}
2867-
spin_unlock(&vxlan->hash_lock[h]);
28682861
}
2862+
spin_unlock(&vxlan->hash_lock);
28692863

28702864
mod_timer(&vxlan->age_timer, next_timer);
28712865
}
@@ -3056,10 +3050,10 @@ static void vxlan_flush(struct vxlan_dev *vxlan,
30563050
bool match_remotes = vxlan_fdb_flush_should_match_remotes(desc);
30573051
unsigned int h;
30583052

3053+
spin_lock_bh(&vxlan->hash_lock);
30593054
for (h = 0; h < FDB_HASH_SIZE; ++h) {
30603055
struct hlist_node *p, *n;
30613056

3062-
spin_lock_bh(&vxlan->hash_lock[h]);
30633057
hlist_for_each_safe(p, n, &vxlan->fdb_head[h]) {
30643058
struct vxlan_fdb *f
30653059
= container_of(p, struct vxlan_fdb, hlist);
@@ -3079,8 +3073,8 @@ static void vxlan_flush(struct vxlan_dev *vxlan,
30793073

30803074
vxlan_fdb_destroy(vxlan, f, true, true);
30813075
}
3082-
spin_unlock_bh(&vxlan->hash_lock[h]);
30833076
}
3077+
spin_unlock_bh(&vxlan->hash_lock);
30843078
}
30853079

30863080
static const struct nla_policy vxlan_del_bulk_policy[NDA_MAX + 1] = {
@@ -3358,15 +3352,14 @@ static void vxlan_setup(struct net_device *dev)
33583352

33593353
dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
33603354
INIT_LIST_HEAD(&vxlan->next);
3355+
spin_lock_init(&vxlan->hash_lock);
33613356

33623357
timer_setup(&vxlan->age_timer, vxlan_cleanup, TIMER_DEFERRABLE);
33633358

33643359
vxlan->dev = dev;
33653360

3366-
for (h = 0; h < FDB_HASH_SIZE; ++h) {
3367-
spin_lock_init(&vxlan->hash_lock[h]);
3361+
for (h = 0; h < FDB_HASH_SIZE; ++h)
33683362
INIT_HLIST_HEAD(&vxlan->fdb_head[h]);
3369-
}
33703363
}
33713364

33723365
static void vxlan_ether_setup(struct net_device *dev)
@@ -3977,10 +3970,7 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
39773970

39783971
/* create an fdb entry for a valid default destination */
39793972
if (!vxlan_addr_any(&dst->remote_ip)) {
3980-
u32 hash_index = fdb_head_index(vxlan, all_zeros_mac,
3981-
dst->remote_vni);
3982-
3983-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
3973+
spin_lock_bh(&vxlan->hash_lock);
39843974
err = vxlan_fdb_update(vxlan, all_zeros_mac,
39853975
&dst->remote_ip,
39863976
NUD_REACHABLE | NUD_PERMANENT,
@@ -3990,7 +3980,7 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
39903980
dst->remote_vni,
39913981
dst->remote_ifindex,
39923982
NTF_SELF, 0, true, extack);
3993-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
3983+
spin_unlock_bh(&vxlan->hash_lock);
39943984
if (err)
39953985
goto unlink;
39963986
}
@@ -4420,9 +4410,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
44204410

44214411
/* handle default dst entry */
44224412
if (rem_ip_changed) {
4423-
u32 hash_index = fdb_head_index(vxlan, all_zeros_mac, conf.vni);
4424-
4425-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
4413+
spin_lock_bh(&vxlan->hash_lock);
44264414
if (!vxlan_addr_any(&conf.remote_ip)) {
44274415
err = vxlan_fdb_update(vxlan, all_zeros_mac,
44284416
&conf.remote_ip,
@@ -4433,7 +4421,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
44334421
conf.remote_ifindex,
44344422
NTF_SELF, 0, true, extack);
44354423
if (err) {
4436-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
4424+
spin_unlock_bh(&vxlan->hash_lock);
44374425
netdev_adjacent_change_abort(dst->remote_dev,
44384426
lowerdev, dev);
44394427
return err;
@@ -4447,7 +4435,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
44474435
dst->remote_vni,
44484436
dst->remote_ifindex,
44494437
true);
4450-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
4438+
spin_unlock_bh(&vxlan->hash_lock);
44514439

44524440
/* If vni filtering device, also update fdb entries of
44534441
* all vnis that were using default remote ip
@@ -4747,11 +4735,8 @@ vxlan_fdb_offloaded_set(struct net_device *dev,
47474735
struct vxlan_dev *vxlan = netdev_priv(dev);
47484736
struct vxlan_rdst *rdst;
47494737
struct vxlan_fdb *f;
4750-
u32 hash_index;
4751-
4752-
hash_index = fdb_head_index(vxlan, fdb_info->eth_addr, fdb_info->vni);
47534738

4754-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
4739+
spin_lock_bh(&vxlan->hash_lock);
47554740

47564741
f = __vxlan_find_mac(vxlan, fdb_info->eth_addr, fdb_info->vni);
47574742
if (!f)
@@ -4767,7 +4752,7 @@ vxlan_fdb_offloaded_set(struct net_device *dev,
47674752
rdst->offloaded = fdb_info->offloaded;
47684753

47694754
out:
4770-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
4755+
spin_unlock_bh(&vxlan->hash_lock);
47714756
}
47724757

47734758
static int
@@ -4776,13 +4761,11 @@ vxlan_fdb_external_learn_add(struct net_device *dev,
47764761
{
47774762
struct vxlan_dev *vxlan = netdev_priv(dev);
47784763
struct netlink_ext_ack *extack;
4779-
u32 hash_index;
47804764
int err;
47814765

4782-
hash_index = fdb_head_index(vxlan, fdb_info->eth_addr, fdb_info->vni);
47834766
extack = switchdev_notifier_info_to_extack(&fdb_info->info);
47844767

4785-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
4768+
spin_lock_bh(&vxlan->hash_lock);
47864769
err = vxlan_fdb_update(vxlan, fdb_info->eth_addr, &fdb_info->remote_ip,
47874770
NUD_REACHABLE,
47884771
NLM_F_CREATE | NLM_F_REPLACE,
@@ -4792,7 +4775,7 @@ vxlan_fdb_external_learn_add(struct net_device *dev,
47924775
fdb_info->remote_ifindex,
47934776
NTF_USE | NTF_SELF | NTF_EXT_LEARNED,
47944777
0, false, extack);
4795-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
4778+
spin_unlock_bh(&vxlan->hash_lock);
47964779

47974780
return err;
47984781
}
@@ -4803,11 +4786,9 @@ vxlan_fdb_external_learn_del(struct net_device *dev,
48034786
{
48044787
struct vxlan_dev *vxlan = netdev_priv(dev);
48054788
struct vxlan_fdb *f;
4806-
u32 hash_index;
48074789
int err = 0;
48084790

4809-
hash_index = fdb_head_index(vxlan, fdb_info->eth_addr, fdb_info->vni);
4810-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
4791+
spin_lock_bh(&vxlan->hash_lock);
48114792

48124793
f = __vxlan_find_mac(vxlan, fdb_info->eth_addr, fdb_info->vni);
48134794
if (!f)
@@ -4821,7 +4802,7 @@ vxlan_fdb_external_learn_del(struct net_device *dev,
48214802
fdb_info->remote_ifindex,
48224803
false);
48234804

4824-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
4805+
spin_unlock_bh(&vxlan->hash_lock);
48254806

48264807
return err;
48274808
}
@@ -4870,18 +4851,15 @@ static void vxlan_fdb_nh_flush(struct nexthop *nh)
48704851
{
48714852
struct vxlan_fdb *fdb;
48724853
struct vxlan_dev *vxlan;
4873-
u32 hash_index;
48744854

48754855
rcu_read_lock();
48764856
list_for_each_entry_rcu(fdb, &nh->fdb_list, nh_list) {
48774857
vxlan = rcu_dereference(fdb->vdev);
48784858
WARN_ON(!vxlan);
4879-
hash_index = fdb_head_index(vxlan, fdb->eth_addr,
4880-
vxlan->default_dst.remote_vni);
4881-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
4859+
spin_lock_bh(&vxlan->hash_lock);
48824860
if (!hlist_unhashed(&fdb->hlist))
48834861
vxlan_fdb_destroy(vxlan, fdb, false, false);
4884-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
4862+
spin_unlock_bh(&vxlan->hash_lock);
48854863
}
48864864
rcu_read_unlock();
48874865
}

‎drivers/net/vxlan/vxlan_vnifilter.c‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -482,11 +482,9 @@ static int vxlan_update_default_fdb_entry(struct vxlan_dev *vxlan, __be32 vni,
482482
struct netlink_ext_ack *extack)
483483
{
484484
struct vxlan_rdst *dst = &vxlan->default_dst;
485-
u32 hash_index;
486485
int err = 0;
487486

488-
hash_index = fdb_head_index(vxlan, all_zeros_mac, vni);
489-
spin_lock_bh(&vxlan->hash_lock[hash_index]);
487+
spin_lock_bh(&vxlan->hash_lock);
490488
if (remote_ip && !vxlan_addr_any(remote_ip)) {
491489
err = vxlan_fdb_update(vxlan, all_zeros_mac,
492490
remote_ip,
@@ -498,7 +496,7 @@ static int vxlan_update_default_fdb_entry(struct vxlan_dev *vxlan, __be32 vni,
498496
dst->remote_ifindex,
499497
NTF_SELF, 0, true, extack);
500498
if (err) {
501-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
499+
spin_unlock_bh(&vxlan->hash_lock);
502500
return err;
503501
}
504502
}
@@ -511,7 +509,7 @@ static int vxlan_update_default_fdb_entry(struct vxlan_dev *vxlan, __be32 vni,
511509
dst->remote_ifindex,
512510
true);
513511
}
514-
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
512+
spin_unlock_bh(&vxlan->hash_lock);
515513

516514
return err;
517515
}

‎include/net/vxlan.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ struct vxlan_dev {
296296
struct vxlan_rdst default_dst; /* default destination */
297297

298298
struct timer_list age_timer;
299-
spinlock_t hash_lock[FDB_HASH_SIZE];
299+
spinlock_t hash_lock;
300300
unsigned int addrcnt;
301301
struct gro_cells gro_cells;
302302

0 commit comments

Comments
 (0)