On Mon, Jan 24, 2005 at 08:32:12AM +0500, Boris Kovalenko wrote: > Brooks Davis wrote: > Hello! > > > > > > >I still don't see how this usefull differs from taking action or not > >taking action. > Just more simple to understand (trusted or not trusted vlan (IMHO)), but > taking action via IPFW of course will be more flexible.
I disagree it's more simple.
> >While I think a lower level solution will be the most useful in the
> >end, I don't object to an interface based solution. I think you should
> >proceed with that with the idea that you add a third, optional keyword
> >to vlan initalization to specify priority. That way you can create
> >seperate interfaces for each priority for any vlan tag. Something like:
> >
> >ifconfig vlan create vlan 2 vlandev fxp0 vlanpri 3
> But this is already done with my patch!!! :))) Have You seen it? I've
> added also ability to set apropriate CFI. Attached patch again to this
> message. Please review it again :)
I missread your patch before. Sorry about that. Overall, this seems
like a step forward. I would like this interface to be labled as
subject to change in the ifconfig manpage so that a solution that does
not treat different priorities as seperate interfaces is not precluded
by this specific implementation. I'm sure we can keep an interface that
handles priorities as seperate interfaces, but I'm not sure we'll want
to do it via the vlan device (attractivly simple though that is.)
This patch appears to be against 4 or 5. In 6 we've largly rewritten
ifconfig so the patch won't apply. It looks like a simple matter to fix
this issue. We'll need to commit to 6 before 4 or 5.
I've embeded some comments in the text below.
-- Brooks
> --- sbin/ifconfig/ifconfig.h.orig Wed Jan 19 10:44:20 2005
> +++ sbin/ifconfig/ifconfig.h Fri Jan 21 09:11:22 2005
> @@ -49,6 +49,8 @@
>
> extern void setvlantag(const char *, int, int, const struct afswtch *rafp);
> extern void setvlandev(const char *, int, int, const struct afswtch *rafp);
> +extern void setvlanpri(const char *, int, int, const struct afswtch *rafp);
> +extern void setvlancfi(const char *, int, int, const struct afswtch *rafp);
> extern void unsetvlandev(const char *, int, int, const struct afswtch *rafp);
> extern void vlan_status(int s, struct rt_addrinfo *);
>
> --- sbin/ifconfig/ifconfig.c.orig Wed Jan 19 10:56:44 2005
> +++ sbin/ifconfig/ifconfig.c Fri Jan 21 09:11:54 2005
> @@ -247,6 +247,8 @@
> #endif
> #ifdef USE_VLANS
> { "vlan", NEXTARG, setvlantag },
> + { "vlanpri", NEXTARG, setvlanpri },
> + { "vlancfi", NEXTARG, setvlancfi },
> { "vlandev", NEXTARG, setvlandev },
> { "-vlandev", NEXTARG, unsetvlandev },
> #endif
> --- sbin/ifconfig/ifvlan.c.orig Thu Apr 18 23:14:09 2002
> +++ sbin/ifconfig/ifvlan.c Fri Jan 21 12:19:38 2005
> @@ -59,6 +59,8 @@
> "$FreeBSD: src/sbin/ifconfig/ifvlan.c,v 1.5 2002/04/18 17:14:09 imp Exp $";
> #endif
> static int __tag = 0;
> +static int __pri = 0;
> +static int __cfi = 0;
> static int __have_tag = 0;
>
> void
> @@ -72,9 +74,10 @@
> if (ioctl(s, SIOCGETVLAN, (caddr_t)&ifr) == -1)
> return;
>
> - printf("\tvlan: %d parent interface: %s\n",
> - vreq.vlr_tag, vreq.vlr_parent[0] == '\0' ?
> - "<none>" : vreq.vlr_parent);
> + printf("\tvlan: %d 802.1p: %d CFI: %d parent interface: %s \n",
> + EVL_VLANOFTAG(vreq.vlr_tag), EVL_PRIOFTAG(vreq.vlr_tag),
> + EVL_CFIOFTAG(vreq.vlr_tag),
> + vreq.vlr_parent[0] == '\0' ? "<none>" : vreq.vlr_parent );
>
> return;
> }
> @@ -88,13 +91,66 @@
> __tag = tag = atoi(val);
> __have_tag = 1;
>
> + if(tag < 1 || tag > 4094)
> + errx(1, "VLAN ID shoud be in range 1..4094");
errx should be fully indented.
> +
> + 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 = EVL_MAKETAG(tag, __pri, __cfi);
> +
> + if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> + err(1, "SIOCSETVLAN");
> +
> + return;
> +}
> +
> +void
> +setvlanpri(const char *val, int d, int s, const struct afswtch *afp)
> +{
> + u_int16_t pri;
> + struct vlanreq vreq;
> +
> + __pri = pri = atoi(val);
I know other nearby code does this, but atoi should not be used. It has
not useful error checking. strtoul should be used instead.
> +
> + if(pri > 7)
> + errx(1, "VLAN 802.1p shoud be in range 0..7");
errx should be fully indented.
> +
> + 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 = EVL_MAKETAG(EVL_VLANOFTAG(vreq.vlr_tag), pri, __cfi);
> +
> + if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> + err(1, "SIOCSETVLAN");
> +
> + return;
> +}
> +
> +void
> +setvlancfi(const char *val, int d, int s, const struct afswtch *afp)
> +{
> + u_int16_t cfi;
> + struct vlanreq vreq;
> +
> + __cfi = cfi = atoi(val);
strtoul
> +
> + if(cfi > 1)
> + errx(1, "VLAN CFI shoud be 0 or 1");
indent.
> +
> 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;
> + vreq.vlr_tag = EVL_MAKETAG(EVL_VLANOFTAG(vreq.vlr_tag), __pri, cfi);
>
> if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> err(1, "SIOCSETVLAN");
> @@ -117,7 +173,7 @@
> err(1, "SIOCGETVLAN");
>
> strncpy(vreq.vlr_parent, val, sizeof(vreq.vlr_parent));
> - vreq.vlr_tag = __tag;
> + vreq.vlr_tag = EVL_MAKETAG(__tag, __pri, __cfi);
>
> if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
> err(1, "SIOCSETVLAN");
> --- sbin/ifconfig/ifconfig.8.orig Thu Sep 30 20:25:39 2004
> +++ sbin/ifconfig/ifconfig.8 Fri Jan 21 09:39:24 2005
> @@ -386,15 +386,35 @@
> pseudo interface, set the VLAN tag value
> to
> .Ar vlan_tag .
> -This value is a 16-bit number which is used to create an 802.1Q
> +This value is a 12-bit number which is used to create an 802.1Q
> VLAN header for packets sent from the
> .Xr vlan 4
> interface.
> Note that
> -.Cm vlan
> +.Cm vlan, vlanpri, vlancfi
> and
> .Cm vlandev
> -must both be set at the same time.
> +must be set at the same time.
> +.It Cm vlanpri Ar vlan_pri
> +If the interface is a
> +.Xr vlan 4
> +pseudo interface, set the 802.1p priority value
> +to
> +.Ar vlan_pri .
> +This value is a 3-bit number which is used to tag outgoing
> +VLAN packtes with apropriate priority. If
> +.Cm vlanpri
> +is omitted it default to 0.
> +.It Cm vlancfi Ar vlan_cfi
> +If the interface is a
> +.Xr vlan 4
> +pseudo interface, set the CFI value
> +to
> +.Ar vlan_cfi .
> +This value is a 1-bit number which is used to tag outgoing
> +VLAN packtes with apropriate CFI value. If
> +.Cm vlancfi
> +is omitted it default to 0.
> .It Cm vlandev Ar iface
> If the interface is a
> .Xr vlan 4
> --- sys/net/if_vlan_var.h.orig Mon Jan 19 00:29:04 2004
> +++ sys/net/if_vlan_var.h Fri Jan 21 09:46:46 2005
> @@ -43,6 +43,8 @@
> #define EVL_VLID_MASK 0x0FFF
> #define EVL_VLANOFTAG(tag) ((tag) & EVL_VLID_MASK)
> #define EVL_PRIOFTAG(tag) (((tag) >> 13) & 7)
> +#define EVL_CFIOFTAG(tag) (((tag) >> 12) & 1)
> +#define EVL_MAKETAG(vlid,pri,cfi) ((((((pri) & 7) << 1) | ((cfi) & 1))
> << 12) | ((vlid) & EVL_VLID_MASK))
>
> /* sysctl(3) tags, for compatibility purposes */
> #define VLANCTL_PROTO 1
> @@ -52,8 +54,8 @@
> * Configuration structure for SIOCSETVLAN and SIOCGETVLAN ioctls.
> */
> struct vlanreq {
> - char vlr_parent[IFNAMSIZ];
> - u_short vlr_tag;
> + char vlr_parent[IFNAMSIZ];
> + u_int16_t vlr_tag;
This appears to be a no-op. Is it needed?
> };
> #define SIOCSETVLAN SIOCSIFGENERIC
> #define SIOCGETVLAN SIOCGIFGENERIC
> --- sys/net/if_vlan.c.orig Wed Jan 19 10:40:32 2005
> +++ sys/net/if_vlan.c Fri Jan 21 09:05:45 2005
> @@ -930,15 +930,6 @@
> error = ENOENT;
> break;
> }
> - /*
> - * Don't let the caller set up a VLAN tag with
> - * anything except VLID bits.
> - */
> -
> - if (vlr.vlr_tag & ~EVL_VLID_MASK) {
> - error = EINVAL;
> - break;
> - }
>
> VLAN_LOCK();
> error = vlan_config(ifv, p);
--
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
pgpBJ8mINIGt4.pgp
Description: PGP signature
