On Thu, 15 Jun 2017 10:30:16 +0800
Hangbin Liu <liuhang...@gmail.com> wrote:
> 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 <liuhang...@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 <liuhang...@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
That looks better, could you send an updated patch. I will apply that
