[openib-general] [PATCHv2] IB/ipoib: Fix ipoib handling for pkey reordering

[Michael S. Tsirkin]

I just gave this a cursory glance.
A suggestion: would it not be much simpler to modify the QP from RTS to RTS on pkey
change?

> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> index f2aa923..b0287c1 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> @@ -415,21 +415,22 @@ int ipoib_ib_dev_open(struct net_device 
>  
>  	ret = ipoib_init_qp(dev);
>  	if (ret) {
> -		ipoib_warn(priv, "ipoib_init_qp returned %d\n", ret);
> +		if (ret != -ENOENT)
> +			ipoib_warn(priv, "ipoib_init_qp returned %d\n", ret);
>  		return -1;
>  	}
 
What's the reason for this?

> @@ -993,6 +993,7 @@ static void ipoib_setup(struct net_devic
>  	INIT_DELAYED_WORK(&priv->pkey_task,    ipoib_pkey_poll);
>  	INIT_DELAYED_WORK(&priv->mcast_task,   ipoib_mcast_join_task);
>  	INIT_WORK(&priv->flush_task,   ipoib_ib_dev_flush);
> +	INIT_WORK(&priv->flush_restart_qp_task, ipoib_ib_dev_flush_restart_qp);
>  	INIT_WORK(&priv->restart_task, ipoib_mcast_restart_task);
>  	INIT_DELAYED_WORK(&priv->ah_reap_task, ipoib_reap_ah);
>  }

Shorter name?

> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> index b303ce6..27d6fd4 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> @@ -232,9 +232,10 @@ static int ipoib_mcast_join_finish(struc
>  		ret = ipoib_mcast_attach(dev, be16_to_cpu(mcast->mcmember.mlid),
>  					 &mcast->mcmember.mgid);
>  		if (ret < 0) {
> -			ipoib_warn(priv, "couldn't attach QP to multicast group "
> -				   IPOIB_GID_FMT "\n",
> -				   IPOIB_GID_ARG(mcast->mcmember.mgid));
> +			if (ret != -ENXIO) /* No PKEY found */
> +				ipoib_warn(priv, "couldn't attach QP to multicast group "
> +					   IPOIB_GID_FMT "\n",
> +					   IPOIB_GID_ARG(mcast->mcmember.mgid));
>  
>  			clear_bit(IPOIB_MCAST_FLAG_ATTACHED, &mcast->flags);
>  			return ret;
> @@ -312,7 +313,7 @@ ipoib_mcast_sendonly_join_complete(int s
>  		status = ipoib_mcast_join_finish(mcast, &multicast->rec);
>  
>  	if (status) {
> -		if (mcast->logcount++ < 20)
> +		if (mcast->logcount++ < 20 && status != -ENXIO)
>  			ipoib_dbg_mcast(netdev_priv(dev), "multicast join failed for "
>  					IPOIB_GID_FMT ", status %d\n",
>  					IPOIB_GID_ARG(mcast->mcmember.mgid), status);
> @@ -416,7 +417,7 @@ static int ipoib_mcast_join_complete(int
>  					", status %d\n",
>  					IPOIB_GID_ARG(mcast->mcmember.mgid),
>  					status);
> -		} else {
> +		} else if (status != -ENXIO) {
>  			ipoib_warn(priv, "multicast join failed for "
>  				   IPOIB_GID_FMT ", status %d\n",
>  				   IPOIB_GID_ARG(mcast->mcmember.mgid),
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> index 3cb551b..d0384ea 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> @@ -52,8 +52,10 @@ int ipoib_mcast_attach(struct net_device
>  	if (ib_find_cached_pkey(priv->ca, priv->port, priv->pkey, &pkey_index)) {
>  		clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
>  		ret = -ENXIO;
> +		ipoib_dbg(priv, "PKEY %X not found\n", priv->pkey);
>  		goto out;
>  	}
> +	ipoib_dbg(priv, "PKEY %X found at index %d\n", priv->pkey, pkey_index);
>  	set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
>  
>  	/* set correct QKey for QP */

Make it PKey or pkey: no text in uppercase in log messages please.

> @@ -105,9 +107,11 @@ int ipoib_init_qp(struct net_device *dev
>  	 */
>  	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey, &pkey_index);
>  	if (ret) {
> +		ipoib_dbg(priv, "PKEY %X not found.\n", priv->pkey);
>  		clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
>  		return ret;
>  	}
> +	ipoib_dbg(priv, "PKEY %X found at index %d.\n", priv->pkey, pkey_index);
>  	set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
>  
>  	qp_attr.qp_state = IB_QPS_INIT;

going a bit overboard on the number of debug messages here.

> @@ -260,12 +264,14 @@ void ipoib_event(struct ib_event_handler
>  		container_of(handler, struct ipoib_dev_priv, event_handler);
>  
>  	if (record->event == IB_EVENT_PORT_ERR    ||
> -	    record->event == IB_EVENT_PKEY_CHANGE ||
>  	    record->event == IB_EVENT_PORT_ACTIVE ||
>  	    record->event == IB_EVENT_LID_CHANGE  ||
>  	    record->event == IB_EVENT_SM_CHANGE   ||
>  	    record->event == IB_EVENT_CLIENT_REREGISTER) {
>  		ipoib_dbg(priv, "Port state change event\n");
>  		queue_work(ipoib_workqueue, &priv->flush_task);
> +	} else if (record->event == IB_EVENT_PKEY_CHANGE) {
> +		ipoib_dbg(priv, "PKEY change event\n");
> +		queue_work(ipoib_workqueue, &priv->flush_restart_qp_task);
>  	}
>  }


-- 
MST