[ofa-general] Re: [PATCH RFC v2] IB/ipoib: enable IGMP for userpsace multicast IB apps

[Or Gerlitz]

On 9/26/07, Roland Dreier <rdreier at cisco.com> wrote:
>
> > To support this inter-op for the case where the receiving party resides
> at
> > the IB side, there is a need to handle IGMP (reports/queries) else the
> local
> > IP router would not forward multicast traffic towards the IB network.
> >
> > This patch does a lookup on the database used for multicast reference
> counting and
> > enhances IPoIB to ignore mulicast group which is already handled by user
> space, all
> > this under a per device policy flag. That is when the policy flag allows
> it, IPoIB
> > will not join and attach its QP to a multicast group which has an entry
> on the database.
>
> I don't really follow this explanation.  OK, I see in the first
> paragraph that you want to handle IGMP.  How does the second paragraph
> follow?  Why does IGMP mean the kernel IPoIB interface should avoid
> joining certain multicast groups?  (Which groups?)


The user space app first joins to the multicast group through the rdma-cm
(by calling rdma_join_multicast etc)  and then lets the kernel IGMP state
machine that it has to join / respond on queries for this group.

This can be achieved if, second, the app issues a  SOL_IP /
IP_ADD_MEMBERSHIP setsockopt call. Since this setsockopt has two impcast A)
IGMP etc B) IPoIB set_multicast_list is called, the patch comes to avoid
IPoIB from joining / attaching to this group, since the app actually
attaches its own UD QP to the group.

So my change log comment wasn't detailed enough to make it clear this is the
design, sorry.


> > +                    /* ignore group which is directly joined by user
> space */
> > +                    if (test_bit(IPOIB_FLAG_ADMIN_UMCAST_ALLOWED,
> &priv->flags) &&
> > +                        !ib_sa_get_mcmember_rec(priv->ca, priv->port,
> &mgid, &rec))
>
> I don't follow this.  Why does ib_sa_get_mcmember_rec() returning 0
> imply that userspace has already joined the multicast group?


Since both the rdma-cm and ipoib are consumers of the core mutlicast
management code (core/multicast.c which is linked into ib_sa.ko), and the
app (through the rdma-cm) --first--  inserts a record into the database and
only then issues the setsockopt call, if ipoib has a hit on a group it was
told to join, this group must be offloaded by the rdma-cm consumer.


> > +module_param_named(umcast_allowed, ipoib_umcast_allowed, int, 0444);
>
> Not sure I understand why you added the module parameter...


The per device flag is initialized by the module param value at
ipoib_dev_init()

> +static DEVICE_ATTR(umcast, S_IWUSR | S_IRUGO, show_umcast, set_umcast);
>
> The set_umcast attribute is writable by root anyway so why are there
> two ways of setting this?


I am not sure to fully follow your comment. I just wanted to make the sysfs
/sys/class/net/$dev/umcast entry writable and I actually did copy-paste from
the set_mode code...

> +    if (!strcmp(buf, "1\n")) {
>
> I don't think this is the most robust way of parsing things.  for
> example it will break in a very confusing way if someone uses "echo -n"
> Could you use simple_strtoul() or something like that to handle
> leading/trailing whitespace etc?


sure, I will fix it.

Or.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20070927/1e8fde71/attachment.html>