Message-ID: <CAEP_g=-xqgrS1hjApx5bPE2GFkAPJa6meaL0nmUcUbvSOPK-hQ@mail.gmail.com>
Date:	Mon, 26 Sep 2011 16:08:47 -0700
From:	Jesse Gross <jesse@...ira.com>
To:	"Rose, Gregory V" <gregory.v.rose@...el.com>
Cc:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"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

On Mon, Sep 26, 2011 at 8:57 AM, Rose, Gregory V
<gregory.v.rose@...el.com> wrote:
>> -----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.

I understand that VFs are limited in what they can do.  What I'm
concerned about are differences in how the driver behaves when a vlan
device is configured on it vs. not.  There are two cases that I think
you will see different behavior:

* The VF driver has it's own vlan filters.  I'm assuming (correct me
if I'm wrong) that this will further restrict the vlans that the guest
sees from the set that have been configured on the PF driver.  If that
is correct, then putting the VF interface in promiscuous mode should
disable the filters and allow packets from all vlans that the PF
allows.  This situation is the same as if an upstream physical switch
is trunking only a subset of vlans to an end host and the NIC is put
in promiscuous mode.  However, it doesn't look like the VF driver does
anything in response to being in promiscuous mode.  This shouldn't
require any hardware support because it is just clearing the filter
table.
* It looks like the VF driver is receiving not just an indication that
a tag was present but the actual tag and although it was already
stripped by the NIC it can be passed up to the network stack just as
if it was stripped in a non-SR-IOV NIC.  After this patch, if you run
tcpdump on the VF interface you will see packets with tags if a vlan
device is configured but packets without tags if there is no vlan
device.

Does the VF know whether it is assigned to a particular vlan or is
allowed to send/receive tagged packets (essentially the difference
between an access port or a trunk port)?  Or is that why the presence
of vlan device is used as an indication?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists