Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 4 Dec 2021 11:04:35 -0800
From:      Gleb Smirnoff <glebius@freebsd.org>
To:        "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net>
Cc:        src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: ad2a0aec2954 - main - nhop: hash ifnet pointer instead of if_index
Message-ID:  <Yau7w%2BOLSTt7%2B0YO@FreeBSD.org>
In-Reply-To: <alpine.BSF.2.00.2112041814540.68830@ai.fobar.qr>
References:  <202112041806.1B4I6HGV058876@gitrepo.freebsd.org> <alpine.BSF.2.00.2112041814540.68830@ai.fobar.qr>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Dec 04, 2021 at 06:16:42PM +0000, Bjoern A. Zeeb wrote:
B> > commit ad2a0aec295478e750158b8985422f15deee0e54
B> > Author:     Gleb Smirnoff <glebius@FreeBSD.org>
B> > AuthorDate: 2021-12-04 18:05:46 +0000
B> > Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
B> > CommitDate: 2021-12-04 18:05:46 +0000
B> >
B> >    nhop: hash ifnet pointer instead of if_index
B> >
B> >    Yet another problem created by VIMAGE/if_vmove/epair design that
B> >    relocates ifnet between vnets and changes if_index.  Since if_index
B> >    changes, nhop hash values also changes, unlink_nhop() isn't able to
B> >    find entry in hash and leaks the nhop.  Since nhop references ifnet,
B> >    the latter is also leaked.  As result running network tests leaks
B> >    memory on every single test that creates vnet jail.
B> 
B> That sounds like something (new) is done in wrong sequence for these
B> cases.  Plastering around that sounds wrong as it simply hides the
B> real problem.

There is nothing new here. if_vmove() make if_index field non-immutable
and that creates a list of problems. For this particular case using pointer
isn't a plaster, even a microoptimisation - one less dereference. But for
other problems, something different needs to be done.

I have WIP that maintains two indexes - a virtualized and global one:

https://github.com/glebius/FreeBSD/commits/ifindex

Other approach I am considering - keep only the global one and use
a function if_index(ifp) to calculate sequential number of given
ifnet within its vnet, so that there are no holes. That's what
userland API expects.

-- 
Gleb Smirnoff



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Yau7w%2BOLSTt7%2B0YO>