On Tue, Jun 16, 2009 at 10:27:33AM -0700, Sean Hefty wrote:
> >+
> >+ w->member->multicast.rec.qkey = cpu_to_be32(0xc2c);
>
> How can a user control this? An app needs the same qkey for unicast traffic.
In RDMAoE, the qkey has a fixed well-known value, which will be
returned both by multicast and path queries.
>
> >+ atomic_inc(&w->member->refcount);
>
> This needs to be moved below...
I don't follow you - please explain.
> >+static struct ib_sa_multicast *
> >+eth_join_multicast(struct ib_sa_client *client,
> >+ struct ib_device *device, u8 port_num,
> >+ struct ib_sa_mcmember_rec *rec,
> >+ ib_sa_comp_mask comp_mask, gfp_t gfp_mask,
> >+ int (*callback)(int status,
> >+ struct ib_sa_multicast *multicast),
> >+ void *context)
> >+{
> >+ struct mcast_device *dev;
> >+ struct eth_work *w;
> >+ struct mcast_member *member;
> >+ int err;
> >+
> >+ dev = ib_get_client_data(device, &mcast_client);
> >+ if (!dev)
> >+ return ERR_PTR(-ENODEV);
> >+
> >+ member = kzalloc(sizeof *member, gfp_mask);
> >+ if (!member)
> >+ return ERR_PTR(-ENOMEM);
> >+
> >+ w = kzalloc(sizeof *w, gfp_mask);
> >+ if (!w) {
> >+ err = -ENOMEM;
> >+ goto out1;
> >+ }
> >+ w->member = member;
> >+ w->device = device;
> >+ w->port_num = port_num;
> >+
> >+ member->multicast.context = context;
> >+ member->multicast.callback = callback;
> >+ member->client = client;
> >+ member->multicast.rec.mgid = rec->mgid;
> >+ init_completion(&member->comp);
> >+ atomic_set(&member->refcount, 1);
> >+
> >+ ib_sa_client_get(client);
> >+ INIT_WORK(&w->work, eth_mcast_work_handler);
> >+ queue_work(mcast_wq, &w->work);
> >+
> >+ return &member->multicast;
>
> The user could leave/destroy the multicast join request before the queued work
> item runs. We need to hold an additional reference on the member until the
> work
> item completes.
Yes, thanks for catching this. I'll fix and resend.
>
> There's substantial differences in functionality between an IB multicast group
> and the Ethernet group. I would rather see the differences hidden by the
> rdma_cm, than the IB SA module.
>
This is a question of transparency. The motivation is to enable
non-cma apps that use the ib_sa_join_multicast() API to work without
changes.
> >diff --git a/drivers/infiniband/core/sa_query.c
> >b/drivers/infiniband/core/sa_query.c
> >index 1865049..7bf9b5c 100644
> >--- a/drivers/infiniband/core/sa_query.c
> >+++ b/drivers/infiniband/core/sa_query.c
> >@@ -45,6 +45,7 @@
>
> There's not a userspace interface into the sa_query module. How will those
> apps
> work, or apps that send MADs on QPs other than QP1?
Currently, these apps won't work. Sending MADs directly on QPs other
than QP1 will not work.
However, we expect that a userpsace interface to the sa_query module
will be implemented in the future, which will forward queries to
the kernel module.
Naturally, most kernel ULPs and user-level apps will use these
standard interfaces instead of implementing MAD queries themselves.
_______________________________________________
ewg mailing list
ewg@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
