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

[...]

+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.

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