On Wed, Aug 31, 2011 at 04:37:49PM +0200, Tony Sarendal wrote:
> On Wed, Aug 31, 2011 at 4:24 PM, Josh Hoppes <josh.hop...@gmail.com> wrote:
> 
> > Why are you using "set nexthop self" and then trying to change that
> > with the filter "allow quick to 172.29.1.52 set nexthop 172.29.1.200".
> > If you don't want your nexthop to be yourself don't tell bgpd to do
> > that.
> >
> >
> To show a bug in bgpctl/bgpd (or where ever it may be).
> Dont you want to be able to trust the information bgpctl gives you ?
> 

Yes there is a bug in bgpd. The problem is that set nexthop self was
sticky and overriding the other set nexthop that came after.

The following diff should solve those issues.
-- 
:wq Claudio

Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.138
diff -u -p -r1.138 rde.h
--- rde.h       18 Nov 2010 12:18:31 -0000      1.138
+++ rde.h       16 Sep 2011 12:29:21 -0000
@@ -163,6 +163,7 @@ LIST_HEAD(prefix_head, prefix);
 #define        F_NEXTHOP_REJECT        0x02000
 #define        F_NEXTHOP_BLACKHOLE     0x04000
 #define        F_NEXTHOP_NOMODIFY      0x08000
+#define        F_NEXTHOP_MASK          0x0f000
 #define        F_ATTR_PARSE_ERR        0x10000
 #define        F_ATTR_LINKED           0x20000
 
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
retrieving revision 1.128
diff -u -p -r1.128 rde_rib.c
--- rde_rib.c   14 Jan 2011 20:07:00 -0000      1.128
+++ rde_rib.c   16 Sep 2011 12:29:21 -0000
@@ -1141,31 +1141,34 @@ nexthop_modify(struct rde_aspath *asp, s
 {
        struct nexthop  *nh;
 
-       if (type == ACTION_SET_NEXTHOP_REJECT) {
-               asp->flags |= F_NEXTHOP_REJECT;
+       if (type == ACTION_SET_NEXTHOP && aid != nexthop->aid)
                return;
-       }
-       if (type  == ACTION_SET_NEXTHOP_BLACKHOLE) {
+
+       asp->flags &= ~F_NEXTHOP_MASK;
+       switch (type) {
+       case ACTION_SET_NEXTHOP_REJECT:
+               asp->flags |= F_NEXTHOP_REJECT;
+               break;
+       case ACTION_SET_NEXTHOP_BLACKHOLE:
                asp->flags |= F_NEXTHOP_BLACKHOLE;
-               return;
-       }
-       if (type == ACTION_SET_NEXTHOP_NOMODIFY) {
+               break;
+       case ACTION_SET_NEXTHOP_NOMODIFY:
                asp->flags |= F_NEXTHOP_NOMODIFY;
-               return;
-       }
-       if (type == ACTION_SET_NEXTHOP_SELF) {
+               break;
+       case ACTION_SET_NEXTHOP_SELF:
                asp->flags |= F_NEXTHOP_SELF;
-               return;
+               break;
+       case ACTION_SET_NEXTHOP:
+               nh = nexthop_get(nexthop);
+               if (asp->flags & F_ATTR_LINKED)
+                       nexthop_unlink(asp);
+               asp->nexthop = nh;
+               if (asp->flags & F_ATTR_LINKED)
+                       nexthop_link(asp);
+               break;
+       default:
+               break;
        }
-       if (aid != nexthop->aid)
-               return;
-
-       nh = nexthop_get(nexthop);
-       if (asp->flags & F_ATTR_LINKED)
-               nexthop_unlink(asp);
-       asp->nexthop = nh;
-       if (asp->flags & F_ATTR_LINKED)
-               nexthop_link(asp);
 }
 
 void

Reply via email to