ARMmbed / sal-driver-lwip-k64f-eth

LwIP platform-specific implementation for k64f
Other
1 stars 3 forks source link

lwip raw only guarantees thread safety for memory and pbuf functions #8

Open bremoran opened 8 years ago

bremoran commented 8 years ago

TCP/UDP input/output functions are not guaranteed to be threadsafe. Only memory management and pbuf functions have that guarantee. Therefore, the ethernet driver should defer all packet processing to thread context via scheduled events, since a race condition could easily occur.

See here: http://lwip.wikia.com/wiki/Raw/native_API

The k64f_enetif_input function is probably the prime candidate for deferring.

This function calls through to ethernet_input in etharp.c, which performs many functions that are not guaranteed to be irq safe.

I suggest making the following change:

void call_input(struct pbuf *p, struct netif *netif) {
    /* full packet send to tcpip_thread to process */
    if (netif->input(p, netif) != ERR_OK) {
      LWIP_DEBUGF(NETIF_DEBUG, ("k64f_enetif_input: IP input error\n"));
      /* Free buffer */
      pbuf_free(p);
    }
}

/** \brief  Attempt to read a packet from the EMAC interface.
 *
 *  \param[in] netif the lwip network interface structure
 *  \param[in] idx   index of packet to be read
 */
void k64f_enetif_input(struct netif *netif, int idx)
{
  struct eth_hdr *ethhdr;
  struct pbuf *p;

  /* move received packet into a new pbuf */
  p = k64f_low_level_input(netif, idx);
  if (p == NULL) {
    return;
  }

  /* points to packet payload, which starts with an Ethernet header */
  ethhdr = (struct eth_hdr*)p->payload;

  switch (htons(ethhdr->type)) {
    case ETHTYPE_IP:
    case ETHTYPE_ARP:
#if PPPOE_SUPPORT
    case ETHTYPE_PPPOEDISC:
    case ETHTYPE_PPPOE:
#endif /* PPPOE_SUPPORT */
      minar::Scheduler::postCallback(
          FunctionPointer2<void, struct pbuf *, struct netif *>(call_input)
            .bind(p, netif));
      break;

    default:
      /* Return buffer */
      pbuf_free(p);
      break;
  }
}

This is greatly oversimplified, since it k64f_emac.c is not C++

ciarmcom commented 8 years ago

ARM Internal Ref: IOTSFW-1459