Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 5 Aug 2022 11:55:57 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: a5f19abeb719 - stable/12 - if_vlan: avoid hash table thrashing when adding and removing entries
Message-ID:  <202208051155.275BtvwE047634@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by kp:

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(-)

diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c
index ae1e05e8bc93..491c1a4fa371 100644
--- a/sys/net/if_vlan.c
+++ b/sys/net/if_vlan.c
@@ -408,7 +408,8 @@ 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) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202208051155.275BtvwE047634>