Discussion:
vlan(4) support for hardware VLAN filtering
(too old to reply)
Jared McNeill
2017-11-20 17:19:21 UTC
Permalink
Hi folks --

As part of the fix for kern/52733, I need a mechanism to notify parent
ethernet devices when a VLAN is added or removed. I'm proposing adding an
optional callback to struct ethercom, as demonstrated by the patch below.
Any comments?

Thanks!
Jared



Index: if_ether.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_ether.h,v
retrieving revision 1.68
diff -u -p -r1.68 if_ether.h
--- if_ether.h 28 Sep 2017 16:26:14 -0000 1.68
+++ if_ether.h 20 Nov 2017 17:15:32 -0000
@@ -153,6 +153,7 @@ struct mii_data;
struct ethercom;

typedef int (*ether_cb_t)(struct ethercom *);
+typedef int (*ether_vlancb_t)(struct ethercom *, uint16_t, bool);

/*
* Structure shared between the ethernet driver modules and
@@ -178,6 +179,11 @@ struct ethercom {
* ec_if.if_init, 0 on success, not 0 on failure.
*/
ether_cb_t ec_ifflags_cb;
+ /* Called whenever a vlan interface is configured or
+ * unconfigured. Args include the vlan tag and a flag
+ * indicating whether the tag is being added or removed.
+ */
+ ether_vlancb_t ec_vlan_cb;
kmutex_t *ec_lock;
#ifdef MBUFTRACE
struct mowner ec_rx_mowner; /* mbufs received */
@@ -210,6 +216,7 @@ extern const uint8_t ether_ipmulticast_m
extern const uint8_t ether_ipmulticast_max[ETHER_ADDR_LEN];

void ether_set_ifflags_cb(struct ethercom *, ether_cb_t);
+void ether_set_vlan_cb(struct ethercom *, ether_vlancb_t);
int ether_ioctl(struct ifnet *, u_long, void *);
int ether_addmulti(const struct sockaddr *, struct ethercom *);
int ether_delmulti(const struct sockaddr *, struct ethercom *);
Index: if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.246
diff -u -p -r1.246 if_ethersubr.c
--- if_ethersubr.c 16 Nov 2017 03:07:18 -0000 1.246
+++ if_ethersubr.c 20 Nov 2017 17:15:32 -0000
@@ -1328,6 +1328,12 @@ ether_set_ifflags_cb(struct ethercom *ec
ec->ec_ifflags_cb = cb;
}

+void
+ether_set_vlan_cb(struct ethercom *ec, ether_vlancb_t cb)
+{
+ ec->ec_vlan_cb = cb;
+}
+
/*
* Common ioctls for Ethernet interfaces. Note, we must be
* called at splnet().
Index: if_vlan.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_vlan.c,v
retrieving revision 1.107
diff -u -p -r1.107 if_vlan.c
--- if_vlan.c 16 Nov 2017 03:07:18 -0000 1.107
+++ if_vlan.c 20 Nov 2017 17:15:32 -0000
@@ -455,6 +455,16 @@ vlan_config(struct ifvlan *ifv, struct i
error = 0;
}

+ if (ec->ec_vlan_cb != NULL && tag != 0) {
+ error = (*ec->ec_vlan_cb)(ec, tag, true);
+ if (error) {
+ ec->ec_nvlans--;
+ if (ec->ec_nvlans == 0)
+ (void)ether_disable_vlan_mtu(p);
+ goto done;
+ }
+ }
+
/*
* If the parent interface can do hardware-assisted
* VLAN encapsulation, then propagate its hardware-
@@ -577,6 +587,8 @@ vlan_unconfig_locked(struct ifvlan *ifv,
case IFT_ETHER:
{
struct ethercom *ec = (void *)p;
+ if (ec->ec_vlan_cb != NULL && nmib->ifvm_tag != 0)
+ (void)(*ec->ec_vlan_cb)(ec, nmib->ifvm_tag, false);
if (--ec->ec_nvlans == 0)
(void)ether_disable_vlan_mtu(p);


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2017-11-20 17:28:41 UTC
Permalink
Post by Jared McNeill
Hi folks --
As part of the fix for kern/52733, I need a mechanism to notify parent
ethernet devices when a VLAN is added or removed. I'm proposing adding an
optional callback to struct ethercom, as demonstrated by the patch below.
Any comments?
Thanks!
Jared
typedef int (*ether_cb_t)(struct ethercom *);
+typedef int (*ether_vlancb_t)(struct ethercom *, uint16_t, bool);
Should we rename "ether_cb_t" to "ether_ifflags_cb_t" for consistency?

christos


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Masanobu SAITOH
2017-11-22 10:31:49 UTC
Permalink
Hi.
Post by Jared McNeill
Hi folks --
As part of the fix for kern/52733, I need a mechanism to notify parent ethernet devices when a VLAN is added or removed.
ixv(4) has the same problem. When I noticed this problem, I wrote the following
quick hack:

http://mail-index.netbsd.org/source-changes/2017/09/15/msg088211.html

After committing the above change, I started writing ETHERCAP_VLAN_HWFILTER
stuff because the above quick hack is very dirty AND it doesn't work on some
configuration. The work have not completed yet. It'll take a little time to finish.

http://www.netbsd.org/~msaitoh/vlan-hwfilter-20171122-0.dif

It's just FYI. The interface would be changed while writing ixg(4), ixv(4) and wm(4).
I'll mail formal proposal for ETHERCAP_VLAN_HWFILTER after writing patch for above
three drivers. Any advice is welcomed.

------ output of ifconfig -------
kvmnbsd: {20} ifconfig ixv0
ixv0: flags=0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
capabilities=fff80<TSO4,IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx>
capabilities=fff80<TCP4CSUM_Tx,UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx>
capabilities=fff80<TCP6CSUM_Tx,UDP6CSUM_Rx,UDP6CSUM_Tx,TSO6,LRO>
enabled=7ff80<TSO4,IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx>
enabled=7ff80<TCP4CSUM_Tx,UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx>
enabled=7ff80<TCP6CSUM_Tx,UDP6CSUM_Rx,UDP6CSUM_Tx,TSO6>
ec_capabilities=f<VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWFILTER>
ec_enabled=f<VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWFILTER>
address: 42:06:5e:3e:5c:de
media: Ethernet autoselect (1000baseT full-duplex)
status: active
inet 172.16.0.3/24 broadcast 172.16.0.255 flags 0x0
inet6 fe80::4006:5eff:fe3e:5cde%ixv0/64 flags 0x0 scopeid 0x2
--------------------------------
Post by Jared McNeill
I'm proposing adding an optional callback to struct ethercom, as demonstrated by the patch below. Any comments?
Thanks!
Jared
Index: if_ether.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_ether.h,v
retrieving revision 1.68
diff -u -p -r1.68 if_ether.h
--- if_ether.h    28 Sep 2017 16:26:14 -0000    1.68
+++ if_ether.h    20 Nov 2017 17:15:32 -0000
@@ -153,6 +153,7 @@ struct mii_data;
 struct ethercom;
 typedef int (*ether_cb_t)(struct ethercom *);
+typedef int (*ether_vlancb_t)(struct ethercom *, uint16_t, bool);
 /*
  * Structure shared between the ethernet driver modules and
@@ -178,6 +179,11 @@ struct ethercom {
      * ec_if.if_init, 0 on success, not 0 on failure.
      */
     ether_cb_t                ec_ifflags_cb;
+    /* Called whenever a vlan interface is configured or
+     * unconfigured. Args include the vlan tag and a flag
+     * indicating whether the tag is being added or removed.
+     */
+    ether_vlancb_t                ec_vlan_cb;
     kmutex_t                *ec_lock;
 #ifdef MBUFTRACE
     struct    mowner ec_rx_mowner;        /* mbufs received */
@@ -210,6 +216,7 @@ extern const uint8_t ether_ipmulticast_m
 extern const uint8_t ether_ipmulticast_max[ETHER_ADDR_LEN];
 void    ether_set_ifflags_cb(struct ethercom *, ether_cb_t);
+void    ether_set_vlan_cb(struct ethercom *, ether_vlancb_t);
 int    ether_ioctl(struct ifnet *, u_long, void *);
 int    ether_addmulti(const struct sockaddr *, struct ethercom *);
 int    ether_delmulti(const struct sockaddr *, struct ethercom *);
Index: if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.246
diff -u -p -r1.246 if_ethersubr.c
--- if_ethersubr.c    16 Nov 2017 03:07:18 -0000    1.246
+++ if_ethersubr.c    20 Nov 2017 17:15:32 -0000
@@ -1328,6 +1328,12 @@ ether_set_ifflags_cb(struct ethercom *ec
     ec->ec_ifflags_cb = cb;
 }
+void
+ether_set_vlan_cb(struct ethercom *ec, ether_vlancb_t cb)
+{
+    ec->ec_vlan_cb = cb;
+}
+
 /*
  * Common ioctls for Ethernet interfaces.  Note, we must be
  * called at splnet().
Index: if_vlan.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_vlan.c,v
retrieving revision 1.107
diff -u -p -r1.107 if_vlan.c
--- if_vlan.c    16 Nov 2017 03:07:18 -0000    1.107
+++ if_vlan.c    20 Nov 2017 17:15:32 -0000
@@ -455,6 +455,16 @@ vlan_config(struct ifvlan *ifv, struct i
             error = 0;
         }
+        if (ec->ec_vlan_cb != NULL && tag != 0) {
+            error = (*ec->ec_vlan_cb)(ec, tag, true);
+            if (error) {
+                ec->ec_nvlans--;
+                if (ec->ec_nvlans == 0)
+                    (void)ether_disable_vlan_mtu(p);
+                goto done;
+            }
+        }
+
         /*
          * If the parent interface can do hardware-assisted
          * VLAN encapsulation, then propagate its hardware-
@@ -577,6 +587,8 @@ vlan_unconfig_locked(struct ifvlan *ifv,
         {
         struct ethercom *ec = (void *)p;
+        if (ec->ec_vlan_cb != NULL && nmib->ifvm_tag != 0)
+            (void)(*ec->ec_vlan_cb)(ec, nmib->ifvm_tag, false);
         if (--ec->ec_nvlans == 0)
             (void)ether_disable_vlan_mtu(p);
--
-----------------------------------------------
SAITOH Masanobu (***@execsw.org
***@netbsd.org)

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Jared McNeill
2017-11-22 13:38:05 UTC
Permalink
Thank you for the feedback!

Do we need both ec_vlan_cb and ec_vids? I think I can achieve the same
functionality on vioif(4) w/ ec_vids and ec_ifflags_cb.

I trust your judgement here as I'm not very familiar with this area of the
kernel. How would you like to proceed?

Cheers,
Jared
Post by Masanobu SAITOH
Hi.
Post by Jared McNeill
Hi folks --
As part of the fix for kern/52733, I need a mechanism to notify parent
ethernet devices when a VLAN is added or removed.
ixv(4) has the same problem. When I noticed this problem, I wrote the following
http://mail-index.netbsd.org/source-changes/2017/09/15/msg088211.html
After committing the above change, I started writing ETHERCAP_VLAN_HWFILTER
stuff because the above quick hack is very dirty AND it doesn't work on some
configuration. The work have not completed yet. It'll take a little time to finish.
http://www.netbsd.org/~msaitoh/vlan-hwfilter-20171122-0.dif
It's just FYI. The interface would be changed while writing ixg(4), ixv(4) and wm(4).
I'll mail formal proposal for ETHERCAP_VLAN_HWFILTER after writing patch for above
three drivers. Any advice is welcomed.
------ output of ifconfig -------
kvmnbsd: {20} ifconfig ixv0
ixv0: flags=0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
capabilities=fff80<TSO4,IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx>
capabilities=fff80<TCP4CSUM_Tx,UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx>
capabilities=fff80<TCP6CSUM_Tx,UDP6CSUM_Rx,UDP6CSUM_Tx,TSO6,LRO>
enabled=7ff80<TSO4,IP4CSUM_Rx,IP4CSUM_Tx,TCP4CSUM_Rx>
enabled=7ff80<TCP4CSUM_Tx,UDP4CSUM_Rx,UDP4CSUM_Tx,TCP6CSUM_Rx>
enabled=7ff80<TCP6CSUM_Tx,UDP6CSUM_Rx,UDP6CSUM_Tx,TSO6>
ec_capabilities=f<VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWFILTER>
ec_enabled=f<VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWFILTER>
address: 42:06:5e:3e:5c:de
media: Ethernet autoselect (1000baseT full-duplex)
status: active
inet 172.16.0.3/24 broadcast 172.16.0.255 flags 0x0
inet6 fe80::4006:5eff:fe3e:5cde%ixv0/64 flags 0x0 scopeid 0x2
--------------------------------
Post by Jared McNeill
I'm proposing adding an optional callback to struct ethercom, as
demonstrated by the patch below. Any comments?
Thanks!
Jared
Index: if_ether.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_ether.h,v
retrieving revision 1.68
diff -u -p -r1.68 if_ether.h
--- if_ether.h    28 Sep 2017 16:26:14 -0000    1.68
+++ if_ether.h    20 Nov 2017 17:15:32 -0000
@@ -153,6 +153,7 @@ struct mii_data;
 struct ethercom;
 typedef int (*ether_cb_t)(struct ethercom *);
+typedef int (*ether_vlancb_t)(struct ethercom *, uint16_t, bool);
 /*
  * Structure shared between the ethernet driver modules and
@@ -178,6 +179,11 @@ struct ethercom {
      * ec_if.if_init, 0 on success, not 0 on failure.
      */
     ether_cb_t                ec_ifflags_cb;
+    /* Called whenever a vlan interface is configured or
+     * unconfigured. Args include the vlan tag and a flag
+     * indicating whether the tag is being added or removed.
+     */
+    ether_vlancb_t                ec_vlan_cb;
     kmutex_t                *ec_lock;
 #ifdef MBUFTRACE
     struct    mowner ec_rx_mowner;        /* mbufs received */
@@ -210,6 +216,7 @@ extern const uint8_t ether_ipmulticast_m
 extern const uint8_t ether_ipmulticast_max[ETHER_ADDR_LEN];
 void    ether_set_ifflags_cb(struct ethercom *, ether_cb_t);
+void    ether_set_vlan_cb(struct ethercom *, ether_vlancb_t);
 int    ether_ioctl(struct ifnet *, u_long, void *);
 int    ether_addmulti(const struct sockaddr *, struct ethercom *);
 int    ether_delmulti(const struct sockaddr *, struct ethercom *);
Index: if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.246
diff -u -p -r1.246 if_ethersubr.c
--- if_ethersubr.c    16 Nov 2017 03:07:18 -0000    1.246
+++ if_ethersubr.c    20 Nov 2017 17:15:32 -0000
@@ -1328,6 +1328,12 @@ ether_set_ifflags_cb(struct ethercom *ec
     ec->ec_ifflags_cb = cb;
 }
+void
+ether_set_vlan_cb(struct ethercom *ec, ether_vlancb_t cb)
+{
+    ec->ec_vlan_cb = cb;
+}
+
 /*
  * Common ioctls for Ethernet interfaces.  Note, we must be
  * called at splnet().
Index: if_vlan.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_vlan.c,v
retrieving revision 1.107
diff -u -p -r1.107 if_vlan.c
--- if_vlan.c    16 Nov 2017 03:07:18 -0000    1.107
+++ if_vlan.c    20 Nov 2017 17:15:32 -0000
@@ -455,6 +455,16 @@ vlan_config(struct ifvlan *ifv, struct i
             error = 0;
         }
+        if (ec->ec_vlan_cb != NULL && tag != 0) {
+            error = (*ec->ec_vlan_cb)(ec, tag, true);
+            if (error) {
+                ec->ec_nvlans--;
+                if (ec->ec_nvlans == 0)
+                    (void)ether_disable_vlan_mtu(p);
+                goto done;
+            }
+        }
+
         /*
          * If the parent interface can do hardware-assisted
          * VLAN encapsulation, then propagate its hardware-
@@ -577,6 +587,8 @@ vlan_unconfig_locked(struct ifvlan *ifv,
         {
         struct ethercom *ec = (void *)p;
+        if (ec->ec_vlan_cb != NULL && nmib->ifvm_tag != 0)
+            (void)(*ec->ec_vlan_cb)(ec, nmib->ifvm_tag, false);
         if (--ec->ec_nvlans == 0)
             (void)ether_disable_vlan_mtu(p);
--
-----------------------------------------------
Loading...