[ECOS] Re: RedBoot DHCP failure due to race condition.

[Grant Edwards <grant dot b dot edwards at gmail dot com>]

On 2011-03-16, Grant Edwards <grant.b.edwards@gmail.com> wrote:

Having the DHCP state machine split in two was just too hard to work
with.  I've re-written it so that the state machine is in one place
and the retry loop is smaller and easier to understand.  I think the
race conditions are eliminated and the timeout/retry mechanism now
works.  Yea, it's now got gotos -- and it's a heck of a lot easier to
read and understand that way.  It's also got a few long lines, and
it'll stay that way until I'm done working on it [I hate it when
'grep' doesn't print a complete function prototype or function call.]

I've done some prelinary testing in both BOOTP mode and DHCP mode.

If people think this looks good, I'll reformat it in eCos style and
submit it.  Please feel free to comment.

================================bootp.c========================================
#include <redboot.h>
#include <net/net.h>
#include <net/bootp.h>

#define RETRY_TIME_MS  2000
#define MAX_RETRIES    4 

static int          xid;     // BOOTP transaction ID,  should be random/unique
static ip_route_t   route;

#ifdef CYGSEM_REDBOOT_NETWORKING_DHCP
static const unsigned char dhcpCookie[] = {99,130,83,99};
static const unsigned char dhcpEnd[] = {255};
static const unsigned char dhcpDiscover[] = {53,1,1};
static const unsigned char dhcpRequest[] = {53,1,3};
static const unsigned char dhcpRequestIP[] = {50,4};
static const unsigned char dhcpParamRequestList[] = {55,3,1,3,6};
#endif

static enum
{
  DHCP_NONE=0,
  DHCP_WAITING_FOR_OFFER,
  DHCP_WAITING_FOR_ACK,
  DHCP_DONE,
  DHCP_FAILED
} dhcpState;                        // only NONE and DONE are used in BOOTP mode

#define DebugOutputEnable

#if !DebugOutputEnable
# define debug_printf(format, ...)  /* noop */
#else
# define debug_printf(format, ...)  diag_printf(format, ##__VA_ARGS__)
  static const char *dhcpStateString[] = {"NONE", "WAITING_FOR_OFFER","WAITING_FOR_ACK","DONE","FAILED"};
# ifdef CYGSEM_REDBOOT_NETWORKING_DHCP
    static const char *dhcpTypeString[]  = {"0x00", "DISCOVER", "OFFER", "REQUEST", "0x04", "ACK", "NAK", "0x07"};
# endif
#endif

# ifdef CYGSEM_REDBOOT_NETWORKING_DHCP
// parse network configuration from a DHCP ACK packet
static void parseACK(bootp_header_t *bp)
{
  unsigned char *end, *p;
  int optlen;
  memcpy(__local_ip_addr, &bp->bp_yiaddr, 4);
#ifdef CYGSEM_REDBOOT_NETWORKING_USE_GATEWAY
  memcpy(__local_ip_gate, &bp->bp_giaddr, 4);
#endif
  p = bp->bp_vend+4;
  end = (unsigned char *)bp+sizeof(*bp);
  while (p < end)
    {
      unsigned char tag = *p;
      if (tag == TAG_END)
        break;
      if (tag == TAG_PAD)
        optlen = 1;
      else
        {
          optlen = p[1];
          p += 2;
          switch (tag)
            {
#ifdef CYGSEM_REDBOOT_NETWORKING_USE_GATEWAY
            case TAG_SUBNET_MASK:  // subnet mask
              memcpy(__local_ip_mask,p,4);
              break;
            case TAG_GATEWAY:  // router
              memcpy(__local_ip_gate,p,4);
              break;
#endif
#ifdef CYGPKG_REDBOOT_NETWORKING_DNS
            case TAG_DOMAIN_SERVER:
              memcpy(&__bootp_dns_addr, p, 4);
              __bootp_dns_set = 1;
              break;
#endif
            default:
              break;
            }
        }
      p += optlen;
    }
}
#endif

// functions used to prepare BOOTP/DHCP tx packets

// basic BOOTP request
void prep_bootp_request(bootp_header_t *b)
{
  memset(b, 0, sizeof *b);
  b->bp_op = BOOTREQUEST;
  b->bp_htype = HTYPE_ETHERNET;
  b->bp_hlen = 6;
  b->bp_xid = xid++;
  memcpy(b->bp_chaddr, __local_enet_addr, 6);
}

#ifdef CYGSEM_REDBOOT_NETWORKING_DHCP

#define AddOption(p,d) do {memcpy(p,d,sizeof d); p += sizeof d;} while (0)

// add DHCP DISCOVER fields to a basic BOOTP request
int prep_dhcp_discover(bootp_header_t *b)
{
  unsigned char *p = b->bp_vend;
  AddOption(p,dhcpCookie);
  AddOption(p,dhcpDiscover);
  AddOption(p,dhcpParamRequestList);
  AddOption(p,dhcpEnd);
  if (p < &b->bp_vend[BP_MIN_VEND_SIZE])
    p = &b->bp_vend[BP_MIN_VEND_SIZE];
  return p - (unsigned char*)b;
}

// add DHCP REQUEST fields to a basic BOOTP request using data from supplied DHCP OFFER
int prep_dhcp_request(bootp_header_t *b, bootp_header_t *offer)
{
  unsigned char *p = b->bp_vend;
  b->bp_xid = offer->bp_xid;  // Match what server sent
  AddOption(p,dhcpCookie);
  AddOption(p,dhcpRequest);
  AddOption(p,dhcpRequestIP);
  memcpy(p, &offer->bp_yiaddr, 4);
  p += 4;  // Ask for the address just given
  AddOption(p,dhcpParamRequestList);
  AddOption(p,dhcpEnd);
  if (p < &b->bp_vend[BP_MIN_VEND_SIZE])
    p = &b->bp_vend[BP_MIN_VEND_SIZE];
  return p - (unsigned char*)b;
}
#endif

// Macro used to change state of BOOTP/DHCP state machine
#define NewDhcpState(state)  do {dhcpState = state; debug_printf("DHCP state: %s\n",dhcpStateString[state]);}while(0)

// Handler for received BOOTP/DHCP packets
static void bootp_handler(udp_socket_t *skt, char *buf, int len, ip_route_t *src_route, word src_port)
{
  bootp_header_t *b;
#ifdef CYGSEM_REDBOOT_NETWORKING_DHCP
  int txSize;
  unsigned char type;
  bootp_header_t txpkt;
  unsigned char *p, expected = 0;
#endif

  b = (bootp_header_t *)buf;

  // Only accept BOOTP REPLY responses
  if (b->bp_op != BOOTREPLY)
    return;

  // Must be sent to me, as well!
  if (memcmp(b->bp_chaddr, __local_enet_addr, 6))
    return;

#if !defined(CYGSEM_REDBOOT_NETWORKING_DHCP)
  // Simple BOOTP - this is all there is!
  debug_printf("BOOTP recv: REPLY\n");
  memcpy(__local_ip_addr, &b->bp_yiaddr, 4);
  NewDhcpState(DHCP_DONE);
#else
  // everything from here down is for "real" DHCP

  // make sure it's a DHCP packet
  p = b->bp_vend;
  if (memcmp(p, dhcpCookie, sizeof(dhcpCookie)))
    return;

  p += 4;

  // Find the DHCP Message Type tag
  while (*p != TAG_DHCP_MESS_TYPE)
    {
      p += p[1] + 2;
      if (p >= (unsigned char*)b + sizeof(*b))
        return;
    }

  type = p[2];
  debug_printf("DHCP  recv: %s [%d] %s\n",dhcpTypeString[type],type);

  switch (dhcpState)
    {
    case DHCP_WAITING_FOR_OFFER:
      if (type == (expected = DHCP_MESS_TYPE_OFFER))
        {
          prep_bootp_request(&txpkt);
          txSize = prep_dhcp_request(&txpkt,b);
          debug_printf("DHCP  send: REQUEST\n");
          NewDhcpState(DHCP_WAITING_FOR_ACK);
          __udp_send((char *)&txpkt, txSize, &route, IPPORT_BOOTPS, IPPORT_BOOTPC);
          return;
        }
      break;

    case DHCP_WAITING_FOR_ACK:
      if (type == (expected = DHCP_MESS_TYPE_ACK))
        {
          parseACK(b);
          NewDhcpState(DHCP_DONE);
          return;
        }
      break;

    default:
      debug_printf("DHCP packet ignored\n");
      return;
    }

  if (type == DHCP_MESS_TYPE_NAK)
    {
      NewDhcpState(DHCP_FAILED);
      return;
    }

  debug_printf("DHCP packet ignored -- expected %d[%s]\n", expected, dhcpTypeString[expected]);
#endif
}


// Request IP configuration via BOOTP/DHCP.
// Return zero if successful, -1 if not.

int __bootp_find_local_ip(bootp_header_t *info)
{
  udp_socket_t udp_skt;
  bootp_header_t b;
  int            retry;
  unsigned long  start;
  ip_addr_t saved_ip_addr;
  int txSize;
  char xiddata[16];

  diag_printf("\nRequesting IP conf via BOOTP/DHCP...\n");

  memcpy(xiddata+0,__local_enet_addr,6);           // it would be nice to have some entropy to mix in...
  xid = cyg_posix_crc32(xiddata,sizeof xiddata);

  memcpy(saved_ip_addr, __local_ip_addr, sizeof(__local_ip_addr));   // save our IP in case of failure

  // fill out route for a broadcast
  route.ip_addr[0] = 255;
  route.ip_addr[1] = 255;
  route.ip_addr[2] = 255;
  route.ip_addr[3] = 255;
  route.enet_addr[0] = 255;
  route.enet_addr[1] = 255;
  route.enet_addr[2] = 255;
  route.enet_addr[3] = 255;
  route.enet_addr[4] = 255;
  route.enet_addr[5] = 255;

  NewDhcpState(DHCP_NONE);

  __udp_install_listener(&udp_skt, IPPORT_BOOTPC, bootp_handler);

  retry = MAX_RETRIES;

  while (retry > 0)
    {
      start = MS_TICKS();
      memset(__local_ip_addr, 0, sizeof(__local_ip_addr));

      // send out a BOOTP request or DHCP DISCOVER
      prep_bootp_request(&b);                          // basic BOOTP request
#ifdef CYGSEM_REDBOOT_NETWORKING_DHCP
      debug_printf("DHCP  send: DISCOVER\n");
      NewDhcpState(DHCP_WAITING_FOR_OFFER);
      txSize = prep_dhcp_discover(&b);                 // make it into DHCP DISCOVER
#else
      debug_printf("BOOTP send: REQUEST\n");
      txSize = sizeof(b);
#endif
      __udp_send((char *)&b, txSize, &route, IPPORT_BOOTPS, IPPORT_BOOTPC);

      // wait for timeout, user-abort, or for receive packet handler to fail/succeed
      while ((MS_TICKS_DELAY() - start) < RETRY_TIME_MS)
        {
          __enet_poll();
          if (dhcpState == DHCP_FAILED)
            break;
          if (dhcpState == DHCP_DONE)
            goto done;
          if (_rb_break(1))    // did user hit ^C?
            goto failed;
          MS_TICKS_DELAY();
        } 
      --retry;
      diag_printf("TIMEOUT%s\n", retry ? ", retrying..." : "");
    }

 failed:
  diag_printf("FAIL\n");
  __udp_remove_listener(IPPORT_BOOTPC);
  memcpy(__local_ip_addr, saved_ip_addr, sizeof(__local_ip_addr));   // restore prev IP
  return -1;

 done:
  diag_printf("OK\n");
  __udp_remove_listener(IPPORT_BOOTPC);
  return 0;
}
================================bootp.c========================================


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss