Hi, On Mon, Jul 4, 2016 at 5:23 AM, Arne Schwabe <a...@rfc2549.org> wrote:
> - sndbuf and recvbuf default now to OS default instead of 64k > > > > @@ -120,6 +120,10 @@ User-visible Changes > > - --http-proxy-retry and --sock-proxy-retry have been removed. Proxy > connections > > will now behave like regular connection entries and generate a USR1 > on failure. > > > > +- --connect-retry gets a optional third argument that specifies the > maximum > > + time in seconds to wait between reconnection attempts when an > exponential > > + backoff is triggered due to repeated retries. Default = 300 seconds. > Shouldn't that be 2nd argument? Ugh... should be 2nd. Will do a v3.. > + backoff = (c->options.unsuccessful_attempts / > c->options.connection_list->len) - 4; > > + if (backoff > 0 && sec < 1<<16) > the 1<<16 seems a bit random here. Could you at least add a comment of > why? I don't see any valid reason here since sec is limited between 1 and > connect_retry_seconds_max by the next two lines anyway. > > + sec = max_int (sec, 1) << min_int (backoff, 15); > The 15 here is the same. Both lines seem to only make sense with 16 bit > integers. > Something like this is needed to protect against overflow of the 32 bit int (signed int in this case). Think of the user specifying sec = 2^17. Any shift by more than 14 will make sec = 0. A more sophisticated logic that counts the bits in sec before shifting is possible, but this looked simple.. Now that a v3 is needed, will change to an explicit check on overflow. > + if (sec > c->options.ce.connect_retry_seconds_max) > > + sec = c->options.ce.connect_retry_seconds_max; > Oh, my suggestion made the max time time configurable per connection > entry. We should address that in a seperate patch. I think connect-retry > as a whole can be made a non connection config entry. (connect-retry > traditionally was the time between tcp connects for a tcp connection > which looped on its own before the dual stack patch if anyone wonders > why it was connection specific). It can still be specified as a common number outside connection entries, so leaving it as a connection-specific number looks fine to me. Selva