SLIP corrections

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3376 42af7a65-404d-4744-a932-0658087f49c3

ChangeLog

@@ -1555,3 +1555,10 @@
 
 5.20 2011-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr>
 
+	* lib/lib_fopen() -- fopen() was not returning the correct errno value
+	  when the underlying open() failed.
+	* include/net/uip/uip-arch.h -- The uIP interface has been extended
+	  slightly so that drivers can be concurrenly filling and sending
+	  packet buffers.  This capability was needed by the SLIP driver.
+	* drivers/net/slip.c -- Several corrections and some re-design of
+	  of the driver.

Documentation/NuttX.html

@@ -8,7 +8,7 @@
   <tr align="center" bgcolor="#e4e4e4">
     <td>
       <h1><big><font color="#3c34ec"><i>NuttX RTOS</i></font></big></h1>
-      <p>Last Updated: March 12, 2011</p>
+      <p>Last Updated: March 13, 2011</p>
     </td>
   </tr>
 </table>
@@ -2177,6 +2177,14 @@ buildroot-1.9 2011-02-10 &lt;spudmonkey@racsa.co.cr&gt;
 <ul><pre>
 nuttx-5.20 2011-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 
+    * lib/lib_fopen() -- fopen() was not returning the correct errno value
+      when the underlying open() failed.
+    * include/net/uip/uip-arch.h -- The uIP interface has been extended
+      slightly so that drivers can be concurrenly filling and sending
+      packet buffers.  This capability was needed by the SLIP driver.
+    * drivers/net/slip.c -- Several corrections and some re-design of
+      of the driver.
+
 pascal-2.1 2011-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 
 buildroot-1.10 2011-xx-xx &lt;spudmonkey@racsa.co.cr&gt;

configs/olimex-lpc1766stk/slip-httpd/defconfig

@@ -336,7 +336,7 @@ CONFIG_START_DAY=23
 CONFIG_GREGORIAN_TIME=n
 CONFIG_JULIAN_TIME=n
 CONFIG_DEV_CONSOLE=y
-CONFIG_DEV_LOWCONSOLE=y
+CONFIG_DEV_LOWCONSOLE=n
 CONFIG_MUTEX_TYPES=n
 CONFIG_PRIORITY_INHERITANCE=n
 CONFIG_SEM_PREALLOCHOLDERS=0
@@ -520,6 +520,7 @@ CONFIG_MMCSD_HAVECARDDETECT=n
 # TCP/IP and UDP support via uIP
 # CONFIG_NET - Enable or disable all network features
 # CONFIG_NET_SLIP - Use the SLIP data link layer (L2)
+# CONFIG_NET_MULTIBUFFER - Use multiple input/output buffers (probably no)
 # CONFIG_NET_IPv6 - Build in support for IPv6
 # CONFIG_NSOCKET_DESCRIPTORS - Maximum number of socket descriptors per task/thread.
 # CONFIG_NET_SOCKOPTS - Enable or disable support for socket options
@@ -545,6 +546,7 @@ CONFIG_MMCSD_HAVECARDDETECT=n
 #
 CONFIG_NET=y
 CONFIG_NET_SLIP=y
+CONFIG_NET_MULTIBUFFER=y
 CONFIG_NET_IPv6=n
 CONFIG_NSOCKET_DESCRIPTORS=16
 CONFIG_NET_SOCKOPTS=y

drivers/net/slip.c

@@ -52,7 +52,6 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <wdog.h>
-#include <poll.h>
 #include <errno.h>
 #include <debug.h>
 
@@ -138,9 +137,10 @@ struct slip_driver_s
 {
   bool     bifup;           /* true:ifup false:ifdown */
   WDOG_ID  txpoll;          /* TX poll timer */
-  int      fd;              /* File descriptor associated with stream */
   FILE    *stream;          /* The contained serial stream */
+  pid_t    pid;             /* Receiver thread ID */
   sem_t    waitsem;         /* Only used at start-up */
+  uint16_t rxlen;           /* The number of bytes in rxbuf */
 
   /* Driver statistics */
 
@@ -151,6 +151,8 @@ struct slip_driver_s
   /* This holds the information visible to uIP/NuttX */
 
   struct uip_driver_s dev;  /* Interface understood by uIP */
+  uint8_t rxbuf[CONFIG_NET_BUFSIZE + 2];
+  uint8_t txbuf[CONFIG_NET_BUFSIZE + 2];
 };
 
 /****************************************************************************
@@ -180,7 +182,6 @@ static int  slip_uiptxpoll(struct uip_driver_s *dev);
 /* Packet receiver task */
 
 static inline int slip_getc(FAR struct slip_driver_s *priv);
-static inline void slip_readpacket(FAR struct slip_driver_s *priv);
 static inline void slip_receive(FAR struct slip_driver_s *priv);
 static int slip_rxtask(int argc, char *argv[]);
 
@@ -375,6 +376,7 @@ static int slip_transmit(FAR struct slip_driver_s *priv)
     {
       slip_write(priv, start, len);
     }
+  fflush(priv->stream);
 
   /* And send the END token */
 
@@ -448,7 +450,7 @@ static inline int slip_getc(FAR struct slip_driver_s *priv)
 }
 
 /****************************************************************************
- * Function: slip_readpacket
+ * Function: slip_receive
  *
  * Description:
  *   Read a packet from the serial input
@@ -461,12 +463,11 @@ static inline int slip_getc(FAR struct slip_driver_s *priv)
  *
  ****************************************************************************/
 
-static inline void slip_readpacket(FAR struct slip_driver_s *priv)
+static inline void slip_receive(FAR struct slip_driver_s *priv)
 {
   uint8_t ch;
-  int     nbytes = 0;
 
-  /* Copy the data data from the hardware to priv->dev.d_buf until we
+  /* Copy the data data from the hardware to to the RX buffer until we
    * put together a whole packet. Make sure not to copy them into the
    * packet if we run out of room.
    */
@@ -474,7 +475,7 @@ static inline void slip_readpacket(FAR struct slip_driver_s *priv)
   nvdbg("Receiving packet\n");
   for (;;)
     {
-      /* Get the next character in the stream */
+      /* Get the next character in the stream. */
 
       ch = slip_getc(priv);
 
@@ -489,16 +490,15 @@ static inline void slip_readpacket(FAR struct slip_driver_s *priv)
         case SLIP_END:
           nvdbg("END\n");
 
-          /* a minor optimization: if there is no data in the packet, ignore
+          /* A minor optimization: if there is no data in the packet, ignore
            * it. This is meant to avoid bothering IP with all the empty
            * packets generated by the duplicate END characters which are in
            * turn sent to try to detect line noise.
            */
 
-          if (nbytes > 0)
+          if (priv->rxlen > 0)
             {
-              nvdbg("Received packet size %d\n", nbytes);
+              nvdbg("Received packet size %d\n", priv->rxlen);
-              priv->dev.d_len = nbytes;
               return;
             }
           break;
@@ -523,8 +523,8 @@ static inline void slip_readpacket(FAR struct slip_driver_s *priv)
               nvdbg("ESC-END\n");
               ch = SLIP_END;
               break;
-              nvdbg("ESC-ESC\n");
              case SLIP_ESC_ESC:
+              nvdbg("ESC-ESC\n");
               ch = SLIP_ESC;
               break;
             default:
@@ -537,57 +537,15 @@ static inline void slip_readpacket(FAR struct slip_driver_s *priv)
            */
 
         default:
-          if (nbytes < priv->dev.d_len)
+          if (priv->rxlen < CONFIG_NET_BUFSIZE+2)
             {
-              priv->dev.d_buf[nbytes++] = ch;
+              priv->rxbuf[priv->rxlen++] = ch;
             }
           break;
         }
     }
 }
 
-/****************************************************************************
- * Function: slip_receive
- *
- * Description:
- *   Accept and handle an incoming packet.
- *
- * Parameters:
- *   priv  - Reference to the driver state structure
- *
- * Returned Value:
- *   None
- *
- * Assumptions:
- *   Global interrupts are disabled by interrupt handling logic.
- *
- ****************************************************************************/
-
-static inline void slip_receive(FAR struct slip_driver_s *priv)
-{
-  /* Copy the data data from the hardware to priv->dev.d_buf until we put
-   * together a whole packet.
-   */
-
-  slip_readpacket(priv);
-  SLIP_STAT(priv, received);
-
-  /* All packets are assumed to be IP packets (we don't have a choice.. there
-   * is no Ethernet header containing the EtherType)
-   */
-
-  uip_input(&priv->dev);
-
-  /* If the above function invocation resulted in data that should be
-   * sent out on the network, the field  d_len will set to a value > 0.
-   */
-
-  if (priv->dev.d_len > 0)
-    {
-      slip_transmit(priv);
-    }
-}
-
 /****************************************************************************
  * Function: slip_rxtask
  *
@@ -608,10 +566,8 @@ static inline void slip_receive(FAR struct slip_driver_s *priv)
 static int slip_rxtask(int argc, char *argv[])
 {
   FAR struct slip_driver_s *priv;
-  struct pollfd fds[1];
   unsigned int index = *(argv[1]) - '0';
-  irqstate_t flags;
+  int ch;
-  int ret;
 
   ndbg("index: %d\n", index);
   DEBUGASSERT(index < CONFIG_SLIP_NINTERFACES);
@@ -625,27 +581,79 @@ static int slip_rxtask(int argc, char *argv[])
 
   for (;;)
     {
-      /* Set up the poll */
+      /* Wait for the next character to be available on the input stream. */
 
-      memset(fds, 0, sizeof(struct pollfd));
+      ch = slip_getc(priv);
-      fds[0].fd      = priv->fd;
-      fds[0].events  = POLLIN;
-      fds[0].revents = 0;
 
-      /* Wait for incoming data (with no timeout) */
+      /* We have something... three interesting cases.  First there might
+       * be a read error.  Anything other an EINTR would indicate a serious
+       * device-related problem or software bug.
+       */
 
-      ret = poll(fds, 1, -1);
+      if (ch == EOF)
-      DEBUGASSERT(ret >= 0);
-      if (ret > 0)
         {
-          /* Call slip_receive() to handle incoming data.  Interrupts must
+          DEBUGASSERT(errno == EINTR);
-           * be disabled here to keept the POLL watchdoe from firing.
+          continue;
+        }
+
+      /* END characters may appear at packet boundaries BEFORE as well as
+       * after the beginning of the packet.  This is normal and expected.
+       */
+
+      else if (ch == SLIP_END)
+        {
+          priv->rxlen = 0;
+        }
+
+      /* Otherwise, we are in danger of being out-of-sync.  Apparently the
+       * leading END character is optional.  Let's try to continue.
+       */
+
+      else
+        {
+          priv->rxbuf[0] = (uint8_t)ch;
+          priv->rxlen    = 1;
+        }
+
+      /* Copy the data data from the hardware to priv->rxbuf until we put
+       * together a whole packet.
+       */
+
+      slip_receive(priv);
+      SLIP_STAT(priv, received);
+
+      /* All packets are assumed to be IP packets (we don't have a choice..
+       * there is no Ethernet header containing the EtherType).  So pass the
+       * received packet on for IP processing -- but only if it is big
+       * enough to hold an IP header.
+       */
+
+      if (priv->rxlen >= UIP_IPH_LEN)
+        {
+          /* Handle the IP input.  Interrupts must be disabled here to
+           * keep the POLL watchdog from firing.
            */
 
-          flags = irqsave();
+          irqstate_t flags = irqsave();
-          slip_receive(priv);
+          priv->dev.d_buf = priv->rxbuf;
+          priv->dev.d_len = priv->rxlen;
+          uip_input(&priv->dev);
+
+          /* If the above function invocation resulted in data that should
+           * be sent out on the network, the field  d_len will set to a
+           * value > 0.  NOTE that we are transmitting using the RX buffer!
+           */
+
+          if (priv->dev.d_len > 0)
+            {
+              slip_transmit(priv);
+            }
           irqrestore(flags);
         }
+      else
+        {
+          SLIP_STAT(priv, rxsmallpacket);
+        }
     }
 
   /* We won't get here */
@@ -677,6 +685,7 @@ static void slip_polltimer(int argc, uint32_t arg, ...)
 
   /* If so, update TCP timing states and poll uIP for new XMIT data. */
 
+  priv->dev.d_buf = priv->txbuf;
   (void)uip_timer(&priv->dev, slip_uiptxpoll, SLIP_POLLHSEC);
 
   /* Setup the watchdog poll timer again */
@@ -791,6 +800,7 @@ static int slip_txavail(struct uip_driver_s *dev)
     {
       /* Poll uIP for new XMIT data */
 
+      priv->dev.d_buf = priv->txbuf;
       (void)uip_poll(&priv->dev, slip_uiptxpoll);
     }
 
@@ -881,8 +891,8 @@ static int slip_rmmac(struct uip_driver_s *dev, FAR const uint8_t *mac)
 int slip_initialize(int intf, const char *devname)
 {
   struct slip_driver_s *priv;
-  char buffer[IFNAMSIZ];
+  char buffer[8];
-  pid_t pid;
+  const char *argv[2];
 
   /* Get the interface structure associated with this interface number. */
 
@@ -903,18 +913,10 @@ int slip_initialize(int intf, const char *devname)
 
   /* Open the device */
 
-  priv->fd            = open(devname, O_RDWR, 0666);
+  priv->stream        = fopen(devname, "rw");
-  if (priv->fd < 0)
-    {
-      ndbg("ERROR: Failed to open %s: %d\n", devname, errno);
-      return -errno;
-    }
-
-  priv->stream        = fdopen(priv->fd, "rw");
   if (priv->stream == NULL)
     {
       ndbg("ERROR: Failed to fdopen %s: %d\n", devname, errno);
-      close(priv->fd);
       return -errno;
     }
 
@@ -930,16 +932,18 @@ int slip_initialize(int intf, const char *devname)
 
   /* Start the SLIP receiver task */
 
-  snprintf(buffer, IFNAMSIZ, "%d", intf);
+  snprintf(buffer, 8, "%d", intf);
+  argv[0] = buffer;
+  argv[1] = NULL;
 
 #ifndef CONFIG_CUSTOM_STACK
-  pid = task_create("usbhost", CONFIG_SLIP_DEFPRIO, CONFIG_SLIP_STACKSIZE,
+  priv->pid = task_create("usbhost", CONFIG_SLIP_DEFPRIO,
-                    (main_t)slip_rxtask, (const char **)buffer);
+                         CONFIG_SLIP_STACKSIZE, (main_t)slip_rxtask, argv);
 #else
-  pid = task_create("usbhost", CONFIG_SLIP_DEFPRIO,
+  priv->pid = task_create("usbhost", CONFIG_SLIP_DEFPRIO,
-                    (main_t)slip_rxtask, (const char **)buffer);
+                         (main_t)slip_rxtask, argv);
 #endif
-  if (pid < 0)
+  if (priv->pid < 0)
     {
       ndbg("ERROR: Failed to start receiver task\n");
       return -errno;
@@ -956,7 +960,7 @@ int slip_initialize(int intf, const char *devname)
   /* Register the device with the OS so that socket IOCTLs can be performed */
 
 #if CONFIG_NSOCKET_DESCRIPTORS > 0
-  snprintf(buffer, IFNAMSIZ, "slip%d", intf);
+  snprintf(priv->dev.d_ifname, IFNAMSIZ, "slip%d", intf);
 #endif
   (void)netdev_register(&priv->dev);
   return OK;

include/net/uip/uip-arch.h

@@ -102,33 +102,23 @@ struct uip_driver_s
   uip_ipaddr_t d_draddr;  /* Default router IP address */
   uip_ipaddr_t d_netmask; /* Network subnet mask */
 
-  /* The d_buf array is used to hold incoming and outgoing
+  /* The d_buf array is used to hold incoming and outgoing packets. The device
-   * packets. The device driver should place incoming data into this
+   * driver should place incoming data into this buffer. When sending data,
-   * buffer. When sending data, the device driver should read the link
+   * the device driver should read the link level headers and the TCP/IP
-   * level headers and the TCP/IP headers from this buffer. The size of
+   * headers from this buffer. The size of the link level headers is
-   * the link level headers is configured by the UIP_LLH_LEN define.
+   * configured by the UIP_LLH_LEN define.
    *
-   * Note: The application data need not be placed in this buffer, so
+   * uIP will handle only a single buffer for both incoming and outgoing
-   * the device driver must read it from the place pointed to by the
+   * packets.  However, the drive design may be concurrently send and
-   * d_appdata pointer as illustrated by the following example:
+   * filling separate, break-off buffers if CONFIG_NET_MULTIBUFFER is
-   *
+   * defined.  That buffer management must be controlled by the driver.
-   *    void
-   *    devicedriver_send(void)
-   *    {
-   *      hwsend(&dev->d_buf[0], UIP_LLH_LEN);
-   *      if(dev->d_len <= UIP_LLH_LEN + UIP_TCPIP_HLEN)
-   *        {
-   *          hwsend(&dev->d_buf[UIP_LLH_LEN], dev->d_len - UIP_LLH_LEN);
-   *        }
-   *      else
-   *        {
-   *          hwsend(&dev->d_buf[UIP_LLH_LEN], UIP_TCPIP_HLEN);
-   *          hwsend(dev->d_appdata, dev->d_len - UIP_TCPIP_HLEN - UIP_LLH_LEN);
-   *        }
-   *    }
    */
 
+#ifdef CONFIG_NET_MULTIBUFFER
+  uint8_t *d_buf;
+#else
   uint8_t d_buf[CONFIG_NET_BUFSIZE + 2];
+#endif
 
   /* d_appdata points to the location where application data can be read from
    * or written into a packet.

lib/lib_fopen.c

@@ -1,7 +1,7 @@
 /****************************************************************************
  * lib/lib_fopen.c
  *
- *   Copyright (C) 2007-2010 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2011 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -251,15 +251,34 @@ FAR FILE *fdopen(int fd, FAR const char *mode)
 
 FAR FILE *fopen(FAR const char *path, FAR const char *mode)
 {
-  FAR struct filelist   *flist = sched_getfiles();
+  FAR FILE *ret = NULL;;
-  FAR struct streamlist *slist = sched_getstreams();
+  int oflags;
-  int oflags = lib_mode2oflags(mode);
+  int fd;
-  int fd     = open(path, oflags, 0666);
 
-  FAR FILE *ret = lib_fdopen(fd, mode, flist, slist);
+  /* Open the file */
-  if (!ret)
+
+  oflags = lib_mode2oflags(mode);
+  fd = open(path, oflags, 0666);
+
+  /* If the open was successful, then fdopen() the fil using the file
+   * desciptor returned by open.  If open failed, then just return the
+   * NULL stream -- open() has already set the errno.
+   */
+
+  if (fd >= 0)
     {
-      (void)close(fd);
+      FAR struct filelist   *flist = sched_getfiles();
+      FAR struct streamlist *slist = sched_getstreams();
+
+      ret = lib_fdopen(fd, mode, flist, slist);
+      if (!ret)
+        {
+          /* Don't forget to close the file descriptor if any other
+           * failures are reported by fdopen().
+           */
+
+          (void)close(fd);
+        }
     }
   return ret;
 }