From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH RFC net-next] net: ipvs: Adjust gso_size for IPPROTO_TCP Date: Thu, 3 May 2018 21:50:49 -0600 Message-ID: References: <20180419212324.1542504-1-kafai@fb.com> <20180502171041.s3euld6i7hm6bw5c@kafai-mbp> <20180503070114.bcuusvzga45klccs@kafai-mbp> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, Tom Herbert , Eric Dumazet , Nikita Shirokov , kernel-team@fb.com, lvs-devel@vger.kernel.org To: Martin KaFai Lau , Julian Anastasov Return-path: Received: from mail-pf0-f182.google.com ([209.85.192.182]:40713 "EHLO mail-pf0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751236AbeEDDux (ORCPT ); Thu, 3 May 2018 23:50:53 -0400 In-Reply-To: <20180503070114.bcuusvzga45klccs@kafai-mbp> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 5/3/18 1:01 AM, Martin KaFai Lau wrote: > On Wed, May 02, 2018 at 10:30:32PM +0300, Julian Anastasov wrote: >> >> Hello, >> >> On Wed, 2 May 2018, Martin KaFai Lau wrote: >> >>> On Wed, May 02, 2018 at 09:38:43AM +0300, Julian Anastasov wrote: >>>> >>>> - initial traffic for port 21 does not use GSO. But after >>>> every packet IPVS calls maybe_update_pmtu (rt->dst.ops->update_pmtu) >>>> to report the reduced MTU. These updates are stored in fnhe_pmtu >>>> but they do not go to any route, even if we try to get fresh >>>> output route. Why? Because the local routes are not cached, so >>>> they can not use the fnhe. This is what my patch for route.c >>>> will fix. With this fix FTP-DATA gets route with reduced PMTU. >>> For IPv6, the 'if (rt6->rt6i_flags & RTF_LOCAL)' gate in >>> __ip6_rt_update_pmtu() may need to be lifted also. >> >> Probably. I completely forgot the IPv6 part >> but as I don't know the IPv6 code enough, it may take >> some time to understand what can be the problem there... >> I'm not sure whether everything started with commit 0a6b2a1dc2a2, >> so that in some configurations before that commit things >> worked and problem was not noticed. >> >> I think, we should focus on such direction for IPv6: >> >> - do we remember per-VIP PMTU for the local routes > IPv6 used not to create cache route for DST_HOST route which > is a /128 route (that includes local /128 route). > > Because of this, it had a bug such that a PMTU for the DST_HOST > route will trigger dst.ops->update_pmtu() which then set > an expire on the permanent /128 route instead of a cache > route. The permanent route got unexpectedly expired/removed > later. > > The fix was to allow creating /128 cache route as long as > it is not RTF_LOCAL in 653437d02f1f and 7035870d1219. The > first post spelled out the problem better: > https://patchwork.ozlabs.org/patch/456050/ > > Later, when we only create cache route after seeing PMTU > in 45e4fd26683c, this RTF_LOCAL checking was carried over > to __ip6_rt_update_pmtu(). > > Out of my head, I don't see issue removing the > RTF_LOCAL check from __ip6_rt_update_pmtu(). > DavidA, what do you think? I agree; I think it is fine. A route is route. The IPVS use cases with local redirects are blurring the line with needs for local routes.