[ofa-general] PATCH] IB/core: ignore membership bit when looking for a P_Key in the table

[Or Gerlitz]

Roland Dreier wrote:
> Looks OK I guess.  But it seems that we should fix up the code in
> sa_query.c too, right?

Hi Roland,

(getting back to you with a little delay) I am not sure to fully follow 
your question. I can think of two questions here:

A) what PKEY the ib_sa / ib_mad modules for the path query MAD

B) what PKEY is being places in the path record used for the query.

my guess is that you are asking on B), but to clear up things, if you 
ask on A), I think that for all the mad sent to the SA/SM the "default 
IB pkey" is used (that is 0xffff or 0x7fff), so this is not relevant to 
the --ulp-- code, correct?

now, as for B) : my understanding of the IPoIB design/code wrt partial 
membership in a partition, is that the driver "normalizes" each pkey it 
uses to be seen as the full membership instance for this partition, 
namely before a pkey is used it is being blindly OR-ed with 0x8000, and 
then the driver never cares if it was a full or partial one, that's it.

This design/code makes much sense to me, also it was live, kicking and 
operating fine till the patch that moved it to use ib_find_pkey()...

Specifically re the SA interaction of ipoib, as of this design, the SA 
is always being asked on the full membership pkey. So, for your question 
I don't see what need to be changed in sa_query.c, please clarify.

I also have pasted below the relevant code from ipoib_main.c

Or.

  > +static ssize_t create_child(struct device *dev,
> +                           struct device_attribute *attr,
> +                           const char *buf, size_t count)
> +{
> +       int pkey;
> +       int ret;
> +
> +       if (sscanf(buf, "%i", &pkey) != 1)
> +               return -EINVAL;
> +
> +       if (pkey < 0 || pkey > 0xffff)
> +               return -EINVAL;
> +
> +       /*
> +        * Set the full membership bit, so that we join the right
> +        * broadcast group, etc.
> +        */
> +       pkey |= 0x8000;
> +
> +       ret = ipoib_vlan_add(to_net_dev(dev), pkey);
> +
> +       return ret ? ret : count;
> +}
> 
> ......
> 
> +static struct net_device *ipoib_add_port(const char *format,
> +					 struct ib_device *hca, u8 port)
> +{
> +	struct ipoib_dev_priv *priv;
> +	int result = -ENOMEM;
> +
> +	priv = ipoib_intf_alloc(format);
> +	if (!priv)
> +		goto alloc_mem_failed;
> +
> +	SET_NETDEV_DEV(priv->dev, hca->dma_device);
> +
> +	result = ib_query_pkey(hca, port, 0, &priv->pkey);
> +	if (result) {
> +		printk(KERN_WARNING "%s: ib_query_pkey port %d failed (ret = %d)\n",
> +		       hca->name, port, result);
> +		goto alloc_mem_failed;
> +	}
> +
> +	/*
> +	 * Set the full membership bit, so that we join the right
> +	 * broadcast group, etc.
> +	 */
> +	priv->pkey |= 0x8000;
> +
> +	priv->dev->broadcast[8] = priv->pkey >> 8;
> +	priv->dev->broadcast[9] = priv->pkey & 0xff;
> 
> ..........
> 
> 
> +static struct ipoib_path *path_rec_create(struct net_device *dev, void *gid)
> +{
> +	struct ipoib_dev_priv *priv = netdev_priv(dev);
> +	struct ipoib_path *path;
> +
> +	path = kzalloc(sizeof *path, GFP_ATOMIC);
> +	if (!path)
> +		return NULL;
> +
> +	path->dev = dev;
> +
> +	skb_queue_head_init(&path->queue);
> +
> +	INIT_LIST_HEAD(&path->neigh_list);
> +
> +	memcpy(path->pathrec.dgid.raw, gid, sizeof (union ib_gid));
> +	path->pathrec.sgid      = priv->local_gid;
> +	path->pathrec.pkey      = cpu_to_be16(priv->pkey);
> +	path->pathrec.numb_path = 1;
> +
> +	return path;
> +}

use priv->pkey in the PR

> +
> +static int path_rec_start(struct net_device *dev,
> +			  struct ipoib_path *path)
> +{
> +	struct ipoib_dev_priv *priv = netdev_priv(dev);
> +
> +	ipoib_dbg(priv, "Start path record lookup for " IPOIB_GID_FMT "\n",
> +		  IPOIB_GID_ARG(path->pathrec.dgid));
> +
> +	init_completion(&path->done);
> +
> +	path->query_id =
> +		ib_sa_path_rec_get(&ipoib_sa_client, priv->ca, priv->port,
> +				   &path->pathrec,
> +				   IB_SA_PATH_REC_DGID		|
> +				   IB_SA_PATH_REC_SGID		|
> +				   IB_SA_PATH_REC_NUMB_PATH	|
> +				   IB_SA_PATH_REC_PKEY,
> +				   1000, GFP_ATOMIC,
> +				   path_rec_completion,
> +				   path, &path->query);