[Intel-wired-lan] [PATCH iwl-net 0/3] idpf: Fix issues in rxhash and rss lut logic

[Intel-wired-lan] [PATCH iwl-net 0/3] idpf: Fix issues in rxhash and rss lut logic

[Sreedevi Joshi]

ethtool -K rxhash on/off and ethtool -x/-X related to setting up RSS LUT
indirection table have dependencies. There were some issues in the logic
that were resulting in NULL pointer issues and configuration constraints.
This series fixes these issues.

Sreedevi Joshi (3):
  idpf: Fix RSS LUT NULL pointer crash on early ethtool operations
  idpf: Fix RSS LUT configuration on down interfaces
  idpf: Fix RSS LUT NULL ptr issue after soft reset

 drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
 .../net/ethernet/intel/idpf/idpf_ethtool.c    | 17 ++--
 drivers/net/ethernet/intel/idpf/idpf_lib.c    | 91 +++++++++----------
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 38 +++-----
 drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  5 +-
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
 6 files changed, 79 insertions(+), 83 deletions(-)

-- 
2.43.0

[Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations

[Sreedevi Joshi]

The RSS LUT is not initialized until the interface comes up, causing
the following NULL pointer crash when ethtool operations like rxhash on/off
are performed before the interface is brought up for the first time.

Move RSS LUT initialization from ndo_open to vport creation to ensure LUT
is always available. This enables RSS configuration via ethtool before
bringing the interface up. Simplify LUT management by maintaining all
changes in the driver's soft copy and programming zeros to the indirection
table when rxhash is disabled. Defer HW programming until the interface
comes up if it is down during rxhash and LUT configuration changes.

[89408.371875] BUG: kernel NULL pointer dereference, address: 0000000000000000
[89408.371908] #PF: supervisor read access in kernel mode
[89408.371924] #PF: error_code(0x0000) - not-present page
[89408.371940] PGD 0 P4D 0
[89408.371953] Oops: Oops: 0000 [#1] SMP NOPTI
<snip>
[89408.372052] RIP: 0010:memcpy_orig+0x16/0x130
[89408.372310] Call Trace:
[89408.372317]  <TASK>
[89408.372326]  ? idpf_set_features+0xfc/0x180 [idpf]
[89408.372363]  __netdev_update_features+0x295/0xde0
[89408.372384]  ethnl_set_features+0x15e/0x460
[89408.372406]  genl_family_rcv_msg_doit+0x11f/0x180
[89408.372429]  genl_rcv_msg+0x1ad/0x2b0
[89408.372446]  ? __pfx_ethnl_set_features+0x10/0x10
[89408.372465]  ? __pfx_genl_rcv_msg+0x10/0x10
[89408.372482]  netlink_rcv_skb+0x58/0x100
[89408.372502]  genl_rcv+0x2c/0x50
[89408.372516]  netlink_unicast+0x289/0x3e0
[89408.372533]  netlink_sendmsg+0x215/0x440
[89408.372551]  __sys_sendto+0x234/0x240
[89408.372571]  __x64_sys_sendto+0x28/0x30
[89408.372585]  x64_sys_call+0x1909/0x1da0
[89408.372604]  do_syscall_64+0x7a/0xfa0
[89408.373140]  ? clear_bhb_loop+0x60/0xb0
[89408.373647]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[89408.378887]  </TASK>
<snip>

Fixes: a251eee62133 ("idpf: add SRIOV support and other ndo_ops")
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
 drivers/net/ethernet/intel/idpf/idpf_lib.c    | 89 +++++++++----------
 drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 36 +++-----
 drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  4 +-
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
 5 files changed, 64 insertions(+), 76 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index e69ce1d852f8..cdaa603ad82c 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -425,14 +425,12 @@ enum idpf_user_flags {
  * @rss_key: RSS hash key
  * @rss_lut_size: Size of RSS lookup table
  * @rss_lut: RSS lookup table
- * @cached_lut: Used to restore previously init RSS lut
  */
 struct idpf_rss_data {
 	u16 rss_key_size;
 	u8 *rss_key;
 	u16 rss_lut_size;
 	u32 *rss_lut;
-	u32 *cached_lut;
 };
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 8c3041f00cda..7359677d8a3d 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1073,7 +1073,7 @@ static void idpf_vport_rel(struct idpf_vport *vport)
 	u16 idx = vport->idx;
 
 	vport_config = adapter->vport_config[vport->idx];
-	idpf_deinit_rss(vport);
+	idpf_deinit_rss_lut(vport);
 	rss_data = &vport_config->user_config.rss_data;
 	kfree(rss_data->rss_key);
 	rss_data->rss_key = NULL;
@@ -1226,6 +1226,7 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
 	u16 idx = adapter->next_vport;
 	struct idpf_vport *vport;
 	u16 num_max_q;
+	int err;
 
 	if (idx == IDPF_NO_FREE_SLOT)
 		return NULL;
@@ -1276,10 +1277,11 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
 
 	idpf_vport_init(vport, max_q);
 
-	/* This alloc is done separate from the LUT because it's not strictly
-	 * dependent on how many queues we have. If we change number of queues
-	 * and soft reset we'll need a new LUT but the key can remain the same
-	 * for as long as the vport exists.
+	/* LUT and key are both initialized here. Key is not strictly dependent
+	 * on how many queues we have. If we change number of queues and soft
+	 * reset is initiated, LUT will be freed and a new LUT will be allocated
+	 * as per the updated number of queues during vport bringup. However,
+	 * the key remains the same for as long as the vport exists.
 	 */
 	rss_data = &adapter->vport_config[idx]->user_config.rss_data;
 	rss_data->rss_key = kzalloc(rss_data->rss_key_size, GFP_KERNEL);
@@ -1289,6 +1291,13 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
 	/* Initialize default rss key */
 	netdev_rss_key_fill((void *)rss_data->rss_key, rss_data->rss_key_size);
 
+	/* Initialize default rss LUT */
+	err = idpf_init_rss_lut(vport);
+	if (err) {
+		kfree(rss_data->rss_key);
+		goto free_vport;
+	}
+
 	/* fill vport slot in the adapter struct */
 	adapter->vports[idx] = vport;
 	adapter->vport_ids[idx] = idpf_get_vport_id(vport);
@@ -1476,6 +1485,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
 	struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
 	struct idpf_adapter *adapter = vport->adapter;
 	struct idpf_vport_config *vport_config;
+	struct idpf_rss_data *rss_data;
 	int err;
 
 	if (np->state != __IDPF_VPORT_DOWN)
@@ -1570,14 +1580,23 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
 	idpf_restore_features(vport);
 
 	vport_config = adapter->vport_config[vport->idx];
-	if (vport_config->user_config.rss_data.rss_lut)
-		err = idpf_config_rss(vport);
-	else
-		err = idpf_init_rss(vport);
+	rss_data = &vport_config->user_config.rss_data;
+
+	if (!rss_data->rss_lut) {
+		err = idpf_init_rss_lut(vport);
+		if (err) {
+			dev_err(&adapter->pdev->dev,
+				"Failed to initialize RSS LUT for vport %u: %d\n",
+				vport->vport_id, err);
+			goto disable_vport;
+		}
+	}
+
+	err = idpf_config_rss(vport);
 	if (err) {
-		dev_err(&adapter->pdev->dev, "Failed to initialize RSS for vport %u: %d\n",
+		dev_err(&adapter->pdev->dev, "Failed to configure RSS for vport %u: %d\n",
 			vport->vport_id, err);
-		goto disable_vport;
+		goto deinit_rss;
 	}
 
 	err = idpf_up_complete(vport);
@@ -1593,7 +1612,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
 	return 0;
 
 deinit_rss:
-	idpf_deinit_rss(vport);
+	idpf_deinit_rss_lut(vport);
 disable_vport:
 	idpf_send_disable_vport_msg(vport);
 disable_queues:
@@ -2042,7 +2061,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
 		idpf_vport_stop(vport, false);
 	}
 
-	idpf_deinit_rss(vport);
+	idpf_deinit_rss_lut(vport);
 	/* We're passing in vport here because we need its wait_queue
 	 * to send a message and it should be getting all the vport
 	 * config data out of the adapter but we need to be careful not
@@ -2210,40 +2229,6 @@ static void idpf_set_rx_mode(struct net_device *netdev)
 		dev_err(dev, "Failed to set promiscuous mode: %d\n", err);
 }
 
-/**
- * idpf_vport_manage_rss_lut - disable/enable RSS
- * @vport: the vport being changed
- *
- * In the event of disable request for RSS, this function will zero out RSS
- * LUT, while in the event of enable request for RSS, it will reconfigure RSS
- * LUT with the default LUT configuration.
- */
-static int idpf_vport_manage_rss_lut(struct idpf_vport *vport)
-{
-	bool ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
-	struct idpf_rss_data *rss_data;
-	u16 idx = vport->idx;
-	int lut_size;
-
-	rss_data = &vport->adapter->vport_config[idx]->user_config.rss_data;
-	lut_size = rss_data->rss_lut_size * sizeof(u32);
-
-	if (ena) {
-		/* This will contain the default or user configured LUT */
-		memcpy(rss_data->rss_lut, rss_data->cached_lut, lut_size);
-	} else {
-		/* Save a copy of the current LUT to be restored later if
-		 * requested.
-		 */
-		memcpy(rss_data->cached_lut, rss_data->rss_lut, lut_size);
-
-		/* Zero out the current LUT to disable */
-		memset(rss_data->rss_lut, 0, lut_size);
-	}
-
-	return idpf_config_rss(vport);
-}
-
 /**
  * idpf_set_features - set the netdev feature flags
  * @netdev: ptr to the netdev being adjusted
@@ -2269,8 +2254,16 @@ static int idpf_set_features(struct net_device *netdev,
 	}
 
 	if (changed & NETIF_F_RXHASH) {
+		struct idpf_netdev_priv *np = netdev_priv(netdev);
+
 		netdev->features ^= NETIF_F_RXHASH;
-		err = idpf_vport_manage_rss_lut(vport);
+
+		/* If the Interface is not up when changing the rxhash, update to the HW is
+		 * skipped. The updated LUT will be committed to the HW when the interface
+		 * is brought up.
+		 */
+		if (np->state == __IDPF_VPORT_UP)
+			err = idpf_config_rss(vport);
 		if (err)
 			goto unlock_mutex;
 	}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index dcdd4fef1c7a..11f711997db8 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -2868,7 +2868,6 @@ int idpf_tso(struct sk_buff *skb, struct idpf_tx_offload_params *off)
 	return 1;
 }
 
-
 /**
  * idpf_tx_splitq_get_ctx_desc - grab next desc and update buffer ring
  * @txq: queue to put context descriptor on
@@ -4486,6 +4485,7 @@ static void idpf_vport_intr_napi_add_all(struct idpf_vport *vport)
 
 	for (v_idx = 0; v_idx < vport->num_q_vectors; v_idx++) {
 		struct idpf_q_vector *q_vector = &vport->q_vectors[v_idx];
+
 		qv_idx = vport->q_vector_idxs[v_idx];
 		irq_num = vport->adapter->msix_entries[qv_idx].vector;
 
@@ -4652,57 +4652,47 @@ static void idpf_fill_dflt_rss_lut(struct idpf_vport *vport)
 
 	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
 
-	for (i = 0; i < rss_data->rss_lut_size; i++) {
+	for (i = 0; i < rss_data->rss_lut_size; i++)
 		rss_data->rss_lut[i] = i % num_active_rxq;
-		rss_data->cached_lut[i] = rss_data->rss_lut[i];
-	}
 }
 
 /**
- * idpf_init_rss - Allocate and initialize RSS resources
+ * idpf_init_rss_lut - Allocate and initialize RSS LUT
  * @vport: virtual port
  *
  * Return 0 on success, negative on failure
  */
-int idpf_init_rss(struct idpf_vport *vport)
+int idpf_init_rss_lut(struct idpf_vport *vport)
 {
 	struct idpf_adapter *adapter = vport->adapter;
 	struct idpf_rss_data *rss_data;
-	u32 lut_size;
 
 	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
+	if (!rss_data->rss_lut) {
+		u32 lut_size;
 
-	lut_size = rss_data->rss_lut_size * sizeof(u32);
-	rss_data->rss_lut = kzalloc(lut_size, GFP_KERNEL);
-	if (!rss_data->rss_lut)
-		return -ENOMEM;
-
-	rss_data->cached_lut = kzalloc(lut_size, GFP_KERNEL);
-	if (!rss_data->cached_lut) {
-		kfree(rss_data->rss_lut);
-		rss_data->rss_lut = NULL;
-
-		return -ENOMEM;
+		lut_size = rss_data->rss_lut_size * sizeof(u32);
+		rss_data->rss_lut = kzalloc(lut_size, GFP_KERNEL);
+		if (!rss_data->rss_lut)
+			return -ENOMEM;
 	}
 
 	/* Fill the default RSS lut values */
 	idpf_fill_dflt_rss_lut(vport);
 
-	return idpf_config_rss(vport);
+	return 0;
 }
 
 /**
- * idpf_deinit_rss - Release RSS resources
+ * idpf_deinit_rss_lut - Release RSS LUT
  * @vport: virtual port
  */
-void idpf_deinit_rss(struct idpf_vport *vport)
+void idpf_deinit_rss_lut(struct idpf_vport *vport)
 {
 	struct idpf_adapter *adapter = vport->adapter;
 	struct idpf_rss_data *rss_data;
 
 	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
-	kfree(rss_data->cached_lut);
-	rss_data->cached_lut = NULL;
 	kfree(rss_data->rss_lut);
 	rss_data->rss_lut = NULL;
 }
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index a1255099656f..2bfb87b82a9b 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -1087,8 +1087,8 @@ void idpf_vport_intr_deinit(struct idpf_vport *vport);
 int idpf_vport_intr_init(struct idpf_vport *vport);
 void idpf_vport_intr_ena(struct idpf_vport *vport);
 int idpf_config_rss(struct idpf_vport *vport);
-int idpf_init_rss(struct idpf_vport *vport);
-void idpf_deinit_rss(struct idpf_vport *vport);
+int idpf_init_rss_lut(struct idpf_vport *vport);
+void idpf_deinit_rss_lut(struct idpf_vport *vport);
 int idpf_rx_bufs_init_all(struct idpf_vport *vport);
 
 struct idpf_q_vector *idpf_find_rxq_vec(const struct idpf_vport *vport,
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index ca302df9ff40..13a7581c07e6 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -2804,6 +2804,10 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport)
  * @vport: virtual port data structure
  * @get: flag to set or get rss look up table
  *
+ * When rxhash is disabled, RSS LUT will be configured with zeros.  If rxhash
+ * is enabled, the LUT values stored in driver's soft copy will be used to setup
+ * the HW.
+ *
  * Returns 0 on success, negative on failure.
  */
 int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
@@ -2814,10 +2818,12 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
 	struct idpf_rss_data *rss_data;
 	int buf_size, lut_buf_size;
 	ssize_t reply_sz;
+	bool rxhash_ena;
 	int i;
 
 	rss_data =
 		&vport->adapter->vport_config[vport->idx]->user_config.rss_data;
+	rxhash_ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
 	buf_size = struct_size(rl, lut, rss_data->rss_lut_size);
 	rl = kzalloc(buf_size, GFP_KERNEL);
 	if (!rl)
@@ -2839,7 +2845,8 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
 	} else {
 		rl->lut_entries = cpu_to_le16(rss_data->rss_lut_size);
 		for (i = 0; i < rss_data->rss_lut_size; i++)
-			rl->lut[i] = cpu_to_le32(rss_data->rss_lut[i]);
+			rl->lut[i] = (rxhash_ena) ?
+				cpu_to_le32(rss_data->rss_lut[i]) : 0;
 
 		xn_params.vc_op = VIRTCHNL2_OP_SET_RSS_LUT;
 	}
-- 
2.43.0

[Intel-wired-lan] [PATCH iwl-net 2/3] idpf: Fix RSS LUT configuration on down interfaces

[Sreedevi Joshi]

RSS LUT provisioning and queries on a down interface currently return
silently without effect. Users should be able to configure RSS settings
even when the interface is down.

Fix by maintaining RSS configuration changes in the driver's soft copy and
deferring HW programming until the interface comes up.

Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks")
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
index 4c6e52253ae4..d9903a21972a 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
@@ -393,7 +393,10 @@ static u32 idpf_get_rxfh_indir_size(struct net_device *netdev)
  * @netdev: network interface device structure
  * @rxfh: pointer to param struct (indir, key, hfunc)
  *
- * Reads the indirection table directly from the hardware. Always returns 0.
+ * RSS LUT and Key information are read from driver's cached
+ * copy. When rxhash is off, rss lut will be displayed as zeros.
+ *
+ * Returns 0 on success.
  */
 static int idpf_get_rxfh(struct net_device *netdev,
 			 struct ethtool_rxfh_param *rxfh)
@@ -401,10 +404,13 @@ static int idpf_get_rxfh(struct net_device *netdev,
 	struct idpf_netdev_priv *np = netdev_priv(netdev);
 	struct idpf_rss_data *rss_data;
 	struct idpf_adapter *adapter;
+	struct idpf_vport *vport;
+	bool rxhash_ena;
 	int err = 0;
 	u16 i;
 
 	idpf_vport_ctrl_lock(netdev);
+	vport = idpf_netdev_to_vport(netdev);
 
 	adapter = np->adapter;
 
@@ -414,9 +420,8 @@ static int idpf_get_rxfh(struct net_device *netdev,
 	}
 
 	rss_data = &adapter->vport_config[np->vport_idx]->user_config.rss_data;
-	if (np->state != __IDPF_VPORT_UP)
-		goto unlock_mutex;
 
+	rxhash_ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
 	rxfh->hfunc = ETH_RSS_HASH_TOP;
 
 	if (rxfh->key)
@@ -424,7 +429,7 @@ static int idpf_get_rxfh(struct net_device *netdev,
 
 	if (rxfh->indir) {
 		for (i = 0; i < rss_data->rss_lut_size; i++)
-			rxfh->indir[i] = rss_data->rss_lut[i];
+			rxfh->indir[i] = (rxhash_ena) ? rss_data->rss_lut[i] : 0;
 	}
 
 unlock_mutex:
@@ -464,8 +469,6 @@ static int idpf_set_rxfh(struct net_device *netdev,
 	}
 
 	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
-	if (np->state != __IDPF_VPORT_UP)
-		goto unlock_mutex;
 
 	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
 	    rxfh->hfunc != ETH_RSS_HASH_TOP) {
@@ -481,6 +484,8 @@ static int idpf_set_rxfh(struct net_device *netdev,
 			rss_data->rss_lut[lut] = rxfh->indir[lut];
 	}
 
+	if (np->state != __IDPF_VPORT_UP)
+		goto unlock_mutex;
 	err = idpf_config_rss(vport);
 
 unlock_mutex:
-- 
2.43.0

[Intel-wired-lan] [PATCH iwl-net 3/3] idpf: Fix RSS LUT NULL ptr issue after soft reset

[Sreedevi Joshi]

During soft reset, the RSS LUT is freed and not restored unless the
interface is up. If an ethtool command that accesses the rss lut is
attempted immediately after reset, it will result in NULL ptr
dereference. Also, there is no need to reset the rss lut if the soft reset
does not involve queue count change.

After soft reset, set the RSS LUT to default values based on the updated
queue count only if the reset was a result of a queue count change. For
all other reset causes, don't touch the LUT.

Steps to reproduce:

** Bring the interface down (if up)
ifconfig eth1 down

** update the queue count (eg., 27->20)
ethtool -L eth1 combined 20

** display the RSS LUT
ethtool -x eth1

[82375.558338] BUG: kernel NULL pointer dereference, address: 0000000000000000
[82375.558373] #PF: supervisor read access in kernel mode
[82375.558391] #PF: error_code(0x0000) - not-present page
[82375.558408] PGD 0 P4D 0
[82375.558421] Oops: Oops: 0000 [#1] SMP NOPTI
<snip>
[82375.558516] RIP: 0010:idpf_get_rxfh+0x108/0x150 [idpf]
[82375.558786] Call Trace:
[82375.558793]  <TASK>
[82375.558804]  rss_prepare.isra.0+0x187/0x2a0
[82375.558827]  rss_prepare_data+0x3a/0x50
[82375.558845]  ethnl_default_doit+0x13d/0x3e0
[82375.558863]  genl_family_rcv_msg_doit+0x11f/0x180
[82375.558886]  genl_rcv_msg+0x1ad/0x2b0
[82375.558902]  ? __pfx_ethnl_default_doit+0x10/0x10
[82375.558920]  ? __pfx_genl_rcv_msg+0x10/0x10
[82375.558937]  netlink_rcv_skb+0x58/0x100
[82375.558957]  genl_rcv+0x2c/0x50
[82375.558971]  netlink_unicast+0x289/0x3e0
[82375.558988]  netlink_sendmsg+0x215/0x440
[82375.559005]  __sys_sendto+0x234/0x240
[82375.559555]  __x64_sys_sendto+0x28/0x30
[82375.560068]  x64_sys_call+0x1909/0x1da0
[82375.560576]  do_syscall_64+0x7a/0xfa0
[82375.561076]  ? clear_bhb_loop+0x60/0xb0
[82375.561567]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
<snip>

Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks")
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_lib.c  | 4 +++-
 drivers/net/ethernet/intel/idpf/idpf_txrx.c | 2 +-
 drivers/net/ethernet/intel/idpf/idpf_txrx.h | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 7359677d8a3d..51e1fccfb6d1 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -2061,7 +2061,6 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
 		idpf_vport_stop(vport, false);
 	}
 
-	idpf_deinit_rss_lut(vport);
 	/* We're passing in vport here because we need its wait_queue
 	 * to send a message and it should be getting all the vport
 	 * config data out of the adapter but we need to be careful not
@@ -2087,6 +2086,9 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
 	if (err)
 		goto err_open;
 
+	if (reset_cause == IDPF_SR_Q_CHANGE)
+		idpf_fill_dflt_rss_lut(vport);
+
 	if (current_state == __IDPF_VPORT_UP)
 		err = idpf_vport_open(vport, false);
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 11f711997db8..214e7c93b106 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -4643,7 +4643,7 @@ int idpf_config_rss(struct idpf_vport *vport)
  * idpf_fill_dflt_rss_lut - Fill the indirection table with the default values
  * @vport: virtual port structure
  */
-static void idpf_fill_dflt_rss_lut(struct idpf_vport *vport)
+void idpf_fill_dflt_rss_lut(struct idpf_vport *vport)
 {
 	struct idpf_adapter *adapter = vport->adapter;
 	u16 num_active_rxq = vport->num_rxq;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index 2bfb87b82a9b..423cc9486dce 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -1086,6 +1086,7 @@ void idpf_vport_intr_update_itr_ena_irq(struct idpf_q_vector *q_vector);
 void idpf_vport_intr_deinit(struct idpf_vport *vport);
 int idpf_vport_intr_init(struct idpf_vport *vport);
 void idpf_vport_intr_ena(struct idpf_vport *vport);
+void idpf_fill_dflt_rss_lut(struct idpf_vport *vport);
 int idpf_config_rss(struct idpf_vport *vport);
 int idpf_init_rss_lut(struct idpf_vport *vport);
 void idpf_deinit_rss_lut(struct idpf_vport *vport);
-- 
2.43.0

Re: [Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations

[Paul Menzel]

Dear Sreedevi,


Thank you for your patch.

Am 18.11.25 um 05:22 schrieb Sreedevi Joshi:
> The RSS LUT is not initialized until the interface comes up, causing
> the following NULL pointer crash when ethtool operations like rxhash on/off
> are performed before the interface is brought up for the first time.
> 
> Move RSS LUT initialization from ndo_open to vport creation to ensure LUT
> is always available. This enables RSS configuration via ethtool before
> bringing the interface up. Simplify LUT management by maintaining all
> changes in the driver's soft copy and programming zeros to the indirection
> table when rxhash is disabled. Defer HW programming until the interface
> comes up if it is down during rxhash and LUT configuration changes.
> 
> [89408.371875] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [89408.371908] #PF: supervisor read access in kernel mode
> [89408.371924] #PF: error_code(0x0000) - not-present page
> [89408.371940] PGD 0 P4D 0
> [89408.371953] Oops: Oops: 0000 [#1] SMP NOPTI
> <snip>
> [89408.372052] RIP: 0010:memcpy_orig+0x16/0x130
> [89408.372310] Call Trace:
> [89408.372317]  <TASK>
> [89408.372326]  ? idpf_set_features+0xfc/0x180 [idpf]
> [89408.372363]  __netdev_update_features+0x295/0xde0
> [89408.372384]  ethnl_set_features+0x15e/0x460
> [89408.372406]  genl_family_rcv_msg_doit+0x11f/0x180
> [89408.372429]  genl_rcv_msg+0x1ad/0x2b0
> [89408.372446]  ? __pfx_ethnl_set_features+0x10/0x10
> [89408.372465]  ? __pfx_genl_rcv_msg+0x10/0x10
> [89408.372482]  netlink_rcv_skb+0x58/0x100
> [89408.372502]  genl_rcv+0x2c/0x50
> [89408.372516]  netlink_unicast+0x289/0x3e0
> [89408.372533]  netlink_sendmsg+0x215/0x440
> [89408.372551]  __sys_sendto+0x234/0x240
> [89408.372571]  __x64_sys_sendto+0x28/0x30
> [89408.372585]  x64_sys_call+0x1909/0x1da0
> [89408.372604]  do_syscall_64+0x7a/0xfa0
> [89408.373140]  ? clear_bhb_loop+0x60/0xb0
> [89408.373647]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [89408.378887]  </TASK>
> <snip>

It’d be great if you described your test system.

> Fixes: a251eee62133 ("idpf: add SRIOV support and other ndo_ops")
> Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
>   drivers/net/ethernet/intel/idpf/idpf_lib.c    | 89 +++++++++----------
>   drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 36 +++-----
>   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  4 +-
>   .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
>   5 files changed, 64 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index e69ce1d852f8..cdaa603ad82c 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -425,14 +425,12 @@ enum idpf_user_flags {
>    * @rss_key: RSS hash key
>    * @rss_lut_size: Size of RSS lookup table
>    * @rss_lut: RSS lookup table
> - * @cached_lut: Used to restore previously init RSS lut
>    */
>   struct idpf_rss_data {
>   	u16 rss_key_size;
>   	u8 *rss_key;
>   	u16 rss_lut_size;
>   	u32 *rss_lut;
> -	u32 *cached_lut;
>   };
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index 8c3041f00cda..7359677d8a3d 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -1073,7 +1073,7 @@ static void idpf_vport_rel(struct idpf_vport *vport)
>   	u16 idx = vport->idx;
>   
>   	vport_config = adapter->vport_config[vport->idx];
> -	idpf_deinit_rss(vport);
> +	idpf_deinit_rss_lut(vport);
>   	rss_data = &vport_config->user_config.rss_data;
>   	kfree(rss_data->rss_key);
>   	rss_data->rss_key = NULL;
> @@ -1226,6 +1226,7 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
>   	u16 idx = adapter->next_vport;
>   	struct idpf_vport *vport;
>   	u16 num_max_q;
> +	int err;
>   
>   	if (idx == IDPF_NO_FREE_SLOT)
>   		return NULL;
> @@ -1276,10 +1277,11 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
>   
>   	idpf_vport_init(vport, max_q);
>   
> -	/* This alloc is done separate from the LUT because it's not strictly
> -	 * dependent on how many queues we have. If we change number of queues
> -	 * and soft reset we'll need a new LUT but the key can remain the same
> -	 * for as long as the vport exists.
> +	/* LUT and key are both initialized here. Key is not strictly dependent
> +	 * on how many queues we have. If we change number of queues and soft
> +	 * reset is initiated, LUT will be freed and a new LUT will be allocated
> +	 * as per the updated number of queues during vport bringup. However,
> +	 * the key remains the same for as long as the vport exists.
>   	 */
>   	rss_data = &adapter->vport_config[idx]->user_config.rss_data;
>   	rss_data->rss_key = kzalloc(rss_data->rss_key_size, GFP_KERNEL);
> @@ -1289,6 +1291,13 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
>   	/* Initialize default rss key */
>   	netdev_rss_key_fill((void *)rss_data->rss_key, rss_data->rss_key_size);
>   
> +	/* Initialize default rss LUT */
> +	err = idpf_init_rss_lut(vport);
> +	if (err) {
> +		kfree(rss_data->rss_key);
> +		goto free_vport;
> +	}
> +
>   	/* fill vport slot in the adapter struct */
>   	adapter->vports[idx] = vport;
>   	adapter->vport_ids[idx] = idpf_get_vport_id(vport);
> @@ -1476,6 +1485,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
>   	struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
>   	struct idpf_adapter *adapter = vport->adapter;
>   	struct idpf_vport_config *vport_config;
> +	struct idpf_rss_data *rss_data;
>   	int err;
>   
>   	if (np->state != __IDPF_VPORT_DOWN)
> @@ -1570,14 +1580,23 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
>   	idpf_restore_features(vport);
>   
>   	vport_config = adapter->vport_config[vport->idx];
> -	if (vport_config->user_config.rss_data.rss_lut)
> -		err = idpf_config_rss(vport);
> -	else
> -		err = idpf_init_rss(vport);
> +	rss_data = &vport_config->user_config.rss_data;
> +
> +	if (!rss_data->rss_lut) {
> +		err = idpf_init_rss_lut(vport);
> +		if (err) {
> +			dev_err(&adapter->pdev->dev,
> +				"Failed to initialize RSS LUT for vport %u: %d\n",
> +				vport->vport_id, err);
> +			goto disable_vport;
> +		}
> +	}
> +
> +	err = idpf_config_rss(vport);
>   	if (err) {
> -		dev_err(&adapter->pdev->dev, "Failed to initialize RSS for vport %u: %d\n",
> +		dev_err(&adapter->pdev->dev, "Failed to configure RSS for vport %u: %d\n",
>   			vport->vport_id, err);
> -		goto disable_vport;
> +		goto deinit_rss;
>   	}
>   
>   	err = idpf_up_complete(vport);
> @@ -1593,7 +1612,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
>   	return 0;
>   
>   deinit_rss:
> -	idpf_deinit_rss(vport);
> +	idpf_deinit_rss_lut(vport);
>   disable_vport:
>   	idpf_send_disable_vport_msg(vport);
>   disable_queues:
> @@ -2042,7 +2061,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
>   		idpf_vport_stop(vport, false);
>   	}
>   
> -	idpf_deinit_rss(vport);
> +	idpf_deinit_rss_lut(vport);
>   	/* We're passing in vport here because we need its wait_queue
>   	 * to send a message and it should be getting all the vport
>   	 * config data out of the adapter but we need to be careful not
> @@ -2210,40 +2229,6 @@ static void idpf_set_rx_mode(struct net_device *netdev)
>   		dev_err(dev, "Failed to set promiscuous mode: %d\n", err);
>   }
>   
> -/**
> - * idpf_vport_manage_rss_lut - disable/enable RSS
> - * @vport: the vport being changed
> - *
> - * In the event of disable request for RSS, this function will zero out RSS
> - * LUT, while in the event of enable request for RSS, it will reconfigure RSS
> - * LUT with the default LUT configuration.
> - */
> -static int idpf_vport_manage_rss_lut(struct idpf_vport *vport)
> -{
> -	bool ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
> -	struct idpf_rss_data *rss_data;
> -	u16 idx = vport->idx;
> -	int lut_size;
> -
> -	rss_data = &vport->adapter->vport_config[idx]->user_config.rss_data;
> -	lut_size = rss_data->rss_lut_size * sizeof(u32);
> -
> -	if (ena) {
> -		/* This will contain the default or user configured LUT */
> -		memcpy(rss_data->rss_lut, rss_data->cached_lut, lut_size);
> -	} else {
> -		/* Save a copy of the current LUT to be restored later if
> -		 * requested.
> -		 */
> -		memcpy(rss_data->cached_lut, rss_data->rss_lut, lut_size);
> -
> -		/* Zero out the current LUT to disable */
> -		memset(rss_data->rss_lut, 0, lut_size);
> -	}
> -
> -	return idpf_config_rss(vport);
> -}
> -
>   /**
>    * idpf_set_features - set the netdev feature flags
>    * @netdev: ptr to the netdev being adjusted
> @@ -2269,8 +2254,16 @@ static int idpf_set_features(struct net_device *netdev,
>   	}
>   
>   	if (changed & NETIF_F_RXHASH) {
> +		struct idpf_netdev_priv *np = netdev_priv(netdev);
> +
>   		netdev->features ^= NETIF_F_RXHASH;
> -		err = idpf_vport_manage_rss_lut(vport);
> +
> +		/* If the Interface is not up when changing the rxhash, update to the HW is

I’d spell interface lowercase.

> +		 * skipped. The updated LUT will be committed to the HW when the interface
> +		 * is brought up.
> +		 */
> +		if (np->state == __IDPF_VPORT_UP)
> +			err = idpf_config_rss(vport);
>   		if (err)
>   			goto unlock_mutex;
>   	}
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index dcdd4fef1c7a..11f711997db8 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -2868,7 +2868,6 @@ int idpf_tso(struct sk_buff *skb, struct idpf_tx_offload_params *off)
>   	return 1;
>   }
>   
> -

Unrelated.

>   /**
>    * idpf_tx_splitq_get_ctx_desc - grab next desc and update buffer ring
>    * @txq: queue to put context descriptor on
> @@ -4486,6 +4485,7 @@ static void idpf_vport_intr_napi_add_all(struct idpf_vport *vport)
>   
>   	for (v_idx = 0; v_idx < vport->num_q_vectors; v_idx++) {
>   		struct idpf_q_vector *q_vector = &vport->q_vectors[v_idx];
> +

Unrelated.

>   		qv_idx = vport->q_vector_idxs[v_idx];
>   		irq_num = vport->adapter->msix_entries[qv_idx].vector;
>   
> @@ -4652,57 +4652,47 @@ static void idpf_fill_dflt_rss_lut(struct idpf_vport *vport)
>   
>   	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
>   
> -	for (i = 0; i < rss_data->rss_lut_size; i++) {
> +	for (i = 0; i < rss_data->rss_lut_size; i++)
>   		rss_data->rss_lut[i] = i % num_active_rxq;
> -		rss_data->cached_lut[i] = rss_data->rss_lut[i];
> -	}
>   }
>   
>   /**
> - * idpf_init_rss - Allocate and initialize RSS resources
> + * idpf_init_rss_lut - Allocate and initialize RSS LUT
>    * @vport: virtual port
>    *
>    * Return 0 on success, negative on failure
>    */
> -int idpf_init_rss(struct idpf_vport *vport)
> +int idpf_init_rss_lut(struct idpf_vport *vport)
>   {
>   	struct idpf_adapter *adapter = vport->adapter;
>   	struct idpf_rss_data *rss_data;
> -	u32 lut_size;
>   
>   	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
> +	if (!rss_data->rss_lut) {
> +		u32 lut_size;
>   
> -	lut_size = rss_data->rss_lut_size * sizeof(u32);
> -	rss_data->rss_lut = kzalloc(lut_size, GFP_KERNEL);
> -	if (!rss_data->rss_lut)
> -		return -ENOMEM;
> -
> -	rss_data->cached_lut = kzalloc(lut_size, GFP_KERNEL);
> -	if (!rss_data->cached_lut) {
> -		kfree(rss_data->rss_lut);
> -		rss_data->rss_lut = NULL;
> -
> -		return -ENOMEM;
> +		lut_size = rss_data->rss_lut_size * sizeof(u32);
> +		rss_data->rss_lut = kzalloc(lut_size, GFP_KERNEL);
> +		if (!rss_data->rss_lut)
> +			return -ENOMEM;
>   	}
>   
>   	/* Fill the default RSS lut values */
>   	idpf_fill_dflt_rss_lut(vport);
>   
> -	return idpf_config_rss(vport);
> +	return 0;
>   }
>   
>   /**
> - * idpf_deinit_rss - Release RSS resources
> + * idpf_deinit_rss_lut - Release RSS LUT
>    * @vport: virtual port
>    */
> -void idpf_deinit_rss(struct idpf_vport *vport)
> +void idpf_deinit_rss_lut(struct idpf_vport *vport)
>   {
>   	struct idpf_adapter *adapter = vport->adapter;
>   	struct idpf_rss_data *rss_data;
>   
>   	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
> -	kfree(rss_data->cached_lut);
> -	rss_data->cached_lut = NULL;
>   	kfree(rss_data->rss_lut);
>   	rss_data->rss_lut = NULL;
>   }
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> index a1255099656f..2bfb87b82a9b 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> @@ -1087,8 +1087,8 @@ void idpf_vport_intr_deinit(struct idpf_vport *vport);
>   int idpf_vport_intr_init(struct idpf_vport *vport);
>   void idpf_vport_intr_ena(struct idpf_vport *vport);
>   int idpf_config_rss(struct idpf_vport *vport);
> -int idpf_init_rss(struct idpf_vport *vport);
> -void idpf_deinit_rss(struct idpf_vport *vport);
> +int idpf_init_rss_lut(struct idpf_vport *vport);
> +void idpf_deinit_rss_lut(struct idpf_vport *vport);
>   int idpf_rx_bufs_init_all(struct idpf_vport *vport);
>   
>   struct idpf_q_vector *idpf_find_rxq_vec(const struct idpf_vport *vport,
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index ca302df9ff40..13a7581c07e6 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -2804,6 +2804,10 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport)
>    * @vport: virtual port data structure
>    * @get: flag to set or get rss look up table
>    *
> + * When rxhash is disabled, RSS LUT will be configured with zeros.  If rxhash
> + * is enabled, the LUT values stored in driver's soft copy will be used to setup
> + * the HW.
> + *
>    * Returns 0 on success, negative on failure.
>    */
>   int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
> @@ -2814,10 +2818,12 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
>   	struct idpf_rss_data *rss_data;
>   	int buf_size, lut_buf_size;
>   	ssize_t reply_sz;
> +	bool rxhash_ena;
>   	int i;
>   
>   	rss_data =
>   		&vport->adapter->vport_config[vport->idx]->user_config.rss_data;
> +	rxhash_ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
>   	buf_size = struct_size(rl, lut, rss_data->rss_lut_size);
>   	rl = kzalloc(buf_size, GFP_KERNEL);
>   	if (!rl)
> @@ -2839,7 +2845,8 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
>   	} else {
>   		rl->lut_entries = cpu_to_le16(rss_data->rss_lut_size);
>   		for (i = 0; i < rss_data->rss_lut_size; i++)
> -			rl->lut[i] = cpu_to_le32(rss_data->rss_lut[i]);
> +			rl->lut[i] = (rxhash_ena) ?
> +				cpu_to_le32(rss_data->rss_lut[i]) : 0;
>   
>   		xn_params.vc_op = VIRTCHNL2_OP_SET_RSS_LUT;
>   	}

With the minor comments above addressed, feel free to add:

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations

[Joshi, Sreedevi]

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Tuesday, November 18, 2025 2:22 AM
> To: Joshi, Sreedevi <sreedevi.joshi@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Samudrala, Sridhar <sridhar.samudrala@intel.com>; Tantilov, Emil S
> <emil.s.tantilov@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations
> 
> Dear Sreedevi,
> 
> 
> Thank you for your patch.
> 
> Am 18.11.25 um 05:22 schrieb Sreedevi Joshi:
> > The RSS LUT is not initialized until the interface comes up, causing
> > the following NULL pointer crash when ethtool operations like rxhash on/off
> > are performed before the interface is brought up for the first time.
> >
> > Move RSS LUT initialization from ndo_open to vport creation to ensure LUT
> > is always available. This enables RSS configuration via ethtool before
> > bringing the interface up. Simplify LUT management by maintaining all
> > changes in the driver's soft copy and programming zeros to the indirection
> > table when rxhash is disabled. Defer HW programming until the interface
> > comes up if it is down during rxhash and LUT configuration changes.
> >
> > [89408.371875] BUG: kernel NULL pointer dereference, address: 0000000000000000
> > [89408.371908] #PF: supervisor read access in kernel mode
> > [89408.371924] #PF: error_code(0x0000) - not-present page
> > [89408.371940] PGD 0 P4D 0
> > [89408.371953] Oops: Oops: 0000 [#1] SMP NOPTI
> > <snip>
> > [89408.372052] RIP: 0010:memcpy_orig+0x16/0x130
> > [89408.372310] Call Trace:
> > [89408.372317]  <TASK>
> > [89408.372326]  ? idpf_set_features+0xfc/0x180 [idpf]
> > [89408.372363]  __netdev_update_features+0x295/0xde0
> > [89408.372384]  ethnl_set_features+0x15e/0x460
> > [89408.372406]  genl_family_rcv_msg_doit+0x11f/0x180
> > [89408.372429]  genl_rcv_msg+0x1ad/0x2b0
> > [89408.372446]  ? __pfx_ethnl_set_features+0x10/0x10
> > [89408.372465]  ? __pfx_genl_rcv_msg+0x10/0x10
> > [89408.372482]  netlink_rcv_skb+0x58/0x100
> > [89408.372502]  genl_rcv+0x2c/0x50
> > [89408.372516]  netlink_unicast+0x289/0x3e0
> > [89408.372533]  netlink_sendmsg+0x215/0x440
> > [89408.372551]  __sys_sendto+0x234/0x240
> > [89408.372571]  __x64_sys_sendto+0x28/0x30
> > [89408.372585]  x64_sys_call+0x1909/0x1da0
> > [89408.372604]  do_syscall_64+0x7a/0xfa0
> > [89408.373140]  ? clear_bhb_loop+0x60/0xb0
> > [89408.373647]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > [89408.378887]  </TASK>
> > <snip>
> 
> It’d be great if you described your test system.
Will do. I could list the commands leading up to the crash above. 
> 
> > Fixes: a251eee62133 ("idpf: add SRIOV support and other ndo_ops")
> > Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> > ---
> >   drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
> >   drivers/net/ethernet/intel/idpf/idpf_lib.c    | 89 +++++++++----------
> >   drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 36 +++-----
> >   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  4 +-
> >   .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
> >   5 files changed, 64 insertions(+), 76 deletions(-)
.......
> >   	if (changed & NETIF_F_RXHASH) {
> > +		struct idpf_netdev_priv *np = netdev_priv(netdev);
> > +
> >   		netdev->features ^= NETIF_F_RXHASH;
> > -		err = idpf_vport_manage_rss_lut(vport);
> > +
> > +		/* If the Interface is not up when changing the rxhash, update to the HW is
> 
> I’d spell interface lowercase.
Will do.
> 
> > +		 * skipped. The updated LUT will be committed to the HW when the interface
> > +		 * is brought up.
> > +		 */
> > +		if (np->state == __IDPF_VPORT_UP)
> > +			err = idpf_config_rss(vport);
> >   		if (err)
> >   			goto unlock_mutex;
> >   	}
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > index dcdd4fef1c7a..11f711997db8 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > @@ -2868,7 +2868,6 @@ int idpf_tso(struct sk_buff *skb, struct idpf_tx_offload_params *off)
> >   	return 1;
> >   }
> >
> > -
> 
> Unrelated.
> 
> >   /**
> >    * idpf_tx_splitq_get_ctx_desc - grab next desc and update buffer ring
> >    * @txq: queue to put context descriptor on
> > @@ -4486,6 +4485,7 @@ static void idpf_vport_intr_napi_add_all(struct idpf_vport *vport)
> >
> >   	for (v_idx = 0; v_idx < vport->num_q_vectors; v_idx++) {
> >   		struct idpf_q_vector *q_vector = &vport->q_vectors[v_idx];
> > +
> 
> Unrelated.
I will move them to a separate cleanup patch.
> 
> >   		qv_idx = vport->q_vector_idxs[v_idx];
> >   		irq_num = vport->adapter->msix_entries[qv_idx].vector;
> >
> 
> With the minor comments above addressed, feel free to add:
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul
Thank you for the review and feedback, Paul!
Best Regards,
Sreedevi

Re: [Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations

[Tony Nguyen]

On 11/17/2025 8:22 PM, Sreedevi Joshi wrote:
> The RSS LUT is not initialized until the interface comes up, causing
> the following NULL pointer crash when ethtool operations like rxhash on/off
> are performed before the interface is brought up for the first time.
> 
> Move RSS LUT initialization from ndo_open to vport creation to ensure LUT
> is always available. This enables RSS configuration via ethtool before
> bringing the interface up. Simplify LUT management by maintaining all
> changes in the driver's soft copy and programming zeros to the indirection
> table when rxhash is disabled. Defer HW programming until the interface
> comes up if it is down during rxhash and LUT configuration changes.
> 
> [89408.371875] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [89408.371908] #PF: supervisor read access in kernel mode
> [89408.371924] #PF: error_code(0x0000) - not-present page
> [89408.371940] PGD 0 P4D 0
> [89408.371953] Oops: Oops: 0000 [#1] SMP NOPTI
> <snip>
> [89408.372052] RIP: 0010:memcpy_orig+0x16/0x130
> [89408.372310] Call Trace:
> [89408.372317]  <TASK>
> [89408.372326]  ? idpf_set_features+0xfc/0x180 [idpf]
> [89408.372363]  __netdev_update_features+0x295/0xde0
> [89408.372384]  ethnl_set_features+0x15e/0x460
> [89408.372406]  genl_family_rcv_msg_doit+0x11f/0x180
> [89408.372429]  genl_rcv_msg+0x1ad/0x2b0
> [89408.372446]  ? __pfx_ethnl_set_features+0x10/0x10
> [89408.372465]  ? __pfx_genl_rcv_msg+0x10/0x10
> [89408.372482]  netlink_rcv_skb+0x58/0x100
> [89408.372502]  genl_rcv+0x2c/0x50
> [89408.372516]  netlink_unicast+0x289/0x3e0
> [89408.372533]  netlink_sendmsg+0x215/0x440
> [89408.372551]  __sys_sendto+0x234/0x240
> [89408.372571]  __x64_sys_sendto+0x28/0x30
> [89408.372585]  x64_sys_call+0x1909/0x1da0
> [89408.372604]  do_syscall_64+0x7a/0xfa0
> [89408.373140]  ? clear_bhb_loop+0x60/0xb0
> [89408.373647]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [89408.378887]  </TASK>
> <snip>
> 
> Fixes: a251eee62133 ("idpf: add SRIOV support and other ndo_ops")
> Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
>   drivers/net/ethernet/intel/idpf/idpf_lib.c    | 89 +++++++++----------
>   drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 36 +++-----
>   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  4 +-
>   .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
>   5 files changed, 64 insertions(+), 76 deletions(-)
> 

...


> @@ -1289,6 +1291,13 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
>   	/* Initialize default rss key */
>   	netdev_rss_key_fill((void *)rss_data->rss_key, rss_data->rss_key_size);
>   
> +	/* Initialize default rss LUT */
> +	err = idpf_init_rss_lut(vport);
> +	if (err) {
> +		kfree(rss_data->rss_key);

Can you move this free into the goto path? In case anything new gets 
added in the future, it'll already be there.

> +		goto free_vport;

The previous error/goto goes to free_vector_idxs. Would this goto leak 
q_vector_idxs?

> +	}
> +
>   	/* fill vport slot in the adapter struct */
>   	adapter->vports[idx] = vport;
>   	adapter->vport_ids[idx] = idpf_get_vport_id(vport);

...

> @@ -1593,7 +1612,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
>   	return 0;
>   
>   deinit_rss:
> -	idpf_deinit_rss(vport);
> +	idpf_deinit_rss_lut(vport);

Since this patch moved init out of open, should this be moved out too?

>   disable_vport:
>   	idpf_send_disable_vport_msg(vport);
>   disable_queues:
>

...

> @@ -2839,7 +2845,8 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
>   	} else {
>   		rl->lut_entries = cpu_to_le16(rss_data->rss_lut_size);
>   		for (i = 0; i < rss_data->rss_lut_size; i++)
> -			rl->lut[i] = cpu_to_le32(rss_data->rss_lut[i]);
> +			rl->lut[i] = (rxhash_ena) ?

The parens don't look needed.

Thanks,
Tony

> +				cpu_to_le32(rss_data->rss_lut[i]) : 0;
>   
>   		xn_params.vc_op = VIRTCHNL2_OP_SET_RSS_LUT;
>   	}

Re: [Intel-wired-lan] [PATCH iwl-net 2/3] idpf: Fix RSS LUT configuration on down interfaces

[Tony Nguyen]

On 11/17/2025 8:22 PM, Sreedevi Joshi wrote:
> RSS LUT provisioning and queries on a down interface currently return
> silently without effect. Users should be able to configure RSS settings
> even when the interface is down.
> 
> Fix by maintaining RSS configuration changes in the driver's soft copy and
> deferring HW programming until the interface comes up.
> 
> Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks")
> Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> index 4c6e52253ae4..d9903a21972a 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> @@ -393,7 +393,10 @@ static u32 idpf_get_rxfh_indir_size(struct net_device *netdev)
>    * @netdev: network interface device structure
>    * @rxfh: pointer to param struct (indir, key, hfunc)
>    *
> - * Reads the indirection table directly from the hardware. Always returns 0.
> + * RSS LUT and Key information are read from driver's cached
> + * copy. When rxhash is off, rss lut will be displayed as zeros.
> + *
> + * Returns 0 on success.

Can you make this Return: to make kdoc happy?

>    */
>   static int idpf_get_rxfh(struct net_device *netdev,
>   			 struct ethtool_rxfh_param *rxfh)
> @@ -401,10 +404,13 @@ static int idpf_get_rxfh(struct net_device *netdev,
>   	struct idpf_netdev_priv *np = netdev_priv(netdev);
>   	struct idpf_rss_data *rss_data;
>   	struct idpf_adapter *adapter;
> +	struct idpf_vport *vport;
> +	bool rxhash_ena;
>   	int err = 0;
>   	u16 i;
>   
>   	idpf_vport_ctrl_lock(netdev);
> +	vport = idpf_netdev_to_vport(netdev);
>   
>   	adapter = np->adapter;
>   
> @@ -414,9 +420,8 @@ static int idpf_get_rxfh(struct net_device *netdev,
>   	}
>   
>   	rss_data = &adapter->vport_config[np->vport_idx]->user_config.rss_data;
> -	if (np->state != __IDPF_VPORT_UP)
> -		goto unlock_mutex;
>   
> +	rxhash_ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
>   	rxfh->hfunc = ETH_RSS_HASH_TOP;
>   
>   	if (rxfh->key)
> @@ -424,7 +429,7 @@ static int idpf_get_rxfh(struct net_device *netdev,
>   
>   	if (rxfh->indir) {
>   		for (i = 0; i < rss_data->rss_lut_size; i++)
> -			rxfh->indir[i] = rss_data->rss_lut[i];
> +			rxfh->indir[i] = (rxhash_ena) ? rss_data->rss_lut[i] : 0;

Parens not needed

>   	}
>   
>   unlock_mutex:
> @@ -464,8 +469,6 @@ static int idpf_set_rxfh(struct net_device *netdev,
>   	}
>   
>   	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
> -	if (np->state != __IDPF_VPORT_UP)
> -		goto unlock_mutex;
>   
>   	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
>   	    rxfh->hfunc != ETH_RSS_HASH_TOP) {
> @@ -481,6 +484,8 @@ static int idpf_set_rxfh(struct net_device *netdev,
>   			rss_data->rss_lut[lut] = rxfh->indir[lut];
>   	}
>   
> +	if (np->state != __IDPF_VPORT_UP)
> +		goto unlock_mutex;
>   	err = idpf_config_rss(vport);
>   
>   unlock_mutex:

Re: [Intel-wired-lan] [PATCH iwl-net 2/3] idpf: Fix RSS LUT configuration on down interfaces

[Joshi, Sreedevi]

> -----Original Message-----
> From: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Sent: Friday, November 21, 2025 4:27 PM
> To: Joshi, Sreedevi <sreedevi.joshi@intel.com>; intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Samudrala, Sridhar
> <sridhar.samudrala@intel.com>; Tantilov, Emil S <emil.s.tantilov@intel.com>
> Subject: Re: [PATCH iwl-net 2/3] idpf: Fix RSS LUT configuration on down interfaces
> 
> 
> 
> On 11/17/2025 8:22 PM, Sreedevi Joshi wrote:
> > RSS LUT provisioning and queries on a down interface currently return
> > silently without effect. Users should be able to configure RSS settings
> > even when the interface is down.
> >
> > Fix by maintaining RSS configuration changes in the driver's soft copy and
> > deferring HW programming until the interface comes up.
> >
> > Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks")
> > Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> > ---
> >   drivers/net/ethernet/intel/idpf/idpf_ethtool.c | 17 +++++++++++------
> >   1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> > index 4c6e52253ae4..d9903a21972a 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> > @@ -393,7 +393,10 @@ static u32 idpf_get_rxfh_indir_size(struct net_device *netdev)
> >    * @netdev: network interface device structure
> >    * @rxfh: pointer to param struct (indir, key, hfunc)
> >    *
> > - * Reads the indirection table directly from the hardware. Always returns 0.
> > + * RSS LUT and Key information are read from driver's cached
> > + * copy. When rxhash is off, rss lut will be displayed as zeros.
> > + *
> > + * Returns 0 on success.
> 
> Can you make this Return: to make kdoc happy?
Will do. 
BTW, looks like there are other occurrences of this in that file.
> 
> >    */
> >   static int idpf_get_rxfh(struct net_device *netdev,
> >   			 struct ethtool_rxfh_param *rxfh)
> > @@ -401,10 +404,13 @@ static int idpf_get_rxfh(struct net_device *netdev,
> >   	struct idpf_netdev_priv *np = netdev_priv(netdev);
> >   	struct idpf_rss_data *rss_data;
> >   	struct idpf_adapter *adapter;
> > +	struct idpf_vport *vport;
> > +	bool rxhash_ena;
> >   	int err = 0;
> >   	u16 i;
> >
> >   	idpf_vport_ctrl_lock(netdev);
> > +	vport = idpf_netdev_to_vport(netdev);
> >
> >   	adapter = np->adapter;
> >
> > @@ -414,9 +420,8 @@ static int idpf_get_rxfh(struct net_device *netdev,
> >   	}
> >
> >   	rss_data = &adapter->vport_config[np->vport_idx]->user_config.rss_data;
> > -	if (np->state != __IDPF_VPORT_UP)
> > -		goto unlock_mutex;
> >
> > +	rxhash_ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
> >   	rxfh->hfunc = ETH_RSS_HASH_TOP;
> >
> >   	if (rxfh->key)
> > @@ -424,7 +429,7 @@ static int idpf_get_rxfh(struct net_device *netdev,
> >
> >   	if (rxfh->indir) {
> >   		for (i = 0; i < rss_data->rss_lut_size; i++)
> > -			rxfh->indir[i] = rss_data->rss_lut[i];
> > +			rxfh->indir[i] = (rxhash_ena) ? rss_data->rss_lut[i] : 0;
> 
> Parens not needed
Ok. Will fix.

Thanks for the review!
Regards,
Sreedevi
> 
> >   	}
> >
> >   unlock_mutex:
> > @@ -464,8 +469,6 @@ static int idpf_set_rxfh(struct net_device *netdev,
> >   	}
> >
> >   	rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
> > -	if (np->state != __IDPF_VPORT_UP)
> > -		goto unlock_mutex;
> >
> >   	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
> >   	    rxfh->hfunc != ETH_RSS_HASH_TOP) {
> > @@ -481,6 +484,8 @@ static int idpf_set_rxfh(struct net_device *netdev,
> >   			rss_data->rss_lut[lut] = rxfh->indir[lut];
> >   	}
> >
> > +	if (np->state != __IDPF_VPORT_UP)
> > +		goto unlock_mutex;
> >   	err = idpf_config_rss(vport);
> >
> >   unlock_mutex:

Re: [Intel-wired-lan] [PATCH iwl-net 2/3] idpf: Fix RSS LUT configuration on down interfaces

[Tony Nguyen]

On 11/21/2025 1:53 PM, Joshi, Sreedevi wrote:

...

>>> @@ -393,7 +393,10 @@ static u32 idpf_get_rxfh_indir_size(struct net_device *netdev)
>>>     * @netdev: network interface device structure
>>>     * @rxfh: pointer to param struct (indir, key, hfunc)
>>>     *
>>> - * Reads the indirection table directly from the hardware. Always returns 0.
>>> + * RSS LUT and Key information are read from driver's cached
>>> + * copy. When rxhash is off, rss lut will be displayed as zeros.
>>> + *
>>> + * Returns 0 on success.
>>
>> Can you make this Return: to make kdoc happy?
> Will do.
> BTW, looks like there are other occurrences of this in that file.

Yea, much of this file was committed before that check was put in 
place/enforced so there are many occurrences of this in the file :( We 
don't want to introduce any new violations though.

Thanks,
Tony

Re: [Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations

[Joshi, Sreedevi]

> -----Original Message-----
> From: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Sent: Friday, November 21, 2025 4:22 PM
> To: Joshi, Sreedevi <sreedevi.joshi@intel.com>; intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Samudrala, Sridhar <sridhar.samudrala@intel.com>; Tantilov, Emil S <emil.s.tantilov@intel.com>
> Subject: Re: [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations
> 
> 
> 
> On 11/17/2025 8:22 PM, Sreedevi Joshi wrote:
> > The RSS LUT is not initialized until the interface comes up, causing
> > the following NULL pointer crash when ethtool operations like rxhash on/off
> > are performed before the interface is brought up for the first time.
> >
> > Move RSS LUT initialization from ndo_open to vport creation to ensure LUT
> > is always available. This enables RSS configuration via ethtool before
> > bringing the interface up. Simplify LUT management by maintaining all
> > changes in the driver's soft copy and programming zeros to the indirection
> > table when rxhash is disabled. Defer HW programming until the interface
> > comes up if it is down during rxhash and LUT configuration changes.
> >
> > [89408.371875] BUG: kernel NULL pointer dereference, address: 0000000000000000
> > [89408.371908] #PF: supervisor read access in kernel mode
> > [89408.371924] #PF: error_code(0x0000) - not-present page
> > [89408.371940] PGD 0 P4D 0
> > [89408.371953] Oops: Oops: 0000 [#1] SMP NOPTI
> > <snip>
> > [89408.372052] RIP: 0010:memcpy_orig+0x16/0x130
> > [89408.372310] Call Trace:
> > [89408.372317]  <TASK>
> > [89408.372326]  ? idpf_set_features+0xfc/0x180 [idpf]
> > [89408.372363]  __netdev_update_features+0x295/0xde0
> > [89408.372384]  ethnl_set_features+0x15e/0x460
> > [89408.372406]  genl_family_rcv_msg_doit+0x11f/0x180
> > [89408.372429]  genl_rcv_msg+0x1ad/0x2b0
> > [89408.372446]  ? __pfx_ethnl_set_features+0x10/0x10
> > [89408.372465]  ? __pfx_genl_rcv_msg+0x10/0x10
> > [89408.372482]  netlink_rcv_skb+0x58/0x100
> > [89408.372502]  genl_rcv+0x2c/0x50
> > [89408.372516]  netlink_unicast+0x289/0x3e0
> > [89408.372533]  netlink_sendmsg+0x215/0x440
> > [89408.372551]  __sys_sendto+0x234/0x240
> > [89408.372571]  __x64_sys_sendto+0x28/0x30
> > [89408.372585]  x64_sys_call+0x1909/0x1da0
> > [89408.372604]  do_syscall_64+0x7a/0xfa0
> > [89408.373140]  ? clear_bhb_loop+0x60/0xb0
> > [89408.373647]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > [89408.378887]  </TASK>
> > <snip>
> >
> > Fixes: a251eee62133 ("idpf: add SRIOV support and other ndo_ops")
> > Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
> > ---
> >   drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
> >   drivers/net/ethernet/intel/idpf/idpf_lib.c    | 89 +++++++++----------
> >   drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 36 +++-----
> >   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  4 +-
> >   .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
> >   5 files changed, 64 insertions(+), 76 deletions(-)
> >
> 
> ...
> 
> 
> > @@ -1289,6 +1291,13 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
> >   	/* Initialize default rss key */
> >   	netdev_rss_key_fill((void *)rss_data->rss_key, rss_data->rss_key_size);
> >
> > +	/* Initialize default rss LUT */
> > +	err = idpf_init_rss_lut(vport);
> > +	if (err) {
> > +		kfree(rss_data->rss_key);
> 
> Can you move this free into the goto path? In case anything new gets
> added in the future, it'll already be there.
> 
Will do.
> > +		goto free_vport;
> 
> The previous error/goto goes to free_vector_idxs. Would this goto leak
> q_vector_idxs?
> 
Good catch! Will fix.
> > +	}
> > +
> >   	/* fill vport slot in the adapter struct */
> >   	adapter->vports[idx] = vport;
> >   	adapter->vport_ids[idx] = idpf_get_vport_id(vport);
> 
> ...
> 
> > @@ -1593,7 +1612,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
> >   	return 0;
> >
> >   deinit_rss:
> > -	idpf_deinit_rss(vport);
> > +	idpf_deinit_rss_lut(vport);
> 
> Since this patch moved init out of open, should this be moved out too?
> 
Wanted to have a corresponding deinit when init is done in this function. However, I think it is ok to not deinit here.
The logic will be updated for soft reset logic anyway to not dealloc the rss_lut in patch3 and then there is no need for
init from this function.
While reviewing all the goto's in this function, I found two more changes that will be required. Shall 
create a separate patch for this? (they are not directly related to the RSS)
@@ -1526,14 +1526,14 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
        if (err) {
                dev_err(&adapter->pdev->dev, "Failed to initialize queue registers for vport %u: %d\n",
                        vport->vport_id, err);
-               goto queues_rel;
+              goto intr_deinit;
        }
 
        err = idpf_rx_bufs_init_all(vport);
        if (err) {
                dev_err(&adapter->pdev->dev, "Failed to initialize RX buffers for vport %u: %d\n",
                        vport->vport_id, err);
-               goto queues_rel;
+              goto intr_deinit;
        }
> >   disable_vport:
> >   	idpf_send_disable_vport_msg(vport);
> >   disable_queues:
> >
> 
> ...
> 
> > @@ -2839,7 +2845,8 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
> >   	} else {
> >   		rl->lut_entries = cpu_to_le16(rss_data->rss_lut_size);
> >   		for (i = 0; i < rss_data->rss_lut_size; i++)
> > -			rl->lut[i] = cpu_to_le32(rss_data->rss_lut[i]);
> > +			rl->lut[i] = (rxhash_ena) ?
> 
> The parens don't look needed.
> 
Will fix.
Thanks for the review, Tony!
Regards,
Sreedevi

> Thanks,
> Tony
> 
> > +				cpu_to_le32(rss_data->rss_lut[i]) : 0;
> >
> >   		xn_params.vc_op = VIRTCHNL2_OP_SET_RSS_LUT;
> >   	}