Re: vlan(4) support for hardware VLAN filtering

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

 Hi.

On 2017/11/21 2:19, Jared McNeill wrote:
> 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
--------------------------------

> 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);


--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)