These debug messages are uninformative and extremely alarming. Rather
than reporting a peer as a treacherous attacker, report it more
correctly as simply broken.
Signed-off-by: Matt Mackall <[email protected]>
diff -r 320a7bd38b8d net/ipv4/tcp_timer.c
--- a/net/ipv4/tcp_timer.c Mon Oct 27 17:33:24 2008 -0500
+++ b/net/ipv4/tcp_timer.c Thu Dec 18 16:06:39 2008 -0600
@@ -299,14 +299,14 @@
#ifdef TCP_DEBUG
struct inet_sock *inet = inet_sk(sk);
if (sk->sk_family == AF_INET) {
- LIMIT_NETDEBUG(KERN_DEBUG "TCP: Treason uncloaked! Peer " NIPQUAD_FMT ":%u/%u shrinks window %u:%u. Repaired.\n",
+ LIMIT_NETDEBUG(KERN_DEBUG "TCP: Broken peer " NIPQUAD_FMT ":%u/%u shrinks window %u:%u. Repaired.\n",
NIPQUAD(inet->daddr), ntohs(inet->dport),
inet->num, tp->snd_una, tp->snd_nxt);
}
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
else if (sk->sk_family == AF_INET6) {
struct ipv6_pinfo *np = inet6_sk(sk);
- LIMIT_NETDEBUG(KERN_DEBUG "TCP: Treason uncloaked! Peer " NIP6_FMT ":%u/%u shrinks window %u:%u. Repaired.\n",
+ LIMIT_NETDEBUG(KERN_DEBUG "TCP: Broken peer " NIP6_FMT ":%u/%u shrinks window %u:%u. Repaired.\n",
NIP6(np->daddr), ntohs(inet->dport),
inet->num, tp->snd_una, tp->snd_nxt);
}
--
Mathematics is the supreme nostalgia of our time.
On Thu, 18 Dec 2008, Matt Mackall wrote:
> These debug messages are uninformative and extremely alarming. Rather
> than reporting a peer as a treacherous attacker, report it more
> correctly as simply broken.
Last time (well, ignoring some report on ancient vendor kernel) I've seen
this to trigger it wasn't the fault of the peer but ours (we sent past the
advertized window and window ended up being shrunk to zero). I agree that
the message as-is is not very good but your change is not very good
either.
> - LIMIT_NETDEBUG(KERN_DEBUG "TCP: Treason uncloaked! Peer " NIPQUAD_FMT ":%u/%u shrinks window %u:%u. Repaired.\n",
> + LIMIT_NETDEBUG(KERN_DEBUG "TCP: Broken peer " NIPQUAD_FMT ":%u/%u shrinks window %u:%u. Repaired.\n",
This won't apply to net-next on which you should base anything new (and
net related).
--
i.
On Thu, Dec 18, 2008 at 10:33:46PM +0000, Ilpo J?rvinen wrote:
>
> Last time (well, ignoring some report on ancient vendor kernel) I've seen
> this to trigger it wasn't the fault of the peer but ours (we sent past the
> advertized window and window ended up being shrunk to zero). I agree that
> the message as-is is not very good but your change is not very good
> either.
Indeed I had fixed a bug in our kernel that triggered this too
so this is definitely treason :)
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Matt Mackall <[email protected]> wrote:
> These debug messages are uninformative and extremely alarming. Rather
> than reporting a peer as a treacherous attacker, report it more
> correctly as simply broken.
What's next, you're going to remove "printer on fire" as well?
This message has been there for eons and is part of Linux lore.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, 19 Dec 2008 09:44:32 +1100
Herbert Xu <[email protected]> wrote:
> Matt Mackall <[email protected]> wrote:
> > These debug messages are uninformative and extremely alarming. Rather
> > than reporting a peer as a treacherous attacker, report it more
> > correctly as simply broken.
>
> What's next, you're going to remove "printer on fire" as well?
> This message has been there for eons and is part of Linux lore.
It was changed. The printer on fire ? bit was adjusted to make it clear
because the original message *did* confuse and worry a few people.
IMHO Matt is right on this.
Alan
On Thu, Dec 18, 2008 at 10:49:05PM +0000, Alan Cox wrote:
>
> It was changed. The printer on fire ? bit was adjusted to make it clear
> because the original message *did* confuse and worry a few people.
Well
$ grep fire drivers/char/lp.c
printk(KERN_INFO "lp%d on fire\n", minor);
$
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, 2008-12-19 at 09:44 +1100, Herbert Xu wrote:
> Matt Mackall <[email protected]> wrote:
> > These debug messages are uninformative and extremely alarming. Rather
> > than reporting a peer as a treacherous attacker, report it more
> > correctly as simply broken.
>
> What's next, you're going to remove "printer on fire" as well?
> This message has been there for eons and is part of Linux lore.
No one has printer ports any more so it hardly seems worth the trouble.
Most people won't actually think their printer is on fire. But most
people WILL think there is serious cause for concern when they see this
for the first time in dmesg. Many will search the net for explanations
and come away confused and not entirely reassured. And at least one
clueless guy will call the police because he still thinks he's under
attack.
Now that certainly fits my definition of amusing and if my goal for
Linux was to amuse myself at the expense of users, I'd be all for
keeping it[1]. But perversely, I actually want users to enjoy their
Linux experience.
[1] Hell, I'd probably even get them to use git.
--
Mathematics is the supreme nostalgia of our time.
On Fri, 19 Dec 2008 10:02:53 +1100
Herbert Xu <[email protected]> wrote:
> On Thu, Dec 18, 2008 at 10:49:05PM +0000, Alan Cox wrote:
> >
> > It was changed. The printer on fire ? bit was adjusted to make it clear
> > because the original message *did* confuse and worry a few people.
>
> Well
>
> $ grep fire drivers/char/lp.c
> printk(KERN_INFO "lp%d on fire\n", minor);
Interesting. It got changed and then the change got lost between 2.2 and
2.4 somewhere. It was changed to report
"lp0 reported invalid error status (on fire, eh?)"
and it looks like that needs re-fixing.
On Fri, 2008-12-19 at 00:33 +0200, Ilpo Järvinen wrote:
> On Thu, 18 Dec 2008, Matt Mackall wrote:
>
> > These debug messages are uninformative and extremely alarming. Rather
> > than reporting a peer as a treacherous attacker, report it more
> > correctly as simply broken.
>
> Last time (well, ignoring some report on ancient vendor kernel) I've seen
> this to trigger it wasn't the fault of the peer but ours (we sent past the
> advertized window and window ended up being shrunk to zero). I agree that
> the message as-is is not very good but your change is not very good
> either.
Here this is the bulk of messages on my server (which is otherwise
fairly quiet).
It really doesn't matter whether the message is technically accurate -
it's mostly there for the benefit of kernel hackers. The average end
user is not going to have the slightest clue what is going on nor should
they even care. An advanced network admin might be able to figure things
out by poking at packet traces, but again: not much reason to care. For
most admins, the vast majority of these treasonous peers won't be under
their administrative control.
If you want, we can instead say "Peer %d unexpectedly shrunk window...".
So long as the scary bit is gone, I don't care.
--
Mathematics is the supreme nostalgia of our time.
From: Alan Cox <[email protected]>
Date: Thu, 18 Dec 2008 23:14:07 +0000
> On Fri, 19 Dec 2008 10:02:53 +1100
> Herbert Xu <[email protected]> wrote:
>
> > Well
> >
> > $ grep fire drivers/char/lp.c
> > printk(KERN_INFO "lp%d on fire\n", minor);
>
> Interesting. It got changed and then the change got lost between 2.2 and
> 2.4 somewhere. It was changed to report
>
> "lp0 reported invalid error status (on fire, eh?)"
>
> and it looks like that needs re-fixing.
This is just proof that it doesn't matter. If it got lost in
2.4.x that means it's been like this forever as far as %99
of current systems are concerned.
I'm all for seriousness, but if we can't have some comedic license
with a few kernel log messages then the fun of kernel development
truly suffers :-)
From: "Ilpo J?rvinen" <[email protected]>
Date: Fri, 19 Dec 2008 00:33:46 +0200 (EET)
> On Thu, 18 Dec 2008, Matt Mackall wrote:
>
> > These debug messages are uninformative and extremely alarming. Rather
> > than reporting a peer as a treacherous attacker, report it more
> > correctly as simply broken.
>
> Last time (well, ignoring some report on ancient vendor kernel) I've seen
> this to trigger it wasn't the fault of the peer but ours (we sent past the
> advertized window and window ended up being shrunk to zero). I agree that
> the message as-is is not very good but your change is not very good
> either.
...
> This won't apply to net-next on which you should base anything new (and
> net related).
Agreed.
From: Matt Mackall <[email protected]>
Date: Thu, 18 Dec 2008 17:50:34 -0600
> If you want, we can instead say "Peer %d unexpectedly shrunk window...".
> So long as the scary bit is gone, I don't care.
Since we have established that the blame is equally possible to
be the local as the remote system, something like:
TCP: Unexpected shrink of advertised window detected ...
is probably the best.
> Most people won't actually think their printer is on fire. But most
> people WILL think there is serious cause for concern when they see this
> for the first time in dmesg. Many will search the net for explanations
> and come away confused and not entirely reassured. And at least one
> clueless guy will call the police because he still thinks he's under
> attack.
Messages about something or other being "illegal" occur about 345 times in
the Linux kernel source code. Are we going to start patching those too?
We have "illegal norm", "illegal input", "illegal call", "illegal type",
"illegal bits", "illegal root port number", "illegal host number", "illegal
DMA data", "illegal dimensions" (who do you call about that one?), "illegal
phase", "illegal page number", "illegal seek", and on, and on, and on.
I suspect that if I submitted a patch to change all of those to "invalid",
I'd be considered a kook. (Well, more of a kook, anyway.)
DS
"David Schwartz" <[email protected]> writes:
> Messages about something or other being "illegal" occur about 345 times in
> the Linux kernel source code. Are we going to start patching those too?
I think we should.
Except for those which are part of some reasonable userspace interface.
--
Krzysztof Halasa
David Schwartz wrote:
>>Most people won't actually think their printer is on fire. But most
>>people WILL think there is serious cause for concern when they see this
>>for the first time in dmesg. Many will search the net for explanations
>>and come away confused and not entirely reassured. And at least one
>>clueless guy will call the police because he still thinks he's under
>>attack.
>
>
> Messages about something or other being "illegal" occur about 345 times in
> the Linux kernel source code. Are we going to start patching those too?
>
> We have "illegal norm", "illegal input", "illegal call", "illegal type",
> "illegal bits", "illegal root port number", "illegal host number", "illegal
> DMA data", "illegal dimensions" (who do you call about that one?), "illegal
> phase", "illegal page number", "illegal seek", and on, and on, and on.
>
> I suspect that if I submitted a patch to change all of those to "invalid",
> I'd be considered a kook. (Well, more of a kook, anyway.)
All issues of your kookiness aside :) I suspect that the continuing
spread of "linux" among the masses will mean a need for greater care in
message wording. Whether that means that "illegal <foo>" needs to be
changed I cannot say, but if it does it shouldn't come as a surprise.
rick jones
On Thu, 2008-12-18 at 16:15 -0800, David Miller wrote:
> From: Matt Mackall <[email protected]>
> Date: Thu, 18 Dec 2008 17:50:34 -0600
>
> > If you want, we can instead say "Peer %d unexpectedly shrunk window...".
> > So long as the scary bit is gone, I don't care.
>
> Since we have established that the blame is equally possible to
> be the local as the remote system, something like:
>
> TCP: Unexpected shrink of advertised window detected ...
>
> is probably the best.
How's this:
The original message was unhelpful and extremely alarming to our poor
users, despite its charm. Make it less frightening.
Signed-off-by: Matt Mackall <[email protected]>
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index cc4e6d2..0170e91 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -299,14 +299,14 @@ static void tcp_retransmit_timer(struct sock *sk)
#ifdef TCP_DEBUG
struct inet_sock *inet = inet_sk(sk);
if (sk->sk_family == AF_INET) {
- LIMIT_NETDEBUG(KERN_DEBUG "TCP: Treason uncloaked! Peer %pI4:%u/%u shrinks window %u:%u. Repaired.\n",
+ LIMIT_NETDEBUG(KERN_DEBUG "TCP: Peer %pI4:%u/%u unexpectedly shrunk window %u:%u (repaired)\n",
&inet->daddr, ntohs(inet->dport),
inet->num, tp->snd_una, tp->snd_nxt);
}
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
else if (sk->sk_family == AF_INET6) {
struct ipv6_pinfo *np = inet6_sk(sk);
- LIMIT_NETDEBUG(KERN_DEBUG "TCP: Treason uncloaked! Peer %pI6:%u/%u shrinks window %u:%u. Repaired.\n",
+ LIMIT_NETDEBUG(KERN_DEBUG "TCP: Peer %pI6:%u/%u unexpectedly shrunk window %u:%u (repaired)\n",
&np->daddr, ntohs(inet->dport),
inet->num, tp->snd_una, tp->snd_nxt);
}
--
Mathematics is the supreme nostalgia of our time.
From: Matt Mackall <[email protected]>
Date: Thu, 18 Dec 2008 21:49:21 -0600
> On Thu, 2008-12-18 at 16:15 -0800, David Miller wrote:
> > From: Matt Mackall <[email protected]>
> > Date: Thu, 18 Dec 2008 17:50:34 -0600
> >
> > > If you want, we can instead say "Peer %d unexpectedly shrunk window...".
> > > So long as the scary bit is gone, I don't care.
> >
> > Since we have established that the blame is equally possible to
> > be the local as the remote system, something like:
> >
> > TCP: Unexpected shrink of advertised window detected ...
> >
> > is probably the best.
>
> How's this:
>
> The original message was unhelpful and extremely alarming to our poor
> users, despite its charm. Make it less frightening.
>
> Signed-off-by: Matt Mackall <[email protected]>
Looks good, applied, thanks Matt.
* David Miller <[email protected]> wrote:
> TCP: Unexpected shrink of advertised window detected ...
>
> is probably the best.
What about this?
TCP: A Momentary Lapse Of Treason detected ...
SCNR
--
left blank, right bald
On Thu, Dec 18, 2008 at 05:06:13PM -0600, Matt Mackall wrote:
> No one has printer ports any more so it hardly seems worth the trouble.
USB printers go through the same code.
> Most people won't actually think their printer is on fire. But most
> people WILL think there is serious cause for concern when they see this
> for the first time in dmesg. Many will search the net for explanations
> and come away confused and not entirely reassured. And at least one
> clueless guy will call the police because he still thinks he's under
> attack.
I dealt with an epson inkjet (USB based) a few years ago that would
reliably trigger that message when it was out of ink. I thought it was
ammusing that they picked that signal to indicate out of ink.
Not sure if current models do it too.
> Now that certainly fits my definition of amusing and if my goal for
> Linux was to amuse myself at the expense of users, I'd be all for
> keeping it[1]. But perversely, I actually want users to enjoy their
> Linux experience.
>
> [1] Hell, I'd probably even get them to use git.
--
Len Sorensen