if_vlan: avoid hash table thrashing when adding and removing entries

vlan_remhash() uses incorrect value for b.

When using the default value for VLAN_DEF_HWIDTH (4), the VLAN hash-list table
expands from 16 chains to 32 chains as the 129th entry is added. trunk->hwidth
becomes 5. Say a few more entries are added and there are now 135 entries.
trunk-hwidth will still be 5. If an entry is removed, vlan_remhash() will
calculate a value of 32 for b. refcnt will be decremented to 134. The if
comparison at line 473 will return true and vlan_growhash() will be called. The
VLAN hash-list table will be compressed from 32 chains wide to 16 chains wide.
hwidth will become 4. This is an error, and it can be seen when a new VLAN is
added. The table will again be expanded. If an entry is then removed, again
the table is contracted.

If the number of VLANS stays in the range of 128-512, each time an insert
follows a remove, the table will expand. Each time a remove follows an
insert, the table will be contracted.

The fix is simple. The line 473 should test that the number of entries has
decreased such that the table should be contracted using what would be the new
value of hwidth. line 467 should be:

	b = 1 << (trunk->hwidth - 1);

PR:		265382
Reviewed by:	kp
MFC after:	2 weeks
Sponsored by:	NetApp, Inc.
This commit is contained in:
Kristof Provost 2022-07-22 19:17:04 +02:00
parent bb7ddd077c
commit 151abc80cd

View File

@ -464,7 +464,7 @@ vlan_remhash(struct ifvlantrunk *trunk, struct ifvlan *ifv)
VLAN_XLOCK_ASSERT();
KASSERT(trunk->hwidth > 0, ("%s: hwidth not positive", __func__));
b = 1 << trunk->hwidth;
b = 1 << (trunk->hwidth - 1);
i = HASH(ifv->ifv_vid, trunk->hmask);
CK_SLIST_FOREACH(ifv2, &trunk->hash[i], ifv_list)
if (ifv2 == ifv) {