lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b61284b2-8e8e-c867-62fd-0f8090f1eac1@gmail.com>
Date: Thu, 5 Oct 2023 21:54:24 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: edward.cree@....com, linux-net-drivers@....com, davem@...emloft.net,
 kuba@...nel.org, edumazet@...gle.com, pabeni@...hat.com,
 netdev@...r.kernel.org, sudheer.mogilappagari@...el.com, jdamato@...tly.com,
 andrew@...n.ch, mw@...ihalf.com, linux@...linux.org.uk,
 sgoutham@...vell.com, gakula@...vell.com, sbhatta@...vell.com,
 hkelam@...vell.com, saeedm@...dia.com, leon@...nel.org
Subject: Re: [PATCH v4 net-next 7/7] sfc: use new rxfh_context API

On 02/10/2023 14:01, Martin Habets wrote:
> On Wed, Sep 27, 2023 at 07:13:38PM +0100, edward.cree@....com wrote:
>> From: Edward Cree <ecree.xilinx@...il.com>
>>
>> The core is now responsible for allocating IDs and a memory region for
>>  us to store our state (struct efx_rss_context_priv), so we no longer
>>  need efx_alloc_rss_context_entry() and friends.
>> Since the contexts are now maintained by the core, use the core's lock
>>  (net_dev->ethtool->rss_lock), rather than our own mutex (efx->rss_lock),
>>  to serialise access against changes; and remove the now-unused
>>  efx->rss_lock from struct efx_nic.
>>
>> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
...
>> +		rc = efx_mcdi_rx_push_rss_context_config(efx, priv, indir, key,
>> +							 false);
>>  		if (rc)
>>  			netif_warn(efx, probe, efx->net_dev,
>> -				   "failed to restore RSS context %u, rc=%d"
>> +				   "failed to restore RSS context %lu, rc=%d"
>>  				   "; RSS filters may fail to be applied\n",
>> -				   ctx->user_id, rc);
>> +				   context, rc);
> 
> If this fails the state in the core is out-of-sync with that in the NIC.
> Should we remove the RSS context from efx->net_dev->ethtool->rss_ctx, or do
> we expect admins to do that manually?
> 
> Martin

I definitely think it's a bad idea to have drivers removing things
 from the array behind the kernel's back.  Besides, I think it ought
 to keep the configuration the user asked for, so that if they fix
 something and then trigger another MC reboot the original config
 will get reapplied.
Possibly if we go with Jakub's suggestion of a replay mechanism
 then the core can react to failure returns from the reoffload call,
 either by removing the context from the array or by setting some
 kind of 'broken' flag that can be reported to userspace in dumps.

-ed

Powered by blists - more mailing lists