Skip to content

Commit

Permalink
BGPaaSv2: Implement AS override functionality
Browse files Browse the repository at this point in the history
Consider 2 bgpaas-clients in the same VN that are configured with the
same ASN. Routes sent by one client will not be advertised to the other
client by default since CN implements a sender side AS loop check.

CN needs to implement equivalent of JUNOS as-override knob in order to
work around this issue. RibOut AS is replaced by CN AS in the AS Path.

Pending items:

- Schema and config parsing changes to enable AS override
- Do not advertise routes back to peer even if as override is enabled

Change-Id: I963a8cb6cdbdf4ed27cd18726371e9a8bb19dd39
Partial-Bug: 1552952
  • Loading branch information
Nischal Sheth committed Mar 12, 2016
1 parent 0c46e09 commit feeb423
Show file tree
Hide file tree
Showing 15 changed files with 243 additions and 80 deletions.
22 changes: 20 additions & 2 deletions src/bgp/bgp_aspath.cc
Expand Up @@ -103,7 +103,9 @@ bool AsPathSpec::AsPathLoop(as_t as, uint8_t max_loop_count) const {
return false;
}

// Create a new AsPathSpec by prepending the given asn at the beginning
//
// Create a new AsPathSpec by prepending the given asn at the beginning.
//
AsPathSpec *AsPathSpec::Add(as_t asn) const {
AsPathSpec *new_spec = new AsPathSpec;
PathSegment *ps = new PathSegment;
Expand All @@ -122,7 +124,8 @@ AsPathSpec *AsPathSpec::Add(as_t asn) const {
} else {
new_spec->path_segments.push_back(ps);
}
if (first == last) return new_spec;
if (first == last)
return new_spec;
for (int i = first; i < last; i++) {
PathSegment *ps = new PathSegment;
*ps = *path_segments[i];
Expand All @@ -131,6 +134,21 @@ AsPathSpec *AsPathSpec::Add(as_t asn) const {
return new_spec;
}

//
// Create a new AsPathSpec by replacing the old asn with given asn.
//
AsPathSpec *AsPathSpec::Replace(as_t old_asn, as_t asn) const {
AsPathSpec *new_spec = new AsPathSpec(*this);
for (size_t i = 0; i < new_spec->path_segments.size(); ++i) {
PathSegment *ps = new_spec->path_segments[i];
for (size_t j = 0; j < ps->path_segment.size(); ++j) {
if (ps->path_segment[j] == old_asn)
ps->path_segment[j] = asn;
}
}
return new_spec;
}

void AsPath::Remove() {
aspath_db_->Delete(this);
}
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_aspath.h
Expand Up @@ -62,6 +62,7 @@ struct AsPathSpec : public BgpAttribute {
virtual size_t EncodeLength() const;
virtual std::string ToString() const;
AsPathSpec *Add(as_t asn) const;
AsPathSpec *Replace(as_t old_asn, as_t asn) const;
std::vector<PathSegment *> path_segments;
};

Expand Down
3 changes: 3 additions & 0 deletions src/bgp/bgp_config.cc
Expand Up @@ -150,6 +150,7 @@ BgpNeighborConfig::BgpNeighborConfig()
: type_(UNSPECIFIED),
admin_down_(false),
passive_(false),
as_override_(false),
peer_as_(0),
identifier_(0),
port_(BgpConfigManager::kDefaultPort),
Expand All @@ -167,6 +168,7 @@ void BgpNeighborConfig::CopyValues(const BgpNeighborConfig &rhs) {
type_ = rhs.type_;
admin_down_ = rhs.admin_down_;
passive_ = rhs.passive_;
as_override_ = rhs.as_override_;
peer_as_ = rhs.peer_as_;
identifier_ = rhs.identifier_;
address_ = rhs.address_;
Expand All @@ -189,6 +191,7 @@ int BgpNeighborConfig::CompareTo(const BgpNeighborConfig &rhs) const {
KEY_COMPARE(type_, rhs.type_);
KEY_COMPARE(admin_down_, rhs.admin_down_);
KEY_COMPARE(passive_, rhs.passive_);
KEY_COMPARE(as_override_, rhs.as_override_);
KEY_COMPARE(peer_as_, rhs.peer_as_);
KEY_COMPARE(identifier_, rhs.identifier_);
KEY_COMPARE(address_, rhs.address_);
Expand Down
4 changes: 4 additions & 0 deletions src/bgp/bgp_config.h
Expand Up @@ -168,6 +168,9 @@ class BgpNeighborConfig {
bool passive() const { return passive_; }
void set_passive(bool passive) { passive_ = passive; }

bool as_override() const { return as_override_; }
void set_as_override(bool as_override) { as_override_ = as_override; }

uint32_t peer_as() const { return peer_as_; }
void set_peer_as(uint32_t peer_as) { peer_as_ = peer_as; }

Expand Down Expand Up @@ -252,6 +255,7 @@ class BgpNeighborConfig {
std::string router_type_;
bool admin_down_;
bool passive_;
bool as_override_;
uint32_t peer_as_;
uint32_t identifier_;
IpAddress address_;
Expand Down
17 changes: 13 additions & 4 deletions src/bgp/bgp_peer.cc
Expand Up @@ -263,12 +263,12 @@ RibExportPolicy BgpPeer::BuildRibExportPolicy(Address::Family family) const {
family_attributes_list_[family];
if (!family_attributes ||
family_attributes->gateway_address.is_unspecified()) {
return RibExportPolicy(
peer_type_, RibExportPolicy::BGP, peer_as_, -1, 0);
return RibExportPolicy(peer_type_, RibExportPolicy::BGP, peer_as_,
as_override_, -1, 0);
} else {
IpAddress nexthop = family_attributes->gateway_address;
return RibExportPolicy(
peer_type_, RibExportPolicy::BGP, peer_as_, nexthop, -1, 0);
return RibExportPolicy(peer_type_, RibExportPolicy::BGP, peer_as_,
as_override_, nexthop, -1, 0);
}
}

Expand Down Expand Up @@ -370,6 +370,7 @@ BgpPeer::BgpPeer(BgpServer *server, RoutingInstance *instance,
admin_down_(config->admin_down()),
passive_(config->passive()),
resolve_paths_(config->router_type() == "bgpaas-client"),
as_override_(config->as_override()),
membership_req_pending_(0),
defer_close_(false),
vpn_tables_registered_(false),
Expand Down Expand Up @@ -407,6 +408,7 @@ BgpPeer::BgpPeer(BgpServer *server, RoutingInstance *instance,
peer_info.set_name(ToUVEKey());
peer_info.set_admin_down(admin_down_);
peer_info.set_passive(passive_);
peer_info.set_as_override(as_override_);
peer_info.set_router_type(router_type_);
peer_info.set_peer_type(
PeerType() == BgpProto::IBGP ? "internal" : "external");
Expand Down Expand Up @@ -604,6 +606,12 @@ void BgpPeer::ConfigUpdate(const BgpNeighborConfig *config) {
clear_session = true;
}

if (as_override_ != config->as_override()) {
as_override_ = config->as_override();
peer_info.set_as_override(as_override_);
clear_session = true;
}

if (router_type_ != config->router_type()) {
router_type_ = config->router_type();
peer_info.set_router_type(router_type_);
Expand Down Expand Up @@ -1743,6 +1751,7 @@ void BgpPeer::FillNeighborInfo(const BgpSandeshContext *bsc,
bnr->set_closed_at(UTCUsecToString(deleter_->delete_time_stamp_usecs()));
bnr->set_admin_down(admin_down_);
bnr->set_passive(passive_);
bnr->set_as_override(as_override_);
bnr->set_peer_address(peer_address_string());
bnr->set_peer_id(bgp_identifier_string());
bnr->set_peer_asn(peer_as());
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_peer.h
Expand Up @@ -386,6 +386,7 @@ class BgpPeer : public IPeer {
bool admin_down_;
bool passive_;
bool resolve_paths_;
bool as_override_;

uint64_t membership_req_pending_;
bool defer_close_;
Expand Down
15 changes: 9 additions & 6 deletions src/bgp/bgp_peer.sandesh
Expand Up @@ -61,18 +61,19 @@ struct BgpNeighborResp {
53: string instance_name;
1: string peer (link="BgpNeighborReq"); // Peer name
36: bool deleted; // Deletion in progress
43: string closed_at;
47: bool admin_down;
48: bool passive;
2: string peer_address (link="BgpNeighborReq");
25: string peer_id;
3: u32 peer_asn;
50: u32 peer_port;
51: string transport_address;
6: string encoding; // BGP/XMPP
7: string peer_type // internal/external
52: string router_type; // bgp_schema.xsd:BgpRouterType
8: string state;
43: string closed_at;
52: string router_type; // bgp_schema.xsd:BgpRouterType
47: bool admin_down;
48: bool passive;
55: bool as_override;
50: u32 peer_port;
51: string transport_address;
4: string local_address; // local ip address and port
26: string local_id;
5: u32 local_asn;
Expand Down Expand Up @@ -550,6 +551,7 @@ struct ShowBgpNeighborConfig {
2: string name;
13: bool admin_down;
15: bool passive;
20: bool as_override;
18: string router_type;
8: string local_identifier;
9: i32 local_as;
Expand Down Expand Up @@ -602,6 +604,7 @@ struct BgpPeerInfoData {
2: optional bool deleted
23: optional bool admin_down;
24: optional bool passive;
27: optional bool as_override;
25: optional string router_type; // bgp_schema.xsd:BgpRouterType
3: optional string peer_type; // internal/external
20: optional string peer_address;
Expand Down
6 changes: 6 additions & 0 deletions src/bgp/bgp_ribout.cc
Expand Up @@ -41,6 +41,12 @@ bool RibExportPolicy::operator<(const RibExportPolicy &rhs) const {
if (as_number > rhs.as_number) {
return false;
}
if (as_override < rhs.as_override) {
return true;
}
if (as_override > rhs.as_override) {
return false;
}
if (nexthop < rhs.nexthop) {
return true;
}
Expand Down
19 changes: 13 additions & 6 deletions src/bgp/bgp_ribout.h
Expand Up @@ -218,6 +218,9 @@ class RouteState : public DBState {
// practical deployment scenarios all eBGP peers will belong to the same
// neighbor AS anyway.
//
// Including AS override as part of the policy results in creation of a
// different RibOuts for neighbors that need and don't AS override.
//
// Including the nexthop as part of the policy results in creation of a
// different RibOut for each set of BGPaaS clients that belong to the same
// subnet. This allows us to rewrite the nexthop to the specified value.
Expand All @@ -234,12 +237,12 @@ struct RibExportPolicy {

RibExportPolicy()
: type(BgpProto::IBGP), encoding(BGP),
as_number(0), affinity(-1), cluster_id(0) {
as_number(0), as_override(false), affinity(-1), cluster_id(0) {
}

RibExportPolicy(BgpProto::BgpPeerType type, Encoding encoding,
int affinity, u_int32_t cluster_id)
: type(type), encoding(encoding), as_number(0),
: type(type), encoding(encoding), as_number(0), as_override(false),
affinity(affinity), cluster_id(cluster_id) {
if (encoding == XMPP)
assert(type == BgpProto::XMPP);
Expand All @@ -248,19 +251,21 @@ struct RibExportPolicy {
}

RibExportPolicy(BgpProto::BgpPeerType type, Encoding encoding,
as_t as_number, int affinity, u_int32_t cluster_id)
as_t as_number, bool as_override, int affinity, u_int32_t cluster_id)
: type(type), encoding(encoding), as_number(as_number),
affinity(affinity), cluster_id(cluster_id) {
as_override(as_override), affinity(affinity), cluster_id(cluster_id) {
if (encoding == XMPP)
assert(type == BgpProto::XMPP);
if (encoding == BGP)
assert(type == BgpProto::IBGP || type == BgpProto::EBGP);
}

RibExportPolicy(BgpProto::BgpPeerType type, Encoding encoding,
as_t as_number, IpAddress nexthop, int affinity, u_int32_t cluster_id)
as_t as_number, bool as_override, IpAddress nexthop,
int affinity, u_int32_t cluster_id)
: type(type), encoding(BGP), as_number(as_number),
nexthop(nexthop), affinity(affinity), cluster_id(cluster_id) {
as_override(as_override), nexthop(nexthop),
affinity(affinity), cluster_id(cluster_id) {
assert(type == BgpProto::IBGP || type == BgpProto::EBGP);
assert(encoding == BGP);
}
Expand All @@ -270,6 +275,7 @@ struct RibExportPolicy {
BgpProto::BgpPeerType type;
Encoding encoding;
as_t as_number;
bool as_override;
IpAddress nexthop;
int affinity;
uint32_t cluster_id;
Expand Down Expand Up @@ -342,6 +348,7 @@ class RibOut {

BgpProto::BgpPeerType peer_type() const { return policy_.type; }
as_t peer_as() const { return policy_.as_number; }
bool as_override() const { return policy_.as_override; }
const IpAddress &nexthop() const { return policy_.nexthop; }
bool IsEncodingXmpp() const {
return (policy_.encoding == RibExportPolicy::XMPP);
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_show_config.cc
Expand Up @@ -327,6 +327,7 @@ static void FillBgpNeighborConfigInfo(ShowBgpNeighborConfig *sbnc,
sbnc->set_name(neighbor->name());
sbnc->set_admin_down(neighbor->admin_down());
sbnc->set_passive(neighbor->passive());
sbnc->set_as_override(neighbor->as_override());
sbnc->set_router_type(neighbor->router_type());
sbnc->set_local_identifier(neighbor->local_identifier_string());
sbnc->set_local_as(neighbor->local_as());
Expand Down
32 changes: 21 additions & 11 deletions src/bgp/bgp_table.cc
Expand Up @@ -181,19 +181,19 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route,
BgpAttr *clone = new BgpAttr(*attr);

// Retain LocalPref value if set, else set default to 100.
if (attr->local_pref() == 0)
if (clone->local_pref() == 0)
clone->set_local_pref(100);

// If the route is locally originated i.e. there's no AsPath,
// then generate a Nil AsPath i.e. one with 0 length. No need
// to modify the AsPath if it already exists since this is an
// iBGP RibOut.
if (attr->as_path() == NULL) {
if (clone->as_path() == NULL) {
AsPathSpec as_path;
clone->set_as_path(&as_path);
}

attr_ptr = attr->attr_db()->Locate(clone);
attr_ptr = clone->attr_db()->Locate(clone);
attr = attr_ptr.get();
} else if (ribout->peer_type() == BgpProto::EBGP) {
// Don't advertise routes from non-master instances if there's
Expand All @@ -205,7 +205,7 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route,
}

// Sender side AS path loop check.
if (attr->as_path() &&
if (!ribout->as_override() && attr->as_path() &&
attr->as_path()->path().AsPathLoop(ribout->peer_as())) {
return NULL;
}
Expand All @@ -225,21 +225,31 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route,
clone->set_nexthop(ribout->nexthop());

// Reset LocalPref.
if (attr->local_pref())
if (clone->local_pref())
clone->set_local_pref(0);

// Reset Med if the path did not originate from an xmpp peer.
// The AS path is NULL if the originating xmpp peer is locally
// connected. It's non-NULL but empty if the originating xmpp
// peer is connected to another bgp speaker in the iBGP mesh.
if (attr->med() && attr->as_path() && !attr->as_path()->empty())
if (clone->med() && clone->as_path() && !clone->as_path()->empty())
clone->set_med(0);

// Prepend the local AS to AsPath.
as_t local_as =
attr->attr_db()->server()->local_autonomous_system();
if (attr->as_path() != NULL) {
const AsPathSpec &as_path = attr->as_path()->path();
clone->attr_db()->server()->local_autonomous_system();

// Override the peer AS with local AS in AsPath.
if (ribout->as_override() && clone->as_path() != NULL) {
const AsPathSpec &as_path = clone->as_path()->path();
AsPathSpec *as_path_ptr =
as_path.Replace(ribout->peer_as(), local_as);
clone->set_as_path(as_path_ptr);
delete as_path_ptr;
}

// Prepend the local AS to AsPath.
if (clone->as_path() != NULL) {
const AsPathSpec &as_path = clone->as_path()->path();
AsPathSpec *as_path_ptr = as_path.Add(local_as);
clone->set_as_path(as_path_ptr);
delete as_path_ptr;
Expand All @@ -250,7 +260,7 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route,
delete as_path_ptr;
}

attr_ptr = attr->attr_db()->Locate(clone);
attr_ptr = clone->attr_db()->Locate(clone);
attr = attr_ptr.get();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/bgp/bgp_xmpp_channel.cc
Expand Up @@ -502,7 +502,7 @@ BgpXmppChannel::BgpXmppChannel(XmppChannel *channel, BgpServer *bgp_server,
peer_(new XmppPeer(bgp_server, this)),
peer_close_(new PeerClose(this)),
peer_stats_(new PeerStats(this)),
bgp_policy_(peer_->PeerType(), RibExportPolicy::XMPP, 0, -1, 0),
bgp_policy_(peer_->PeerType(), RibExportPolicy::XMPP, -1, 0),
manager_(manager),
delete_in_progress_(false),
deleted_(false),
Expand Down

0 comments on commit feeb423

Please sign in to comment.