[PATCH v3 05/12] net-lwip: add ping command

[Ilias Apalodimas]

On Thu, 6 Jun 2024 at 20:02, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Thu, 6 Jun 2024 at 19:53, Ilias Apalodimas
> <ilias.apalodimas at 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
> > >