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); For reference: 458 static int 459 vlan_remhash(struct ifvlantrunk *trunk, struct ifvlan *ifv) 460 { 461 int i, b; 462 struct ifvlan *ifv2; 463 464 VLAN_XLOCK_ASSERT(); 465 KASSERT(trunk->hwidth > 0, ("%s: hwidth not positive", __func__)); 466 467 b = 1 << trunk->hwidth; 468 i = HASH(ifv->ifv_vid, trunk->hmask); 469 CK_SLIST_FOREACH(ifv2, &trunk->hash[i], ifv_list) 470 if (ifv2 == ifv) { 471 trunk->refcnt--; 472 CK_SLIST_REMOVE(&trunk->hash[i], ifv2, ifvlan, ifv_list); 473 if (trunk->refcnt < (b * b) / 2) 474 vlan_growhash(trunk, -1); 475 return (0); 476 } 477 478 panic("%s: vlan not found\n", __func__); 479 return (ENOENT); /*NOTREACHED*/ 480 }
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=151abc80cde778bc18b91c334d07fbd52bbb38fb commit 151abc80cde778bc18b91c334d07fbd52bbb38fb Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2022-07-22 17:17:04 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2022-07-22 17:18:41 +0000 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. sys/net/if_vlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a5f19abeb7191cb3cd89f06ff0eb150b89d6b8bf commit a5f19abeb7191cb3cd89f06ff0eb150b89d6b8bf Author: David Sips <dsips@netapp.com> AuthorDate: 2022-07-22 17:17:04 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2022-08-05 11:54:32 +0000 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. (cherry picked from commit 151abc80cde778bc18b91c334d07fbd52bbb38fb) sys/net/if_vlan.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6c6e7cbe47b0d7200947ebf0d57570fa6f771ec2 commit 6c6e7cbe47b0d7200947ebf0d57570fa6f771ec2 Author: David Sips <dsips@netapp.com> AuthorDate: 2022-07-22 17:17:04 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2022-08-05 11:54:25 +0000 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. (cherry picked from commit 151abc80cde778bc18b91c334d07fbd52bbb38fb) sys/net/if_vlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)