route/max_size sysctl in ipv4

route/max_size sysctl in ipv4

[Ani Sinha]

Hi all :

Please see : http://lxr.free-electrons.com/source/Documentation/networking/ip-sysctl.txt

I am looking at the code and it looks like since the route cache for
ipv4 was removed from the kernel, this sysctl parameter no longer
serves the same purpose. It does not look like it is even used in the
ipv4/route.c module. Is there an equivalent sysctl parameter limiting
the number of route entries in the kernel? Or is there now no
mechanism to limit the number of route entries?

Please someone enlighten me.

Thanks a lot in advance,
ani

Re: route/max_size sysctl in ipv4

[David Miller]

From: Ani Sinha <ani@arista.com>
Date: Mon, 5 Jan 2015 15:48:11 -0800

> I am looking at the code and it looks like since the route cache for
> ipv4 was removed from the kernel, this sysctl parameter no longer
> serves the same purpose. It does not look like it is even used in the
> ipv4/route.c module. Is there an equivalent sysctl parameter limiting
> the number of route entries in the kernel? Or is there now no
> mechanism to limit the number of route entries?

There is nothing to limit, since the cache was removed.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Mon, Jan 5, 2015 at 4:36 PM, David Miller <davem@davemloft.net> wrote:
> From: Ani Sinha <ani@arista.com>
> Date: Mon, 5 Jan 2015 15:48:11 -0800
>
>> I am looking at the code and it looks like since the route cache for
>> ipv4 was removed from the kernel, this sysctl parameter no longer
>> serves the same purpose. It does not look like it is even used in the
>> ipv4/route.c module. Is there an equivalent sysctl parameter limiting
>> the number of route entries in the kernel? Or is there now no
>> mechanism to limit the number of route entries?
>
> There is nothing to limit, since the cache was removed.

Shouldn't the documentation be updated to reflect that? Also what's
the point of having a dummy variable that does nothing? Should we not
simply remove it?

Re: route/max_size sysctl in ipv4

[David Miller]

From: Ani Sinha <ani@arista.com>
Date: Mon, 5 Jan 2015 16:43:30 -0800

> On Mon, Jan 5, 2015 at 4:36 PM, David Miller <davem@davemloft.net> wrote:
>> From: Ani Sinha <ani@arista.com>
>> Date: Mon, 5 Jan 2015 15:48:11 -0800
>>
>>> I am looking at the code and it looks like since the route cache for
>>> ipv4 was removed from the kernel, this sysctl parameter no longer
>>> serves the same purpose. It does not look like it is even used in the
>>> ipv4/route.c module. Is there an equivalent sysctl parameter limiting
>>> the number of route entries in the kernel? Or is there now no
>>> mechanism to limit the number of route entries?
>>
>> There is nothing to limit, since the cache was removed.
> 
> Shouldn't the documentation be updated to reflect that? Also what's
> the point of having a dummy variable that does nothing? Should we not
> simply remove it?

There is nothing to update, the behavior is completely transparent.
Absolutely no cache entries exist, therefore the limit cannot be
reached.

The sysctl is kept so that scripts reading it don't suddenly stop
working.  We can't just remove sysctl values.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
> From: Ani Sinha <ani@arista.com>
> Date: Mon, 5 Jan 2015 16:43:30 -0800
>
>> On Mon, Jan 5, 2015 at 4:36 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Ani Sinha <ani@arista.com>
>>> Date: Mon, 5 Jan 2015 15:48:11 -0800
>>>
>>>> I am looking at the code and it looks like since the route cache for
>>>> ipv4 was removed from the kernel, this sysctl parameter no longer
>>>> serves the same purpose. It does not look like it is even used in the
>>>> ipv4/route.c module. Is there an equivalent sysctl parameter limiting
>>>> the number of route entries in the kernel? Or is there now no
>>>> mechanism to limit the number of route entries?
>>>
>>> There is nothing to limit, since the cache was removed.
>>
>> Shouldn't the documentation be updated to reflect that? Also what's
>> the point of having a dummy variable that does nothing? Should we not
>> simply remove it?
>
> There is nothing to update, the behavior is completely transparent.
> Absolutely no cache entries exist, therefore the limit cannot be
> reached.

I disagree. You are advertising a feature in an official documentation
that simply does not exist for ipv4. This is very confusing. If I did
not dig into the code, I wouldn't know that this particular knob is a
noop since the time the route cache was removed.


>
> The sysctl is kept so that scripts reading it don't suddenly stop
> working.  We can't just remove sysctl values.
>

Re: route/max_size sysctl in ipv4

[Pádraig Brady]

On 06/01/15 00:56, Ani Sinha wrote:
> On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
>> From: Ani Sinha <ani@arista.com>
>> Date: Mon, 5 Jan 2015 16:43:30 -0800
>>
>>> On Mon, Jan 5, 2015 at 4:36 PM, David Miller <davem@davemloft.net> wrote:
>>>> From: Ani Sinha <ani@arista.com>
>>>> Date: Mon, 5 Jan 2015 15:48:11 -0800
>>>>
>>>>> I am looking at the code and it looks like since the route cache for
>>>>> ipv4 was removed from the kernel, this sysctl parameter no longer
>>>>> serves the same purpose. It does not look like it is even used in the
>>>>> ipv4/route.c module. Is there an equivalent sysctl parameter limiting
>>>>> the number of route entries in the kernel? Or is there now no
>>>>> mechanism to limit the number of route entries?
>>>>
>>>> There is nothing to limit, since the cache was removed.
>>>
>>> Shouldn't the documentation be updated to reflect that? Also what's
>>> the point of having a dummy variable that does nothing? Should we not
>>> simply remove it?
>>
>> There is nothing to update, the behavior is completely transparent.
>> Absolutely no cache entries exist, therefore the limit cannot be
>> reached.
> 
> I disagree. You are advertising a feature in an official documentation
> that simply does not exist for ipv4. This is very confusing. If I did
> not dig into the code, I wouldn't know that this particular knob is a
> noop since the time the route cache was removed.

You can't change APIs with impunity.

>> The sysctl is kept so that scripts reading it don't suddenly stop
>> working.  We can't just remove sysctl values.

Perhaps /proc/sys/net/ipv4/route/max_size should always return 0 when read,
and discard written values?

thanks,
Pádraig

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Tue, Jan 6, 2015 at 4:41 AM, Pádraig Brady <P@draigbrady.com> wrote:
> On 06/01/15 00:56, Ani Sinha wrote:
>> On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Ani Sinha <ani@arista.com>
>>> Date: Mon, 5 Jan 2015 16:43:30 -0800
>>>
>>>> On Mon, Jan 5, 2015 at 4:36 PM, David Miller <davem@davemloft.net> wrote:
>>>>> From: Ani Sinha <ani@arista.com>
>>>>> Date: Mon, 5 Jan 2015 15:48:11 -0800
>>>>>
>>>>>> I am looking at the code and it looks like since the route cache for
>>>>>> ipv4 was removed from the kernel, this sysctl parameter no longer
>>>>>> serves the same purpose. It does not look like it is even used in the
>>>>>> ipv4/route.c module. Is there an equivalent sysctl parameter limiting
>>>>>> the number of route entries in the kernel? Or is there now no
>>>>>> mechanism to limit the number of route entries?
>>>>>
>>>>> There is nothing to limit, since the cache was removed.
>>>>
>>>> Shouldn't the documentation be updated to reflect that? Also what's
>>>> the point of having a dummy variable that does nothing? Should we not
>>>> simply remove it?
>>>
>>> There is nothing to update, the behavior is completely transparent.
>>> Absolutely no cache entries exist, therefore the limit cannot be
>>> reached.
>>
>> I disagree. You are advertising a feature in an official documentation
>> that simply does not exist for ipv4. This is very confusing. If I did
>> not dig into the code, I wouldn't know that this particular knob is a
>> noop since the time the route cache was removed.
>
> You can't change APIs with impunity.
>
>>> The sysctl is kept so that scripts reading it don't suddenly stop
>>> working.  We can't just remove sysctl values.
>
> Perhaps /proc/sys/net/ipv4/route/max_size should always return 0 when read,
> and discard written values?

Why can't se simply change the documentation to reflect the fact that
this sysctl is no longer in operation?

Re: route/max_size sysctl in ipv4

[Eric Dumazet]

On Tue, 2015-01-06 at 09:11 -0800, Ani Sinha wrote:

> Why can't se simply change the documentation to reflect the fact that
> this sysctl is no longer in operation?

It is still in operation for IPv6

Looks like you propose to update the documentation.

This is great !

Why don't you send an official patch ?

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Tue, Jan 6, 2015 at 10:09 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2015-01-06 at 09:11 -0800, Ani Sinha wrote:
>
>> Why can't se simply change the documentation to reflect the fact that
>> this sysctl is no longer in operation?
>
> It is still in operation for IPv6
>
> Looks like you propose to update the documentation.
>
> This is great !
>
> Why don't you send an official patch ?

Just did.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:

> The sysctl is kept so that scripts reading it don't suddenly stop
> working.  We can't just remove sysctl values.

Interestingly, one of our scripts did break. It broke because now this
sysctl is only available in the global net namespace and not in the
child namespaces. If not breaking scripts is the fundamental logic in
keeping in sysctl intact, would you guys be open to accepting a patch
where we make this sysctl available for all net namespaces?

Re: route/max_size sysctl in ipv4

[Ani Sinha]

hey guys,

On Tue, Jan 6, 2015 at 5:56 PM, Ani Sinha <ani@arista.com> wrote:
> On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
>
>> The sysctl is kept so that scripts reading it don't suddenly stop
>> working.  We can't just remove sysctl values.
>
> Interestingly, one of our scripts did break. It broke because now this
> sysctl is only available in the global net namespace and not in the
> child namespaces. If not breaking scripts is the fundamental logic in
> keeping in sysctl intact, would you guys be open to accepting a patch
> where we make this sysctl available for all net namespaces?

Any thoughts on this?

Re: route/max_size sysctl in ipv4

[Eric Dumazet]

On Wed, 2015-01-07 at 17:25 -0800, Ani Sinha wrote:
> hey guys,
> 
> On Tue, Jan 6, 2015 at 5:56 PM, Ani Sinha <ani@arista.com> wrote:
> > On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
> >
> >> The sysctl is kept so that scripts reading it don't suddenly stop
> >> working.  We can't just remove sysctl values.
> >
> > Interestingly, one of our scripts did break. It broke because now this
> > sysctl is only available in the global net namespace and not in the
> > child namespaces. If not breaking scripts is the fundamental logic in
> > keeping in sysctl intact, would you guys be open to accepting a patch
> > where we make this sysctl available for all net namespaces?
> 
> Any thoughts on this?

AFAIK, this sysctl (and others) always have been global.

Only /proc/sys/net/ipv4/route/flush is per ns

namespaces usage can indeed remove some sysctl :
Only initial namespace show them.

Presumably the global sysctl could be shadowed as read only, but still
some scripts expecting them being rw would break anyway.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Wed, Jan 7, 2015 at 6:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2015-01-07 at 17:25 -0800, Ani Sinha wrote:
>> hey guys,
>>
>> On Tue, Jan 6, 2015 at 5:56 PM, Ani Sinha <ani@arista.com> wrote:
>> > On Mon, Jan 5, 2015 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
>> >
>> >> The sysctl is kept so that scripts reading it don't suddenly stop
>> >> working.  We can't just remove sysctl values.
>> >
>> > Interestingly, one of our scripts did break. It broke because now this
>> > sysctl is only available in the global net namespace and not in the
>> > child namespaces. If not breaking scripts is the fundamental logic in
>> > keeping in sysctl intact, would you guys be open to accepting a patch
>> > where we make this sysctl available for all net namespaces?
>>
>> Any thoughts on this?
>
> AFAIK, this sysctl (and others) always have been global.
>
> Only /proc/sys/net/ipv4/route/flush is per ns

my apologies for not conveying my thoughts clearer. Eric, you are
correct. However, notice that except /proc/sys/net/ipv4/route/flush,
rest of the sysctls are not available from a child net namespace.

>
> namespaces usage can indeed remove some sysctl :
> Only initial namespace show them.

yes, this is what I am talking about.

>
> Presumably the global sysctl could be shadowed as read only, but still
> some scripts expecting them being rw would break anyway.

true but at least those scripts which does only read the values will
continue to function in a child namespace. In our case, our script
only reads the max_size sysctl but since it operates from within a
child namespace, it doesn't find it.

thoughts?

Re: route/max_size sysctl in ipv4

[Eric Dumazet]

On Wed, 2015-01-07 at 19:40 -0800, Ani Sinha wrote:

> true but at least those scripts which does only read the values will
> continue to function in a child namespace. In our case, our script
> only reads the max_size sysctl but since it operates from within a
> child namespace, it doesn't find it.
> 
> thoughts?

Fix your script, or do not use namespaces.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Wed, Jan 7, 2015 at 9:41 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2015-01-07 at 19:40 -0800, Ani Sinha wrote:
>
>> true but at least those scripts which does only read the values will
>> continue to function in a child namespace. In our case, our script
>> only reads the max_size sysctl but since it operates from within a
>> child namespace, it doesn't find it.
>>
>> thoughts?
>
> Fix your script,

But the whole point of keeping this sysctl still around is not to
break the userland scripts! If we are trying to achieve that, I think
we should go one step further and make sure that the sysctl is still
available for reading even in child namespaces (as it used to be the
case).

Re: route/max_size sysctl in ipv4

[Eric Dumazet]

On Thu, 2015-01-08 at 10:39 -0800, Ani Sinha wrote:

> But the whole point of keeping this sysctl still around is not to
> break the userland scripts! If we are trying to achieve that, I think
> we should go one step further and make sure that the sysctl is still
> available for reading even in child namespaces (as it used to be the
> case).


network namespaces make some sysctl disappear.

That is how it is since day 0.
Some scripts might even use this knowledge.

It is too late to change something now. Nobody but you ever complained.

If you want to use network namespaces, you have to adapt your scripts.

Nobody claimed network namespaces were totally transparent.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Thu, Jan 8, 2015 at 11:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

>
> It is too late to change something now. Nobody but you ever complained.

Well, that is a strange argument! Nobody complained for a long time
that live capturing with vlan tagged packets were broken in tcpdump
because how the kernel started putting the tags in the aux data. But
someone eventually one day did notice that the issue was brought up.

>
> If you want to use network namespaces, you have to adapt your scripts.
>
> Nobody claimed network namespaces were totally transparent.
>

I see. I am going back to an old thread here where Linus says that the
#1 rule is:

""We don't regress user space"

https://lkml.org/lkml/2013/7/16/565

Breaking scripts seems to me to fall into the category of regressing
userspace. Or may be we can treat these sysctls more softly since they
are not strictly speaking linux ABIs.

Re: route/max_size sysctl in ipv4

[Eric Dumazet]

On Thu, 2015-01-08 at 12:13 -0800, Ani Sinha wrote:

> I see. I am going back to an old thread here where Linus says that the
> #1 rule is:
> 
> ""We don't regress user space"
> 
> https://lkml.org/lkml/2013/7/16/565

Thats totally different.

Look, I am going to stop the discussion here, because it seems you know
better than me.

Re: route/max_size sysctl in ipv4

[Cong Wang]

On Thu, Jan 8, 2015 at 12:13 PM, Ani Sinha <ani@arista.com> wrote:
> On Thu, Jan 8, 2015 at 11:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> If you want to use network namespaces, you have to adapt your scripts.
>>
>> Nobody claimed network namespaces were totally transparent.
>>
>
> I see. I am going back to an old thread here where Linus says that the
> #1 rule is:
>
> ""We don't regress user space"
>
> https://lkml.org/lkml/2013/7/16/565
>
> Breaking scripts seems to me to fall into the category of regressing
> userspace. Or may be we can treat these sysctls more softly since they
> are not strictly speaking linux ABIs.

As Eric said, it has been like this since day 0, why you still think
we break something? It is you who misunderstands the interface
not us who break your script.

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Fri, Jan 9, 2015 at 10:47 AM, Cong Wang <cwang@twopensource.com> wrote:
> On Thu, Jan 8, 2015 at 12:13 PM, Ani Sinha <ani@arista.com> wrote:
>> On Thu, Jan 8, 2015 at 11:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>
>>> If you want to use network namespaces, you have to adapt your scripts.
>>>
>>> Nobody claimed network namespaces were totally transparent.
>>>
>>
>> I see. I am going back to an old thread here where Linus says that the
>> #1 rule is:
>>
>> ""We don't regress user space"
>>
>> https://lkml.org/lkml/2013/7/16/565
>>
>> Breaking scripts seems to me to fall into the category of regressing
>> userspace. Or may be we can treat these sysctls more softly since they
>> are not strictly speaking linux ABIs.
>
> As Eric said, it has been like this since day 0,

I beg to differ. It has not been like this for that particular sysctl
from day 0. That sysctl was available from a child namespace and now
it isn't.

why you still think
> we break something? It is you who misunderstands the interface
> not us who break your script.

Perhaps. What I am truly confused about is :

- We are keeping a sysctl interface that does absolutely nothing in
the kernel and is completely useless in case some userland
scripts/tools are rendered broken from it's removal.

- surprisingly, we contradict ourselves when we let scripts break when
running from a child namespace because the  same sysctl is no longer
available!

When the source is available for a script or tool, it's easy to change
the code to conform to the new semantics. However, for old binaries
for which we do not have any source, it's not easy or is impossible to
fix them.

I rest my case. We will of course find a way to fix our code if that
is what netdev thinks is the way to go.

Re: route/max_size sysctl in ipv4

[Cong Wang]

On Fri, Jan 9, 2015 at 11:08 AM, Ani Sinha <ani@arista.com> wrote:
>
> Perhaps. What I am truly confused about is :
>
> - We are keeping a sysctl interface that does absolutely nothing in
> the kernel and is completely useless in case some userland
> scripts/tools are rendered broken from it's removal.

I am all for getting rid of it, sane script should always check
the sysctl existence first. I think we did remove some sysctl's
from kernel before.

>
> - surprisingly, we contradict ourselves when we let scripts break when
> running from a child namespace because the  same sysctl is no longer
> available!
>

I am not sure how exactly your script is broken, but it should
test the existence of any /proc file before reading it, even when
you don't use netns, because this could be a new sysctl introduced
recently (probably not the case for ip_rt_max_size, but you get my point).

Re: route/max_size sysctl in ipv4

[Ani Sinha]

On Fri, Jan 9, 2015 at 11:37 AM, Cong Wang <cwang@twopensource.com> wrote:
> On Fri, Jan 9, 2015 at 11:08 AM, Ani Sinha <ani@arista.com> wrote:
>>
>> Perhaps. What I am truly confused about is :
>>
>> - We are keeping a sysctl interface that does absolutely nothing in
>> the kernel and is completely useless in case some userland
>> scripts/tools are rendered broken from it's removal.
>
> I am all for getting rid of it, sane script should always check
> the sysctl existence first. I think we did remove some sysctl's
> from kernel before.

I'd be much happier if things break uniformly everywhere - within and
outside namespaces. Besides, keeping a useless sysctl around that does
nothing is confusing and error prone, to say the least.

If we do decide to keep the sysctl around, I beg that we at least
update the documentation to reflect the true fact. I have already sent
a patch for this.