netdev - RE: [PATCH net v1] ice: do not reserve resources for RDMA when disabled

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: <IA1PR11MB61942704E6FF9FDEE627DE7BDD232@IA1PR11MB6194.namprd11.prod.outlook.com>
Date: Fri, 22 Nov 2024 22:35:05 +0000
From: "Ertman, David M" <david.m.ertman@...el.com>
To: Jesse Brandeburg <jesse.brandeburg@...ux.dev>, "jbrandeb@...nel.org"
	<jbrandeb@...nel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net v1] ice: do not reserve resources for RDMA when
 disabled

> -----Original Message-----
> From: Jesse Brandeburg <jesse.brandeburg@...ux.dev>
> Sent: Thursday, November 21, 2024 5:50 PM
> To: Ertman, David M <david.m.ertman@...el.com>; jbrandeb@...nel.org;
> netdev@...r.kernel.org
> Subject: Re: [PATCH net v1] ice: do not reserve resources for RDMA when
> disabled
> 
> On 11/15/24 10:46 AM, Jesse Brandeburg wrote:
> > On 11/14/24 10:06 AM, Ertman, David M wrote:
> >>>       case ICE_AQC_CAPS_RDMA:
> >>> -        caps->rdma = (number == 1);
> >>> +        if (IS_ENABLED(CONFIG_INFINIBAND_IRDMA))
> >>> +            caps->rdma = (number == 1);
> >>>           ice_debug(hw, ICE_DBG_INIT, "%s: rdma = %d\n", prefix,
> >>
> >> The HW caps struct should always accurately reflect the capabilities
> >> of the HW being probed.  Since this
> >
> > why must it accurately reflect the capability of the hardware? The
> > driver state and capability is a reflection of the combination of both,
> > so I'm not sure what the point of your statement.
> >
> >> is a kernel configuration (i.e. software) consideration, the more
> >> appropriate approach would be to control
> >> the PF flag "ICE_FLAG_RDMA_ENA" based on the kernel CONFIG setting.
> >
> > I started making the changes you suggested, but the ICE_FLAG_RDMA_ENA is
> > blindly set by the LAG code, if the cap.rdma is enabled. see
> > ice_set_rdma_cap(). This means the disable won't stick.
> >
> > Unless I'm misunderstanding something, ICE_FLAG_RDMA_ENA is used both as
> > a gate and as a state, which is a design issue. This leaves no choice
> > but to implement the way I did in this v1 patch. Do you see any other
> > option to make a simple change that is safe for backporting to stable?
> 
> Any comments here Dave? 

Jesse,

Looking at the FLAG as used in the ice driver, it is used as an ephemeral state for
the enablement of RDMA (gated by the value of the capabilities struct).  So, I agree
that a kernel-wide config value should not be tied to it as well.

Since the kernel config value will not change for the life of the driver, it should be
OK to alter the capabilities struct value based on the kernel's configuration.

Sorry for the thrash.

DaveE

Acked-by: Dave Ertman <david.m.ertman@...el.com>