if: store original ifname

[melifaro]

Summary

Interface renaming is "hard".

"Clonable" interfaces can be created either by providing an explicit name, or just providing a type, with the kernel-provided name auto-assignment.
The current code tries to support the efficient performance of the second case. It works pretty good when the interface names are stable. When the interfaces are renamed, a number of problems arise.

For example, there are all sorts of spurious errors when trying to create a seemingly-non-existent interface, when such interface was created and then renamed.
Some of the APIs we provide (/dev/ or linux sysfs interface names) do not account for renames.

To (somewhat) aid the problem, I'm proposing to provide an explicit method to get an original interface name in the kernel and export it to userland via Netlink.
This diff uses the simplest possible implementation - by embedding original interface name into the struct ifnet (e.g. IF_NAMESIZE == 16 bytes). It is mergeable to 13-S, as there are exactly 16 spare bytes available in the end of the structure.

[kp]

sys/net/if.c

4299

It's const if_t ifp in the prototype but not here.

sys/net/if_private.h

187

I'd stick it right after if_xname, but that does make a merge to 13 a bit harder.

[melifaro]

sys/net/if.c

4299

Whoops, good catch, I'll update the review.

sys/net/if_private.h

187

I understand and agree with the readability argument here.
I'd still prefer to leave it as is for performance reasons. Of course, right now struct ifnet is in chaotic state w.r.t. performance-optimized layout, that should change soon once we adopt more of ifapi inside the stack. Still, some important fields that are relevant for per-packet processing (including if_xnameare located within first 2 cache lines). Adding 16 bytes of data that's used only in control-plane code would probably negatively impact the packet processing performance.

To rephrase: I'd leave it as is for now, once we proceed further with ifapi, we can split the structure into the dataplane-part and control-part, grouping the control part fields more logically.

What do you think?

[kp]

sys/net/if_private.h

187

Yeah, that makes sense.

[zlei]

@kp
Do you think it is the right time to consider https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=185619 as well ?

I'd propose creating a stable host global unique name, maybe UUID, as the original name for the interface. It is unchangeable across the interface's lifecycle.

[melifaro]

In D39689#903283, @zlei wrote:

@kp
Do you think it is the right time to consider https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=185619 as well ?

I'd propose creating a stable host global unique name, maybe UUID, as the original name for the interface. It is unchangeable across the interface's lifecycle.

I'd say it's worth agreeing on what problem is being solved. Or, alternatively, how do we _want_ interfaces to behave with renaming / vnet-moving. After that it would be easier to approach to (any) solution.
Without it, strictly speaking, we can say that we already have the proposed UUID - that's the interface pointer, as it both host-unique and unchangeable.

[zlei]

In D39689#903359, @melifaro wrote:

In D39689#903283, @zlei wrote:

@kp
Do you think it is the right time to consider https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=185619 as well ?

I'd propose creating a stable host global unique name, maybe UUID, as the original name for the interface. It is unchangeable across the interface's lifecycle.

I'd say it's worth agreeing on what problem is being solved. Or, alternatively, how do we _want_ interfaces to behave with renaming / vnet-moving. After that it would be easier to approach to (any) solution.
Without it, strictly speaking, we can say that we already have the proposed UUID - that's the interface pointer, as it both host-unique and unchangeable.

The problem is that the pointers are not exposed to userspace.

[zlei]

And for physical interfaces, the UUIDs can derivate from MAC addresses so their's names are stable cross reboot / PCI slot swap .

I have to admit that we still need extra efforts to get a general approach to fix https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=118111 .
Maybe at install time the installer generate a mapping say em0 -> 2b709ed4-df21-11ed-b5d2-000c29734f98 ?
And devd checks new interface's arrival event and create a new mapping if it does not exist.

Looks perfect now ;)

[zlei]

Some software interface such as tap(4) can also benefit this UUID name.

For tap(4), on clone create tap0, a special control device /dev/tap0 is created alongside. If rename tap0 to tap1 then an alias /dev/tap1 -> /dev/tap0 is created. What if tap2 want to be renamed to tap0 ? That is not elegant.

If we have UUID name, then we can have

tap0 -> /dev/tap/332a4210-df24-11ed-b5d2-000c29734f98

then on rename, we then have

tap1 -> /dev/tap/332a4210-df24-11ed-b5d2-000c29734f98

So the name / alias tap0 is free to use for new tap devices.

For applications that open /dev/tap0, they are redirected to the UUID control device /dev/tap/332a4210-df24-11ed-b5d2-000c29734f98 and should work fluently, then you are free to rename tap0 without disturbing the applications.

Xref PR about interface renaming: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235920

[franco_opnsense.org]

I wonder why adding a new field is needed when IFDATA_DRIVERNAME via if_dname/if_dunit exists? It's also exposed via libifconfig but sadly not via ifconfig command. I only noticed recently by looking at ifinfo command which we use in OPNsense because it provides better interface overview than ifconfig, but is not built in the base system.

[kp]

In D39689#903283, @zlei wrote:

@kp
Do you think it is the right time to consider https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=185619 as well ?

I'd propose creating a stable host global unique name, maybe UUID, as the original name for the interface. It is unchangeable across the interface's lifecycle.

That one gets to be a lot of fun, because when the stack was extended to support renaming interfaces the work was basically only half done.
The issues that I know about are:

• interface groups were not handled. Previously interfaces were always <string><number> and groups were always <string>. That is, groups never end with a number and interfaces always do. The second of those is no longer true, but for some reason we still enforce the first. There is no check to ensure that interface groups cannot have the same name as interfaces. (This can trigger a panic in pf, which is how I know about this)
• there is no locking around interface names
• there is no error handling path in most places where we might want to automatically rename interfaces, and no good way to ensure we end up with identifiable (i.e. we can figure out what interface 'foobaz' used to be) unique names.

Storing the original interface name is probably not useful to fix 185619.

[melifaro]

In D39689#903605, @zlei wrote:

In D39689#903359, @melifaro wrote:

In D39689#903283, @zlei wrote:

@kp
Do you think it is the right time to consider https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=185619 as well ?

I'd propose creating a stable host global unique name, maybe UUID, as the original name for the interface. It is unchangeable across the interface's lifecycle.

I'd say it's worth agreeing on what problem is being solved. Or, alternatively, how do we _want_ interfaces to behave with renaming / vnet-moving. After that it would be easier to approach to (any) solution.
Without it, strictly speaking, we can say that we already have the proposed UUID - that's the interface pointer, as it both host-unique and unchangeable.

The problem is that the pointers are not exposed to userspace.

Exporting question is orthogonal to the source of UUID. In the end you get _some_ UUID string in the kernel and export it to the userspace somehow. It is possible to print the pointer as hex to that string and use it as that UUID.
What I'm trying to say with this absurd example is that we need to state (a) what problem UUIDs solve, (b) why UUID is the (best) solution and (c) what are the desired properties of UUID

[melifaro]

In D39689#903668, @franco_opnsense.org wrote:

I wonder why adding a new field is needed when IFDATA_DRIVERNAME via if_dname/if_dunit exists? It's also exposed via libifconfig but sadly not via ifconfig command. I only noticed recently by looking at ifinfo command which we use in OPNsense because it provides better interface overview than ifconfig, but is not built in the base system.

Sure. The only thing actually used in if_mib in base is exactly this IFDATA_DRIVERNAME functionality. It is undocumented (and wrong for tunap/epair/soem other devices). I'm going to deprecate the module by providing this missing piece of functionality via Netlink ( D39659 ).

[franco_opnsense.org]

In D39689#903716, @melifaro wrote:

Sure. The only thing actually used in if_mib in base is exactly this IFDATA_DRIVERNAME functionality. It is undocumented (and wrong for tunap/epair/soem other devices). I'm going to deprecate the module by providing this missing piece of functionality via Netlink ( D39659 ).

Which part in e.g. tuntap is wrong? Just trying to understand. Thanks!

[melifaro]

In D39689#903693, @kp wrote:

In D39689#903283, @zlei wrote:

@kp
Do you think it is the right time to consider https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=185619 as well ?

I'd propose creating a stable host global unique name, maybe UUID, as the original name for the interface. It is unchangeable across the interface's lifecycle.

That one gets to be a lot of fun, because when the stack was extended to support renaming interfaces the work was basically only half done.
The issues that I know about are:

• interface groups were not handled. Previously interfaces were always <string><number> and groups were always <string>. That is, groups never end with a number and interfaces always do. The second of those is no longer true, but for some reason we still enforce the first. There is no check to ensure that interface groups cannot have the same name as interfaces. (This can trigger a panic in pf, which is how I know about this)
• there is no locking around interface names
• there is no error handling path in most places where we might want to automatically rename interfaces, and no good way to ensure we end up with identifiable (i.e. we can figure out what interface 'foobaz' used to be) unique names.

It will also be good to have everyone on the same page on the desired target state.
Do we actually _want_ renaming interfaces or interface aliases can be enough? The latter can (architecturally) simplify quite a lot of things and may be a good alternative if we can think of a non-invasive way of introducing it w/o breaking a lot of assumptions.
Do we actually _want_ to allow moving clonable interfaces to VNETs instead of creating them there?

Storing the original interface name is probably not useful to fix 185619.

[kp]

(Good questions snipped)

Do we actually _want_ to allow moving clonable interfaces to VNETs instead of creating them there?

We have to. The obvious way to connect a vnet to a given network is using if_epair. That's a clonable interface, and we'd usually create one in the host and move half of the pair into the jail.

[franco_opnsense.org]

Yes, 1:1 alias name assignment would be desirable and sidestep constraints with interface name length for descriptive interfaces (QinQ is too much actually) as well as avoid renaming interfaces when a VLAN ID changes for example. Just change alias and done (possibly with more character support).

Saying this the biggest issue is indeed renaming due to configuration changes which are embedded in the name and the very short IFNAMSIZ.

[kp]

In D39689#903720, @melifaro wrote:

It will also be good to have everyone on the same page on the desired target state.
Do we actually _want_ renaming interfaces or interface aliases can be enough? The latter can (architecturally) simplify quite a lot of things and may be a good alternative if we can think of a non-invasive way of introducing it w/o breaking a lot of assumptions.

That's an intriguing idea. It probably won't fix everything (e.g. the overlap between interface group names and interface aliases would still be an issue), but it would simplify some things. We'd also have to check both the if_xname and if_alias_name where we currently check if_xname, but that's probably a lot simpler than what we do right now.

[melifaro]

In D39689#903721, @kp wrote:

(Good questions snipped)

Do we actually _want_ to allow moving clonable interfaces to VNETs instead of creating them there?

We have to. The obvious way to connect a vnet to a given network is using if_epair. That's a clonable interface, and we'd usually create one in the host and move half of the pair into the jail.

I agree that we _have to_ in the current implementation as it's dictated by the current API. The question is - do we want it to be that way?
For example, the API can be different - you don't explicitly create "interface pairs", but create a single interface at a time within a vnet. Then you issue a "bridge" call that glue two epair interfaces together. In that case you remove all of the complexity associated with creation of "pair" interfaces in the kernel/userland.

[freebsd_igalic.co]

how is the renaming going to interact with sysctl entries?

currently, standard freebsd devices have sysctl entries of "dev.{drivername}.{index}.*"

additionally, infiniband devices provide a Linux compatibility layer, by putting their info into "sys.class.infiniband.{ib_device}.*"

this includes info like, sys.class.infiniband.{ib_device}.ports.1.gid_attrs.ndevs.{id}: {if_name}

that if_name reference should probably also become our uuid

[melifaro]

In D39689#903736, @freebsd_igalic.co wrote:

how is the renaming going to interact with sysctl entries?

currently, standard freebsd devices have sysctl entries of "dev.{drivername}.{index}.*"

I don't have a (good) short-term answer, but as the long-term I see the custom driver-specific data being exported via standard Netlink ethtool modeled interface, where you don't have this problem at all. Some of it will be readable available via libifconfig, or can be read via snl(3) or any other netlink library.

additionally, infiniband devices provide a Linux compatibility layer, by putting their info into "sys.class.infiniband.{ib_device}.*"

this includes info like, sys.class.infiniband.{ib_device}.ports.1.gid_attrs.ndevs.{id}: {if_name}

that if_name reference should probably also become our uuid

[pi]

how is the renaming going to interact with ipfw or pf rules which reference interface names ?

[melifaro]

In D39689#903739, @pi wrote:

how is the renaming going to interact with ipfw or pf rules which reference interface names ?

IMO regardless of the approach taken, we have to support the way it currently works now (e.g. user defines the interface by name and ipfw/pf matches it if the name (or aliases) matches).

[kp]

In D39689#903830, @melifaro wrote:

In D39689#903739, @pi wrote:

how is the renaming going to interact with ipfw or pf rules which reference interface names ?

IMO regardless of the approach taken, we have to support the way it currently works now (e.g. user defines the interface by name and ipfw/pf matches it if the name (or aliases) matches).

Agreed. From pf's perspective that'd require adding an event for interface renames. Currently that's an interface delete / interface add event. It'd probably be generally helpful to distinguish a rename from a delete/add operation as well.