[Intel-wired-lan] [PATCH 1/2] fm10k: use ethtool_rxfh_indir_default for default redirection table

[Intel-wired-lan] [PATCH 1/2] fm10k: use ethtool_rxfh_indir_default for default redirection table

[Jacob Keller]

The fm10k driver used its own code for generating a default indirection
table on device load, which was not the same as the default generated by
ethtool when indir_size of 0 is passed to SRXFH. Take advantage of
ethtool_rxfh_indir_default() and simplify code to write the redirection
table to reduce some code duplication.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
I have in fact verified this compiles and runs correctly, now.

 drivers/net/ethernet/intel/fm10k/fm10k.h         |  2 ++
 drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 37 ++++++++++++++----------
 drivers/net/ethernet/intel/fm10k/fm10k_main.c    | 24 +++++++--------
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index 83f386714e87..9c7fafef7cf6 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -510,6 +510,8 @@ int fm10k_close(struct net_device *netdev);
 
 /* Ethtool */
 void fm10k_set_ethtool_ops(struct net_device *dev);
+u32 fm10k_get_reta_size(struct net_device *netdev);
+void fm10k_write_reta(struct fm10k_intfc *interface, const u32 *indir);
 
 /* IOV */
 s32 fm10k_iov_event(struct fm10k_intfc *interface);
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index 185fcbd8df1a..a1342be34be4 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -1095,11 +1095,31 @@ static int fm10k_set_priv_flags(struct net_device *netdev, u32 priv_flags)
 	return 0;
 }
 
-static u32 fm10k_get_reta_size(struct net_device __always_unused *netdev)
+u32 fm10k_get_reta_size(struct net_device __always_unused *netdev)
 {
 	return FM10K_RETA_SIZE * FM10K_RETA_ENTRIES_PER_REG;
 }
 
+void fm10k_write_reta(struct fm10k_intfc *interface, const u32 *indir)
+{
+	struct fm10k_hw *hw = &interface->hw;
+	int i;
+
+	/* record entries to reta table */
+	for (i = 0; i < FM10K_RETA_SIZE; i++, indir += 4) {
+		u32 reta = indir[0] |
+			   (indir[1] << 8) |
+			   (indir[2] << 16) |
+			   (indir[3] << 24);
+
+		if (interface->reta[i] == reta)
+			continue;
+
+		interface->reta[i] = reta;
+		fm10k_write_reg(hw, FM10K_RETA(0, i), reta);
+	}
+}
+
 static int fm10k_get_reta(struct net_device *netdev, u32 *indir)
 {
 	struct fm10k_intfc *interface = netdev_priv(netdev);
@@ -1123,7 +1143,6 @@ static int fm10k_get_reta(struct net_device *netdev, u32 *indir)
 static int fm10k_set_reta(struct net_device *netdev, const u32 *indir)
 {
 	struct fm10k_intfc *interface = netdev_priv(netdev);
-	struct fm10k_hw *hw = &interface->hw;
 	int i;
 	u16 rss_i;
 
@@ -1138,19 +1157,7 @@ static int fm10k_set_reta(struct net_device *netdev, const u32 *indir)
 		return -EINVAL;
 	}
 
-	/* record entries to reta table */
-	for (i = 0; i < FM10K_RETA_SIZE; i++, indir += 4) {
-		u32 reta = indir[0] |
-			   (indir[1] << 8) |
-			   (indir[2] << 16) |
-			   (indir[3] << 24);
-
-		if (interface->reta[i] == reta)
-			continue;
-
-		interface->reta[i] = reta;
-		fm10k_write_reg(hw, FM10K_RETA(0, i), reta);
-	}
+	fm10k_write_reta(interface, indir);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index d3ff17da3c2a..43f3674501cc 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1943,7 +1943,8 @@ static void fm10k_assign_rings(struct fm10k_intfc *interface)
 static void fm10k_init_reta(struct fm10k_intfc *interface)
 {
 	u16 i, rss_i = interface->ring_feature[RING_F_RSS].indices;
-	u32 reta, base;
+	struct net_device *netdev = interface->netdev;
+	u32 reta, *indir;
 
 	/* If the Rx flow indirection table has been configured manually, we
 	 * need to maintain it when possible.
@@ -1968,21 +1969,16 @@ static void fm10k_init_reta(struct fm10k_intfc *interface)
 	}
 
 repopulate_reta:
-	/* Populate the redirection table 4 entries at a time.  To do this
-	 * we are generating the results for n and n+2 and then interleaving
-	 * those with the results with n+1 and n+3.
-	 */
-	for (i = FM10K_RETA_SIZE; i--;) {
-		/* first pass generates n and n+2 */
-		base = ((i * 0x00040004) + 0x00020000) * rss_i;
-		reta = (base & 0x3F803F80) >> 7;
+	indir = kcalloc(fm10k_get_reta_size(netdev),
+			sizeof(indir[0]), GFP_KERNEL);
 
-		/* second pass generates n+1 and n+3 */
-		base += 0x00010001 * rss_i;
-		reta |= (base & 0x3F803F80) << 1;
+	/* generate redirection table using the default kernel policy */
+	for (i = 0; i < fm10k_get_reta_size(netdev); i++)
+		indir[i] = ethtool_rxfh_indir_default(i, rss_i);
 
-		interface->reta[i] = reta;
-	}
+	fm10k_write_reta(interface, indir);
+
+	kfree(indir);
 }
 
 /**
-- 
2.7.0.236.gda096a0.dirty

[Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel

[Jacob Keller]

Use BUILD_BUG_ON() instead of BUG_ON() where appropriate to get a compile
error rather than crash the kernel.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
I checked this works against allmodconfig, and defconfig. Note that
only allmodconfig seems to break the use of BUILD_BUG_ON in the latter
case, so I added a comment to indicate this. Sorry for the failures
earlier.

 drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index a1342be34be4..0b0531417835 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -468,7 +468,7 @@ static void fm10k_get_reg_q(struct fm10k_hw *hw, u32 *buff, int i)
 	buff[idx++] = fm10k_read_reg(hw, FM10K_TX_SGLORT(i));
 	buff[idx++] = fm10k_read_reg(hw, FM10K_PFVTCTL(i));
 
-	BUG_ON(idx != FM10K_REGS_LEN_Q);
+	BUILD_BUG_ON(idx != FM10K_REGS_LEN_Q);
 }
 
 /* If function above adds more registers this define needs to be updated */
@@ -484,6 +484,9 @@ static void fm10k_get_reg_vsi(struct fm10k_hw *hw, u32 *buff, int i)
 	for (j = 0; j < 32; j++)
 		buff[idx++] = fm10k_read_reg(hw, FM10K_RETA(i, j));
 
+	/* Unfortunately some kernel configurations prevent us from using
+	 * BUILD_BUG_ON here, notably allmodconfig.
+	 */
 	BUG_ON(idx != FM10K_REGS_LEN_VSI);
 }
 
-- 
2.7.0.236.gda096a0.dirty

[Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, February 16, 2016 4:19 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel

Use BUILD_BUG_ON() instead of BUG_ON() where appropriate to get a compile error rather than crash the kernel.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>

[Intel-wired-lan] [PATCH 1/2] fm10k: use ethtool_rxfh_indir_default for default redirection table

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Tuesday, February 16, 2016 4:19 PM
To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
Subject: [Intel-wired-lan] [PATCH 1/2] fm10k: use ethtool_rxfh_indir_default for default redirection table

The fm10k driver used its own code for generating a default indirection table on device load, which was not the same as the default generated by ethtool when indir_size of 0 is passed to SRXFH. Take advantage of
ethtool_rxfh_indir_default() and simplify code to write the redirection table to reduce some code duplication.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>

[Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel

[Allan, Bruce W]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Singh, Krishneil K
> Sent: Thursday, February 25, 2016 12:27 PM
> To: Keller, Jacob E; Intel Wired LAN
> Subject: Re: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel
> 
> 
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Jacob Keller
> Sent: Tuesday, February 16, 2016 4:19 PM
> To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> Subject: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel
> 
> Use BUILD_BUG_ON() instead of BUG_ON() where appropriate to get a
> compile error rather than crash the kernel.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>

Please hold off on submitting this patch to netdev.

[Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel

[Keller, Jacob E]

On Thu, 2016-02-25 at 21:46 +0000, Allan, Bruce W wrote:
> > 
> > -----Original Message-----
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.
> > org] On
> > Behalf Of Singh, Krishneil K
> > Sent: Thursday, February 25, 2016 12:27 PM
> > To: Keller, Jacob E; Intel Wired LAN
> > Subject: Re: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing
> > the kernel
> > 
> > 
> > -----Original Message-----
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.
> > org] On
> > Behalf Of Jacob Keller
> > Sent: Tuesday, February 16, 2016 4:19 PM
> > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> > Subject: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the
> > kernel
> > 
> > Use BUILD_BUG_ON() instead of BUG_ON() where appropriate to get a
> > compile error rather than crash the kernel.
> > 
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> > Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
> Please hold off on submitting this patch to netdev.

What's the reasoning here for holding submission?

Thanks,
Jake

[Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel

[Allan, Bruce W]

> -----Original Message-----
> From: Keller, Jacob E
> Sent: Thursday, February 25, 2016 5:12 PM
> To: Singh, Krishneil K; Allan, Bruce W; intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the kernel
> 
> On Thu, 2016-02-25 at 21:46 +0000, Allan, Bruce W wrote:
> > >
> > > -----Original Message-----
> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.
> > > org] On
> > > Behalf Of Singh, Krishneil K
> > > Sent: Thursday, February 25, 2016 12:27 PM
> > > To: Keller, Jacob E; Intel Wired LAN
> > > Subject: Re: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing
> > > the kernel
> > >
> > >
> > > -----Original Message-----
> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.
> > > org] On
> > > Behalf Of Jacob Keller
> > > Sent: Tuesday, February 16, 2016 4:19 PM
> > > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org>
> > > Subject: [Intel-wired-lan] [PATCH 2/2] fm10k: Avoid crashing the
> > > kernel
> > >
> > > Use BUILD_BUG_ON() instead of BUG_ON() where appropriate to get a
> > > compile error rather than crash the kernel.
> > >
> > > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > > ---
> > > Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
> > Please hold off on submitting this patch to netdev.
> 
> What's the reasoning here for holding submission?
> 
> Thanks,
> Jake

I want to look into this further.