On Wed, 25 Nov 2015, Daniel Walton wrote:

      +bool
      +bgp_mpath_is_configured_sort (struct bgp *bgp, bgp_peer_sort_t
      sort,
      +                              afi_t afi, safi_t safi)
      +{
      +  struct bgp_maxpaths_cfg *cfg = &bgp->maxpaths[afi][safi];
      +
      +  /* XXX: BGP_DEFAULT_MAXPATHS is 1, and this test only seems
      to make sense
      +   * if if it stays 1, so not sure the DEFAULT define is that
      useful.
      +   */
      +  switch (sort)
      +    {
      +      case BGP_PEER_IBGP:
      +        return cfg->maxpaths_ibgp != BGP_DEFAULT_MAXPATHS;


It would be worth future proofing this

Agreed. It's a bit odd at present.

(we have a patch to increase BGP_DEFAULT_MAXPATHS) and return true if cfg->maxpaths_ibgp > 1

Cool.

      @@ -473,41 +473,32 @@ bgp_info_cmp (struct bgp *bgp, struct
      bgp_info *new, struct bgp_info *exist,
           existm = exist->extra->igpmetric;

         if (newm < existm)
      -    ret = 1;
      +    return -1;
         if (newm > existm)
      -    ret = 0;
      +    return 1;

         /* 9. Maximum path check. */
      -  if (newm == existm)
      +  if (bgp_mpath_is_configured (bgp, afi, safi))
           {
             if (bgp_flag_check(bgp, BGP_FLAG_ASPATH_MULTIPATH_RELAX))
               {
      -
      -         /*
      -          * For the two paths, all comparison steps till IGP
      metric
      -          * have succeeded - including AS_PATH hop count. Since
      'bgp
      -          * bestpath as-path multipath-relax' knob is on, we
      don't need
      -          * an exact match of AS_PATH. Thus, mark the paths are
      equal.
      -          * That will trigger both these paths to get into the
      multipath
      -          * array.
      -          */
      -         *paths_eq = 1;
      +          /*
      +           * For the two paths, all comparison steps till IGP
      metric
      +           * have succeeded - including AS_PATH hop count.
      Since 'bgp
      +           * bestpath as-path multipath-relax' knob is on, we
      don't need
      +           * an exact match of AS_PATH. Thus, mark the paths
      are equal.
      +           * That will trigger both these paths to get into the
      multipath
      +           * array.
      +           */
      +          return 0;


I don't follow how bestpath selection is working when max-paths is enabled. Say we have two paths who only differ by router-id...we still need to get to step #11 to determine which is the winner.

This had me scratching my head a bit initially too.

The original mpath code allows best-path selection to continue on, to find a 'winner', but then rolls them together if paths_eq was set to 1 anyway.

So, if mpath says they are equal, there's no point continuing on to pick a 'winner' cause mpath is going to treat them as equal and combine them. So, it can just return "equal" at this point.

My goal here is to make each check as self-contained as possible, and make bgp_info_cmp a pipe-line of checks (bgp_info_cmp becomes a monad), so that it's easier to understand and describe its structure and effects.

regards,
--
Paul Jakma, HPE Networking, Advanced Technology Group
Fortune:
Life is the urge to ecstasy.
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

Reply via email to