> Am 05.06.2017 um 09:35 schrieb Reyk Floeter <r...@openbsd.org>: > > >> Am 05.06.2017 um 09:26 schrieb David Gwynne <da...@gwynne.id.au>: >> >> >>> On 5 Jun 2017, at 17:05, Reyk Floeter <r...@openbsd.org> wrote: >>> >>> Well, not just muscle memory but the fact that some people including me had >>> hostname.vlanX files without an explicit "vlan X" in it. >> >> hrm. yes. >> >> that means if you change the vnetid on such an interface at runtime, and >> then re-run netstart later on to try and reset the stack, it wont configure >> it back to the state it would be on boot. im not sure we support running >> netstart after boot though, so maybe it doesnt matter. >> > > We do and I run it all the time, for example > > # sh /etc/netstart vlan5 > > But I'd usually destroy a cloner before creating it again. >
Let me rephrase: I'm fine with the change as long as we never error out on vnetid/parent transitions. The kernel should just cope with it, eg. running the following commands in sequence should be fine and not throw EBUSY or similar: # ifconfig vlan5 create # ifconfig vlan5 up # ifconfig vlan5 vlandev em0 # ifconfig vlan5 vlan 5 # ifconfig vlan5 vlan 6 # ifconfig vlan5 vlandev em1 It think that is the case now. I once updated vlan and a few other cloners to stop giving errors in certain "unconfigured" states because it made it hard in scripts to gather enough input before it works. I also have to be able to change the underlying parent or vnetid on the fly. Reyk >>> >>> And I did like the implicit tags, despite your vlan6000 problem that nobody >>> ever had ;-) >> >> i had the opposite problem where new vlan interfaces would collide with >> existing ones. >> > > Having the same tag on multiple interfaces? Yes, that was a bit clunky with > the implicit tags. > >>> >>> But it is time to move on, we have to cope with it. >>> >>> So no objections anymore, OK reyk >> >> thank you. >> > > Reyk > >>> >>>> Am 05.06.2017 um 07:28 schrieb David Gwynne <da...@gwynne.id.au>: >>>> >>>> vlan(4) handles the vnetid and ifparent ioctls, so we dont need the >>>> vlan specific handling. i previously removed the code that requests >>>> info with a vlan specific ioctl, but this removes the vlan settings >>>> code. >>>> >>>> there's a couple of semantic changes to note though. >>>> >>>> firstly, this aliases the "vlan", "vlandev" and "-vlandev" config >>>> options to setvnetid, setifparent, and delifparent respectively. >>>> this means hostname.vlan and muscle memory will largely keep working. >>>> >>>> secondly, if ifconfig didnt think you'd already configured a vlan >>>> tag on an interface, it would set one for you based on the unit >>>> number of the interface. eg, the following: >>>> >>>> # ifconfig vlan7 create >>>> # ifconfig vlan7 vlandev em0 >>>> >>>> ... would cause vlan7 to be configured with a vnetid of 7 too. >>>> >>>> i have proposed this change before, but some people pushed back, >>>> mostly claiming they didnt want to learn new muscle memory. >>>> >>>> id still like to do this though. the main reasons are: muscle memory >>>> hasn't been an excuse not to change things. eg, im still learning >>>> to cope with the removal of the sparc architecture, and i still >>>> type sudo a few times before remembering it's doas now. >>>> >>>> secondly, the implicit vlan tag generation is inconsistent, both >>>> with other drivers, and with vlan itself. if you create vlan6000, >>>> this doesnt kick in. if you create vxlan0, vxlan1, vxlan6000, there's >>>> no implicit tag there either. >>>> >>>> id rather the config was explicit, rather than conditially implicit. >>>> >>>> ok? >>>> >>>> Index: ifconfig.c >>>> =================================================================== >>>> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v >>>> retrieving revision 1.342 >>>> diff -u -p -r1.342 ifconfig.c >>>> --- ifconfig.c 5 Jun 2017 05:10:23 -0000 1.342 >>>> +++ ifconfig.c 5 Jun 2017 05:16:36 -0000 >>>> @@ -91,8 +91,6 @@ >>>> >>>> #include <netdb.h> >>>> >>>> -#include <net/if_vlan_var.h> >>>> - >>>> #include <netmpls/mpls.h> >>>> >>>> #include <ctype.h> >>>> @@ -216,9 +214,6 @@ void setmpwencap(const char *, int); >>>> void setmpwlabel(const char *, const char *); >>>> void setmpwneighbor(const char *, int); >>>> void setmpwcontrolword(const char *, int); >>>> -void setvlantag(const char *, int); >>>> -void setvlandev(const char *, int); >>>> -void unsetvlandev(const char *, int); >>>> void mpe_status(void); >>>> void mpw_status(void); >>>> void setrdomain(const char *, int); >>>> @@ -367,9 +362,9 @@ const struct cmd { >>>> { "scan", NEXTARG0, 0, setifscan }, >>>> { "broadcast", NEXTARG, 0, setifbroadaddr }, >>>> { "prefixlen", NEXTARG, 0, setifprefixlen}, >>>> - { "vlan", NEXTARG, 0, setvlantag }, >>>> - { "vlandev", NEXTARG, 0, setvlandev }, >>>> - { "-vlandev", 1, 0, unsetvlandev }, >>>> + { "vlan", NEXTARG, 0, setvnetid }, >>>> + { "vlandev", NEXTARG, 0, setifparent }, >>>> + { "-vlandev", 1, 0, delifparent }, >>>> { "group", NEXTARG, 0, setifgroup }, >>>> { "-group", NEXTARG, 0, unsetifgroup }, >>>> { "autoconf", 1, 0, setautoconf }, >>>> @@ -3768,83 +3763,6 @@ getencap(void) >>>> } >>>> >>>> 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; >>>> - >>>> - __tag = tag = strtonum(val, 0, 4095, &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; >>>> - >>>> - 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; >>>> - >>>> - 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 >>>> >>> >>