On 9/26/07, <b class="gmail_sendername">Roland Dreier</b> <<a href="mailto:rdreier@cisco.com">rdreier@cisco.com</a>> wrote:<div><span class="gmail_quote"></span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
 > To support this inter-op for the case where the receiving party resides at<br> > the IB side, there is a need to handle IGMP (reports/queries) else the local<br> > IP router would not forward multicast traffic towards the IB network.
<br> ><br> > This patch does a lookup on the database used for multicast reference counting and<br> > enhances IPoIB to ignore mulicast group which is already handled by user space, all<br> > this under a per device policy flag. That is when the policy flag allows it, IPoIB
<br> > will not join and attach its QP to a multicast group which has an entry on the database.<br><br>I don't really follow this explanation.  OK, I see in the first<br>paragraph that you want to handle IGMP.  How does the second paragraph
<br>follow?  Why does IGMP mean the kernel IPoIB interface should avoid<br>joining certain multicast groups?  (Which groups?)</blockquote><div><br>
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. <br>
<br>
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.<br>
<br>
So my change log comment wasn't detailed enough to make it clear this is the design, sorry.<br>
<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <br>
>
+                    /*
ignore group which is directly joined by user space */<br> >
+                    if
(test_bit(IPOIB_FLAG_ADMIN_UMCAST_ALLOWED, &priv->flags) &&<br>
>
+                        !ib_sa_get_mcmember_rec(priv->ca, priv->port, &mgid, &rec))<br><br>I don't follow this.  Why does ib_sa_get_mcmember_rec() returning 0<br>imply that userspace has already joined the multicast group?
</blockquote><div><br>
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.<br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> > +module_param_named(umcast_allowed, ipoib_umcast_allowed, int, 0444);<br><br>Not sure I understand why you added the module parameter...
</blockquote><div><br>
The per device flag is initialized by the module param value at ipoib_dev_init()<br>
</div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> > +static DEVICE_ATTR(umcast, S_IWUSR | S_IRUGO, show_umcast, set_umcast);<br>
<br>The set_umcast attribute is writable by root anyway so why are there<br>two ways of setting this?</blockquote><div><br>
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...<br>
</div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> > +    if (!strcmp(buf, "1\n")) {<br><br>I don't think this is the most robust way of parsing things.  for
<br>example it will break in a very confusing way if someone uses "echo -n"<br>Could you use simple_strtoul() or something like that to handle<br>leading/trailing whitespace etc?</blockquote><div><br>
sure, I will fix it. <br>
</div></div><br>
Or.<br>