netdev - Re: [PATCH net-next] nfp: add VLAN filtering support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170615122345.1360b1c1@cakuba.netronome.com>
Date:   Thu, 15 Jun 2017 12:23:45 -0700
From:   Jakub Kicinski <kubakici@...pl>
To:     "Mintz, Yuval" <Yuval.Mintz@...ium.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "oss-drivers@...ronome.com" <oss-drivers@...ronome.com>,
        Pablo Cascón <pablo.cascon@...ronome.com>
Subject: Re: [PATCH net-next] nfp: add VLAN filtering support

On Thu, 15 Jun 2017 12:50:57 +0000, Mintz, Yuval wrote:
> > From: Pablo Cascón <pablo.cascon@...ronome.com>
> > 
> > Add general use per-vNIC mailbox area and use it for VLAN filtering support.
> > Initially proto is hardcoded to 802.1q.
> >   
> ...
> > +static int
> > +nfp_net_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16
> > +vid) {
> > +	struct nfp_net *nn = netdev_priv(netdev);
> > +
> > +	/* Priority tagged packets with vlan id 0 are processed by the
> > +	 * NFP as untagged packets
> > +	 */
> > +	if (!vid)
> > +		return 0;
> > +
> > +	nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_VID, vid);
> > +	nn_writew(nn, NFP_NET_CFG_VLAN_FILTER_PROTO, ETH_P_8021Q);  
> 
> If it's hard-coded, don't you want to fail the request in case proto is different?

We have NETIF_F_HW_VLAN_CTAG_FILTER and NETIF_F_HW_VLAN_STAG_FILTER as
separate features exactly for that reason.  If we don't advertise STAG
support, VLAN ndos will never be called with non-802.1q proto.  See
vlan_hw_filter_capable(), called from __vlan_vid_add().  You can also
find multiple drivers in the tree which depend on the same behaviour
(e.g. Intel drivers).

So we considered checking proto, but we realized we don't have to,
because the stack is nice enough to do it for us.