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

Igor Maravić igorm at etf.rs
Fri Jun 1 12:17:55 PDT 2012


> 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.txt
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.

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.

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?


!             // 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 ;


! 	    }
! 	    nexthop = find_or_create_peer_nexthop(nexthop_addr);

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

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

BR
Igor



More information about the Xorp-hackers mailing list