dnsdist: Format and delint the XSK code

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 81ad234bfe2713e00bb31195a9feb4feb344705f..7bc864dbb5bcc9fb5c427527c17b0d6c05aafd1d 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -3203,7 +3203,6 @@ int main(int argc, char** argv)
     g_hashperturb = dnsdist::getRandomValue(0xffffffff);
 
 #ifdef HAVE_XSK
-#warning FIXME: we need to provide a way to clear the map from Lua, as well as a way to change the map path
     try {
       dnsdist::xsk::clearDestinationAddresses();
     }

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index 254e64af61e4d3d82b2ac14a7e111d7c0fb752c7..1a92f25dab10487617d6c12768535bb024cc304c 100644 (file)
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -987,7 +987,7 @@ public:
   void handleUDPTimeouts();
   void reportTimeoutOrError();
   void reportResponse(uint8_t rcode);
-  void submitHealthCheckResult(bool initial, bool newState);
+  void submitHealthCheckResult(bool initial, bool newResult);
   time_t getNextLazyHealthCheck();
   uint16_t saveState(InternalQueryState&&);
   void restoreState(uint16_t id, InternalQueryState&&);

diff --git a/pdns/dnsdistdist/dnsdist-xsk.cc b/pdns/dnsdistdist/dnsdist-xsk.cc
index 4d1bec33cdc07e31e9a149936188615fc9089e5c..fbc0313ae76c2a64d7c1f79b6eb00dd73380a1a9 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-xsk.cc
+++ b/pdns/dnsdistdist/dnsdist-xsk.cc
@@ -150,7 +150,7 @@ void XskRouter(std::shared_ptr<XskSocket> xsk)
       if ((fds.at(0).revents & POLLIN) != 0) {
         auto packets = xsk->recv(64, &failed);
         dnsdist::metrics::g_stats.nonCompliantQueries += failed;
-        for (auto &packet : packets) {
+        for (auto& packet : packets) {
           const auto dest = packet.getToAddr();
           auto worker = xsk->getWorkerByDestination(dest);
           if (!worker) {
@@ -172,7 +172,6 @@ void XskRouter(std::shared_ptr<XskSocket> xsk)
         needNotify.clear();
         ready--;
       }
-      const auto backup = ready;
       for (size_t fdIndex = 1; fdIndex < fds.size() && ready > 0; fdIndex++) {
         if ((fds.at(fdIndex).revents & POLLIN) != 0) {
           ready--;
@@ -199,7 +198,6 @@ void XskRouter(std::shared_ptr<XskSocket> xsk)
       xsk->recycle(4096);
       xsk->fillFq();
       xsk->send(fillInTx);
-      ready = backup;
     }
     catch (...) {
       vinfolog("Exception in XSK router loop");
@@ -238,8 +236,9 @@ void XskClientThread(ClientState* clientState)
   }
 }
 
-static std::string getDestinationMap(bool isV6) {
- return !isV6 ? "/sys/fs/bpf/dnsdist/xsk-destinations-v4" : "/sys/fs/bpf/dnsdist/xsk-destinations-v6";
+static std::string getDestinationMap(bool isV6)
+{
+  return !isV6 ? "/sys/fs/bpf/dnsdist/xsk-destinations-v4" : "/sys/fs/bpf/dnsdist/xsk-destinations-v6";
 }
 
 void addDestinationAddress(const ComboAddress& addr)

diff --git a/pdns/xsk.cc b/pdns/xsk.cc
index b656757a2d22e679347a7b23bb736293b70aa01c..cfa6912419b71930116e3ad50a06d95a1f9273b2 100644 (file)
--- a/pdns/xsk.cc
+++ b/pdns/xsk.cc
@@ -59,10 +59,17 @@ extern "C"
 #include "xsk.hh"
 
 #ifdef DEBUG_UMEM
-namespace {
+namespace
+{
 struct UmemEntryStatus
 {
-  enum class Status: uint8_t { Free, FillQueue, Received, TXQueue };
+  enum class Status : uint8_t
+  {
+    Free,
+    FillQueue,
+    Received,
+    TXQueue
+  };
   Status status{Status::Free};
 };
 
@@ -144,7 +151,6 @@ XskSocket::XskSocket(size_t frameNum_, std::string ifName_, uint32_t queue_id, c
   uniqueEmptyFrameOffset.reserve(frameNum);
   {
     for (uint64_t i = 0; i < frameNum; i++) {
-      //uniqueEmptyFrameOffset.push_back(i * frameSize);
       uniqueEmptyFrameOffset.push_back(i * frameSize + XDP_PACKET_HEADROOM);
 #ifdef DEBUG_UMEM
       {
@@ -261,11 +267,14 @@ void XskSocket::removeDestinationAddress(const std::string& mapPath, const Combo
 void XskSocket::fillFq(uint32_t fillSize) noexcept
 {
   {
-#warning why are we collecting frames from unique into shared here, even though we need unique ones?
+    // if we have less than holdThreshold frames in the shared queue (which might be an issue
+    // when the XskWorker needs empty frames), move frames from the unique container into the
+    // shared one. This might not be optimal right now.
     auto frames = sharedEmptyFrameOffset->lock();
     if (frames->size() < holdThreshold) {
       const auto moveSize = std::min(holdThreshold - frames->size(), uniqueEmptyFrameOffset.size());
       if (moveSize > 0) {
+        // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
         frames->insert(frames->end(), std::make_move_iterator(uniqueEmptyFrameOffset.end() - moveSize), std::make_move_iterator(uniqueEmptyFrameOffset.end()));
         uniqueEmptyFrameOffset.resize(uniqueEmptyFrameOffset.size() - moveSize);
       }
@@ -304,7 +313,8 @@ int XskSocket::wait(int timeout)
   return packet.getFrameOffsetFrom(umem.bufBase);
 }
 
-[[nodiscard]] int XskSocket::xskFd() const noexcept {
+[[nodiscard]] int XskSocket::xskFd() const noexcept
+{
   return xsk_socket__fd(socket.get());
 }
 
@@ -359,7 +369,7 @@ std::vector<XskPacket> XskSocket::recv(uint32_t recvSizeMax, uint32_t* failedCou
   for (; processed < recvSize; processed++) {
     try {
       const auto* desc = xsk_ring_cons__rx_desc(&rx, idx++);
-      // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+      // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast,performance-no-int-to-ptr)
       XskPacket packet = XskPacket(reinterpret_cast<uint8_t*>(desc->addr + baseAddr), desc->len, frameSize);
 #ifdef DEBUG_UMEM
       checkUmemIntegrity(__PRETTY_FUNCTION__, __LINE__, frameOffset(packet), {UmemEntryStatus::Status::Free, UmemEntryStatus::Status::FillQueue}, UmemEntryStatus::Status::Received);
@@ -565,6 +575,7 @@ void XskPacket::setIPv6Header(const ipv6hdr& ipv6Header) noexcept
 void XskPacket::setUDPHeader(const udphdr& udpHeader) noexcept
 {
   assert(frameLength >= (sizeof(ethhdr) + (v6 ? sizeof(ipv6hdr) : sizeof(iphdr)) + sizeof(udpHeader)));
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
   memcpy(frame + getL4HeaderOffset(), &udpHeader, sizeof(udpHeader));
 }
 
@@ -661,11 +672,14 @@ void XskPacket::changeDirectAndUpdateChecksum() noexcept
 {
   auto ethHeader = getEthernetHeader();
   {
-    std::array<uint8_t, ETH_ALEN> tmp;
+    std::array<uint8_t, ETH_ALEN> tmp{};
     static_assert(tmp.size() == sizeof(ethHeader.h_dest), "Size Error");
     static_assert(tmp.size() == sizeof(ethHeader.h_source), "Size Error");
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
     memcpy(tmp.data(), ethHeader.h_dest, tmp.size());
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
     memcpy(ethHeader.h_dest, ethHeader.h_source, tmp.size());
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
     memcpy(ethHeader.h_source, tmp.data(), tmp.size());
   }
   if (ethHeader.h_proto == htons(ETH_P_IPV6)) {
@@ -841,7 +855,9 @@ const void* XskPacket::getPayloadData() const
 void XskPacket::setAddr(const ComboAddress& from_, MACAddr fromMAC, const ComboAddress& to_, MACAddr toMAC) noexcept
 {
   auto ethHeader = getEthernetHeader();
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   memcpy(ethHeader.h_dest, toMAC.data(), toMAC.size());
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   memcpy(ethHeader.h_source, fromMAC.data(), fromMAC.size());
   setEthernetHeader(ethHeader);
   to = to_;
@@ -993,7 +1009,7 @@ void XskPacket::rewrite() noexcept
       uint32_t words[3];
     };
   };
-  struct ipv4_pseudo_header_t pseudo_header;
+  struct ipv4_pseudo_header_t pseudo_header{};
   static_assert(sizeof(pseudo_header) == 12, "IPv4 pseudo-header size is incorrect");
 
   /* Fill in the pseudo-header. */
@@ -1027,7 +1043,7 @@ void XskPacket::rewrite() noexcept
       uint32_t words[10];
     };
   };
-  struct ipv6_pseudo_header_t pseudo_header;
+  struct ipv6_pseudo_header_t pseudo_header{};
   static_assert(sizeof(pseudo_header) == 40, "IPv6 pseudo-header size is incorrect");
 
   /* Fill in the pseudo-header. */
@@ -1070,7 +1086,7 @@ int XskWorker::createEventfd()
 
 void XskWorker::waitForXskSocket() const noexcept
 {
-  uint64_t x = read(workerWaker, &x, sizeof(x));
+  uint64_t value = read(workerWaker, &value, sizeof(value));
 }
 
 void XskWorker::notifyXskSocket() const
@@ -1109,7 +1125,7 @@ uint64_t XskWorker::frameOffset(const XskPacket& packet) const noexcept
   return packet.getFrameOffsetFrom(umemBufBase);
 }
 
-void XskWorker::notifyWorker() noexcept
+void XskWorker::notifyWorker() const
 {
   notify(workerWaker);
 }
@@ -1126,11 +1142,13 @@ void XskSocket::getMACFromIfName()
     throw std::runtime_error("Unable to get MAC address for interface " + ifName + ": name too long");
   }
 
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   strncpy(ifr.ifr_name, ifName.c_str(), ifName.length() + 1);
   if (ioctl(desc.getHandle(), SIOCGIFHWADDR, &ifr) < 0 || ifr.ifr_hwaddr.sa_family != ARPHRD_ETHER) {
     throw std::runtime_error("Error getting MAC address for interface " + ifName);
   }
   static_assert(sizeof(ifr.ifr_hwaddr.sa_data) >= std::tuple_size<decltype(source)>{}, "The size of an ARPHRD_ETHER MAC address is smaller than expected");
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   memcpy(source.data(), ifr.ifr_hwaddr.sa_data, source.size());
 }
 
@@ -1175,6 +1193,7 @@ void XskWorker::fillUniqueEmptyOffset()
   auto frames = sharedEmptyFrameOffset->lock();
   const auto moveSize = std::min(static_cast<size_t>(32), frames->size());
   if (moveSize > 0) {
+    // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
     uniqueEmptyFrameOffset.insert(uniqueEmptyFrameOffset.end(), std::make_move_iterator(frames->end() - moveSize), std::make_move_iterator(frames->end()));
     frames->resize(frames->size() - moveSize);
   }

diff --git a/pdns/xsk.hh b/pdns/xsk.hh
index a2cc6f036b71c79614564cc066e41db3af3ccaba..4ea36e38b36af5bddaef6c670bb37bb5672ccdc8 100644 (file)
--- a/pdns/xsk.hh
+++ b/pdns/xsk.hh
@@ -29,12 +29,12 @@
 #include <boost/multi_index/hashed_index.hpp>
 #include <boost/multi_index_container.hpp>
 #include <boost/multi_index/member.hpp>
+#include <cstdint>
 #include <memory>
 #include <poll.h>
 #include <queue>
 #include <stdexcept>
 #include <string>
-//#include <sys/types.h>
 #include <unistd.h>
 #include <unordered_map>
 #include <vector>
@@ -120,7 +120,7 @@ class XskSocket
   static constexpr uint32_t txCapacity = XSK_RING_PROD__DEFAULT_NUM_DESCS * 2;
 
   constexpr static bool isPowOfTwo(uint32_t value) noexcept;
-  [[nodiscard]] static int timeDifference(const timespec& t1, const timespec& t2) noexcept;
+  [[nodiscard]] static int timeDifference(const timespec& lhs, const timespec& rhs) noexcept;
 
   [[nodiscard]] uint64_t frameOffset(const XskPacket& packet) const noexcept;
   [[nodiscard]] int firstTimeout();
@@ -224,7 +224,7 @@ private:
   [[nodiscard]] __be16 tcp_udp_v4_checksum(const struct iphdr*) const noexcept;
   // You must set l4Header.check = 0 before calling this method
   [[nodiscard]] __be16 tcp_udp_v6_checksum(const struct ipv6hdr*) const noexcept;
-    /* offset of the L4 (udphdr) header (after ethhdr and iphdr/ipv6hdr) */
+  /* offset of the L4 (udphdr) header (after ethhdr and iphdr/ipv6hdr) */
   [[nodiscard]] size_t getL4HeaderOffset() const noexcept;
   /* offset of the data after the UDP header */
   [[nodiscard]] size_t getDataOffset() const noexcept;
@@ -271,7 +271,7 @@ public:
     return frame - base;
   }
 };
-bool operator<(const XskPacket& s1, const XskPacket& s2) noexcept;
+bool operator<(const XskPacket& lhs, const XskPacket& rhs) noexcept;
 
 /* g++ defines __SANITIZE_THREAD__
    clang++ supports the nice __has_feature(thread_sanitizer),
@@ -288,9 +288,9 @@ bool operator<(const XskPacket& s1, const XskPacket& s2) noexcept;
 class XskWorker
 {
 #if defined(__SANITIZE_THREAD__)
-  using XskPacketRing = LockGuarded<boost::lockfree::spsc_queue<XskPacket, boost::lockfree::capacity<XSK_RING_CONS__DEFAULT_NUM_DESCS*2>>>;
+  using XskPacketRing = LockGuarded<boost::lockfree::spsc_queue<XskPacket, boost::lockfree::capacity<XSK_RING_CONS__DEFAULT_NUM_DESCS * 2>>>;
 #else
-  using XskPacketRing = boost::lockfree::spsc_queue<XskPacket, boost::lockfree::capacity<XSK_RING_CONS__DEFAULT_NUM_DESCS*2>>;
+  using XskPacketRing = boost::lockfree::spsc_queue<XskPacket, boost::lockfree::capacity<XSK_RING_CONS__DEFAULT_NUM_DESCS * 2>>;
 #endif
 
 public:
@@ -299,7 +299,7 @@ public:
   // queue of packets processed by this worker (to be sent, or discarded)
   XskPacketRing outgoingPacketsQueue;
 
-  uint8_t* umemBufBase;
+  uint8_t* umemBufBase{nullptr};
   // list of frames that are shared with the XskRouter
   std::shared_ptr<LockGuarded<vector<uint64_t>>> sharedEmptyFrameOffset;
   // list of frames that we own, used to generate new packets (health-check)
@@ -310,13 +310,13 @@ public:
 
   XskWorker();
   static int createEventfd();
-  static void notify(int fd);
+  static void notify(int desc);
   static std::shared_ptr<XskWorker> create();
   void pushToProcessingQueue(XskPacket& packet);
   void pushToSendQueue(XskPacket& packet);
   void markAsFree(const XskPacket& packet);
   // notify worker that at least one packet is available for processing
-  void notifyWorker() noexcept;
+  void notifyWorker() const;
   // notify the router that packets are ready to be sent
   void notifyXskSocket() const;
   void waitForXskSocket() const noexcept;