Page MenuHomeFreeBSD

ifnet: Release unit number on renaming interfaces
Needs ReviewPublic

Authored by zlei on Apr 20 2023, 10:31 AM.
Tags
None
Referenced Files
F82544642: D39713.id.diff
Tue, Apr 30, 2:13 AM
F82530079: D39713.id120709.diff
Mon, Apr 29, 9:37 PM
Unknown Object (File)
Sat, Apr 27, 5:47 AM
Unknown Object (File)
Sat, Apr 27, 5:47 AM
Unknown Object (File)
Sat, Apr 27, 3:51 AM
Unknown Object (File)
Fri, Apr 26, 2:29 PM
Unknown Object (File)
Mon, Apr 1, 1:16 PM
Unknown Object (File)
Mar 31 2024, 5:28 AM

Details

Reviewers
kevans
phk
Group Reviewers
network
Summary

And reacquire one if the cloner of new interface name is same as the original.

PR: 235920
MFC after: 2 weeks

Test Plan

Run the test script: F59932955

Some basic tests:

  1. rename physical interfaces
  2. rename interface that its cloner do not have IFC_F_AUTOUNIT flag, such as epair(4)
  3. rename interface that its cloner have IFC_F_AUTOUNIT` flag, such as bridge(4)
  4. move interface to another vnet, and move back
# ifconfig bridge0 create name br0
br0
# ifconfig bridge0 create name br1
br1
# ifconfig br1 name bridge0
bridge0
# ifconfig bridge create
bridge0
# ifconfig br0 destroy
# ifconfig bridge0 destroy
# ifconfig bridge create name br2
br2
# kldunload if_bridge

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zlei requested review of this revision.Apr 20 2023, 10:31 AM
zlei edited the summary of this revision. (Show Details)

For tap(4) interfaces, this fix is not enough.

I'd suggest testing interface renaming after moving to VNET. I struggled a lot with VNET moves when I did D36632.

What happens if we rename an interface without a number? Do we keep the old unit number? So we'd still have overlap/conflict issues in that case.

This is mostly another manifestation of the incompleteness of our interface rename support. It's good that you've identified this problem, but I think we need to take it along in a wider discussion about interface (re-)naming, rather than trying to address one issue at a time.

I'd suggest testing interface renaming after moving to VNET. I struggled a lot with VNET moves when I did D36632.

For vnet jails, they have their own cloners, no problems found.

# ifconfig bridge0 create
# jail -c name=j vnet persist
# ifconfig bridge0 vnet j
# jexec j ifconfig bridge create
bridge1

But the loaned interface still occupy an unit number in home vnet's cloner.

# ifconfig bridge0 create
# jail -c name=j vnet persist
# ifconfig bridge0 vnet j
# ifconfig bridge0 create
ifconfig: interface bridge0 already exists

Since bridge0 is moved into other vnet, the home vnet should be able to have another "bridge0" .

sys/net/if_clone.c
892

This leads regression on physical interface renaming.

# ifconfig em0 name eth0
ifconfig: ioctl SIOCSIFNAME (set name): Invalid argument
In D39713#903855, @zlei wrote:

I'd suggest testing interface renaming after moving to VNET. I struggled a lot with VNET moves when I did D36632.

For vnet jails, they have their own cloners, no problems found.

# ifconfig bridge0 create
# jail -c name=j vnet persist
# ifconfig bridge0 vnet j
# jexec j ifconfig bridge create
bridge1

But the loaned interface still occupy an unit number in home vnet's cloner.

# ifconfig bridge0 create
# jail -c name=j vnet persist
# ifconfig bridge0 vnet j
# ifconfig bridge0 create
ifconfig: interface bridge0 already exists

Since bridge0 is moved into other vnet, the home vnet should be able to have another "bridge0" .

And as soon as that vnet is destroyed, bridge0 is re-homed back to the original. What happens if the unr is already re-taken at that point? Just destroying it isn't the right answer, neither is reassigning it.

In D39713#903771, @kp wrote:

What happens if we rename an interface without a number? Do we keep the old unit number? So we'd still have overlap/conflict issues in that case.

Do you mean this?

# ifconfig bridge create name br
# ifconfig br name bridge0

This is mostly another manifestation of the incompleteness of our interface rename support. It's good that you've identified this problem, but I think we need to take it along in a wider discussion about interface (re-)naming, rather than trying to address one issue at a time.

Yes I agree.

sys/net/if_clone.c
909

ifp->if_dunit might be IF_DUNIT_NONE.

In D39713#903855, @zlei wrote:

I'd suggest testing interface renaming after moving to VNET. I struggled a lot with VNET moves when I did D36632.

For vnet jails, they have their own cloners, no problems found.

# ifconfig bridge0 create
# jail -c name=j vnet persist
# ifconfig bridge0 vnet j
# jexec j ifconfig bridge create
bridge1

But the loaned interface still occupy an unit number in home vnet's cloner.

# ifconfig bridge0 create
# jail -c name=j vnet persist
# ifconfig bridge0 vnet j
# ifconfig bridge0 create
ifconfig: interface bridge0 already exists

Since bridge0 is moved into other vnet, the home vnet should be able to have another "bridge0" .

And as soon as that vnet is destroyed, bridge0 is re-homed back to the original. What happens if the unr is already re-taken at that point? Just destroying it isn't the right answer, neither is reassigning it.

I think the correct way is leave it as is, if the unr is already taken when bridge0 return home vnet.

As for the duplicate name, it is a known issue ( prior this change ).

Release / re-acquire unit number on vnet move.

Rewrite the logic of reassign unit number.

I have mixed feelings about this. I understand the desire to fix some user-visible corner cases. From kernel standpoint, we already have quite a lot of additional complexity with per-vnet cloners. This diff makes the code even more complex and I’m not sure if it’s worth the benefits. I’d rather invest in the design that makes the renaming/moving logic simpler, not the other way round.

I have mixed feelings about this. I understand the desire to fix some user-visible corner cases. From kernel standpoint, we already have quite a lot of additional complexity with per-vnet cloners. This diff makes the code even more complex and I’m not sure if it’s worth the benefits. I’d rather invest in the design that makes the renaming/moving logic simpler, not the other way round.

I think it is the design problem of interface naming ( auto unit number ) and renaming. After vimage is introduced into base system, there are multiple namespaces, so the naming / renaming / unit number is even more complex. That's why I suggested unique global name such as UUID.

A unit number is only meaningful when the name is within the cloner itself, say em0, em1, em2. So the logic is not that complex for unit number.
When an interface is not using the schema em0, then unit number should not be take into account.

Event we do not have ifc_unrhdr we can still archive auto unit number, although it will not be efficient as current implementation.