Comapre MED only if both paths are received from same neighbor AS

Change-Id: I9992fd751a5e284d213da3ff377cc01d49203939 Closes-Bug: 1508785
Juniper · May 21, 2016 · a3f97ba · a3f97ba
1 parent 9c5a4d1
commit a3f97ba
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 15 deletions.
diff --git a/src/bgp/bgp_aspath.cc b/src/bgp/bgp_aspath.cc
@@ -13,6 +13,30 @@ using std::copy;
 using std::ostringstream;
 using std::string;
 
+//
+// Return the left most AS.
+//
+as_t AsPathSpec::AsLeftMost() const {
+    if (path_segments.empty())
+        return 0;
+    if (path_segments[0]->path_segment_type == PathSegment::AS_SET)
+        return 0;
+    if (path_segments[0]->path_segment.empty())
+        return 0;
+    return (path_segments[0]->path_segment[0]);
+}
+
+//
+// Return true if left most AS matches the input.
+//
+bool AsPathSpec::AsLeftMostMatch(as_t as) const {
+    if (path_segments.empty())
+        return false;
+    if (path_segments[0]->path_segment.empty())
+        return false;
+    return (path_segments[0]->path_segment[0] == as);
+}
+
 int AsPathSpec::CompareTo(const BgpAttribute &rhs_attr) const {
     int ret = BgpAttribute::CompareTo(rhs_attr);
     if (ret != 0) return ret;

diff --git a/src/bgp/bgp_aspath.h b/src/bgp/bgp_aspath.h
@@ -53,15 +53,8 @@ struct AsPathSpec : public BgpAttribute {
         std::vector<as_t> path_segment;
     };
 
-    // Return true if left most AS matches the input
-    bool AsLeftMostMatch(as_t as) const {
-        if (path_segments.empty())
-            return false;
-        if (path_segments[0]->path_segment.empty())
-            return false;
-        return (path_segments[0]->path_segment[0] == as);
-    }
-
+    as_t AsLeftMost() const;
+    bool AsLeftMostMatch(as_t as) const;
     bool AsPathLoop(as_t as) const;
 
     virtual int CompareTo(const BgpAttribute &rhs_attr) const;
@@ -118,6 +111,8 @@ class AsPath {
         return 0;
     }
     const AsPathSpec &path() const { return path_; }
+    bool empty() const { return path_.path_segments.empty(); }
+    as_t neighbor_as() const { return path_.AsLeftMost(); }
 
     friend std::size_t hash_value(AsPath const &as_path) {
         size_t hash = 0;

diff --git a/src/bgp/bgp_attr.cc b/src/bgp/bgp_attr.cc
@@ -827,9 +827,8 @@ void BgpAttr::set_leaf_olist(const BgpOListSpec *leaf_olist_spec) {
     }
 }
 
-// TODO(nsheth): Return the left-most AS number in the path.
-uint32_t BgpAttr::neighbor_as() const {
-    return 0;
+as_t BgpAttr::neighbor_as() const {
+    return (as_path_.get() ? as_path_->neighbor_as() : 0);
 }
 
 uint32_t BgpAttr::sequence_number() const {

diff --git a/src/bgp/bgp_attr.h b/src/bgp/bgp_attr.h
@@ -717,7 +717,7 @@ class BgpAttr {
     uint32_t local_pref() const { return local_pref_; }
     bool atomic_aggregate() const { return atomic_aggregate_; }
     as_t aggregator_as_num() const { return aggregator_as_num_; }
-    uint32_t neighbor_as() const;
+    as_t neighbor_as() const;
     const IpAddress &aggregator_adderess() const { return aggregator_address_; }
     const Ip4Address &originator_id() const { return originator_id_; }
     const RouteDistinguisher &source_rd() const { return source_rd_; }

diff --git a/src/bgp/bgp_path.cc b/src/bgp/bgp_path.cc
@@ -82,9 +82,9 @@ int BgpPath::PathCompare(const BgpPath &rhs, bool allow_ecmp) const {
 
     KEY_COMPARE(attr_->origin(), rattr->origin());
 
-    if (attr_->neighbor_as() == rattr->neighbor_as()) {
+    // Compare med if both paths are learnt from the same neighbor as.
+    if (attr_->neighbor_as() && attr_->neighbor_as() == rattr->neighbor_as())
         KEY_COMPARE(attr_->med(), rattr->med());
-    }
 
     // Prefer locally generated routes over bgp and xmpp routes.
     BOOL_COMPARE(peer_ == NULL, rhs.peer_ == NULL);

diff --git a/src/bgp/test/bgp_attr_test.cc b/src/bgp/test/bgp_attr_test.cc
@@ -127,6 +127,43 @@ TEST_F(BgpAttrTest, AggregatorToString) {
     EXPECT_EQ("Aggregator <code: 7, flags: c0> : 100:0101010a", agg.ToString());
 }
 
+TEST_F(BgpAttrTest, AsPathLeftMost1) {
+    AsPathSpec spec;
+    EXPECT_EQ(0, spec.AsLeftMost());
+}
+
+TEST_F(BgpAttrTest, AsPathLeftMost2) {
+    AsPathSpec spec;
+    AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment;
+    spec.path_segments.push_back(ps);
+    ps->path_segment_type = AsPathSpec::PathSegment::AS_SET;
+    EXPECT_EQ(0, spec.AsLeftMost());
+    ps->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE;
+    EXPECT_EQ(0, spec.AsLeftMost());
+}
+
+TEST_F(BgpAttrTest, AsPathLeftMost3) {
+    AsPathSpec spec;
+    AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment;
+    spec.path_segments.push_back(ps);
+    ps->path_segment_type = AsPathSpec::PathSegment::AS_SET;
+    ps->path_segment.push_back(64512);
+    ps->path_segment.push_back(64513);
+    ps->path_segment.push_back(64514);
+    EXPECT_EQ(0, spec.AsLeftMost());
+}
+
+TEST_F(BgpAttrTest, AsPathLeftMost4) {
+    AsPathSpec spec;
+    AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment;
+    spec.path_segments.push_back(ps);
+    ps->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE;
+    ps->path_segment.push_back(64512);
+    ps->path_segment.push_back(64513);
+    ps->path_segment.push_back(64514);
+    EXPECT_EQ(64512, spec.AsLeftMost());
+}
+
 TEST_F(BgpAttrTest, AsPathCompare_Sequence) {
     AsPathSpec spec1;
     AsPathSpec::PathSegment *ps1 = new AsPathSpec::PathSegment;