linux-kernel - Re: [Intel-wired-lan] [RFC PATCH net] e1000e: keep vlan interfaces functional after rxvlan off

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:	Wed, 1 Jun 2016 07:31:53 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Jarod Wilson <jarod@...hat.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Netdev <netdev@...r.kernel.org>,
	intel-wired-lan <intel-wired-lan@...ts.osuosl.org>
Subject: Re: [Intel-wired-lan] [RFC PATCH net] e1000e: keep vlan interfaces
 functional after rxvlan off

On Tue, May 17, 2016 at 12:03 PM, Jarod Wilson <jarod@...hat.com> wrote:
> I've got a bug report about an e1000e interface, where a vlan interface is
> set up on top of it:
>
> $ ip link add link ens1f0 name ens1f0.99 type vlan id 99
> $ ip link set ens1f0 up
> $ ip link set ens1f0.99 up
> $ ip addr add 192.168.99.92 dev ens1f0.99
>
> At this point, I can ping another host on vlan 99, ip 192.168.99.91.
> However, if I do the following:
>
> $ ethtool -K ens1f0 rxvlan off
>
> Then no traffic passes on ens1f0.99. It comes back if I toggle rxvlan on
> again. I'm not sure if this is actually intended behavior, or if there's a
> lack of software vlan stripping fallback, or what, but things continue to
> work if I simply don't call e1000e_vlan_strip_disable() if there are
> active vlans (plagiarizing a function from the e1000 driver here) on the
> interface.
>
> Also slipped a related-ish fix to the kerneldoc text for
> e1000e_vlan_strip_disable here...
>
> CC: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> CC: intel-wired-lan@...ts.osuosl.org
> CC: netdev@...r.kernel.org
> Signed-off-by: Jarod Wilson <jarod@...hat.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 75e6089..73f7452 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -154,6 +154,16 @@ void __ew32(struct e1000_hw *hw, unsigned long reg, u32 val)
>         writel(val, hw->hw_addr + reg);
>  }
>
> +static bool e1000e_vlan_used(struct e1000_adapter *adapter)
> +{
> +       u16 vid;
> +
> +       for_each_set_bit(vid, adapter->active_vlans, VLAN_N_VID)
> +               return true;
> +

I'm pretty sure this is always going to return true if 8021q is loaded
because VLAN 0 is always added to the device even if no other VLANs
are in use.

> +       return false;
> +}
> +
>  /**
>   * e1000_regdump - register printout routine
>   * @hw: pointer to the HW structure
> @@ -2789,7 +2799,7 @@ static void e1000e_vlan_filter_enable(struct e1000_adapter *adapter)
>  }
>
>  /**
> - * e1000e_vlan_strip_enable - helper to disable HW VLAN stripping
> + * e1000e_vlan_strip_disable - helper to disable HW VLAN stripping
>   * @adapter: board private structure to initialize
>   **/
>  static void e1000e_vlan_strip_disable(struct e1000_adapter *adapter)
> @@ -3443,7 +3453,8 @@ static void e1000e_set_rx_mode(struct net_device *netdev)
>
>         ew32(RCTL, rctl);
>
> -       if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX)
> +       if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX ||
> +           e1000e_vlan_used(adapter))
>                 e1000e_vlan_strip_enable(adapter);
>         else
>                 e1000e_vlan_strip_disable(adapter);

So if the VLAN tag stripping is disabled what happens that is causing
the VLAN test to fail?  It sounds like this might be working around a
kernel bug where a VLAN created on a device that supports hardware tag
stripping only supports hardware tag stripping.  Maybe a better fix
would be to add a fall back so if the VLAN tag is in the frame instead
of stripped it still makes it to the correct spot.

- Alex