From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 15 Nov 2021 17:11:02 +0000 (+0100)
Subject: dnsdist: Apply suggestions from code review on the new eBPF map type
X-Git-Tag: dnsdist-1.7.0-beta1^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a918f0cf399ef1207dc78d11ac99a333e452cb6e;p=thirdparty%2Fpdns.git

dnsdist: Apply suggestions from code review on the new eBPF map type
---

diff --git a/pdns/bpf-filter.cc b/pdns/bpf-filter.cc
index 2b5df7a07f..29bd6fe313 100644
--- a/pdns/bpf-filter.cc
+++ b/pdns/bpf-filter.cc
@@ -89,7 +89,6 @@ void bpf_check_map_sizes(int fd, uint32_t expectedKeySize, uint32_t expectedValu
   if (info.value_size != expectedValueSize) {
     throw std::runtime_error("Error checking the size of eBPF map: value size mismatch (" + std::to_string(info.value_size) + " VS " + std::to_string(expectedValueSize) + ")");
   }
-
 }
 
 int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags)
@@ -133,8 +132,8 @@ int bpf_get_next_key(int fd, void *key, void *next_key)
 }
 
 int bpf_prog_load(enum bpf_prog_type prog_type,
-		  const struct bpf_insn *insns, int prog_len,
-		  const char *license, int kern_version)
+                  const struct bpf_insn *insns, int prog_len,
+                  const char *license, int kern_version)
 {
   char log_buf[65535];
   union bpf_attr attr;
@@ -254,38 +253,30 @@ BPFFilter::Map::Map(const BPFFilter::MapConfiguration& config, BPFFilter::MapFor
 
         if (d_config.d_type == MapType::IPv4) {
           uint32_t key = 0;
-          int res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
-          while (res == 0) {
+          while (bpf_get_next_key(d_fd.getHandle(), &key, &key) == 0) {
             ++d_count;
-            res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
           }
         }
         else if (d_config.d_type == MapType::IPv6) {
           KeyV6 key;
           memset(&key, 0, sizeof(key));
-          int res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
-          while (res == 0) {
+          while (bpf_get_next_key(d_fd.getHandle(), &key, &key) == 0) {
             ++d_count;
-            res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
           }
         }
         else if (d_config.d_type == MapType::QNames) {
           if (format == MapFormat::Legacy) {
             QNameKey key;
             memset(&key, 0, sizeof(key));
-            int res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
-            while (res == 0) {
+            while (bpf_get_next_key(d_fd.getHandle(), &key, &key) == 0) {
               ++d_count;
-              res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
             }
           }
           else {
             QNameAndQTypeKey key;
             memset(&key, 0, sizeof(key));
-            int res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
-            while (res == 0) {
+            while (bpf_get_next_key(d_fd.getHandle(), &key, &key) == 0) {
               ++d_count;
-              res = bpf_get_next_key(d_fd.getHandle(), &key, &key);
             }
           }
         }
diff --git a/pdns/dnsdist-lua-bindings.cc b/pdns/dnsdist-lua-bindings.cc
index 43c3d04122..ae44c7b631 100644
--- a/pdns/dnsdist-lua-bindings.cc
+++ b/pdns/dnsdist-lua-bindings.cc
@@ -478,7 +478,7 @@ void setupLuaBindings(LuaContext& luaCtx, bool client)
             match = BPFFilter::MatchAction::Truncate;
             break;
           default:
-            throw std::runtime_error("Unsupported action for BPFFilter::block");
+            throw std::runtime_error("Unsupported action for BPFFilter::blockQName");
           }
           return bpf->block(qname, match, qtype.value_or(255));
         }
diff --git a/pdns/dnsdistdist/docs/reference/ebpf.rst b/pdns/dnsdistdist/docs/reference/ebpf.rst
index f723c1b51c..5fc3da9f0e 100644
--- a/pdns/dnsdistdist/docs/reference/ebpf.rst
+++ b/pdns/dnsdistdist/docs/reference/ebpf.rst
@@ -20,7 +20,7 @@ These are all the functions, objects and methods related to the :doc:`../advance
     This function now supports a table for each parameters, and the ability to use pinned eBPF maps.
 
   Return a new eBPF socket filter with a maximum of maxV4 IPv4, maxV6 IPv6 and maxQNames qname entries in the block tables.
-  Maps can be pinned to a filesystem path, which makes their content persistent across restarts and allows external programs to read their content and to add new entries. dnsdist will try to load maps that are pinned to a filesystem path on startups, inheriting any existing entries, and fall back to creating them if they do not exist yet. Note that the user dnsdist is running under must have the right privileges to read and write to the given file, and to go through all the directories in the path leading to that file.
+  Maps can be pinned to a filesystem path, which makes their content persistent across restarts and allows external programs to read their content and to add new entries. dnsdist will try to load maps that are pinned to a filesystem path on startups, inheriting any existing entries, and fall back to creating them if they do not exist yet. Note that the user dnsdist is running under must have the right privileges to read and write to the given file, and to go through all the directories in the path leading to that file. The pinned path must be on a filesystem of type ``BPF``, usually below ``/sys/fs/bpf/``.
 
   :param int maxV4: Maximum number of IPv4 entries in this filter
   :param int maxV6: Maximum number of IPv6 entries in this filter
@@ -33,7 +33,7 @@ These are all the functions, objects and methods related to the :doc:`../advance
   Options:
 
   * ``maxItems``: int - The maximum number of entries in a given map. Default is 0 which will not allow any entry at all.
-  * ``pinnedPaths``: str - The filesystem path this map should be pinned to.
+  * ``pinnedPath``: str - The filesystem path this map should be pinned to.
 
 .. function:: newDynBPFFilter(bpf) -> DynBPFFilter