From ae7eb67c17b7161eba60c7392a66f3f29ec8cd35 Mon Sep 17 00:00:00 2001 From: roland-sipos Date: Mon, 18 Dec 2023 16:16:39 +0100 Subject: [PATCH 1/5] Require PCI addr to configure available interface Fixed EAL argc/argv construction too. This makes it possible to hide and show only PCIe devices that we want for the module/process. To be tested --- include/dpdklibs/EALSetup.hpp | 15 +++++++-------- include/dpdklibs/RTEIfaceSetup.hpp | 21 +++++++++++++++++++++ plugins/NICReceiver.cpp | 28 ++++++++++++++++++++++------ plugins/NICReceiver.hpp | 1 + schema/dpdklibs/nicreader.jsonnet | 5 ++++- test/apps/test_dpdk_stats.cxx | 4 ++++ 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/include/dpdklibs/EALSetup.hpp b/include/dpdklibs/EALSetup.hpp index 491779f..bc85c79 100644 --- a/include/dpdklibs/EALSetup.hpp +++ b/include/dpdklibs/EALSetup.hpp @@ -266,17 +266,16 @@ get_mempool(const std::string& pool_name, return std::unique_ptr(mbuf_pool); } -std::vector -string_to_eal_args(const std::string& params) -{ - auto parts = boost::program_options::split_unix(params); - std::vector cstrings; - for(auto& str : parts){ - cstrings.push_back(const_cast (str.c_str())); +std::vector +construct_eal_argv(std::vector &std_argv){ + std::vector vec_argv; + for (int i=0; i < std_argv.size() ; i++){ + vec_argv.insert(vec_argv.end(), std_argv[i].data()); } - return cstrings; + return vec_argv; } + void init_eal(int argc, char* argv[]) { diff --git a/include/dpdklibs/RTEIfaceSetup.hpp b/include/dpdklibs/RTEIfaceSetup.hpp index f147ee5..4faad84 100644 --- a/include/dpdklibs/RTEIfaceSetup.hpp +++ b/include/dpdklibs/RTEIfaceSetup.hpp @@ -126,6 +126,27 @@ get_iface_mac_str(uint16_t iface) } } +inline std::string +get_iface_pci_str(uint16_t iface) +{ + std::string iface_pci_addr_str; + int retval = -1; + struct rte_eth_dev_info dev_info; + // Get interface info + retval = rte_eth_dev_info_get(iface, &dev_info); + if (retval != 0) { + TLOG() << "Error during getting device (iface " << iface << ") retval: " << retval; + } else if (dev_info.device) { + auto dev_name = rte_dev_name(dev_info.device); + iface_pci_addr_str = dev_name; + //TLOG() << "Dev name: " << dev_name; + //const auto bus = rte_dev_bus(dev_info.device); + //const auto bus_info = rte_dev_bus_info(dev_info.device); + //const auto dev_driver = rte_dev_driver(dev_info.device); + } + return iface_pci_addr_str; +} + inline int iface_reset(uint16_t iface) { diff --git a/plugins/NICReceiver.cpp b/plugins/NICReceiver.cpp index bb42a6d..73c5fe3 100644 --- a/plugins/NICReceiver.cpp +++ b/plugins/NICReceiver.cpp @@ -120,25 +120,41 @@ NICReceiver::do_configure(const data_t& args) // EAL setup TLOG() << "Setting up EAL with params from config."; - std::vector eal_params = ealutils::string_to_eal_args(m_cfg.eal_arg_list); - ealutils::init_eal(eal_params.size(), eal_params.data()); + std::vector eal_args; + eal_args.push_back("daq_application"); + std::istringstream iss(m_cfg.eal_arg_list); + std::string arg_from_str; + while (iss >> arg_from_str) { + if (!arg_from_str.empty()) { + eal_args.push_back(arg_from_str); + } + } + std::vector eal_argv = ealutils::construct_eal_argv(eal_args); + char** constructed_eal_argv = eal_argv.data(); + int constructed_eal_argc = eal_args.size(); + ealutils::init_eal(constructed_eal_argc, constructed_eal_argv); // Get available Interfaces from EAL auto available_ifaces = ifaceutils::get_num_available_ifaces(); TLOG() << "Number of available interfaces: " << available_ifaces; for (unsigned int ifc_id=0; ifc_id IfaceWrapper std::map m_mac_to_id_map; + std::map m_pci_to_id_map; std::map> m_ifaces; // Sinks (SourceConcepts) diff --git a/schema/dpdklibs/nicreader.jsonnet b/schema/dpdklibs/nicreader.jsonnet index aea41fa..89d1c10 100644 --- a/schema/dpdklibs/nicreader.jsonnet +++ b/schema/dpdklibs/nicreader.jsonnet @@ -31,6 +31,8 @@ local nicreader = { ipv4: s.string("ipv4", pattern=moo.re.ipv4, doc="ipv4 string"), + pci: s.string("pci", doc="PCIe address string"), + mac: s.string("mac", pattern="^[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}$", doc="mac string"), float : s.number("Float", "f4", doc="A float number"), @@ -67,7 +69,8 @@ local nicreader = { ], doc="Source field"), iface : s.record("Interface", [ - s.field("mac_addr", self.mac, "AA:BB:CC:DD:EE:FF", doc="Logical Interface ID"), + s.field("pci_addr", self.pci, "0000:00:00.0", doc="PCIe address of the interface"), + s.field("mac_addr", self.mac, "AA:BB:CC:DD:EE:FF", doc="MAC address of the interface"), s.field("ip_addr", self.ipv4, "192.168.0.1", doc="IP address of interface"), s.field("with_flow_control", self.choice, true, doc="FlowAPI enabled"), s.field("promiscuous_mode", self.choice, false, doc="Promiscuous mode enabled"), diff --git a/test/apps/test_dpdk_stats.cxx b/test/apps/test_dpdk_stats.cxx index 81384bb..46f14ef 100644 --- a/test/apps/test_dpdk_stats.cxx +++ b/test/apps/test_dpdk_stats.cxx @@ -1,6 +1,7 @@ /* Application will run until quit or killed. */ #include "dpdklibs/EALSetup.hpp" +#include "dpdklibs/RTEIfaceSetup.hpp" #include "logging/Logging.hpp" #include "dpdklibs/udp/PacketCtor.hpp" #include "dpdklibs/udp/Utils.hpp" @@ -198,6 +199,9 @@ main(int argc, char* argv[]) auto nb_ifaces = rte_eth_dev_count_avail(); TLOG() << "# of available interfaces: " << nb_ifaces; TLOG() << "Initialize interface " << iface_id; + TLOG() << " -> Iface MAC: " << ifaceutils::get_iface_mac_str(iface_id); + TLOG() << " -> Iface PCI: " << ifaceutils::get_iface_pci_str(iface_id); + ealutils::iface_init(iface_id, rx_qs, tx_qs, rx_ring_size, tx_ring_size, mbuf_pools, false, false); ealutils::iface_promiscuous_mode(iface_id, false); // should come from config From 7182761c7344ed21f31444996326e7d7c34eefff Mon Sep 17 00:00:00 2001 From: roland-sipos Date: Mon, 18 Dec 2023 16:28:04 +0100 Subject: [PATCH 2/5] Don't bake into params. Expect it to be provided. --- plugins/NICReceiver.cpp | 1 - schema/dpdklibs/nicreader.jsonnet | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/NICReceiver.cpp b/plugins/NICReceiver.cpp index 73c5fe3..337b38a 100644 --- a/plugins/NICReceiver.cpp +++ b/plugins/NICReceiver.cpp @@ -121,7 +121,6 @@ NICReceiver::do_configure(const data_t& args) // EAL setup TLOG() << "Setting up EAL with params from config."; std::vector eal_args; - eal_args.push_back("daq_application"); std::istringstream iss(m_cfg.eal_arg_list); std::string arg_from_str; while (iss >> arg_from_str) { diff --git a/schema/dpdklibs/nicreader.jsonnet b/schema/dpdklibs/nicreader.jsonnet index 89d1c10..c826d5e 100644 --- a/schema/dpdklibs/nicreader.jsonnet +++ b/schema/dpdklibs/nicreader.jsonnet @@ -91,7 +91,7 @@ local nicreader = { s.field("ifaces", self.ifaces, doc="List of interfaces to configure"), - s.field("eal_arg_list", self.string, "", + s.field("eal_arg_list", self.string, "daq_application", doc="A string with EAL arguments"), ], doc="Generic UIO reader DAQ Module Configuration"), From d681211b9fe20c4f8aa1aec99e0c5e2d6a0aed93 Mon Sep 17 00:00:00 2001 From: roland-sipos Date: Mon, 18 Dec 2023 16:36:57 +0100 Subject: [PATCH 3/5] fix exceptionally broken info log spacing --- include/dpdklibs/EALSetup.hpp | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/include/dpdklibs/EALSetup.hpp b/include/dpdklibs/EALSetup.hpp index bc85c79..cd56392 100644 --- a/include/dpdklibs/EALSetup.hpp +++ b/include/dpdklibs/EALSetup.hpp @@ -200,8 +200,6 @@ iface_init(uint16_t iface, uint16_t rx_rings, uint16_t tx_rings, return retval; } - - // Get interface info retval = rte_eth_dev_info_get(iface, &dev_info); if (retval != 0) { @@ -209,18 +207,14 @@ iface_init(uint16_t iface, uint16_t rx_rings, uint16_t tx_rings, return retval; } - - TLOG() << "Iface " << iface << " Rx Ring info :" - << "min " << dev_info.rx_desc_lim.nb_min - << "max " << dev_info.rx_desc_lim.nb_max - << "align " << dev_info.rx_desc_lim.nb_align - ; - - TLOG() << "Iface " << iface << " Tx Ring info :" - << "min " << dev_info.rx_desc_lim.nb_min - << "max " << dev_info.rx_desc_lim.nb_max - << "align " << dev_info.rx_desc_lim.nb_align - ; + TLOG() << "Iface[" << iface << "] Rx Ring info:" + << " min=" << dev_info.rx_desc_lim.nb_min + << " max=" << dev_info.rx_desc_lim.nb_max + << " align=" << dev_info.rx_desc_lim.nb_align; + TLOG() << "Iface[" << iface << "] Tx Ring info:" + << " min=" << dev_info.rx_desc_lim.nb_min + << " max=" << dev_info.rx_desc_lim.nb_max + << " align=" << dev_info.rx_desc_lim.nb_align; for (size_t j = 0; j < dev_info.nb_rx_queues; j++) { @@ -232,16 +226,13 @@ iface_init(uint16_t iface, uint16_t rx_rings, uint16_t tx_rings, break; count = rte_eth_rx_queue_count(iface, j); - TLOG() << "rx " << j << " descriptors " << count << "/" << queue_info.nb_desc; - TLOG() << "rx " << j << " scattered " << (queue_info.scattered_rx ? " yes" : "no"); - TLOG() << "rx " << j << " conf.drop_en " << (queue_info.conf.rx_drop_en ? " yes" : "no"); - TLOG() << "rx " << j << " conf.rx_deferred_start " << (queue_info.conf.rx_deferred_start ? " yes" : "no"); - TLOG() << "rx " << j << " rx_buf_size " << queue_info.rx_buf_size; - - + TLOG() << "rx[" << j << "] descriptors=" << count << "/" << queue_info.nb_desc + << " scattered=" << (queue_info.scattered_rx ? "yes" : "no") + << " conf.drop_en=" << (queue_info.conf.rx_drop_en ? "yes" : "no") + << " conf.rx_deferred_start=" << (queue_info.conf.rx_deferred_start ? "yes" : "no") + << " rx_buf_size=" << queue_info.rx_buf_size; } - return 0; } From 9a5ba9098b5f7bd31f5904670d621c41dec37f46 Mon Sep 17 00:00:00 2001 From: Alessandro Thea Date: Wed, 20 Dec 2023 22:28:21 +0100 Subject: [PATCH 4/5] Missing space added --- plugins/NICReceiver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/NICReceiver.cpp b/plugins/NICReceiver.cpp index 337b38a..3ac8911 100644 --- a/plugins/NICReceiver.cpp +++ b/plugins/NICReceiver.cpp @@ -153,7 +153,7 @@ NICReceiver::do_configure(const data_t& args) (m_pci_to_id_map.count(iface_pci_addr) != 0)) { auto iface_id = m_mac_to_id_map[iface_mac_addr]; TLOG() << "Configuring expected interface with MAC=" << iface_mac_addr - << "PCIe=" << iface_pci_addr << " Logical ID=" << iface_id; + << " PCIe=" << iface_pci_addr << " Logical ID=" << iface_id; m_ifaces[iface_id] = std::make_unique(iface_id, m_sources, m_run_marker); m_ifaces[iface_id]->conf(iface_cfg); m_ifaces[iface_id]->allocate_mbufs(); From 44eca93551c4d0befb409bddb081c805a36420c8 Mon Sep 17 00:00:00 2001 From: Alessandro Thea Date: Fri, 22 Dec 2023 08:56:50 +0100 Subject: [PATCH 5/5] EAL initialization fixed --- plugins/NICReceiver.cpp | 128 +++++++--------------------------- test/apps/test_multi_proc.cxx | 5 +- 2 files changed, 30 insertions(+), 103 deletions(-) diff --git a/plugins/NICReceiver.cpp b/plugins/NICReceiver.cpp index 3ac8911..e650297 100644 --- a/plugins/NICReceiver.cpp +++ b/plugins/NICReceiver.cpp @@ -117,10 +117,29 @@ NICReceiver::do_configure(const data_t& args) { TLOG() << get_name() << ": Entering do_conf() method"; m_cfg = args.get(); + auto ifaces_cfg = m_cfg.ifaces; // EAL setup TLOG() << "Setting up EAL with params from config."; std::vector eal_args; + + eal_args.push_back("eal_cmdline"); + + // Enforce the process type to primary + eal_args.push_back("--proc-type=primary"); + + // Construct the pcie devices allowed mask + for (const auto& iface_cfg : ifaces_cfg) { + eal_args.push_back("-a"); + eal_args.push_back(iface_cfg.pci_addr); + } + + // Use the first pcie device id as file prefix + // FIXME: Review this strategy - should work in most of cases, but it could be + // confusing in configs with multiple interfaces + eal_args.push_back(fmt::format("--file-prefix={}", ifaces_cfg.front().pci_addr)); + + // Append the remaining dpdk parameters std::istringstream iss(m_cfg.eal_arg_list); std::string arg_from_str; while (iss >> arg_from_str) { @@ -128,6 +147,13 @@ NICReceiver::do_configure(const data_t& args) eal_args.push_back(arg_from_str); } } + + std::stringstream ss; + for( const auto& arg : eal_args) { + ss << arg << " "; + } + TLOG() << "EAL Init arguments: " << ss.str(); + std::vector eal_argv = ealutils::construct_eal_argv(eal_args); char** constructed_eal_argv = eal_argv.data(); int constructed_eal_argc = eal_args.size(); @@ -145,7 +171,6 @@ NICReceiver::do_configure(const data_t& args) } // Configure expected (and available!) interfaces - auto ifaces_cfg = m_cfg.ifaces; for (const auto& iface_cfg : ifaces_cfg) { auto iface_mac_addr = iface_cfg.mac_addr; auto iface_pci_addr = iface_cfg.pci_addr; @@ -168,105 +193,6 @@ NICReceiver::do_configure(const data_t& args) } return; - -// #warning RS FIXME -> Removed for conf overhaul -// auto ip_sources = nullptr; -// auto rx_cores = nullptr; -// // m_iface_id = (uint16_t)m_cfg.card_id; -// // m_dest_ip = m_cfg.dest_ip; -// // auto ip_sources = m_cfg.ip_sources; -// // auto rx_cores = m_cfg.rx_cores; -// // m_num_ip_sources = ip_sources.size(); -// // m_num_rx_cores = rx_cores.size(); -// /* -// // Initialize RX core map -// for (auto rxc : rx_cores) { -// for (auto qid : rxc.rx_qs) { -// m_rx_core_map[rxc.lcore_id][qid] = ""; -// } -// } - -// // Setup expected IP sources -// for (auto src : ip_sources) { -// TLOG() << "IP source to register: ID=" << src.id << " IP=" << src.ip << " RX_Q=" << src.rx_q << " LC=" << src.lcore; -// // Extend mapping -// m_rx_core_map[src.lcore][src.rx_q] = src.ip; -// m_rx_qs.insert(src.rx_q); -// // Create frame counter metric -// m_num_frames[src.id] = { 0 }; -// m_num_bytes[src.id] = { 0 }; -// } -// */ - -// // Setup SourceConcepts -// // m_sources[]->configure if needed!? - -// // Allocate pools and mbufs per queue -// TLOG() << "Allocating pools and mbufs."; -// for (size_t i=0; i Check for flow flush return! -// for (auto const& [lcoreid, rxqs] : m_rx_core_map) { -// for (auto const& [rxqid, srcip] : rxqs) { - -// // Put the IP numbers temporarily in a vector, so they can be converted easily to uint32_t -// TLOG() << "Current ip is " << srcip; -// size_t ind = 0, current_ind = 0; -// std::vector v; -// for (int i = 0; i < 4; ++i) { -// v.push_back(std::stoi(srcip.substr(current_ind, srcip.size() - current_ind), &ind)); -// current_ind += ind + 1; -// } - -// flow = generate_ipv4_flow(m_iface_id, rxqid, -// RTE_IPV4(v[0], v[1], v[2], v[3]), 0xffffffff, 0, 0, &error); - -// if (not flow) { // ers::fatal -// TLOG() << "Flow can't be created for " << rxqid -// << " Error type: " << (unsigned)error.type -// << " Message: " << error.message; -// ers::fatal(dunedaq::readoutlibs::InitializationError( -// ERS_HERE, "Couldn't create Flow API rules!")); -// rte_exit(EXIT_FAILURE, "error in creating flow"); -// } -// } -// } - -// #warning RS FIXME -> Removed for conf overhaul -// // if (m_cfg.with_drop_flow) { -// // Adding drop flow -// TLOG() << "Adding Drop Flow."; -// flow = generate_drop_flow(m_iface_id, &error); -// if (not flow) { // ers::fatal -// TLOG() << "Drop flow can't be created for interface!" -// << " Error type: " << (unsigned)error.type -// << " Message: " << error.message; -// rte_exit(EXIT_FAILURE, "error in creating flow"); -// } -// // } - -// TLOG() << "DPDK EAL & RTE configured."; - } void @@ -349,6 +275,4 @@ NICReceiver::set_running(bool should_run) } // namespace dpdklibs } // namespace dunedaq -// #include "detail/NICReceiver.hxx" - DEFINE_DUNE_DAQ_MODULE(dunedaq::dpdklibs::NICReceiver) diff --git a/test/apps/test_multi_proc.cxx b/test/apps/test_multi_proc.cxx index cc16656..a053f3c 100644 --- a/test/apps/test_multi_proc.cxx +++ b/test/apps/test_multi_proc.cxx @@ -362,6 +362,7 @@ int main(int argc, char** argv){ app.add_option("-i", iface, "Interface to init"); app.add_option("-t", time_per_report, "Time Per Report"); app.add_option("-a", allow_dev, "device to allow"); + app.add_flag("--check-time", check_timestamp, "Report back differences in timestamp"); app.add_flag("-p", per_stream_reports, "Detailed per stream reports"); CLI11_PARSE(app, argc, argv); @@ -372,9 +373,11 @@ int main(int argc, char** argv){ std::vector eal_args; eal_args.push_back("dpdklibds_test_frame_receiver"); eal_args.push_back("--proc-type=primary"); - // eal_args.push_back(fmt::format("--file-prexix={}")); + if (!allow_dev.empty()) { eal_args.push_back(fmt::format("--allow={}",allow_dev)); + // Use the device id as file prefix to + eal_args.push_back(fmt::format("--file-prefix={}", allow_dev)); } TLOG() << "Status of RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE=" << RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE;