netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] infiniband: addr: Avoid unnecessary neigh lookup.
@ 2011-12-02 19:55 David Miller
  2011-12-02 23:18 ` Roland Dreier
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2011-12-02 19:55 UTC (permalink / raw)
  To: roland; +Cc: netdev


The explicit neigh_lookup() in addr4_resolve() is unnecessary because this
is the exact same calculation the routing code already made to attach the
neigh to the route.

Therefore, just use dst_get_neighbour().

Signed-off-by: David S. Miller <davem@davemloft.net>
---

Roland, I'm auditing all of the uses of dst_get_neighbour() with the
end result being that a dst_put_neighbour() interface will be added
and all access will be reduced to quick:

	rcu_read_lock();
	n = dst_get_neighbour();
	...
	dst_put_neighbour();
	rcu_read_unlock();

sequences.  And then, at the next step, things will be further
whittled down into a refcount-less variant:

	rcu_read_lock();
	n = dst_get_neighbour_noref();
	...
	rcu_read_unlock();

Another key difference is that we are making the code able to handle
dst_get_neighbour() returning NULL.

So if it's OK with you I'd like to merge this stuff directly in the
net-next tree after getting your ACK on each patch.

In the case here, the neigh_lookup() call is spurious and is exactly
mimicking the neigh lookup that the ipv4 routing code does to attach
the neighbour to the route.  There is no need to duplicate that
operation, and we can thus use the route attached neigh directly.

 drivers/infiniband/core/addr.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index a20c3c8..83d88e1 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -214,20 +214,19 @@ static int addr4_resolve(struct sockaddr_in *src_in,
 		goto put;
 	}
 
-	neigh = neigh_lookup(&arp_tbl, &rt->rt_gateway, rt->dst.dev);
-	if (!neigh || !(neigh->nud_state & NUD_VALID)) {
-		rcu_read_lock();
-		neigh_event_send(dst_get_neighbour(&rt->dst), NULL);
-		rcu_read_unlock();
-		ret = -ENODATA;
-		if (neigh)
-			goto release;
-		goto put;
+	rcu_read_lock();
+	neigh = dst_get_neighbour(&rt->dst);
+	ret = -ENODATA;
+	if (!neigh)
+		goto unlock_put;
+	if (!(neigh->nud_state & NUD_VALID)) {
+		neigh_event_send(neigh, NULL);
+		goto unlock_put;
 	}
 
 	ret = rdma_copy_addr(addr, neigh->dev, neigh->ha);
-release:
-	neigh_release(neigh);
+unlock_put:
+	rcu_read_unlock();
 put:
 	ip_rt_put(rt);
 out:
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] infiniband: addr: Avoid unnecessary neigh lookup.
  2011-12-02 19:55 [PATCH] infiniband: addr: Avoid unnecessary neigh lookup David Miller
@ 2011-12-02 23:18 ` Roland Dreier
  2011-12-03  2:48   ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Roland Dreier @ 2011-12-02 23:18 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

> So if it's OK with you I'd like to merge this stuff directly in the
> net-next tree after getting your ACK on each patch.

Sure, that's fine, and for this particular patch:

Acked-by: Roland Dreier <roland@purestorage.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] infiniband: addr: Avoid unnecessary neigh lookup.
  2011-12-02 23:18 ` Roland Dreier
@ 2011-12-03  2:48   ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2011-12-03  2:48 UTC (permalink / raw)
  To: roland; +Cc: netdev

From: Roland Dreier <roland@kernel.org>
Date: Fri, 2 Dec 2011 15:18:00 -0800

>> So if it's OK with you I'd like to merge this stuff directly in the
>> net-next tree after getting your ACK on each patch.
> 
> Sure, that's fine, and for this particular patch:
> 
> Acked-by: Roland Dreier <roland@purestorage.com>

Thanks for reviewing Roland.

I reread this code and it turns out that the ipv6 handling does
the same exact thing as what I transformed ipv4 to do, so it's
better to have a helper function and make ipv4 and ipv6 use it.

I've sanitized the neighbour handling in the entire Infiniband
stack tonight, and I'll send that series out for review right now.
It will include an updated version of this patch.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-12-03  2:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-02 19:55 [PATCH] infiniband: addr: Avoid unnecessary neigh lookup David Miller
2011-12-02 23:18 ` Roland Dreier
2011-12-03  2:48   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).