On 2019/04/15 08:16, David Gwynne wrote:
> > # 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.
Yep. I am absolutely happy to remove the old ioctls.
> 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.
This may affect some people but I think considerably fewer than requiring
the rename everywhere. (I had already gone on a clean-up effort for vlan->vnetid
some time ago on my machines and still keep running into ones I've missed).
> You have my OK on this diff.
Mine too.
> 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
> >
>
