bgpd: conditional default-originate using route-map

Incorporate a patch by Svetozar Mihailov which implements
default-originate route-maps to behave as expected, i.e. allowing
the default route to be advertised conditionally, depending on a
criterion given by the route-map.

I am aware that the performance attributes of the following implementation
are far from optimal. However, this affects only code paths belonging to
a feature that is broken without this patch, therefore, it seems reasonable
to me to have this in the mainline for now.

Cc: Svetozar Mihailov <quagga@j.zarhi.com>
Reported-by: Sébastien Cramatte <scramatte@gmail.com>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/NEWS b/NEWS
index d67d66469e3c0296c54d475cbc27934f06196725..fe0d5ad26ef5c20638aa61e44fcd9fd8a3df42c0 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,10 @@
 Note: this file lists major user-visible changes only.
 
+- [bgpd] The semantics of default-originate route-map have changed.
+  The route-map is now used to advertise the default route conditionally.
+  The old behaviour which allowed to set attributes on the originated
+  default route is no longer supported.
+
 * Changes in Quagga 0.99.21
 
 - [bgpd] BGP multipath support has been merged

diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index 0e56d368665b098fea67eca519742dfb9a1a8c50..d469236651a81b687503d87e356ed338325b4fb7 100644 (file)
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -506,6 +506,16 @@ bgp_scan (afi_t afi, safi_t safi)
       else if (afi == AFI_IP6)
        zlog_debug ("scanning IPv6 Unicast routing tables");
     }
+
+  /* Reevaluate default-originate route-maps and announce/withdraw
+   * default route if neccesary. */
+  for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer))
+    {
+      if (peer->status == Established
+         && CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_DEFAULT_ORIGINATE)
+         && peer->default_rmap[afi][safi].name)
+       bgp_default_originate (peer, afi, safi, 0);
+    }
 }
 
 /* BGP scan thread.  This thread check nexthop reachability. */

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 06bd59916cdd0e5c2cd8b528b8bdaa9a8d2a3095..8bc72d7be36b6692a0202e1609147b4230fc9f4e 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2462,8 +2462,9 @@ bgp_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw)
   struct attr attr;
   struct aspath *aspath;
   struct prefix p;
-  struct bgp_info binfo;
   struct peer *from;
+  struct bgp_node *rn;
+  struct bgp_info *ri;
   int ret = RMAP_DENYMATCH;
   
   if (!(afi == AFI_IP || afi == AFI_IP6))
@@ -2505,21 +2506,37 @@ bgp_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw)
 
   if (peer->default_rmap[afi][safi].name)
     {
-      binfo.peer = bgp->peer_self;
-      binfo.attr = &attr;
-
       SET_FLAG (bgp->peer_self->rmap_type, PEER_RMAP_TYPE_DEFAULT);
-
-      ret = route_map_apply (peer->default_rmap[afi][safi].map, &p,
-                            RMAP_BGP, &binfo);
-
+      for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; rn = bgp_route_next(rn))
+        {
+          for (ri = rn->info; ri; ri = ri->next)
+            {
+              struct attr dummy_attr;
+              struct attr_extra dummy_extra;
+              struct bgp_info info;
+
+              /* Provide dummy so the route-map can't modify the attributes */
+              dummy_attr.extra = &dummy_extra;
+              bgp_attr_dup(&dummy_attr, ri->attr);
+              info.peer = ri->peer;
+              info.attr = &dummy_attr;
+
+              ret = route_map_apply(peer->default_rmap[afi][safi].map, &rn->p,
+                                    RMAP_BGP, &info);
+
+              /* The route map might have set attributes. If we don't flush them
+               * here, they will be leaked. */
+              bgp_attr_flush(&dummy_attr);
+              if (ret != RMAP_DENYMATCH)
+                break;
+            }
+          if (ret != RMAP_DENYMATCH)
+            break;
+        }
       bgp->peer_self->rmap_type = 0;
 
       if (ret == RMAP_DENYMATCH)
-       {
-         bgp_attr_flush (&attr);
-         withdraw = 1;
-       }
+        withdraw = 1;
     }
 
   if (withdraw)
@@ -2530,8 +2547,11 @@ bgp_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw)
     }
   else
     {
-      SET_FLAG (peer->af_sflags[afi][safi], PEER_STATUS_DEFAULT_ORIGINATE);
-      bgp_default_update_send (peer, &attr, afi, safi, from);
+      if (! CHECK_FLAG (peer->af_sflags[afi][safi], PEER_STATUS_DEFAULT_ORIGINATE))
+        {
+          SET_FLAG (peer->af_sflags[afi][safi], PEER_STATUS_DEFAULT_ORIGINATE);
+          bgp_default_update_send (peer, &attr, afi, safi, from);
+        }
     }
   
   bgp_attr_extra_free (&attr);