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] [day] [month] [year] [list]

Message-ID: <acd9c54a-bfaa-44f3-94b3-85442277a65f@linux.dev>
Date: Thu, 21 Nov 2024 17:50:25 -0800
From: Jesse Brandeburg <jesse.brandeburg@...ux.dev>
To: "Ertman, David M" <david.m.ertman@...el.com>,
 "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

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?