Re: [PATCH iproute2 net-next] ip neigh: allow flush FAILED neighbour entry

[Hangbin Liu <liuhangbin@gmail.com>]

Hi Stephen,
On Wed, Jun 14, 2017 at 09:54:50AM -0700, Stephen Hemminger wrote:
> On Mon,  5 Jun 2017 16:31:29 +0800
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> > After upstream commit 5071034e4af7 ('neigh: Really delete an arp/neigh entry
> > on "ip neigh delete" or "arp -d"'), we could delete a single FAILED neighbour
> > entry now. But `ip neigh flush` still skip the FAILED entry.
> > 
> > Let's remove this filter so we can also flush FAILED entry.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> This might create a problem. iproute2 has to be forward/backwards compatiable
> with multiple kernel versions. Users must be able to run newer versions of ip
> command on older kernels.
> 
> What happens if you run the ip command with your patch on older kernels?
> 
On older kernel with tihs patch. The result is same, we could not delete
FAILED entry. Since we could not delete it, `ip neigh` will try MAX_ROUNDS
and fail at the end. But IMHO, this should be the correct behavoir. Because
this is just what kernel did and what kernel fixed now.

Here is the result on old kernel:

With unpatched ip cmd
# ip -4 neigh show dev eth1
192.168.1.5  FAILED
192.168.1.4  FAILED
192.168.1.3  FAILED
192.168.1.6  FAILED
192.168.1.2  FAILED
# ip -4 neigh flush dev eth1
# ip -4 -s neigh flush dev eth1
Nothing to flush.
# echo $?
0

With patched ip cmd
# ./ip -4 neigh flush dev eth1
*** Flush not complete bailing out after 10 rounds
# ./ip -4 -s neigh flush dev eth1

*** Round 1, deleting 5 entries ***

*** Round 2, deleting 5 entries ***

*** Round 3, deleting 5 entries ***

*** Round 4, deleting 5 entries ***

*** Round 5, deleting 5 entries ***

*** Round 6, deleting 5 entries ***

*** Round 7, deleting 5 entries ***

*** Round 8, deleting 5 entries ***

*** Round 9, deleting 5 entries ***

*** Round 10, deleting 5 entries ***
*** Flush not complete bailing out after 10 rounds
# echo $?
255

But if you thought the message reallly annoying. Then how about this patch,
which is a little tricky.

diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 4d8fc85..9088a05 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -445,7 +445,7 @@ static int do_show_or_flush(int argc, char **argv, int flush)
                filter.flushb = flushb;
                filter.flushp = 0;
                filter.flushe = sizeof(flushb);
-               filter.state &= ~NUD_FAILED;

                while (round < MAX_ROUNDS) {
                        if (rtnl_dump_request_n(&rth, &req.n) < 0) {
@@ -474,6 +474,7 @@ static int do_show_or_flush(int argc, char **argv, int flush)
                                printf("\n*** Round %d, deleting %d entries ***\n", round, filter.flushed);
                                fflush(stdout);
                        }
+                       filter.state &= ~NUD_FAILED;
                }
                printf("*** Flush not complete bailing out after %d rounds\n",
                        MAX_ROUNDS);


With this patch, the result will looks almost the same with old behavior,
except it will filter out the FAILED entry the first time.

# ./ip -4 neigh flush dev eth1
# ./ip -4 -s neigh flush dev eth1

*** Round 1, deleting 5 entries ***
*** Flush is complete after 1 round ***
# echo $?
0


Thanks
Hangbin