arduino / ArduinoCore-renesas

MIT License
110 stars 76 forks source link

Lock in C33 using ethernet adapter and TCP connection in a long run #240

Open Eduardo-Lopes opened 9 months ago

Eduardo-Lopes commented 9 months ago

Hey all. I implemented a HTTP server on C33. But in a stress test, the board doesn't do well. I take the smallest sample.

#include <EthernetC33.h>

EthernetServer server(7);

void setup() {
  Serial.begin(9600);

  if (Ethernet.begin() == 0) {
    Serial.println("Failed to configure Ethernet using DHCP");
  }
  server.begin();
  Serial.print("server is at ");
  Serial.println(Ethernet.localIP());
}

void loop() {
  EthernetClient client = server.available();
  if (client) {
    char previous_c = 0;
    int sequential_newline = 0;
    char buffer[1024];
    int pos = 0;

    while (client.connected()) {
      if (client.available()) {
        char c = client.read();

        if (previous_c == '\r' && c == '\n')
          sequential_newline++;
        else if (previous_c != '\n' || c != '\r')
          sequential_newline = 0;

        if (sequential_newline == 2)
          break;

        previous_c = c;
        buffer[pos++] = c;
      }
    }

    client.write(buffer, pos );

    delay(1);
    // close the connection:
    client.stop();
  }
}

and the python code to call

import socket

server_address = ('192.168.0.103', 7)  # Replace with the desired server address and port

while True:
    tcp_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    tcp_socket.connect(server_address)

    data = b"POST /mode/IDLE HTTP/1.1\r\nHost: 192.168.0.103\r\nUser-Agent: python-requests/2.28.1\r\nAccept-Encoding: gzip, deflate, br\r\nAccept: */*\r\nConnection: keep-alive\r\nContent-Length: 0\r\n\r\n"
    tcp_socket.sendall(data)

    while True:
        response = tcp_socket.recv(1024)

        if not response:
            break

        print(response.decode(), end='')

    tcp_socket.close()

I take a deep look at the source code too. It looks like a problem in lwip function tcp_output that doesn't clean the recv buffer in some way. And it looks like that is something low level because the C33 stops responding to ping too. In lwip functions is called tcp_write to add the buffer and tcp_output to send the buffer, but tcp_write keeps returning ERR_MEM.

Also, there is no SDCard on the shield.

Any help?

Eduardo-Lopes commented 9 months ago

Adding... This code uses the lwip directly and responds the same way.

#include <EthernetC33.h>

#include "lwipServer.h"
#include "EthernetClient.h"

void tcpecho_raw_init(void);
err_t tcpecho_raw_accept(void *arg, struct tcp_pcb *newpcb, err_t err);
err_t tcpecho_raw_sent(void *arg, struct tcp_pcb *tpcb, u16_t len);

static struct tcp_pcb *tcpecho_raw_pcb;

bool restart = true;

enum tcpecho_raw_states
{
  ES_NONE = 0,
  ES_ACCEPTED,
  ES_RECEIVED,
  ES_CLOSING
};

struct tcpecho_raw_state
{
  u8_t state;
  u8_t retries;
  struct tcp_pcb *pcb;
  /* pbuf (chain) to recycle */
  struct pbuf *p;
};

void
tcpecho_raw_free(struct tcpecho_raw_state *es)
{
  Serial.println("raw_free");

  if (es != NULL) {
    if (es->p) {
      /* free the buffer chain if present */
      pbuf_free(es->p);
    }

    mem_free(es);
  }  
}

void
tcpecho_raw_close(struct tcp_pcb *tpcb, struct tcpecho_raw_state *es)
{
  Serial.println("raw_close");

  tcp_arg(tpcb, NULL);
  tcp_sent(tpcb, NULL);
  tcp_recv(tpcb, NULL);
  tcp_err(tpcb, NULL);
  tcp_poll(tpcb, NULL, 0);

  tcpecho_raw_free(es);

  tcp_close(tpcb);

  restart = true;
}

void
tcpecho_raw_send(struct tcp_pcb *tpcb, struct tcpecho_raw_state *es)
{
  Serial.println("raw_send");
  Serial.flush();
  Serial.print("pcb:"); 
  /*
  Serial.print(tpcb->state); Serial.print(",");
  Serial.print(tpcb->bytes_acked); Serial.print(",");
  Serial.print((int)tpcb->snd_buf); Serial.print(",");
  Serial.print((int)tpcb->snd_queuelen); Serial.print(",");
  Serial.print((int)tpcb->snd_wnd); Serial.print(",");
  Serial.print((int)tpcb->snd_wnd_max); Serial.print(",");
  Serial.println();
  */
  Serial.flush();
  Serial.print((int)tpcb->keep_cnt_sent); Serial.print(",");
  Serial.print((int)tpcb->mss); Serial.print(",");
  Serial.print((int)tpcb->snd_lbb); Serial.print(",");
  Serial.print((int)tpcb->snd_nxt); Serial.print(",");
  Serial.print((int)tpcb->snd_wl1); Serial.print(",");
  Serial.print((int)tpcb->snd_wl2); Serial.print(",");

  struct pbuf *ptr;
  err_t wr_err = ERR_OK;

  while ((wr_err == ERR_OK) &&
         (es->p != NULL) && 
         (es->p->len <= tcp_sndbuf(tpcb))) {
    ptr = es->p;

    /* enqueue data for transmission */
    wr_err = tcp_write(tpcb, ptr->payload, ptr->len, TCP_WRITE_FLAG_COPY | TCP_WRITE_FLAG_MORE);
    Serial.println((int)wr_err);
    if (wr_err == ERR_OK) {
      u16_t plen;

      plen = ptr->len;
      /* continue with next pbuf in chain (if any) */
      es->p = ptr->next;
      if(es->p != NULL) {
        /* new reference! */
        pbuf_ref(es->p);
      }
      /* chop first pbuf from chain */
      pbuf_free(ptr);

      es->retries = 0;

      /* we can read more data now */
      tcp_recved(tpcb, plen);
    } else if(wr_err == ERR_MEM) {
      /* we are low on memory, try later / harder, defer to poll */
      es->p = ptr;
      es->retries++;
      break;
    } else {
      /* other problem ?? */
    }
  }
}

void
tcpecho_raw_error(void *arg, err_t err)
{
  Serial.println("raw_error");

  struct tcpecho_raw_state *es;

  LWIP_UNUSED_ARG(err);

  es = (struct tcpecho_raw_state *)arg;

  tcpecho_raw_free(es);
}

err_t
tcpecho_raw_poll(void *arg, struct tcp_pcb *tpcb)
{
  Serial.println("raw_poll");

  err_t ret_err;
  struct tcpecho_raw_state *es;

  es = (struct tcpecho_raw_state *)arg;
  if (es != NULL) {
    //Serial.print("state:"); Serial.print((int)es->state); Serial.print(",");
    //Serial.print("retries:"); Serial.print((int)es->retries); Serial.print(",");
    if (es->p != NULL) {

      if (es->retries < 5) {
      /* there is a remaining pbuf (chain)  */
      tcpecho_raw_send(tpcb, es);
      }
      else
      {
        tcp_abort(tpcb);
        ret_err = ERR_ABRT;
      }
      /*
      Serial.print((int)tpcb->unacked); Serial.print(",");
      Serial.print(tpcb->cwnd); Serial.print(",");
      Serial.print((int)tpcb->dupacks); Serial.print(",");
      Serial.print((int)tpcb->flags); Serial.print(",");
      Serial.print((int)tpcb->last_timer); Serial.print(",");
      */
      Serial.println("");
    } else {
      /* no remaining pbuf (chain)  */
      //if(es->state == ES_CLOSING) {
        tcpecho_raw_close(tpcb, es);
      //}
    }
    ret_err = ERR_OK;
  } else {
    /* nothing to be done */
    tcp_abort(tpcb);
    ret_err = ERR_ABRT;
  }
  return ret_err;
}

err_t
tcpecho_raw_sent(void *arg, struct tcp_pcb *tpcb, u16_t len)
{
  Serial.println("raw_sent");

  struct tcpecho_raw_state *es;

  LWIP_UNUSED_ARG(len);

  es = (struct tcpecho_raw_state *)arg;
  es->retries = 0;

  if(es->p != NULL) {
    /* still got pbufs to send */
    tcp_sent(tpcb, tcpecho_raw_sent);
    tcpecho_raw_send(tpcb, es);
  } else {
    /* no more pbufs to send */
    if(es->state == ES_CLOSING) {
      tcpecho_raw_close(tpcb, es);
    }
  }

  return ERR_OK;
}

err_t
tcpecho_raw_recv(void *arg, struct tcp_pcb *tpcb, struct pbuf *p, err_t err)
{
  Serial.println("raw_recv");

  struct tcpecho_raw_state *es;
  err_t ret_err;

  LWIP_ASSERT("arg != NULL",arg != NULL);
  es = (struct tcpecho_raw_state *)arg;
  if (p == NULL) {
    /* remote host closed connection */
    es->state = ES_CLOSING;
    if(es->p == NULL) {
      /* we're done sending, close it */
      tcpecho_raw_close(tpcb, es);
    } else {
      /* we're not done yet */
      tcpecho_raw_send(tpcb, es);
    }
    ret_err = ERR_OK;
  } else if(err != ERR_OK) {
    /* cleanup, for unknown reason */
    if (p != NULL) {
      pbuf_free(p);
    }
    ret_err = err;
  }
  else if(es->state == ES_ACCEPTED) {
    /* first data chunk in p->payload */
    es->state = ES_RECEIVED;
    /* store reference to incoming pbuf (chain) */
    es->p = p;
    tcpecho_raw_send(tpcb, es);
    ret_err = ERR_OK;
  } else if (es->state == ES_RECEIVED) {
    /* read some more data */
    if(es->p == NULL) {
      es->p = p;
      tcpecho_raw_send(tpcb, es);
    } else {
      struct pbuf *ptr;

      /* chain pbufs to the end of what we recv'ed previously  */
      ptr = es->p;
      pbuf_cat(ptr,p);
    }
    ret_err = ERR_OK;
  } else {
    /* unkown es->state, trash data  */
    tcp_recved(tpcb, p->tot_len);
    pbuf_free(p);
    ret_err = ERR_OK;
  }

  return ret_err;
}

err_t
tcpecho_raw_accept(void *arg, struct tcp_pcb *newpcb, err_t err)
{
  Serial.println("raw_accept");

  err_t ret_err;
  struct tcpecho_raw_state *es;

  LWIP_UNUSED_ARG(arg);
  if ((err != ERR_OK) || (newpcb == NULL)) {
    return ERR_VAL;
  }

  /* Unless this pcb should have NORMAL priority, set its priority now.
     When running out of pcbs, low priority pcbs can be aborted to create
     new pcbs of higher priority. */
  tcp_setprio(newpcb, TCP_PRIO_MAX);
  tcp_set_flags(newpcb, TF_NODELAY);

  es = (struct tcpecho_raw_state *)mem_malloc(sizeof(struct tcpecho_raw_state));
  if (es != NULL) {
    es->state = ES_ACCEPTED;
    es->pcb = newpcb;
    es->retries = 0;
    es->p = NULL;
    /* pass newly allocated es to our callbacks */
    tcp_arg(newpcb, es);
    tcp_recv(newpcb, tcpecho_raw_recv);
    tcp_err(newpcb, tcpecho_raw_error);
    tcp_poll(newpcb, tcpecho_raw_poll, 0);
    tcp_sent(newpcb, tcpecho_raw_sent);
    ret_err = ERR_OK;
  } else {
    ret_err = ERR_MEM;
  }
  return ret_err;
}

void
tcpecho_raw_init(void)
{
  Serial.println("raw_init");
  tcpecho_raw_pcb = tcp_new(); // tcp_new_ip_type(IPADDR_TYPE_ANY);
  if (tcpecho_raw_pcb != NULL) {
    err_t err;

    err = tcp_bind(tcpecho_raw_pcb, IP_ANY_TYPE, 7);
    if (err == ERR_OK) {
      tcpecho_raw_pcb = tcp_listen(tcpecho_raw_pcb);
      tcp_accept(tcpecho_raw_pcb, tcpecho_raw_accept);
    } else {
      /* abort? output diagnostic? */
    }
  } else {
    /* abort? output diagnostic? */
  }
}

void setup()
{
  if (Ethernet.begin() == 0) {
    Serial.println("Failed to set static IP");
  }

  Serial.print("server is at ");
  Serial.println(Ethernet.localIP());
}

void loop()
{
  if (restart)
  {
    restart = false;
    tcpecho_raw_init();
  }
  delay(10);
}
facchinm commented 9 months ago

@Eduardo-Lopes can you try applying this PR https://github.com/arduino/ArduinoCore-renesas/pull/234 ? It should improve things drastically. /cc @andreagilardoni

andreagilardoni commented 9 months ago

@Eduardo-Lopes As I remember I encountered this issue while dealing with #234, it has to be something related to missing deallocation of pbuf. If you use #234 this should be fixed. That PR should be able to handle higher loads with ethernet, I will also provide some docs on how to tweak performances to reach the highest possible performances, if you need that now I can provide that to you.

That PR also addresses issues on memory leakage of Server classes.

Any feedback is appreciated.

Eduardo-Lopes commented 9 months ago

Thanks! I was trying to compile the PR with e2 Studio, but no success.

If there is a way that you can help me.

My case doesn't involve high performance, but reliability.

On Wed, Jan 17, 2024, 06:42 Andrea Gilardoni @.***> wrote:

@Eduardo-Lopes https://github.com/Eduardo-Lopes As I remember I encountered this issue while dealing with #234 https://github.com/arduino/ArduinoCore-renesas/pull/234, it has to be something related to missing deallocation of pbuf. If you use #234 https://github.com/arduino/ArduinoCore-renesas/pull/234 this should be fixed. That PR should be able to handle higher loads with ethernet, I will also provide some docs on how to tweak performances to reach the highest possible performances, if you need that now I can provide that to you.

— Reply to this email directly, view it on GitHub https://github.com/arduino/ArduinoCore-renesas/issues/240#issuecomment-1895441090, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIWXSAU7W5H4LUDGI34UX3YO6MHLAVCNFSM6AAAAABB3FQZEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJVGQ2DCMBZGA . You are receiving this because you were mentioned.Message ID: @.***>

andreagilardoni commented 9 months ago

I don't know how to help you with e2 Studio, you can have a try with arduino IDE or arduino-cli.

As described, #234 also addresses reliability.