From f44bde2d9f4f6a4f2fe452eb6cbfd525cb3f618f Mon Sep 17 00:00:00 2001
From: David Girault <dgirault@sevenhugs.com>
Date: Fri, 18 Jan 2019 18:20:08 +0100
Subject: [PATCH 2/2] lwiperf: small fixes on TCP client/server mode

- support specifying amount to transfert or duration for client test
- added some debug messages for dual and tradeoff mode
- ensure tcp_pcb isn't freed two times on error
- fix report callback calls
---
 src/apps/lwiperf/lwiperf.c      | 58 +++++++++++++++++++++------------
 src/include/lwip/apps/lwiperf.h |  2 +-
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/apps/lwiperf/lwiperf.c b/src/apps/lwiperf/lwiperf.c
index 3f7dc3d5..224e33ff 100644
--- a/src/apps/lwiperf/lwiperf.c
+++ b/src/apps/lwiperf/lwiperf.c
@@ -315,12 +315,20 @@ lwip_tcp_conn_report(lwiperf_state_tcp_t *conn, enum lwiperf_report_type report_
     if (duration_ms == 0) {
       bandwidth_kbitpsec = 0;
     } else {
-      bandwidth_kbitpsec = (conn->bytes_transferred / duration_ms) * 8U;
+      bandwidth_kbitpsec = (conn->bytes_transferred * 8U) / duration_ms;
     }
-    conn->report_fn(conn->report_arg, report_type,
-                    &conn->conn_pcb->local_ip, conn->conn_pcb->local_port,
-                    &conn->conn_pcb->remote_ip, conn->conn_pcb->remote_port,
-                    conn->bytes_transferred, duration_ms, bandwidth_kbitpsec);
+    if (conn->conn_pcb)
+      conn->report_fn(conn->report_arg, report_type,
+                      &conn->conn_pcb->local_ip, conn->conn_pcb->local_port,
+                      &conn->conn_pcb->remote_ip, conn->conn_pcb->remote_port,
+                      conn->bytes_transferred, duration_ms, bandwidth_kbitpsec);
+    else if (conn->server_pcb)
+      conn->report_fn(conn->report_arg, report_type,
+                      &conn->server_pcb->local_ip, conn->server_pcb->local_port,
+                      &conn->server_pcb->remote_ip, conn->server_pcb->remote_port,
+                      conn->bytes_transferred, duration_ms, bandwidth_kbitpsec);
+    else
+      conn->report_fn(conn->report_arg, report_type, NULL, 0, NULL, 0, 0, 0, 0);
   }
 }
 
@@ -343,7 +351,8 @@ lwiperf_tcp_close(lwiperf_state_tcp_t *conn, enum lwiperf_report_type report_typ
       /* don't want to wait for free memory here... */
       tcp_abort(conn->conn_pcb);
     }
-  } else {
+  }
+  else if (conn->server_pcb) {
     /* no conn pcb, this is the listener pcb */
     err = tcp_close(conn->server_pcb);
     LWIP_ASSERT("error", err == ERR_OK);
@@ -370,7 +379,7 @@ lwiperf_tcp_client_send_more(lwiperf_state_tcp_t *conn)
       /* this session is time-limited */
       u32_t now = sys_now();
       u32_t diff_ms = now - conn->time_started;
-      u32_t time = (u32_t) - (s32_t)lwip_htonl(conn->settings.amount);
+      u32_t time = (u32_t) - (s32_t)lwip_ntohl(conn->settings.amount);
       u32_t time_ms = time * 10;
       if (diff_ms >= time_ms) {
         /* time specified by the client is over -> close the connection */
@@ -379,7 +388,7 @@ lwiperf_tcp_client_send_more(lwiperf_state_tcp_t *conn)
       }
     } else {
       /* this session is byte-limited */
-      u32_t amount_bytes = lwip_htonl(conn->settings.amount);
+      u32_t amount_bytes = lwip_ntohl(conn->settings.amount);
       /* @todo: this can send up to 1*MSS more than requested... */
       if (amount_bytes >= conn->bytes_transferred) {
         /* all requested bytes transferred -> close the connection */
@@ -553,6 +562,7 @@ lwiperf_tcp_recv(void *arg, struct tcp_pcb *tpcb, struct pbuf *p, err_t err)
     if (conn->settings.flags & PP_HTONL(LWIPERF_FLAGS_ANSWER_TEST)) {
       if ((conn->settings.flags & PP_HTONL(LWIPERF_FLAGS_ANSWER_NOW)) == 0) {
         /* client requested transmission after end of test */
+        LWIP_PLATFORM_DIAG(("client requested transmission after end of test\n"));
         lwiperf_tx_start_passive(conn);
       }
     }
@@ -577,9 +587,11 @@ lwiperf_tcp_recv(void *arg, struct tcp_pcb *tpcb, struct pbuf *p, err_t err)
         return ERR_OK;
       }
       conn->have_settings_buf = 1;
+      LWIP_PLATFORM_DIAG(("New TCP client (settings flags 0x%x)\n", lwip_ntohl(conn->settings.flags)));
       if (conn->settings.flags & PP_HTONL(LWIPERF_FLAGS_ANSWER_TEST)) {
         if (conn->settings.flags & PP_HTONL(LWIPERF_FLAGS_ANSWER_NOW)) {
           /* client requested parallel transmission test */
+          LWIP_PLATFORM_DIAG(("client requested parallel transmission test\n"));
           err_t err2 = lwiperf_tx_start_passive(conn);
           if (err2 != ERR_OK) {
             lwiperf_tcp_close(conn, LWIPERF_TCP_ABORTED_LOCAL_TXERROR);
@@ -645,6 +657,8 @@ lwiperf_tcp_err(void *arg, err_t err)
 {
   lwiperf_state_tcp_t *conn = (lwiperf_state_tcp_t *)arg;
   LWIP_UNUSED_ARG(err);
+  /* lwiperf_tcp_close MUST not close itself conn_pcb */
+  conn->conn_pcb = NULL;
   lwiperf_tcp_close(conn, LWIPERF_TCP_ABORTED_REMOTE);
 }
 
@@ -655,7 +669,7 @@ lwiperf_tcp_poll(void *arg, struct tcp_pcb *tpcb)
   lwiperf_state_tcp_t *conn = (lwiperf_state_tcp_t *)arg;
   LWIP_ASSERT("pcb mismatch", conn->conn_pcb == tpcb);
   LWIP_UNUSED_ARG(tpcb);
-  if (++conn->poll_count >= LWIPERF_TCP_MAX_IDLE_SEC) {
+  if (++conn->poll_count >= LWIPERF_MAX_IDLE_SEC) {
     lwiperf_tcp_close(conn, LWIPERF_TCP_ABORTED_LOCAL);
     return ERR_OK; /* lwiperf_tcp_close frees conn */
   }
@@ -707,7 +721,7 @@ lwiperf_tcp_accept(void *arg, struct tcp_pcb *newpcb, err_t err)
   tcp_arg(newpcb, conn);
   tcp_recv(newpcb, lwiperf_tcp_recv);
   tcp_poll(newpcb, lwiperf_tcp_poll, 2U);
-  tcp_err(conn->conn_pcb, lwiperf_tcp_err);
+  tcp_err(newpcb, lwiperf_tcp_err);
 
   if (s->specific_remote) {
     /* this listener belongs to a client, so make the client the master of the newly created connection */
@@ -761,9 +775,11 @@ lwiperf_start_tcp_server(const ip_addr_t *local_addr, u16_t local_port,
   return NULL;
 }
 
-static err_t lwiperf_start_tcp_server_impl(const ip_addr_t *local_addr, u16_t local_port,
-                                           lwiperf_report_fn report_fn, void *report_arg,
-                                           lwiperf_state_base_t *related_master_state, lwiperf_state_tcp_t **state)
+static err_t
+lwiperf_start_tcp_server_impl(const ip_addr_t *local_addr, u16_t local_port,
+                              lwiperf_report_fn report_fn, void *report_arg,
+                              lwiperf_state_base_t *related_master_state,
+                              lwiperf_state_tcp_t **state)
 {
   err_t err;
   struct tcp_pcb *pcb;
@@ -821,10 +837,11 @@ static err_t lwiperf_start_tcp_server_impl(const ip_addr_t *local_addr, u16_t lo
  * @returns a connection handle that can be used to abort the client
  *          by calling @ref lwiperf_abort()
  */
-void* lwiperf_start_tcp_client_default(const ip_addr_t* remote_addr,
-                               lwiperf_report_fn report_fn, void* report_arg)
+void *
+lwiperf_start_tcp_client_default(const ip_addr_t* remote_addr,
+                                 lwiperf_report_fn report_fn, void* report_arg)
 {
-  return lwiperf_start_tcp_client(remote_addr, LWIPERF_TCP_PORT_DEFAULT, LWIPERF_CLIENT,
+  return lwiperf_start_tcp_client(remote_addr, LWIPERF_TCP_PORT_DEFAULT, LWIPERF_CLIENT, -1000,
                                   report_fn, report_arg);
 }
 
@@ -835,8 +852,10 @@ void* lwiperf_start_tcp_client_default(const ip_addr_t* remote_addr,
  * @returns a connection handle that can be used to abort the client
  *          by calling @ref lwiperf_abort()
  */
-void* lwiperf_start_tcp_client(const ip_addr_t* remote_addr, u16_t remote_port,
-  enum lwiperf_client_type type, lwiperf_report_fn report_fn, void* report_arg)
+void *
+lwiperf_start_tcp_client(const ip_addr_t* remote_addr, u16_t remote_port,
+                         enum lwiperf_client_type type, int amount,
+                         lwiperf_report_fn report_fn, void* report_arg)
 {
   err_t ret;
   lwiperf_settings_t settings;
@@ -862,8 +881,7 @@ void* lwiperf_start_tcp_client(const ip_addr_t* remote_addr, u16_t remote_port,
   }
   settings.num_threads = htonl(1);
   settings.remote_port = htonl(LWIPERF_TCP_PORT_DEFAULT);
-  /* TODO: implement passing duration/amount of bytes to transfer */
-  settings.amount = htonl((u32_t)-1000);
+  settings.amount = htonl((u32_t)amount);
 
   ret = lwiperf_tx_start_impl(remote_addr, remote_port, &settings, report_fn, report_arg, NULL, &state);
   if (ret == ERR_OK) {
diff --git a/src/include/lwip/apps/lwiperf.h b/src/include/lwip/apps/lwiperf.h
index 4b4a34fe..56632234 100644
--- a/src/include/lwip/apps/lwiperf.h
+++ b/src/include/lwip/apps/lwiperf.h
@@ -97,7 +97,7 @@ void* lwiperf_start_tcp_server(const ip_addr_t* local_addr, u16_t local_port,
                                lwiperf_report_fn report_fn, void* report_arg);
 void* lwiperf_start_tcp_server_default(lwiperf_report_fn report_fn, void* report_arg);
 void* lwiperf_start_tcp_client(const ip_addr_t* remote_addr, u16_t remote_port,
-                               enum lwiperf_client_type type,
+                               enum lwiperf_client_type type, int amount,
                                lwiperf_report_fn report_fn, void* report_arg);
 void* lwiperf_start_tcp_client_default(const ip_addr_t* remote_addr,
                                lwiperf_report_fn report_fn, void* report_arg);
-- 
2.17.1

