> On 15 Apr 2019, at 05:56, Klemens Nanni <k...@openbsd.org> wrote:
>
> On Sun, Apr 14, 2019 at 07:46:59PM +0200, Sebastian Benoit wrote:
>> I dont mind keeping vlan/vlandev either, but then they should be aliases,
>> not with their own function.
> Fine with me as well.
>
> Diff below removes the old functions while keeping `[-]vlan' and
> `[-]vlandev' as aliases for `[-]vnetid' and `[-]parent' respectively.
>
> While testing this, I noticed that the old and new interfaces do not
> work well together, although they're supposed to do the same.
>
> This is -CURRENT without my diff. Create one using the old interface:
>
> # ifconfig vlan0 vlan 1 vlandev trunk0
> ifconfig: The 'vlan' option is deprecated, use 'vnetid'
> ifconfig: The 'vlandev' option is deprecated, use 'parent'
>
> Deconfigure it using the new interface:
>
> # ifconfig vlan0 -vnetid
> # ifconfig vlan0 -parent
> ifconfig: SIOCDIFPARENT: Device busy
> # ifconfig vlan0 | grep encap
> encap: vnetid none parent trunk0 txprio packet
>
>
> Clean up, do it again but the other way around; create with new:
>
> # ifconfig vlan0 destroy
> # ifconfig vlan0 vnetid 1 parent trunk0
>
> Delete with old:
>
> # ifconfig vlan0 -vnetid
> # ifconfig vlan0 -parent
> # ifconfig vlan0 | grep encap
> encap: vnetid none parent none txprio packet
>
>
> Shouldn't there be completely interoperability? If so, that seems like
> one more reason for removing the old interface, with or without aliases.
I don't mind if we remove the old command names or alias them to the new ones,
I mostly want the ioctl interface the old names call to go away. Removing or
aliasing achieves that.
The behaviour changes you're seeing above are from the ioctl interface the old
commands are using. The one in particular that you're hitting is that the old
ioctl implicitly brings the vlan interface up, but up which is when the config
is "committed". You can't change or remove the parent while the vlan interface
is up, and it's now up even though you didn't ifconfig vlan0 up.
Another difference you should be aware of is that the code you're removing used
the interface minor as the vnetid if another value wasn't explicitly set. eg,
"ifconfig vlan10 vlandev trunk0" on a newly created interface turns into the
following:
ifconfig vlan10 parent trunk0
ifconfig vlan10 vnetid 10
ifconfig vlan10 up
I'm trying to get rid of implicit behaviours like this with side effects in the
network stack.
You have my OK on this diff.
dlg
>
> So like this? No manual bits so far.
>
> Index: ifconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.399
> diff -u -p -r1.399 ifconfig.c
> --- ifconfig.c 11 Apr 2019 11:32:24 -0000 1.399
> +++ ifconfig.c 14 Apr 2019 19:52:33 -0000
> @@ -250,9 +250,6 @@ void setpwe3fat(const char *, int);
> void unsetpwe3fat(const char *, int);
> void setpwe3neighbor(const char *, const char *);
> void unsetpwe3neighbor(const char *, int);
> -void setvlantag(const char *, int);
> -void setvlandev(const char *, int);
> -void unsetvlandev(const char *, int);
> void mpls_status(void);
> void setrdomain(const char *, int);
> void unsetrdomain(const char *, int);
> @@ -424,9 +421,10 @@ const struct cmd {
> { "-vnetid", 0, 0, delvnetid },
> { "parent", NEXTARG, 0, setifparent },
> { "-parent", 1, 0, delifparent },
> - { "vlan", NEXTARG, 0, setvlantag },
> - { "vlandev", NEXTARG, 0, setvlandev },
> - { "-vlandev", 1, 0, unsetvlandev },
> + { "vlan", NEXTARG, 0, setvnetid },
> + { "-vlan", 0, 0, delvnetid },
> + { "vlandev", NEXTARG, 0, setifparent },
> + { "-vlandev", 1, 0, delifparent },
> { "group", NEXTARG, 0, setifgroup },
> { "-group", NEXTARG, 0, unsetifgroup },
> { "autoconf", 1, 0, setautoconf },
> @@ -4273,89 +4271,6 @@ getencap(void)
> #endif
>
> printf("\n");
> -}
> -
> -static int __tag = 0;
> -static int __have_tag = 0;
> -
> -/* ARGSUSED */
> -void
> -setvlantag(const char *val, int d)
> -{
> - u_int16_t tag;
> - struct vlanreq vreq;
> - const char *errmsg = NULL;
> -
> - warnx("The 'vlan' option is deprecated, use 'vnetid'");
> -
> - __tag = tag = strtonum(val, EVL_VLID_MIN, EVL_VLID_MAX, &errmsg);
> - if (errmsg)
> - errx(1, "vlan tag %s: %s", val, errmsg);
> - __have_tag = 1;
> -
> - bzero((char *)&vreq, sizeof(struct vlanreq));
> - ifr.ifr_data = (caddr_t)&vreq;
> -
> - if (ioctl(s, SIOCGETVLAN, (caddr_t)&ifr) == -1)
> - err(1, "SIOCGETVLAN");
> -
> - vreq.vlr_tag = tag;
> -
> - if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> - err(1, "SIOCSETVLAN");
> -}
> -
> -/* ARGSUSED */
> -void
> -setvlandev(const char *val, int d)
> -{
> - struct vlanreq vreq;
> - int tag;
> - size_t skip;
> - const char *estr;
> -
> - warnx("The 'vlandev' option is deprecated, use 'parent'");
> -
> - bzero((char *)&vreq, sizeof(struct vlanreq));
> - ifr.ifr_data = (caddr_t)&vreq;
> -
> - if (ioctl(s, SIOCGETVLAN, (caddr_t)&ifr) == -1)
> - err(1, "SIOCGETVLAN");
> -
> - (void) strlcpy(vreq.vlr_parent, val, sizeof(vreq.vlr_parent));
> -
> - if (!__have_tag && vreq.vlr_tag == 0) {
> - skip = strcspn(ifr.ifr_name, "0123456789");
> - tag = strtonum(ifr.ifr_name + skip, 0, 4095, &estr);
> - if (estr != NULL)
> - errx(1, "invalid vlan tag and device specification");
> - vreq.vlr_tag = tag;
> - } else if (__have_tag)
> - vreq.vlr_tag = __tag;
> -
> - if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> - err(1, "SIOCSETVLAN");
> -}
> -
> -/* ARGSUSED */
> -void
> -unsetvlandev(const char *val, int d)
> -{
> - struct vlanreq vreq;
> -
> - warnx("The '-vlandev' option is deprecated, use '-parent'");
> -
> - bzero((char *)&vreq, sizeof(struct vlanreq));
> - ifr.ifr_data = (caddr_t)&vreq;
> -
> - if (ioctl(s, SIOCGETVLAN, (caddr_t)&ifr) == -1)
> - err(1, "SIOCGETVLAN");
> -
> - bzero((char *)&vreq.vlr_parent, sizeof(vreq.vlr_parent));
> - vreq.vlr_tag = 0;
> -
> - if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> - err(1, "SIOCSETVLAN");
> }
>
> void
>
