Re: [PATCH 10/27] ixgb: Don't check for vlan group on transmit

On Mon, Dec 13, 2010 at 10:43 AM, Tantilov, Emil S
[off-list ref] wrote:

quoted

-----Original Message-----
From: Jesse Gross [mailto:jesse@nicira.com]
Sent: Sunday, December 12, 2010 5:01 PM
To: Kirsher, Jeffrey T
Cc: davem@davemloft.net; Tantilov, Emil S; netdev@vger.kernel.org;
gospo@redhat.com; bphilips@novell.com
Subject: Re: [PATCH 10/27] ixgb: Don't check for vlan group on transmit

On Fri, Dec 10, 2010 at 10:17 PM, Jeff Kirsher
[off-list ref] wrote:

quoted

From: Emil Tantilov <redacted>

Based on a patch from Jesse Gross.

Enable vlan tag insertion even when vlan group is not configured.

For ixgb HW both CTRL0.VME and VLE bit in the Tx descriptor need to be

set

quoted

in order to enable HW acceleration.

Introduced separate functions for enabling/disabling of vlan tag

stripping

quoted

similar to ixgbe.

Thanks for working on this.  However, I don't think that this patch
actually does what it says.

In ixgb_xmit_frame() it's still checking whether adapter->vlgrp is
non-null before inserting a tag, so it will drop tags unless a vlan
group is configured.  Also, since it's not currently possible to
toggle NETIF_F_HW_VLAN_RX, vlan stripping will never get disabled.
This is actually a regression since before vlan stripping would get
disabled if no vlan group was configured.  Now, vlan headers will get
silently dropped if there is no vlan group.

I'm sorry. This patch was supposed to include your original patch that
removed the vlgrp check on Tx. Somehow that didn't make it (I may have generated the patch from the wrong branch).

quoted

Regardless of that, I still think this is a useful change on the road
towards adopting the new vlan interfaces, the problem is just that
currently it's halfway in between the old and the new.  Given that, it
would obviously be much better to move all the way over the new when
addressing this.

Since this patch is already applied can you submit your change again?

Sure.  I think it's probably best to just complete the conversion to
the new vlan interfaces, so I went ahead and did that.  It's much
easier now that you've pulled the hardware specific bits out for
enabling/disabling vlan offloading.  I'll send out that patch shortly
- please take a look since I don't have the hardware to test it.

quoted

Out of curiosity, is the implication of this that vlan insertion on
transmit and stripping on receive are always configured together?  I
don't have hardware supported by this driver but when I worked on
ixgbe (which is at least superficially similar in this area) I didn't
have any problems configuring them independently.

Yes - the design of the original 10GB HW supported in ixgb does not allow
setting Tx and Rx independently. As you mentioned ixgbe does not have
this problem.

OK, thanks for the explanation.