Message-ID: <4AE5C579.2070104@codefidence.com>
Date:	Mon, 26 Oct 2009 17:51:21 +0200
From:	Gilad Ben-Yossef <gilad@...efidence.com>
To:	Bill Fink <billfink@...dspring.com>
CC:	Eric Dumazet <eric.dumazet@...il.com>,
	William Allen Simpson <william.allen.simpson@...il.com>,
	netdev@...r.kernel.org,
	Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
Subject: Re: [PATCH v2 8/8] Document future removal of sysctl_tcp_* options

Bill Fink wrote:

>
>> 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?
>   
Now it is my turn to get confused, because I didn't understand your 
question :-)

What I suggest is to leave the sysctl exactly as they are now:

- You leave them be (value of 1), the respective TCP option is 
supported. This is the default.
- You turn them off (write 0), the respective TCP option is not supported.

What I suggest to *add* is the following ability:

- If you have the TCP option support turned on (default, value of one), 
you can turn support for the option for a specific route using a ip 
route option.

Hope that made it clearer.
>   
>> 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.
>   
Yes. You understand my intention perfectly.

Let me try to explain why I believe this is the correct behavior to 
implement:

1. This is the closest thing to what we have now. Today you write 0 to 
the sysctl and that TCP option is turned off globally. Period.  My 
suggestion leaves this behavior as is now regardless if you've used per 
route settings. The other way make a very subtle change in the meaning 
of writing 0 to the sysctl.

I believe very subtle changes to meaning of long established interfaces 
is bad way to go. It's better to change interfaces on users, but it is 
even worse to maek something that they have long used do something just 
slightly different.

2. If the per route options needs to be "default, of or off" instead of 
"on or off", we'd need to move from 1 bit to store the option to, well 
2s bit in theory, but probably 32 bits in practice, since we can't use 
RTAX_FEATURES any longer.

Yes, we can invent RTAX_FEATURES_TWO_BITS or some such, but I'd say that 
is ugly :-)

3. I believe that the scenario of needing to set the support of a TCP 
option globally off and just turn it on for a specific route is not very 
likely to be needed and losing it is a small price to pay for 1 + 2.


> 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.
>
>   
Well, I do not believe this is what Eric meant (Eric?) but if it is then 
I fail to see why
to require from the per route TCP options switches what is not required 
of any other
route specific option already existing, since AFAIK we don't have a 
"reset to default values" to the other options already supported.


Having said all that, I have no issue with re-spinning the patch with 
your suggestion.
I don't feel all that much which is the correct way- I just want to get 
as much feedback as possible
since I'm suggesting to add a new user interface options and we all know 
it is very hard to back peddle
on those, so I'm trying to make sure to get enough feedback to do it 
right the firs time.

So any feedback from anyone regarding favorite interface? it seems each 
person fancy a different one :-)

Thanks!
Gilad



-- 
Gilad Ben-Yossef
Chief Coffee Drinker & CTO
Codefidence Ltd.

Web:   http://codefidence.com
Cell:  +972-52-8260388
Skype: gilad_codefidence
Tel:   +972-8-9316883 ext. 201
Fax:   +972-8-9316884
Email: gilad@...efidence.com

Check out our Open Source technology and training blog - http://tuxology.net

	"The biggest risk you can take it is to take no risk."
		-- Mark Zuckerberg and probably others

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists