From: Florian Westphal <fw@strlen.de>
To: "Vicente Jiménez" <googuy@gmail.com>
Cc: Florian Westphal <fw@strlen.de>, David Miller <davem@redhat.com>,
Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
James Morris <jmorris@namei.org>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
Patrick McHardy <kaber@trash.net>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] icmp: Restore resistence to abnormal messages
Date: Wed, 16 Nov 2016 02:14:04 +0100 [thread overview]
Message-ID: <20161116011404.GF30581@breakpoint.cc> (raw)
In-Reply-To: <CAO1wt+aZjRE_KTY0iRNJryeHvehrK6kuGMn6wOOeFuT1ncpPxA@mail.gmail.com>
Vicente Jiménez <googuy@gmail.com> wrote:
> I agree that both patches try to solve the same problem in a very similar way.
> Florian Westphal's patch do two more things:
> 1- add warning with pr_warn_ratelimited. I like this idea. I also
> though about adding some message but I have no kernel experience and I
> preferred to have just a working solution.
I added this only to show whats happening.
I don't like such printks because end users can't do anything about it.
> 2- Check if the packet size is lower than (536 + 8). I think this is
> not necessary because low values (even the zero case) is already
> handled by the protocol. Also I don't understand why you choose this
> value, it seems to be related to TCP MSS and the compared value is IP
> packet size.
Right, no need for this check.
> Finally, both patches decrement current packet by a value: Mine by 2
> and Florian's by 8 bytes. Both arbitrary values. Personally I prefer
> to go by small steps. If the small step fails, it just iterate again
> and with 4 iterations, my patch also decrement the original value by 8
> bytes (4x2).
> Basically they are the same but my patch take smaller steps and miss
> the warning message.
IIRC I chose 8 because connection recovered faster in my case.
I have not experienced this issue again (I dropped the patch from
my kernel at some point and the connection stalls did not reappear so
this got fixed elsewhere).
I'd just apply your patch, possibly with an additional comment that
says that we're grasping at straws because some middlebox is evidently
feeding bogus pmtu information.
next prev parent reply other threads:[~2016-11-16 1:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-11 20:20 [PATCH] icmp: Restore resistence to abnormal messages Vicente Jimenez Aguilar
2016-11-14 18:36 ` David Miller
2016-11-15 16:49 ` Vicente Jiménez
2016-11-15 16:56 ` David Miller
2016-11-15 17:30 ` Florian Westphal
2016-11-15 19:32 ` Vicente Jiménez
2016-11-16 1:14 ` Florian Westphal [this message]
2016-11-17 1:17 ` Vicente Jiménez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161116011404.GF30581@breakpoint.cc \
--to=fw@strlen.de \
--cc=davem@redhat.com \
--cc=googuy@gmail.com \
--cc=jmorris@namei.org \
--cc=kaber@trash.net \
--cc=kuznet@ms2.inr.ac.ru \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).