Subject: Re: [PATCH v2 8/8] Document future removal of sysctl_tcp_* options

From: Bill Fink <[email-masked]>
Date: Mon, 26 Oct 2009 11:08:55 -0400

On Mon, 26 Oct 2009, Gilad Ben-Yossef wrote:

> Eric Dumazet wrote:
> 
> > Bill Fink a écrit :
> >   
> >> On Sun, 25 Oct 2009, Gilad Ben-Yossef wrote:
> >>
> >>     
> >>> Eric Dumazet wrote:
> >>>
> >>>
> >>> I still think having a global kill switch and per route options better 
> >>> (basically use the exiting patch but not retire the global kill 
> >>> switch|), but if you must Hgow about we leave the global sysctl as they 
> >>> are and just have a two bit route option:
> >>>
> >>> 0 Use global default
> >>> 1 Off
> >>> 2 On
> >>>
> >>> It's kind of funny, because this is what the original patch from 
> >>> Comsleep does and I thought it needlessly complicates things.
> >>>
> >>> So, what do you say - which will it be?
> >>>       
> >> I personally feel the 2-bit settings are overkill.  What i think
> >> makes the most sense is for the global options to act as they always
> >> have in the absence of any route specific settings, and for any
> >> route specific settings to override the related global settings.
> >> This is both simple and maintains backward compatibility.
> >>     
> >
> > Backward compatibility is important, very important, if not the most
> > important thing. Then usability comes.
> >   
> I tend to agree.
> > I know some busy servers where adding/changing a single route makes them
> > go crazy (because of ip route flush cache)
> >
> > So if a route is overriding a global conf, and the admin wants to make an
> > emergency change during peak hours, he should do it by a global setting,
> > or he wont use at all this new stuff, and stay conservative.
> >
> > Alternative would be to not trigger the flush of cache when changing
> > features flags.
> >
> >   
> OK. It really sounds like we should go with my first suggestion: global 
> sysctl based kill switches, just as we have now and in addition, the 
> ability to kill TCP options per route. The TCP option will be used if 
> and only if both kill switches (global and per route) are not set.

This wording is confusing.  The global kill switch not being set
really means that the sysctl is set.  And this assumes the per-route
default is not set.  Correct?

> What we achieve is:
> 
> 1. Global kill switches work exactly as they do now, whether you use the 
> new per route options or not, so backwards compatible.
> 
> 2. In addition, if the global kill switch is not in effect, you can also 
> kill the options on a per route basis.
> 
> I'm going to send third version of the patch to this effect, minus the 
> new remote DoS possibility that Ilpo pointed out and leaving the global 
> sysctl kill switches be.
> 
> If you like it, please ACK ;-)

IIUC this doesn't seem right to me.  I believe the global setting
should be a default and the route specific an override.  Your scheme
would mean that if I set a global option to disable timestamps, then
I couldn't enable timestamps on specific routes using the per route
setting.

And it also doesn't seem to address Eric's scenario.  If I understand
his concern correctly, what seems to be needed is a third global
reset value (not calling it a setting since the actual global setting
wouldn't be changed), which would reset any per-route override settings
to the global default setting.

						-Bill