Hello,

On Wed, 31 May 2017, Sowmini Varadhan wrote:

On (06/01/17 00:41), Julian Anastasov wrote:

quoted

	So, we do not hold reference to neigh while accessing
its fields. I suspect we need to move the table lock from
neigh_remove_one here, for example:

good point, let me think over your suggestion carefully (it sounds
right, I want to make sure I dont miss any other race-windows)
and post patch v4 tomorrow..

	Another problem is that neigh_update() changes the
state but before we go and unlink the entry another CPU
can reactivate the entry, i.e. NUD_INCOMPLETE entered
in __neigh_event_send(). So, there will be always some
small window where surprises can happen and the entry is
not really deleted.

	Checking for NUD_FAILED may not be needed, the
atomic_read(&n->refcnt) == 1 check under lock ensures
that we are in some state without timer, i.e. not in
NUD_INCOMPLETE, so it is ok to remove the entry.

quoted

	Another solution to cause faster removal would be
to cancel the gc_work and to schedule it after 1 jiffie.
It helps when many entries are deleted at once but the
work prefers to just sleep when gc_thresh1 is not reached,
so such solution is not good enough.

Right the other drawback of relying on gc for cleanup is
that it doesn't give higher preference to cleaning up FAILED
entries first- so it can end up cleaning up other useful entries
(as I was pointing out to David Ahern)

	I see, ok.

Regards