All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lorenzo Colitti <lorenzo@google.com>
To: David Ahern <dsa@cumulusnetworks.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH v3 net-next] net: diag: support SOCK_DESTROY for UDP sockets
Date: Wed, 24 Aug 2016 02:13:31 +0900	[thread overview]
Message-ID: <CAKD1Yr0FM9YKHFTYMeYipvyTWDLoM8EvcxVpcYwAWFHQZuS+6Q@mail.gmail.com> (raw)
In-Reply-To: <417574bb-d15d-f347-d951-49d0eede02c0@cumulusnetworks.com>

On Wed, Aug 24, 2016 at 2:02 AM, David Ahern <dsa@cumulusnetworks.com> wrote:
>> Looking at the code again, it seems that there's a bug in
>> sock_diag_destroy. If the destroy operation does not occur (e.g., if
>> sock_diag_destroy returns EPERM, or the protocol doesn't support
>> destroy), then it doesn't release the refcount. This affects the TCP
>> code as well and as such is my fault, not yours. The most obvious way
>> to fix this might be to call sock_gen_put in sock_diag_destroy.
>
> sock_gen_put seems specific to tcp which is why I used sock_put here.

I thought sock_gen_put can be used on UDP as well, but it does seem a
bit counterintuitive.

> Perhaps better to have the callers of sock_diag_destroy handle the refcnt? This
> function took it; it should release it success or fail. Same for tcp.

So you'd remove the sock_put and sock_gen_put calls from tcp_abort and
just add one sock_gen_put in tcp_diag_destroy? That does seem simpler.

  reply	other threads:[~2016-08-23 17:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-23 15:02 [PATCH v3 net-next] net: diag: support SOCK_DESTROY for UDP sockets David Ahern
2016-08-23 15:54 ` Eric Dumazet
2016-08-23 15:55 ` Eric Dumazet
2016-08-23 16:37 ` Lorenzo Colitti
2016-08-23 17:02   ` David Ahern
2016-08-23 17:13     ` Lorenzo Colitti [this message]
2016-08-23 17:16       ` David Ahern
2016-08-23 17:24         ` Lorenzo Colitti
2016-08-23 17:54           ` Eric Dumazet

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=CAKD1Yr0FM9YKHFTYMeYipvyTWDLoM8EvcxVpcYwAWFHQZuS+6Q@mail.gmail.com \
    --to=lorenzo@google.com \
    --cc=dsa@cumulusnetworks.com \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.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 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.