2015-04-01 09:58:57

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH v3] net: sysctl for RA default route MTU

31.03.2015, 23:49, "David Miller" <[email protected]>:
> From: Hannes Frederic Sowa <[email protected]>
> Date: Tue, 31 Mar 2015 22:35:48 +0200
>> ?Could you quickly comment on what you had in mind? I guess it is about
>> ?handling RA in user space on the end hosts and overwriting MTU during
>> ?insertion of the routes?
>
> Even after reading your email I have no idea why you can't just have
> RA provide a 1500 byte MTU, everything else uses the device's 9000
> MTU, problem solved?

Because the MTU (provided by RA) is assigned to the device.

Thanks,
Roman


2015-04-01 17:55:36

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3] net: sysctl for RA default route MTU

From: Roman Gushchin <[email protected]>
Date: Wed, 01 Apr 2015 12:58:50 +0300

> 31.03.2015, 23:49, "David Miller" <[email protected]>:
>> From: Hannes Frederic Sowa <[email protected]>
>> Date: Tue, 31 Mar 2015 22:35:48 +0200
>>> ?Could you quickly comment on what you had in mind? I guess it is about
>>> ?handling RA in user space on the end hosts and overwriting MTU during
>>> ?insertion of the routes?
>>
>> Even after reading your email I have no idea why you can't just have
>> RA provide a 1500 byte MTU, everything else uses the device's 9000
>> MTU, problem solved?
>
> Because the MTU (provided by RA) is assigned to the device.

Ok, that severely limits the usefulness of this option I guess.

The next question I have is about the behavior of the new setting
in the presence of an RA MTU option. It seems like the sysctl
doesn't override that RA MTU option, but rather just clamps it.

And then if it's in range, this controls only whether the default
route has it's MTU adjusted.

That doesn't make any sense to me if we then go and do the
rt6_mtu_change() call unconditionally. The route metric update
and the rt6_mtu_change() go hand in hand.

2015-04-01 19:33:08

by Hannes Frederic Sowa

[permalink] [raw]
Subject: Re: [PATCH v3] net: sysctl for RA default route MTU



On Wed, Apr 1, 2015, at 19:55, David Miller wrote:
> From: Roman Gushchin <[email protected]>
> Date: Wed, 01 Apr 2015 12:58:50 +0300
>
> > 31.03.2015, 23:49, "David Miller" <[email protected]>:
> >> From: Hannes Frederic Sowa <[email protected]>
> >> Date: Tue, 31 Mar 2015 22:35:48 +0200
> >>>  Could you quickly comment on what you had in mind? I guess it is about
> >>>  handling RA in user space on the end hosts and overwriting MTU during
> >>>  insertion of the routes?
> >>
> >> Even after reading your email I have no idea why you can't just have
> >> RA provide a 1500 byte MTU, everything else uses the device's 9000
> >> MTU, problem solved?
> >
> > Because the MTU (provided by RA) is assigned to the device.
>
> Ok, that severely limits the usefulness of this option I guess.
>
> The next question I have is about the behavior of the new setting
> in the presence of an RA MTU option. It seems like the sysctl
> doesn't override that RA MTU option, but rather just clamps it.
>
> And then if it's in range, this controls only whether the default
> route has it's MTU adjusted.
>
> That doesn't make any sense to me if we then go and do the
> rt6_mtu_change() call unconditionally. The route metric update
> and the rt6_mtu_change() go hand in hand.

Agreed but that gets interesting:

I guess during testing the cnf.mtu6 value was equal to the newly
announced mtu value, so the rt6_mtu_change call does not happen. We
update cnf.mtu6 so a second RA packet would actually bring the system
into the desired state but we have a moment where the default route
carries a too big MTU. That's not good.

Easiest solution is to reorder those calls but that also leaves us with
a time frame where we carry the incorrect MTU on the default route.
Otherwise we must conditionally filter out the default routes.

Roman, any ideas?

Thanks,
Hannes

2015-04-02 18:08:48

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH v3] net: sysctl for RA default route MTU

>> ?The next question I have is about the behavior of the new setting
>> ?in the presence of an RA MTU option. ?It seems like the sysctl
>> ?doesn't override that RA MTU option, but rather just clamps it.
>>
>> ?And then if it's in range, this controls only whether the default
>> ?route has it's MTU adjusted.
>>
>> ?That doesn't make any sense to me if we then go and do the
>> ?rt6_mtu_change() call unconditionally. ?The route metric update
>> ?and the rt6_mtu_change() go hand in hand.
>
> Agreed but that gets interesting:
>
> I guess during testing the cnf.mtu6 value was equal to the newly
> announced mtu value, so the rt6_mtu_change call does not happen. We
> update cnf.mtu6 so a second RA packet would actually bring the system
> into the desired state but we have a moment where the default route
> carries a too big MTU. That's not good.

Agreed.

> Easiest solution is to reorder those calls but that also leaves us with
> a time frame where we carry the incorrect MTU on the default route.
> Otherwise we must conditionally filter out the default routes.
> Roman, any ideas?

I think, such approach will work on practise, but looks not very beatiful.

May be, a better idea is to serarate per-route and per-device MTU,
so an updating of per-device MTU will not affect per-route MTU.
Actual MTU can always been calculated as min(route_mtu, device_mtu),
but we wouldn't need to update mtu on each route on receiving RA MTU option,
for instance.

Do you see any problems with such approach?

Thanks,
Roman

2015-04-07 15:58:36

by Hannes Frederic Sowa

[permalink] [raw]
Subject: Re: [PATCH v3] net: sysctl for RA default route MTU

On Do, 2015-04-02 at 21:08 +0300, Roman Gushchin wrote:
> >> The next question I have is about the behavior of the new setting
> >> in the presence of an RA MTU option. It seems like the sysctl
> >> doesn't override that RA MTU option, but rather just clamps it.
> >>
> >> And then if it's in range, this controls only whether the default
> >> route has it's MTU adjusted.
> >>
> >> That doesn't make any sense to me if we then go and do the
> >> rt6_mtu_change() call unconditionally. The route metric update
> >> and the rt6_mtu_change() go hand in hand.
> >
> > Agreed but that gets interesting:
> >
> > I guess during testing the cnf.mtu6 value was equal to the newly
> > announced mtu value, so the rt6_mtu_change call does not happen. We
> > update cnf.mtu6 so a second RA packet would actually bring the system
> > into the desired state but we have a moment where the default route
> > carries a too big MTU. That's not good.
>
> Agreed.
>
> > Easiest solution is to reorder those calls but that also leaves us with
> > a time frame where we carry the incorrect MTU on the default route.
> > Otherwise we must conditionally filter out the default routes.
> > Roman, any ideas?
>
> I think, such approach will work on practise, but looks not very beatiful.
>
> May be, a better idea is to serarate per-route and per-device MTU,
> so an updating of per-device MTU will not affect per-route MTU.
> Actual MTU can always been calculated as min(route_mtu, device_mtu),
> but we wouldn't need to update mtu on each route on receiving RA MTU option,
> for instance.
>
> Do you see any problems with such approach?

If I understood you correct this actually seems to be quite an intrusive
change? :/ Can you show me some code how to do this?

I would also dislike adding a filtering capability to the route mtu
updates. Currently I don't have a god idea, sorry.

Bye,
Hannes

2015-04-08 19:04:01

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH v3] net: sysctl for RA default route MTU

07.04.2015, 18:58, "Hannes Frederic Sowa" <[email protected]>:
> ?On Do, 2015-04-02 at 21:08 +0300, Roman Gushchin wrote:
>>>> ???The next question I have is about the behavior of the new setting
>>>> ???in the presence of an RA MTU option. ?It seems like the sysctl
>>>> ???doesn't override that RA MTU option, but rather just clamps it.
>>>>
>>>> ???And then if it's in range, this controls only whether the default
>>>> ???route has it's MTU adjusted.
>>>>
>>>> ???That doesn't make any sense to me if we then go and do the
>>>> ???rt6_mtu_change() call unconditionally. ?The route metric update
>>>> ???and the rt6_mtu_change() go hand in hand.
>>> ??Agreed but that gets interesting:
>>>
>>> ??I guess during testing the cnf.mtu6 value was equal to the newly
>>> ??announced mtu value, so the rt6_mtu_change call does not happen. We
>>> ??update cnf.mtu6 so a second RA packet would actually bring the system
>>> ??into the desired state but we have a moment where the default route
>>> ??carries a too big MTU. That's not good.
>> ??Agreed.
>>> ??Easiest solution is to reorder those calls but that also leaves us with
>>> ??a time frame where we carry the incorrect MTU on the default route.
>>> ??Otherwise we must conditionally filter out the default routes.
>>> ??Roman, any ideas?
>> ??I think, such approach will work on practise, but looks not very beatiful.
>>
>> ??May be, a better idea is to serarate per-route and per-device MTU,
>> ??so an updating of per-device MTU will not affect per-route MTU.
>> ??Actual MTU can always been calculated as min(route_mtu, device_mtu),
>> ??but we wouldn't need to update mtu on each route on receiving RA MTU option,
>> ??for instance.
>>
>> ??Do you see any problems with such approach?
> ?If I understood you correct this actually seems to be quite an intrusive
> ?change? :/ Can you show me some code how to do this?

Too intrusive, really)

> ?I would also dislike adding a filtering capability to the route mtu
> ?updates. Currently I don't have a god idea, sorry.

Hmm, I thought a bit more about this issue... And It seems to me now, that there is no issue at all.
If RA MTU is larger than ra_default_route_mtu, rt6_mtu_change() will not update it,
because dst_mtu(&rt->dst) != idev->cnf.mtu6 :
if (rt->dst.dev == arg->dev &&
!dst_metric_locked(&rt->dst, RTAX_MTU) &&
(dst_mtu(&rt->dst) >= arg->mtu ||
(dst_mtu(&rt->dst) < arg->mtu &&
dst_mtu(&rt->dst) == idev->cnf.mtu6))) {
dst_metric_set(&rt->dst, RTAX_MTU, arg->mtu);
}
So, it's ok.

Otherwise, if RA MTU is lower than ra_default_route_mtu, rt6_mtu_change() will lower default route mtu, and it's ok too. There is a short period of time, when a newly created default route has too large MTU, but it's not scary. And it's exactly as it works now if new RA advertise MTU smaller than previous.

Do I miss something?

Thanks!

Regards,
Roman