From: Baptiste Courtois <b.courtois@criteo.com>
Date: Sat, 24 Nov 2018 15:16:11 +0000 (+0100)
Subject: auth: geoip, compute weight per QType
X-Git-Tag: auth-4.2.0-rc1~16^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=618824cb73db384d52cf379c7d49744dbb5b8f6d;p=thirdparty%2Fpdns.git

auth: geoip, compute weight per QType

As seen in #7051 the former behavior of the geoip backend in mixed
weighted QType cases, was erroneous.
Having per QType weights allows proper static lookup.
It also enables to question with ANY and retrieve one record per QType.

Closes #7051
---

diff --git a/docs/backends/geoip.rst b/docs/backends/geoip.rst
index 6667988c47..dfadcebc77 100644
--- a/docs/backends/geoip.rst
+++ b/docs/backends/geoip.rst
@@ -221,7 +221,7 @@ Using the ``weight`` attribute
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 You can use record attributes to define positive and non-zero weight.
-If this is given, only one record is chosen randomly based on the weight.
+If this is given, only one record per type is chosen randomly based on the weight.
 
 Probability is calculated by summing up the weights and dividing each weight with the sum.
 
diff --git a/modules/geoipbackend/geoipbackend.cc b/modules/geoipbackend/geoipbackend.cc
index 4baabe2fe7..b3e7480d3f 100644
--- a/modules/geoipbackend/geoipbackend.cc
+++ b/modules/geoipbackend/geoipbackend.cc
@@ -261,24 +261,30 @@ void GeoIPBackend::initialize() {
 
     // finally fix weights
     for(auto &item: dom.records) {
-      float weight=0;
-      float sum=0;
+      map<uint16_t, float> weights;
+      map<uint16_t, float> sums;
+      map<uint16_t, GeoIPDNSResourceRecord> lasts;
       bool has_weight=false;
       // first we look for used weight
       for(const auto &rr: item.second) {
-        weight+=rr.weight;
+        weights[rr.qtype.getCode()] += rr.weight;
         if (rr.has_weight) has_weight = true;
       }
       if (has_weight) {
         // put them back as probabilities and values..
         for(auto &rr: item.second) {
-          rr.weight=static_cast<int>((static_cast<float>(rr.weight) / weight)*1000.0);
-          sum += rr.weight;
+          uint16_t rr_type = rr.qtype.getCode();
+          rr.weight=static_cast<int>((static_cast<float>(rr.weight) / weights[rr_type])*1000.0);
+          sums[rr_type] += rr.weight;
           rr.has_weight = has_weight;
+          lasts[rr_type] = rr;
         }
         // remove rounding gap
-        if (sum < 1000)
-          item.second.back().weight += (1000-sum);
+        for(auto &x: lasts) {
+          float sum = sums[x.first];
+          if (sum < 1000)
+            x.second.weight += (1000-sum);
+        }
       }
     }
 
@@ -307,15 +313,15 @@ GeoIPBackend::~GeoIPBackend() {
 
 bool GeoIPBackend::lookup_static(const GeoIPDomain &dom, const DNSName &search, const QType &qtype, const DNSName& qdomain, const std::string &ip, GeoIPNetmask &gl, bool v6) {
   const auto& i = dom.records.find(search);
-  int cumul_probability = 0;
+  map<uint16_t,int> cumul_probabilities;
   int probability_rnd = 1+(dns_random(1000)); // setting probability=0 means it never is used
 
   if (i != dom.records.end()) { // return static value
     for(const auto& rr : i->second) {
       if (rr.has_weight) {
         gl.netmask = (v6?128:32);
-        int comp = cumul_probability;
-        cumul_probability += rr.weight;
+        int comp = cumul_probabilities[rr.qtype.getCode()];
+        cumul_probabilities[rr.qtype.getCode()] += rr.weight;
         if (rr.weight == 0 || probability_rnd < comp || probability_rnd > (comp + rr.weight))
           continue;
       }