Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 04/01/2016 08:33 PM, David Miller wrote: From: Daniel Borkmann Date: Fri, 01 Apr 2016 10:10:11 +0200 Dave, do you need me to resubmit this one w/o changes: http://patchwork.ozlabs.org/patch/603903/ ? I'll apply this and queue it up for -stable, thanks. Ok, thanks!

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

From: Daniel Borkmann Date: Fri, 01 Apr 2016 10:10:11 +0200 > Dave, do you need me to resubmit this one w/o changes: > http://patchwork.ozlabs.org/patch/603903/ ? I'll apply this and queue it up for -stable, thanks.

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 04/01/2016 06:33 AM, Hannes Frederic Sowa wrote: On 01.04.2016 06:26, Alexei Starovoitov wrote: On Fri, Apr 01, 2016 at 06:12:49AM +0200, Hannes Frederic Sowa wrote: On 01.04.2016 06:04, Alexei Starovoitov wrote: On Thu, Mar 31, 2016 at 08:03:38PM -0700, Eric Dumazet wrote: On Thu,

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 01.04.2016 06:26, Alexei Starovoitov wrote: On Fri, Apr 01, 2016 at 06:12:49AM +0200, Hannes Frederic Sowa wrote: On 01.04.2016 06:04, Alexei Starovoitov wrote: On Thu, Mar 31, 2016 at 08:03:38PM -0700, Eric Dumazet wrote: On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote:

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, Apr 01, 2016 at 06:12:49AM +0200, Hannes Frederic Sowa wrote: > On 01.04.2016 06:04, Alexei Starovoitov wrote: > >On Thu, Mar 31, 2016 at 08:03:38PM -0700, Eric Dumazet wrote: > >>On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote: > >> > >>>Eric, what's your take on Hannes's

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 01.04.2016 06:04, Alexei Starovoitov wrote: On Thu, Mar 31, 2016 at 08:03:38PM -0700, Eric Dumazet wrote: On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote: Eric, what's your take on Hannes's patch 2 ? Is it more accurate to ask lockdep to check for actual lock or lockdep can

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Thu, Mar 31, 2016 at 08:03:38PM -0700, Eric Dumazet wrote: > On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote: > > > Eric, what's your take on Hannes's patch 2 ? > > Is it more accurate to ask lockdep to check for actual lock > > or lockdep can rely on owned flag? > > Potentially

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, Apr 1, 2016, at 05:13, Eric Dumazet wrote: > On Fri, 2016-04-01 at 04:01 +0200, Hannes Frederic Sowa wrote: > > > I thought so first, as well. But given the double check for the > > spin_lock and the "mutex" we end up with the same result for the > > lockdep_sock_is_held check. > > >

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, 2016-04-01 at 04:01 +0200, Hannes Frederic Sowa wrote: > I thought so first, as well. But given the double check for the > spin_lock and the "mutex" we end up with the same result for the > lockdep_sock_is_held check. > > Do you see other consequences? Well, we release the spinlock in

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, Apr 1, 2016, at 05:03, Eric Dumazet wrote: > On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote: > > > Eric, what's your take on Hannes's patch 2 ? > > Is it more accurate to ask lockdep to check for actual lock > > or lockdep can rely on owned flag? > > Potentially there could

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Thu, 2016-03-31 at 18:45 -0700, Alexei Starovoitov wrote: > Eric, what's your take on Hannes's patch 2 ? > Is it more accurate to ask lockdep to check for actual lock > or lockdep can rely on owned flag? > Potentially there could be races between setting the flag and > actual lock... but that

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 01.04.2016 03:45, Eric Dumazet wrote: On Thu, 2016-03-31 at 18:39 -0700, Eric Dumazet wrote: On Fri, 2016-04-01 at 03:36 +0200, Hannes Frederic Sowa wrote: On Fri, Apr 1, 2016, at 03:19, Eric Dumazet wrote: Thanks. As you can see, release_sock() messes badly lockdep (once your other

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 01.04.2016 03:39, Eric Dumazet wrote: On Fri, 2016-04-01 at 03:36 +0200, Hannes Frederic Sowa wrote: On Fri, Apr 1, 2016, at 03:19, Eric Dumazet wrote: Thanks. As you can see, release_sock() messes badly lockdep (once your other patches are in ) Once we properly fix release_sock() and/or

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Thu, Mar 31, 2016 at 06:19:52PM -0700, Eric Dumazet wrote: > On Fri, 2016-04-01 at 02:21 +0200, Hannes Frederic Sowa wrote: > > > > > [ 31.064029] === > > [ 31.064030] [ INFO: suspicious RCU usage. ] > > [ 31.064032] 4.5.0+ #13 Not tainted > > [ 31.064033]

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Thu, 2016-03-31 at 18:39 -0700, Eric Dumazet wrote: > On Fri, 2016-04-01 at 03:36 +0200, Hannes Frederic Sowa wrote: > > On Fri, Apr 1, 2016, at 03:19, Eric Dumazet wrote: > > > Thanks. > > > > > > As you can see, release_sock() messes badly lockdep (once your other > > > patches are in ) > >

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, 2016-04-01 at 03:36 +0200, Hannes Frederic Sowa wrote: > On Fri, Apr 1, 2016, at 03:19, Eric Dumazet wrote: > > Thanks. > > > > As you can see, release_sock() messes badly lockdep (once your other > > patches are in ) > > > > Once we properly fix release_sock() and/or __release_sock(),

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, Apr 1, 2016, at 03:23, Eric Dumazet wrote: > On Fri, 2016-04-01 at 02:30 +0200, Hannes Frederic Sowa wrote: > > > I fixed this one, I wait with review a bit and collapse some of the > > newer fixes into one and check and repost again tomorrow. > > No change should be needed in TCP,

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, Apr 1, 2016, at 03:19, Eric Dumazet wrote: > Thanks. > > As you can see, release_sock() messes badly lockdep (once your other > patches are in ) > > Once we properly fix release_sock() and/or __release_sock(), all these > false positives disappear. This was a loopback connection. I need

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, 2016-04-01 at 02:30 +0200, Hannes Frederic Sowa wrote: > I fixed this one, I wait with review a bit and collapse some of the > newer fixes into one and check and repost again tomorrow. No change should be needed in TCP, once net/core/sock.c is fixed. When someone fixes a lockdep issue,

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, 2016-04-01 at 02:21 +0200, Hannes Frederic Sowa wrote: > > [ 31.064029] === > [ 31.064030] [ INFO: suspicious RCU usage. ] > [ 31.064032] 4.5.0+ #13 Not tainted > [ 31.064033] --- > [ 31.064034] include/net/sock.h:1594

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 01.04.2016 02:12, Eric Dumazet wrote: Since this function does not take a reference on the dst, how could it be safe ? As soon as you exit the function, dst would possibly point to something obsolete/freed. This works because the caller owns the socket. If you believe one path needs to be

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On 01.04.2016 02:12, Eric Dumazet wrote: On Fri, 2016-04-01 at 02:01 +0200, Hannes Frederic Sowa wrote: Hi Eric, On 01.04.2016 01:39, Eric Dumazet wrote: On Fri, 2016-04-01 at 01:29 +0200, Hannes Frederic Sowa wrote: Various fixes which were discovered by this changeset. More probably to

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, 2016-04-01 at 02:01 +0200, Hannes Frederic Sowa wrote: > Hi Eric, > > On 01.04.2016 01:39, Eric Dumazet wrote: > > On Fri, 2016-04-01 at 01:29 +0200, Hannes Frederic Sowa wrote: > >> Various fixes which were discovered by this changeset. More probably > >> to come... > > > > > > Really ?

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

Hi Eric, On 01.04.2016 01:39, Eric Dumazet wrote: On Fri, 2016-04-01 at 01:29 +0200, Hannes Frederic Sowa wrote: Various fixes which were discovered by this changeset. More probably to come... Really ? Again, TCP stack locks the socket most of the time. The fact that lockdep does not

Re: [PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

On Fri, 2016-04-01 at 01:29 +0200, Hannes Frederic Sowa wrote: > Various fixes which were discovered by this changeset. More probably > to come... Really ? Again, TCP stack locks the socket most of the time. The fact that lockdep does not understand this is not a reason to add this overhead.

[PATCH net 4/4] tcp: various missing rcu_read_lock around __sk_dst_get

Various fixes which were discovered by this changeset. More probably to come... Signed-off-by: Hannes Frederic Sowa --- include/net/tcp.h | 5 - net/core/sock.c| 7 +-- net/ipv4/tcp_input.c | 18 ++ net/ipv4/tcp_metrics.c | 12