[ofa-general] RE: [RFC V3 PATCH 4/5] rdma/cma: implement RDMA_CM_EVENT_NETDEV_CHANGE notification

[Sean Hefty]

>+static void cma_ndev_work_handler(struct work_struct *_work)
>+{
>+	struct cma_ndev_work *work = container_of(_work, struct cma_ndev_work,
>work);
>+	struct rdma_id_private *id_priv = work->id;
>+	int destroy = 0;
>+
>+	mutex_lock(&id_priv->handler_mutex);
>+
>+	if (id_priv->id.event_handler(&id_priv->id, &work->event)) {

How do we know that the user hasn't tried to destroy the id from another
callback?  We need some sort of state check here.

>+		cma_exch(id_priv, CMA_DESTROYING);
>+		destroy = 1;
>+	}
>+
>+	cma_enable_remove(id_priv);

I didn't see the matching cma_disable_remove() call.

>+	cma_deref_id(id_priv);
>+	if (destroy)
>+		rdma_destroy_id(&id_priv->id);
>+	kfree(work);
>+}
>+
> static int cma_resolve_ib_route(struct rdma_id_private *id_priv, int
>timeout_ms)
> {
> 	struct rdma_route *route = &id_priv->id.route;
>@@ -2726,6 +2752,61 @@ void rdma_leave_multicast(struct rdma_cm
> }
> EXPORT_SYMBOL(rdma_leave_multicast);
>
>+static int cma_netdev_align_id(struct net_device *ndev, struct rdma_id_private
>*id_priv)
>+{

nit - function name isn't clear to me.  Maybe something like
cma_netdev_change_handler()?  Although I'm not sure that netdev change is what
the user is really interested in.  What they really want to know is if IP
address mapping/resolution changed.  netdev is hidden from the user.

>+	struct rdma_dev_addr *dev_addr;
>+	struct cma_ndev_work *work;
>+
>+	dev_addr = &id_priv->id.route.addr.dev_addr;
>+
>+	if (!memcmp(dev_addr->src_dev_name, ndev->name, IFNAMSIZ) &&
>+	  memcmp(dev_addr->src_dev_addr, ndev->dev_addr, ndev->addr_len)) {
>+		printk(KERN_ERR "addr change for device %s used by id %p,
>notifying\n",
>+				ndev->name, &id_priv->id);
>+		work = kzalloc(sizeof *work, GFP_ATOMIC);
>+		if (!work)
>+			return -ENOMEM;
>+		INIT_WORK(&work->work, cma_ndev_work_handler);
>+		work->id = id_priv;
>+		work->event.event = RDMA_CM_EVENT_NETDEV_CHANGE;

Maybe call this RDMA_CM_EVENT_ADDR_CHANGE?

>+		atomic_inc(&id_priv->refcount);
>+		queue_work(cma_wq, &work->work);
>+	}
>+}
>+
>+static int cma_netdev_callback(struct notifier_block *self, unsigned long
>event,
>+	void *ctx)
>+{
>+	struct net_device *ndev = (struct net_device *)ctx;
>+	struct cma_device *cma_dev;
>+	struct rdma_id_private *id_priv;
>+	int ret = NOTIFY_DONE;
>+
>+	if (dev_net(ndev) != &init_net)
>+		return NOTIFY_DONE;
>+
>+	if (event != NETDEV_BONDING_FAILOVER)
>+		return NOTIFY_DONE;
>+
>+	if (!(ndev->flags & IFF_MASTER) || !(ndev->priv_flags & IFF_BONDING))
>+		return NOTIFY_DONE;
>+
>+	mutex_lock(&lock);
>+	list_for_each_entry(cma_dev, &dev_list, list)

It seems like we just need to find the cma_dev that has the current mapping

- Sean