Re: [PATCH V4] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"

[Sowmini Varadhan <sowmini.varadhan@oracle.com>]

On (06/01/17 22:34), Julian Anastasov wrote:
> > +	np = &nht->hash_buckets[hash_val];
> > +	while ((n = rcu_dereference_protected(*np,
> > +				lockdep_is_held(&tbl->lock))) != NULL) {
> 
> 	checkpatch shows some warnings:
> 
> scripts/checkpatch.pl --strict /tmp/file.patch

Yes, checkpatch complained about 
"CHECK: Alignment should match open parenthesis"
but trying to meet that requirement (without exceeding the 80 char limit)
would need additional variables, and I noticed that there are
other places in the code (e.g., neigh_forced_gc()) where the alignment
prescription is not observed, so I let things follow existing style..

[ In neigh_remove_one()] 
> 	In case there is another patch version,
> the retval can be removed:

Let me see if there are additional review comments, and I can update
with the retval removed. 

Thanks much for the review!

> 	Looks like we can also call neigh_remove_one only when !err.
> But this is some corner case when n->dead is set by GC and entry
> was unlinked, neigh_remove_one simply will not find it in the list,
> so it is not fatal to call neigh_remove_one unconditionally.

> > @@ -1113,13 +1113,17 @@ static int arp_invalidate(struct net_device *dev, __be32 ip)
        :
> 	Here the same race with GC already assigned
> neigh->dead to 1 is possible but it is more tricky to catch
> that exactly neigh_update() has failed. So, may be better to
> call neigh_remove_one like now.