netdev - Re: [PATCH v3 08/14] RDMA/ionic: Register auxiliary module for ionic ethernet adapter

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: <f0050f1b-67f9-8a72-9365-2451a6f360ff@amd.com>
Date: Fri, 27 Jun 2025 15:48:18 +0530
From: Abhijit Gangurde <abhijit.gangurde@....com>
To: Leon Romanovsky <leon@...nel.org>
Cc: shannon.nelson@....com, brett.creeley@....com, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, corbet@....net,
 jgg@...pe.ca, andrew+netdev@...n.ch, allen.hubbe@....com,
 nikhil.agarwal@....com, linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 Andrew Boyer <andrew.boyer@....com>
Subject: Re: [PATCH v3 08/14] RDMA/ionic: Register auxiliary module for ionic
 ethernet adapter

On 6/26/25 12:56, Leon Romanovsky wrote:
> On Tue, Jun 24, 2025 at 05:43:09PM +0530, Abhijit Gangurde wrote:
>> Register auxiliary module to create ibdevice for ionic
>> ethernet adapter.
>>
>> Co-developed-by: Andrew Boyer <andrew.boyer@....com>
>> Signed-off-by: Andrew Boyer <andrew.boyer@....com>
>> Co-developed-by: Allen Hubbe <allen.hubbe@....com>
>> Signed-off-by: Allen Hubbe <allen.hubbe@....com>
>> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@....com>
>> ---
>> v1->v2
>>    - Removed netdev references from ionic RDMA driver
>>    - Moved to ionic_lif* instead of void* to convey information between
>>      aux devices and drivers.
>>
>>   drivers/infiniband/hw/ionic/ionic_ibdev.c   | 133 ++++++++++++++++++++
>>   drivers/infiniband/hw/ionic/ionic_ibdev.h   |  21 ++++
>>   drivers/infiniband/hw/ionic/ionic_lif_cfg.c | 118 +++++++++++++++++
>>   drivers/infiniband/hw/ionic/ionic_lif_cfg.h |  65 ++++++++++
>>   4 files changed, 337 insertions(+)
>>   create mode 100644 drivers/infiniband/hw/ionic/ionic_ibdev.c
>>   create mode 100644 drivers/infiniband/hw/ionic/ionic_ibdev.h
>>   create mode 100644 drivers/infiniband/hw/ionic/ionic_lif_cfg.c
>>   create mode 100644 drivers/infiniband/hw/ionic/ionic_lif_cfg.h
> <...>
>
>> +	rc = ionic_version_check(&ionic_adev->adev.dev, ionic_adev->lif);
>> +	if (rc)
>> +		return ERR_PTR(rc);
> <...>
>
>> +struct net_device *ionic_lif_netdev(struct ionic_lif *lif)
>> +{
>> +	return lif->netdev;
>> +}
> Why do you need to store netdev pointer?
> Why can't you use existing ib_device_get_netdev/ib_device_set_netdev?

We are not storing the netdev in RDMA driver. This function is accessing
the ethernet copy to set netdev and guid. I can add dev_hold() here
till netdev is registered with ib device.

>
>> +
>> +int ionic_version_check(const struct device *dev, struct ionic_lif *lif)
>> +{
>> +	union ionic_lif_identity *ident = &lif->ionic->ident.lif;
>> +
>> +	if (ident->rdma.version < IONIC_MIN_RDMA_VERSION ||
>> +	    ident->rdma.version > IONIC_MAX_RDMA_VERSION) {
>> +		dev_err_probe(dev, -EINVAL,
>> +			      "ionic_rdma: incompatible version, fw ver %u\n",
>> +			      ident->rdma.version);
>> +		dev_err_probe(dev, -EINVAL,
>> +			      "ionic_rdma: Driver Min Version %u\n",
>> +			      IONIC_MIN_RDMA_VERSION);
>> +		dev_err_probe(dev, -EINVAL,
>> +			      "ionic_rdma: Driver Max Version %u\n",
>> +			      IONIC_MAX_RDMA_VERSION);
>> +	}
>> +
>> +	return 0;
>> +}
> Upstream code has all subsystems in sync, and RDMA driver is always
> compatible with its netdev counterpart. Please remove this part.
>
> This is not full review yet, please wait till next week, so we will
> review more deeply.
>
> Thanks
I will remove this.

Thanks,
Abhijit