All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ollie Wild <aaw@rincewind.tv>
To: Ollie Wild <aaw@rincewind.tv>
Cc: Patrick McHardy <kaber@trash.net>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fix dst_entry leak in icmp_push_reply()
Date: Thu, 18 Aug 2005 11:45:55 -0700	[thread overview]
Message-ID: <4304D763.4090001@rincewind.tv> (raw)
In-Reply-To: <43042D94.4030303@rincewind.tv>

Ollie Wild wrote:

> Patrick McHardy wrote:
>
>> Your patch doesn't fit your description, the else-condition you're
>> adding triggers when the queue is empty, so what is the point?
>
>
> Since we're only calling ip_append_data() once here, the two 
> conditions are identical.

I should mention that this problem is not academic.  We've run into it 
in the field.  If a lot of ICMP destination unreachable messages are 
generated (by flooding a net_device with bad UDP packets for instance), 
the net_device can no longer be unregistered.

That said, I appreciate that the if-else condition doesn't seem quite 
right.  The problem is, the icmp_push_reply() routine is implicitly 
using the queue as a success indicator.  I put the 
ip_flush_pending_frames() call inside the else block because I wanted to 
guarantee that one of ip_push_pending_frames() and 
ip_flush_pending_frames() is always called.  Both will do proper cleanup.

I'm open to suggestions if you think there's a cleaner way to implement 
this.

Ollie

  parent reply	other threads:[~2005-08-18 18:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-17 20:21 [PATCH] fix dst_entry leak in icmp_push_reply() Ollie Wild
2005-08-17 23:56 ` Patrick McHardy
2005-08-18  6:41   ` Ollie Wild
2005-08-18 18:42     ` Patrick McHardy
2005-08-18 18:42     ` Patrick McHardy
2005-08-18 18:45     ` Ollie Wild [this message]
2005-08-18 18:59       ` Patrick McHardy
2005-08-18 18:59       ` Patrick McHardy
2005-08-18 19:05         ` Ollie Wild
2005-08-18 21:32           ` David S. Miller
2005-08-18 21:32           ` David S. Miller
2005-08-18 19:05         ` Ollie Wild

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=4304D763.4090001@rincewind.tv \
    --to=aaw@rincewind.tv \
    --cc=kaber@trash.net \
    --cc=linux-kernel@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.