[Intel-wired-lan] [next PATCH S93 01/11] i40e: add helper function for copying strings from stat arrays

[Intel-wired-lan] [next PATCH S93 01/11] i40e: add helper function for copying strings from stat arrays

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

Many of the ethtool statistics use the same basic logic for copying
strings into the supplied buffer. A set of stats are stored in a const
array of i40e_stats structures, and we apply these all together.

Simplify the stats code by introducing a helper function which can take
a stats array and copy the strings into the buffer, updating the buffer
pointer as we go.

We use a macro to implement i40e_add_stat_strings so that ARRAY_SIZE can
be used on the array passed in. This ensures that we always use the
matching size in __i40e_add_stat_strings.

More complex stats currently do not use i40e_stats arrays, usually due
to custom formatted strings, or because the stats are not laid out in
the expected way. These stats will be updated to use the helper function
in separate future patches.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 59 +++++++++++++++++---------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 6947a2a..20e8630 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1816,6 +1816,37 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 }
 
 /**
+ * __i40e_add_stat_strings - copy stat strings into ethtool buffer
+ * @p: ethtool supplied buffer
+ * @stats: stat definitions array
+ * @size: size of the stats array
+ *
+ * Copy the strings described by stats into the buffer pointed at by p.
+ **/
+static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+				    const unsigned int size)
+{
+	unsigned int i;
+
+	for (i = 0; i < size; i++) {
+		snprintf(*p, ETH_GSTRING_LEN, "%s", stats[i].stat_string);
+		*p += ETH_GSTRING_LEN;
+	}
+}
+
+/**
+ * 40e_add_stat_strings - copy stat strings into ethtool buffer
+ * @p: ethtool supplied buffer
+ * @stats: stat definitions array
+ *
+ * Format and copy the strings described by the const static stats value into
+ * the buffer pointed@by p. Assumes that stats can have ARRAY_SIZE called
+ * for it.
+ **/
+#define i40e_add_stat_strings(p, stats, ...) \
+	__i40e_add_stat_strings(p, stats, ARRAY_SIZE(stats))
+
+/**
  * i40e_get_stat_strings - copy stat strings into supplied buffer
  * @netdev: the netdev to collect strings for
  * @data: supplied buffer to copy strings into
@@ -1833,16 +1864,10 @@ static void i40e_get_stat_strings(struct net_device *netdev, u8 *data)
 	unsigned int i;
 	u8 *p = data;
 
-	for (i = 0; i < I40E_NETDEV_STATS_LEN; i++) {
-		snprintf(data, ETH_GSTRING_LEN, "%s",
-			 i40e_gstrings_net_stats[i].stat_string);
-		data += ETH_GSTRING_LEN;
-	}
-	for (i = 0; i < I40E_MISC_STATS_LEN; i++) {
-		snprintf(data, ETH_GSTRING_LEN, "%s",
-			 i40e_gstrings_misc_stats[i].stat_string);
-		data += ETH_GSTRING_LEN;
-	}
+	i40e_add_stat_strings(&data, i40e_gstrings_net_stats);
+
+	i40e_add_stat_strings(&data, i40e_gstrings_misc_stats);
+
 	for (i = 0; i < I40E_MAX_NUM_QUEUES(netdev); i++) {
 		snprintf(data, ETH_GSTRING_LEN, "tx-%u.tx_packets", i);
 		data += ETH_GSTRING_LEN;
@@ -1856,11 +1881,8 @@ static void i40e_get_stat_strings(struct net_device *netdev, u8 *data)
 	if (vsi != pf->vsi[pf->lan_vsi] || pf->hw.partition_id != 1)
 		return;
 
-	for (i = 0; i < I40E_VEB_STATS_LEN; i++) {
-		snprintf(data, ETH_GSTRING_LEN, "%s",
-			 i40e_gstrings_veb_stats[i].stat_string);
-		data += ETH_GSTRING_LEN;
-	}
+	i40e_add_stat_strings(&data, i40e_gstrings_veb_stats);
+
 	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
 		snprintf(data, ETH_GSTRING_LEN,
 			 "veb.tc_%u_tx_packets", i);
@@ -1876,11 +1898,8 @@ static void i40e_get_stat_strings(struct net_device *netdev, u8 *data)
 		data += ETH_GSTRING_LEN;
 	}
 
-	for (i = 0; i < I40E_GLOBAL_STATS_LEN; i++) {
-		snprintf(data, ETH_GSTRING_LEN, "%s",
-			 i40e_gstrings_stats[i].stat_string);
-		data += ETH_GSTRING_LEN;
-	}
+	i40e_add_stat_strings(&data, i40e_gstrings_stats);
+
 	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
 		snprintf(data, ETH_GSTRING_LEN,
 			 "port.tx_priority_%u_xon", i);
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 02/11] i40e: add helper to copy statistic values into ethtool buffer

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

Similar to the helper function to copy the ethtool stats strings, add
and use a helper function for copying the ethtool stats into the
supplied buffer.

Just like before, we use a macro to avoid having to pass ARRAY_SIZE
manually, so as to reduce chance of bugs.

Some of the stats, especially queue stats, are a bit trickier, and will
be handled in future patches.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116 ++++++++++++++++++++-----
 1 file changed, 93 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 20e8630..c051afe 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1706,6 +1706,89 @@ static int i40e_get_sset_count(struct net_device *netdev, int sset)
 }
 
 /**
+ * i40e_add_one_ethtool_stat - copy the stat into the supplied buffer
+ * @data: location to store the stat value
+ * @pointer: basis for where to copy from
+ * @stat: the stat definition
+ *
+ * Copies the stat data defined by the pointer and stat structure pair into
+ * the memory supplied as data. Used to implement i40e_add_ethtool_stats.
+ * If the pointer is null, data will be zero'd.
+ */
+static inline void
+i40e_add_one_ethtool_stat(u64 *data, void *pointer,
+			  const struct i40e_stats *stat)
+{
+	char *p;
+
+	if (!pointer) {
+		/* ensure that the ethtool data buffer is zero'd for any stats
+		 * which don't have a valid pointer.
+		 */
+		*data = 0;
+		return;
+	}
+
+	p = (char *)pointer + stat->stat_offset;
+	switch (stat->sizeof_stat) {
+	case sizeof(u64):
+		*data = *((u64 *)p);
+		break;
+	case sizeof(u32):
+		*data = *((u32 *)p);
+		break;
+	case sizeof(u16):
+		*data = *((u16 *)p);
+		break;
+	case sizeof(u8):
+		*data = *((u8 *)p);
+		break;
+	default:
+		WARN_ONCE(1, "unexpected stat size for %s",
+			  stat->stat_string);
+		*data = 0;
+	}
+}
+
+/**
+ * __i40e_add_ethtool_stats - copy stats into the ethtool supplied buffer
+ * @data: ethtool stats buffer
+ * @pointer: location to copy stats from
+ * @stats: array of stats to copy
+ * @size: the size of the stats definition
+ *
+ * Copy the stats defined by the stats array using the pointer as a base into
+ * the data buffer supplied by ethtool. Updates the data pointer to point to
+ * the next empty location for successive calls to __i40e_add_ethtool_stats.
+ * If pointer is null, set the data values to zero and update the pointer to
+ * skip these stats.
+ **/
+static inline void
+__i40e_add_ethtool_stats(u64 **data, void *pointer,
+			 const struct i40e_stats stats[],
+			 const unsigned int size)
+{
+	unsigned int i;
+
+	for (i = 0; i < size; i++)
+		i40e_add_one_ethtool_stat((*data)++, pointer, &stats[i]);
+}
+
+/**
+ * i40e_add_ethtool_stats - copy stats into ethtool supplied buffer
+ * @data: ethtool stats buffer
+ * @pointer: location where stats are stored
+ * @stats: static const array of stat definitions
+ *
+ * Macro to ease the use of __i40e_add_ethtool_stats by taking a static
+ * constant stats array and passing the ARRAY_SIZE(). This avoids typos by
+ * ensuring that we pass the size associated with the given stats array.
+ * Assumes that stats is an array.
+ **/
+#define i40e_add_ethtool_stats(data, pointer, stats) \
+	__i40e_add_ethtool_stats(data, pointer, stats, ARRAY_SIZE(stats))
+
+/**
  * i40e_get_ethtool_stats - copy stat values into supplied buffer
  * @netdev: the netdev to collect stats for
  * @stats: ethtool stats command structure
@@ -1727,22 +1810,15 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
 	unsigned int i;
-	char *p;
-	struct rtnl_link_stats64 *net_stats = i40e_get_vsi_stats_struct(vsi);
 	unsigned int start;
 
 	i40e_update_stats(vsi);
 
-	for (i = 0; i < I40E_NETDEV_STATS_LEN; i++) {
-		p = (char *)net_stats + i40e_gstrings_net_stats[i].stat_offset;
-		*(data++) = (i40e_gstrings_net_stats[i].sizeof_stat ==
-			sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
-	}
-	for (i = 0; i < I40E_MISC_STATS_LEN; i++) {
-		p = (char *)vsi + i40e_gstrings_misc_stats[i].stat_offset;
-		*(data++) = (i40e_gstrings_misc_stats[i].sizeof_stat ==
-			    sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
-	}
+	i40e_add_ethtool_stats(&data, i40e_get_vsi_stats_struct(vsi),
+			       i40e_gstrings_net_stats);
+
+	i40e_add_ethtool_stats(&data, vsi, i40e_gstrings_misc_stats);
+
 	rcu_read_lock();
 	for (i = 0; i < I40E_MAX_NUM_QUEUES(netdev) ; i++) {
 		tx_ring = READ_ONCE(vsi->tx_rings[i]);
@@ -1783,12 +1859,8 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	    (pf->flags & I40E_FLAG_VEB_STATS_ENABLED)) {
 		struct i40e_veb *veb = pf->veb[pf->lan_veb];
 
-		for (i = 0; i < I40E_VEB_STATS_LEN; i++) {
-			p = (char *)veb;
-			p += i40e_gstrings_veb_stats[i].stat_offset;
-			*(data++) = (i40e_gstrings_veb_stats[i].sizeof_stat ==
-				     sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
-		}
+		i40e_add_ethtool_stats(&data, veb, i40e_gstrings_veb_stats);
+
 		for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
 			*(data++) = veb->tc_stats.tc_tx_packets[i];
 			*(data++) = veb->tc_stats.tc_tx_bytes[i];
@@ -1798,11 +1870,9 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	} else {
 		data += I40E_VEB_STATS_TOTAL;
 	}
-	for (i = 0; i < I40E_GLOBAL_STATS_LEN; i++) {
-		p = (char *)pf + i40e_gstrings_stats[i].stat_offset;
-		*(data++) = (i40e_gstrings_stats[i].sizeof_stat ==
-			     sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
-	}
+
+	i40e_add_ethtool_stats(&data, pf, i40e_gstrings_stats);
+
 	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
 		*(data++) = pf->stats.priority_xon_tx[i];
 		*(data++) = pf->stats.priority_xoff_tx[i];
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 02/11] i40e: add helper to copy statistic values into ethtool buffer

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 02/11] i40e: add helper to copy
> statistic values into ethtool buffer
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Similar to the helper function to copy the ethtool stats strings, add and use a
> helper function for copying the ethtool stats into the supplied buffer.
> 
> Just like before, we use a macro to avoid having to pass ARRAY_SIZE
> manually, so as to reduce chance of bugs.
> 
> Some of the stats, especially queue stats, are a bit trickier, and will be
> handled in future patches.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116
> ++++++++++++++++++++-----
>  1 file changed, 93 insertions(+), 23 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 03/11] i40e: Set fec_config when forcing link state

[Alice Michael]

From: Mariusz Stachura <mariusz.stachura@intel.com>

This patch configures FEC setting in i40e_force_link_state().
For some reason setting this field was overlooked thus causing
25G link to be configured incorrectly.

Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 13940e0..a730f48 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6597,6 +6597,8 @@ static i40e_status i40e_force_link_state(struct i40e_pf *pf, bool is_up)
 	config.eee_capability = abilities.eee_capability;
 	config.eeer = abilities.eeer_val;
 	config.low_power_ctrl = abilities.d3_lpan;
+	config.fec_config = abilities.fec_cfg_curr_mod_ext_info &
+			    I40E_AQ_PHY_FEC_CONFIG_MASK;
 	err = i40e_aq_set_phy_config(hw, &config, NULL);
 
 	if (err) {
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 03/11] i40e: Set fec_config when forcing link state

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 03/11] i40e: Set fec_config when
> forcing link state
> 
> From: Mariusz Stachura <mariusz.stachura@intel.com>
> 
> This patch configures FEC setting in i40e_force_link_state().
> For some reason setting this field was overlooked thus causing 25G link to be
> configured incorrectly.
> 
> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++
>  1 file changed, 2 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 04/11] i40e: convert VEB TC stats to use an i40e_stats array

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

The VEB TC stats are currently implemented with separate parsing,
instead of using the i40e_stats array and associated helper functions.
This is likely because the stats rely on embedding the TC number into
the stat name.

Update i40e_add_stat_strings to take variadic arguments, and use these
to vsnprintf the i40e_stats string as a string containing format
specifiers.

Create a stats array for the VEB TC related stats,
i40e_gstrings_veb_tc_stats, and use this along with the helper functions
to remove the specialized boiler plate code.

Always call i40e_add_ethtool_stats for both this array and the general
VEB stats array. This ensures that we zero out any memory in case it was
not zero-allocated for us.

This ultimately results in less boiler plate code for the
i40e_get_stat_strings and i40e_get_ethtool_stats.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 83 +++++++++++++-------------
 1 file changed, 43 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index c051afe..52ccafe 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -7,6 +7,11 @@
 #include "i40e_diag.h"
 
 struct i40e_stats {
+	/* The stat_string is expected to be a format string formatted using
+	 * vsnprintf by i40e_add_stat_strings. Every member of a stats array
+	 * should use the same format specifiers as they will be formatted
+	 * using the same variadic arguments.
+	 */
 	char stat_string[ETH_GSTRING_LEN];
 	int sizeof_stat;
 	int stat_offset;
@@ -56,6 +61,13 @@ static const struct i40e_stats i40e_gstrings_veb_stats[] = {
 	I40E_VEB_STAT("veb.rx_unknown_protocol", stats.rx_unknown_protocol),
 };
 
+static const struct i40e_stats i40e_gstrings_veb_tc_stats[] = {
+	I40E_VEB_STAT("veb.tc_%u_tx_packets", tc_stats.tc_tx_packets),
+	I40E_VEB_STAT("veb.tc_%u_tx_bytes", tc_stats.tc_tx_bytes),
+	I40E_VEB_STAT("veb.tc_%u_rx_packets", tc_stats.tc_rx_packets),
+	I40E_VEB_STAT("veb.tc_%u_rx_bytes", tc_stats.tc_rx_bytes),
+};
+
 static const struct i40e_stats i40e_gstrings_misc_stats[] = {
 	I40E_VSI_STAT("rx_unicast", eth_stats.rx_unicast),
 	I40E_VSI_STAT("tx_unicast", eth_stats.tx_unicast),
@@ -162,16 +174,14 @@ static const struct i40e_stats i40e_gstrings_stats[] = {
 		 FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_tx) + \
 		 FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_2_xoff)) \
 		 / sizeof(u64))
-#define I40E_VEB_TC_STATS_LEN ( \
-		(FIELD_SIZEOF(struct i40e_veb, tc_stats.tc_rx_packets) + \
-		 FIELD_SIZEOF(struct i40e_veb, tc_stats.tc_rx_bytes) + \
-		 FIELD_SIZEOF(struct i40e_veb, tc_stats.tc_tx_packets) + \
-		 FIELD_SIZEOF(struct i40e_veb, tc_stats.tc_tx_bytes)) \
-		 / sizeof(u64))
-#define I40E_VEB_STATS_LEN	ARRAY_SIZE(i40e_gstrings_veb_stats)
-#define I40E_VEB_STATS_TOTAL	(I40E_VEB_STATS_LEN + I40E_VEB_TC_STATS_LEN)
+
+#define I40E_VEB_STATS_LEN	(ARRAY_SIZE(i40e_gstrings_veb_stats) + \
+				 (ARRAY_SIZE(i40e_gstrings_veb_tc_stats) * \
+				  I40E_MAX_TRAFFIC_CLASS))
+
 #define I40E_PF_STATS_LEN(n)	(I40E_GLOBAL_STATS_LEN + \
 				 I40E_PFC_STATS_LEN + \
+				 I40E_VEB_STATS_LEN + \
 				 I40E_VSI_STATS_LEN((n)))
 
 enum i40e_ethtool_test_id {
@@ -1681,7 +1691,7 @@ static int i40e_get_stats_count(struct net_device *netdev)
 	struct i40e_pf *pf = vsi->back;
 
 	if (vsi == pf->vsi[pf->lan_vsi] && pf->hw.partition_id == 1)
-		return I40E_PF_STATS_LEN(netdev) + I40E_VEB_STATS_TOTAL;
+		return I40E_PF_STATS_LEN(netdev);
 	else
 		return I40E_VSI_STATS_LEN(netdev);
 }
@@ -1809,8 +1819,10 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	struct i40e_ring *tx_ring, *rx_ring;
 	struct i40e_vsi *vsi = np->vsi;
 	struct i40e_pf *pf = vsi->back;
+	struct i40e_veb *veb = pf->veb[pf->lan_veb];
 	unsigned int i;
 	unsigned int start;
+	bool veb_stats;
 
 	i40e_update_stats(vsi);
 
@@ -1855,21 +1867,19 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	if (vsi != pf->vsi[pf->lan_vsi] || pf->hw.partition_id != 1)
 		return;
 
-	if ((pf->lan_veb != I40E_NO_VEB) &&
-	    (pf->flags & I40E_FLAG_VEB_STATS_ENABLED)) {
-		struct i40e_veb *veb = pf->veb[pf->lan_veb];
+	veb_stats = ((pf->lan_veb != I40E_NO_VEB) &&
+		     (pf->flags & I40E_FLAG_VEB_STATS_ENABLED));
 
-		i40e_add_ethtool_stats(&data, veb, i40e_gstrings_veb_stats);
+	/* If veb stats aren't enabled, pass NULL instead of the veb so that
+	 * we initialize stats to zero and update the data pointer
+	 * intelligently
+	 */
+	i40e_add_ethtool_stats(&data, veb_stats ? veb : NULL,
+			       i40e_gstrings_veb_stats);
 
-		for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
-			*(data++) = veb->tc_stats.tc_tx_packets[i];
-			*(data++) = veb->tc_stats.tc_tx_bytes[i];
-			*(data++) = veb->tc_stats.tc_rx_packets[i];
-			*(data++) = veb->tc_stats.tc_rx_bytes[i];
-		}
-	} else {
-		data += I40E_VEB_STATS_TOTAL;
-	}
+	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
+		i40e_add_ethtool_stats(&data, veb_stats ? veb : NULL,
+				       i40e_gstrings_veb_tc_stats);
 
 	i40e_add_ethtool_stats(&data, pf, i40e_gstrings_stats);
 
@@ -1891,16 +1901,21 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
  * @stats: stat definitions array
  * @size: size of the stats array
  *
- * Copy the strings described by stats into the buffer pointed@by p.
+ * Format and copy the strings described by stats into the buffer pointed at
+ * by p.
  **/
 static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
-				    const unsigned int size)
+				    const unsigned int size, ...)
 {
 	unsigned int i;
 
 	for (i = 0; i < size; i++) {
-		snprintf(*p, ETH_GSTRING_LEN, "%s", stats[i].stat_string);
+		va_list args;
+
+		va_start(args, size);
+		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
 		*p += ETH_GSTRING_LEN;
+		va_end(args);
 	}
 }
 
@@ -1914,7 +1929,7 @@ static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
  * for it.
  **/
 #define i40e_add_stat_strings(p, stats, ...) \
-	__i40e_add_stat_strings(p, stats, ARRAY_SIZE(stats))
+	__i40e_add_stat_strings(p, stats, ARRAY_SIZE(stats), ## __VA_ARGS__)
 
 /**
  * i40e_get_stat_strings - copy stat strings into supplied buffer
@@ -1953,20 +1968,8 @@ static void i40e_get_stat_strings(struct net_device *netdev, u8 *data)
 
 	i40e_add_stat_strings(&data, i40e_gstrings_veb_stats);
 
-	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++) {
-		snprintf(data, ETH_GSTRING_LEN,
-			 "veb.tc_%u_tx_packets", i);
-		data += ETH_GSTRING_LEN;
-		snprintf(data, ETH_GSTRING_LEN,
-			 "veb.tc_%u_tx_bytes", i);
-		data += ETH_GSTRING_LEN;
-		snprintf(data, ETH_GSTRING_LEN,
-			 "veb.tc_%u_rx_packets", i);
-		data += ETH_GSTRING_LEN;
-		snprintf(data, ETH_GSTRING_LEN,
-			 "veb.tc_%u_rx_bytes", i);
-		data += ETH_GSTRING_LEN;
-	}
+	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
+		i40e_add_stat_strings(&data, i40e_gstrings_veb_tc_stats, i);
 
 	i40e_add_stat_strings(&data, i40e_gstrings_stats);
 
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 04/11] i40e: convert VEB TC stats to use an i40e_stats array

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 04/11] i40e: convert VEB TC stats
> to use an i40e_stats array
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The VEB TC stats are currently implemented with separate parsing, instead of
> using the i40e_stats array and associated helper functions.
> This is likely because the stats rely on embedding the TC number into the stat
> name.
> 
> Update i40e_add_stat_strings to take variadic arguments, and use these to
> vsnprintf the i40e_stats string as a string containing format specifiers.
> 
> Create a stats array for the VEB TC related stats, i40e_gstrings_veb_tc_stats,
> and use this along with the helper functions to remove the specialized boiler
> plate code.
> 
> Always call i40e_add_ethtool_stats for both this array and the general VEB
> stats array. This ensures that we zero out any memory in case it was not
> zero-allocated for us.
> 
> This ultimately results in less boiler plate code for the i40e_get_stat_strings
> and i40e_get_ethtool_stats.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 83 +++++++++++++---------
> ----
>  1 file changed, 43 insertions(+), 40 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 05/11] i40e: convert priority flow control stats to use helpers

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

The priority flow control statistics are laid out in the stats structure
using arrays. This made it unwieldy to use as part of an i40e_stats
array.

Add a new structure type, i40e_pfc_stats, and a helper function
i40e_get_pfc_stats which can return the stats for a given priority
value as an i40e_pfc_stats structure.

Use this to create an i40e_stats array, which we'll use to format and
copy the strings and stats into the supplied buffers.

This reduces even more boiler plate code in i40e_get_ethtool_stats and
i40e_get_stat_strings.

An alternative would be to modify the structure definition for the pfc
stats, but this is more invasive to the rest of the code base.

Note that a macro was used to setup the copy of stats from the
pf->stats, as this reduces the chance of typos in the code names. It
will produce a checkpatch.pl warning due to re-use of a macro argument.
In this case, it should be safe, as the macro will fail to compile in
cases where the argument is not a simple structure member name, and thus
arguments with side effects should not be an issue.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 87 +++++++++++++++-----------
 1 file changed, 51 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 52ccafe..9c380c0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -31,6 +31,8 @@ struct i40e_stats {
 	I40E_STAT(struct i40e_vsi, _name, _stat)
 #define I40E_VEB_STAT(_name, _stat) \
 	I40E_STAT(struct i40e_veb, _name, _stat)
+#define I40E_PFC_STAT(_name, _stat) \
+	I40E_STAT(struct i40e_pfc_stats, _name, _stat)
 
 static const struct i40e_stats i40e_gstrings_net_stats[] = {
 	I40E_NETDEV_STAT(rx_packets),
@@ -153,6 +155,22 @@ static const struct i40e_stats i40e_gstrings_stats[] = {
 	I40E_PF_STAT("port.rx_lpi_count", stats.rx_lpi_count),
 };
 
+struct i40e_pfc_stats {
+	u64 priority_xon_rx;
+	u64 priority_xoff_rx;
+	u64 priority_xon_tx;
+	u64 priority_xoff_tx;
+	u64 priority_xon_2_xoff;
+};
+
+static const struct i40e_stats i40e_gstrings_pfc_stats[] = {
+	I40E_PFC_STAT("port.tx_priority_%u_xon_tx", priority_xon_tx),
+	I40E_PFC_STAT("port.tx_priority_%u_xoff_tx", priority_xoff_tx),
+	I40E_PFC_STAT("port.rx_priority_%u_xon_rx", priority_xon_rx),
+	I40E_PFC_STAT("port.rx_priority_%u_xoff_rx", priority_xoff_rx),
+	I40E_PFC_STAT("port.rx_priority_%u_xon_2_xoff", priority_xon_2_xoff),
+};
+
 /* We use num_tx_queues here as a proxy for the maximum number of queues
  * available because we always allocate queues symmetrically.
  */
@@ -167,13 +185,9 @@ static const struct i40e_stats i40e_gstrings_stats[] = {
 #define I40E_VSI_STATS_LEN(n)	(I40E_NETDEV_STATS_LEN + \
 				 I40E_MISC_STATS_LEN + \
 				 I40E_QUEUE_STATS_LEN((n)))
-#define I40E_PFC_STATS_LEN ( \
-		(FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_rx) + \
-		 FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_rx) + \
-		 FIELD_SIZEOF(struct i40e_pf, stats.priority_xoff_tx) + \
-		 FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_tx) + \
-		 FIELD_SIZEOF(struct i40e_pf, stats.priority_xon_2_xoff)) \
-		 / sizeof(u64))
+
+#define I40E_PFC_STATS_LEN	(ARRAY_SIZE(i40e_gstrings_pfc_stats) * \
+				 I40E_MAX_USER_PRIORITY)
 
 #define I40E_VEB_STATS_LEN	(ARRAY_SIZE(i40e_gstrings_veb_stats) + \
 				 (ARRAY_SIZE(i40e_gstrings_veb_tc_stats) * \
@@ -1799,6 +1813,31 @@ __i40e_add_ethtool_stats(u64 **data, void *pointer,
 	__i40e_add_ethtool_stats(data, pointer, stats, ARRAY_SIZE(stats))
 
 /**
+ * i40e_get_pfc_stats - copy HW PFC statistics to formatted structure
+ * @pf: the PF device structure
+ * @i: the priority value to copy
+ *
+ * The PFC stats are found as arrays in pf->stats, which is not easy to pass
+ * into i40e_add_ethtool_stats. Produce a formatted i40e_pfc_stats structure
+ * of the PFC stats for the given priority.
+ **/
+static inline struct i40e_pfc_stats
+i40e_get_pfc_stats(struct i40e_pf *pf, unsigned int i)
+{
+#define I40E_GET_PFC_STAT(stat, priority) \
+	.stat = pf->stats.stat[priority]
+
+	struct i40e_pfc_stats pfc = {
+		I40E_GET_PFC_STAT(priority_xon_rx, i),
+		I40E_GET_PFC_STAT(priority_xoff_rx, i),
+		I40E_GET_PFC_STAT(priority_xon_tx, i),
+		I40E_GET_PFC_STAT(priority_xoff_tx, i),
+		I40E_GET_PFC_STAT(priority_xon_2_xoff, i),
+	};
+	return pfc;
+}
+
+/**
  * i40e_get_ethtool_stats - copy stat values into supplied buffer
  * @netdev: the netdev to collect stats for
  * @stats: ethtool stats command structure
@@ -1884,15 +1923,10 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	i40e_add_ethtool_stats(&data, pf, i40e_gstrings_stats);
 
 	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
-		*(data++) = pf->stats.priority_xon_tx[i];
-		*(data++) = pf->stats.priority_xoff_tx[i];
-	}
-	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
-		*(data++) = pf->stats.priority_xon_rx[i];
-		*(data++) = pf->stats.priority_xoff_rx[i];
+		struct i40e_pfc_stats pfc = i40e_get_pfc_stats(pf, i);
+
+		i40e_add_ethtool_stats(&data, &pfc, i40e_gstrings_pfc_stats);
 	}
-	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++)
-		*(data++) = pf->stats.priority_xon_2_xoff[i];
 }
 
 /**
@@ -1973,27 +2007,8 @@ static void i40e_get_stat_strings(struct net_device *netdev, u8 *data)
 
 	i40e_add_stat_strings(&data, i40e_gstrings_stats);
 
-	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
-		snprintf(data, ETH_GSTRING_LEN,
-			 "port.tx_priority_%u_xon", i);
-		data += ETH_GSTRING_LEN;
-		snprintf(data, ETH_GSTRING_LEN,
-			 "port.tx_priority_%u_xoff", i);
-		data += ETH_GSTRING_LEN;
-	}
-	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
-		snprintf(data, ETH_GSTRING_LEN,
-			 "port.rx_priority_%u_xon", i);
-		data += ETH_GSTRING_LEN;
-		snprintf(data, ETH_GSTRING_LEN,
-			 "port.rx_priority_%u_xoff", i);
-		data += ETH_GSTRING_LEN;
-	}
-	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++) {
-		snprintf(data, ETH_GSTRING_LEN,
-			 "port.rx_priority_%u_xon_2_xoff", i);
-		data += ETH_GSTRING_LEN;
-	}
+	for (i = 0; i < I40E_MAX_USER_PRIORITY; i++)
+		i40e_add_stat_strings(&data, i40e_gstrings_pfc_stats, i);
 
 	WARN_ONCE(p - data != i40e_get_stats_count(netdev) * ETH_GSTRING_LEN,
 		  "stat strings count mismatch!");
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 05/11] i40e: convert priority flow control stats to use helpers

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 05/11] i40e: convert priority flow
> control stats to use helpers
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The priority flow control statistics are laid out in the stats structure using
> arrays. This made it unwieldy to use as part of an i40e_stats array.
> 
> Add a new structure type, i40e_pfc_stats, and a helper function
> i40e_get_pfc_stats which can return the stats for a given priority value as an
> i40e_pfc_stats structure.
> 
> Use this to create an i40e_stats array, which we'll use to format and copy the
> strings and stats into the supplied buffers.
> 
> This reduces even more boiler plate code in i40e_get_ethtool_stats and
> i40e_get_stat_strings.
> 
> An alternative would be to modify the structure definition for the pfc stats,
> but this is more invasive to the rest of the code base.
> 
> Note that a macro was used to setup the copy of stats from the
> pf->stats, as this reduces the chance of typos in the code names. It
> will produce a checkpatch.pl warning due to re-use of a macro argument.
> In this case, it should be safe, as the macro will fail to compile in cases where
> the argument is not a simple structure member name, and thus arguments
> with side effects should not be an issue.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 87 +++++++++++++++-----
> ------
>  1 file changed, 51 insertions(+), 36 deletions(-)


Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 06/11] i40e: Write access protected registers through AQC

[Alice Michael]

From: Piotr Azarewicz <piotr.azarewicz@intel.com>

Write access protected registers (GLGEN_RTRIG and GLGEN_RSTCTL) through
AQC.

Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_common.c   | 4 ++++
 drivers/net/ethernet/intel/i40evf/i40e_common.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index eb2d153..ad710b2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -6,6 +6,10 @@
 #include "i40e_prototype.h"
 #include <linux/avf/virtchnl.h>
 
+/* For 32-bit AQ writes. */
+#define wraq32(base_addr, reg, value) i40e_aq_debug_write_register( \
+					(base_addr), (reg), (value), NULL)
+
 /**
  * i40e_set_mac_type - Sets MAC type
  * @hw: pointer to the HW structure
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_common.c b/drivers/net/ethernet/intel/i40evf/i40e_common.c
index eea280ba..04c6c6b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_common.c
@@ -6,6 +6,10 @@
 #include "i40e_prototype.h"
 #include <linux/avf/virtchnl.h>
 
+/* For 32-bit AQ writes. */
+#define wraq32(base_addr, reg, value) i40e_aq_debug_write_register( \
+					(base_addr), (reg), (value), NULL)
+
 /**
  * i40e_set_mac_type - Sets MAC type
  * @hw: pointer to the HW structure
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 06/11] i40e: Write access protected registers through AQC

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Cc: Azarewicz, Piotr <piotr.azarewicz@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S93 06/11] i40e: Write access
> protected registers through AQC
> 
> From: Piotr Azarewicz <piotr.azarewicz@intel.com>
> 
> Write access protected registers (GLGEN_RTRIG and GLGEN_RSTCTL)
> through AQC.
> 
> Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_common.c   | 4 ++++
>  drivers/net/ethernet/intel/i40evf/i40e_common.c | 4 ++++
>  2 files changed, 8 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 07/11] i40e: remove unnecessary i variable causing -Wshadow warning

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

Commit c61c8fe1d592 ("i40e: Implement an ethtool private flag to stop
LLDP in FW") added an extra for-loop which added a shadowing 'i'
variable as the index.

However, the local variable i already exists, and we already use it as
a loop index. Additionally, at this point, there is no further use of
the variable, so it's safe to simply overwrite the variable contents.

This fixes a -Wshadow warning which has started being enabled on some
distributions

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Malek, Patryk <patryk.malek@intel.com>
Change-ID: I1d2259d27ccab6927efd40fb1853af74678680b8
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 9c380c0..12d279a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -4642,7 +4642,6 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 	if (changed_flags & I40E_FLAG_DISABLE_FW_LLDP) {
 		if (pf->flags & I40E_FLAG_DISABLE_FW_LLDP) {
 			struct i40e_dcbx_config *dcbcfg;
-			int i;
 
 			i40e_aq_stop_lldp(&pf->hw, true, NULL);
 			i40e_aq_set_dcb_parameters(&pf->hw, true, NULL);
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 07/11] i40e: remove unnecessary i variable causing -Wshadow warning

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 07/11] i40e: remove unnecessary i
> variable causing -Wshadow warning
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Commit c61c8fe1d592 ("i40e: Implement an ethtool private flag to stop LLDP
> in FW") added an extra for-loop which added a shadowing 'i'
> variable as the index.
> 
> However, the local variable i already exists, and we already use it as a loop
> index. Additionally, at this point, there is no further use of the variable, so it's
> safe to simply overwrite the variable contents.
> 
> This fixes a -Wshadow warning which has started being enabled on some
> distributions
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Malek, Patryk <patryk.malek@intel.com>
> Change-ID: I1d2259d27ccab6927efd40fb1853af74678680b8
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 1 -
>  1 file changed, 1 deletion(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 08/11] i40e: fix warning about shadowed ring parameter

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

In commit 147e81ec7568 ("i40e: Test memory before ethtool alloc succeeds")
code was added to handle ring allocation on systems with low memory.

It shadowed the ring parameter pointer by introducing a local ring
pointer inside the for loop. Most of the code in the loop already just
accessed the ring via &rx_rings[i]. Since most of the code already does
this, just remove the local variable.

If someone considers it worth keeping a local around, they should use it
for the whole section instead of just a couple of accesses.

This fixes a warning when -Wshadow is enabled

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

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 12d279a..cd23d1e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1589,7 +1589,6 @@ static int i40e_set_ringparam(struct net_device *netdev,
 		}
 
 		for (i = 0; i < vsi->num_queue_pairs; i++) {
-			struct i40e_ring *ring;
 			u16 unused;
 
 			/* clone ring and setup updated count */
@@ -1613,9 +1612,8 @@ static int i40e_set_ringparam(struct net_device *netdev,
 			/* now allocate the Rx buffers to make sure the OS
 			 * has enough memory, any failure here means abort
 			 */
-			ring = &rx_rings[i];
-			unused = I40E_DESC_UNUSED(ring);
-			err = i40e_alloc_rx_buffers(ring, unused);
+			unused = I40E_DESC_UNUSED(&rx_rings[i]);
+			err = i40e_alloc_rx_buffers(&rx_rings[i], unused);
 rx_unwind:
 			if (err) {
 				do {
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 08/11] i40e: fix warning about shadowed ring parameter

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 08/11] i40e: fix warning about
> shadowed ring parameter
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> In commit 147e81ec7568 ("i40e: Test memory before ethtool alloc succeeds")
> code was added to handle ring allocation on systems with low memory.
> 
> It shadowed the ring parameter pointer by introducing a local ring pointer
> inside the for loop. Most of the code in the loop already just accessed the
> ring via &rx_rings[i]. Since most of the code already does this, just remove
> the local variable.
> 
> If someone considers it worth keeping a local around, they should use it for
> the whole section instead of just a couple of accesses.
> 
> This fixes a warning when -Wshadow is enabled
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 09/11] i40e: Add additional return code to i40e_asq_send_command

[Alice Michael]

From: Piotr Azarewicz <piotr.azarewicz@intel.com>

Firmware can return a busy state, so the function return
I40E_ERR_NOT_READY.

Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c   | 2 ++
 drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index ddbea79..501ee71 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -868,6 +868,8 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 		cmd_completed = true;
 		if ((enum i40e_admin_queue_err)retval == I40E_AQ_RC_OK)
 			status = 0;
+		else if ((enum i40e_admin_queue_err)retval == I40E_AQ_RC_EBUSY)
+			status = I40E_ERR_NOT_READY;
 		else
 			status = I40E_ERR_ADMIN_QUEUE_ERROR;
 		hw->aq.asq_last_status = (enum i40e_admin_queue_err)retval;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
index c355120..21a0dbf 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
@@ -797,6 +797,8 @@ i40e_status i40evf_asq_send_command(struct i40e_hw *hw,
 		cmd_completed = true;
 		if ((enum i40e_admin_queue_err)retval == I40E_AQ_RC_OK)
 			status = 0;
+		else if ((enum i40e_admin_queue_err)retval == I40E_AQ_RC_EBUSY)
+			status = I40E_ERR_NOT_READY;
 		else
 			status = I40E_ERR_ADMIN_QUEUE_ERROR;
 		hw->aq.asq_last_status = (enum i40e_admin_queue_err)retval;
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 09/11] i40e: Add additional return code to i40e_asq_send_command

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Cc: Azarewicz, Piotr <piotr.azarewicz@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S93 09/11] i40e: Add additional return
> code to i40e_asq_send_command
> 
> From: Piotr Azarewicz <piotr.azarewicz@intel.com>
> 
> Firmware can return a busy state, so the function return
> I40E_ERR_NOT_READY.
> 
> Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_adminq.c   | 2 ++
>  drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 2 ++
>  2 files changed, 4 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 10/11] i40e: Add AQ command for rearrange NVM structure

[Alice Michael]

From: Piotr Azarewicz <piotr.azarewicz@intel.com>

During switching between old NVM structure approach (called structured
NVM) to new one (called flat NVM) or backward flash needs to be
rearranged to required NVM structure. This is a part of transition from
one NVM structure to another. The function is introduced to command
firmware to start rearrangement process.

Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h  |  2 ++
 drivers/net/ethernet/intel/i40e/i40e_common.c      | 35 ++++++++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_prototype.h   |  3 ++
 .../net/ethernet/intel/i40evf/i40e_adminq_cmd.h    |  2 ++
 4 files changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
index 7d888e0..80e3eec 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
@@ -2247,6 +2247,8 @@ I40E_CHECK_CMD_LENGTH(i40e_aqc_phy_register_access);
 struct i40e_aqc_nvm_update {
 	u8	command_flags;
 #define I40E_AQ_NVM_LAST_CMD			0x01
+#define I40E_AQ_NVM_REARRANGE_TO_FLAT		0x20
+#define I40E_AQ_NVM_REARRANGE_TO_STRUCT		0x40
 #define I40E_AQ_NVM_FLASH_ONLY			0x80
 #define I40E_AQ_NVM_PRESERVATION_FLAGS_SHIFT	1
 #define I40E_AQ_NVM_PRESERVATION_FLAGS_MASK	0x03
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index ad710b2..cc196de 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -3545,6 +3545,41 @@ i40e_status i40e_aq_update_nvm(struct i40e_hw *hw, u8 module_pointer,
 }
 
 /**
+ * i40e_aq_rearrange_nvm
+ * @hw: pointer to the hw struct
+ * @rearrange_nvm: defines direction of rearrangement
+ * @cmd_details: pointer to command details structure or NULL
+ *
+ * Rearrange NVM structure, available only for transition FW
+ **/
+i40e_status i40e_aq_rearrange_nvm(struct i40e_hw *hw,
+				  u8 rearrange_nvm,
+				  struct i40e_asq_cmd_details *cmd_details)
+{
+	struct i40e_aqc_nvm_update *cmd;
+	i40e_status status;
+	struct i40e_aq_desc desc;
+
+	cmd = (struct i40e_aqc_nvm_update *)&desc.params.raw;
+
+	i40e_fill_default_direct_cmd_desc(&desc, i40e_aqc_opc_nvm_update);
+
+	rearrange_nvm &= (I40E_AQ_NVM_REARRANGE_TO_FLAT |
+			 I40E_AQ_NVM_REARRANGE_TO_STRUCT);
+
+	if (!rearrange_nvm) {
+		status = I40E_ERR_PARAM;
+		goto i40e_aq_rearrange_nvm_exit;
+	}
+
+	cmd->command_flags |= rearrange_nvm;
+	status = i40e_asq_send_command(hw, &desc, NULL, 0, cmd_details);
+
+i40e_aq_rearrange_nvm_exit:
+	return status;
+}
+
+/**
  * i40e_aq_get_lldp_mib
  * @hw: pointer to the hw struct
  * @bridge_type: type of bridge requested
diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index 3170655..e08d754 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -193,6 +193,9 @@ i40e_status i40e_aq_update_nvm(struct i40e_hw *hw, u8 module_pointer,
 				u32 offset, u16 length, void *data,
 				bool last_command, u8 preservation_flags,
 				struct i40e_asq_cmd_details *cmd_details);
+i40e_status i40e_aq_rearrange_nvm(struct i40e_hw *hw,
+				  u8 rearrange_nvm,
+				  struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_get_lldp_mib(struct i40e_hw *hw, u8 bridge_type,
 				u8 mib_type, void *buff, u16 buff_size,
 				u16 *local_len, u16 *remote_len,
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h
index aa81e87..5fd8529 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq_cmd.h
@@ -2175,6 +2175,8 @@ I40E_CHECK_CMD_LENGTH(i40e_aqc_phy_register_access);
 struct i40e_aqc_nvm_update {
 	u8	command_flags;
 #define I40E_AQ_NVM_LAST_CMD			0x01
+#define I40E_AQ_NVM_REARRANGE_TO_FLAT		0x20
+#define I40E_AQ_NVM_REARRANGE_TO_STRUCT		0x40
 #define I40E_AQ_NVM_FLASH_ONLY			0x80
 #define I40E_AQ_NVM_PRESERVATION_FLAGS_SHIFT	1
 #define I40E_AQ_NVM_PRESERVATION_FLAGS_MASK	0x03
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 10/11] i40e: Add AQ command for rearrange NVM structure

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Cc: Azarewicz, Piotr <piotr.azarewicz@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S93 10/11] i40e: Add AQ command for
> rearrange NVM structure
> 
> From: Piotr Azarewicz <piotr.azarewicz@intel.com>
> 
> During switching between old NVM structure approach (called structured
> NVM) to new one (called flat NVM) or backward flash needs to be
> rearranged to required NVM structure. This is a part of transition from one
> NVM structure to another. The function is introduced to command firmware
> to start rearrangement process.
> 
> Signed-off-by: Piotr Azarewicz <piotr.azarewicz@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h  |  2 ++
>  drivers/net/ethernet/intel/i40e/i40e_common.c      | 35
> ++++++++++++++++++++++
>  drivers/net/ethernet/intel/i40e/i40e_prototype.h   |  3 ++
>  .../net/ethernet/intel/i40evf/i40e_adminq_cmd.h    |  2 ++
>  4 files changed, 42 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 11/11] i40e: fix i40e_add_queue_stats data pointer update

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

This function accidentally failed to update the data pointer, which
caused the reported stats to be incorrect. Additionally, statistics
which follow queue stats in the output would potentially read non-zeroed
garbage data from the ethtool buffer.

This occurred because the data double pointer was not dereferenced
before incrementing the size.

Additionally, make sure this issue is more visible by adding a WARN_ONCE
to the i40e_get_ethtool_stats function. This warning will trigger
whenever the data pointer is not at the expected address, similar to the
check that we make in the i40e_get_stat_strings() function.

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

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index cd23d1e..abcd096 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1860,6 +1860,7 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	unsigned int i;
 	unsigned int start;
 	bool veb_stats;
+	u64 *p = data;
 
 	i40e_update_stats(vsi);
 
@@ -1902,7 +1903,7 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 	}
 	rcu_read_unlock();
 	if (vsi != pf->vsi[pf->lan_vsi] || pf->hw.partition_id != 1)
-		return;
+		goto check_data_pointer;
 
 	veb_stats = ((pf->lan_veb != I40E_NO_VEB) &&
 		     (pf->flags & I40E_FLAG_VEB_STATS_ENABLED));
@@ -1925,6 +1926,10 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 
 		i40e_add_ethtool_stats(&data, &pfc, i40e_gstrings_pfc_stats);
 	}
+
+check_data_pointer:
+	WARN_ONCE(data - p != i40e_get_stats_count(netdev),
+		  "ethtool stats count mismatch!");
 }
 
 /**
-- 
2.9.5

[Intel-wired-lan] [next PATCH S93 11/11] i40e: fix i40e_add_queue_stats data pointer update

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 11/11] i40e: fix
> i40e_add_queue_stats data pointer update
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> This function accidentally failed to update the data pointer, which caused the
> reported stats to be incorrect. Additionally, statistics which follow queue
> stats in the output would potentially read non-zeroed garbage data from the
> ethtool buffer.
> 
> This occurred because the data double pointer was not dereferenced before
> incrementing the size.
> 
> Additionally, make sure this issue is more visible by adding a WARN_ONCE to
> the i40e_get_ethtool_stats function. This warning will trigger whenever the
> data pointer is not at the expected address, similar to the check that we
> make in the i40e_get_stat_strings() function.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S93 01/11] i40e: add helper function for copying strings from stat arrays

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Tuesday, July 31, 2018 3:42 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S93 01/11] i40e: add helper function
> for copying strings from stat arrays
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Many of the ethtool statistics use the same basic logic for copying strings into
> the supplied buffer. A set of stats are stored in a const array of i40e_stats
> structures, and we apply these all together.
> 
> Simplify the stats code by introducing a helper function which can take a stats
> array and copy the strings into the buffer, updating the buffer pointer as we
> go.
> 
> We use a macro to implement i40e_add_stat_strings so that ARRAY_SIZE can
> be used on the array passed in. This ensures that we always use the
> matching size in __i40e_add_stat_strings.
> 
> More complex stats currently do not use i40e_stats arrays, usually due to
> custom formatted strings, or because the stats are not laid out in the
> expected way. These stats will be updated to use the helper function in
> separate future patches.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 59 +++++++++++++++++--
> -------
>  1 file changed, 39 insertions(+), 20 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>