Skip to content

Commit

Permalink
Merge "Use the next-hop specified in host route DHCP option." into R2.20
Browse files Browse the repository at this point in the history
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Aug 12, 2015
2 parents 53a28c1 + e924ed8 commit bad108e
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 66 deletions.
37 changes: 23 additions & 14 deletions src/vnsw/agent/oper/oper_dhcp_options.h
View file
Expand Up @@ -18,23 +18,28 @@ class OperDhcpOptions {
typedef std::vector<autogen::DhcpOptionType> DhcpOptionsList;
typedef std::vector<autogen::RouteType> HostOptionsList;

struct Subnet {
struct HostRoute {
Ip4Address prefix_;
uint32_t plen_;
Ip4Address gw_;

Subnet() : prefix_(), plen_(0) {}
bool operator<(const Subnet &rhs) const {
HostRoute() : prefix_(), plen_(0), gw_() {}
bool operator<(const HostRoute &rhs) const {
if (prefix_ != rhs.prefix_)
return prefix_ < rhs.prefix_;
return plen_ < rhs.plen_;
if (plen_ != rhs.plen_)
return plen_ < rhs.plen_;
return gw_ < rhs.gw_;
}
bool operator==(const Subnet &rhs) const {
return prefix_ == rhs.prefix_ && plen_ == rhs.plen_;
bool operator==(const HostRoute &rhs) const {
return prefix_ == rhs.prefix_ && plen_ == rhs.plen_ &&
gw_ == rhs.gw_;
}
std::string ToString() const {
char len[32];
snprintf(len, sizeof(len), "%u", plen_);
return prefix_.to_string() + "/" + std::string(len);
return prefix_.to_string() + "/" + std::string(len) +
" -> " + gw_.to_string();
}
};

Expand All @@ -46,7 +51,7 @@ class OperDhcpOptions {
virtual ~OperDhcpOptions() {}

const DhcpOptionsList &dhcp_options() const { return dhcp_options_; }
const std::vector<Subnet> &host_routes() const { return host_routes_; }
const std::vector<HostRoute> &host_routes() const { return host_routes_; }
void set_options(const DhcpOptionsList &options) { dhcp_options_ = options; }
void set_host_routes(const HostOptionsList &host_routes) {
host_routes_.clear();
Expand All @@ -55,14 +60,18 @@ class OperDhcpOptions {
void update_host_routes(const HostOptionsList &host_routes) {
host_routes_.clear();
for (unsigned int i = 0; i < host_routes.size(); ++i) {
Subnet subnet;
HostRoute host_route;
boost::system::error_code ec = Ip4PrefixParse(host_routes[i].prefix,
&subnet.prefix_,
(int *)&subnet.plen_);
if (ec || subnet.plen_ > 32) {
&host_route.prefix_,
(int *)&host_route.plen_);
if (ec || host_route.plen_ > 32) {
continue;
}
host_routes_.push_back(subnet);
host_route.gw_ = Ip4Address::from_string(host_routes[i].next_hop, ec);
if (ec) {
host_route.gw_ = Ip4Address();
}
host_routes_.push_back(host_route);
}
}

Expand All @@ -75,7 +84,7 @@ class OperDhcpOptions {

private:
DhcpOptionsList dhcp_options_;
std::vector<Subnet> host_routes_;
std::vector<HostRoute> host_routes_;
};

#endif // vnsw_agent_oper_dhcp_options_h_
4 changes: 2 additions & 2 deletions src/vnsw/agent/oper/vn.cc
View file
Expand Up @@ -122,7 +122,7 @@ AgentDBTable *VnEntry::DBToTable() const {
}

bool VnEntry::GetVnHostRoutes(const std::string &ipam,
std::vector<OperDhcpOptions::Subnet> *routes) const {
std::vector<OperDhcpOptions::HostRoute> *routes) const {
VnData::VnIpamDataMap::const_iterator it = vn_ipam_data_.find(ipam);
if (it != vn_ipam_data_.end()) {
*routes = it->second.oper_dhcp_options_.host_routes();
Expand Down Expand Up @@ -1140,7 +1140,7 @@ bool VnEntry::DBEntrySandesh(Sandesh *sresp, std::string &name) const {
it != vn_ipam_data_.end(); ++it) {
VnIpamHostRoutes vn_ipam_host_routes;
vn_ipam_host_routes.ipam_name = it->first;
const std::vector<OperDhcpOptions::Subnet> &host_routes =
const std::vector<OperDhcpOptions::HostRoute> &host_routes =
it->second.oper_dhcp_options_.host_routes();
for (uint32_t i = 0; i < host_routes.size(); ++i) {
vn_ipam_host_routes.host_routes.push_back(
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/vn.h
View file
Expand Up @@ -140,7 +140,7 @@ class VnEntry : AgentRefCount<VnEntry>, public AgentOperDBEntry {
const std::vector<VnIpam> &GetVnIpam() const { return ipam_; };
const VnIpam *GetIpam(const IpAddress &ip) const;
bool GetVnHostRoutes(const std::string &ipam,
std::vector<OperDhcpOptions::Subnet> *routes) const;
std::vector<OperDhcpOptions::HostRoute> *routes) const;
bool GetIpamName(const IpAddress &vm_addr, std::string *ipam_name) const;
bool GetIpamData(const IpAddress &vm_addr, std::string *ipam_name,
autogen::IpamType *ipam_type) const;
Expand Down
6 changes: 1 addition & 5 deletions src/vnsw/agent/services/dhcp_handler.cc
View file
Expand Up @@ -1104,11 +1104,7 @@ bool DhcpHandler::IsOptionRequested(uint8_t option) {

bool DhcpHandler::IsRouterOptionNeeded() {
// If GW is not configured, dont include
if (config_.gw_addr.is_unspecified())
return false;

// If router option is already included, nothing to do
if (is_flag_set(DHCP_OPTION_ROUTER))
if (config_.gw_addr.is_unspecified() && routers_.empty())
return false;

// When client requests Classless Static Routes option and this is
Expand Down
46 changes: 24 additions & 22 deletions src/vnsw/agent/services/dhcp_handler_base.cc
View file
Expand Up @@ -361,18 +361,22 @@ void DhcpHandlerBase::ReadClasslessRoute(uint32_t option, uint16_t opt_len,
while (value.good()) {
std::string snetstr;
value >> snetstr;
OperDhcpOptions::Subnet subnet;
boost::system::error_code ec = Ip4PrefixParse(snetstr, &subnet.prefix_,
(int *)&subnet.plen_);
if (ec || subnet.plen_ > 32 || !value.good()) {
OperDhcpOptions::HostRoute host_route;
boost::system::error_code ec = Ip4PrefixParse(snetstr, &host_route.prefix_,
(int *)&host_route.plen_);
if (ec || host_route.plen_ > 32 || !value.good()) {
DHCP_BASE_TRACE("Invalid Classless route DHCP option -"
"has to be list of <subnet/plen gw>");
break;
}
host_routes_.push_back(subnet);

// ignore the gw, as we use always use subnet's gw
value >> snetstr;
host_route.gw_ = Ip4Address::from_string(snetstr, ec);
if (ec) {
host_route.gw_ = Ip4Address();
}

host_routes_.push_back(host_route);
}
}

Expand All @@ -385,7 +389,7 @@ void DhcpHandlerBase::ReadClasslessRoute(uint32_t option, uint16_t opt_len,
// 6) options at IPAM level (classless routes from IPAM dhcp options)
// Add the options defined at the highest level in priority
uint16_t DhcpHandlerBase::AddClasslessRouteOption(uint16_t opt_len) {
std::vector<OperDhcpOptions::Subnet> host_routes;
std::vector<OperDhcpOptions::HostRoute> host_routes;
do {
// Port specific host route options
// TODO: should we remove host routes at port level from schema ?
Expand Down Expand Up @@ -429,18 +433,10 @@ uint16_t DhcpHandlerBase::AddClasslessRouteOption(uint16_t opt_len) {
}

// IPAM level host route options
const std::vector<autogen::RouteType> &routes =
ipam_type_.host_routes.route;
for (unsigned int i = 0; i < routes.size(); ++i) {
OperDhcpOptions::Subnet subnet;
boost::system::error_code ec = Ip4PrefixParse(routes[i].prefix,
&subnet.prefix_,
(int *)&subnet.plen_);
if (ec || subnet.plen_ > 32) {
continue;
}
host_routes.push_back(subnet);
}
OperDhcpOptions oper_dhcp_options;
oper_dhcp_options.update_host_routes(ipam_type_.host_routes.route);
host_routes = oper_dhcp_options.host_routes();

// Host route options from IPAM level DHCP options
if (!host_routes.size() && host_routes_.size()) {
host_routes.swap(host_routes_);
Expand All @@ -455,13 +451,18 @@ uint16_t DhcpHandlerBase::AddClasslessRouteOption(uint16_t opt_len) {
for (uint32_t i = 0; i < host_routes.size(); ++i) {
uint32_t prefix = host_routes[i].prefix_.to_ulong();
uint32_t plen = host_routes[i].plen_;
uint32_t gw = host_routes[i].gw_.to_ulong();
*ptr++ = plen;
len++;
for (unsigned int i = 0; plen && i <= (plen - 1) / 8; ++i) {
*ptr++ = (prefix >> 8 * (3 - i)) & 0xFF;
for (unsigned int j = 0; plen && j <= (plen - 1) / 8; ++j) {
*ptr++ = (prefix >> 8 * (3 - j)) & 0xFF;
len++;
}
*(uint32_t *)ptr = htonl(config_.gw_addr.to_v4().to_ulong());
// if GW is not specified, set it to subnet's GW
if (gw)
*(uint32_t *)ptr = htonl(gw);
else
*(uint32_t *)ptr = htonl(config_.gw_addr.to_v4().to_ulong());
ptr += sizeof(uint32_t);
len += sizeof(uint32_t);
}
Expand Down Expand Up @@ -559,6 +560,7 @@ uint16_t DhcpHandlerBase::AddDhcpOptions(
if (option == DHCP_OPTION_ROUTER) {
// Router option is added later
routers_ = options[i].dhcp_option_value;
set_flag(DHCP_OPTION_ROUTER);
} else {
opt_len = AddIpv4Option(option, opt_len,
options[i].dhcp_option_value,
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/services/dhcp_handler_base.h
View file
Expand Up @@ -149,7 +149,7 @@ class DhcpHandlerBase : public ProtoHandler {
std::string routers_;

// parse and store the host routes coming in config
std::vector<OperDhcpOptions::Subnet> host_routes_;
std::vector<OperDhcpOptions::HostRoute> host_routes_;
DhcpOptionLevel host_routes_level_;

std::string ipam_name_;
Expand Down
44 changes: 26 additions & 18 deletions src/vnsw/agent/services/test/dhcp_test.cc
View file
Expand Up @@ -31,17 +31,17 @@
#define CLIENT_REQ_IP "1.2.3.4"
#define CLIENT_REQ_PREFIX "1.2.3.0"
#define CLIENT_REQ_GW "1.2.3.1"
#define MAX_WAIT_COUNT 1000
#define MAX_WAIT_COUNT 3000
#define BUF_SIZE 8192
MacAddress src_mac(0x00, 0x01, 0x02, 0x03, 0x04, 0x05);
MacAddress dest_mac(0x00, 0x11, 0x12, 0x13, 0x14, 0x15);
#define DHCP_RESPONSE_STRING "Server : 1.1.1.200; Lease time : 4294967295; Subnet mask : 255.255.255.0; Broadcast : 1.1.1.255; Gateway : 1.1.1.200; Host Name : vm1; DNS : 1.1.1.200; Domain Name : test.contrail.juniper.net; "
#define HOST_ROUTE_STRING "Host Routes : 10.1.1.0/24 -> 1.1.1.200;10.1.2.0/24 -> 1.1.1.200;150.25.75.0/24 -> 1.1.1.200;192.168.1.128/28 -> 1.1.1.200;"
#define HOST_ROUTE_STRING "Host Routes : 10.1.1.0/24 -> 1.1.1.200;10.1.2.0/24 -> 1.1.1.200;150.25.75.0/24 -> 150.25.75.254;192.168.1.128/28 -> 1.1.1.200;"
#define CHANGED_HOST_ROUTE_STRING "Host Routes : 150.2.2.0/24 -> 1.1.1.200;192.1.1.1/28 -> 1.1.1.200;"
#define IPAM_DHCP_OPTIONS_STRING "DNS : 1.2.3.4; Domain Name : test.com; Time Server : 3.2.14.5"
#define SUBNET_DHCP_OPTIONS_STRING "DNS : 11.12.13.14; Domain Name : subnet.com; Time Server : 3.2.14.5;"
#define SUBNET_DHCP_OPTIONS_STRING "DNS : 11.12.13.14; Domain Name : subnet.com; Time Server : 3.2.14.5; Host Routes : 10.1.1.0/24 -> 1.1.1.200;10.1.2.0/24 -> 1.1.1.200;150.25.75.0/24 -> 150.25.75.254;192.168.1.128/28 -> 1.1.1.200;Gateway : 1.2.3.4; Gateway : 5.6.7.8; Gateway : 1.1.1.200;"
#define PORT_DHCP_OPTIONS_STRING "DNS : 21.22.23.24; Time Server : 13.12.14.15; Domain Name : test.com;"
#define PORT_HOST_ROUTE_STRING "Host Routes : 99.2.3.0/24 -> 1.1.1.200;99.5.0.0/16 -> 1.1.1.200;"
#define PORT_HOST_ROUTE_STRING "Host Routes : 99.2.3.0/24 -> 1.1.1.200;99.5.0.0/16 -> 99.5.0.1;"

#define DHCP_CHECK(condition) \
do { \
Expand Down Expand Up @@ -1093,9 +1093,9 @@ TEST_F(DhcpTest, IpamSpecificDhcpOptions) {
</dhcp-option-list>\
<host-routes>\
<route><prefix>10.1.1.0/24</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>10.1.2.0/24</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>150.25.75.0/24</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>192.168.1.128/28</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>10.1.2.0/24</prefix> <next-hop>junk</next-hop> <next-hop-type /></route>\
<route><prefix>150.25.75.0/24</prefix> <next-hop>150.25.75.254</next-hop> <next-hop-type /></route>\
<route><prefix>192.168.1.128/28</prefix> <next-hop>0.0.0.0</next-hop> <next-hop-type /></route>\
</host-routes>\
</network-ipam-mgmt>";

Expand Down Expand Up @@ -1204,10 +1204,14 @@ TEST_F(DhcpTest, SubnetSpecificDhcpOptions) {
<dhcp-option-name>4</dhcp-option-name>\
<dhcp-option-value>3.2.14.5</dhcp-option-value>\
</dhcp-option>\
<dhcp-option>\
<dhcp-option-name>3</dhcp-option-name>\
<dhcp-option-value>12.13.14.15 23.24.25.26</dhcp-option-value>\
</dhcp-option>\
</dhcp-option-list>\
<host-routes>\
<route><prefix>1.2.3.0/24</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>4.5.0.0/16</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>4.5.0.0/16</prefix> <next-hop>4.5.0.254</next-hop> <next-hop-type /></route>\
</host-routes>\
</network-ipam-mgmt>";
char add_subnet_tags[] =
Expand All @@ -1220,14 +1224,18 @@ TEST_F(DhcpTest, SubnetSpecificDhcpOptions) {
<dhcp-option-name>15</dhcp-option-name>\
<dhcp-option-value>subnet.com</dhcp-option-value>\
</dhcp-option>\
<dhcp-option>\
<dhcp-option-name>3</dhcp-option-name>\
<dhcp-option-value>1.2.3.4 5.6.7.8</dhcp-option-value>\
</dhcp-option>\
</dhcp-option-list>";
// option 4 from ipam and other options from subnet should be used

std::vector<std::string> vm_host_routes;
vm_host_routes.push_back("10.1.1.0/24");
vm_host_routes.push_back("10.1.2.0/24");
vm_host_routes.push_back("150.25.75.0/24");
vm_host_routes.push_back("192.168.1.128/28");
vm_host_routes.push_back("10.1.2.0/24 junk");
vm_host_routes.push_back("150.25.75.0/24 150.25.75.254");
vm_host_routes.push_back("192.168.1.128/28 0.0.0.0");

CreateVmportEnv(input, 2, 0);
client->WaitForIdle();
Expand Down Expand Up @@ -1309,8 +1317,8 @@ TEST_F(DhcpTest, PortSpecificDhcpOptions) {
</dhcp-option>\
</dhcp-option-list>\
<host-routes>\
<route><prefix>1.2.3.0/24</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>4.5.0.0/16</prefix> <next-hop /> <next-hop-type /></route>\
<route><prefix>1.2.3.0/24</prefix> <next-hop>0.0.0.0</next-hop> <next-hop-type /></route>\
<route><prefix>4.5.0.0/16</prefix> <next-hop>4.5.0.254</next-hop> <next-hop-type /></route>\
</host-routes>\
</network-ipam-mgmt>";

Expand All @@ -1334,8 +1342,8 @@ TEST_F(DhcpTest, PortSpecificDhcpOptions) {
</dhcp-option>\
</virtual-machine-interface-dhcp-option-list>\
<virtual-machine-interface-host-routes>\
<route><prefix>99.2.3.0/24</prefix> <next-hop /> <next-hop-type /> </route>\
<route><prefix>99.5.0.0/16</prefix> <next-hop /> <next-hop-type /> </route>\
<route><prefix>99.2.3.0/24</prefix> <next-hop>0.0.0.0</next-hop> <next-hop-type /> </route>\
<route><prefix>99.5.0.0/16</prefix> <next-hop>99.5.0.1</next-hop> <next-hop-type /> </route>\
</virtual-machine-interface-host-routes>";
// option 6 from vm interface, option 4 from subnet and option 15
// from ipam should be used
Expand Down Expand Up @@ -1911,11 +1919,11 @@ TEST_F(DhcpTest, ClasslessOption) {
"<virtual-machine-interface-dhcp-option-list>\
<dhcp-option>\
<dhcp-option-name>classless-static-routes</dhcp-option-name>\
<dhcp-option-value>10.1.2.0/24 1.2.3.4 20.20.20.0/24 20.20.20.1</dhcp-option-value>\
<dhcp-option-value>10.1.2.0/24 0.0.0.0 20.20.20.0/24 20.20.20.1</dhcp-option-value>\
</dhcp-option>\
</virtual-machine-interface-dhcp-option-list>";

#define OPTION_CATEGORY_CLASSLESS "Host Routes : 10.1.2.0/24 -> 1.1.1.200;20.20.20.0/24 -> 1.1.1.200;"
#define OPTION_CATEGORY_CLASSLESS "Host Routes : 10.1.2.0/24 -> 1.1.1.200;20.20.20.0/24 -> 20.20.20.1;"
DhcpOptionCategoryTest(vm_interface_attr, true, OPTION_CATEGORY_CLASSLESS,
false, "");
}
Expand All @@ -1926,7 +1934,7 @@ TEST_F(DhcpTest, ClasslessOptionError) {
"<virtual-machine-interface-dhcp-option-list>\
<dhcp-option>\
<dhcp-option-name>classless-static-routes</dhcp-option-name>\
<dhcp-option-value>10.1.2.0/24 1.2.3.4 abcd</dhcp-option-value>\
<dhcp-option-value>10.1.2.0/24 0.0.0.0 abcd</dhcp-option-value>\
</dhcp-option>\
<dhcp-option>\
<dhcp-option-name>classless-static-routes</dhcp-option-name>\
Expand Down
17 changes: 14 additions & 3 deletions src/vnsw/agent/test/test_util.cc
View file
Expand Up @@ -237,9 +237,20 @@ void AddNodeString(char *buff, int &len, const char *nodename, const char *name,
if (vm_host_routes && vm_host_routes->size()) {
str << " <host-routes>\n";
for (unsigned int i = 0; i < vm_host_routes->size(); i++) {
std::string prefix, nexthop;
std::vector<std::string> tokens;
boost::split(tokens, (*vm_host_routes)[i], boost::is_any_of(" \t"));
vector<string>::iterator it = tokens.begin();
if (it != tokens.end()) {
prefix = *it;
it++;
}
if (it != tokens.end()) {
nexthop = *it;
}
str << " <route>\n";
str << " <prefix>" << (*vm_host_routes)[i] << "</prefix>\n";
str << " <next-hop />\n";
str << " <prefix>" << prefix << "</prefix>\n";
str << " <next-hop>" << nexthop << "</next-hop>\n";
str << " <next-hop-type />\n";
str << " </route>\n";
}
Expand Down Expand Up @@ -1830,7 +1841,7 @@ void DelVmPortVrf(const char *name) {
void AddIPAM(const char *name, IpamInfo *ipam, int ipam_size, const char *ipam_attr,
const char *vdns_name, const std::vector<std::string> *vm_host_routes,
const char *add_subnet_tags) {
char buff[8192];
char buff[10240];
char node_name[128];
char ipam_name[128];
int len = 0;
Expand Down

0 comments on commit bad108e

Please sign in to comment.