[Xorp-hackers] XORP enhancement for wireless mesh network routing

[Jiangxin Hu]

I am new to XORP and routing protocols so there may be some mis-understand
on handle routes. 

See my comments below.

Jiangxin

-----Original Message-----
From: Igor Maravic [mailto:igorm at etf.rs] 
Sent: Friday, June 01, 2012 3:18 PM
To: Ben Greear
Cc: Jiangxin Hu; xorp-hackers at icir.org
Subject: Re: [Xorp-hackers] XORP enhancement for wireless mesh network
routing

> First, I believe it would be possible to want this feature enabled on 
> non-wireless interfaces, so maybe instead of having a 'wireless' 
> attribute, we could call it something like 'allow-disconnected-routes' or
something like that.
>

Agreed++


@Jiangxin I have few comments on the patch:

1. Style and indetitation should be fixed - consult
https://github.com/greearb/xorp.ct/blob/master/xorp/devnotes/coding-style.tx
t

This is a work related task so I did it in hurry. Will check the style.

2.
---
***************
*** 843,848 ****
--- 844,857 ----
  		debug_msg("**directly connected route found for nexthop\n");
  		break;
  	    }
+ 	    int    family = re->net().af();
+ 	    IPNextHop<A> *np = (IPNextHop<A> *)re->nexthop();
+ 	    if (IPvX::ZERO(family) == np->addr() ||
+ 		(re->net()).contains(np->addr())) {
+ 		tryWireless = true;
+ //XLOG_INFO("**** find wireless route %s  %s  %s    for         %s
%s",
vif->str().c_str(),(re->net()).str().c_str(),(np->addr()).str().c_str(),net.
str().c_str(),
nexthop_addr.str().c_str());
+ 		break;
+ 	    }
  	}

  	//
---
Instead of using IPvX::ZERO(family) You can use A::ZERO(). Rib is
templatized after all.

I am not very faimily with XORP, I just copy IPvX::ZERO(family) from other
place and it works. That's the best I can do at that time.

When You don't something You should delete it, not comment it out.

Also I didn't quite understand what do You wand to do here:

+ 	    if (IPvX::ZERO(family) == np->addr() ||
+ 		(re->net()).contains(np->addr())) {
+ 		tryWireless = true;

Why is this a special case? Didn't you say the problem is when the nexthop
route isn't from the same subnet as vif? I think that it's quite ok that
route have the nexthop isn't from it's subnet, if this isn't a direct
nexthop.

The problem is the nexthop is a direct nexthop but not from it's subnet.
Here is the guess if such case happens, we need to check wheter it is a
route for a wireless interface. The actual check is not here.



3.
---
--- 865,916 ----
  	break;
      } while (false);

!     if (vif == NULL && !tryWireless) {
  	debug_msg("**not directly connected route found for nexthop\n");
  	//
  	// XXX: If the route came from an IGP, then we must have
  	// a directly-connected interface toward the next-hop router.
  	//
  	if (protocol->protocol_type() == IGP) { !
!             if (tryWireless || net.contains(nexthop_addr)) {

----> You wouldn't be here if tryWireless was true, why do you check it
again?

Right, I forget reomve it.

!             // need find correct vif
! 		tryWireless = true;
!             }
!
! 	    else {
! 	        XLOG_ERROR("Attempting to add IGP route to table \"%s\" "
  		       "(prefix %s next-hop %s): no directly connected "
  		       "interface toward the next-hop router",
  		       tablename.c_str(), net.str().c_str(),
  		       nexthop_addr.str().c_str());
! 	        return XORP_ERROR;
! 	    }
  	}
      }

      if (vif != NULL) {
  	nexthop = find_or_create_peer_nexthop(nexthop_addr);
      } else {
! 	if (tryWireless) {
! 	    map<string, RibVif*>::iterator iter;
!
! 	    for (iter = _vifs.begin(); iter != _vifs.end(); ++iter) {
!         	vif = iter->second;
!         	if (! vif->is_underlying_vif_up())
!             	    continue;           // XXX: ignore vifs that are not up
! 		if (_rib_manager.is_wireless_if(vif->ifname()))
! 		    break;;

--> double ;
It is not double, it try to find a wireless vif for the route if addroute()
does not specify a vifname.

! 	    }
! 	    nexthop = find_or_create_peer_nexthop(nexthop_addr);

--> what if you can find the "wireless" route and you had to make
external nexthop?

It will lead to an error just like original XORP, the error will be "no
direct connected interface ..." I guess.

! 	}
! 	else
! 	    nexthop = find_or_create_external_nexthop(nexthop_addr);
      }
---

BR
Igor