* inet_diag_dump_icsk() change seems bogus...
@ 2014-02-02 8:45 David Miller
2014-02-02 15:43 ` Neal Cardwell
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2014-02-02 8:45 UTC (permalink / raw)
To: netdev; +Cc: ncardwell
Upon further review, commit 70315d22d3c7383f9a508d0aab21e2eb35b2303a
("inet_diag: fix inet_diag_dump_icsk() to use correct state for
timewait sockets") doesn't seem valid to me.
Take a close look at get_tcp4_sock() and get_timewait4_sock() which
you reference in your commit message.
The former always gets it's socket from head->chain and the latter
always gets it's socket from head->twchain.
Yet in this inet_diag_dump_icsk() change, you're changing the
head->chain iterator to check for timewait sockets. That doesn't
seem possible.
Only head->twchain holds timewait sockets, and this code was handling
it correctly already.
What gives?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: inet_diag_dump_icsk() change seems bogus...
2014-02-02 8:45 inet_diag_dump_icsk() change seems bogus David Miller
@ 2014-02-02 15:43 ` Neal Cardwell
2014-02-02 18:52 ` Eric Dumazet
2014-02-02 21:00 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Neal Cardwell @ 2014-02-02 15:43 UTC (permalink / raw)
To: David Miller; +Cc: Netdev
Hi David,
That was true in 3.12 and earlier, but AFAICT since Eric's 05dbc7b
("tcp/dccp: remove twchain") in 3.13-rc1, there is no head->twchain,
and instead all the connections (both tcp_sock and timewait flavor)
are on the head->chain, so that we do need to check for timewait
sockets. :-)
neal
On Sun, Feb 2, 2014 at 3:45 AM, David Miller <davem@davemloft.net> wrote:
>
> Upon further review, commit 70315d22d3c7383f9a508d0aab21e2eb35b2303a
> ("inet_diag: fix inet_diag_dump_icsk() to use correct state for
> timewait sockets") doesn't seem valid to me.
>
> Take a close look at get_tcp4_sock() and get_timewait4_sock() which
> you reference in your commit message.
>
> The former always gets it's socket from head->chain and the latter
> always gets it's socket from head->twchain.
>
> Yet in this inet_diag_dump_icsk() change, you're changing the
> head->chain iterator to check for timewait sockets. That doesn't
> seem possible.
>
> Only head->twchain holds timewait sockets, and this code was handling
> it correctly already.
>
> What gives?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: inet_diag_dump_icsk() change seems bogus...
2014-02-02 15:43 ` Neal Cardwell
@ 2014-02-02 18:52 ` Eric Dumazet
2014-02-02 18:59 ` Neal Cardwell
2014-02-02 21:00 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2014-02-02 18:52 UTC (permalink / raw)
To: Neal Cardwell; +Cc: David Miller, Netdev
On Sun, 2014-02-02 at 10:43 -0500, Neal Cardwell wrote:
> Hi David,
>
> That was true in 3.12 and earlier, but AFAICT since Eric's 05dbc7b
> ("tcp/dccp: remove twchain") in 3.13-rc1, there is no head->twchain,
> and instead all the connections (both tcp_sock and timewait flavor)
> are on the head->chain, so that we do need to check for timewait
> sockets. :-)
Note that Neal fix needs a different form for stable trees.
Thats maybe David concern ;)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: inet_diag_dump_icsk() change seems bogus...
2014-02-02 18:52 ` Eric Dumazet
@ 2014-02-02 18:59 ` Neal Cardwell
2014-02-02 21:03 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Neal Cardwell @ 2014-02-02 18:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Netdev
On Sun, Feb 2, 2014 at 1:52 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sun, 2014-02-02 at 10:43 -0500, Neal Cardwell wrote:
>> Hi David,
>>
>> That was true in 3.12 and earlier, but AFAICT since Eric's 05dbc7b
>> ("tcp/dccp: remove twchain") in 3.13-rc1, there is no head->twchain,
>> and instead all the connections (both tcp_sock and timewait flavor)
>> are on the head->chain, so that we do need to check for timewait
>> sockets. :-)
>
>
> Note that Neal fix needs a different form for stable trees.
>
> Thats maybe David concern ;)
Ah, makes sense. :-) I have a tested version of the patch for pre-3.13
kernels if anyone is interested in that.
neal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: inet_diag_dump_icsk() change seems bogus...
2014-02-02 15:43 ` Neal Cardwell
2014-02-02 18:52 ` Eric Dumazet
@ 2014-02-02 21:00 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2014-02-02 21:00 UTC (permalink / raw)
To: ncardwell; +Cc: netdev
From: Neal Cardwell <ncardwell@google.com>
Date: Sun, 2 Feb 2014 10:43:10 -0500
> That was true in 3.12 and earlier, but AFAICT since Eric's 05dbc7b
> ("tcp/dccp: remove twchain") in 3.13-rc1, there is no head->twchain,
> and instead all the connections (both tcp_sock and timewait flavor)
> are on the head->chain, so that we do need to check for timewait
> sockets. :-)
Therefore I don't need to -stable backport this change, ok makes
sense, thanks for explaining.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: inet_diag_dump_icsk() change seems bogus...
2014-02-02 18:59 ` Neal Cardwell
@ 2014-02-02 21:03 ` David Miller
2014-02-03 1:41 ` Neal Cardwell
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2014-02-02 21:03 UTC (permalink / raw)
To: ncardwell; +Cc: eric.dumazet, netdev
From: Neal Cardwell <ncardwell@google.com>
Date: Sun, 2 Feb 2014 13:59:06 -0500
> Ah, makes sense. :-) I have a tested version of the patch for pre-3.13
> kernels if anyone is interested in that.
Please post :-) Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: inet_diag_dump_icsk() change seems bogus...
2014-02-02 21:03 ` David Miller
@ 2014-02-03 1:41 ` Neal Cardwell
0 siblings, 0 replies; 7+ messages in thread
From: Neal Cardwell @ 2014-02-03 1:41 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, Netdev
On Sun, Feb 2, 2014 at 4:03 PM, David Miller <davem@davemloft.net> wrote:
> From: Neal Cardwell <ncardwell@google.com>
> Date: Sun, 2 Feb 2014 13:59:06 -0500
>
>> Ah, makes sense. :-) I have a tested version of the patch for pre-3.13
>> kernels if anyone is interested in that.
>
> Please post :-) Thanks!
Sure. Posted here:
http://patchwork.ozlabs.org/patch/316053/
neal
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-03 1:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-02 8:45 inet_diag_dump_icsk() change seems bogus David Miller
2014-02-02 15:43 ` Neal Cardwell
2014-02-02 18:52 ` Eric Dumazet
2014-02-02 18:59 ` Neal Cardwell
2014-02-02 21:03 ` David Miller
2014-02-03 1:41 ` Neal Cardwell
2014-02-02 21:00 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.