netdev - RE: [net-next 02/10] ixgbevf: Fix broken trunk vlan

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 26 Sep 2011 08:57:03 -0700
From:	"Rose, Gregory V" <gregory.v.rose@...el.com>
To:	Jesse Gross <jesse@...ira.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	Jiri Pirko <jpirko@...hat.com>
Subject: RE: [net-next 02/10] ixgbevf: Fix broken trunk vlan

> -----Original Message-----
> From: Jesse Gross [mailto:jesse@...ira.com]
> Sent: Saturday, September 24, 2011 9:33 AM
> To: Kirsher, Jeffrey T
> Cc: davem@...emloft.net; Rose, Gregory V; netdev@...r.kernel.org;
> gospo@...hat.com; Jiri Pirko
> Subject: Re: [net-next 02/10] ixgbevf: Fix broken trunk vlan
> 
> On Sat, Sep 24, 2011 at 2:17 AM, Jeff Kirsher
> <jeffrey.t.kirsher@...el.com> wrote:
> > From: Greg Rose <gregory.v.rose@...el.com>
> >
> > Changes to clean up the vlan rx path broke trunk vlan.  Trunk vlans in
> > a VF driver are those set using:
> >
> > "ip link set <pfdev> vf <n> <vlanid>"
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@...el.com>
> > CC: Jiri Pirko <jpirko@...hat.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> > index d72905b..4930c46 100644
> > --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> > @@ -293,12 +293,10 @@ static void ixgbevf_receive_skb(struct
> ixgbevf_q_vector *q_vector,
> >  {
> >        struct ixgbevf_adapter *adapter = q_vector->adapter;
> >        bool is_vlan = (status & IXGBE_RXD_STAT_VP);
> > +       u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
> >
> > -       if (is_vlan) {
> > -               u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
> > -
> > +       if (is_vlan && test_bit(tag, adapter->active_vlans))
> >                __vlan_hwaccel_put_tag(skb, tag);
> > -       }
> 
> What happens if you run tcpdump without configuring vlan devices?
> Shouldn't you see tagged packets for the vlans that are being trunked
> to you?  I think this will strip tags in that case.  The apparent
> behavior of vlan filters here is also surprising to me because on one
> hand if they're truly filtering this test shouldn't be needed and on
> the other hand they don't seem to be disabled in promiscuous mode.

I think you're not quite understanding the action the HW is taking here.  In the physical function driver we have put the VF on a VLAN without it knowing that it's on a VLAN.  Once that is done by the PF, the VF is not allowed to configure its own VLANs anymore.  However, the descriptor still includes a bit for the VLAN tag indicating it was a packet that arrived on a VLAN.  The HW is inserting and stripping the VLAN tag though without any awareness of that by the VF driver.

It's a security measure to allow an administrator to put a VF on a VLAN to provide another level of isolation for the VF.

Intel VFs don't support promiscuous mode.  If you ran tcpdump you wouldn't see the VLAN tags because they've been stripped by the HW.  The VF has no choice in this.

- Greg