[PATCH] tg3: Make the RSS indir tbl admin configurable

[PATCH] tg3: Make the RSS indir tbl admin configurable

[Matt Carlson]

This patch adds the ethtool callbacks necessary to change the rss
indirection table from userspace.  Should the number of interrupts
change (e.g.  across a close / open call, or through a reset) and
any one of the indirection table values fall out-of-range, the driver
will reset the indirection table to a default layout.

[Integrated many suggestions made by Ben Hutchings.]

Changes since v3

* Removed TG3_FLAG_SUPPORT_MSIX checks at the start of
  tg3_get_rxfh_indir() and tg3_set_rxfh_indir().

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |  108 ++++++++++++++++++++++++++++++++--
 1 files changed, 101 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 8bf11ca..0021396 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8220,21 +8220,38 @@ static void tg3_setup_rxbd_thresholds(struct tg3 *tp)
 		tw32(JMB_REPLENISH_LWM, bdcache_maxcnt);
 }
 
-void tg3_rss_init_indir_tbl(struct tg3 *tp)
+static void tg3_rss_init_dflt_indir_tbl(struct tg3 *tp)
+{
+	int i;
+
+	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+		tp->rss_ind_tbl[i] =
+			ethtool_rxfh_indir_default(i, tp->irq_cnt - 1);
+}
+
+static void tg3_rss_check_indir_tbl(struct tg3 *tp)
 {
 	int i;
 
 	if (!tg3_flag(tp, SUPPORT_MSIX))
 		return;
 
-	if (tp->irq_cnt <= 2)
+	if (tp->irq_cnt <= 2) {
 		memset(&tp->rss_ind_tbl[0], 0, sizeof(tp->rss_ind_tbl));
-	else
-		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
-			tp->rss_ind_tbl[i] = i % (tp->irq_cnt - 1);
+		return;
+	}
+
+	/* Validate table against current IRQ count */
+	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++) {
+		if (tp->rss_ind_tbl[i] >= tp->irq_cnt - 1)
+			break;
+	}
+
+	if (i != TG3_RSS_INDIR_TBL_SIZE)
+		tg3_rss_init_dflt_indir_tbl(tp);
 }
 
-void tg3_rss_write_indir_tbl(struct tg3 *tp)
+static void tg3_rss_write_indir_tbl(struct tg3 *tp)
 {
 	int i = 0;
 	u32 reg = MAC_RSS_INDIR_TBL_0;
@@ -9668,7 +9685,7 @@ static int tg3_open(struct net_device *dev)
 	 */
 	tg3_ints_init(tp);
 
-	tg3_rss_init_indir_tbl(tp);
+	tg3_rss_check_indir_tbl(tp);
 
 	/* The placement of this call is tied
 	 * to the setup and use of Host TX descriptors.
@@ -10719,6 +10736,78 @@ static int tg3_get_sset_count(struct net_device *dev, int sset)
 	}
 }
 
+static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
+			 u32 *rules __always_unused)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	switch (info->cmd) {
+	case ETHTOOL_GRXRINGS:
+		if (netif_running(tp->dev))
+			info->data = tp->irq_cnt;
+		else {
+			info->data = num_online_cpus();
+			if (info->data > TG3_IRQ_MAX_VECS_RSS)
+				info->data = TG3_IRQ_MAX_VECS_RSS;
+		}
+
+		/* The first interrupt vector only
+		 * handles link interrupts.
+		 */
+		info->data -= 1;
+		return 0;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static u32 tg3_get_rxfh_indir_size(struct net_device *dev)
+{
+	u32 size = 0;
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (tg3_flag(tp, SUPPORT_MSIX))
+		size = TG3_RSS_INDIR_TBL_SIZE;
+
+	return size;
+}
+
+static int tg3_get_rxfh_indir(struct net_device *dev, u32 *indir)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	int i;
+
+	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+		indir[i] = tp->rss_ind_tbl[i];
+
+	return 0;
+}
+
+static int tg3_set_rxfh_indir(struct net_device *dev, const u32 *indir)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	size_t i;
+
+	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+		tp->rss_ind_tbl[i] = indir[i];
+
+	if (!netif_running(dev) || !tg3_flag(tp, ENABLE_RSS))
+		return 0;
+
+	/* It is legal to write the indirection
+	 * table while the device is running.
+	 */
+	tg3_full_lock(tp, 0);
+	tg3_rss_write_indir_tbl(tp);
+	tg3_full_unlock(tp);
+
+	return 0;
+}
+
 static void tg3_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 {
 	switch (stringset) {
@@ -11949,6 +12038,10 @@ static const struct ethtool_ops tg3_ethtool_ops = {
 	.get_coalesce		= tg3_get_coalesce,
 	.set_coalesce		= tg3_set_coalesce,
 	.get_sset_count		= tg3_get_sset_count,
+	.get_rxnfc		= tg3_get_rxnfc,
+	.get_rxfh_indir_size    = tg3_get_rxfh_indir_size,
+	.get_rxfh_indir		= tg3_get_rxfh_indir,
+	.set_rxfh_indir		= tg3_set_rxfh_indir,
 };
 
 static void __devinit tg3_get_eeprom_size(struct tg3 *tp)
@@ -14051,6 +14144,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 		if (tg3_flag(tp, 57765_PLUS)) {
 			tg3_flag_set(tp, SUPPORT_MSIX);
 			tp->irq_max = TG3_IRQ_MAX_VECS;
+			tg3_rss_init_dflt_indir_tbl(tp);
 		}
 	}
 
-- 
1.7.3.4

Re: [PATCH] tg3: Make the RSS indir tbl admin configurable

[David Miller]

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Fri, 16 Dec 2011 15:33:23 -0800

> This patch adds the ethtool callbacks necessary to change the rss
> indirection table from userspace.  Should the number of interrupts
> change (e.g.  across a close / open call, or through a reset) and
> any one of the indirection table values fall out-of-range, the driver
> will reset the indirection table to a default layout.
> 
> [Integrated many suggestions made by Ben Hutchings.]
> 
> Changes since v3
> 
> * Removed TG3_FLAG_SUPPORT_MSIX checks at the start of
>   tg3_get_rxfh_indir() and tg3_set_rxfh_indir().
> 
> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Reviewed-by: Benjamin Li <benli@broadcom.com>

Applied.

[PATCH] tg3: Make the RSS indir tbl admin configurable

[Matt Carlson]

This patch adds the ethtool callbacks necessary to change the rss
indirection table from userspace.  When setting the indirection table
through set_rxfh_indir, an indirection table size of zero is
interpreted to mean that the admin wants to relinquish control of the
table to the driver.  Should the number of interrupts change (e.g.
across a close / open call, or through a reset), any indirection table
values that exceed the number of RSS queues or interrupt vectors will
be automatically scaled back to values within range.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |  126 ++++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/tg3.h |    1 +
 2 files changed, 126 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 8bf11ca..87f70f6 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8229,9 +8229,19 @@ void tg3_rss_init_indir_tbl(struct tg3 *tp)
 
 	if (tp->irq_cnt <= 2)
 		memset(&tp->rss_ind_tbl[0], 0, sizeof(tp->rss_ind_tbl));
-	else
+	else if (tg3_flag(tp, USER_INDIR_TBL)) {
+		/* Validate table against current IRQ count */
+		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++) {
+			if (tp->rss_ind_tbl[i] >= tp->irq_cnt - 1) {
+				/* Bring the index within range */
+				tp->rss_ind_tbl[i] = tp->rss_ind_tbl[i] %
+						     (tp->irq_cnt - 1);
+			}
+		}
+	} else {
 		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
 			tp->rss_ind_tbl[i] = i % (tp->irq_cnt - 1);
+	}
 }
 
 void tg3_rss_write_indir_tbl(struct tg3 *tp)
@@ -10719,6 +10729,117 @@ static int tg3_get_sset_count(struct net_device *dev, int sset)
 	}
 }
 
+static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
+			 u32 *rules __always_unused)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	switch (info->cmd) {
+	case ETHTOOL_GRXRINGS:
+		if (netif_running(tp->dev))
+			info->data = tp->irq_cnt;
+		else {
+			info->data = num_online_cpus();
+			if (info->data > TG3_IRQ_MAX_VECS_RSS)
+				info->data = TG3_IRQ_MAX_VECS_RSS;
+		}
+
+		/* The first interrupt vector only
+		 * handles link interrupts.
+		 */
+		info->data -= 1;
+		return 0;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int tg3_get_rxfh_indir(struct net_device *dev,
+			      struct ethtool_rxfh_indir *indir)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	int i;
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	if (!indir->size) {
+		indir->size = TG3_RSS_INDIR_TBL_SIZE;
+		return 0;
+	}
+
+	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
+		return -EINVAL;
+
+	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+		indir->ring_index[i] = tp->rss_ind_tbl[i];
+
+	return 0;
+}
+
+static int tg3_set_rxfh_indir(struct net_device *dev,
+			      const struct ethtool_rxfh_indir *indir)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	size_t i;
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	if (!indir->size) {
+		tg3_flag_clear(tp, USER_INDIR_TBL);
+		tg3_rss_init_indir_tbl(tp);
+	} else {
+		int limit;
+
+		/* Validate size and indices */
+		if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
+			return -EINVAL;
+
+		if (netif_running(dev))
+			limit = tp->irq_cnt;
+		else {
+			limit = num_online_cpus();
+			if (limit > TG3_IRQ_MAX_VECS_RSS)
+				limit = TG3_IRQ_MAX_VECS_RSS;
+		}
+
+		/* The first interrupt vector only
+		 * handles link interrupts.
+		 */
+		limit -= 1;
+
+		/* Check the indices in the table.
+		 * Leave the existing table unmodified
+		 * if an error is detected.
+		 */
+		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+			if (indir->ring_index[i] >= limit)
+				return -EINVAL;
+
+		tg3_flag_set(tp, USER_INDIR_TBL);
+
+		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+			tp->rss_ind_tbl[i] = indir->ring_index[i];
+	}
+
+	if (!netif_running(dev) || !tg3_flag(tp, ENABLE_RSS))
+		return 0;
+
+	/* It is legal to write the indirection
+	 * table while the device is running.
+	 */
+	tg3_full_lock(tp, 0);
+	tg3_rss_write_indir_tbl(tp);
+	tg3_full_unlock(tp);
+
+	return 0;
+}
+
 static void tg3_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 {
 	switch (stringset) {
@@ -11949,6 +12070,9 @@ static const struct ethtool_ops tg3_ethtool_ops = {
 	.get_coalesce		= tg3_get_coalesce,
 	.set_coalesce		= tg3_set_coalesce,
 	.get_sset_count		= tg3_get_sset_count,
+	.get_rxnfc		= tg3_get_rxnfc,
+	.get_rxfh_indir		= tg3_get_rxfh_indir,
+	.set_rxfh_indir		= tg3_set_rxfh_indir,
 };
 
 static void __devinit tg3_get_eeprom_size(struct tg3 *tp)
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index aea8f72..4800595 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2932,6 +2932,7 @@ enum TG3_FLAGS {
 	TG3_FLAG_APE_HAS_NCSI,
 	TG3_FLAG_4K_FIFO_LIMIT,
 	TG3_FLAG_RESET_TASK_PENDING,
+	TG3_FLAG_USER_INDIR_TBL,
 	TG3_FLAG_5705_PLUS,
 	TG3_FLAG_IS_5788,
 	TG3_FLAG_5750_PLUS,
-- 
1.7.3.4

Re: [PATCH] tg3: Make the RSS indir tbl admin configurable

[Ben Hutchings]

On Thu, 2011-12-15 at 16:47 -0800, Matt Carlson wrote:
> This patch adds the ethtool callbacks necessary to change the rss
> indirection table from userspace.  When setting the indirection table
> through set_rxfh_indir, an indirection table size of zero is
> interpreted to mean that the admin wants to relinquish control of the
> table to the driver.  Should the number of interrupts change (e.g.
> across a close / open call, or through a reset), any indirection table
> values that exceed the number of RSS queues or interrupt vectors will
> be automatically scaled back to values within range.
> 
> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

No I didn't!

Ben.

> Reviewed-by: Benjamin Li <benli@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c |  126 ++++++++++++++++++++++++++++++++++-
>  drivers/net/ethernet/broadcom/tg3.h |    1 +
>  2 files changed, 126 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 8bf11ca..87f70f6 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -8229,9 +8229,19 @@ void tg3_rss_init_indir_tbl(struct tg3 *tp)
>  
>  	if (tp->irq_cnt <= 2)
>  		memset(&tp->rss_ind_tbl[0], 0, sizeof(tp->rss_ind_tbl));
> -	else
> +	else if (tg3_flag(tp, USER_INDIR_TBL)) {
> +		/* Validate table against current IRQ count */
> +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++) {
> +			if (tp->rss_ind_tbl[i] >= tp->irq_cnt - 1) {
> +				/* Bring the index within range */
> +				tp->rss_ind_tbl[i] = tp->rss_ind_tbl[i] %
> +						     (tp->irq_cnt - 1);
> +			}
> +		}
> +	} else {
>  		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
>  			tp->rss_ind_tbl[i] = i % (tp->irq_cnt - 1);
> +	}
>  }
>  
>  void tg3_rss_write_indir_tbl(struct tg3 *tp)
> @@ -10719,6 +10729,117 @@ static int tg3_get_sset_count(struct net_device *dev, int sset)
>  	}
>  }
>  
> +static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
> +			 u32 *rules __always_unused)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +
> +	if (!tg3_flag(tp, SUPPORT_MSIX))
> +		return -EOPNOTSUPP;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		if (netif_running(tp->dev))
> +			info->data = tp->irq_cnt;
> +		else {
> +			info->data = num_online_cpus();
> +			if (info->data > TG3_IRQ_MAX_VECS_RSS)
> +				info->data = TG3_IRQ_MAX_VECS_RSS;
> +		}
> +
> +		/* The first interrupt vector only
> +		 * handles link interrupts.
> +		 */
> +		info->data -= 1;
> +		return 0;
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int tg3_get_rxfh_indir(struct net_device *dev,
> +			      struct ethtool_rxfh_indir *indir)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +	int i;
> +
> +	if (!tg3_flag(tp, SUPPORT_MSIX))
> +		return -EOPNOTSUPP;
> +
> +	if (!indir->size) {
> +		indir->size = TG3_RSS_INDIR_TBL_SIZE;
> +		return 0;
> +	}
> +
> +	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> +		return -EINVAL;
> +
> +	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> +		indir->ring_index[i] = tp->rss_ind_tbl[i];
> +
> +	return 0;
> +}
> +
> +static int tg3_set_rxfh_indir(struct net_device *dev,
> +			      const struct ethtool_rxfh_indir *indir)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +	size_t i;
> +
> +	if (!tg3_flag(tp, SUPPORT_MSIX))
> +		return -EOPNOTSUPP;
> +
> +	if (!indir->size) {
> +		tg3_flag_clear(tp, USER_INDIR_TBL);
> +		tg3_rss_init_indir_tbl(tp);
> +	} else {
> +		int limit;
> +
> +		/* Validate size and indices */
> +		if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> +			return -EINVAL;
> +
> +		if (netif_running(dev))
> +			limit = tp->irq_cnt;
> +		else {
> +			limit = num_online_cpus();
> +			if (limit > TG3_IRQ_MAX_VECS_RSS)
> +				limit = TG3_IRQ_MAX_VECS_RSS;
> +		}
> +
> +		/* The first interrupt vector only
> +		 * handles link interrupts.
> +		 */
> +		limit -= 1;
> +
> +		/* Check the indices in the table.
> +		 * Leave the existing table unmodified
> +		 * if an error is detected.
> +		 */
> +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> +			if (indir->ring_index[i] >= limit)
> +				return -EINVAL;
> +
> +		tg3_flag_set(tp, USER_INDIR_TBL);
> +
> +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> +			tp->rss_ind_tbl[i] = indir->ring_index[i];
> +	}
> +
> +	if (!netif_running(dev) || !tg3_flag(tp, ENABLE_RSS))
> +		return 0;
> +
> +	/* It is legal to write the indirection
> +	 * table while the device is running.
> +	 */
> +	tg3_full_lock(tp, 0);
> +	tg3_rss_write_indir_tbl(tp);
> +	tg3_full_unlock(tp);
> +
> +	return 0;
> +}
> +
>  static void tg3_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
>  {
>  	switch (stringset) {
> @@ -11949,6 +12070,9 @@ static const struct ethtool_ops tg3_ethtool_ops = {
>  	.get_coalesce		= tg3_get_coalesce,
>  	.set_coalesce		= tg3_set_coalesce,
>  	.get_sset_count		= tg3_get_sset_count,
> +	.get_rxnfc		= tg3_get_rxnfc,
> +	.get_rxfh_indir		= tg3_get_rxfh_indir,
> +	.set_rxfh_indir		= tg3_set_rxfh_indir,
>  };
>  
>  static void __devinit tg3_get_eeprom_size(struct tg3 *tp)
> diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
> index aea8f72..4800595 100644
> --- a/drivers/net/ethernet/broadcom/tg3.h
> +++ b/drivers/net/ethernet/broadcom/tg3.h
> @@ -2932,6 +2932,7 @@ enum TG3_FLAGS {
>  	TG3_FLAG_APE_HAS_NCSI,
>  	TG3_FLAG_4K_FIFO_LIMIT,
>  	TG3_FLAG_RESET_TASK_PENDING,
> +	TG3_FLAG_USER_INDIR_TBL,
>  	TG3_FLAG_5705_PLUS,
>  	TG3_FLAG_IS_5788,
>  	TG3_FLAG_5750_PLUS,

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] tg3: Make the RSS indir tbl admin configurable

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 16 Dec 2011 03:04:34 +0000

> On Thu, 2011-12-15 at 16:47 -0800, Matt Carlson wrote:
>> This patch adds the ethtool callbacks necessary to change the rss
>> indirection table from userspace.  When setting the indirection table
>> through set_rxfh_indir, an indirection table size of zero is
>> interpreted to mean that the admin wants to relinquish control of the
>> table to the driver.  Should the number of interrupts change (e.g.
>> across a close / open call, or through a reset), any indirection table
>> values that exceed the number of RSS queues or interrupt vectors will
>> be automatically scaled back to values within range.
>> 
>> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
>> Signed-off-by: Michael Chan <mchan@broadcom.com>
>> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> 
> No I didn't!

Matt, really, don't do crap like this.  It's extremely rude and
completely unprofessional.

Re: [PATCH] tg3: Make the RSS indir tbl admin configurable

[Matt Carlson]

On Thu, Dec 15, 2011 at 07:04:34PM -0800, Ben Hutchings wrote:
> On Thu, 2011-12-15 at 16:47 -0800, Matt Carlson wrote:
> > This patch adds the ethtool callbacks necessary to change the rss
> > indirection table from userspace.  When setting the indirection table
> > through set_rxfh_indir, an indirection table size of zero is
> > interpreted to mean that the admin wants to relinquish control of the
> > table to the driver.  Should the number of interrupts change (e.g.
> > across a close / open call, or through a reset), any indirection table
> > values that exceed the number of RSS queues or interrupt vectors will
> > be automatically scaled back to values within range.
> > 
> > Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> 
> No I didn't!
> 
> Ben.

Sorry.  This part of procedure I'm unsure about.  I didn't want to lose
the ack of your contributions to this patch.  The SOB does imply that
you are O.K. with it as-is though, which you may or may not be aligned
with.  I apologize if I was putting words in your mouth.

What should I have done instead?  Would Dave put the SOB on your behalf
as he accepts it?

> > Reviewed-by: Benjamin Li <benli@broadcom.com>
> > ---
> >  drivers/net/ethernet/broadcom/tg3.c |  126 ++++++++++++++++++++++++++++++++++-
> >  drivers/net/ethernet/broadcom/tg3.h |    1 +
> >  2 files changed, 126 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> > index 8bf11ca..87f70f6 100644
> > --- a/drivers/net/ethernet/broadcom/tg3.c
> > +++ b/drivers/net/ethernet/broadcom/tg3.c
> > @@ -8229,9 +8229,19 @@ void tg3_rss_init_indir_tbl(struct tg3 *tp)
> >  
> >  	if (tp->irq_cnt <= 2)
> >  		memset(&tp->rss_ind_tbl[0], 0, sizeof(tp->rss_ind_tbl));
> > -	else
> > +	else if (tg3_flag(tp, USER_INDIR_TBL)) {
> > +		/* Validate table against current IRQ count */
> > +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++) {
> > +			if (tp->rss_ind_tbl[i] >= tp->irq_cnt - 1) {
> > +				/* Bring the index within range */
> > +				tp->rss_ind_tbl[i] = tp->rss_ind_tbl[i] %
> > +						     (tp->irq_cnt - 1);
> > +			}
> > +		}
> > +	} else {
> >  		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> >  			tp->rss_ind_tbl[i] = i % (tp->irq_cnt - 1);
> > +	}
> >  }
> >  
> >  void tg3_rss_write_indir_tbl(struct tg3 *tp)
> > @@ -10719,6 +10729,117 @@ static int tg3_get_sset_count(struct net_device *dev, int sset)
> >  	}
> >  }
> >  
> > +static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
> > +			 u32 *rules __always_unused)
> > +{
> > +	struct tg3 *tp = netdev_priv(dev);
> > +
> > +	if (!tg3_flag(tp, SUPPORT_MSIX))
> > +		return -EOPNOTSUPP;
> > +
> > +	switch (info->cmd) {
> > +	case ETHTOOL_GRXRINGS:
> > +		if (netif_running(tp->dev))
> > +			info->data = tp->irq_cnt;
> > +		else {
> > +			info->data = num_online_cpus();
> > +			if (info->data > TG3_IRQ_MAX_VECS_RSS)
> > +				info->data = TG3_IRQ_MAX_VECS_RSS;
> > +		}
> > +
> > +		/* The first interrupt vector only
> > +		 * handles link interrupts.
> > +		 */
> > +		info->data -= 1;
> > +		return 0;
> > +
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +}
> > +
> > +static int tg3_get_rxfh_indir(struct net_device *dev,
> > +			      struct ethtool_rxfh_indir *indir)
> > +{
> > +	struct tg3 *tp = netdev_priv(dev);
> > +	int i;
> > +
> > +	if (!tg3_flag(tp, SUPPORT_MSIX))
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!indir->size) {
> > +		indir->size = TG3_RSS_INDIR_TBL_SIZE;
> > +		return 0;
> > +	}
> > +
> > +	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> > +		indir->ring_index[i] = tp->rss_ind_tbl[i];
> > +
> > +	return 0;
> > +}
> > +
> > +static int tg3_set_rxfh_indir(struct net_device *dev,
> > +			      const struct ethtool_rxfh_indir *indir)
> > +{
> > +	struct tg3 *tp = netdev_priv(dev);
> > +	size_t i;
> > +
> > +	if (!tg3_flag(tp, SUPPORT_MSIX))
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!indir->size) {
> > +		tg3_flag_clear(tp, USER_INDIR_TBL);
> > +		tg3_rss_init_indir_tbl(tp);
> > +	} else {
> > +		int limit;
> > +
> > +		/* Validate size and indices */
> > +		if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> > +			return -EINVAL;
> > +
> > +		if (netif_running(dev))
> > +			limit = tp->irq_cnt;
> > +		else {
> > +			limit = num_online_cpus();
> > +			if (limit > TG3_IRQ_MAX_VECS_RSS)
> > +				limit = TG3_IRQ_MAX_VECS_RSS;
> > +		}
> > +
> > +		/* The first interrupt vector only
> > +		 * handles link interrupts.
> > +		 */
> > +		limit -= 1;
> > +
> > +		/* Check the indices in the table.
> > +		 * Leave the existing table unmodified
> > +		 * if an error is detected.
> > +		 */
> > +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> > +			if (indir->ring_index[i] >= limit)
> > +				return -EINVAL;
> > +
> > +		tg3_flag_set(tp, USER_INDIR_TBL);
> > +
> > +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> > +			tp->rss_ind_tbl[i] = indir->ring_index[i];
> > +	}
> > +
> > +	if (!netif_running(dev) || !tg3_flag(tp, ENABLE_RSS))
> > +		return 0;
> > +
> > +	/* It is legal to write the indirection
> > +	 * table while the device is running.
> > +	 */
> > +	tg3_full_lock(tp, 0);
> > +	tg3_rss_write_indir_tbl(tp);
> > +	tg3_full_unlock(tp);
> > +
> > +	return 0;
> > +}
> > +
> >  static void tg3_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> >  {
> >  	switch (stringset) {
> > @@ -11949,6 +12070,9 @@ static const struct ethtool_ops tg3_ethtool_ops = {
> >  	.get_coalesce		= tg3_get_coalesce,
> >  	.set_coalesce		= tg3_set_coalesce,
> >  	.get_sset_count		= tg3_get_sset_count,
> > +	.get_rxnfc		= tg3_get_rxnfc,
> > +	.get_rxfh_indir		= tg3_get_rxfh_indir,
> > +	.set_rxfh_indir		= tg3_set_rxfh_indir,
> >  };
> >  
> >  static void __devinit tg3_get_eeprom_size(struct tg3 *tp)
> > diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
> > index aea8f72..4800595 100644
> > --- a/drivers/net/ethernet/broadcom/tg3.h
> > +++ b/drivers/net/ethernet/broadcom/tg3.h
> > @@ -2932,6 +2932,7 @@ enum TG3_FLAGS {
> >  	TG3_FLAG_APE_HAS_NCSI,
> >  	TG3_FLAG_4K_FIFO_LIMIT,
> >  	TG3_FLAG_RESET_TASK_PENDING,
> > +	TG3_FLAG_USER_INDIR_TBL,
> >  	TG3_FLAG_5705_PLUS,
> >  	TG3_FLAG_IS_5788,
> >  	TG3_FLAG_5750_PLUS,
> 
> -- 
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> 

Re: [PATCH] tg3: Make the RSS indir tbl admin configurable

[Ben Hutchings]

On Fri, 2011-12-16 at 11:19 -0800, Matt Carlson wrote:
> On Thu, Dec 15, 2011 at 07:04:34PM -0800, Ben Hutchings wrote:
> > On Thu, 2011-12-15 at 16:47 -0800, Matt Carlson wrote:
> > > This patch adds the ethtool callbacks necessary to change the rss
> > > indirection table from userspace.  When setting the indirection table
> > > through set_rxfh_indir, an indirection table size of zero is
> > > interpreted to mean that the admin wants to relinquish control of the
> > > table to the driver.  Should the number of interrupts change (e.g.
> > > across a close / open call, or through a reset), any indirection table
> > > values that exceed the number of RSS queues or interrupt vectors will
> > > be automatically scaled back to values within range.
> > > 
> > > Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> > > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > 
> > No I didn't!
> > 
> > Ben.
> 
> Sorry.  This part of procedure I'm unsure about.  I didn't want to lose
> the ack of your contributions to this patch.  The SOB does imply that
> you are O.K. with it as-is though, which you may or may not be aligned
> with.  I apologize if I was putting words in your mouth.
> 
> What should I have done instead?  Would Dave put the SOB on your behalf
> as he accepts it?
[...]

If you really want to credit me for the tiny change I suggested, just
put that in free-form.  But I'm afraid you'll have to respin this
completely, as David applied my changes to centralise validation and the
default policy.  I think instead of adding the USER_INDIR_TBL flag you
could reasonably reset the table to default whenever the interface is
brought up and you fail to get the wanted number of IRQs.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.