Re: git: ad2a0aec2954 - main - nhop: hash ifnet pointer instead of if_index

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