Skip to content

Commit

Permalink
changing internal representation of ip addresses from string to binary
Browse files Browse the repository at this point in the history 
as stated. currently katran is exposed to capslock like issues.
e.g. we could add same v6 address multiple times and that would pass all the tests
(e.g. real fc00::1 fc00:0::1 fc00:0:0::1 etc).
current behavior wont break anything, as in forwarding plane everything
is in binary anyway, but we would consume more resources than needed.
(e.g. real for regular vip could in any format, but quic's real
in exploded)

tested by:
- it compiles (c)
- unittests
- katran_tester

---
1. this is taking care only about real's addresses, struct VipKey still has
vip's address as a string. so it would take more work to modify that as well
(as VipKey is a public API. current changes wont break anything external
as API's return values have not changed. only internal's ones)

2. if i run clang-format on KatranLb.h/cpp it makes some changes
in unrelated lines (most likely either format from 8.0 has some modifications
or it was not run on that files for a while). what would you prefer?
would you accept diff w/ some unrelated changed caused by clang format?
  • Loading branch information
@tehnerd
tehnerd committed Aug 2, 2019
1 parent b91de66 commit f6d9ebf
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 64 deletions.
25 changes: 16 additions & 9 deletions katran/lib/IpHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#include "IpHelpers.h"

#include <folly/IPAddress.h>
#include <folly/lang/Bits.h>
#include <stdexcept>

Expand All @@ -25,24 +24,28 @@ namespace katran {
constexpr int Uint32_bytes = 4;
constexpr uint8_t V6DADDR = 1;

struct beaddr IpHelpers::parseAddrToBe(
const std::string& addr,
bool bigendian) {
struct beaddr IpHelpers::parseAddrToBe(const std::string &addr,
bool bigendian) {
auto ipaddr = folly::IPAddress(addr);
return parseAddrToBe(ipaddr, bigendian);
}

struct beaddr IpHelpers::parseAddrToBe(const folly::IPAddress &addr,
bool bigendian) {
struct beaddr translated_addr = {};
if (ipaddr.isV4()) {
if (addr.isV4()) {
translated_addr.flags = 0;
if (bigendian) {
translated_addr.daddr = ipaddr.asV4().toLong();
translated_addr.daddr = addr.asV4().toLong();
} else {
translated_addr.daddr = ipaddr.asV4().toLongHBO();
translated_addr.daddr = addr.asV4().toLongHBO();
}
} else {
for (int partition = 0; partition < 4; partition++) {
// bytes() return a ptr to char* array
// so we are doing some ptr arithmetics here
uint32_t addr_part =
*(uint32_t*)(ipaddr.bytes() + Uint32_bytes * partition);
*(uint32_t *)(addr.bytes() + Uint32_bytes * partition);
if (bigendian) {
translated_addr.v6daddr[partition] = addr_part;
} else {
Expand All @@ -54,7 +57,11 @@ struct beaddr IpHelpers::parseAddrToBe(
return translated_addr;
};

struct beaddr IpHelpers::parseAddrToInt(const std::string& addr) {
struct beaddr IpHelpers::parseAddrToInt(const std::string &addr) {
return parseAddrToBe(addr, false);
};

struct beaddr IpHelpers::parseAddrToInt(const folly::IPAddress &addr) {
return parseAddrToBe(addr, false);
};

Expand Down
14 changes: 9 additions & 5 deletions katran/lib/IpHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#pragma once

#include <folly/IPAddress.h>
#include <string>

namespace katran {
Expand All @@ -33,7 +34,7 @@ struct beaddr {
};

class IpHelpers {
public:
public:
/**
* @param const string addr address to translate
* @return struct beaddr representation of given address
Expand All @@ -42,10 +43,13 @@ class IpHelpers {
* of beaddr structure. this function could throw, if given string is not
* an ip address.
*/
static struct beaddr parseAddrToBe(
const std::string& addr,
bool bigendian = true);
static struct beaddr parseAddrToInt(const std::string& addr);
static struct beaddr parseAddrToBe(const std::string &addr,
bool bigendian = true);
static struct beaddr parseAddrToInt(const std::string &addr);

static struct beaddr parseAddrToBe(const folly::IPAddress &addr,
bool bigendian = true);
static struct beaddr parseAddrToInt(const folly::IPAddress &addr);
};

} // namespace katran
95 changes: 58 additions & 37 deletions katran/lib/KatranLb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ AddressType KatranLb::validateAddress(
return AddressType::NETWORK;
}
}
lbStats_.addrValidationFailed++;
LOG(ERROR) << "Invalid address: " << addr;
return AddressType::INVALID;
}
Expand Down Expand Up @@ -566,6 +567,7 @@ bool KatranLb::modifyRealsForVip(
LOG(ERROR) << "Invalid real's address: " << real.address;
continue;
}
auto raddr = folly::IPAddress(real.address);
VLOG(4) << folly::format(
"modifying real: {} with weight {} for vip {}:{}:{}",
real.address,
Expand All @@ -575,7 +577,7 @@ bool KatranLb::modifyRealsForVip(
vip.proto);

if (action == ModifyAction::DEL) {
auto real_iter = reals_.find(real.address);
auto real_iter = reals_.find(raddr);
if (real_iter == reals_.end()) {
LOG(INFO) << "trying to delete non-existing real";
continue;
Expand All @@ -588,28 +590,28 @@ bool KatranLb::modifyRealsForVip(
continue;
}
ureal.updatedReal.num = real_iter->second.num;
decreaseRefCountForReal(real.address);
decreaseRefCountForReal(raddr);
} else {
auto real_iter = reals_.find(real.address);
auto real_iter = reals_.find(raddr);
if (real_iter != reals_.end()) {
if (std::find(
cur_reals.begin(), cur_reals.end(), real_iter->second.num) ==
cur_reals.end()) {
// increment ref count if it's a new real for this vip
increaseRefCountForReal(real.address);
increaseRefCountForReal(raddr);
cur_reals.push_back(real_iter->second.num);
}
ureal.updatedReal.num = real_iter->second.num;
} else {
auto rnum = increaseRefCountForReal(real.address);
auto rnum = increaseRefCountForReal(raddr);
if (rnum == config_.maxReals) {
LOG(INFO) << "exhausted real's space";
continue;
}
ureal.updatedReal.num = rnum;
}
ureal.updatedReal.weight = real.weight;
ureal.updatedReal.hash = folly::IPAddress(real.address).hash();
ureal.updatedReal.hash = raddr.hash();
}
ureals.push_back(ureal);
}
Expand Down Expand Up @@ -642,19 +644,21 @@ std::vector<NewReal> KatranLb::getRealsForVip(const VipKey& vip) {
int i = 0;
for (auto real_id : vip_reals_ids) {
reals[i].weight = real_id.weight;
reals[i].address = numToReals_[real_id.num];
reals[i].address = numToReals_[real_id.num].str();
++i;
}
return reals;
}

int64_t KatranLb::getIndexForReal(const std::string& real) {
auto real_iter = reals_.find(real);
if (real_iter == reals_.end()) {
return kError;
} else {
return real_iter->second.num;
if (validateAddress(real) != AddressType::INVALID) {
auto raddr = folly::IPAddress(real);
auto real_iter = reals_.find(raddr);
if (real_iter != reals_.end()) {
return real_iter->second.num;
}
}
return kError;
}

int KatranLb::addSrcRoutingRule(
Expand Down Expand Up @@ -713,7 +717,7 @@ int KatranLb::addSrcRoutingRule(
// no point to continue. bailing out
return kError;
}
auto rnum = increaseRefCountForReal(dst);
auto rnum = increaseRefCountForReal(folly::IPAddress(dst));
if (rnum == config_.maxReals) {
LOG(ERROR) << "exhausted real's space";
// all src using same dst. no point to continue if we can't add this dst
Expand Down Expand Up @@ -794,7 +798,7 @@ std::unordered_map<std::string, std::string> KatranLb::getSrcRoutingRule() {
auto real = numToReals_[src.second];
auto src_network =
folly::sformat("{}/{}", src.first.first.str(), src.first.second);
src_mapping[src_network] = real;
src_mapping[src_network] = real.str();
}
return src_mapping;
}
Expand All @@ -808,11 +812,19 @@ KatranLb::getSrcRoutingRuleCidr() {
}
for (auto& src : lpmSrcMapping_) {
auto real = numToReals_[src.second];
src_mapping[src.first] = real;
src_mapping[src.first] = real.str();
}
return src_mapping;
}

const std::unordered_map<uint32_t, std::string> KatranLb::getNumToRealMap() {
std::unordered_map<uint32_t, std::string> reals;
for (auto& real: numToReals_) {
reals[real.first] = real.second.str();
}
return reals;
}

bool KatranLb::stopKatranMonitor() {
if (!features_.introspection) {
return false;
Expand Down Expand Up @@ -910,7 +922,8 @@ bool KatranLb::addInlineDecapDst(const std::string& dst) {
LOG(ERROR) << "invalid decap destination address: " << dst;
return false;
}
if (decapDsts_.find(dst) != decapDsts_.end()) {
auto daddr = folly::IPAddress(dst);
if (decapDsts_.find(daddr) != decapDsts_.end()) {
LOG(ERROR) << "trying to add already existing decap dst";
return false;
}
Expand All @@ -919,9 +932,9 @@ bool KatranLb::addInlineDecapDst(const std::string& dst) {
return false;
}
VLOG(2) << "adding decap dst " << dst;
decapDsts_.insert(dst);
decapDsts_.insert(daddr);
if (!config_.testing) {
modifyDecapDst(ModifyAction::ADD, dst);
modifyDecapDst(ModifyAction::ADD, daddr);
}
return true;
}
Expand All @@ -931,30 +944,35 @@ bool KatranLb::delInlineDecapDst(const std::string& dst) {
LOG(ERROR) << "source based routing is not enabled in forwarding plane";
return false;
}
auto dst_iter = decapDsts_.find(dst);
if (validateAddress(dst) == AddressType::INVALID) {
LOG(ERROR) << "provided address in invalid format: " << dst;
return false;
}
auto daddr = folly::IPAddress(dst);
auto dst_iter = decapDsts_.find(daddr);
if (dst_iter == decapDsts_.end()) {
LOG(ERROR) << "trying to delete non-existing decap dst " << dst;
return false;
}
VLOG(2) << "deleting decap dst " << dst;
decapDsts_.erase(dst_iter);
if (!config_.testing) {
modifyDecapDst(ModifyAction::DEL, dst);
modifyDecapDst(ModifyAction::DEL, daddr);
}
return true;
}

std::vector<std::string> KatranLb::getInlineDecapDst() {
std::vector<std::string> dsts;
for (auto& dst : decapDsts_) {
dsts.push_back(dst);
dsts.push_back(dst.str());
}
return dsts;
}

bool KatranLb::modifyDecapDst(
ModifyAction action,
const std::string& dst,
const folly::IPAddress& dst,
uint32_t flags) {
auto addr = IpHelpers::parseAddrToBe(dst);
if (action == ModifyAction::ADD) {
Expand Down Expand Up @@ -992,15 +1010,16 @@ void KatranLb::modifyQuicRealsMapping(
continue;
}
VLOG(4) << folly::sformat("modifying quic's real {}", real.address);
auto real_iter = quicMapping_.find(real.address);
auto raddr = folly::IPAddress(real.address);
auto real_iter = quicMapping_.find(raddr);
if (action == ModifyAction::DEL) {
if (real_iter == quicMapping_.end()) {
LOG(ERROR) << folly::sformat(
"trying to delete nonexisting mapping for address {}",
real.address);
continue;
}
decreaseRefCountForReal(real.address);
decreaseRefCountForReal(raddr);
quicMapping_.erase(real_iter);
} else {
if (real_iter != quicMapping_.end()) {
Expand All @@ -1009,13 +1028,13 @@ void KatranLb::modifyQuicRealsMapping(
// or we could silently delete old mapping instead.
continue;
}
auto rnum = increaseRefCountForReal(real.address);
auto rnum = increaseRefCountForReal(raddr);
if (rnum == config_.maxReals) {
LOG(ERROR) << "exhausted real's space";
continue;
}
to_update[real.id] = rnum;
quicMapping_[real.address] = real.id;
quicMapping_[raddr] = real.id;
}
}
if (!config_.testing) {
Expand All @@ -1038,7 +1057,7 @@ std::vector<QuicReal> KatranLb::getQuicRealsMapping() {
std::vector<QuicReal> reals;
QuicReal real;
for (auto& mapping : quicMapping_) {
real.address = mapping.first;
real.address = mapping.first.str();
real.id = mapping.second;
reals.push_back(real);
}
Expand Down Expand Up @@ -1119,7 +1138,7 @@ bool KatranLb::addHealthcheckerDst(
}
VLOG(4) << folly::format(
"adding healtcheck with so_mark {} to dst {}", somark, dst);

auto hcaddr = folly::IPAddress(dst);
uint32_t key = somark;
beaddr addr;

Expand All @@ -1128,13 +1147,12 @@ bool KatranLb::addHealthcheckerDst(
LOG(INFO) << "healthchecker's reals space exhausted";
return false;
}
folly::IPAddress dst_addr(dst);
// for md bassed tunnels remote_ipv4 must be in host endian format
// and v6 in be
if (dst_addr.isV4()) {
addr = IpHelpers::parseAddrToInt(dst);
if (hcaddr.isV4()) {
addr = IpHelpers::parseAddrToInt(hcaddr);
} else {
addr = IpHelpers::parseAddrToBe(dst);
addr = IpHelpers::parseAddrToBe(hcaddr);
}
if (!config_.testing) {
auto res = bpfAdapter_.bpfUpdateMap(
Expand All @@ -1145,7 +1163,7 @@ bool KatranLb::addHealthcheckerDst(
return false;
}
}
hcReals_[somark] = dst;
hcReals_[somark] = hcaddr;
return true;
}

Expand Down Expand Up @@ -1177,7 +1195,10 @@ bool KatranLb::delHealthcheckerDst(const uint32_t somark) {

std::unordered_map<uint32_t, std::string> KatranLb::getHealthcheckersDst() {
// would be empty map in case if enableHc_ is false
std::unordered_map<uint32_t, std::string> hcs(hcReals_);
std::unordered_map<uint32_t, std::string> hcs;
for (auto& hc : hcReals_) {
hcs[hc.first] = hc.second.str();
}
return hcs;
}

Expand Down Expand Up @@ -1214,7 +1235,7 @@ bool KatranLb::updateVipMap(
return true;
}

bool KatranLb::updateRealsMap(const std::string& real, uint32_t num) {
bool KatranLb::updateRealsMap(const folly::IPAddress& real, uint32_t num) {
auto real_addr = IpHelpers::parseAddrToBe(real);
auto res = bpfAdapter_.bpfUpdateMap(
bpfAdapter_.getMapFdByName("reals"), &num, &real_addr);
Expand All @@ -1227,7 +1248,7 @@ bool KatranLb::updateRealsMap(const std::string& real, uint32_t num) {
}
};

void KatranLb::decreaseRefCountForReal(const std::string& real) {
void KatranLb::decreaseRefCountForReal(const folly::IPAddress& real) {
auto real_iter = reals_.find(real);
if (real_iter == reals_.end()) {
// it's expected that caller must call this function only after explicit
Expand All @@ -1244,7 +1265,7 @@ void KatranLb::decreaseRefCountForReal(const std::string& real) {
}
}

uint32_t KatranLb::increaseRefCountForReal(const std::string& real) {
uint32_t KatranLb::increaseRefCountForReal(const folly::IPAddress& real) {
auto real_iter = reals_.find(real);
if (real_iter != reals_.end()) {
real_iter->second.refCount++;
Expand Down
Loading

0 comments on commit f6d9ebf

Please sign in to comment.