This two FIFO handling bugs in LM3S ethernet driver

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@2031 42af7a65-404d-4744-a932-0658087f49c3
2009-09-09 18:00:13 +00:00 · 2009-09-09 18:00:13 +00:00 · 9a651a8b0e
commit 9a651a8b0e
parent 570d52d5a5
10 changed files with 57 additions and 11 deletions
--- a/8
+++ b/8
@ -848,3 +848,11 @@
 	  argument to macro caused strcasecmp() and strncasecmp() to fail.
 	* lib/lib_strstr.c:  Length of substring off by one causes false alarm
 	  sub-string matches.
+	* arch/arm/src/lm3s/lm3s_ethernet.c: Fix errors in LMS6918 FIFO length
+	  handling.  (1) The incorrect size of the ethernet header was being
+	  subtracted on outgoing messages (4 vs 14), which caused outgoing messages to
+	  be a little too long.  (2) The size of incoming FIFO messages is 6 bytes
+	  larger than it expected (2 for the length and 4 for the FCS).  The unhandled
+	  extra two bytes of length cause the driver to sometimes read one too many
+	  words from the received FIFO (corrupting the next queued receive packet,
+	  if any).
--- a/Documentation/NuttX.html
+++ b/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: August 15, 2009</p>
+      <p>Last Updated: September 09, 2009</p>
    </td>
  </tr>
 </table>
@ -1509,6 +1509,14 @@ nuttx-0.4.11 2009-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 	  argument to macro caused strcasecmp() and strncasecmp() to fail.
 	* lib/lib_strstr.c:  Length of substring off by one causes false alarm
 	  sub-string matches.
+	* arch/arm/src/lm3s/lm3s_ethernet.c: Fix errors in LMS6918 FIFO length
+	  handling.  (1) The incorrect size of the ethernet header was being
+	  subtracted on outgoing messages (4 vs 14), which caused outgoing messages to
+	  be a little too long.  (2) The size of incoming FIFO messages is 6 bytes
+	  larger than it expected (2 for the length and 4 for the FCS).  The unhandled
+	  extra two bytes of length cause the driver to sometimes read one too many
+	  words from the received FIFO (corrupting the next queued receive packet,
+	  if any).

 pascal-0.1.3 2009-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;

--- a/arch/arm/src/lm3s/lm3s_ethernet.c
+++ b/arch/arm/src/lm3s/lm3s_ethernet.c
@ -127,6 +127,14 @@
 #define LM3S_RCTCL_SETBITS (LM3S_AMUL_SETBITS|LM3S_PRMS_SETBITS|LM3S_BADCRC_SETBITS)
 #define LM3S_RCTCL_CLRBITS (LM3S_AMUL_CLRBITS|LM3S_PRMS_CLRBITS|LM3S_BADCRC_CLRBITS)

+/* CONFIG_LM3S_DUMPPACKET will dump the contents of each packet to the console. */
+
+#ifdef CONFIG_LM3S_DUMPPACKET
+#  define lm3s_dumppacket(m,a,n) lib_dumpbuffer(m,a,n)
+#else
+#  define lm3s_dumppacket(m,a,n)
+#endif
+
 /* TX poll deley = 1 seconds. CLK_TCK is the number of clock ticks per second */

 #define LM3S_WDDELAY   (1*CLK_TCK)
@ -483,6 +491,7 @@ static int lm3s_transmit(struct lm3s_driver_s *priv)
      /* Increment statistics */

      EMAC_STAT(priv, tx_packets);
+      lm3s_dumppacket("Transmit packet", priv->ld_dev.d_buf, priv->ld_dev.d_len);

      /* Transfer the packet into the Tx FIFO.  The LS 16-bits of the first
       * 32-bit word written to the Tx FIFO contains the Ethernet payload
@ -495,7 +504,7 @@ static int lm3s_transmit(struct lm3s_driver_s *priv)
      DEBUGASSERT(pktlen > UIP_LLH_LEN);

      dbuf       = priv->ld_dev.d_buf;
-      regval     = (uint32)(pktlen - 4);
+      regval     = (uint32)(pktlen - 14);
      regval    |= ((uint32)(*dbuf++) << 16);
      regval    |= ((uint32)(*dbuf++) << 24);
      lm3s_ethout(priv, LM3S_MAC_DATA_OFFSET, regval);
@ -639,7 +648,8 @@ static void lm3s_receive(struct lm3s_driver_s *priv)
       * word from the FIFO followed by the Ethernet header beginning
       * in the MS 16-bits of the first word.
       *
-       * Pick off the packet length from the first word.
+       * Pick off the packet length from the first word.  This packet length
+       * includes the len/type field (size 2) and the FCS (size 4).
       */

      regval = lm3s_ethin(priv, LM3S_MAC_DATA_OFFSET);
@ -660,11 +670,11 @@ static void lm3s_receive(struct lm3s_driver_s *priv)
          ndbg("Bad packet size dropped (%d)\n", pktlen);
          EMAC_STAT(priv, rx_pktsize);

-          /* This is the number of bytes and words left to read (including,
-           * the final, possibly partial word).
+          /* The number of bytes and words left to read is pktlen - 4 (including,
+           * the final, possibly partial word) because we've already read 4 bytes.
           */

-          wordlen = (pktlen + 1) >> 4;
+          wordlen = (pktlen - 1) >> 2;

          /* Read and discard the remaining words in the FIFO */

@ -683,9 +693,12 @@ static void lm3s_receive(struct lm3s_driver_s *priv)
      *dbuf++   = (ubyte)((regval >> 16) & 0xff);
      *dbuf++   = (ubyte)((regval >> 24) & 0xff);

-      /* Read all of the whole, 32-bit values in the middle of the packet */
+      /* Read all of the whole, 32-bit values in the middle of the packet.
+       * We've already read the length (2 bytes) plus the first two bytes
+       * of data
+       */

-      for (bytesleft = pktlen - 2; bytesleft > 3; bytesleft -= 4, dbuf += 4)
+      for (bytesleft = pktlen - 4; bytesleft > 3; bytesleft -= 4, dbuf += 4)
        {
          /* Transfer a whole word to the user buffer.  Note, the user
           * buffer may be un-aligned.
@ -717,9 +730,13 @@ static void lm3s_receive(struct lm3s_driver_s *priv)
            }
        }

-      /* Pass the packet length to uIP */
+      lm3s_dumppacket("Received packet", priv->ld_dev.d_buf, pktlen);

-      priv->ld_dev.d_len = pktlen;
+      /* Pass the packet length to uIP MINUS 2 bytes for the length and
+       * 4 bytes for the FCS.
+       */
+
+      priv->ld_dev.d_len = pktlen - 6;

      /* We only accept IP packets of the configured type and ARP packets */

@ -916,7 +933,7 @@ static void lm3s_txtimeout(int argc, uint32 arg, ...)
 {
  struct lm3s_driver_s *priv = (struct lm3s_driver_s *)arg;

-  /* Increment statistics and dump debug info */
+  /* Increment statistics */

  ndbg("Tx timeout\n");
  EMAC_STAT(priv, tx_timeouts);
--- a/configs/eagle100/README.txt
+++ b/configs/eagle100/README.txt
@ -287,6 +287,7 @@ Eagle100-specific Configuration Options
 	CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 	CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 	CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+	CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.

 Configurations
 ^^^^^^^^^^^^^^
--- a/configs/eagle100/httpd/defconfig
+++ b/configs/eagle100/httpd/defconfig
@ -152,6 +152,7 @@ CONFIG_SSI_POLLWAIT=y
 # CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 # CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 # CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+# CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.
 #
 CONFIG_LM3S_ETHERNET=y
 CONFIG_LM3S_ETHLEDS=n
@ -162,6 +163,7 @@ CONFIG_LM3S_ETHNOPAD=n
 CONFIG_LM3S_MULTICAST=n
 CONFIG_LM3S_PROMISCUOUS=n
 CONFIG_LM3S_BADCRC=n
+CONFIG_LM3S_DUMPPACKET=n

 #
 # General build options
--- a/configs/eagle100/nettest/defconfig
+++ b/configs/eagle100/nettest/defconfig
@ -152,6 +152,7 @@ CONFIG_SSI_POLLWAIT=y
 # CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 # CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 # CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+# CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.
 #
 CONFIG_LM3S_ETHERNET=y
 CONFIG_LM3S_ETHLEDS=n
@ -162,6 +163,7 @@ CONFIG_LM3S_ETHNOPAD=n
 CONFIG_LM3S_MULTICAST=n
 CONFIG_LM3S_PROMISCUOUS=n
 CONFIG_LM3S_BADCRC=n
+CONFIG_LM3S_DUMPPACKET=n

 #
 # General build options
--- a/configs/eagle100/nsh/defconfig
+++ b/configs/eagle100/nsh/defconfig
@ -152,6 +152,7 @@ CONFIG_SSI_POLLWAIT=y
 # CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 # CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 # CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+# CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.
 #
 CONFIG_LM3S_ETHERNET=n
 CONFIG_LM3S_ETHLEDS=n
@ -162,6 +163,7 @@ CONFIG_LM3S_ETHNOPAD=n
 CONFIG_LM3S_MULTICAST=n
 CONFIG_LM3S_PROMISCUOUS=n
 CONFIG_LM3S_BADCRC=n
+CONFIG_LM3S_DUMPPACKET=n

 #
 # General build options
--- a/configs/eagle100/nxflat/defconfig
+++ b/configs/eagle100/nxflat/defconfig
@ -152,6 +152,7 @@ CONFIG_SSI_POLLWAIT=y
 # CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 # CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 # CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+# CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.
 #
 CONFIG_LM3S_ETHERNET=n
 CONFIG_LM3S_ETHLEDS=n
@ -162,6 +163,7 @@ CONFIG_LM3S_ETHNOPAD=n
 CONFIG_LM3S_MULTICAST=n
 CONFIG_LM3S_PROMISCUOUS=n
 CONFIG_LM3S_BADCRC=n
+CONFIG_LM3S_DUMPPACKET=n

 #
 # General build options
--- a/configs/eagle100/ostest/defconfig
+++ b/configs/eagle100/ostest/defconfig
@ -152,6 +152,7 @@ CONFIG_SSI_POLLWAIT=y
 # CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 # CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 # CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+# CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.
 #
 CONFIG_LM3S_ETHERNET=n
 CONFIG_LM3S_ETHLEDS=n
@ -162,6 +163,7 @@ CONFIG_LM3S_ETHNOPAD=n
 CONFIG_LM3S_MULTICAST=n
 CONFIG_LM3S_PROMISCUOUS=n
 CONFIG_LM3S_BADCRC=n
+CONFIG_LM3S_DUMPPACKET=n

 #
 # General build options
--- a/configs/eagle100/thttpd/defconfig
+++ b/configs/eagle100/thttpd/defconfig
@ -152,6 +152,7 @@ CONFIG_SSI_POLLWAIT=y
 # CONFIG_LM3S_MULTICAST - Set to enable multicast frames
 # CONFIG_LM3S_PROMISCUOUS - Set to enable promiscuous mode
 # CONFIG_LM3S_BADCRC - Set to enable bad CRC rejection.
+# CONFIG_LM3S_DUMPPACKET - Dump each packet received/sent to the console.
 #
 CONFIG_LM3S_ETHERNET=y
 CONFIG_LM3S_ETHLEDS=n
@ -162,6 +163,7 @@ CONFIG_LM3S_ETHNOPAD=n
 CONFIG_LM3S_MULTICAST=n
 CONFIG_LM3S_PROMISCUOUS=n
 CONFIG_LM3S_BADCRC=n
+CONFIG_LM3S_DUMPPACKET=n

 #
 # General build options