Skip to content

Commit 83f38f2

Browse files
sreedeviintelanguy11
authored andcommitted
idpf: Fix RSS LUT NULL pointer crash on early ethtool operations
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. Steps to reproduce: ** Load idpf driver; interfaces will be created modprobe idpf ** Before bringing the interfaces up, turn rxhash off ethtool -K eth2 rxhash off [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: a251eee ("idpf: add SRIOV support and other ndo_ops") Signed-off-by: Sreedevi Joshi <[email protected]> Reviewed-by: Sridhar Samudrala <[email protected]> Reviewed-by: Emil Tantilov <[email protected]> Reviewed-by: Aleksandr Loktionov <[email protected]> Reviewed-by: Paul Menzel <[email protected]> Reviewed-by: Simon Horman <[email protected]> Tested-by: Samuel Salin <[email protected]> Signed-off-by: Tony Nguyen <[email protected]>
1 parent 36aae2e commit 83f38f2

File tree

5 files changed

+66
-79
lines changed

5 files changed

+66
-79
lines changed

drivers/net/ethernet/intel/idpf/idpf.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,14 +423,12 @@ enum idpf_user_flags {
423423
* @rss_key: RSS hash key
424424
* @rss_lut_size: Size of RSS lookup table
425425
* @rss_lut: RSS lookup table
426-
* @cached_lut: Used to restore previously init RSS lut
427426
*/
428427
struct idpf_rss_data {
429428
u16 rss_key_size;
430429
u8 *rss_key;
431430
u16 rss_lut_size;
432431
u32 *rss_lut;
433-
u32 *cached_lut;
434432
};
435433

436434
/**

drivers/net/ethernet/intel/idpf/idpf_lib.c

Lines changed: 43 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,7 @@ static void idpf_vport_rel(struct idpf_vport *vport)
10731073
u16 idx = vport->idx;
10741074

10751075
vport_config = adapter->vport_config[vport->idx];
1076-
idpf_deinit_rss(vport);
1076+
idpf_deinit_rss_lut(vport);
10771077
rss_data = &vport_config->user_config.rss_data;
10781078
kfree(rss_data->rss_key);
10791079
rss_data->rss_key = NULL;
@@ -1226,6 +1226,7 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
12261226
u16 idx = adapter->next_vport;
12271227
struct idpf_vport *vport;
12281228
u16 num_max_q;
1229+
int err;
12291230

12301231
if (idx == IDPF_NO_FREE_SLOT)
12311232
return NULL;
@@ -1276,10 +1277,11 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
12761277

12771278
idpf_vport_init(vport, max_q);
12781279

1279-
/* This alloc is done separate from the LUT because it's not strictly
1280-
* dependent on how many queues we have. If we change number of queues
1281-
* and soft reset we'll need a new LUT but the key can remain the same
1282-
* for as long as the vport exists.
1280+
/* LUT and key are both initialized here. Key is not strictly dependent
1281+
* on how many queues we have. If we change number of queues and soft
1282+
* reset is initiated, LUT will be freed and a new LUT will be allocated
1283+
* as per the updated number of queues during vport bringup. However,
1284+
* the key remains the same for as long as the vport exists.
12831285
*/
12841286
rss_data = &adapter->vport_config[idx]->user_config.rss_data;
12851287
rss_data->rss_key = kzalloc(rss_data->rss_key_size, GFP_KERNEL);
@@ -1289,6 +1291,11 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
12891291
/* Initialize default rss key */
12901292
netdev_rss_key_fill((void *)rss_data->rss_key, rss_data->rss_key_size);
12911293

1294+
/* Initialize default rss LUT */
1295+
err = idpf_init_rss_lut(vport);
1296+
if (err)
1297+
goto free_rss_key;
1298+
12921299
/* fill vport slot in the adapter struct */
12931300
adapter->vports[idx] = vport;
12941301
adapter->vport_ids[idx] = idpf_get_vport_id(vport);
@@ -1299,6 +1306,8 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
12991306

13001307
return vport;
13011308

1309+
free_rss_key:
1310+
kfree(rss_data->rss_key);
13021311
free_vector_idxs:
13031312
kfree(vport->q_vector_idxs);
13041313
free_vport:
@@ -1476,6 +1485,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
14761485
struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
14771486
struct idpf_adapter *adapter = vport->adapter;
14781487
struct idpf_vport_config *vport_config;
1488+
struct idpf_rss_data *rss_data;
14791489
int err;
14801490

14811491
if (test_bit(IDPF_VPORT_UP, np->state))
@@ -1570,12 +1580,21 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
15701580
idpf_restore_features(vport);
15711581

15721582
vport_config = adapter->vport_config[vport->idx];
1573-
if (vport_config->user_config.rss_data.rss_lut)
1574-
err = idpf_config_rss(vport);
1575-
else
1576-
err = idpf_init_rss(vport);
1583+
rss_data = &vport_config->user_config.rss_data;
1584+
1585+
if (!rss_data->rss_lut) {
1586+
err = idpf_init_rss_lut(vport);
1587+
if (err) {
1588+
dev_err(&adapter->pdev->dev,
1589+
"Failed to initialize RSS LUT for vport %u: %d\n",
1590+
vport->vport_id, err);
1591+
goto disable_vport;
1592+
}
1593+
}
1594+
1595+
err = idpf_config_rss(vport);
15771596
if (err) {
1578-
dev_err(&adapter->pdev->dev, "Failed to initialize RSS for vport %u: %d\n",
1597+
dev_err(&adapter->pdev->dev, "Failed to configure RSS for vport %u: %d\n",
15791598
vport->vport_id, err);
15801599
goto disable_vport;
15811600
}
@@ -1584,16 +1603,14 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
15841603
if (err) {
15851604
dev_err(&adapter->pdev->dev, "Failed to complete interface up for vport %u: %d\n",
15861605
vport->vport_id, err);
1587-
goto deinit_rss;
1606+
goto disable_vport;
15881607
}
15891608

15901609
if (rtnl)
15911610
rtnl_unlock();
15921611

15931612
return 0;
15941613

1595-
deinit_rss:
1596-
idpf_deinit_rss(vport);
15971614
disable_vport:
15981615
idpf_send_disable_vport_msg(vport);
15991616
disable_queues:
@@ -2051,7 +2068,7 @@ int idpf_initiate_soft_reset(struct idpf_vport *vport,
20512068
idpf_vport_stop(vport, false);
20522069
}
20532070

2054-
idpf_deinit_rss(vport);
2071+
idpf_deinit_rss_lut(vport);
20552072
/* We're passing in vport here because we need its wait_queue
20562073
* to send a message and it should be getting all the vport
20572074
* config data out of the adapter but we need to be careful not
@@ -2219,40 +2236,6 @@ static void idpf_set_rx_mode(struct net_device *netdev)
22192236
dev_err(dev, "Failed to set promiscuous mode: %d\n", err);
22202237
}
22212238

2222-
/**
2223-
* idpf_vport_manage_rss_lut - disable/enable RSS
2224-
* @vport: the vport being changed
2225-
*
2226-
* In the event of disable request for RSS, this function will zero out RSS
2227-
* LUT, while in the event of enable request for RSS, it will reconfigure RSS
2228-
* LUT with the default LUT configuration.
2229-
*/
2230-
static int idpf_vport_manage_rss_lut(struct idpf_vport *vport)
2231-
{
2232-
bool ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
2233-
struct idpf_rss_data *rss_data;
2234-
u16 idx = vport->idx;
2235-
int lut_size;
2236-
2237-
rss_data = &vport->adapter->vport_config[idx]->user_config.rss_data;
2238-
lut_size = rss_data->rss_lut_size * sizeof(u32);
2239-
2240-
if (ena) {
2241-
/* This will contain the default or user configured LUT */
2242-
memcpy(rss_data->rss_lut, rss_data->cached_lut, lut_size);
2243-
} else {
2244-
/* Save a copy of the current LUT to be restored later if
2245-
* requested.
2246-
*/
2247-
memcpy(rss_data->cached_lut, rss_data->rss_lut, lut_size);
2248-
2249-
/* Zero out the current LUT to disable */
2250-
memset(rss_data->rss_lut, 0, lut_size);
2251-
}
2252-
2253-
return idpf_config_rss(vport);
2254-
}
2255-
22562239
/**
22572240
* idpf_set_features - set the netdev feature flags
22582241
* @netdev: ptr to the netdev being adjusted
@@ -2278,10 +2261,19 @@ static int idpf_set_features(struct net_device *netdev,
22782261
}
22792262

22802263
if (changed & NETIF_F_RXHASH) {
2264+
struct idpf_netdev_priv *np = netdev_priv(netdev);
2265+
22812266
netdev->features ^= NETIF_F_RXHASH;
2282-
err = idpf_vport_manage_rss_lut(vport);
2283-
if (err)
2284-
goto unlock_mutex;
2267+
2268+
/* If the interface is not up when changing the rxhash, update
2269+
* to the HW is skipped. The updated LUT will be committed to
2270+
* the HW when the interface is brought up.
2271+
*/
2272+
if (test_bit(IDPF_VPORT_UP, np->state)) {
2273+
err = idpf_config_rss(vport);
2274+
if (err)
2275+
goto unlock_mutex;
2276+
}
22852277
}
22862278

22872279
if (changed & NETIF_F_GRO_HW) {

drivers/net/ethernet/intel/idpf/idpf_txrx.c

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4650,57 +4650,47 @@ static void idpf_fill_dflt_rss_lut(struct idpf_vport *vport)
46504650

46514651
rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
46524652

4653-
for (i = 0; i < rss_data->rss_lut_size; i++) {
4653+
for (i = 0; i < rss_data->rss_lut_size; i++)
46544654
rss_data->rss_lut[i] = i % num_active_rxq;
4655-
rss_data->cached_lut[i] = rss_data->rss_lut[i];
4656-
}
46574655
}
46584656

46594657
/**
4660-
* idpf_init_rss - Allocate and initialize RSS resources
4658+
* idpf_init_rss_lut - Allocate and initialize RSS LUT
46614659
* @vport: virtual port
46624660
*
4663-
* Return 0 on success, negative on failure
4661+
* Return: 0 on success, negative on failure
46644662
*/
4665-
int idpf_init_rss(struct idpf_vport *vport)
4663+
int idpf_init_rss_lut(struct idpf_vport *vport)
46664664
{
46674665
struct idpf_adapter *adapter = vport->adapter;
46684666
struct idpf_rss_data *rss_data;
4669-
u32 lut_size;
46704667

46714668
rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
4669+
if (!rss_data->rss_lut) {
4670+
u32 lut_size;
46724671

4673-
lut_size = rss_data->rss_lut_size * sizeof(u32);
4674-
rss_data->rss_lut = kzalloc(lut_size, GFP_KERNEL);
4675-
if (!rss_data->rss_lut)
4676-
return -ENOMEM;
4677-
4678-
rss_data->cached_lut = kzalloc(lut_size, GFP_KERNEL);
4679-
if (!rss_data->cached_lut) {
4680-
kfree(rss_data->rss_lut);
4681-
rss_data->rss_lut = NULL;
4682-
4683-
return -ENOMEM;
4672+
lut_size = rss_data->rss_lut_size * sizeof(u32);
4673+
rss_data->rss_lut = kzalloc(lut_size, GFP_KERNEL);
4674+
if (!rss_data->rss_lut)
4675+
return -ENOMEM;
46844676
}
46854677

46864678
/* Fill the default RSS lut values */
46874679
idpf_fill_dflt_rss_lut(vport);
46884680

4689-
return idpf_config_rss(vport);
4681+
return 0;
46904682
}
46914683

46924684
/**
4693-
* idpf_deinit_rss - Release RSS resources
4685+
* idpf_deinit_rss_lut - Release RSS LUT
46944686
* @vport: virtual port
46954687
*/
4696-
void idpf_deinit_rss(struct idpf_vport *vport)
4688+
void idpf_deinit_rss_lut(struct idpf_vport *vport)
46974689
{
46984690
struct idpf_adapter *adapter = vport->adapter;
46994691
struct idpf_rss_data *rss_data;
47004692

47014693
rss_data = &adapter->vport_config[vport->idx]->user_config.rss_data;
4702-
kfree(rss_data->cached_lut);
4703-
rss_data->cached_lut = NULL;
47044694
kfree(rss_data->rss_lut);
47054695
rss_data->rss_lut = NULL;
47064696
}

drivers/net/ethernet/intel/idpf/idpf_txrx.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,8 +1086,8 @@ void idpf_vport_intr_deinit(struct idpf_vport *vport);
10861086
int idpf_vport_intr_init(struct idpf_vport *vport);
10871087
void idpf_vport_intr_ena(struct idpf_vport *vport);
10881088
int idpf_config_rss(struct idpf_vport *vport);
1089-
int idpf_init_rss(struct idpf_vport *vport);
1090-
void idpf_deinit_rss(struct idpf_vport *vport);
1089+
int idpf_init_rss_lut(struct idpf_vport *vport);
1090+
void idpf_deinit_rss_lut(struct idpf_vport *vport);
10911091
int idpf_rx_bufs_init_all(struct idpf_vport *vport);
10921092

10931093
struct idpf_q_vector *idpf_find_rxq_vec(const struct idpf_vport *vport,

drivers/net/ethernet/intel/idpf/idpf_virtchnl.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2804,6 +2804,10 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport)
28042804
* @vport: virtual port data structure
28052805
* @get: flag to set or get rss look up table
28062806
*
2807+
* When rxhash is disabled, RSS LUT will be configured with zeros. If rxhash
2808+
* is enabled, the LUT values stored in driver's soft copy will be used to setup
2809+
* the HW.
2810+
*
28072811
* Returns 0 on success, negative on failure.
28082812
*/
28092813
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)
28142818
struct idpf_rss_data *rss_data;
28152819
int buf_size, lut_buf_size;
28162820
ssize_t reply_sz;
2821+
bool rxhash_ena;
28172822
int i;
28182823

28192824
rss_data =
28202825
&vport->adapter->vport_config[vport->idx]->user_config.rss_data;
2826+
rxhash_ena = idpf_is_feature_ena(vport, NETIF_F_RXHASH);
28212827
buf_size = struct_size(rl, lut, rss_data->rss_lut_size);
28222828
rl = kzalloc(buf_size, GFP_KERNEL);
28232829
if (!rl)
@@ -2839,7 +2845,8 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
28392845
} else {
28402846
rl->lut_entries = cpu_to_le16(rss_data->rss_lut_size);
28412847
for (i = 0; i < rss_data->rss_lut_size; i++)
2842-
rl->lut[i] = cpu_to_le32(rss_data->rss_lut[i]);
2848+
rl->lut[i] = rxhash_ena ?
2849+
cpu_to_le32(rss_data->rss_lut[i]) : 0;
28432850

28442851
xn_params.vc_op = VIRTCHNL2_OP_SET_RSS_LUT;
28452852
}

0 commit comments

Comments
 (0)