From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754769AbbDHTEB (ORCPT ); Wed, 8 Apr 2015 15:04:01 -0400 Received: from forward-corp1g.mail.yandex.net ([95.108.253.251]:57487 "EHLO forward-corp1g.mail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753163AbbDHTD7 (ORCPT ); Wed, 8 Apr 2015 15:03:59 -0400 From: Roman Gushchin To: Hannes Frederic Sowa Cc: David Miller , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" In-Reply-To: <1428422310.2928.8.camel@stressinduktion.org> References: <1427834148.979686.247747037.29964C1B@webmail.messagingengine.com> <20150331.164923.229749666073516444.davem@davemloft.net> <68621427882330@webcorp01g.yandex-team.ru> <20150401.135532.1368728758929086692.davem@davemloft.net> <1427916447.1816412.248215129.072B6ABF@webmail.messagingengine.com> <36791427998117@webcorp02f.yandex-team.ru> <1428422310.2928.8.camel@stressinduktion.org> Subject: Re: [PATCH v3] net: sysctl for RA default route MTU MIME-Version: 1.0 Message-Id: <211371428519836@webcorp02f.yandex-team.ru> X-Mailer: Yamail [ http://yandex.ru ] 5.0 Date: Wed, 08 Apr 2015 22:03:56 +0300 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=koi8-r Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 07.04.2015, 18:58, "Hannes Frederic Sowa" : > š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