I'd love to be able to do something like the below. DMED doesn't seem like
it's really fixing things, but it does add costs. Alternatively, it could
be made the default, but getting rid of combinatorial loops is more
attractive to me.
Are there any really good reasons to keep DMED?
Paul
-----8<---------8<---------8<---------8<---------8<---------8<----
* Deterministic MED exists, as far as I can tell, primarily to try mitigate
the issues MED introduced in BGP, due to it inducing non-transitive
preferences between pairs of routes - oscillation particularly. D.MED
does not however solve the problems MED can cause, e.g. it doesn't solve
MED oscillation, never mind oscillation generally - which can occur even
without MED being involved (e.g. IGP metrics alone can cause it, in some
cases where BGP doesn't follow IGP topology). So given it doesn't really
solve the MED problems, why pay the cost of D.MED?
1. Admins might enable it (it's there, and deterministic is a good thing,
right?), not realising it can have a significant runtime cost.
2. The existence of the code incurs costs on developers.
Nuke.
* bgp_mpath.{c,h}: (bgp_mp_dmed_deselect) gone.
* bgp_route.c: (bgp_best_selection) The whole combinatorial checking of
routes against each other can go, along with the maintenance of DMED
related flags.
* bgp_route.h: (struct bgp_info) Bye bye BGP_INFO_DMED_{CHECK,SELECTED}
* bgp_vty.c: ({,no}bgp_deterministic_med_cmd) become DEPRECATED and
do-nothings. Warn to vty on the set case.
---
bgpd/bgp_mpath.c | 25 ---------------
bgpd/bgp_mpath.h | 1 -
bgpd/bgp_route.c | 96 ++++----------------------------------------------------
bgpd/bgp_route.h | 12 +++----
bgpd/bgp_vty.c | 31 +++++++-----------
bgpd/bgpd.c | 4 ---
bgpd/bgpd.h | 2 +-
7 files changed, 24 insertions(+), 147 deletions(-)
diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c
index db5996e..5ee023e 100644
--- a/bgpd/bgp_mpath.c
+++ b/bgpd/bgp_mpath.c
@@ -582,31 +582,6 @@ bgp_info_mpath_update (struct bgp_node *rn, struct
bgp_info *new_best,
}
/*
- * bgp_mp_dmed_deselect
- *
- * Clean up multipath information for BGP_INFO_DMED_SELECTED path that
- * is not selected as best path
- */
-void
-bgp_mp_dmed_deselect (struct bgp_info *dmed_best)
-{
- struct bgp_info *mpinfo, *mpnext;
-
- if (!dmed_best)
- return;
-
- for (mpinfo = bgp_info_mpath_first (dmed_best); mpinfo; mpinfo = mpnext)
- {
- mpnext = bgp_info_mpath_next (mpinfo);
- bgp_info_mpath_dequeue (mpinfo);
- }
-
- bgp_info_mpath_count_set (dmed_best, 0);
- UNSET_FLAG (dmed_best->flags, BGP_INFO_MULTIPATH_CHG);
- assert (bgp_info_mpath_first (dmed_best) == 0);
-}
-
-/*
* bgp_info_mpath_aggregate_update
*
* Set the multipath aggregate attribute. We need to see if the
diff --git a/bgpd/bgp_mpath.h b/bgpd/bgp_mpath.h
index 2a84d5e..ba4e2c5 100644
--- a/bgpd/bgp_mpath.h
+++ b/bgpd/bgp_mpath.h
@@ -60,7 +60,6 @@ bool bgp_mpath_is_configured (struct bgp *, afi_t, safi_t);
extern void bgp_mp_list_init (struct list *);
extern void bgp_mp_list_clear (struct list *);
extern void bgp_mp_list_add (struct list *, struct bgp_info *);
-extern void bgp_mp_dmed_deselect (struct bgp_info *);
extern void bgp_info_mpath_update (struct bgp_node *, struct bgp_info *,
struct bgp_info *, struct list *,
afi_t, safi_t);
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 1a49cb3..604ebac 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1322,77 +1322,13 @@ bgp_best_selection (struct bgp *bgp, struct bgp_node
*rn,
struct bgp_info *new_select;
struct bgp_info *old_select;
struct bgp_info *ri;
- struct bgp_info *ri1;
- struct bgp_info *ri2;
struct bgp_info *nextri = NULL;
int cmpret, do_mpath;
struct list mp_list;
bgp_mp_list_init (&mp_list);
do_mpath = bgp_mpath_is_configured (bgp, afi, safi);
-
- /* bgp deterministic-med */
- new_select = NULL;
- if (bgp_flag_check (bgp, BGP_FLAG_DETERMINISTIC_MED))
- for (ri1 = rn->info; ri1; ri1 = ri1->next)
- {
- if (CHECK_FLAG (ri1->flags, BGP_INFO_DMED_CHECK))
- continue;
- if (BGP_INFO_HOLDDOWN (ri1))
- continue;
- if (ri1->peer && ri1->peer != bgp->peer_self)
- if (ri1->peer->status != Established)
- continue;
-
- new_select = ri1;
- if (do_mpath)
- bgp_mp_list_add (&mp_list, ri1);
- old_select = CHECK_FLAG (ri1->flags, BGP_INFO_SELECTED) ? ri1 : NULL;
- if (ri1->next)
- for (ri2 = ri1->next; ri2; ri2 = ri2->next)
- {
- if (CHECK_FLAG (ri2->flags, BGP_INFO_DMED_CHECK))
- continue;
- if (BGP_INFO_HOLDDOWN (ri2))
- continue;
- if (ri2->peer &&
- ri2->peer != bgp->peer_self &&
- !CHECK_FLAG (ri2->peer->sflags, PEER_STATUS_NSF_WAIT))
- if (ri2->peer->status != Established)
- continue;
-
- if (aspath_cmp_left (ri1->attr->aspath, ri2->attr->aspath)
- || aspath_cmp_left_confed (ri1->attr->aspath,
- ri2->attr->aspath))
- {
- if (CHECK_FLAG (ri2->flags, BGP_INFO_SELECTED))
- old_select = ri2;
- if ((cmpret = bgp_info_cmp (bgp, ri2, new_select, afi, safi))
- == -1)
- {
- bgp_info_unset_flag (rn, new_select,
BGP_INFO_DMED_SELECTED);
- new_select = ri2;
- }
-
- if (do_mpath)
- {
- if (cmpret != 0)
- bgp_mp_list_clear (&mp_list);
-
- if (cmpret == 0 || cmpret == -1)
- bgp_mp_list_add (&mp_list, ri);
- }
-
- bgp_info_set_flag (rn, ri2, BGP_INFO_DMED_CHECK);
- }
- }
- bgp_info_set_flag (rn, new_select, BGP_INFO_DMED_CHECK);
- bgp_info_set_flag (rn, new_select, BGP_INFO_DMED_SELECTED);
-
- bgp_info_mpath_update (rn, new_select, old_select, &mp_list, afi, safi);
- bgp_mp_list_clear (&mp_list);
- }
-
+
/* Check old selected route and new selected route. */
old_select = NULL;
new_select = NULL;
@@ -1418,27 +1354,10 @@ bgp_best_selection (struct bgp *bgp, struct bgp_node
*rn,
!CHECK_FLAG (ri->peer->sflags, PEER_STATUS_NSF_WAIT))
if (ri->peer->status != Established)
continue;
-
- if (bgp_flag_check (bgp, BGP_FLAG_DETERMINISTIC_MED)
- && (! CHECK_FLAG (ri->flags, BGP_INFO_DMED_SELECTED)))
- {
- bgp_info_unset_flag (rn, ri, BGP_INFO_DMED_CHECK);
- continue;
- }
- bgp_info_unset_flag (rn, ri, BGP_INFO_DMED_CHECK);
- bgp_info_unset_flag (rn, ri, BGP_INFO_DMED_SELECTED);
-
+
if ((cmpret = bgp_info_cmp (bgp, ri, new_select, afi, safi)) == -1)
- {
- if (do_mpath && bgp_flag_check (bgp, BGP_FLAG_DETERMINISTIC_MED))
- bgp_mp_dmed_deselect (new_select);
-
- new_select = ri;
- }
- else if (cmpret == 1 && do_mpath
- && bgp_flag_check (bgp, BGP_FLAG_DETERMINISTIC_MED))
- bgp_mp_dmed_deselect (ri);
-
+ new_select = ri;
+
if (do_mpath)
{
if (cmpret != 0)
@@ -1448,11 +1367,8 @@ bgp_best_selection (struct bgp *bgp, struct bgp_node *rn,
bgp_mp_list_add (&mp_list, ri);
}
}
-
-
- if (!bgp_flag_check (bgp, BGP_FLAG_DETERMINISTIC_MED))
- bgp_info_mpath_update (rn, new_select, old_select, &mp_list, afi, safi);
-
+
+ bgp_info_mpath_update (rn, new_select, old_select, &mp_list, afi, safi);
bgp_info_mpath_aggregate_update (new_select, old_select);
bgp_mp_list_clear (&mp_list);
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 3d2eea5..2d91b15 100644
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -75,13 +75,11 @@ struct bgp_info
#define BGP_INFO_SELECTED (1 << 3)
#define BGP_INFO_VALID (1 << 4)
#define BGP_INFO_ATTR_CHANGED (1 << 5)
-#define BGP_INFO_DMED_CHECK (1 << 6)
-#define BGP_INFO_DMED_SELECTED (1 << 7)
-#define BGP_INFO_STALE (1 << 8)
-#define BGP_INFO_REMOVED (1 << 9)
-#define BGP_INFO_COUNTED (1 << 10)
-#define BGP_INFO_MULTIPATH (1 << 11)
-#define BGP_INFO_MULTIPATH_CHG (1 << 12)
+#define BGP_INFO_STALE (1 << 6)
+#define BGP_INFO_REMOVED (1 << 7)
+#define BGP_INFO_COUNTED (1 << 8)
+#define BGP_INFO_MULTIPATH (1 << 9)
+#define BGP_INFO_MULTIPATH_CHG (1 << 10)
/* BGP route type. This can be static, RIP, OSPF, BGP etc. */
u_char type;
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 5fe3b4e..5093170 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -918,30 +918,23 @@ DEFUN (no_bgp_always_compare_med,
}
/* "bgp deterministic-med" configuration. */
-DEFUN (bgp_deterministic_med,
- bgp_deterministic_med_cmd,
- "bgp deterministic-med",
- "BGP specific commands\n"
- "Pick the best-MED path among paths advertised from the neighboring
AS\n")
+DEFUN_DEPRECATED (bgp_deterministic_med,
+ bgp_deterministic_med_cmd,
+ "bgp deterministic-med",
+ "BGP specific commands\n"
+ "For compatibility, no effect\n")
{
- struct bgp *bgp;
-
- bgp = vty->index;
- bgp_flag_set (bgp, BGP_FLAG_DETERMINISTIC_MED);
+ vty_out (vty, "%% 'bgp deterministic-med no longer implemented'%s",
VTY_NEWLINE);
return CMD_SUCCESS;
}
-DEFUN (no_bgp_deterministic_med,
- no_bgp_deterministic_med_cmd,
- "no bgp deterministic-med",
- NO_STR
- "BGP specific commands\n"
- "Pick the best-MED path among paths advertised from the neighboring
AS\n")
+DEFUN_DEPRECATED (no_bgp_deterministic_med,
+ no_bgp_deterministic_med_cmd,
+ "no bgp deterministic-med",
+ NO_STR
+ "BGP specific commands\n"
+ "For compatibility, no effect\n")
{
- struct bgp *bgp;
-
- bgp = vty->index;
- bgp_flag_unset (bgp, BGP_FLAG_DETERMINISTIC_MED);
return CMD_SUCCESS;
}
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 8029abc..a3c7d1f 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -5330,10 +5330,6 @@ bgp_config_write (struct vty *vty)
if (bgp_flag_check (bgp, BGP_FLAG_ENFORCE_FIRST_AS))
vty_out (vty, " bgp enforce-first-as%s", VTY_NEWLINE);
- /* BGP deterministic-med. */
- if (bgp_flag_check (bgp, BGP_FLAG_DETERMINISTIC_MED))
- vty_out (vty, " bgp deterministic-med%s", VTY_NEWLINE);
-
/* BGP graceful-restart. */
if (bgp->stalepath_time != BGP_DEFAULT_STALEPATH_TIME)
vty_out (vty, " bgp graceful-restart stalepath-time %d%s",
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 9c7419e..633f9e1 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -123,7 +123,7 @@ struct bgp
/* BGP flags. */
u_int16_t flags;
#define BGP_FLAG_ALWAYS_COMPARE_MED (1 << 0)
-#define BGP_FLAG_DETERMINISTIC_MED (1 << 1)
+#define BGP_FLAG_UNUSED (1 << 1)
#define BGP_FLAG_MED_MISSING_AS_WORST (1 << 2)
#define BGP_FLAG_MED_CONFED (1 << 3)
#define BGP_FLAG_NO_DEFAULT_IPV4 (1 << 4)
--
2.5.0
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
