Skip to content

Commit

Permalink
Add a source port field to BgpRouterParams
Browse files Browse the repository at this point in the history 
Use an explicit field for source port instead of reusing port field
which is really meant for destination port.

Change-Id: I792bede998c995eebea4d144899e5c7b6431911c
Partial-Bug: 1518047
  • Loading branch information
Nischal Sheth committed Dec 3, 2015
1 parent 2fd930e commit 4a6bdc5
Show file tree
Hide file tree
Showing 12 changed files with 41 additions and 20 deletions.
3 changes: 3 additions & 0 deletions src/bgp/bgp_config.cc
View file
Expand Up @@ -143,6 +143,7 @@ BgpNeighborConfig::BgpNeighborConfig()
peer_as_(0),
identifier_(0),
port_(BgpConfigManager::kDefaultPort),
source_port_(0),
hold_time_(0),
loop_count_(0),
local_as_(0),
Expand All @@ -160,6 +161,7 @@ void BgpNeighborConfig::CopyValues(const BgpNeighborConfig &rhs) {
identifier_ = rhs.identifier_;
address_ = rhs.address_;
port_ = rhs.port_;
source_port_ = rhs.source_port_;
hold_time_ = rhs.hold_time_;
loop_count_ = rhs.loop_count_;
local_as_ = rhs.local_as_;
Expand All @@ -179,6 +181,7 @@ int BgpNeighborConfig::CompareTo(const BgpNeighborConfig &rhs) const {
KEY_COMPARE(identifier_, rhs.identifier_);
KEY_COMPARE(address_, rhs.address_);
KEY_COMPARE(port_, rhs.port_);
KEY_COMPARE(source_port_, rhs.source_port_);
KEY_COMPARE(hold_time_, rhs.hold_time_);
KEY_COMPARE(loop_count_, rhs.loop_count_);
KEY_COMPARE(local_as_, rhs.local_as_);
Expand Down
10 changes: 7 additions & 3 deletions src/bgp/bgp_config.h
View file
Expand Up @@ -182,8 +182,11 @@ class BgpNeighborConfig {
return Ip4Address(ntohl(identifier_)).to_string();
}

int port() const { return port_; }
void set_port(int port) { port_ = port; }
uint16_t port() const { return port_; }
void set_port(uint16_t port) { port_ = port; }

uint16_t source_port() const { return source_port_; }
void set_source_port(uint16_t source_port) { source_port_ = source_port; }

std::string router_type() const { return router_type_; }
void set_router_type(const std::string &router_type) {
Expand Down Expand Up @@ -248,7 +251,8 @@ class BgpNeighborConfig {
uint32_t peer_as_;
uint32_t identifier_;
IpAddress address_;
int port_;
uint16_t port_;
uint16_t source_port_;
TcpSession::Endpoint remote_endpoint_;
int hold_time_;
uint8_t loop_count_;
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_config_ifmap.cc
View file
Expand Up @@ -296,6 +296,7 @@ static BgpNeighborConfig *MakeBgpNeighborConfig(

neighbor->set_peer_identifier(IpAddressToBgpIdentifier(identifier));
neighbor->set_port(params.port);
neighbor->set_source_port(params.source_port);
neighbor->set_hold_time(params.hold_time);

if (session != NULL) {
Expand Down
14 changes: 12 additions & 2 deletions src/bgp/bgp_peer.cc
View file
Expand Up @@ -343,6 +343,7 @@ BgpPeer::BgpPeer(BgpServer *server, RoutingInstance *instance,
: server_(server),
rtinstance_(instance),
peer_key_(config),
peer_port_(config->source_port()),
peer_name_(config->name()),
router_type_(config->router_type()),
config_(config),
Expand Down Expand Up @@ -401,10 +402,13 @@ 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_router_type(router_type_);
peer_info.set_peer_type(
PeerType() == BgpProto::IBGP ? "internal" : "external");
peer_info.set_local_asn(local_as_);
peer_info.set_peer_asn(peer_as_);
peer_info.set_peer_port(peer_port_);
peer_info.set_hold_time(hold_time_);
peer_info.set_local_id(local_bgp_id_);
peer_info.set_configured_families(config->GetAddressFamilies());
peer_info.set_peer_address(peer_key_.endpoint.address().to_string());
Expand Down Expand Up @@ -552,7 +556,7 @@ bool BgpPeer::ProcessFamilyAttributesConfig(const BgpNeighborConfig *config) {

void BgpPeer::ProcessEndpointConfig(const BgpNeighborConfig *config) {
if (config->router_type() == "bgpaas-client") {
endpoint_ = TcpSession::Endpoint(Ip4Address(), config->port());
endpoint_ = TcpSession::Endpoint(Ip4Address(), config->source_port());
} else {
endpoint_ = TcpSession::Endpoint();
}
Expand Down Expand Up @@ -604,9 +608,15 @@ void BgpPeer::ConfigUpdate(const BgpNeighborConfig *config) {
peer_info.set_peer_address(peer_key_.endpoint.address().to_string());
clear_session = true;
}
ProcessEndpointConfig(config);
ProcessAuthKeyChainConfig(config);

if (peer_port_ != config->source_port()) {
peer_port_ = config->source_port();
peer_info.set_peer_port(peer_port_);
clear_session = true;
}
ProcessEndpointConfig(config);

// Check if there is any change in the configured address families.
if (ProcessFamilyAttributesConfig(config)) {
peer_info.set_configured_families(configured_families_);
Expand Down
3 changes: 2 additions & 1 deletion src/bgp/bgp_peer.h
View file
Expand Up @@ -137,7 +137,7 @@ class BgpPeer : public IPeer {
return peer_key_.endpoint.address().to_string();
}
const BgpPeerKey &peer_key() const { return peer_key_; }
uint16_t peer_port() const { return peer_key_.endpoint.port(); }
uint16_t peer_port() const { return peer_port_; }
std::string transport_address_string() const;
const std::string &peer_name() const { return peer_name_; }
const std::string &peer_basename() const { return peer_basename_; }
Expand Down Expand Up @@ -342,6 +342,7 @@ class BgpPeer : public IPeer {
RoutingInstance *rtinstance_;
TcpSession::Endpoint endpoint_;
BgpPeerKey peer_key_;
uint16_t peer_port_;
std::string peer_name_;
std::string peer_basename_;
std::string router_type_; // bgp_schema.xsd:BgpRouterType
Expand Down
3 changes: 2 additions & 1 deletion src/bgp/bgp_peer.sandesh
View file
Expand Up @@ -471,7 +471,7 @@ struct ShowBgpNeighborConfig {
4: i32 autonomous_system;
5: string identifier;
6: string address;
19: u32 port;
19: u32 source_port;
14: i32 hold_time;
16: i32 loop_count;
10: string last_change_at;
Expand Down Expand Up @@ -501,6 +501,7 @@ struct BgpPeerInfoData {
20: optional string peer_address;
4: optional u32 local_asn;
5: optional u32 peer_asn;
26: optional u32 peer_port;
6: optional u32 hold_time;
7: optional string notification_in;
8: optional u64 notification_in_at;
Expand Down
2 changes: 1 addition & 1 deletion src/bgp/bgp_show_config.cc
View file
Expand Up @@ -187,7 +187,7 @@ static void FillBgpNeighborConfigInfo(ShowBgpNeighborConfig *sbnc,
sbnc->set_autonomous_system(neighbor->peer_as());
sbnc->set_identifier(neighbor->peer_identifier_string());
sbnc->set_address(neighbor->peer_address().to_string());
sbnc->set_port(neighbor->port());
sbnc->set_source_port(neighbor->source_port());
sbnc->set_address_families(neighbor->GetAddressFamilies());
sbnc->set_hold_time(neighbor->hold_time());
sbnc->set_loop_count(neighbor->loop_count());
Expand Down
12 changes: 6 additions & 6 deletions src/bgp/test/bgp_ifmap_config_manager_test.cc
View file
Expand Up @@ -1327,7 +1327,7 @@ TEST_F(BgpIfmapConfigManagerTest, BGPaaSNeighbors1) {
EXPECT_EQ("192.168.1.1", nbr_config1->local_identifier_string());
EXPECT_EQ(65001, nbr_config1->peer_as());
EXPECT_EQ("10.0.0.1", nbr_config1->peer_address_string());
EXPECT_EQ(1024, nbr_config1->port());
EXPECT_EQ(1024, nbr_config1->source_port());

const BgpNeighborConfig *nbr_config2;
nbr_config2 = config_manager_->FindNeighbor("test", "test:vm2:0");
Expand All @@ -1337,7 +1337,7 @@ TEST_F(BgpIfmapConfigManagerTest, BGPaaSNeighbors1) {
EXPECT_EQ("192.168.1.1", nbr_config2->local_identifier_string());
EXPECT_EQ(65002, nbr_config2->peer_as());
EXPECT_EQ("10.0.0.2", nbr_config2->peer_address_string());
EXPECT_EQ(1025, nbr_config2->port());
EXPECT_EQ(1025, nbr_config2->source_port());

// Change asn and identifier for master.
content = FileRead("controller/src/bgp/testdata/config_test_36b.xml");
Expand Down Expand Up @@ -1378,12 +1378,12 @@ TEST_F(BgpIfmapConfigManagerTest, BGPaaSNeighbors2) {
// Verify that the port is set for test:vm1.
const BgpNeighborConfig *nbr_config1;
nbr_config1 = config_manager_->FindNeighbor("test", "test:vm1:0");
EXPECT_EQ(1024, nbr_config1->port());
EXPECT_EQ(1024, nbr_config1->source_port());

// Verify that the port is set for test:vm2.
const BgpNeighborConfig *nbr_config2;
nbr_config2 = config_manager_->FindNeighbor("test", "test:vm2:0");
EXPECT_EQ(1025, nbr_config2->port());
EXPECT_EQ(1025, nbr_config2->source_port());

// Update port numbers.
content = FileRead("controller/src/bgp/testdata/config_test_36d.xml");
Expand All @@ -1392,11 +1392,11 @@ TEST_F(BgpIfmapConfigManagerTest, BGPaaSNeighbors2) {

// Verify that the port is updated for test:vm1.
nbr_config1 = config_manager_->FindNeighbor("test", "test:vm1:0");
EXPECT_EQ(1025, nbr_config1->port());
EXPECT_EQ(1025, nbr_config1->source_port());

// Verify that the port is updated for test:vm2.
nbr_config2 = config_manager_->FindNeighbor("test", "test:vm2:0");
EXPECT_EQ(1024, nbr_config2->port());
EXPECT_EQ(1024, nbr_config2->source_port());

// Cleanup.
boost::replace_all(content, "<config>", "<delete>");
Expand Down
4 changes: 2 additions & 2 deletions src/bgp/testdata/config_test_36a.xml
View file
Expand Up @@ -37,7 +37,7 @@
<router-type>bgpaas-client</router-type>
<autonomous-system>65001</autonomous-system>
<address>10.0.0.1</address>
<port>1024</port>
<source-port>1024</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
Expand All @@ -54,7 +54,7 @@
<router-type>bgpaas-client</router-type>
<autonomous-system>65002</autonomous-system>
<address>10.0.0.2</address>
<port>1025</port>
<source-port>1025</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
Expand Down
4 changes: 2 additions & 2 deletions src/bgp/testdata/config_test_36c.xml
View file
Expand Up @@ -37,7 +37,7 @@
<router-type>bgpaas-client</router-type>
<autonomous-system>65001</autonomous-system>
<address>10.0.0.1</address>
<port>1025</port>
<source-port>1025</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
Expand All @@ -54,7 +54,7 @@
<router-type>bgpaas-client</router-type>
<autonomous-system>65002</autonomous-system>
<address>10.0.0.2</address>
<port>1025</port>
<source-port>1025</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
Expand Down
4 changes: 2 additions & 2 deletions src/bgp/testdata/config_test_36d.xml
View file
Expand Up @@ -37,7 +37,7 @@
<router-type>bgpaas-client</router-type>
<autonomous-system>65001</autonomous-system>
<address>10.0.0.1</address>
<port>1025</port>
<source-port>1025</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
Expand All @@ -54,7 +54,7 @@
<router-type>bgpaas-client</router-type>
<autonomous-system>65002</autonomous-system>
<address>10.0.0.2</address>
<port>1024</port>
<source-port>1024</source-port>
<session to='bgpaas-server:0'>
<family-attributes>
<address-family>inet</address-family>
Expand Down
1 change: 1 addition & 0 deletions src/schema/bgp_schema.xsd
View file
Expand Up @@ -100,6 +100,7 @@
<xsd:element name='identifier' type='smi:IpAddress'/>
<xsd:element name='address' type='smi:IpAddress'/>
<xsd:element name='port' type='xsd:integer'/>
<xsd:element name='source-port' type='xsd:integer'/>
<xsd:element name='hold-time' type='BgpHoldTime' default='90'/>
<xsd:element name='address-families' type='AddressFamilies'/>
<xsd:element name='auth-data' type='AuthenticationData'/>
Expand Down

0 comments on commit 4a6bdc5

Please sign in to comment.