Skip to content

Commit

Permalink
arping: Fix exit code if receive more replies than sent
Browse files Browse the repository at this point in the history
ARP protocol, unlike ICMP protocol, has no way to link REQUEST and REPLY
together (detect to which sender belongs the response). E.g. running
more arping instances currently causes failure due receiving more
replies than sent probes:

    # ./builddir/arping -c2 -I eth0 192.168.255.1 -w10 &
    # ./builddir/arping -c2 -I eth0 192.168.255.1 -w10 &
    ARPING 192.168.255.1 from 192.168.255.133 eth0
    ARPING 192.168.255.1 from 192.168.255.133 eth0
    Unicast reply from 192.168.255.1 [50:EB:F6:87:9D:D0]  1.722ms
    Unicast reply from 192.168.255.1 [50:EB:F6:87:9D:D0]  1.726ms
    Unicast reply from 192.168.255.1 [50:EB:F6:87:9D:D0]  1.910ms
    Unicast reply from 192.168.255.1 [50:EB:F6:87:9D:D0]  1.915ms
    Sent 1 probes (1 broadcast(s))
    Sent 1 probes (1 broadcast(s))
    Received 2 response(s)
    Received 2 response(s)
    [ ENTER ]
    [1]-  Exit 1                  ./builddir/arping -c2 -I eth0 192.168.255.1 -w10
    [2]+  Exit 1                  ./builddir/arping -c2 -I eth0 192.168.255.1 -w10

84ca65c (fix for 67e070d) introduced this regression.

Later e594ca5 introduced more precise timing - before arping sent 2
probes instead of 1 with -w1.

Then 854873b unified behavior with ping, i.e. using -w (deadline)
*without* -c (count) exit 0 if at least one reply arrived (backwards
incompatibility, also now incompatible with busybox). But that still
kept problematic using -w with -c on multiple instances / replies.

Fixing the problem by adding a special condition.

Also, when at it, move all exit code evaluation into finish() (before
it was in finish() but also event_loop()). This improves code introduced
in 67e070d.

Fixes: 84ca65c ("arping: fix sent vs received packages return value")
Fixes: iputils#538
Reported-by: Mingyang Liu <papillon@yeah.net>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
  • Loading branch information
pevik committed Jun 3, 2024
1 parent 5de892d commit dd8d23e
Showing 1 changed file with 17 additions and 15 deletions.
32 changes: 17 additions & 15 deletions arping.c
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,22 @@ static int finish(struct run_state *ctl)
printf("\n");
fflush(stdout);
}

/* arping exit code evaluation */
if (ctl->dad)
return (!!ctl->received);
return !!ctl->received;

if (ctl->unsolicited)
return 0;
return (!ctl->received);

if (ctl->timeout) {
if (ctl->count > 0)
return !(ctl->count <= ctl->received);
else
return !(ctl->received > 0);
}

return !ctl->received;
}

static void print_hex(unsigned char *p, int len)
Expand Down Expand Up @@ -693,7 +704,7 @@ static void find_broadcast_address(struct run_state *ctl)

static int event_loop(struct run_state *ctl)
{
int exit_loop = 0, rc = 0;
int exit_loop = 0;
ssize_t s;
enum {
POLLFD_SIGNAL = 0,
Expand Down Expand Up @@ -833,7 +844,7 @@ static int event_loop(struct run_state *ctl)
(struct sockaddr *)&from, &addr_len)) < 0) {
error(0, errno, "recvfrom");
if (errno == ENETDOWN)
rc = 2;
return 2;
continue;
}
if (recv_pack
Expand All @@ -849,17 +860,8 @@ static int event_loop(struct run_state *ctl)
close(sfd);
close(tfd);
freeifaddrs(ctl->ifa0);
rc |= finish(ctl);
if (ctl->unsolicited)
/* nothing */;
else if (ctl->dad && ctl->quit_on_reply)
/* Duplicate address detection mode return value */
rc |= !(ctl->brd_sent != ctl->received);
else if (ctl->timeout && !(ctl->count > 0))
rc |= !(ctl->received > 0);
else
rc |= (ctl->sent != ctl->received);
return rc;

return finish(ctl);
}

int main(int argc, char **argv)
Expand Down

0 comments on commit dd8d23e

Please sign in to comment.