Changing MaxAdvertisedBW may not need a republish

Relays no longer publish a new server descriptor if they change
their MaxAdvertisedBandwidth config option but it doesn't end up
changing their advertised bandwidth numbers. Bugfix on 0.2.0.28-rc;
fixes bug 1026. Patch from Sebastian.

4 files changed, 37 insertions, 17 deletions

diff --git a/ChangeLog b/ChangeLog
index 2747420dd6..97972e8a12 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -10,6 +10,10 @@ Changes in version 0.2.1.19 - 2009-07-??
       and confuse fewer users.
 
   o Minor bugfixes:
+    - Relays no longer publish a new server descriptor if they change
+      their MaxAdvertisedBandwidth config option but it doesn't end up
+      changing their advertised bandwidth numbers. Bugfix on 0.2.0.28-rc;
+      fixes bug 1026. Patch from Sebastian.
     - Avoid leaking memory every time we get a create cell but we have
       so many already queued that we refuse it. Bugfix on 0.2.0.19-alpha;
       fixes bug 1034. Reported by BarkerJr.
diff --git a/src/or/config.c b/src/or/config.c
index b744f8faf4..3f45b1e5e2 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1222,6 +1222,30 @@ options_need_geoip_info(or_options_t *options, const char **reason_out)
   return bridge_usage || routerset_usage;
 }
 
+/** Return the bandwidthrate that we are going to report to the authorities
+ * based on the config options. */
+int
+get_effective_bwrate(or_options_t *options)
+{
+  int bw = (int)options->BandwidthRate;
+  if (bw > options->MaxAdvertisedBandwidth)
+    bw = (int)options->MaxAdvertisedBandwidth;
+  if (options->RelayBandwidthRate > 0 && bw > options->RelayBandwidthRate)
+    bw = (int)options->RelayBandwidthRate;
+  return bw;
+}
+
+/** Return the bandwidthburst that we are going to report to the authorities
+ * based on the config options. */
+int
+get_effective_bwburst(or_options_t *options)
+{
+  int bw = (int)options->BandwidthBurst;
+  if (options->RelayBandwidthBurst > 0 && bw > options->RelayBandwidthBurst)
+    bw = (int)options->RelayBandwidthBurst;
+  return bw;
+}
+
 /** Fetch the active option list, and take actions based on it. All of the
  * things we do should survive being done repeatedly.  If present,
  * <b>old_options</b> contains the previous value of the options.
@@ -3744,9 +3768,7 @@ options_transition_affects_descriptor(or_options_t *old_options,
                                       or_options_t *new_options)
 {
   /* XXX We can be smarter here. If your DirPort isn't being
-   * published and you just turned it off, no need to republish. If
-   * you changed your bandwidthrate but maxadvertisedbandwidth still
-   * trumps, no need to republish. Etc. */
+   * published and you just turned it off, no need to republish. Etc. */
   if (!opt_streq(old_options->DataDirectory, new_options->DataDirectory) ||
       !opt_streq(old_options->Nickname,new_options->Nickname) ||
       !opt_streq(old_options->Address,new_options->Address) ||
@@ -3759,10 +3781,9 @@ options_transition_affects_descriptor(or_options_t *old_options,
       old_options->NoPublish != new_options->NoPublish ||
       old_options->_PublishServerDescriptor !=
         new_options->_PublishServerDescriptor ||
-      old_options->BandwidthRate != new_options->BandwidthRate ||
-      old_options->BandwidthBurst != new_options->BandwidthBurst ||
-      old_options->MaxAdvertisedBandwidth !=
-        new_options->MaxAdvertisedBandwidth ||
+      get_effective_bwrate(old_options) != get_effective_bwrate(new_options) ||
+      get_effective_bwburst(old_options) !=
+        get_effective_bwburst(new_options) ||
       !opt_streq(old_options->ContactInfo, new_options->ContactInfo) ||
       !opt_streq(old_options->MyFamily, new_options->MyFamily) ||
       !opt_streq(old_options->AccountingStart, new_options->AccountingStart) ||
diff --git a/src/or/or.h b/src/or/or.h
index fba7af0da0..1dcff28d6d 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2926,6 +2926,9 @@ int options_need_geoip_info(or_options_t *options, const char **reason_out);
 int getinfo_helper_config(control_connection_t *conn,
                           const char *question, char **answer);
 
+int get_effective_bwrate(or_options_t *options);
+int get_effective_bwburst(or_options_t *options);
+
 #ifdef CONFIG_PRIVATE
 /* Used only by config.c and test.c */
 or_options_t *options_new(void);
diff --git a/src/or/router.c b/src/or/router.c
index 6f899854e3..859a1e805a 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1300,18 +1300,10 @@ router_rebuild_descriptor(int force)
   ri->platform = tor_strdup(platform);
 
   /* compute ri->bandwidthrate as the min of various options */
-  ri->bandwidthrate = (int)options->BandwidthRate;
-  if (ri->bandwidthrate > options->MaxAdvertisedBandwidth)
-    ri->bandwidthrate = (int)options->MaxAdvertisedBandwidth;
-  if (options->RelayBandwidthRate > 0 &&
-      ri->bandwidthrate > options->RelayBandwidthRate)
-    ri->bandwidthrate = (int)options->RelayBandwidthRate;
+  ri->bandwidthrate = get_effective_bwrate(options);
 
   /* and compute ri->bandwidthburst similarly */
-  ri->bandwidthburst = (int)options->BandwidthBurst;
-  if (options->RelayBandwidthBurst > 0 &&
-      ri->bandwidthburst > options->RelayBandwidthBurst)
-    ri->bandwidthburst = (int)options->RelayBandwidthBurst;
+  ri->bandwidthburst = get_effective_bwburst(options);
 
   ri->bandwidthcapacity = hibernating ? 0 : rep_hist_bandwidth_assess();