On 09/27/2015 12:39 PM, Christoph Lameter wrote: > > On Sat, 26 Sep 2015, Or Gerlitz wrote: > >> It's possible that this was done for a reason, so > >> sounds good, so taking into account that Erez is away till Oct 6th, we >> can probably pick your patch and later, if Erez proves us that there's >> deep problem there, revert it and take his. > > Ok but if Erez does not have the time to participate in code development > and follow up on the patch as issues arise then I would rather rework the > code so that it is easily understandable and I will continue to follow up > on the issues with the code as they develop. This seems to be much more > important to my company than Mellanox. > Currently I'm testing your patch with a couple other patches. I dropped the patch of mine that added a module option, and added two different patches. However, I'm still waffling on this patch somewhat. In the discussions that Jason and I had, I pretty much decided that I would like to see all send-only multicast sends be sent immediately with no backlog queue. That means that if we had to start a send-only join, or if we started one and it hasn't completed yet, we would send the packet immediately via the broadcast group versus queueing. Doing so might trip this new code up. Right now, we start a join, we queue the packet on the mcast struct, and in join_finish we create an ah, but that's it. We then restart the send by calling dev_queue_xmit on the skb we put in the backlog queue, which takes us back around to mcast_send, where we not have both a mcast and a mcast->ah, so *then* we alloc a new neigh entry, attach this mcast to it, and send using it. If I change mcast_send so that we start a join, but immediately send the packet in the broadcast group, then I would have to change the join_finish routine to alloc a neigh that has the right daddr so it can be found in the future, without the benefit of the daddr passed into the function mcast_send so missing the ipoib header and instead only having the raw mgid in the mcmember struct. But, we would have to have that neigh struct so that the timeout would work in the case were we had a packet or two that triggered a join but were all sent prior to the join completing and so we never got a neigh alloc via mcast_send for this mcast group. -- Doug Ledford <dledford@xxxxxxxxxx> GPG KeyID: 0E572FDD
Attachment:
signature.asc
Description: OpenPGP digital signature