[Bug 265382] VLAN hash-list table thrashing when adding and removing entries

From: <bugzilla-noreply_at_freebsd.org>
Date: Fri, 22 Jul 2022 14:29:52 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=265382

            Bug ID: 265382
           Summary: VLAN hash-list table thrashing when adding and
                    removing entries
           Product: Base System
           Version: Unspecified
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: dsips@netapp.com
                CC: dsips@netapp.com

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 }

-- 
You are receiving this mail because:
You are the assignee for the bug.