
On 6/6/24 19:45, Ilias Apalodimas wrote:
On Thu, 6 Jun 2024 at 20:02, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Thu, 6 Jun 2024 at 19:53, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
[...]
+static int ping_raw_init(void *recv_arg) +{
ping_pcb = raw_new(IP_PROTO_ICMP);
if (!ping_pcb)
return -ENOMEM;
raw_recv(ping_pcb, ping_recv, recv_arg);
raw_bind(ping_pcb, IP_ADDR_ANY);
return 0;
+}
+static void ping_raw_stop(void) +{
if (ping_pcb != NULL) {
nits, but we usually do if (!ping_pcb) for NULL pointers and variables that have a value of 0. Please change it in other files as well
raw_remove(ping_pcb);
ping_pcb = NULL;
}
+}
+static void ping_prepare_echo(struct icmp_echo_hdr *iecho) +{
ICMPH_TYPE_SET(iecho, ICMP_ECHO);
ICMPH_CODE_SET(iecho, 0);
iecho->chksum = 0;
iecho->id = PING_ID;
iecho->seqno = lwip_htons(++ping_seq_num);
iecho->chksum = inet_chksum(iecho, sizeof(*iecho));
+}
+static void ping_send_icmp(struct raw_pcb *raw, const ip_addr_t *addr) +{
struct pbuf *p;
struct icmp_echo_hdr *iecho;
size_t ping_size = sizeof(struct icmp_echo_hdr);
p = pbuf_alloc(PBUF_IP, (u16_t)ping_size, PBUF_RAM);
if (!p)
return;
if ((p->len == p->tot_len) && (p->next == NULL)) {
&& !p->next
iecho = (struct icmp_echo_hdr *)p->payload;
ping_prepare_echo(iecho);
raw_sendto(raw, p, addr);
}
pbuf_free(p);
+}
+static void ping_send(void *arg) +{
struct raw_pcb *pcb = (struct raw_pcb *)arg;
ping_send_icmp(pcb, ping_target);
sys_timeout(PING_DELAY_MS, ping_send, ping_pcb);
+}
+static int ping_loop(const ip_addr_t* addr) +{
bool alive;
ulong start;
int ret;
printf("Using %s device\n", eth_get_name());
ret = ping_raw_init(&alive);
if (ret < 0)
return ret;
ping_target = addr;
ping_seq_num = 0;
start = get_timer(0);
ping_send(ping_pcb);
do {
eth_rx();
if (alive)
break;
sys_check_timeouts();
if (ctrlc()) {
printf("\nAbort\n");
break;
}
} while (get_timer(start) < PING_TIMEOUT_MS);
I am a bit confused about what happens here. ping_send() will send the packet, but it will also schedule itself to rerun after 1 ms and send another ping?
sys_untimeout(ping_send, ping_pcb);
So we need the sys_untimeout() because we queued 2 pings? Because sys_timeout() is supposed to be an one shot.
Ah nvm, I misread that. We always reschedule ping_send_icmp(), that's why we have to delete it here
Ok so looking at it a bit more. Why do we have to schedule contunuous pings? We just have to send one packet and wait for the response no?
We could send just one packet as NET does, but if there's packet loss then the ping command will report the host is unreachable. I think it is safer to allow for a few retries for better reliability. The code for one packet would be marginally simpler anyways. What I can do however is use a send counter rather than a timeout.
Thanks,