[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <IA1PR11MB6289306D2239682C7E5F50B589D6A@IA1PR11MB6289.namprd11.prod.outlook.com>
Date: Tue, 18 Nov 2025 19:20:31 +0000
From: "Joshi, Sreedevi" <sreedevi.joshi@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Samudrala, Sridhar"
<sridhar.samudrala@...el.com>, "Tantilov, Emil S" <emil.s.tantilov@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL
pointer crash on early ethtool operations
> -----Original Message-----
> From: Paul Menzel <pmenzel@...gen.mpg.de>
> Sent: Tuesday, November 18, 2025 2:22 AM
> To: Joshi, Sreedevi <sreedevi.joshi@...el.com>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Samudrala, Sridhar <sridhar.samudrala@...el.com>; Tantilov, Emil S
> <emil.s.tantilov@...el.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@...el.com>
> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@...el.com>
> > Reviewed-by: Emil Tantilov <emil.s.tantilov@...el.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@...gen.mpg.de>
>
>
> Kind regards,
>
> Paul
Thank you for the review and feedback, Paul!
Best Regards,
Sreedevi
Powered by blists - more mailing lists