Page MenuHomeFreeBSD

ifnet: make if_index global
ClosedPublic

Authored by glebius on Dec 27 2021, 9:32 PM.
Tags
None
Referenced Files
F101992951: D33672.diff
Wed, Nov 6, 7:25 AM
Unknown Object (File)
Sun, Oct 27, 5:49 AM
Unknown Object (File)
Sun, Oct 27, 12:34 AM
Unknown Object (File)
Tue, Oct 22, 11:58 PM
Unknown Object (File)
Sun, Oct 20, 5:03 PM
Unknown Object (File)
Sat, Oct 19, 10:39 PM
Unknown Object (File)
Sat, Oct 19, 7:44 AM
Unknown Object (File)
Tue, Oct 8, 12:45 PM
Subscribers

Details

Summary

Now that ifindex is static to if.c we can unvirtualize it. For lifetime
of an ifnet its index never changes. To avoid leaking foreign interfaces
the net.link.generic.ifcount sysctl and the ifnet_byindex() KPI filter
their returned value on curvnet. Since if_vmove() no longer changes the
if_index, inline ifindex_alloc() and ifindex_free() into if_alloc() and
if_free() respectively.

API wise the only change is that now minimum interface index can be
greater than 1. The holes in interface indexes were always allowed.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 43629
Build 40517: arc lint + arc unit

Event Timeline

This is a replacement for https://reviews.freebsd.org/D33265 I will try to replant its dependencies on top of that one, if phab allows that.

sys/net/if.c
375

Doesn't this need an ifp != NULL check?

Address Kristof's comment. Thanks!

This revision is now accepted and ready to land.Dec 28 2021, 5:45 PM

Silly question time: what is the maximum number of interfaces we can have on a system if the ifIndex will be global? What's its size on the RFCs?

I've also just scrolled through https://datatracker.ietf.org/doc/html/rfc2233#section-3.1.5 and its history and future.

One possible problem I do remain seeing with keeping a persistent (global) interface index between networks stacks is that we actually start leaking information back into the vnet about the overall host. There's probably a lot more about that still but we should rather be removing them than adding them.

I am still not convinced this is the better solution (probably it is the simpler solution) and I do understand that we are between Holidays and Years in the western world and that Nativity is coming up in the East and we may just have a lonely discussion here with people being away.

I wonder if the perforce commit email archives have the reason for why ifIndex was virtualized. I wonder if these email archives still exist after the late mailing list move or if we have to dig in personal mailboxes.

In D33672#761532, @bz wrote:

Silly question time: what is the maximum number of interfaces we can have on a system if the ifIndex will be global? What's its size on the RFCs?
I've also just scrolled through https://datatracker.ietf.org/doc/html/rfc2233#section-3.1.5 and its history and future.

We use 16-bit for index and MIB II defines it as 32 bit value. However, the ifindex reported in SNMP != system ifindex. The rfc2233#section-3.1.5 requires uniqueness of the value over time, and system ifindex can be reused. This is the reason why bsnmpd doesn't use system ifindex. So, to put it short. We are not bound by MIB II at all on the implementation of ifindex.

What keeps us sticking to 16 bit value is struct ifreq.

This revision now requires review to proceed.Jan 25 2022, 5:12 AM
This revision is now accepted and ready to land.Jan 25 2022, 1:35 PM
This revision was automatically updated to reflect the committed changes.