Skip to content

Commit

Permalink
Support single-active BGPaaS object attached to multiple VMIs
Browse files Browse the repository at this point in the history
Closes-Bug: #1649707

Vrouter-Agent changes to support the multiple VMI interfaces for the given
BGPAAS session
- Changes to tweak the BGP service port number.
  1. If the given BGP service port is X then the same will be
     shifted by the N number of bits to accomodate the additional
     VMI interfaces who are sharing this port.
  2. It results in the different service port number during the flow
     configuration for each VMI interface.
- Changes to configure the number of bits reserved to share
  the same service port across VMI interfaces
  1. Added new varaible under SERVICE section vrouter-agent.conf file
     to configre the number of bits reserved to hold the multiple VMI interfaces.
- Changes in the UT test cases to validate these changes
  1. Test cases are added to share the same service port across
     multiple VMI interfaces
  2. Test cases are added to valdiate the new variable addition to
     agent.conf file.

UT Failure Fix :
The changes added to verify the reverse flow in test_bgp_service is not
working when the same test cases are run multiple times. This behavior
is observed irrespective of the new changes. So to proceed further on
this change, we have reverted those reverse flwo checks.

Review Comments Handling :
In the proposed change, have used local index allocator to keep track
the usage of service ports by multiple VMI interfaces. It leads to store
this local index across the reboots of compute nodes and will result in
additional burden to resource manager to maintain one more state to keep
track theses indexes for each service port.
To avoid this additional burden, it is decided to re-use the VMI index
which are allocated by the agent during the interface creation. This
index will be used to identify the each VMI uniquely for the given
service port.
Some of the caveats with this re-use model as follows,
- the reserved shift bits are not directly working with allocatted index,
  since both are defined seperately and working independently
- if the VMI index is going beyond the allocated bits space then same
  will not be fit in the reserved space, eventhough there is enough
  space to store the indexes
Assumption:
- In the case of re-usage of same index during the VMI deletion and
  addition, the corresponding flows of deleted VMI interfaces will be
  deleted before adding the newer VMI with re-used index.
Have handles all the review comments
 - updating the source port with newer source port, since the same will
   be used during the flow configuration.
 - Retained older source port to identify the users of the given service
   port from the config

Review Comments Hanling-II:
After discussion, the logic for the derived port is changed as per the
below comment,
https://bugs.launchpad.net/juniperopenstack/+bug/1649707/comments/17

Change-Id: Ia282380dfc3dd209d3916d8f64bce89e89c27994
  • Loading branch information
srinivn committed Apr 23, 2017
1 parent a276c59 commit a35e554
Show file tree
Hide file tree
Showing 12 changed files with 353 additions and 27 deletions.
1 change: 1 addition & 0 deletions src/base/SConscript
Expand Up @@ -56,6 +56,7 @@ env.Append(CPPPATH = env['TOP'])
libbase = env.Library('base',
[VersionInfoSandeshGenSrcs +
['backtrace.cc',
'bgp_as_service_utils.cc',
'contrail_ports.cc',
'misc_utils.cc',
'bitset.cc',
Expand Down
87 changes: 87 additions & 0 deletions src/base/bgp_as_service_utils.cc
@@ -0,0 +1,87 @@
/*
* Copyright (c) 2017 Juniper Networks, Inc. All rights reserved.
*/

#include <base/bgp_as_service_utils.h>
/*
* This utils apis will be used by BGP As A Service feature to map
* between the original service port allocated for the given BgpAsAService
* session and the derived service port for each VMI who is sharing the same
* session
* GetDerivedBgpaasServicePortRange - From the given config port range and max
* session, derive the internal port range
* Port range and maximum number of VMIs who is sharing the given BgpAsAService
* session will be given by config
* EncodeBgpaasServicePort - For the given original service port and
* index, derive the service port which will be used by the corresponding
* flow which is associated with the corresponding VMI.
* DecodeBgpaasServicePort - For the given service port, decode the
* original service port and the index for the given port.
*/
using namespace std;
void BgpaasUtils::GetDerivedBgpaasServicePortRange(
const uint16_t port_range_start,
const uint16_t port_range_end,
const uint32_t max_session,
uint16_t &bgpaas_der_port_start,
uint16_t &bgpaas_der_port_end) {
uint32_t total_der_ports = (port_range_end - port_range_start) *
max_session;
if ( (total_der_ports + port_range_end + 1) > USHRT_MAX) {
bgpaas_der_port_start = bgpaas_der_port_start - total_der_ports;
bgpaas_der_port_end = port_range_start - 1;
} else {
bgpaas_der_port_start = port_range_end + 1;
bgpaas_der_port_end = bgpaas_der_port_start + total_der_ports;
}
}

BgpaasUtils::BgpAsServicePortIndexPair BgpaasUtils::DecodeBgpaasServicePort(
const uint32_t sport,
const uint32_t max_session,
const uint16_t port_range_start,
const uint16_t port_range_end) {
size_t index = 0;
uint32_t original_sport;

if ((sport >= port_range_start) &&
(sport <= port_range_end)) {
return std::make_pair(sport, index);
}

uint16_t bgpaas_der_port_start = 0;
uint16_t bgpaas_der_port_end = 0;
GetDerivedBgpaasServicePortRange(port_range_start,
port_range_end,
max_session,
bgpaas_der_port_start,
bgpaas_der_port_end);

original_sport = ((sport - bgpaas_der_port_start) / (max_session - 1))
+ port_range_start;
index = ((sport - bgpaas_der_port_start) % (max_session - 1)) + 1;

return std::make_pair(original_sport, index);
}

uint32_t BgpaasUtils::EncodeBgpaasServicePort(const uint32_t sport,
const size_t index,
const uint32_t max_session,
const uint16_t port_range_start,
const uint16_t port_range_end) {
if (!index) {
return sport;
}

uint16_t bgpaas_der_port_start = 0;
uint16_t bgpaas_der_port_end = 0;
GetDerivedBgpaasServicePortRange(port_range_start,
port_range_end,
max_session,
bgpaas_der_port_start,
bgpaas_der_port_end);

return (bgpaas_der_port_start + ((sport - port_range_start)
* (max_session - 1))
+ (index - 1));
}
33 changes: 33 additions & 0 deletions src/base/bgp_as_service_utils.h
@@ -0,0 +1,33 @@
/*
* Copyright (c) 2017 Juniper Networks, Inc. All rights reserved.
*/

#ifndef ctrlplane_bgp_as_service_utils_h
#define ctrlplane_bgp_as_service_utils_h
#include <stdlib.h>
#include <limits.h>
#include <inttypes.h>
#include <utility>

class BgpaasUtils {
public:
typedef std::pair<uint32_t, size_t> BgpAsServicePortIndexPair;
static void GetDerivedBgpaasServicePortRange(
const uint16_t port_range_start,
const uint16_t port_range_end,
const uint32_t max_session,
uint16_t &bgpaas_der_port_start,
uint16_t &bgpaas_der_port_end);
static uint32_t EncodeBgpaasServicePort(const uint32_t sport,
const size_t index,
const uint32_t max_session,
const uint16_t port_range_start,
const uint16_t port_range_end);
static BgpAsServicePortIndexPair DecodeBgpaasServicePort(
const uint32_t sport,
const uint32_t max_session,
const uint16_t port_range_start,
const uint16_t port_range_end);
};

#endif // ctrlplane_bgp_service_utils_h
8 changes: 8 additions & 0 deletions src/base/index_allocator.cc
Expand Up @@ -27,3 +27,11 @@ void IndexAllocator::FreeIndex(size_t index) {
assert(index <= max_index_);
bitset_.reset(index);
}

bool IndexAllocator::NoneIndexSet() {
return bitset_.none();
}

bool IndexAllocator::AnyIndexSet() {
return bitset_.any();
}
2 changes: 2 additions & 0 deletions src/base/index_allocator.h
Expand Up @@ -18,6 +18,8 @@ class IndexAllocator {

size_t AllocIndex();
void FreeIndex(size_t index);
bool NoneIndexSet();
bool AnyIndexSet();

private:
BitSet bitset_;
Expand Down
38 changes: 21 additions & 17 deletions src/vnsw/agent/init/agent_param.cc
Expand Up @@ -668,6 +668,8 @@ void AgentParam::ParseServicesArguments
GetOptValue<string>(v, bgp_as_a_service_port_range_,
"SERVICES.bgp_as_a_service_port_range");
GetOptValue<uint32_t>(v, services_queue_limit_, "SERVICES.queue_limit");
GetOptValue<uint32_t>(v, bgpaas_max_shared_sessions_,
"SERVICES.bgpaas_max_shared_sessions");
}

void AgentParam::ParseSandeshArguments
Expand Down Expand Up @@ -802,6 +804,14 @@ void AgentParam::ReInitFromConfig() {
return;
}

void AgentParam::UpdateBgpAsaServicePortRangeValue() {
if (!stringToIntegerList(bgp_as_a_service_port_range_, "-",
bgp_as_a_service_port_range_value_)) {
bgp_as_a_service_port_range_value_.clear();
return;
}
}

void AgentParam::UpdateBgpAsaServicePortRange() {
if (!stringToIntegerList(bgp_as_a_service_port_range_, "-",
bgp_as_a_service_port_range_value_)) {
Expand Down Expand Up @@ -853,12 +863,6 @@ void AgentParam::ComputeFlowLimits() {
max_vm_flows_ = 0;
}

uint16_t bgp_as_a_service_count = 0;
if (bgp_as_a_service_port_range_value_.size() == 2) {
bgp_as_a_service_count = bgp_as_a_service_port_range_value_[1]
- bgp_as_a_service_port_range_value_[0] + 1;
}

struct rlimit rl;
int result = getrlimit(RLIMIT_NOFILE, &rl);
if (result == 0) {
Expand All @@ -867,36 +871,31 @@ void AgentParam::ComputeFlowLimits() {
linklocal_system_flows_ = linklocal_vm_flows_ = 0;
return;
}
if (linklocal_system_flows_ > rl.rlim_max - bgp_as_a_service_count -
if (linklocal_system_flows_ > rl.rlim_max -
Agent::kMaxOtherOpenFds - 1) {
linklocal_system_flows_ = rl.rlim_max - bgp_as_a_service_count -
linklocal_system_flows_ = rl.rlim_max -
Agent::kMaxOtherOpenFds - 1;
cout << "Updating linklocal-system-flows configuration to : " <<
linklocal_system_flows_ << "\n";
}
if (rl.rlim_cur < linklocal_system_flows_ + bgp_as_a_service_count +
if (rl.rlim_cur < linklocal_system_flows_ +
Agent::kMaxOtherOpenFds + 1) {
struct rlimit new_rl;
new_rl.rlim_max = rl.rlim_max;
new_rl.rlim_cur = linklocal_system_flows_ + bgp_as_a_service_count +
new_rl.rlim_cur = linklocal_system_flows_ +
Agent::kMaxOtherOpenFds + 1;
result = setrlimit(RLIMIT_NOFILE, &new_rl);
if (result != 0) {
if (rl.rlim_cur <= Agent::kMaxOtherOpenFds + 1) {
linklocal_system_flows_ = 0;
bgp_as_a_service_count = 0;
bgp_as_a_service_port_range_value_.clear();
} else {
linklocal_system_flows_ = rl.rlim_cur -
bgp_as_a_service_count -
Agent::kMaxOtherOpenFds - 1;
}
cout << "Unable to set Max open files limit to : " <<
new_rl.rlim_cur <<
" Updating linklocal-system-flows configuration to : " <<
linklocal_system_flows_ <<
" and Bgp as a service port count to : " <<
bgp_as_a_service_count << "\n";
linklocal_system_flows_ << "\n";
}
}
if (linklocal_vm_flows_ > linklocal_system_flows_) {
Expand Down Expand Up @@ -1016,7 +1015,7 @@ void AgentParam::Init(const string &config_file, const string &program_name) {
InitFromConfig();
ProcessArguments();
InitVhostAndXenLLPrefix();
UpdateBgpAsaServicePortRange();
UpdateBgpAsaServicePortRangeValue();
ComputeFlowLimits();
vgw_config_table_->InitFromConfig(tree_);
}
Expand Down Expand Up @@ -1109,6 +1108,7 @@ void AgentParam::LogConfig() const {
LOG(DEBUG, "Service instance lbaas auth : " << si_lbaas_auth_conf_);
LOG(DEBUG, "Bgp as a service port range : " << bgp_as_a_service_port_range_);
LOG(DEBUG, "Services queue limit : " << services_queue_limit_);
LOG(DEBUG, "BGPAAS max shared sessions for service port : " << bgpaas_max_shared_sessions_);

LOG(DEBUG, "Sandesh Key file : " << sandesh_config_.keyfile);
LOG(DEBUG, "Sandesh Cert file : " << sandesh_config_.certfile);
Expand Down Expand Up @@ -1232,7 +1232,9 @@ AgentParam::AgentParam(bool enable_flow_options,
flow_trace_enable_(true),
flow_latency_limit_(Agent::kDefaultFlowLatencyLimit),
subnet_hosts_resolvable_(true),
bgp_as_a_service_port_range_("50000-50512"),
services_queue_limit_(1024),
bgpaas_max_shared_sessions_(4),
sandesh_config_(),
restart_backup_enable_(true),
restart_backup_idle_timeout_(CFG_BACKUP_IDLE_TIMEOUT),
Expand Down Expand Up @@ -1521,6 +1523,8 @@ AgentParam::AgentParam(bool enable_flow_options,
"Port range for BgPass ")
("SERVICES.queue_limit", opt::value<uint32_t>()->default_value(1024),
"Work queue for different services")
("SERVICES.bgpaas_max_shared_sessions", opt::value<uint32_t>(),
"BGPAAS max shared sessions for service port")
("SERVICE-INSTANCE.lbaas_auth_conf", opt::value<string>(),
"Credentials fo ssl certificates and private-keys")
;
Expand Down
9 changes: 9 additions & 0 deletions src/vnsw/agent/init/agent_param.h
Expand Up @@ -366,6 +366,13 @@ class AgentParam {
return bgp_as_a_service_port_range_value_;
}
uint32_t services_queue_limit() { return services_queue_limit_; }
uint32_t bgpaas_max_shared_sessions() const {
return bgpaas_max_shared_sessions_;
}
void set_bgpaas_max_shared_sessions(uint32_t val) {
bgpaas_max_shared_sessions_ = val;
}


const SandeshConfig &sandesh_config() const { return sandesh_config_; }

Expand Down Expand Up @@ -520,6 +527,7 @@ class AgentParam {
private:
friend class AgentParamTest;
void UpdateBgpAsaServicePortRange();
void UpdateBgpAsaServicePortRangeValue();
void ComputeFlowLimits();
static std::map<string, std::map<string, string> > ParseDerivedStats(
const std::vector<std::string> &dsvec);
Expand Down Expand Up @@ -690,6 +698,7 @@ class AgentParam {
std::string bgp_as_a_service_port_range_;
std::vector<uint16_t> bgp_as_a_service_port_range_value_;
uint32_t services_queue_limit_;
uint32_t bgpaas_max_shared_sessions_;

// Sandesh config options
SandeshConfig sandesh_config_;
Expand Down
1 change: 1 addition & 0 deletions src/vnsw/agent/init/test/cfg.ini
Expand Up @@ -116,6 +116,7 @@ physical_interface=vnet0
[SERVICES]
bgp_as_a_service_port_range=100-199
queue_limit=8192
bgpaas_max_shared_vmis=4

[GATEWAY-0]
routing_instance=default-domain:admin:public:public
Expand Down
6 changes: 4 additions & 2 deletions src/vnsw/agent/init/test/test_agent_init.cc
Expand Up @@ -76,6 +76,7 @@ TEST_F(AgentParamTest, Agent_Conf_file_1) {
EXPECT_EQ(ports2[0], 100);
EXPECT_EQ(ports2[1], 199);
EXPECT_EQ(param.services_queue_limit(), 8192);
EXPECT_EQ(param.bgpaas_max_shared_sessions(), 4);

// By default, flow-tracing must be enabled
EXPECT_TRUE(param.flow_trace_enable());
Expand Down Expand Up @@ -218,9 +219,10 @@ TEST_F(AgentParamTest, Agent_Conf_file_3) {
param.bgp_as_a_service_port_range_value();
EXPECT_EQ(ports[0], 100);
EXPECT_EQ(ports[1], 199);
EXPECT_EQ(param.bgpaas_max_shared_sessions(), 4);

EXPECT_EQ(param.linklocal_system_flows(), 411);
EXPECT_EQ(param.linklocal_vm_flows(), 411);
EXPECT_EQ(param.linklocal_system_flows(), 511);
EXPECT_EQ(param.linklocal_vm_flows(), 511);
}
}

Expand Down

0 comments on commit a35e554

Please sign in to comment.