From fd17ad2563f944acd30087d848148dc348c8da8e Mon Sep 17 00:00:00 2001 From: Sangjin Han Date: Sun, 8 Jul 2018 15:57:49 -0700 Subject: [PATCH] vport_zc: remove ZeroCopyVPort ZeroCopyVPort has been provided but never really well tested, documented, or maintained. Given the security concern caused by zero copy, perhaps it is time for ZeroCopyVPort to retire. If you want high-performance packet I/O for user-level applications, you can use PMDPort (with vhost-user in BESS, and virtio-user in client ports) as an alternative. --- core/drivers/vport_zc.cc | 182 ---------------------------------- core/drivers/vport_zc.h | 105 -------------------- core/drivers/vport_zc_test.cc | 147 --------------------------- protobuf/ports/port_msg.proto | 4 - 4 files changed, 438 deletions(-) delete mode 100644 core/drivers/vport_zc.cc delete mode 100644 core/drivers/vport_zc.h delete mode 100644 core/drivers/vport_zc_test.cc diff --git a/core/drivers/vport_zc.cc b/core/drivers/vport_zc.cc deleted file mode 100644 index 03e9d5238..000000000 --- a/core/drivers/vport_zc.cc +++ /dev/null @@ -1,182 +0,0 @@ -// Copyright (c) 2014-2016, The Regents of the University of California. -// Copyright (c) 2016-2017, Nefeli Networks, Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, this -// list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the names of the copyright holders nor the names of their -// contributors may be used to endorse or promote products derived from this -// software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. - -#include "vport_zc.h" - -#include -#include -#include -#include -#include -#include - -#include -#include - -#define ROUND_TO_64(x) ((x + 32) & (~0x3f)) - -CommandResponse ZeroCopyVPort::Init(const bess::pb::EmptyArg &) { - struct vport_bar *bar = nullptr; - - int num_inc_q = num_queues[PACKET_DIR_INC]; - int num_out_q = num_queues[PACKET_DIR_OUT]; - - int bytes_per_llring; - int total_bytes; - uint8_t *ptr; - int i; - char port_dir[PORT_NAME_LEN + 256]; - char file_name[PORT_NAME_LEN + 256]; - struct stat sb; - FILE *fp; - size_t bar_address; - - bytes_per_llring = llring_bytes_with_slots(SLOTS_PER_LLRING); - total_bytes = ROUND_TO_64(sizeof(struct vport_bar)) + - ROUND_TO_64(bytes_per_llring) * (num_inc_q + num_out_q) + - ROUND_TO_64(sizeof(struct vport_inc_regs)) * num_inc_q + - ROUND_TO_64(sizeof(struct vport_out_regs)) * num_out_q; - - bar = static_cast(rte_zmalloc(nullptr, total_bytes, 64)); - bar_address = (size_t)bar; - DCHECK(bar); - bar_ = bar; - - strncpy(bar->name, name().c_str(), PORT_NAME_LEN); - bar->num_inc_q = num_inc_q; - bar->num_out_q = num_out_q; - - ptr = (uint8_t *)(bar); - ptr += ROUND_TO_64(sizeof(struct vport_bar)); - - /* Set up inc llrings */ - for (i = 0; i < num_inc_q; i++) { - inc_regs_[i] = bar->inc_regs[i] = - reinterpret_cast(ptr); - ptr += ROUND_TO_64(sizeof(struct vport_inc_regs)); - - llring_init(reinterpret_cast(ptr), SLOTS_PER_LLRING, - SINGLE_P, SINGLE_C); - llring_set_water_mark(reinterpret_cast(ptr), - SLOTS_WATERMARK); - bar->inc_qs[i] = reinterpret_cast(ptr); - inc_qs_[i] = bar->inc_qs[i]; - ptr += ROUND_TO_64(bytes_per_llring); - } - - /* Set up out llrings */ - for (i = 0; i < num_out_q; i++) { - out_regs_[i] = bar->out_regs[i] = - reinterpret_cast(ptr); - ptr += ROUND_TO_64(sizeof(struct vport_out_regs)); - - llring_init(reinterpret_cast(ptr), SLOTS_PER_LLRING, - SINGLE_P, SINGLE_C); - llring_set_water_mark(reinterpret_cast(ptr), - SLOTS_WATERMARK); - bar->out_qs[i] = reinterpret_cast(ptr); - out_qs_[i] = bar->out_qs[i]; - ptr += ROUND_TO_64(bytes_per_llring); - } - - snprintf(port_dir, PORT_NAME_LEN + 256, "%s/%s", P_tmpdir, VPORT_DIR_PREFIX); - - if (stat(port_dir, &sb) == 0) { - DCHECK_EQ((sb.st_mode & S_IFMT), S_IFDIR); - } else { - LOG(INFO) << "Creating directory " << port_dir; - mkdir(port_dir, S_IRWXU | S_IRWXG | S_IRWXO); - } - - for (i = 0; i < num_out_q; i++) { - snprintf(file_name, PORT_NAME_LEN + 256, "%s/%s/%s.rx%d", P_tmpdir, - VPORT_DIR_PREFIX, name().c_str(), i); - - mkfifo(file_name, 0666); - - out_irq_fd_[i] = open(file_name, O_RDWR); - } - - snprintf(file_name, PORT_NAME_LEN + 256, "%s/%s/%s", P_tmpdir, - VPORT_DIR_PREFIX, name().c_str()); - LOG(INFO) << "Writing port information to " << file_name; - fp = fopen(file_name, "w"); - fwrite(&bar_address, 8, 1, fp); - fclose(fp); - - return CommandSuccess(); -} - -void ZeroCopyVPort::DeInit() { - char file_name[PORT_NAME_LEN + 256]; - - int num_out_q = num_queues[PACKET_DIR_OUT]; - - for (int i = 0; i < num_out_q; i++) { - snprintf(file_name, PORT_NAME_LEN + 256, "%s/%s/%s.rx%d", P_tmpdir, - VPORT_DIR_PREFIX, name().c_str(), i); - - unlink(file_name); - close(out_irq_fd_[i]); - } - - snprintf(file_name, PORT_NAME_LEN + 256, "%s/%s/%s", P_tmpdir, - VPORT_DIR_PREFIX, name().c_str()); - unlink(file_name); - - rte_free(bar_); -} - -int ZeroCopyVPort::SendPackets(queue_t qid, bess::Packet **pkts, int cnt) { - struct llring *q = out_qs_[qid]; - int ret; - - ret = llring_enqueue_bulk(q, (void **)pkts, cnt); - if (ret == -LLRING_ERR_NOBUF) - return 0; - - if (__sync_bool_compare_and_swap(&out_regs_[qid]->irq_enabled, 1, 0)) { - char t[1] = {'F'}; - ret = write(out_irq_fd_[qid], reinterpret_cast(t), 1); - } - - return cnt; -} - -int ZeroCopyVPort::RecvPackets(queue_t qid, bess::Packet **pkts, int cnt) { - struct llring *q = inc_qs_[qid]; - int ret; - - ret = llring_dequeue_burst(q, (void **)pkts, cnt); - return ret; -} - -ADD_DRIVER(ZeroCopyVPort, "zcvport", - "zero copy virtual port for trusted user apps") diff --git a/core/drivers/vport_zc.h b/core/drivers/vport_zc.h deleted file mode 100644 index fe91e5636..000000000 --- a/core/drivers/vport_zc.h +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (c) 2014-2016, The Regents of the University of California. -// Copyright (c) 2016-2017, Nefeli Networks, Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, this -// list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the names of the copyright holders nor the names of their -// contributors may be used to endorse or promote products derived from this -// software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. - -#ifndef BESS_DRIVERS_ZERO_COPY_VPORT_ -#define BESS_DRIVERS_ZERO_COPY_VPORT_ -#include - -#include "../kmod/llring.h" -#include "../message.h" -#include "../port.h" - -#define SLOTS_PER_LLRING 1024 - -/* This watermark is to detect congestion and cache bouncing due to - * head-eating-tail (needs at least 8 slots less then the total ring slots). - * Not sure how to tune this... */ -#define SLOTS_WATERMARK ((SLOTS_PER_LLRING >> 3) * 7) /* 87.5% */ - -/* Disable (0) single producer/consumer mode for now. - * This is slower, but just to be on the safe side. :) */ -#define SINGLE_P 0 -#define SINGLE_C 0 - -#define PORT_NAME_LEN 128 - -#define VPORT_DIR_PREFIX "sn_vports" - -struct vport_inc_regs { - uint64_t dropped; -} __cacheline_aligned; - -struct vport_out_regs { - uint32_t irq_enabled; -} __cacheline_aligned; - -/* This is equivalent to the old bar */ -struct vport_bar { - char name[PORT_NAME_LEN]; - - /* The term RX/TX could be very confusing for a virtual switch. - * Instead, we use the "incoming/outgoing" convention: - * - incoming: outside -> BESS - * - outgoing: BESS -> outside */ - int num_inc_q; - int num_out_q; - - struct vport_inc_regs *inc_regs[MAX_QUEUES_PER_DIR]; - struct llring *inc_qs[MAX_QUEUES_PER_DIR]; - - struct vport_out_regs *out_regs[MAX_QUEUES_PER_DIR]; - struct llring *out_qs[MAX_QUEUES_PER_DIR]; -}; - -class ZeroCopyVPort final : public Port { - public: - CommandResponse Init(const bess::pb::EmptyArg &arg); - - void DeInit() override; - - int RecvPackets(queue_t qid, bess::Packet **pkts, int cnt) override; - int SendPackets(queue_t qid, bess::Packet **pkts, int cnt) override; - - private: - friend class ZeroCopyVPortTest; - FRIEND_TEST(ZeroCopyVPortTest, Recv); - - struct vport_bar *bar_ = {}; - - struct vport_inc_regs *inc_regs_[MAX_QUEUES_PER_DIR] = {}; - struct llring *inc_qs_[MAX_QUEUES_PER_DIR] = {}; - - struct vport_out_regs *out_regs_[MAX_QUEUES_PER_DIR] = {}; - struct llring *out_qs_[MAX_QUEUES_PER_DIR] = {}; - - int out_irq_fd_[MAX_QUEUES_PER_DIR] = {}; -}; - -#endif // BESS_DRIVERS_ZERO_COPY_VPORT_ diff --git a/core/drivers/vport_zc_test.cc b/core/drivers/vport_zc_test.cc deleted file mode 100644 index a398163be..000000000 --- a/core/drivers/vport_zc_test.cc +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright (c) 2014-2016, The Regents of the University of California. -// Copyright (c) 2016-2017, Nefeli Networks, Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, this -// list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the names of the copyright holders nor the names of their -// contributors may be used to endorse or promote products derived from this -// software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. - -#include "vport_zc.h" - -#include -#include - -#include - -#include "../dpdk.h" -#include "../kmod/llring.h" -#include "../message.h" -#include "../packet.h" -#include "../pktbatch.h" -#include "../port.h" - -class ZeroCopyVPortTest : public ::testing::Test { - protected: - virtual void SetUp() { - port_ = nullptr; - - if (!dpdk_inited_) { - if (geteuid() == 0) { - init_dpdk("vport_zc_test", 1024, 0, true); - dpdk_inited_ = true; - } else { - LOG(INFO) << "This test requires root privileges. Skipping..."; - return; - } - } - - bess::pb::EmptyArg arg; - ADD_DRIVER(ZeroCopyVPort, "zcvport", - "zero copy virtual port for trusted user apps") - ASSERT_TRUE(__driver__ZeroCopyVPort); - const PortBuilder &builder = - PortBuilder::all_port_builders().find("ZeroCopyVPort")->second; - port_ = reinterpret_cast(builder.CreatePort("p0")); - ASSERT_NE(nullptr, port_); - port_->num_queues[PACKET_DIR_INC] = 1; - port_->num_queues[PACKET_DIR_OUT] = 1; - ASSERT_EQ(0, port_->Init(arg).error().code()); - } - - virtual void TearDown() { - if (!dpdk_inited_) { - return; - } - - PortBuilder::all_port_builders_holder(true); - PortBuilder::all_ports_.clear(); - if (port_) { - port_->DeInit(); - delete port_; - } - } - - ZeroCopyVPort *port_; - static bool dpdk_inited_; -}; - -bool ZeroCopyVPortTest::dpdk_inited_ = false; - -TEST_F(ZeroCopyVPortTest, Send) { - if (!dpdk_inited_) { - return; - } - - int cnt; - bess::PacketBatch tx_batch; - bess::Packet pkts[bess::PacketBatch::kMaxBurst]; - tx_batch.clear(); - for (size_t i = 0; i < bess::PacketBatch::kMaxBurst; i++) { - bess::Packet *pkt = &pkts[i]; - - // this fake packet must not be freed - pkt->set_refcnt(2); - - // not chained - pkt->set_next(nullptr); - - tx_batch.add(pkt); - } - cnt = port_->SendPackets(0, tx_batch.pkts(), tx_batch.cnt()); - ASSERT_EQ(tx_batch.cnt(), cnt); - bess::Packet::Free(&tx_batch); -} - -TEST_F(ZeroCopyVPortTest, Recv) { - if (!dpdk_inited_) { - return; - } - - bess::PacketBatch tx_batch; - bess::PacketBatch rx_batch; - bess::Packet pkts[bess::PacketBatch::kMaxBurst]; - tx_batch.clear(); - for (size_t i = 0; i < bess::PacketBatch::kMaxBurst; i++) { - bess::Packet *pkt = &pkts[i]; - - // this fake packet must not be freed - pkt->set_refcnt(2); - - // not chained - pkt->set_next(nullptr); - - tx_batch.add(pkt); - } - - // Packets arrived from somewhere - llring_enqueue_bulk(port_->inc_qs_[0], - reinterpret_cast(tx_batch.pkts()), - tx_batch.cnt()); - - rx_batch.set_cnt(port_->RecvPackets(0, rx_batch.pkts(), tx_batch.cnt())); - ASSERT_EQ(tx_batch.cnt(), rx_batch.cnt()); - tx_batch.clear(); - bess::Packet::Free(&rx_batch); -} diff --git a/protobuf/ports/port_msg.proto b/protobuf/ports/port_msg.proto index 88175a59c..ff6e65ede 100644 --- a/protobuf/ports/port_msg.proto +++ b/protobuf/ports/port_msg.proto @@ -65,10 +65,6 @@ message UnixSocketPortArg { bool confirm_connect = 3; } -message ZeroCopyVPortArg { - -} - message VPortArg { string ifname = 1; oneof cpid {