On 8/2/25 02:45, Christophe JAILLET wrote:

Le 23/07/2025 à 19:31, Abhijit Gangurde a écrit :

quoted

To support RDMA capable ethernet device, create an auxiliary device in
the ionic Ethernet driver. The RDMA device is modeled as an auxiliary
device to the Ethernet device.

...

quoted

+static DEFINE_IDA(aux_ida);
+
+static void ionic_auxbus_release(struct device *dev)
+{
+    struct ionic_aux_dev *ionic_adev;
+
+    ionic_adev = container_of(dev, struct ionic_aux_dev, adev.dev);
+    kfree(ionic_adev);
+}
+
+int ionic_auxbus_register(struct ionic_lif *lif)

The 2 places that uses thus function don't check its error code.

For the eth driver, RDMA functionality is optional hence return code was 
missed. Although devlink parameter to control this is not included in 
this series, where it needs return value from this function. Till that 
point, I'll make it return void.

quoted

+{
+    struct ionic_aux_dev *ionic_adev;
+    struct auxiliary_device *aux_dev;
+    int err, id;
+
+    if (!(le64_to_cpu(lif->ionic->ident.lif.capabilities) & 
IONIC_LIF_CAP_RDMA))
+        return 0;
+
+    ionic_adev = kzalloc(sizeof(*ionic_adev), GFP_KERNEL);
+    if (!ionic_adev)
+        return -ENOMEM;
+
+    aux_dev = &ionic_adev->adev;
+
+    id = ida_alloc_range(&aux_ida, 0, INT_MAX, GFP_KERNEL);

Nitpick: why not just: ida_alloc(&aux_ida, GFP_KERNEL);

sure. Will use ida_alloc().

quoted

+    if (id < 0) {
+        dev_err(lif->ionic->dev, "Failed to allocate aux id: %d\n",
+            id);
+        err = id;
+        goto err_adev_free;
+    }
+
+    aux_dev->id = id;
+    aux_dev->name = "rdma";
+    aux_dev->dev.parent = &lif->ionic->pdev->dev;
+    aux_dev->dev.release = ionic_auxbus_release;
+    ionic_adev->lif = lif;
+    err = auxiliary_device_init(aux_dev);
+    if (err) {
+        dev_err(lif->ionic->dev, "Failed to initialize %s aux 
device: %d\n",
+            aux_dev->name, err);
+        goto err_ida_free;
+    }
+
+    err = auxiliary_device_add(aux_dev);
+    if (err) {
+        dev_err(lif->ionic->dev, "Failed to add %s aux device: %d\n",
+            aux_dev->name, err);
+        goto err_aux_uninit;
+    }
+
+    lif->ionic_adev = ionic_adev;
+
+    return 0;
+
+err_aux_uninit:
+    auxiliary_device_uninit(aux_dev);

I think a return err; is missing here, because, IMOH, 
auxiliary_device_uninit() will call put_device() that will trigger 
ionic_auxbus_release(). So kfree(ionic_adev) would be called twice.

I also think that ida_free() should also be ionic_auxbus_release() 
(just a guess, not checked in details)

Thanks. I will fix this in next spin.

Abhijit

quoted

+err_ida_free:
+    ida_free(&aux_ida, id);
+err_adev_free:
+    kfree(ionic_adev);
+
+    return err;
+}
+
+void ionic_auxbus_unregister(struct ionic_lif *lif)
+{
+    struct auxiliary_device *aux_dev;
+    int id;
+
+    mutex_lock(&lif->adev_lock);
+    if (!lif->ionic_adev)
+        goto out;
+
+    aux_dev = &lif->ionic_adev->adev;
+    id = aux_dev->id;
+
+    auxiliary_device_delete(aux_dev);
+    auxiliary_device_uninit(aux_dev);
+    ida_free(&aux_ida, id);
+
+    lif->ionic_adev = NULL;
+
+out:
+    mutex_unlock(&lif->adev_lock);
+}

...

CJ