On Thu, 6 Jun 2024 at 20:02, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > On Thu, 6 Jun 2024 at 19:53, Ilias Apalodimas > <ilias.apalodi...@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? Thanks /Ilias > > > > > > Thanks > > /Ilias > > > + ping_raw_stop(); > > > + ping_target = NULL; > > > + > > > + if (alive) { > > > + alive = false; > > > + return 0; > > > + } > > > + printf("ping failed; host %s is not alive\n", ipaddr_ntoa(addr)); > > > + return -1; > > > +} > > > + > > > +int do_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const > > > argv[]) > > > +{ > > > + ip_addr_t addr; > > > + > > > + if (argc < 2) > > > + return CMD_RET_USAGE; > > > + > > > + if (!ipaddr_aton(argv[1], &addr)) > > > + return CMD_RET_USAGE; > > > + > > > + if (ping_loop(&addr) < 0) > > > + return CMD_RET_FAILURE; > > > + > > > + return CMD_RET_SUCCESS; > > > +} > > > -- > > > 2.40.1 > > >