batctl: ping: Add subsecond precision to ping interval
Commit Message
Modify the batctl ping utility to accept both integer and floating-point
values for the interval between sending pings. This enhancement allows
specifying intervals with subsecond precision.
For example:
`sudo batctl ping aa:bb:cc:dd:ee:ff -i 0.5`
Signed-off-by: Noah Peterson <noahbpeterson1997@gmail.com>
---
v4: Rebasing on latest commit, reformatting into reverse X-mas tree order based
on feedback from Sven <sven@narfation.org>
v3: Rebasing on latest commit, reformatting code based on comments by Sven
<sven@narfation.org>
v2: Fixing use-after-free, adding a missing header file, noted by Sven
<sven@narfation.org>
---
ping.c | 41 +++++++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 14 deletions(-)
Comments
On Saturday, 4 January 2025 02:36:45 GMT+1 Noah Peterson wrote:
> Modify the batctl ping utility to accept both integer and floating-point
> values for the interval between sending pings. This enhancement allows
> specifying intervals with subsecond precision.
>
> For example:
> `sudo batctl ping aa:bb:cc:dd:ee:ff -i 0.5`
>
> Signed-off-by: Noah Peterson <noahbpeterson1997@gmail.com>
> ---
> v4: Rebasing on latest commit, reformatting into reverse X-mas tree order based
> on feedback from Sven <sven@narfation.org>
Patch was not marked as v4 in the Subject line
> v3: Rebasing on latest commit, reformatting code based on comments by Sven
> <sven@narfation.org>
> v2: Fixing use-after-free, adding a missing header file, noted by Sven
> <sven@narfation.org>
All the changelogs you added after the v1 are basically wrong (or incomplete)
[...]
> + found_args += ((*((char*)(optarg - 1)) == optchar ) ? 1 : 2);
ERROR: "(foo*)" should be "(foo *)"
#63: FILE: ping.c:120:
+ found_args += ((*((char*)(optarg - 1)) == optchar ) ? 1 : 2);
ERROR: space prohibited before that close parenthesis ')'
#63: FILE: ping.c:120:
+ found_args += ((*((char*)(optarg - 1)) == optchar ) ? 1 : 2);
Please change it back to the original form.
> @@ -178,7 +191,7 @@ static int ping(struct state *state, int argc, char **argv)
> }
>
> printf("PING %s (%s) %zu(%zu) bytes of data\n", dst_string, mac_string,
> - packet_len, packet_len + 28);
> + packet_len, packet_len + 28);
>
> while (!is_aborted) {
> tv.tv_sec = timeout;
This has nothing to do with the thing this patch is supposed to do. And it is
also misaligning things. Please stop this.
> @@ -224,7 +237,7 @@ static int ping(struct state *state, int argc, char **argv)
>
> if ((size_t)read_len < packet_len) {
> printf("Warning - dropping received packet as it is smaller than expected (%zu): %zd\n",
> - packet_len, read_len);
> + packet_len, read_len);
> goto sleep;
> }
>
This has nothing to do with the thing this patch is supposed to do. And it is
also misaligning things. Please stop this.
> @@ -236,10 +249,10 @@ static int ping(struct state *state, int argc, char **argv)
> case BATADV_ECHO_REPLY:
> time_delta = end_timer();
> printf("%zd bytes from %s icmp_seq=%hu ttl=%d time=%.2f ms",
> - read_len, dst_string,
> - ntohs(icmp_packet_in.seqno),
> - icmp_packet_in.ttl,
> - time_delta);
> + read_len, dst_string,
> + ntohs(icmp_packet_in.seqno),
> + icmp_packet_in.ttl,
> + time_delta);
>
> if (read_len == sizeof(struct batadv_icmp_packet_rr)) {
> if (last_rr_cur == icmp_packet_in.rr_cur &&
This has nothing to do with the thing this patch is supposed to do. And it is
also misaligning things. Please stop this.
> @@ -328,9 +341,9 @@ static int ping(struct state *state, int argc, char **argv)
>
> printf("--- %s ping statistics ---\n", dst_string);
> printf("%u packets transmitted, %u received, %u%% packet loss\n",
> - packets_out, packets_in, packets_loss);
> + packets_out, packets_in, packets_loss);
> printf("rtt min/avg/max/mdev = %.3f/%.3f/%.3f/%.3f ms\n",
> - min, avg, max, mdev);
> + min, avg, max, mdev);
>
> if (packets_in)
> ret = EXIT_SUCCESS;
This has nothing to do with the thing this patch is supposed to do. And it is
also misaligning things. Please stop this.
Kind regards,
Sven
On Saturday, 4 January 2025 02:36:45 GMT+1 Noah Peterson wrote:
> @@ -58,12 +59,16 @@ static int ping(struct state *state, int argc, char **argv)
> struct batadv_icmp_packet_rr icmp_packet_out;
> struct batadv_icmp_packet_rr icmp_packet_in;
> uint8_t last_rr[BATADV_RR_LEN][ETH_ALEN];
> + struct timespec loop_interval = {0, 0};
> struct ether_addr *dst_mac = NULL;
> struct ether_addr *rr_mac = NULL;
> int disable_translate_mac = 0;
> + double fractional_part = 0.0;
> unsigned int seq_counter = 0;
> unsigned int packets_out = 0;
> unsigned int packets_in = 0;
> + double ping_interval = 0.0;
> + double integral_part = 0.0;
> unsigned int packets_loss;
> struct bat_host *bat_host;
> struct bat_host *rr_host;
The code doesn't build:
CC ping.o
ping.c: In function ‘ping’:
ping.c:77:13: error: conflicting types for ‘loop_interval’; have ‘int’
77 | int loop_interval = 0;
| ^~~~~~~~~~~~~
ping.c:62:25: note: previous definition of ‘loop_interval’ with type ‘struct timespec’
62 | struct timespec loop_interval = {0, 0};
| ^~~~~~~~~~~~~
ping.c:110:57: error: ‘end’ undeclared (first use in this function); did you mean ‘send’?
110 | ping_interval = strtod(optarg, &end);
| ^~~
| send
ping.c:110:57: note: each undeclared identifier is reported only once for each function it appears in
ping.c:77:13: warning: unused variable ‘loop_interval’ [-Wunused-variable]
77 | int loop_interval = 0;
| ^~~~~~~~~~~~~
Kind regards,
Sven
@@ -19,6 +19,7 @@
#include <stdint.h>
#include <sys/select.h>
#include <sys/time.h>
+#include <time.h>
#include <netinet/if_ether.h>
#include "batadv_packet_compat.h"
@@ -58,12 +59,16 @@ static int ping(struct state *state, int argc, char **argv)
struct batadv_icmp_packet_rr icmp_packet_out;
struct batadv_icmp_packet_rr icmp_packet_in;
uint8_t last_rr[BATADV_RR_LEN][ETH_ALEN];
+ struct timespec loop_interval = {0, 0};
struct ether_addr *dst_mac = NULL;
struct ether_addr *rr_mac = NULL;
int disable_translate_mac = 0;
+ double fractional_part = 0.0;
unsigned int seq_counter = 0;
unsigned int packets_out = 0;
unsigned int packets_in = 0;
+ double ping_interval = 0.0;
+ double integral_part = 0.0;
unsigned int packets_loss;
struct bat_host *bat_host;
struct bat_host *rr_host;
@@ -101,10 +106,18 @@ static int ping(struct state *state, int argc, char **argv)
ping_usage();
return EXIT_SUCCESS;
case 'i':
- loop_interval = strtol(optarg, NULL, 10);
- if (loop_interval < 1)
- loop_interval = 1;
- found_args += ((*((char *)(optarg - 1)) == optchar) ? 1 : 2);
+ errno = 0;
+ ping_interval = strtod(optarg, &end);
+ if (errno) {
+ fprintf(stderr, "Error - invalid ping interval '%s'\n", optarg);
+ goto out;
+ } else {
+ ping_interval = fmax(ping_interval, 0.001);
+ }
+ fractional_part = modf(ping_interval, &integral_part);
+ loop_interval.tv_sec = (time_t)integral_part;
+ loop_interval.tv_nsec = (long)(fractional_part * 1000000000l);
+ found_args += ((*((char*)(optarg - 1)) == optchar ) ? 1 : 2);
break;
case 't':
timeout = strtol(optarg, NULL, 10);
@@ -178,7 +191,7 @@ static int ping(struct state *state, int argc, char **argv)
}
printf("PING %s (%s) %zu(%zu) bytes of data\n", dst_string, mac_string,
- packet_len, packet_len + 28);
+ packet_len, packet_len + 28);
while (!is_aborted) {
tv.tv_sec = timeout;
@@ -224,7 +237,7 @@ static int ping(struct state *state, int argc, char **argv)
if ((size_t)read_len < packet_len) {
printf("Warning - dropping received packet as it is smaller than expected (%zu): %zd\n",
- packet_len, read_len);
+ packet_len, read_len);
goto sleep;
}
@@ -236,10 +249,10 @@ static int ping(struct state *state, int argc, char **argv)
case BATADV_ECHO_REPLY:
time_delta = end_timer();
printf("%zd bytes from %s icmp_seq=%hu ttl=%d time=%.2f ms",
- read_len, dst_string,
- ntohs(icmp_packet_in.seqno),
- icmp_packet_in.ttl,
- time_delta);
+ read_len, dst_string,
+ ntohs(icmp_packet_in.seqno),
+ icmp_packet_in.ttl,
+ time_delta);
if (read_len == sizeof(struct batadv_icmp_packet_rr)) {
if (last_rr_cur == icmp_packet_in.rr_cur &&
@@ -302,8 +315,8 @@ static int ping(struct state *state, int argc, char **argv)
if (loop_count == 0)
continue;
- if (loop_interval > 0)
- sleep(loop_interval);
+ if (loop_interval.tv_sec > 0 || loop_interval.tv_nsec > 0)
+ nanosleep(&loop_interval, NULL);
else if ((tv.tv_sec != 0) || (tv.tv_usec != 0))
select(0, NULL, NULL, NULL, &tv);
}
@@ -328,9 +341,9 @@ static int ping(struct state *state, int argc, char **argv)
printf("--- %s ping statistics ---\n", dst_string);
printf("%u packets transmitted, %u received, %u%% packet loss\n",
- packets_out, packets_in, packets_loss);
+ packets_out, packets_in, packets_loss);
printf("rtt min/avg/max/mdev = %.3f/%.3f/%.3f/%.3f ms\n",
- min, avg, max, mdev);
+ min, avg, max, mdev);
if (packets_in)
ret = EXIT_SUCCESS;