Re: [PATCH] Expire sendonly joins (was Re: [PATCH rdma-rc 0/2] Add mechanism for ipoib neigh state change notifications)

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2472 bytes --]

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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]