Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changing internal representation of ip addresses for reals from string to binary #37

Closed
wants to merge 1 commit into from
Closed

changing internal representation of ip addresses for reals from string to binary #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions katran/lib/IpHelpers.cpp
View file
Open in desktop
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,27 @@ namespace katran {
constexpr int Uint32_bytes = 4;
constexpr uint8_t V6DADDR = 1;

struct beaddr IpHelpers::parseAddrToBe(
const std::string& addr,
bool bigendian) {
auto ipaddr = folly::IPAddress(addr);
struct beaddr IpHelpers::parseAddrToBe(const std::string &addr,
bool bigendian) {
return parseAddrToBe(folly::IPAddress(addr), 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 +56,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
Open in desktop
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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove the space

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format -i . looks like fb's one is too old

/**
* @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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence tho..

* 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all 4 of them can be free functions instead of static functions on the class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the two with bigendian params can be in a anonymous namespace so they are impl detail only local to this file.

Copy link
Contributor Author

@tehnerd tehnerd Aug 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non static non class functions would break public API - do you guys want to do this? (e.g. if anyone already using em otuside of KatranLb.h)

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
Loading