[Intel-wired-lan] [net-next 01/10] fm10k: remove is_slot_appropriate

[Intel-wired-lan] [net-next 01/10] fm10k: remove is_slot_appropriate

[Jacob Keller]

This function is no longer used now that we have updated fm10k_slot_warn
functionality.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c   | 14 --------------
 drivers/net/ethernet/intel/fm10k/fm10k_type.h |  1 -
 drivers/net/ethernet/intel/fm10k/fm10k_vf.c   | 14 --------------
 3 files changed, 29 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
index 3ca0233b3ea2..241b969626bc 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
@@ -185,19 +185,6 @@ static s32 fm10k_init_hw_pf(struct fm10k_hw *hw)
 }
 
 /**
- *  fm10k_is_slot_appropriate_pf - Indicate appropriate slot for this SKU
- *  @hw: pointer to hardware structure
- *
- *  Looks at the PCIe bus info to confirm whether or not this slot can support
- *  the necessary bandwidth for this device.
- **/
-static bool fm10k_is_slot_appropriate_pf(struct fm10k_hw *hw)
-{
-	return (hw->bus.speed == hw->bus_caps.speed) &&
-	       (hw->bus.width == hw->bus_caps.width);
-}
-
-/**
  *  fm10k_update_vlan_pf - Update status of VLAN ID in VLAN filter table
  *  @hw: pointer to hardware structure
  *  @vid: VLAN ID to add to table
@@ -1849,7 +1836,6 @@ static struct fm10k_mac_ops mac_ops_pf = {
 	.init_hw		= &fm10k_init_hw_pf,
 	.start_hw		= &fm10k_start_hw_generic,
 	.stop_hw		= &fm10k_stop_hw_generic,
-	.is_slot_appropriate	= &fm10k_is_slot_appropriate_pf,
 	.update_vlan		= &fm10k_update_vlan_pf,
 	.read_mac_addr		= &fm10k_read_mac_addr_pf,
 	.update_uc_addr		= &fm10k_update_uc_addr_pf,
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_type.h b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
index 2a17d82fa37d..bac8d486d75f 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_type.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
@@ -521,7 +521,6 @@ struct fm10k_mac_ops {
 	s32 (*stop_hw)(struct fm10k_hw *);
 	s32 (*get_bus_info)(struct fm10k_hw *);
 	s32 (*get_host_state)(struct fm10k_hw *, bool *);
-	bool (*is_slot_appropriate)(struct fm10k_hw *);
 	s32 (*update_vlan)(struct fm10k_hw *, u32, u8, bool);
 	s32 (*read_mac_addr)(struct fm10k_hw *);
 	s32 (*update_uc_addr)(struct fm10k_hw *, u16, const u8 *,
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_vf.c b/drivers/net/ethernet/intel/fm10k/fm10k_vf.c
index 94f0f6a146d9..36c8b0aa08fd 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_vf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_vf.c
@@ -131,19 +131,6 @@ static s32 fm10k_init_hw_vf(struct fm10k_hw *hw)
 	return 0;
 }
 
-/**
- *  fm10k_is_slot_appropriate_vf - Indicate appropriate slot for this SKU
- *  @hw: pointer to hardware structure
- *
- *  Looks at the PCIe bus info to confirm whether or not this slot can support
- *  the necessary bandwidth for this device. Since the VF has no control over
- *  the "slot" it is in, always indicate that the slot is appropriate.
- **/
-static bool fm10k_is_slot_appropriate_vf(struct fm10k_hw *hw)
-{
-	return true;
-}
-
 /* This structure defines the attibutes to be parsed below */
 const struct fm10k_tlv_attr fm10k_mac_vlan_msg_attr[] = {
 	FM10K_TLV_ATTR_U32(FM10K_MAC_VLAN_MSG_VLAN),
@@ -552,7 +539,6 @@ static struct fm10k_mac_ops mac_ops_vf = {
 	.init_hw		= &fm10k_init_hw_vf,
 	.start_hw		= &fm10k_start_hw_generic,
 	.stop_hw		= &fm10k_stop_hw_vf,
-	.is_slot_appropriate	= &fm10k_is_slot_appropriate_vf,
 	.update_vlan		= &fm10k_update_vlan_vf,
 	.read_mac_addr		= &fm10k_read_mac_addr_vf,
 	.update_uc_addr		= &fm10k_update_uc_addr_vf,
-- 
2.4.3

[Intel-wired-lan] [net-next 02/10] fm10k: allow creation of vlan interfaces even while down

[Jacob Keller]

We re-sync upon going up, so there is little reason to worry about not
syncing immediately with switch. This prevents an error that occurs if
you add a vlan interface while down.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index b2065cb44edc..e1ceb3a2e2cd 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -779,6 +779,12 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
 	if (!set && vid == hw->mac.default_vid)
 		return 0;
 
+	/* Do not throw an error if the interface is down. We will sync once
+	 * we come up
+	 */
+	if (test_bit(__FM10K_DOWN, &interface->state))
+		return 0;
+
 	fm10k_mbx_lock(interface);
 
 	/* only need to update the VLAN if not in promiscuous mode */
-- 
2.4.3

[Intel-wired-lan] [net-next 03/10] fm10k: don't store sw_vid at reset

[Jacob Keller]

If we store the sw_vid at reset of PF, then we accidentally prevent the
VF from receiving the message to update its default VID. This only
occurs if the VF is created before the PF has come up, which is the
standard way of creating VFs when using the module parameter.

This fixes an issue where we request the incorrect MAC/VLAN
combinations, and prevents us from accidentally reporting some frames as
vlan tagged.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
index 94571e6e790c..0e25a80417b9 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
@@ -228,9 +228,6 @@ int fm10k_iov_resume(struct pci_dev *pdev)
 		hw->iov.ops.set_lport(hw, vf_info, i,
 				      FM10K_VF_FLAG_MULTI_CAPABLE);
 
-		/* assign our default vid to the VF following reset */
-		vf_info->sw_vid = hw->mac.default_vid;
-
 		/* mailbox is disconnected so we don't send a message */
 		hw->iov.ops.assign_default_mac_vlan(hw, vf_info);
 
-- 
2.4.3

[Intel-wired-lan] [net-next 04/10] fm10k: TRIVIAL fix up ordering of __always_unused and style

[Jacob Keller]

Fix some style issues in debugfs code, and correct ordering of void and
__always_unused. Technically, the order does not matter, but preferred
style is to put the macro between the type and name.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
index f45b4d71adb8..08ecf43dffc7 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
@@ -37,7 +37,8 @@ static void *fm10k_dbg_desc_seq_start(struct seq_file *s, loff_t *pos)
 }
 
 static void *fm10k_dbg_desc_seq_next(struct seq_file *s,
-				     void __always_unused *v, loff_t *pos)
+				     void __always_unused *v,
+				     loff_t *pos)
 {
 	struct fm10k_ring *ring = s->private;
 
@@ -45,7 +46,7 @@ static void *fm10k_dbg_desc_seq_next(struct seq_file *s,
 }
 
 static void fm10k_dbg_desc_seq_stop(struct seq_file __always_unused *s,
-				    __always_unused void *v)
+				    void __always_unused *v)
 {
 	/* Do nothing. */
 }
-- 
2.4.3

[Intel-wired-lan] [net-next 05/10] fm10k: add support for extra debug statistics

[Jacob Keller]

Add a private ethtool flag to enable display of these statistics, which
are generally less useful. However, sometimes it can be useful for
debugging purposes. The most useful portion is the ability to see what
the PF thinks the VF mailboxes look like.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k.h         |  19 ++
 drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 224 +++++++++++++++++++----
 drivers/net/ethernet/intel/fm10k/fm10k_iov.c     |   5 +-
 drivers/net/ethernet/intel/fm10k/fm10k_main.c    |  15 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c     |  18 ++
 drivers/net/ethernet/intel/fm10k/fm10k_type.h    |   6 +
 6 files changed, 246 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index c8c8c5baefda..14440200499b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -101,12 +101,19 @@ struct fm10k_tx_queue_stats {
 	u64 csum_err;
 	u64 tx_busy;
 	u64 tx_done_old;
+	u64 csum_good;
 };
 
 struct fm10k_rx_queue_stats {
 	u64 alloc_failed;
 	u64 csum_err;
 	u64 errors;
+	u64 csum_good;
+	u64 switch_errors;
+	u64 drops;
+	u64 pp_errors;
+	u64 link_errors;
+	u64 length_errors;
 };
 
 struct fm10k_ring {
@@ -251,6 +258,7 @@ struct fm10k_intfc {
 #define FM10K_FLAG_RSS_FIELD_IPV6_UDP		(u32)(1 << 2)
 #define FM10K_FLAG_RX_TS_ENABLED		(u32)(1 << 3)
 #define FM10K_FLAG_SWPRI_CONFIG			(u32)(1 << 4)
+#define FM10K_FLAG_DEBUG_STATS			(u32)(1 << 5)
 	int xcast_mode;
 
 	/* Tx fast path data */
@@ -277,6 +285,17 @@ struct fm10k_intfc {
 	u64 rx_drops_nic;
 	u64 rx_overrun_pf;
 	u64 rx_overrun_vf;
+
+	/* Debug Statistics */
+	u64 hw_sm_mbx_full;
+	u64 hw_csum_tx_good;
+	u64 hw_csum_rx_good;
+	u64 rx_switch_errors;
+	u64 rx_drops;
+	u64 rx_pp_errors;
+	u64 rx_link_errors;
+	u64 rx_length_errors;
+
 	u32 tx_timeout_count;
 
 	/* RX */
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index c6dc9683429e..e8736d157d58 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -76,19 +76,20 @@ static const struct fm10k_stats fm10k_gstrings_global_stats[] = {
 	FM10K_STAT("mac_rules_used", hw.swapi.mac.used),
 	FM10K_STAT("mac_rules_avail", hw.swapi.mac.avail),
 
-	FM10K_STAT("mbx_tx_busy", hw.mbx.tx_busy),
-	FM10K_STAT("mbx_tx_oversized", hw.mbx.tx_dropped),
-	FM10K_STAT("mbx_tx_messages", hw.mbx.tx_messages),
-	FM10K_STAT("mbx_tx_dwords", hw.mbx.tx_dwords),
-	FM10K_STAT("mbx_rx_messages", hw.mbx.rx_messages),
-	FM10K_STAT("mbx_rx_dwords", hw.mbx.rx_dwords),
-	FM10K_STAT("mbx_rx_parse_err", hw.mbx.rx_parse_err),
-
 	FM10K_STAT("tx_hang_count", tx_timeout_count),
 
 	FM10K_STAT("tx_hwtstamp_timeouts", tx_hwtstamp_timeouts),
 };
 
+static const struct fm10k_stats fm10k_gstrings_debug_stats[] = {
+	FM10K_STAT("hw_sm_mbx_full", hw_sm_mbx_full),
+	FM10K_STAT("hw_csum_tx_good", hw_csum_tx_good),
+	FM10K_STAT("hw_csum_rx_good", hw_csum_rx_good),
+	FM10K_STAT("rx_pp_errors", rx_pp_errors),
+	FM10K_STAT("rx_link_errors", rx_link_errors),
+	FM10K_STAT("rx_length_errors", rx_length_errors),
+};
+
 static const struct fm10k_stats fm10k_gstrings_pf_stats[] = {
 	FM10K_STAT("timeout", stats.timeout.count),
 	FM10K_STAT("ur", stats.ur.count),
@@ -100,14 +101,33 @@ static const struct fm10k_stats fm10k_gstrings_pf_stats[] = {
 	FM10K_STAT("nodesc_drop", stats.nodesc_drop.count),
 };
 
+#define FM10K_MBX_STAT(_name, _stat) { \
+	.stat_string = _name, \
+	.sizeof_stat = FIELD_SIZEOF(struct fm10k_mbx_info, _stat), \
+	.stat_offset = offsetof(struct fm10k_mbx_info, _stat) \
+}
+
+static const struct fm10k_stats fm10k_gstrings_mbx_stats[] = {
+	FM10K_MBX_STAT("mbx_tx_busy", tx_busy),
+	FM10K_MBX_STAT("mbx_tx_oversized", tx_dropped),
+	FM10K_MBX_STAT("mbx_tx_messages", tx_messages),
+	FM10K_MBX_STAT("mbx_tx_dwords", tx_dwords),
+	FM10K_MBX_STAT("mbx_rx_messages", rx_messages),
+	FM10K_MBX_STAT("mbx_rx_dwords", rx_dwords),
+	FM10K_MBX_STAT("mbx_rx_parse_err", rx_parse_err),
+};
+
 #define FM10K_GLOBAL_STATS_LEN ARRAY_SIZE(fm10k_gstrings_global_stats)
+#define FM10K_DEBUG_STATS_LEN ARRAY_SIZE(fm10k_gstrings_debug_stats)
 #define FM10K_PF_STATS_LEN ARRAY_SIZE(fm10k_gstrings_pf_stats)
+#define FM10K_MBX_STATS_LEN ARRAY_SIZE(fm10k_gstrings_mbx_stats)
 
 #define FM10K_QUEUE_STATS_LEN(_n) \
 	( (_n) * 2 * (sizeof(struct fm10k_queue_stats) / sizeof(u64)))
 
 #define FM10K_STATIC_STATS_LEN (FM10K_GLOBAL_STATS_LEN + \
-				FM10K_NETDEV_STATS_LEN)
+				FM10K_NETDEV_STATS_LEN + \
+				FM10K_MBX_STATS_LEN)
 
 static const char fm10k_gstrings_test[][ETH_GSTRING_LEN] = {
 	"Mailbox test (on/offline)"
@@ -120,11 +140,85 @@ enum fm10k_self_test_types {
 	FM10K_TEST_MAX = FM10K_TEST_LEN
 };
 
-static void fm10k_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+enum {
+	FM10K_PRV_FLAG_DEBUG_STATS,
+	FM10K_PRV_FLAG_LEN,
+};
+
+static const char fm10k_prv_flags[FM10K_PRV_FLAG_LEN][ETH_GSTRING_LEN] = {
+	"debug-statistics",
+};
+
+static void fm10k_get_stat_strings(struct net_device *dev, u8 *data)
 {
 	struct fm10k_intfc *interface = netdev_priv(dev);
+	struct fm10k_iov_data *iov_data = interface->iov_data;
 	char *p = (char *)data;
 	unsigned int i;
+	unsigned int j;
+
+	for (i = 0; i < FM10K_NETDEV_STATS_LEN; i++) {
+		memcpy(p, fm10k_gstrings_net_stats[i].stat_string,
+		       ETH_GSTRING_LEN);
+		p += ETH_GSTRING_LEN;
+	}
+
+	for (i = 0; i < FM10K_GLOBAL_STATS_LEN; i++) {
+		memcpy(p, fm10k_gstrings_global_stats[i].stat_string,
+		       ETH_GSTRING_LEN);
+		p += ETH_GSTRING_LEN;
+	}
+
+	if (interface->flags & FM10K_FLAG_DEBUG_STATS) {
+		for (i = 0; i < FM10K_DEBUG_STATS_LEN; i++) {
+			memcpy(p, fm10k_gstrings_debug_stats[i].stat_string,
+			       ETH_GSTRING_LEN);
+			p += ETH_GSTRING_LEN;
+		}
+	}
+
+	for (i = 0; i < FM10K_MBX_STATS_LEN; i++) {
+		memcpy(p, fm10k_gstrings_mbx_stats[i].stat_string,
+		       ETH_GSTRING_LEN);
+		p += ETH_GSTRING_LEN;
+	}
+
+	if (interface->hw.mac.type != fm10k_mac_vf) {
+		for (i = 0; i < FM10K_PF_STATS_LEN; i++) {
+			memcpy(p, fm10k_gstrings_pf_stats[i].stat_string,
+			       ETH_GSTRING_LEN);
+			p += ETH_GSTRING_LEN;
+		}
+	}
+
+	if ((interface->flags & FM10K_FLAG_DEBUG_STATS) && iov_data) {
+		for (i = 0; i < iov_data->num_vfs; i++) {
+			for (j = 0; j < FM10K_MBX_STATS_LEN; j++) {
+				snprintf(p,
+					 ETH_GSTRING_LEN,
+					 "vf_%u_%s", i,
+					 fm10k_gstrings_mbx_stats[j].stat_string);
+				p += ETH_GSTRING_LEN;
+			}
+		}
+	}
+
+	for (i = 0; i < interface->hw.mac.max_queues; i++) {
+		sprintf(p, "tx_queue_%u_packets", i);
+		p += ETH_GSTRING_LEN;
+		sprintf(p, "tx_queue_%u_bytes", i);
+		p += ETH_GSTRING_LEN;
+		sprintf(p, "rx_queue_%u_packets", i);
+		p += ETH_GSTRING_LEN;
+		sprintf(p, "rx_queue_%u_bytes", i);
+		p += ETH_GSTRING_LEN;
+	}
+}
+
+static void fm10k_get_strings(struct net_device *dev,
+			      u32 stringset, u8 *data)
+{
+	char *p = (char *)data;
 
 	switch (stringset) {
 	case ETH_SS_TEST:
@@ -132,35 +226,11 @@ static void fm10k_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 		       FM10K_TEST_LEN * ETH_GSTRING_LEN);
 		break;
 	case ETH_SS_STATS:
-		for (i = 0; i < FM10K_NETDEV_STATS_LEN; i++) {
-			memcpy(p, fm10k_gstrings_net_stats[i].stat_string,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
-		for (i = 0; i < FM10K_GLOBAL_STATS_LEN; i++) {
-			memcpy(p, fm10k_gstrings_global_stats[i].stat_string,
-			       ETH_GSTRING_LEN);
-			p += ETH_GSTRING_LEN;
-		}
-
-		if (interface->hw.mac.type != fm10k_mac_vf) {
-			for (i = 0; i < FM10K_PF_STATS_LEN; i++) {
-				memcpy(p, fm10k_gstrings_pf_stats[i].stat_string,
-				       ETH_GSTRING_LEN);
-				p += ETH_GSTRING_LEN;
-			}
-		}
-
-		for (i = 0; i < interface->hw.mac.max_queues; i++) {
-			sprintf(p, "tx_queue_%u_packets", i);
-			p += ETH_GSTRING_LEN;
-			sprintf(p, "tx_queue_%u_bytes", i);
-			p += ETH_GSTRING_LEN;
-			sprintf(p, "rx_queue_%u_packets", i);
-			p += ETH_GSTRING_LEN;
-			sprintf(p, "rx_queue_%u_bytes", i);
-			p += ETH_GSTRING_LEN;
-		}
+		fm10k_get_stat_strings(dev, data);
+		break;
+	case ETH_SS_PRIV_FLAGS:
+		memcpy(p, fm10k_prv_flags,
+		       FM10K_PRV_FLAG_LEN * ETH_GSTRING_LEN);
 		break;
 	}
 }
@@ -168,6 +238,7 @@ static void fm10k_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 static int fm10k_get_sset_count(struct net_device *dev, int sset)
 {
 	struct fm10k_intfc *interface = netdev_priv(dev);
+	struct fm10k_iov_data *iov_data = interface->iov_data;
 	struct fm10k_hw *hw = &interface->hw;
 	int stats_len = FM10K_STATIC_STATS_LEN;
 
@@ -180,7 +251,16 @@ static int fm10k_get_sset_count(struct net_device *dev, int sset)
 		if (hw->mac.type != fm10k_mac_vf)
 			stats_len += FM10K_PF_STATS_LEN;
 
+		if (interface->flags & FM10K_FLAG_DEBUG_STATS) {
+			stats_len += FM10K_DEBUG_STATS_LEN;
+
+			if (iov_data)
+				stats_len += FM10K_MBX_STATS_LEN * iov_data->num_vfs;
+		}
+
 		return stats_len;
+	case ETH_SS_PRIV_FLAGS:
+		return FM10K_PRV_FLAG_LEN;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -192,6 +272,7 @@ static void fm10k_get_ethtool_stats(struct net_device *netdev,
 {
 	const int stat_count = sizeof(struct fm10k_queue_stats) / sizeof(u64);
 	struct fm10k_intfc *interface = netdev_priv(netdev);
+	struct fm10k_iov_data *iov_data = interface->iov_data;
 	struct net_device_stats *net_stats = &netdev->stats;
 	char *p;
 	int i, j;
@@ -211,13 +292,47 @@ static void fm10k_get_ethtool_stats(struct net_device *netdev,
 			sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
 	}
 
-	if (interface->hw.mac.type != fm10k_mac_vf)
+	if (interface->flags & FM10K_FLAG_DEBUG_STATS) {
+		for (i = 0; i < FM10K_DEBUG_STATS_LEN; i++) {
+			p = (char *)interface + fm10k_gstrings_debug_stats[i].stat_offset;
+			*(data++) = (fm10k_gstrings_debug_stats[i].sizeof_stat ==
+				     sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
+		}
+	}
+
+	for (i = 0; i < FM10K_MBX_STATS_LEN; i++) {
+		p = (char *)&interface->hw.mbx + fm10k_gstrings_mbx_stats[i].stat_offset;
+		*(data++) = (fm10k_gstrings_mbx_stats[i].sizeof_stat ==
+			sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
+	}
+
+	if (interface->hw.mac.type != fm10k_mac_vf) {
 		for (i = 0; i < FM10K_PF_STATS_LEN; i++) {
 			p = (char *)interface +
 			    fm10k_gstrings_pf_stats[i].stat_offset;
 			*(data++) = (fm10k_gstrings_pf_stats[i].sizeof_stat ==
 				     sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
 		}
+	}
+
+	if ((interface->flags & FM10K_FLAG_DEBUG_STATS) && iov_data) {
+		for (i = 0; i < iov_data->num_vfs; i++) {
+			struct fm10k_vf_info *vf_info;
+			vf_info = &iov_data->vf_info[i];
+
+			/* skip stats if we don't have a vf info */
+			if (!vf_info) {
+				data += FM10K_MBX_STATS_LEN;
+				continue;
+			}
+
+			for (j = 0; j < FM10K_MBX_STATS_LEN; j++) {
+				p = (char *)&vf_info->mbx + fm10k_gstrings_mbx_stats[j].stat_offset;
+				*(data++) = (fm10k_gstrings_mbx_stats[j].sizeof_stat ==
+					     sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
+			}
+		}
+	}
 
 	for (i = 0; i < interface->hw.mac.max_queues; i++) {
 		struct fm10k_ring *ring;
@@ -881,6 +996,33 @@ static void fm10k_self_test(struct net_device *dev,
 		eth_test->flags |= ETH_TEST_FL_FAILED;
 }
 
+static u32 fm10k_get_priv_flags(struct net_device *netdev)
+{
+	struct fm10k_intfc *interface = netdev_priv(netdev);
+	u32 priv_flags = 0;
+
+	if (interface->flags & FM10K_FLAG_DEBUG_STATS)
+		priv_flags |= 1 << FM10K_PRV_FLAG_DEBUG_STATS;
+
+	return priv_flags;
+}
+
+static int fm10k_set_priv_flags(struct net_device *netdev, u32 priv_flags)
+{
+	struct fm10k_intfc *interface = netdev_priv(netdev);
+
+	if (priv_flags >= (1 << FM10K_PRV_FLAG_LEN))
+		return -EINVAL;
+
+	if (priv_flags & (1 << FM10K_PRV_FLAG_DEBUG_STATS))
+		interface->flags |= FM10K_FLAG_DEBUG_STATS;
+	else
+		interface->flags &= ~FM10K_FLAG_DEBUG_STATS;
+
+	return 0;
+}
+
+
 static u32 fm10k_get_reta_size(struct net_device __always_unused *netdev)
 {
 	return FM10K_RETA_SIZE * FM10K_RETA_ENTRIES_PER_REG;
@@ -1094,6 +1236,8 @@ static const struct ethtool_ops fm10k_ethtool_ops = {
 	.get_regs               = fm10k_get_regs,
 	.get_regs_len           = fm10k_get_regs_len,
 	.self_test		= fm10k_self_test,
+	.get_priv_flags		= fm10k_get_priv_flags,
+	.set_priv_flags		= fm10k_set_priv_flags,
 	.get_rxfh_indir_size	= fm10k_get_reta_size,
 	.get_rxfh_key_size	= fm10k_get_rssrk_size,
 	.get_rxfh		= fm10k_get_rssh,
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
index 0e25a80417b9..acfb8b1f88a7 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
@@ -137,8 +137,11 @@ process_mbx:
 		}
 
 		/* guarantee we have free space in the SM mailbox */
-		if (!hw->mbx.ops.tx_ready(&hw->mbx, FM10K_VFMBX_MSG_MTU))
+		if (!hw->mbx.ops.tx_ready(&hw->mbx, FM10K_VFMBX_MSG_MTU)) {
+			/* keep track of how many times this occurs */
+			interface->hw_sm_mbx_full++;
 			break;
+		}
 
 		/* cleanup mailbox and process received messages */
 		mbx->ops.process(hw, mbx);
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 620ff5e9dc59..079772b5d130 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -398,6 +398,8 @@ static inline void fm10k_rx_checksum(struct fm10k_ring *ring,
 		return;
 
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	ring->rx_stats.csum_good++;
 }
 
 #define FM10K_RSS_L4_TYPES_MASK \
@@ -553,6 +555,18 @@ static bool fm10k_cleanup_headers(struct fm10k_ring *rx_ring,
 {
 	if (unlikely((fm10k_test_staterr(rx_desc,
 					 FM10K_RXD_STATUS_RXE)))) {
+#define FM10K_TEST_RXD_BIT(rxd, bit) \
+	((rxd)->w.csum_err & cpu_to_le16(bit))
+		if (FM10K_TEST_RXD_BIT(rx_desc, FM10K_RXD_ERR_SWITCH_ERROR))
+			rx_ring->rx_stats.switch_errors++;
+		if (FM10K_TEST_RXD_BIT(rx_desc, FM10K_RXD_ERR_NO_DESCRIPTOR))
+			rx_ring->rx_stats.drops++;
+		if (FM10K_TEST_RXD_BIT(rx_desc, FM10K_RXD_ERR_PP_ERROR))
+			rx_ring->rx_stats.pp_errors++;
+		if (FM10K_TEST_RXD_BIT(rx_desc, FM10K_RXD_ERR_SWITCH_READY))
+			rx_ring->rx_stats.link_errors++;
+		if (FM10K_TEST_RXD_BIT(rx_desc, FM10K_RXD_ERR_TOO_BIG))
+			rx_ring->rx_stats.length_errors++;
 		dev_kfree_skb_any(skb);
 		rx_ring->rx_stats.errors++;
 		return true;
@@ -878,6 +892,7 @@ static void fm10k_tx_csum(struct fm10k_ring *tx_ring,
 
 	/* update TX checksum flag */
 	first->tx_flags |= FM10K_TX_FLAGS_CSUM;
+	tx_ring->tx_stats.csum_good++;
 
 no_csum:
 	/* populate Tx descriptor header size and mss */
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 8408d7ba46e8..6d1364393a8b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -328,6 +328,9 @@ void fm10k_update_stats(struct fm10k_intfc *interface)
 {
 	struct net_device_stats *net_stats = &interface->netdev->stats;
 	struct fm10k_hw *hw = &interface->hw;
+	u64 hw_csum_tx_good = 0, hw_csum_rx_good = 0, rx_length_errors = 0;
+	u64 rx_switch_errors = 0, rx_drops = 0, rx_pp_errors = 0;
+	u64 rx_link_errors = 0;
 	u64 rx_errors = 0, rx_csum_errors = 0, tx_csum_errors = 0;
 	u64 restart_queue = 0, tx_busy = 0, alloc_failed = 0;
 	u64 rx_bytes_nic = 0, rx_pkts_nic = 0, rx_drops_nic = 0;
@@ -347,6 +350,7 @@ void fm10k_update_stats(struct fm10k_intfc *interface)
 		tx_csum_errors += tx_ring->tx_stats.csum_err;
 		bytes += tx_ring->stats.bytes;
 		pkts += tx_ring->stats.packets;
+		hw_csum_tx_good += tx_ring->tx_stats.csum_good;
 	}
 
 	interface->restart_queue = restart_queue;
@@ -354,6 +358,8 @@ void fm10k_update_stats(struct fm10k_intfc *interface)
 	net_stats->tx_bytes = bytes;
 	net_stats->tx_packets = pkts;
 	interface->tx_csum_errors = tx_csum_errors;
+	interface->hw_csum_tx_good = hw_csum_tx_good;
+
 	/* gather some stats to the interface struct that are per queue */
 	for (bytes = 0, pkts = 0, i = 0; i < interface->num_rx_queues; i++) {
 		struct fm10k_ring *rx_ring = interface->rx_ring[i];
@@ -363,12 +369,24 @@ void fm10k_update_stats(struct fm10k_intfc *interface)
 		alloc_failed += rx_ring->rx_stats.alloc_failed;
 		rx_csum_errors += rx_ring->rx_stats.csum_err;
 		rx_errors += rx_ring->rx_stats.errors;
+		hw_csum_rx_good += rx_ring->rx_stats.csum_good;
+		rx_switch_errors += rx_ring->rx_stats.switch_errors;
+		rx_drops += rx_ring->rx_stats.drops;
+		rx_pp_errors += rx_ring->rx_stats.pp_errors;
+		rx_link_errors += rx_ring->rx_stats.link_errors;
+		rx_length_errors += rx_ring->rx_stats.length_errors;
 	}
 
 	net_stats->rx_bytes = bytes;
 	net_stats->rx_packets = pkts;
 	interface->alloc_failed = alloc_failed;
 	interface->rx_csum_errors = rx_csum_errors;
+	interface->hw_csum_rx_good = hw_csum_rx_good;
+	interface->rx_switch_errors = rx_switch_errors;
+	interface->rx_drops = rx_drops;
+	interface->rx_pp_errors = rx_pp_errors;
+	interface->rx_link_errors = rx_link_errors;
+	interface->rx_length_errors = rx_length_errors;
 
 	hw->mac.ops.update_hw_stats(hw, &interface->stats);
 
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_type.h b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
index bac8d486d75f..318a212f0a78 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_type.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_type.h
@@ -762,6 +762,12 @@ enum fm10k_rxdesc_xc {
 #define FM10K_RXD_STATUS_L4E		0x4000 /* L4 csum error */
 #define FM10K_RXD_STATUS_IPE		0x8000 /* IPv4 csum error */
 
+#define FM10K_RXD_ERR_SWITCH_ERROR	0x0001 /* Switch found bad packet */
+#define FM10K_RXD_ERR_NO_DESCRIPTOR	0x0002 /* No descriptor available */
+#define FM10K_RXD_ERR_PP_ERROR		0x0004 /* RAM error during processing */
+#define FM10K_RXD_ERR_SWITCH_READY	0x0008 /* Link transition mid-packet */
+#define FM10K_RXD_ERR_TOO_BIG		0x0010 /* Pkt too big for single buf */
+
 struct fm10k_ftag {
 	__be16 swpri_type_user;
 	__be16 vlan;
-- 
2.4.3

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Jacob Keller]

This patch ensures that vlan traffic on the default vid will go to the
corresponding vlan device if it exists. To do this, mask the rx_ring vid
if we have an active vlan on that vid.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index e1ceb3a2e2cd..c173c79373fe 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
 	struct fm10k_intfc *interface = netdev_priv(netdev);
 	struct fm10k_hw *hw = &interface->hw;
 	s32 err;
+	int i;
 
 	/* updates do not apply to VLAN 0 */
 	if (!vid)
@@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
 	if (!set)
 		clear_bit(vid, interface->active_vlans);
 
+	/* disable the default vid on ring if we have an active vlan */
+	for (i = 0; i < interface->num_rx_queues; i++) {
+		struct fm10k_ring *rx_ring = interface->rx_ring[i];
+		u16 rx_vid = rx_ring->vid & (VLAN_N_VID - 1);
+
+		if (test_bit(rx_vid, interface->active_vlans))
+			rx_ring->vid |= FM10K_VLAN_CLEAR;
+		else
+			rx_ring->vid &= ~FM10K_VLAN_CLEAR;
+	}
+
 	/* Do not remove default VID related entries from VLAN and MAC tables */
 	if (!set && vid == hw->mac.default_vid)
 		return 0;
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 6d1364393a8b..989981ea3c64 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,
 	/* assign default VLAN to queue */
 	ring->vid = hw->mac.default_vid;
 
+	/* if we have an active VLAN, disable default vid */
+	if (test_bit(hw->mac.default_vid, interface->active_vlans))
+		ring->vid |= FM10K_VLAN_CLEAR;
+
 	/* Map interrupt */
 	if (ring->q_vector) {
 		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
-- 
2.4.3

[Intel-wired-lan] [net-next 07/10] fm10k: TRIVIAL fix typo in fm10k_netdev.c

[Jacob Keller]

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index c173c79373fe..79c0fa2646e7 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1048,7 +1048,7 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface)
 					   vid, true, 0);
 	}
 
-	/* update xcast mode before syncronizing addresses */
+	/* update xcast mode before synchronizing addresses */
 	hw->mac.ops.update_xcast_mode(hw, glort, xcast_mode);
 
 	/* synchronize all of the addresses */
-- 
2.4.3

[Intel-wired-lan] [net-next 08/10] fm10k: re-enable VF after a full reset on detection of a Malicious event

[Jacob Keller]

Modify behavior of Malicious Driver Detection events. Presently, the
hardware disables the VF queues and re-assigns them to the PF. This
causes the VF in question to continuously Tx hang, because it assumes
that it can transmit over the queues in question. For transient events,
this results in continuous logging of malicious events.

New behavior is to reset the LPORT and VF state, so that the VF will
have to reset and re-enable itself. This does mean that malicious VFs
will possibly be able to continue and attempt malicious events again.
However, it is expected that system administrators will step in and
manually remove or disable the VF in question.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 30 ++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 989981ea3c64..1d788085b57a 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -896,10 +896,12 @@ void fm10k_netpoll(struct net_device *netdev)
 
 #endif
 #define FM10K_ERR_MSG(type) case (type): error = #type; break
-static void fm10k_print_fault(struct fm10k_intfc *interface, int type,
+static void fm10k_handle_fault(struct fm10k_intfc *interface, int type,
 			      struct fm10k_fault *fault)
 {
 	struct pci_dev *pdev = interface->pdev;
+	struct fm10k_hw *hw = &interface->hw;
+	struct fm10k_iov_data *iov_data = interface->iov_data;
 	char *error;
 
 	switch (type) {
@@ -953,6 +955,30 @@ static void fm10k_print_fault(struct fm10k_intfc *interface, int type,
 		 "%s Address: 0x%llx SpecInfo: 0x%x Func: %02x.%0x\n",
 		 error, fault->address, fault->specinfo,
 		 PCI_SLOT(fault->func), PCI_FUNC(fault->func));
+
+	/* For VF faults, clear out the respective LPORT, reset the queue
+	 * resources, and then reconnect to the mailbox. This allows the
+	 * VF in question to resume behavior. For transient faults that are
+	 * the result of non-malicious behavior this will log the fault and
+	 * allow the VF to resume functionality. Obviously for malicious VFs
+	 * they will be able to attempt malicious behavior again. In this
+	 * case, the system administrator will need to step in and manually
+	 * remove or disable the VF in question.
+	 */
+	if (fault->func && iov_data) {
+		int vf = fault->func - 1;
+		struct fm10k_vf_info *vf_info = &iov_data->vf_info[vf];
+
+		hw->iov.ops.reset_lport(hw, vf_info);
+		hw->iov.ops.reset_resources(hw, vf_info);
+
+		/* reset_lport disables the VF, so re-enable it */
+		hw->iov.ops.set_lport(hw, vf_info, vf,
+				      FM10K_VF_FLAG_MULTI_CAPABLE);
+
+		/* reset_resources will disconnect from the mbx  */
+		vf_info->mbx.ops.connect(hw, &vf_info->mbx);
+	}
 }
 
 static void fm10k_report_fault(struct fm10k_intfc *interface, u32 eicr)
@@ -976,7 +1002,7 @@ static void fm10k_report_fault(struct fm10k_intfc *interface, u32 eicr)
 			continue;
 		}
 
-		fm10k_print_fault(interface, type, &fault);
+		fm10k_handle_fault(interface, type, &fault);
 	}
 }
 
-- 
2.4.3

[Intel-wired-lan] [net-next 09/10] fm10k: Only trigger data path reset if fabric is up

[Jacob Keller]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change makes it so that we only trigger the data path reset if the
fabric is ready to handle traffic.  The general idea is to avoid
triggering the reset unless the switch API is ready for us.  Otherwise
we can just postpone the reset until we receive a switch ready
notification.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
index 241b969626bc..d806d87a6192 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
@@ -59,6 +59,11 @@ static s32 fm10k_reset_hw_pf(struct fm10k_hw *hw)
 	if (reg & (FM10K_DMA_CTRL_TX_ACTIVE | FM10K_DMA_CTRL_RX_ACTIVE))
 		return FM10K_ERR_DMA_PENDING;
 
+	/* verify the switch is ready for reset */
+	reg = fm10k_read_reg(hw, FM10K_DMA_CTRL2);
+	if (!(reg & FM10K_DMA_CTRL2_SWITCH_READY))
+		goto out;
+
 	/* Inititate data path reset */
 	reg |= FM10K_DMA_CTRL_DATAPATH_RESET;
 	fm10k_write_reg(hw, FM10K_DMA_CTRL, reg);
@@ -72,6 +77,7 @@ static s32 fm10k_reset_hw_pf(struct fm10k_hw *hw)
 	if (!(reg & FM10K_IP_NOTINRESET))
 		err = FM10K_ERR_RESET_FAILED;
 
+out:
 	return err;
 }
 
-- 
2.4.3

[Intel-wired-lan] [net-next 10/10] fm10k: fix iov_msg_mac_vlan_pf VID checks

[Jacob Keller]

The VF will send a message to request multicast addresses with the
default vid. In the current code, if the PF has statically assigned a
VLAN to a VF, then the VF will not get the multicast addresses. Fix up
all of the various vlan messages to use identical checks (since each
check was different). Also use set as a variable, so that it simplifies
our check for whether vlan matches the pf_vid.

The new logic will allow set of a vlan if it is zero, automatically
converting to the default vid. Otherwise it will allow setting the PF
vid, or any VLAN if PF has not statically assigned a VLAN. This is
consistent behavior, and allows VF to request either 0 or the
default_vid without silently failing.

Note that we need the check for zero since VFs might not get the default
VID message in time to actually request non-zero VLANs.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 37 +++++++++++++++++++----------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
index d806d87a6192..9bb57531b5db 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
@@ -1168,9 +1168,10 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
 			      struct fm10k_mbx_info *mbx)
 {
 	struct fm10k_vf_info *vf_info = (struct fm10k_vf_info *)mbx;
-	int err = 0;
 	u8 mac[ETH_ALEN];
 	u32 *result;
+	int err = 0;
+	bool set;
 	u16 vlan;
 	u32 vid;
 
@@ -1186,19 +1187,25 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
 		if (err)
 			return err;
 
+		/* verify upper 16 bits are zero */
+		if (vid >> 16)
+			return FM10K_ERR_PARAM;
+
+		set = !(vid & FM10K_VLAN_CLEAR);
+		vid &= ~FM10K_VLAN_CLEAR;
+
 		/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
-		if (!vid || (vid == FM10K_VLAN_CLEAR)) {
+		if (!vid) {
 			if (vf_info->pf_vid)
 				vid |= vf_info->pf_vid;
 			else
 				vid |= vf_info->sw_vid;
-		} else if (vid != vf_info->pf_vid) {
+		} else if (vf_info->pf_vid && vid != vf_info->pf_vid) {
 			return FM10K_ERR_PARAM;
 		}
 
 		/* update VSI info for VF in regards to VLAN table */
-		err = hw->mac.ops.update_vlan(hw, vid, vf_info->vsi,
-					      !(vid & FM10K_VLAN_CLEAR));
+		err = hw->mac.ops.update_vlan(hw, vid, vf_info->vsi, set);
 	}
 
 	if (!err && !!results[FM10K_MAC_VLAN_MSG_MAC]) {
@@ -1214,19 +1221,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
 		    memcmp(mac, vf_info->mac, ETH_ALEN))
 			return FM10K_ERR_PARAM;
 
+		set = !(vlan & FM10K_VLAN_CLEAR);
+		vlan &= ~FM10K_VLAN_CLEAR;
+
 		/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
-		if (!vlan || (vlan == FM10K_VLAN_CLEAR)) {
+		if (!vlan) {
 			if (vf_info->pf_vid)
 				vlan |= vf_info->pf_vid;
 			else
 				vlan |= vf_info->sw_vid;
-		} else if (vf_info->pf_vid) {
+		} else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {
 			return FM10K_ERR_PARAM;
 		}
 
 		/* notify switch of request for new unicast address */
-		err = hw->mac.ops.update_uc_addr(hw, vf_info->glort, mac, vlan,
-						 !(vlan & FM10K_VLAN_CLEAR), 0);
+		err = hw->mac.ops.update_uc_addr(hw, vf_info->glort,
+						 mac, vlan, set, 0);
 	}
 
 	if (!err && !!results[FM10K_MAC_VLAN_MSG_MULTICAST]) {
@@ -1241,19 +1251,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
 		if (!(vf_info->vf_flags & FM10K_VF_FLAG_MULTI_ENABLED))
 			return FM10K_ERR_PARAM;
 
+		set = !(vlan & FM10K_VLAN_CLEAR);
+		vlan &= ~FM10K_VLAN_CLEAR;
+
 		/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
 		if (!vlan || (vlan == FM10K_VLAN_CLEAR)) {
 			if (vf_info->pf_vid)
 				vlan |= vf_info->pf_vid;
 			else
 				vlan |= vf_info->sw_vid;
-		} else if (vf_info->pf_vid) {
+		} else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {
 			return FM10K_ERR_PARAM;
 		}
 
 		/* notify switch of request for new multicast address */
-		err = hw->mac.ops.update_mc_addr(hw, vf_info->glort, mac, vlan,
-						 !(vlan & FM10K_VLAN_CLEAR));
+		err = hw->mac.ops.update_mc_addr(hw, vf_info->glort,
+						 mac, vlan, set);
 	}
 
 	return err;
-- 
2.4.3

[Intel-wired-lan] [net-next 03/10] fm10k: don't store sw_vid at reset

[Alexander Duyck]

On 06/19/2015 04:37 PM, Jacob Keller wrote:
> If we store the sw_vid at reset of PF, then we accidentally prevent the
> VF from receiving the message to update its default VID. This only
> occurs if the VF is created before the PF has come up, which is the
> standard way of creating VFs when using the module parameter.

The upstream driver doesn't have a module parameter for VFs.  It uses 
sysfs.  It isn't clear but can this issue occur if you use the sysfs?

> This fixes an issue where we request the incorrect MAC/VLAN
> combinations, and prevents us from accidentally reporting some frames as
> vlan tagged.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>   drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
> index 94571e6e790c..0e25a80417b9 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
> @@ -228,9 +228,6 @@ int fm10k_iov_resume(struct pci_dev *pdev)
>   		hw->iov.ops.set_lport(hw, vf_info, i,
>   				      FM10K_VF_FLAG_MULTI_CAPABLE);
>   
> -		/* assign our default vid to the VF following reset */
> -		vf_info->sw_vid = hw->mac.default_vid;
> -
>   		/* mailbox is disconnected so we don't send a message */
>   		hw->iov.ops.assign_default_mac_vlan(hw, vf_info);
>   

The patch itself looks fine.  I assume the Switch API will send a 
message to force us onto the correct VID if there is one already 
reserved for this VF.

I probably shouldn't have used the PF VLAN ID for the VFs anyway. Just 
letting the Switch API bounce a message through is a much better way to go.

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Alexander Duyck]

On 06/19/2015 04:37 PM, Jacob Keller wrote:
> This patch ensures that vlan traffic on the default vid will go to the
> corresponding vlan device if it exists. To do this, mask the rx_ring vid
> if we have an active vlan on that vid.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>   drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++
>   drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |  4 ++++
>   2 files changed, 16 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> index e1ceb3a2e2cd..c173c79373fe 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> @@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
>   	struct fm10k_intfc *interface = netdev_priv(netdev);
>   	struct fm10k_hw *hw = &interface->hw;
>   	s32 err;
> +	int i;
>
>   	/* updates do not apply to VLAN 0 */
>   	if (!vid)
> @@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
>   	if (!set)
>   		clear_bit(vid, interface->active_vlans);
>
> +	/* disable the default vid on ring if we have an active vlan */
> +	for (i = 0; i < interface->num_rx_queues; i++) {
> +		struct fm10k_ring *rx_ring = interface->rx_ring[i];
> +		u16 rx_vid = rx_ring->vid & (VLAN_N_VID - 1);
> +
> +		if (test_bit(rx_vid, interface->active_vlans))
> +			rx_ring->vid |= FM10K_VLAN_CLEAR;
> +		else
> +			rx_ring->vid &= ~FM10K_VLAN_CLEAR;
> +	}
> +
>   	/* Do not remove default VID related entries from VLAN and MAC tables */
>   	if (!set && vid == hw->mac.default_vid)
>   		return 0;
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> index 6d1364393a8b..989981ea3c64 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> @@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,
>   	/* assign default VLAN to queue */
>   	ring->vid = hw->mac.default_vid;
>
> +	/* if we have an active VLAN, disable default vid */
> +	if (test_bit(hw->mac.default_vid, interface->active_vlans))
> +		ring->vid |= FM10K_VLAN_CLEAR;
> +
>   	/* Map interrupt */
>   	if (ring->q_vector) {
>   		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
>

This doesn't quite work since the rx_ring->vid is doing a full 16b 
comparison against a value that includes the QOS value.

I just realized that is a bug in the fm10k_process_skb_fields function. 
  You probably need to update it so that code does something more like

if (rx_desc->w.vlan) {
	u16 vid = le16_to_cpu(rx_desc->w.vlan);

	if ((vid & VLAN_VID_MASK) != rx_ring->vid)
		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
	else if (vid & VLAN_PRIO_MASK)
		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
				       vid & VLAN_PRIO_MASK);
}	

[Intel-wired-lan] [net-next 10/10] fm10k: fix iov_msg_mac_vlan_pf VID checks

[Alexander Duyck]

On 06/19/2015 04:37 PM, Jacob Keller wrote:
> The VF will send a message to request multicast addresses with the
> default vid. In the current code, if the PF has statically assigned a
> VLAN to a VF, then the VF will not get the multicast addresses. Fix up
> all of the various vlan messages to use identical checks (since each
> check was different). Also use set as a variable, so that it simplifies
> our check for whether vlan matches the pf_vid.
>
> The new logic will allow set of a vlan if it is zero, automatically
> converting to the default vid. Otherwise it will allow setting the PF
> vid, or any VLAN if PF has not statically assigned a VLAN. This is
> consistent behavior, and allows VF to request either 0 or the
> default_vid without silently failing.
>
> Note that we need the check for zero since VFs might not get the default
> VID message in time to actually request non-zero VLANs.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>   drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 37 +++++++++++++++++++----------
>   1 file changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
> index d806d87a6192..9bb57531b5db 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
> @@ -1168,9 +1168,10 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
>   			      struct fm10k_mbx_info *mbx)
>   {
>   	struct fm10k_vf_info *vf_info = (struct fm10k_vf_info *)mbx;
> -	int err = 0;
>   	u8 mac[ETH_ALEN];
>   	u32 *result;
> +	int err = 0;
> +	bool set;
>   	u16 vlan;
>   	u32 vid;
>
> @@ -1186,19 +1187,25 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
>   		if (err)
>   			return err;
>
> +		/* verify upper 16 bits are zero */
> +		if (vid >> 16)
> +			return FM10K_ERR_PARAM;
> +
> +		set = !(vid & FM10K_VLAN_CLEAR);
> +		vid &= ~FM10K_VLAN_CLEAR;
> +
>   		/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
> -		if (!vid || (vid == FM10K_VLAN_CLEAR)) {
> +		if (!vid) {
>   			if (vf_info->pf_vid)
>   				vid |= vf_info->pf_vid;
>   			else
>   				vid |= vf_info->sw_vid;
> -		} else if (vid != vf_info->pf_vid) {
> +		} else if (vf_info->pf_vid && vid != vf_info->pf_vid) {
>   			return FM10K_ERR_PARAM;
>   		}
>
>   		/* update VSI info for VF in regards to VLAN table */
> -		err = hw->mac.ops.update_vlan(hw, vid, vf_info->vsi,
> -					      !(vid & FM10K_VLAN_CLEAR));
> +		err = hw->mac.ops.update_vlan(hw, vid, vf_info->vsi, set);
>   	}
>
>   	if (!err && !!results[FM10K_MAC_VLAN_MSG_MAC]) {
> @@ -1214,19 +1221,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
>   		    memcmp(mac, vf_info->mac, ETH_ALEN))
>   			return FM10K_ERR_PARAM;
>
> +		set = !(vlan & FM10K_VLAN_CLEAR);
> +		vlan &= ~FM10K_VLAN_CLEAR;
> +
>   		/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
> -		if (!vlan || (vlan == FM10K_VLAN_CLEAR)) {
> +		if (!vlan) {
>   			if (vf_info->pf_vid)
>   				vlan |= vf_info->pf_vid;
>   			else
>   				vlan |= vf_info->sw_vid;
> -		} else if (vf_info->pf_vid) {
> +		} else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {
>   			return FM10K_ERR_PARAM;
>   		}
>
>   		/* notify switch of request for new unicast address */
> -		err = hw->mac.ops.update_uc_addr(hw, vf_info->glort, mac, vlan,
> -						 !(vlan & FM10K_VLAN_CLEAR), 0);
> +		err = hw->mac.ops.update_uc_addr(hw, vf_info->glort,
> +						 mac, vlan, set, 0);
>   	}
>
>   	if (!err && !!results[FM10K_MAC_VLAN_MSG_MULTICAST]) {
> @@ -1241,19 +1251,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct fm10k_hw *hw, u32 **results,
>   		if (!(vf_info->vf_flags & FM10K_VF_FLAG_MULTI_ENABLED))
>   			return FM10K_ERR_PARAM;
>
> +		set = !(vlan & FM10K_VLAN_CLEAR);
> +		vlan &= ~FM10K_VLAN_CLEAR;
> +
>   		/* if VLAN ID is 0, set the default VLAN ID instead of 0 */
>   		if (!vlan || (vlan == FM10K_VLAN_CLEAR)) {

You missed a spot here.  This should be if(!vlan) since you already 
cleared the FM10K_VLAN_CLEAR flag.

>   			if (vf_info->pf_vid)
>   				vlan |= vf_info->pf_vid;
>   			else
>   				vlan |= vf_info->sw_vid;
> -		} else if (vf_info->pf_vid) {
> +		} else if (vf_info->pf_vid && vlan != vf_info->pf_vid) {
>   			return FM10K_ERR_PARAM;
>   		}
>
>   		/* notify switch of request for new multicast address */
> -		err = hw->mac.ops.update_mc_addr(hw, vf_info->glort, mac, vlan,
> -						 !(vlan & FM10K_VLAN_CLEAR));
> +		err = hw->mac.ops.update_mc_addr(hw, vf_info->glort,
> +						 mac, vlan, set);
>   	}
>
>   	return err;
>

Really since the code is so identical you might just want to create a 
function to do all of this.  You could have it return a value 
representing the VLAN ID if >=0, or error if < 0.

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Vick, Matthew]

On 6/19/15, 9:20 PM, "Alexander Duyck" <alexander.duyck@gmail.com> wrote:

>On 06/19/2015 04:37 PM, Jacob Keller wrote:

[...]

>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>>b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> index 6d1364393a8b..989981ea3c64 100644
>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> @@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct
>>fm10k_intfc *interface,
>>   	/* assign default VLAN to queue */
>>   	ring->vid = hw->mac.default_vid;
>>
>> +	/* if we have an active VLAN, disable default vid */
>> +	if (test_bit(hw->mac.default_vid, interface->active_vlans))
>> +		ring->vid |= FM10K_VLAN_CLEAR;
>> +
>>   	/* Map interrupt */
>>   	if (ring->q_vector) {
>>   		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
>>
>
>This doesn't quite work since the rx_ring->vid is doing a full 16b
>comparison against a value that includes the QOS value.
>
>I just realized that is a bug in the fm10k_process_skb_fields function.
>  You probably need to update it so that code does something more like
>
>if (rx_desc->w.vlan) {
>	u16 vid = le16_to_cpu(rx_desc->w.vlan);
>
>	if ((vid & VLAN_VID_MASK) != rx_ring->vid)
>		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
>	else if (vid & VLAN_PRIO_MASK)
>		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
>				       vid & VLAN_PRIO_MASK);
>}

Agreed. Coincidentally, this bit us last week and we're working internally
to resolve it now. Your solution looks good to me.

[Intel-wired-lan] [net-next 03/10] fm10k: don't store sw_vid at reset

[Keller, Jacob E]

On Fri, 2015-06-19 at 21:03 -0700, Alexander Duyck wrote:
> On 06/19/2015 04:37 PM, Jacob Keller wrote:
> > If we store the sw_vid at reset of PF, then we accidentally prevent 
> > the
> > VF from receiving the message to update its default VID. This only
> > occurs if the VF is created before the PF has come up, which is the
> > standard way of creating VFs when using the module parameter.
> 
> The upstream driver doesn't have a module parameter for VFs.  It uses 
> 
> sysfs.  It isn't clear but can this issue occur if you use the sysfs?
> 

Yes, I can re-work the comment. It can happen if you instantiate VFs
before you bring up the PF such as:

modprobe fm10k
echo N > /sys/class/.../sriov_numvfs
ip link set DEVICE up

If you do that in this order VFs will have the issue.

> > This fixes an issue where we request the incorrect MAC/VLAN
> > combinations, and prevents us from accidentally reporting some 
> > frames as
> > vlan tagged.
> > 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >   drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 3 ---
> >   1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
> > index 94571e6e790c..0e25a80417b9 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
> > @@ -228,9 +228,6 @@ int fm10k_iov_resume(struct pci_dev *pdev)
> >   		hw->iov.ops.set_lport(hw, vf_info, i,
> >   				     
> >  FM10K_VF_FLAG_MULTI_CAPABLE);
> >   
> > -		/* assign our default vid to the VF following 
> > reset */
> > -		vf_info->sw_vid = hw->mac.default_vid;
> > -
> >   		/* mailbox is disconnected so we don't send a 
> > message */
> >   		hw->iov.ops.assign_default_mac_vlan(hw, vf_info);
> >   
> 
> The patch itself looks fine.  I assume the Switch API will send a 
> message to force us onto the correct VID if there is one already 
> reserved for this VF.
> 
> I probably shouldn't have used the PF VLAN ID for the VFs anyway. 
> Just 
> letting the Switch API bounce a message through is a much better way 
> to go.


Yes, the switch sends a message which we receive on coming up.

Regards,
Jake

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Keller, Jacob E]

On Fri, 2015-06-19 at 21:20 -0700, Alexander Duyck wrote:
> On 06/19/2015 04:37 PM, Jacob Keller wrote:
> > This patch ensures that vlan traffic on the default vid will go to 
> > the
> > corresponding vlan device if it exists. To do this, mask the 
> > rx_ring vid
> > if we have an active vlan on that vid.
> > 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >   drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++
> >   drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |  4 ++++
> >   2 files changed, 16 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > index e1ceb3a2e2cd..c173c79373fe 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > @@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device 
> > *netdev, u16 vid, bool set)
> >   	struct fm10k_intfc *interface = netdev_priv(netdev);
> >   	struct fm10k_hw *hw = &interface->hw;
> >   	s32 err;
> > +	int i;
> > 
> >   	/* updates do not apply to VLAN 0 */
> >   	if (!vid)
> > @@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device 
> > *netdev, u16 vid, bool set)
> >   	if (!set)
> >   		clear_bit(vid, interface->active_vlans);
> > 
> > +	/* disable the default vid on ring if we have an active 
> > vlan */
> > +	for (i = 0; i < interface->num_rx_queues; i++) {
> > +		struct fm10k_ring *rx_ring = interface
> > ->rx_ring[i];
> > +		u16 rx_vid = rx_ring->vid & (VLAN_N_VID - 1);
> > +
> > +		if (test_bit(rx_vid, interface->active_vlans))
> > +			rx_ring->vid |= FM10K_VLAN_CLEAR;
> > +		else
> > +			rx_ring->vid &= ~FM10K_VLAN_CLEAR;
> > +	}
> > +
> >   	/* Do not remove default VID related entries from VLAN and 
> > MAC tables */
> >   	if (!set && vid == hw->mac.default_vid)
> >   		return 0;
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > index 6d1364393a8b..989981ea3c64 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > @@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct 
> > fm10k_intfc *interface,
> >   	/* assign default VLAN to queue */
> >   	ring->vid = hw->mac.default_vid;
> > 
> > +	/* if we have an active VLAN, disable default vid */
> > +	if (test_bit(hw->mac.default_vid, interface
> > ->active_vlans))
> > +		ring->vid |= FM10K_VLAN_CLEAR;
> > +
> >   	/* Map interrupt */
> >   	if (ring->q_vector) {
> >   		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
> > 
> 
> This doesn't quite work since the rx_ring->vid is doing a full 16b 
> comparison against a value that includes the QOS value.
> 

So we have to  mask the QOS value?

> I just realized that is a bug in the fm10k_process_skb_fields 
> function. 
>   You probably need to update it so that code does something more 
> like
> 
> if (rx_desc->w.vlan) {
> 	u16 vid = le16_to_cpu(rx_desc->w.vlan);
> 
> 	if ((vid & VLAN_VID_MASK) != rx_ring->vid)
> 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), 
> vid);
> 	else if (vid & VLAN_PRIO_MASK)
> 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> 				       vid & VLAN_PRIO_MASK);
> }	

I'll take a look at that.

Regards,
Jake

[Intel-wired-lan] [net-next 10/10] fm10k: fix iov_msg_mac_vlan_pf VID checks

[Keller, Jacob E]

On Fri, 2015-06-19 at 21:41 -0700, Alexander Duyck wrote:
> On 06/19/2015 04:37 PM, Jacob Keller wrote:
> > The VF will send a message to request multicast addresses with the
> > default vid. In the current code, if the PF has statically assigned 
> > a
> > VLAN to a VF, then the VF will not get the multicast addresses. Fix 
> > up
> > all of the various vlan messages to use identical checks (since 
> > each
> > check was different). Also use set as a variable, so that it 
> > simplifies
> > our check for whether vlan matches the pf_vid.
> > 
> > The new logic will allow set of a vlan if it is zero, automatically
> > converting to the default vid. Otherwise it will allow setting the 
> > PF
> > vid, or any VLAN if PF has not statically assigned a VLAN. This is
> > consistent behavior, and allows VF to request either 0 or the
> > default_vid without silently failing.
> > 
> > Note that we need the check for zero since VFs might not get the 
> > default
> > VID message in time to actually request non-zero VLANs.
> > 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >   drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 37 
> > +++++++++++++++++++----------
> >   1 file changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
> > index d806d87a6192..9bb57531b5db 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
> > @@ -1168,9 +1168,10 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct 
> > fm10k_hw *hw, u32 **results,
> >   			      struct fm10k_mbx_info *mbx)
> >   {
> >   	struct fm10k_vf_info *vf_info = (struct fm10k_vf_info 
> > *)mbx;
> > -	int err = 0;
> >   	u8 mac[ETH_ALEN];
> >   	u32 *result;
> > +	int err = 0;
> > +	bool set;
> >   	u16 vlan;
> >   	u32 vid;
> > 
> > @@ -1186,19 +1187,25 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct 
> > fm10k_hw *hw, u32 **results,
> >   		if (err)
> >   			return err;
> > 
> > +		/* verify upper 16 bits are zero */
> > +		if (vid >> 16)
> > +			return FM10K_ERR_PARAM;
> > +
> > +		set = !(vid & FM10K_VLAN_CLEAR);
> > +		vid &= ~FM10K_VLAN_CLEAR;
> > +
> >   		/* if VLAN ID is 0, set the default VLAN ID 
> > instead of 0 */
> > -		if (!vid || (vid == FM10K_VLAN_CLEAR)) {
> > +		if (!vid) {
> >   			if (vf_info->pf_vid)
> >   				vid |= vf_info->pf_vid;
> >   			else
> >   				vid |= vf_info->sw_vid;
> > -		} else if (vid != vf_info->pf_vid) {
> > +		} else if (vf_info->pf_vid && vid != vf_info
> > ->pf_vid) {
> >   			return FM10K_ERR_PARAM;
> >   		}
> > 
> >   		/* update VSI info for VF in regards to VLAN table 
> > */
> > -		err = hw->mac.ops.update_vlan(hw, vid, vf_info
> > ->vsi,
> > -					      !(vid & 
> > FM10K_VLAN_CLEAR));
> > +		err = hw->mac.ops.update_vlan(hw, vid, vf_info
> > ->vsi, set);
> >   	}
> > 
> >   	if (!err && !!results[FM10K_MAC_VLAN_MSG_MAC]) {
> > @@ -1214,19 +1221,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct 
> > fm10k_hw *hw, u32 **results,
> >   		    memcmp(mac, vf_info->mac, ETH_ALEN))
> >   			return FM10K_ERR_PARAM;
> > 
> > +		set = !(vlan & FM10K_VLAN_CLEAR);
> > +		vlan &= ~FM10K_VLAN_CLEAR;
> > +
> >   		/* if VLAN ID is 0, set the default VLAN ID 
> > instead of 0 */
> > -		if (!vlan || (vlan == FM10K_VLAN_CLEAR)) {
> > +		if (!vlan) {
> >   			if (vf_info->pf_vid)
> >   				vlan |= vf_info->pf_vid;
> >   			else
> >   				vlan |= vf_info->sw_vid;
> > -		} else if (vf_info->pf_vid) {
> > +		} else if (vf_info->pf_vid && vlan != vf_info
> > ->pf_vid) {
> >   			return FM10K_ERR_PARAM;
> >   		}
> > 
> >   		/* notify switch of request for new unicast 
> > address */
> > -		err = hw->mac.ops.update_uc_addr(hw, vf_info
> > ->glort, mac, vlan,
> > -						 !(vlan & 
> > FM10K_VLAN_CLEAR), 0);
> > +		err = hw->mac.ops.update_uc_addr(hw, vf_info
> > ->glort,
> > +						 mac, vlan, set, 
> > 0);
> >   	}
> > 
> >   	if (!err && !!results[FM10K_MAC_VLAN_MSG_MULTICAST]) {
> > @@ -1241,19 +1251,22 @@ s32 fm10k_iov_msg_mac_vlan_pf(struct 
> > fm10k_hw *hw, u32 **results,
> >   		if (!(vf_info->vf_flags & 
> > FM10K_VF_FLAG_MULTI_ENABLED))
> >   			return FM10K_ERR_PARAM;
> > 
> > +		set = !(vlan & FM10K_VLAN_CLEAR);
> > +		vlan &= ~FM10K_VLAN_CLEAR;
> > +
> >   		/* if VLAN ID is 0, set the default VLAN ID 
> > instead of 0 */
> >   		if (!vlan || (vlan == FM10K_VLAN_CLEAR)) {
> 
> You missed a spot here.  This should be if(!vlan) since you already 
> cleared the FM10K_VLAN_CLEAR flag.
> 

Oops yep.

> >   			if (vf_info->pf_vid)
> >   				vlan |= vf_info->pf_vid;
> >   			else
> >   				vlan |= vf_info->sw_vid;
> > -		} else if (vf_info->pf_vid) {
> > +		} else if (vf_info->pf_vid && vlan != vf_info
> > ->pf_vid) {
> >   			return FM10K_ERR_PARAM;
> >   		}
> > 
> >   		/* notify switch of request for new multicast 
> > address */
> > -		err = hw->mac.ops.update_mc_addr(hw, vf_info
> > ->glort, mac, vlan,
> > -						 !(vlan & 
> > FM10K_VLAN_CLEAR));
> > +		err = hw->mac.ops.update_mc_addr(hw, vf_info
> > ->glort,
> > +						 mac, vlan, set);
> >   	}
> > 
> >   	return err;
> > 
> 
> Really since the code is so identical you might just want to create a 
> 
> function to do all of this.  You could have it return a value 
> representing the VLAN ID if >=0, or error if < 0.
> 

I tried that first, but the main issue was different types. I wasn't
sure how type-casting from a 32 to a 16 would do the right thing. For
some flows we have a 16bit value, for others a 32bit value. (since it
stores extra data). Actually, just casting it to 16 should work right?
and then we can have the function just take a 16bit.

I'll look at that.

Regards,
Jake

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Keller, Jacob E]

On Fri, 2015-06-19 at 21:20 -0700, Alexander Duyck wrote:
> On 06/19/2015 04:37 PM, Jacob Keller wrote:
> > This patch ensures that vlan traffic on the default vid will go to 
> > the
> > corresponding vlan device if it exists. To do this, mask the 
> > rx_ring vid
> > if we have an active vlan on that vid.
> > 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >   drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++
> >   drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |  4 ++++
> >   2 files changed, 16 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > index e1ceb3a2e2cd..c173c79373fe 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > @@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device 
> > *netdev, u16 vid, bool set)
> >   	struct fm10k_intfc *interface = netdev_priv(netdev);
> >   	struct fm10k_hw *hw = &interface->hw;
> >   	s32 err;
> > +	int i;
> > 
> >   	/* updates do not apply to VLAN 0 */
> >   	if (!vid)
> > @@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device 
> > *netdev, u16 vid, bool set)
> >   	if (!set)
> >   		clear_bit(vid, interface->active_vlans);
> > 
> > +	/* disable the default vid on ring if we have an active 
> > vlan */
> > +	for (i = 0; i < interface->num_rx_queues; i++) {
> > +		struct fm10k_ring *rx_ring = interface
> > ->rx_ring[i];
> > +		u16 rx_vid = rx_ring->vid & (VLAN_N_VID - 1);
> > +
> > +		if (test_bit(rx_vid, interface->active_vlans))
> > +			rx_ring->vid |= FM10K_VLAN_CLEAR;
> > +		else
> > +			rx_ring->vid &= ~FM10K_VLAN_CLEAR;
> > +	}
> > +
> >   	/* Do not remove default VID related entries from VLAN and 
> > MAC tables */
> >   	if (!set && vid == hw->mac.default_vid)
> >   		return 0;
> > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c 
> > b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > index 6d1364393a8b..989981ea3c64 100644
> > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > @@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct 
> > fm10k_intfc *interface,
> >   	/* assign default VLAN to queue */
> >   	ring->vid = hw->mac.default_vid;
> > 
> > +	/* if we have an active VLAN, disable default vid */
> > +	if (test_bit(hw->mac.default_vid, interface
> > ->active_vlans))
> > +		ring->vid |= FM10K_VLAN_CLEAR;
> > +
> >   	/* Map interrupt */
> >   	if (ring->q_vector) {
> >   		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
> > 
> 
> This doesn't quite work since the rx_ring->vid is doing a full 16b 
> comparison against a value that includes the QOS value.
> 

To clarify, this should work as long as we fix the process_skb_fields?

Regards,
Jake

> I just realized that is a bug in the fm10k_process_skb_fields 
> function. 
>   You probably need to update it so that code does something more 
> like
> 
> if (rx_desc->w.vlan) {
> 	u16 vid = le16_to_cpu(rx_desc->w.vlan);
> 
> 	if ((vid & VLAN_VID_MASK) != rx_ring->vid)
> 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), 
> vid);
> 	else if (vid & VLAN_PRIO_MASK)
> 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> 				       vid & VLAN_PRIO_MASK);
> }	

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Alexander Duyck]

On 06/23/2015 03:18 PM, Keller, Jacob E wrote:
> On Fri, 2015-06-19 at 21:20 -0700, Alexander Duyck wrote:
>> On 06/19/2015 04:37 PM, Jacob Keller wrote:
>>> This patch ensures that vlan traffic on the default vid will go to
>>> the
>>> corresponding vlan device if it exists. To do this, mask the
>>> rx_ring vid
>>> if we have an active vlan on that vid.
>>>
>>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>>> ---
>>>    drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 ++++++++++++
>>>    drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |  4 ++++
>>>    2 files changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>>> b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>>> index e1ceb3a2e2cd..c173c79373fe 100644
>>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>>> @@ -758,6 +758,7 @@ static int fm10k_update_vid(struct net_device
>>> *netdev, u16 vid, bool set)
>>>    	struct fm10k_intfc *interface = netdev_priv(netdev);
>>>    	struct fm10k_hw *hw = &interface->hw;
>>>    	s32 err;
>>> +	int i;
>>>
>>>    	/* updates do not apply to VLAN 0 */
>>>    	if (!vid)
>>> @@ -775,6 +776,17 @@ static int fm10k_update_vid(struct net_device
>>> *netdev, u16 vid, bool set)
>>>    	if (!set)
>>>    		clear_bit(vid, interface->active_vlans);
>>>
>>> +	/* disable the default vid on ring if we have an active
>>> vlan */
>>> +	for (i = 0; i < interface->num_rx_queues; i++) {
>>> +		struct fm10k_ring *rx_ring = interface
>>> ->rx_ring[i];
>>> +		u16 rx_vid = rx_ring->vid & (VLAN_N_VID - 1);
>>> +
>>> +		if (test_bit(rx_vid, interface->active_vlans))
>>> +			rx_ring->vid |= FM10K_VLAN_CLEAR;
>>> +		else
>>> +			rx_ring->vid &= ~FM10K_VLAN_CLEAR;
>>> +	}
>>> +
>>>    	/* Do not remove default VID related entries from VLAN and
>>> MAC tables */
>>>    	if (!set && vid == hw->mac.default_vid)
>>>    		return 0;
>>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>>> b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>>> index 6d1364393a8b..989981ea3c64 100644
>>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>>> @@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct
>>> fm10k_intfc *interface,
>>>    	/* assign default VLAN to queue */
>>>    	ring->vid = hw->mac.default_vid;
>>>
>>> +	/* if we have an active VLAN, disable default vid */
>>> +	if (test_bit(hw->mac.default_vid, interface
>>> ->active_vlans))
>>> +		ring->vid |= FM10K_VLAN_CLEAR;
>>> +
>>>    	/* Map interrupt */
>>>    	if (ring->q_vector) {
>>>    		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
>>>
>> This doesn't quite work since the rx_ring->vid is doing a full 16b
>> comparison against a value that includes the QOS value.
>>
> To clarify, this should work as long as we fix the process_skb_fields?

Yeah, if you fix it then you should be all set as the change below would 
make it so that you could never match with the FM10K_VLAN_CLEAR bit set.

> I just realized that is a bug in the fm10k_process_skb_fields
> function.
>    You probably need to update it so that code does something more
> like
>
> if (rx_desc->w.vlan) {
> 	u16 vid = le16_to_cpu(rx_desc->w.vlan);
>
> 	if ((vid & VLAN_VID_MASK) != rx_ring->vid)
> 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> vid);
> 	else if (vid & VLAN_PRIO_MASK)
> 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> 				       vid & VLAN_PRIO_MASK);
> }	

[Intel-wired-lan] [net-next 06/10] fm10k: send traffic on default vid to vlan device if we have one

[Keller, Jacob E]

On Wed, 2015-06-24 at 15:13 -0700, Alexander Duyck wrote:
> On 06/23/2015 03:18 PM, Keller, Jacob E wrote:
> > On Fri, 2015-06-19 at 21:20 -0700, Alexander Duyck wrote:
> > > On 06/19/2015 04:37 PM, Jacob Keller wrote:
> > > > This patch ensures that vlan traffic on the default vid will go 
> > > > to
> > > > the
> > > > corresponding vlan device if it exists. To do this, mask the
> > > > rx_ring vid
> > > > if we have an active vlan on that vid.
> > > > 
> > > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > > > ---
> > > >    drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 12 
> > > > ++++++++++++
> > > >    drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |  4 ++++
> > > >    2 files changed, 16 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > > > b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > > > index e1ceb3a2e2cd..c173c79373fe 100644
> > > > --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > > > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > > > @@ -758,6 +758,7 @@ static int fm10k_update_vid(struct 
> > > > net_device
> > > > *netdev, u16 vid, bool set)
> > > >    	struct fm10k_intfc *interface = netdev_priv(netdev);
> > > >    	struct fm10k_hw *hw = &interface->hw;
> > > >    	s32 err;
> > > > +	int i;
> > > > 
> > > >    	/* updates do not apply to VLAN 0 */
> > > >    	if (!vid)
> > > > @@ -775,6 +776,17 @@ static int fm10k_update_vid(struct 
> > > > net_device
> > > > *netdev, u16 vid, bool set)
> > > >    	if (!set)
> > > >    		clear_bit(vid, interface->active_vlans);
> > > > 
> > > > +	/* disable the default vid on ring if we have an 
> > > > active
> > > > vlan */
> > > > +	for (i = 0; i < interface->num_rx_queues; i++) {
> > > > +		struct fm10k_ring *rx_ring = interface
> > > > ->rx_ring[i];
> > > > +		u16 rx_vid = rx_ring->vid & (VLAN_N_VID - 1);
> > > > +
> > > > +		if (test_bit(rx_vid, interface->active_vlans))
> > > > +			rx_ring->vid |= FM10K_VLAN_CLEAR;
> > > > +		else
> > > > +			rx_ring->vid &= ~FM10K_VLAN_CLEAR;
> > > > +	}
> > > > +
> > > >    	/* Do not remove default VID related entries from VLAN 
> > > > and
> > > > MAC tables */
> > > >    	if (!set && vid == hw->mac.default_vid)
> > > >    		return 0;
> > > > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > > > b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > > > index 6d1364393a8b..989981ea3c64 100644
> > > > --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > > > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > > > @@ -694,6 +694,10 @@ static void fm10k_configure_rx_ring(struct
> > > > fm10k_intfc *interface,
> > > >    	/* assign default VLAN to queue */
> > > >    	ring->vid = hw->mac.default_vid;
> > > > 
> > > > +	/* if we have an active VLAN, disable default vid */
> > > > +	if (test_bit(hw->mac.default_vid, interface
> > > > ->active_vlans))
> > > > +		ring->vid |= FM10K_VLAN_CLEAR;
> > > > +
> > > >    	/* Map interrupt */
> > > >    	if (ring->q_vector) {
> > > >    		rxint = ring->q_vector->v_idx + 
> > > > NON_Q_VECTORS(hw);
> > > > 
> > > This doesn't quite work since the rx_ring->vid is doing a full 
> > > 16b
> > > comparison against a value that includes the QOS value.
> > > 
> > To clarify, this should work as long as we fix the 
> > process_skb_fields?
> 
> Yeah, if you fix it then you should be all set as the change below 
> would 
> make it so that you could never match with the FM10K_VLAN_CLEAR bit 
> set.
> 

Yep, that's what i figured. I've sent new patch series with this fix in
place.

Regards,
Jake

> > I just realized that is a bug in the fm10k_process_skb_fields
> > function.
> >    You probably need to update it so that code does something more
> > like
> > 
> > if (rx_desc->w.vlan) {
> > 	u16 vid = le16_to_cpu(rx_desc->w.vlan);
> > 
> > 	if ((vid & VLAN_VID_MASK) != rx_ring->vid)
> > 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> > vid);
> > 	else if (vid & VLAN_PRIO_MASK)
> > 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> > 				       vid & VLAN_PRIO_MASK);
> > }	
>