From a3f97bad40a28d345dd9dd855dc6cd4e609111b2 Mon Sep 17 00:00:00 2001 From: Nischal Sheth Date: Thu, 22 Oct 2015 11:17:35 -0700 Subject: [PATCH] Comapre MED only if both paths are received from same neighbor AS Change-Id: I9992fd751a5e284d213da3ff377cc01d49203939 Closes-Bug: 1508785 --- src/bgp/bgp_aspath.cc | 24 +++++++++++++++++++++++ src/bgp/bgp_aspath.h | 13 ++++-------- src/bgp/bgp_attr.cc | 5 ++--- src/bgp/bgp_attr.h | 2 +- src/bgp/bgp_path.cc | 4 ++-- src/bgp/test/bgp_attr_test.cc | 37 +++++++++++++++++++++++++++++++++++ 6 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/bgp/bgp_aspath.cc b/src/bgp/bgp_aspath.cc index 67b94bb656b..8e0beb47c64 100644 --- 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 index 35c67861f99..308aab0fafe 100644 --- a/src/bgp/bgp_aspath.h +++ b/src/bgp/bgp_aspath.h @@ -53,15 +53,8 @@ struct AsPathSpec : public BgpAttribute { std::vector 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 index 6566555979b..231239248c5 100644 --- 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 index be04277bc5f..718318ce7d3 100644 --- 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 index 7fd68fed543..df39d22cb88 100644 --- 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 index c6e38387daa..73912934443 100644 --- 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 : 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;