Re: [ 059/143] sysctl net: Keep tcp_syn_retries inside the boundary

Eric W. Biederman
2014-06-21T00:20:35Z

Willy Tarreau <w@1wt.eu> writes:

> Hi Eric,
>
> On Fri, Jun 20, 2014 at 03:16:07PM -0700, Eric W. Biederman wrote:
>> Willy Tarreau <w@1wt.eu> writes:
>>
>> > Hi Luis,
>> >
>> > On Thu, Jun 12, 2014 at 01:55:53PM +0100, Luis Henriques wrote:
>> >> I was finally able to spend some more time with this and tried (a
>> >> modified) Tyler's patch on top of 2.6.32.62, and it seems to work.
>> >> Although I haven't done any extended testing, I don't see the two
>> >> stack traces and the /proc/sys/net/ipv4/ directory seems to be
>> >> correctly populated.
>> >>
>> >> I'm attaching the patch I've used, based on Tyler's.
>> >
>> > Would any of you or Tyler please kindly pass me a signed-off-by with
>> > a commit message ? That would be great. Alternately I'd do it myself
>> > and mention you authored them.
>>
>> If my memory serves it is possibe in 2.6.32 to set
>> .ctl_name = CTL_UNNEEDED
>>
>> and not need to implement a .strategy routine at all.
>
> Ah that's quite interesting, thanks for the tip!
>
>> Given the fact that most people got the strategy routines
>> slightly wrong and that sys_sysctl is effectively unused
>> a strategy where you don't implement code that no-one
>> will use in a backport I would be preferable.
>
> OK.
>
>> Since you have mentioned this has come up a couple of times if something
>> else this will be something to think about for next time.
>
> I'm keeping your e-mail where I manage patches, hoping to recognize
> this case next time.
>
>> I am puzzled why .ctl_name was populated in a backport at all.
>
> Oh it's simply because I didn't know it did not have to be there,
> and among the few reviewers, I guess that it's not common to know
> what version uses what semantics.

I guess what I meant is that the field .ctl_name does not even exist
anymore for the same reasons .strategy does not exist anymore. So I
was just suprirsed that someone picked a randomish number and stuck
it in there.

If anyone actually were to use those randomish numbers in the binary
sys_sysctl call their applications would break when they eventually
moved to a more recent kernel.

Which is one of the motivations it was decided there would be no more
binary sysctls allocated around the 2.6.32 timeframe.

> Thank you for the exaplanation, it's really helpful. We're not used
> to backport sysctl changes but here I got caught a few times and have
> found some sysctl.conf with bogus values in field a few times, so it
> was really important to backport this one.

Sysctl do have their uses, and at least 2.6.32 has runtime sysctl checks
to keep the insanity to a dull roar.

Eric