Sorry for messing up some of the VLAN unit allocation logic. I have one suggestion though:

It removes an ability to create unnamed vlans ("ifconfig vlan create") as it doesn't make a lot of sense.

Do we really want to relinquish that ? Sometimes (ie. shell scripts, Kyua tests, etc...) it is useful to let the system assign interface names by itself, that would otherwise need to be hard coded.

ifname=$(ifconfig vlan create vlandev em0 vlan 123)
# ... do something with $ifname ...
ifconfig $ifname destroy

In D27505#614852, @freebsd_oprs.eu wrote:

Sorry for messing up some of the VLAN unit allocation logic. I have one suggestion though:

It removes an ability to create unnamed vlans ("ifconfig vlan create") as it doesn't make a lot of sense.

Do we really want to relinquish that ? Sometimes (ie. shell scripts, Kyua tests, etc...) it is useful to let the system assign interface names by itself, that would otherwise need to be hard coded.

ifname=$(ifconfig vlan create vlandev em0 vlan 123)
# ... do something with $ifname ...
ifconfig $ifname destroy

I agree with the point in general - automatic interface name assignments does make a lot of sense and are really useful. Though, I'm a bit unsure about the vlan specifics as we have an explicit notation that "vlanX" corresponds to, well, vlan with id X.
In your example you're explicitly specifying interface name and vlan id. I'm not sure, why bother with dynamic allocation instead of explicit specification?

@melifaro : Did you have any more time to look at this? I guess we need to be backwards compatible and possible need to have a pool allocation for all interface names starting with "vlan" only? Other interface names are excluded from the unit management.

In D27505#615262, @hselasky wrote:

@melifaro : Did you have any more time to look at this? I guess we need to be backwards compatible and possible need to have a pool allocation for all interface names starting with "vlan" only? Other interface names are excluded from the unit management.

wlan can use the same, which is especially useful for doing automated wifi VAP testing scripts.

I'd really hate to see this functionality go away without some atomic-y userland-driven replacement for it :(

I also should go see how the dynamic interface numbering happens for wlan cloning for WDS/DWDS. That right now has /something/ create dynamic wlan interfaces when DWDS stations associate.

Thank you for the feedback, folks!

My rationale for the removal was based on the tradeoff wrt conplexity vs functionality - proper implementation of a bit weird feature adds quite a bit of complexity.

I don’t have an extremely strong opinion on this and will reimplement the auto-numbering for vlan* interfaces.
However: do you folks think that this is something we should support forever?

Any updates here?

@melifaro : Any news about this issue?

13-current is closing soon!

In D27505#630721, @hselasky wrote:

@melifaro : Any news about this issue?

13-current is closing soon!

Yep, I’ll get to this one before the start of the next week - currently trying to finalise routing changes.

Hi,

Can you please test the patch? I get bogus results:

ifconfig vlan0 create
vlan00

ifconfig mce4.2 create

  1913 ifconfig CALL  ioctl(0x3,SIOCIFCREATE2,0x7fffffffe110)
  1913 ifconfig RET   ioctl -1 errno 22 Invalid argument

Still doesn't work like it should:

ifconfig mce4.3 create
ifconfig: SIOCIFCREATE2: Invalid argument

In D27505#634281, @hselasky wrote:

Still doesn't work like it should:

ifconfig mce4.3 create
ifconfig: SIOCIFCREATE2: Invalid argument

Are you sure you're testing the last version?
I'm able to create similar interfaces:

ifconfig -l
em0.5 em0.6 vlan2 vlan2.3 em0.7 em1.2 em1.7

It is still not working. Trying to investigate why.

1) reboot
2) kldload mlx5en
3) ifconfig mceN.2 create # fails

--HPS

Hi,

The following additional diff make things work. Can you integrate and test?

diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c
index 503c6e6a9fd8..3c3dedda3080 100644
--- a/sys/net/if_vlan.c
+++ b/sys/net/if_vlan.c
@@ -977,8 +977,8 @@ static int
 vlan_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
 {
        char *dp;
-       bool wildcard;
-       bool subinterface = false;
+       bool wildcard = false;
+       bool subinterface = true;
        int unit;
        int error;
        int vid = 0;
@@ -1019,25 +1019,20 @@ vlan_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
 
        if ((error = ifc_name2unit(name, &unit)) == 0) {
                wildcard = (unit < 0);
+               subinterface = false;
        } else {
                struct ifnet *p_tmp = vlan_clone_match_ethervid(name, &vid);
                if (p_tmp != NULL) {
-                       subinterface = true;
-                       unit = IF_DUNIT_NONE;
-                       wildcard = false;
                        if (p != NULL) {
                                if_rele(p_tmp);
-                               if (p != p_tmp)
-                                       error = EINVAL;
-                       } else
+                               if (p != p_tmp) {
+                                       if_rele(p);
+                                       return (EINVAL);
+                               }
+                       } else
                                p = p_tmp;
                }
-       }
-
-       if (error != 0) {
-               if (p != NULL)
-                       if_rele(p);
-               return (error);
+               unit = IF_DUNIT_NONE;
        }
 
        if (!subinterface) {
@@ -1052,7 +1047,8 @@ vlan_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
 
        /* In the wildcard case, we need to update the name. */
        if (wildcard) {
-               for (dp = name; *dp != '\0'; dp++);
+               for (dp = name; *dp != '\0'; dp++)
+                       ;
                if (snprintf(dp, len - (dp-name), "%d", unit) >
                    len - (dp-name) - 1) {
                        panic("%s: interface name too long", __func__);

In D27505#634605, @hselasky wrote:

Hi,

The following additional diff make things work. Can you integrate and test?

Unfortunately, this patch doesn't account for both ifc_name2unit() and vlan_clone_match_ethervid () returning NULL.

Let me test your latest patch tomorrow. Thank you!

In D27505#634834, @hselasky wrote:

Let me test your latest patch tomorrow. Thank you!

Sorry for iterating back and forth and thank you for the patience!