Re: [PATCH] fix dst_entry leak in icmp_push_reply()

Ollie Wild
2005-08-18T18:45:58Z

> 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