From 9a58dd347ae6f991b2f3b191eafae2c83d1e5a45 Mon Sep 17 00:00:00 2001 From: Sebastiano Miano Date: Tue, 29 Jan 2019 11:52:10 +0100 Subject: [PATCH 1/2] Remove bpf_ktime from simplebridge and use custom thread This commit removes the bpf_ktime() helper from the Simplebridge used to remove old entries from the filtering database. This helper introduces a performance degradation that could be removed by using a custom thread that updates a percpu map with the timestamp at the same way it is done in the Iptables service. Signed-off-by: Sebastiano Miano --- .../pcn-simplebridge/src/FdbEntry.cpp | 17 ++++----- src/services/pcn-simplebridge/src/FdbEntry.h | 6 +-- .../pcn-simplebridge/src/Simplebridge.cpp | 37 ++++++++++++++++++- .../pcn-simplebridge/src/Simplebridge.h | 8 ++++ .../pcn-simplebridge/src/Simplebridge_dp.c | 26 +++++++------ 5 files changed, 67 insertions(+), 27 deletions(-) diff --git a/src/services/pcn-simplebridge/src/FdbEntry.cpp b/src/services/pcn-simplebridge/src/FdbEntry.cpp index 25246d02b..9b6925e18 100644 --- a/src/services/pcn-simplebridge/src/FdbEntry.cpp +++ b/src/services/pcn-simplebridge/src/FdbEntry.cpp @@ -28,7 +28,7 @@ FdbEntry::FdbEntry(Fdb &parent, const FdbEntryJsonObject &conf): parent_(parent) logger()->info("Creating FdbEntry instance"); } -FdbEntry::FdbEntry(Fdb &parent, const std::string &address, uint64_t entry_age, uint32_t out_port) : +FdbEntry::FdbEntry(Fdb &parent, const std::string &address, uint32_t entry_age, uint32_t out_port) : parent_(parent), address_(address), entry_age_(entry_age), port_no_(out_port) { auto port = parent.parent_.get_port(port_no_); @@ -74,12 +74,10 @@ void FdbEntry::create(Fdb &parent, const std::string &address, const FdbEntryJso struct timespec now_timespec; clock_gettime(CLOCK_MONOTONIC, &now_timespec); - const uint64_t SEC2NANOSEC = 1000000000ULL; - uint64_t now = now_timespec.tv_sec*SEC2NANOSEC + now_timespec.tv_nsec; uint64_t key = utils::mac_string_to_be_uint(address); fwd_entry value { - .timestamp = now, + .timestamp = (uint32_t) now_timespec.tv_sec, .port = port_index, }; @@ -97,22 +95,21 @@ std::shared_ptr FdbEntry::constructFromMap(Fdb &parent, const std::str const fwd_entry &value) { struct timespec now_timespec; clock_gettime(CLOCK_MONOTONIC, &now_timespec); - const uint64_t SEC2NANOSEC = 1000000000ULL; - uint64_t now = now_timespec.tv_sec*SEC2NANOSEC + now_timespec.tv_nsec; + uint32_t now = now_timespec.tv_sec; uint64_t key = utils::mac_string_to_be_uint(address); - uint64_t timestamp = value.timestamp; + uint32_t timestamp = value.timestamp; //Here I'm going to check if the entry in the filtering database is too old.\ //In this case, I'll not show the entry - if ((now - timestamp) > parent.getAgingTime()*SEC2NANOSEC) { + if ((now - timestamp) > parent.getAgingTime()) { parent.logger()->debug("Ignoring old entry: now {0}, last_seen: {1}", - now/SEC2NANOSEC, timestamp/SEC2NANOSEC); + now, timestamp); auto fwdtable = parent.parent_.get_hash_table("fwdtable"); fwdtable.remove(key); return nullptr; } - uint64_t entry_age = (now - timestamp)/SEC2NANOSEC; + uint32_t entry_age = now - timestamp; uint32_t port_no = value.port; return std::make_shared(parent, address, entry_age, port_no); } diff --git a/src/services/pcn-simplebridge/src/FdbEntry.h b/src/services/pcn-simplebridge/src/FdbEntry.h index 4f468c122..bfabfb3bb 100644 --- a/src/services/pcn-simplebridge/src/FdbEntry.h +++ b/src/services/pcn-simplebridge/src/FdbEntry.h @@ -27,7 +27,7 @@ class Fdb; /* definitions copied from datapath */ struct fwd_entry { - uint64_t timestamp; + uint32_t timestamp; uint32_t port; } __attribute__((packed)); @@ -36,7 +36,7 @@ using namespace io::swagger::server::model; class FdbEntry : public FdbEntryInterface { public: FdbEntry(Fdb &parent, const FdbEntryJsonObject &conf); - FdbEntry(Fdb &parent, const std::string &address, uint64_t entry_age, uint32_t out_port); + FdbEntry(Fdb &parent, const std::string &address, uint32_t entry_age, uint32_t out_port); virtual ~FdbEntry(); static void create(Fdb &parent, const std::string &address, const FdbEntryJsonObject &conf); @@ -77,7 +77,7 @@ class FdbEntry : public FdbEntryInterface { std::string address_; std::string port_name_; - uint64_t entry_age_; + uint32_t entry_age_; uint32_t port_no_; }; diff --git a/src/services/pcn-simplebridge/src/Simplebridge.cpp b/src/services/pcn-simplebridge/src/Simplebridge.cpp index 290e9d921..36a9715d2 100644 --- a/src/services/pcn-simplebridge/src/Simplebridge.cpp +++ b/src/services/pcn-simplebridge/src/Simplebridge.cpp @@ -23,23 +23,57 @@ #include #include +#include using namespace Tins; Simplebridge::Simplebridge(const std::string name, const SimplebridgeJsonObject &conf, CubeType type) - : Cube(name, {generate_code()}, {}, type, conf.getPolycubeLoglevel()) { + : Cube(name, {generate_code()}, {}, type, conf.getPolycubeLoglevel()), + quit_thread_(false) { logger()->set_pattern("[%Y-%m-%d %H:%M:%S.%e] [Simplebridge] [%n] [%l] %v"); logger()->info("Creating Simplebridge instance"); addFdb(conf.getFdb()); addPortsList(conf.getPorts()); + + timestamp_update_thread_ = std::thread(&Simplebridge::updateTimestampTimer, this); } Simplebridge::~Simplebridge() { // we are destroying this service, prepare for it. + quitAndJoin(); Cube::dismount(); } +void Simplebridge::quitAndJoin() { + quit_thread_ = true; + timestamp_update_thread_.join(); +} + +void Simplebridge::updateTimestampTimer() { + do { + sleep(1); + updateTimestamp(); + } while (!quit_thread_); +} + +/* + * This method is in charge of updating the timestamp table + * that is used in the dataplane to avoid calling the bpf_ktime helper + * that introduces a non-negligible overhead to the eBPF program. + */ +void Simplebridge::updateTimestamp() { + try { + // get timestamp from system + struct timespec now_timespec; + clock_gettime(CLOCK_MONOTONIC, &now_timespec); + auto timestamp_table = get_array_table("timestamp"); + timestamp_table.set(0, now_timespec.tv_sec); + } catch (...) { + logger()->error("Error while updating the timestamp table"); + } +} + void Simplebridge::update(const SimplebridgeJsonObject &conf) { //This method updates all the object/parameter in Simplebridge object specified in the conf JsonObject. //You can modify this implementation. @@ -141,4 +175,3 @@ void Simplebridge::reloadCodeWithAgingtime(uint32_t aging_time) { - diff --git a/src/services/pcn-simplebridge/src/Simplebridge.h b/src/services/pcn-simplebridge/src/Simplebridge.h index 1437105df..139532ddf 100644 --- a/src/services/pcn-simplebridge/src/Simplebridge.h +++ b/src/services/pcn-simplebridge/src/Simplebridge.h @@ -25,6 +25,7 @@ #include "polycube/services/utils.h" #include +#include #include "Fdb.h" #include "Ports.h" @@ -96,6 +97,13 @@ class Simplebridge : public polycube::service::Cube, public SimplebridgeI std::unordered_map ports_; std::shared_ptr fdb_ = nullptr; + void updateTimestamp(); + void updateTimestampTimer(); + void quitAndJoin(); + + std::thread timestamp_update_thread_; + std::atomic quit_thread_; + void flood_packet(Port &port, PacketInMetadata &md, const std::vector &packet); std::mutex ports_mutex_; }; diff --git a/src/services/pcn-simplebridge/src/Simplebridge_dp.c b/src/services/pcn-simplebridge/src/Simplebridge_dp.c index dbca9d70c..97b9569b3 100644 --- a/src/services/pcn-simplebridge/src/Simplebridge_dp.c +++ b/src/services/pcn-simplebridge/src/Simplebridge_dp.c @@ -32,11 +32,12 @@ #define REASON_FLOODING 0x01 struct fwd_entry { - u64 timestamp; + u32 timestamp; u32 port; -} __attribute__((packed)); +} __attribute__((packed, aligned(8))); BPF_TABLE("hash", __be64, struct fwd_entry, fwdtable, 1024); +BPF_TABLE("array", int, uint32_t, timestamp, 1); struct eth_hdr { __be64 dst:48; @@ -44,6 +45,15 @@ struct eth_hdr { __be16 proto; } __attribute__((packed)); +static __always_inline u32 time_get_sec() { + int key = 0; + u32 *ts = timestamp.lookup(&key); + if (ts) + return *ts; + + return 0; +} + static __always_inline int handle_rx(struct CTXTYPE *ctx, struct pkt_metadata *md) { void *data = (void *)(long)ctx->data; @@ -60,7 +70,7 @@ int handle_rx(struct CTXTYPE *ctx, struct pkt_metadata *md) { // LEARNING PHASE __be64 src_key = eth->src; - u64 now = bpf_ktime_get_ns(); + u32 now = time_get_sec(); struct fwd_entry e; // used to update the entry in the fdb @@ -84,15 +94,7 @@ int handle_rx(struct CTXTYPE *ctx, struct pkt_metadata *md) { u64 timestamp = entry->timestamp; // Check if the entry is still valid (not too old) - // Warning: the bpf_ktime_get_ns() used before is not monotonic and it may return - // a value that is older than a previously returned value. So, we have to add - // the very strange check 'now < timestamp' to handle this special case - if (now < timestamp) { - pcn_log(ctx, LOG_TRACE, "Entry is valid (but 'now < timestamp'). FORWARDING"); - goto FORWARD; - } - - if ((now - timestamp) > FDB_TIMEOUT*1000000000ULL) { + if ((now - timestamp) > FDB_TIMEOUT) { pcn_log(ctx, LOG_TRACE, "Entry is too old. FLOODING"); fwdtable.delete(&dst_mac); goto DO_FLOODING; From 0531903fa3922398d6624cfb4579d4dbed335ae2 Mon Sep 17 00:00:00 2001 From: Sebastiano Miano Date: Sat, 2 Feb 2019 20:20:58 +0100 Subject: [PATCH 2/2] Improved performance of simplebridge during forwarding This commit removes the bpf_map_update from the simplebridge standard path. In the learning phase, the filtering database is first checked with the src MAC address of the received packet; if the entry is not present the learning is performed, otherwise we just need to update the timestamp (or the src port). In the latter cases, the value of the entry is directly updated using the pointer returned from the lookup, instead of calling the update helper. In fact, after some tests, I discovered that this function introduces a huge performance degradation, while updating the value directly does not impact on the overall performance. With this commit, the multicore throughput (during forwarding) of the bridge increased from 3.5Mpps (with 64byte packets) to ~11.5Mpps. Signed-off-by: Sebastiano Miano --- .../pcn-simplebridge/src/Simplebridge_dp.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/services/pcn-simplebridge/src/Simplebridge_dp.c b/src/services/pcn-simplebridge/src/Simplebridge_dp.c index 97b9569b3..23526fbf9 100644 --- a/src/services/pcn-simplebridge/src/Simplebridge_dp.c +++ b/src/services/pcn-simplebridge/src/Simplebridge_dp.c @@ -72,20 +72,25 @@ int handle_rx(struct CTXTYPE *ctx, struct pkt_metadata *md) { __be64 src_key = eth->src; u32 now = time_get_sec(); - struct fwd_entry e; // used to update the entry in the fdb + struct fwd_entry *entry = fwdtable.lookup(&src_key); - e.timestamp = now; - e.port = in_ifc; + if (!entry) { + struct fwd_entry e; // used to update the entry in the fdb - // Updating the timestamp associated to this entry in the fdb - fwdtable.update(&src_key, &e); + e.timestamp = now; + e.port = in_ifc; - pcn_log(ctx, LOG_TRACE, "MAC: %M learned (or timestamp updated)", src_key); + fwdtable.update(&src_key, &e); + pcn_log(ctx, LOG_TRACE, "MAC: %M learned", src_key); + } else { + entry->port = in_ifc; + entry->timestamp = now; + } // FORWARDING PHASE: select interface(s) to send the packet __be64 dst_mac = eth->dst; // lookup in forwarding table fwdtable - struct fwd_entry *entry = fwdtable.lookup(&dst_mac); + entry = fwdtable.lookup(&dst_mac); if (!entry) { pcn_log(ctx, LOG_DEBUG, "Entry not found for dst-mac: %M", dst_mac); goto DO_FLOODING;