Patchwork [net-next,05/11] net/mlx4_en: Fix vlan mask for ethtool steering rules

login
register
mail settings
Submitter Amir Vadai
Date Jan. 30, 2013, 10:34 a.m.
Message ID <1359542067-10760-6-git-send-email-amirv@mellanox.com>
Download mbox | patch
Permalink /patch/216819/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Amir Vadai - Jan. 30, 2013, 10:34 a.m.
From: Hadar Hen Zion <hadarh@mellanox.com>

The vlan mask field should be validated and assigned according to the field
size which is 12 bits. Also replace the numeric 0xfff mask with existing kernel
macro.

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)
Ben Hutchings - Jan. 30, 2013, 5:20 p.m.
On Wed, 2013-01-30 at 12:34 +0200, Amir Vadai wrote:
> From: Hadar Hen Zion <hadarh@mellanox.com>
> 
> The vlan mask field should be validated and assigned according to the field
> size which is 12 bits.
[...]

My understanding is that ethtool_flow_ext::vlan_tci corresponds to the
whole VLAN tag; a filter rule that matches only the VID must have a mask
of htons(0xfff).

The original implementation of this extension was in ixgbe and it's now
unclear to me whether it's programming hardware filters to match just
the VID or the whole VLAN tag.  Alexander, can you answer this?

I was thinking about adding new keywords to ethtool -U ('vid', 'vtag'?)
to make it easier to specify just the VID and clearer when you're
specifying the full tag.  (The 'vlan' keyword would have to remain but
could be removed from documentation.)  But if no-one really implements
matching of the full tag then maybe we can pretend those 4 bits don't
exist.

Ben.

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index f33049f..f36c219 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -666,12 +666,16 @@  static int mlx4_en_validate_flow(struct net_device *dev,
 
 	if ((cmd->fs.flow_type & FLOW_EXT)) {
 		if (cmd->fs.m_ext.vlan_etype ||
-		    !(cmd->fs.m_ext.vlan_tci == 0 ||
-		      cmd->fs.m_ext.vlan_tci == cpu_to_be16(0xfff)))
+		    !((cmd->fs.m_ext.vlan_tci & cpu_to_be16(VLAN_VID_MASK)) ==
+		      0 ||
+		      (cmd->fs.m_ext.vlan_tci & cpu_to_be16(VLAN_VID_MASK)) ==
+		      cpu_to_be16(VLAN_VID_MASK)))
 			return -EINVAL;
+
 		if (cmd->fs.m_ext.vlan_tci) {
 			if (be16_to_cpu(cmd->fs.h_ext.vlan_tci) >= VLAN_N_VID)
 				return -EINVAL;
+
 		}
 	}
 
@@ -690,9 +694,10 @@  static int mlx4_en_ethtool_add_mac_rule(struct ethtool_rxnfc *cmd,
 	memcpy(spec_l2->eth.dst_mac_msk, &mac_msk, ETH_ALEN);
 	memcpy(spec_l2->eth.dst_mac, mac, ETH_ALEN);
 
-	if ((cmd->fs.flow_type & FLOW_EXT) && cmd->fs.m_ext.vlan_tci) {
+	if ((cmd->fs.flow_type & FLOW_EXT) &&
+	    (cmd->fs.m_ext.vlan_tci & cpu_to_be16(VLAN_VID_MASK))) {
 		spec_l2->eth.vlan_id = cmd->fs.h_ext.vlan_tci;
-		spec_l2->eth.vlan_id_msk = cpu_to_be16(0xfff);
+		spec_l2->eth.vlan_id_msk = cpu_to_be16(VLAN_VID_MASK);
 	}
 
 	list_add_tail(&spec_l2->list, rule_list_h);