[openib-general] Re: [PATCH] SDP: In do_link_path_lookup, make sure device is IPoIB hardware type

[Michael S. Tsirkin]

Quoting r. Hal Rosenstock <halr at voltaire.com>:
> Subject: Re: [PATCH] SDP: In do_link_path_lookup, make sure device is IPoIB hardware type
> 
> On Tue, 2005-09-27 at 07:35, Michael S. Tsirkin wrote:
> > Quoting r. Hal Rosenstock <halr at voltaire.com>:
> > > Subject: [PATCH] SDP: In do_link_path_lookup, make sure device is
> IPoIB hardware type
> > > 
> > > In do_link_path_lookup, make sure device selected is IPoIB hardware
> > > type before accessing its local private data
> > 
> > 
> > Hal, does this fix some actual problem you are seeing?
> 
> Yes, I saw it with AT. It has the same exact code in terms of this.
> Try an ethernet IP address as a destination and see what happens.
> 
> > sdp_link.c has
> > 
> >         if (rt->u.dst.neighbour->dev->type != ARPHRD_INFINIBAND &&
> >             !(rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK)) {
> >                 result = -ENETUNREACH;
> >                 goto error;
> >         }
> > 
> > and this seems to check the device type.
> 
> but that's before the device might be changed.
> 
> -- Hal
> 

Hmm, I see the problem. So the fix is just to move it later:
I think the right thing to do is to error out on non-ipoib non-loopback device.
Something like the following (untested).


Index: drivers/infiniband/ulp/sdp/sdp_link.c
===================================================================
--- drivers/infiniband/ulp/sdp/sdp_link.c	(revision 3535)
+++ drivers/infiniband/ulp/sdp/sdp_link.c	(working copy)
@@ -394,9 +394,7 @@ static void do_link_path_lookup(struct s
 		result = -ENETUNREACH;
 		goto error;
 	}
-	/*
-	 * check that device is IPoIB
-	 */
+
 	if (!rt->u.dst.neighbour || !rt->u.dst.neighbour->dev) {
 		sdp_dbg_warn(NULL, "No neighbour found for <%08x:%08x>",
 			     rt->rt_src, rt->rt_dst);
@@ -404,15 +402,6 @@ static void do_link_path_lookup(struct s
 		result = -ENETUNREACH;
 		goto error;
 	}
-	/*
-	 * check for IB device or loopback, the later requires extra
-	 * handling.
-	 */
-	if (rt->u.dst.neighbour->dev->type != ARPHRD_INFINIBAND &&
-	    !(rt->u.dst.neighbour->dev->flags & IFF_LOOPBACK)) {
-		result = -ENETUNREACH;
-		goto error;
-	}
 
 	sdp_dbg_data(NULL, "Found dev <%s>. <%08x:%08x:%08x> state <%02x>",
 		     rt->u.dst.neighbour->dev->name,
@@ -430,6 +419,15 @@ static void do_link_path_lookup(struct s
 		dev_hold(dev);
 	}
 
+	/*
+	 * check for IB device or loopback, the later requires extra
+	 * handling.
+	 */
+	if (dev->type != ARPHRD_INFINIBAND && !(dev->flags & IFF_LOOPBACK)) {
+		result = -ENETUNREACH;
+		goto error;
+	}
+
 	info->gw  = rt->rt_gateway;
 	info->src = rt->rt_src;     /* true source IP address */
 

-- 
MST