Add RX quotas to the net device poll() method. This avoids the problem
authorMichael Brown <mcb30@etherboot.org>
Tue, 9 Jan 2007 21:47:01 +0000 (21:47 +0000)
committerMichael Brown <mcb30@etherboot.org>
Tue, 9 Jan 2007 21:47:01 +0000 (21:47 +0000)
of alloc_pkb() exhaustion when e.g. an iSCSI-booted DOS session is left
idle for a long time at the C:\ prompt and builds up a huge packet
backlog.

src/arch/i386/drivers/net/undinet.c
src/drivers/net/legacy.c
src/drivers/net/pnic.c
src/drivers/net/rtl8139.c
src/include/gpxe/netdevice.h
src/net/netdevice.c

index c53ff53..5ff9bd4 100644 (file)
@@ -364,7 +364,8 @@ static int undinet_transmit ( struct net_device *netdev,
 /** 
  * Poll for received packets
  *
- * @v netdev   Network device
+ * @v netdev           Network device
+ * @v rx_quota         Maximum number of packets to receive
  *
  * Fun, fun, fun.  UNDI drivers don't use polling; they use
  * interrupts.  We therefore cheat and pretend that an interrupt has
@@ -382,7 +383,7 @@ static int undinet_transmit ( struct net_device *netdev,
  * of installing a genuine interrupt service routine and dealing with
  * the wonderful 8259 Programmable Interrupt Controller.  Joy.
  */
-static void undinet_poll ( struct net_device *netdev ) {
+static void undinet_poll ( struct net_device *netdev, unsigned int rx_quota ) {
        struct undi_nic *undinic = netdev->priv;
        struct s_PXENV_UNDI_ISR undi_isr;
        struct pk_buff *pkb = NULL;
@@ -416,7 +417,7 @@ static void undinet_poll ( struct net_device *netdev ) {
        }
 
        /* Run through the ISR loop */
-       while ( 1 ) {
+       while ( rx_quota ) {
                if ( ( rc = undinet_call ( undinic, PXENV_UNDI_ISR, &undi_isr,
                                           sizeof ( undi_isr ) ) ) != 0 )
                        break;
@@ -448,6 +449,7 @@ static void undinet_poll ( struct net_device *netdev ) {
                        if ( pkb_len ( pkb ) == len ) {
                                netdev_rx ( netdev, pkb );
                                pkb = NULL;
+                               --rx_quota;
                        }
                        break;
                case PXENV_UNDI_ISR_OUT_DONE:
@@ -480,8 +482,8 @@ static void undinet_poll ( struct net_device *netdev ) {
  */
 static int undinet_open ( struct net_device *netdev ) {
        struct undi_nic *undinic = netdev->priv;
-       struct s_PXENV_UNDI_SET_STATION_ADDRESS set_address;
-       struct s_PXENV_UNDI_OPEN open;
+       struct s_PXENV_UNDI_SET_STATION_ADDRESS undi_set_address;
+       struct s_PXENV_UNDI_OPEN undi_open;
        int rc;
 
        /* Hook interrupt service routine and enable interrupt */
@@ -494,16 +496,16 @@ static int undinet_open ( struct net_device *netdev ) {
         * use it to set the MAC address to the card's permanent value
         * anyway.
         */
-       memcpy ( set_address.StationAddress, netdev->ll_addr,
-                sizeof ( set_address.StationAddress ) );
+       memcpy ( undi_set_address.StationAddress, netdev->ll_addr,
+                sizeof ( undi_set_address.StationAddress ) );
        undinet_call ( undinic, PXENV_UNDI_SET_STATION_ADDRESS,
-                      &set_address, sizeof ( set_address ) );
+                      &undi_set_address, sizeof ( undi_set_address ) );
 
        /* Open NIC */
-       memset ( &open, 0, sizeof ( open ) );
-       open.PktFilter = ( FLTR_DIRECTED | FLTR_BRDCST );
-       if ( ( rc = undinet_call ( undinic, PXENV_UNDI_OPEN, &open,
-                                  sizeof ( open ) ) ) != 0 )
+       memset ( &undi_open, 0, sizeof ( undi_open ) );
+       undi_open.PktFilter = ( FLTR_DIRECTED | FLTR_BRDCST );
+       if ( ( rc = undinet_call ( undinic, PXENV_UNDI_OPEN, &undi_open,
+                                  sizeof ( undi_open ) ) ) != 0 )
                goto err;
 
        return 0;
@@ -520,14 +522,31 @@ static int undinet_open ( struct net_device *netdev ) {
  */
 static void undinet_close ( struct net_device *netdev ) {
        struct undi_nic *undinic = netdev->priv;
-       struct s_PXENV_UNDI_CLOSE close;
+       struct s_PXENV_UNDI_ISR undi_isr;
+       struct s_PXENV_UNDI_CLOSE undi_close;
+       int rc;
 
        /* Ensure ISR has exited cleanly */
-       while ( undinic->isr_processing )
-               undinet_poll ( netdev );
+       while ( undinic->isr_processing ) {
+               undi_isr.FuncFlag = PXENV_UNDI_ISR_IN_GET_NEXT;
+               if ( ( rc = undinet_call ( undinic, PXENV_UNDI_ISR, &undi_isr,
+                                          sizeof ( undi_isr ) ) ) != 0 )
+                       break;
+               switch ( undi_isr.FuncFlag ) {
+               case PXENV_UNDI_ISR_OUT_TRANSMIT:
+               case PXENV_UNDI_ISR_OUT_RECEIVE:
+                       /* Continue draining */
+                       break;
+               default:
+                       /* Stop processing */
+                       undinic->isr_processing = 0;
+                       break;
+               }
+       }
 
        /* Close NIC */
-       undinet_call ( undinic, PXENV_UNDI_CLOSE, &close, sizeof ( close ) );
+       undinet_call ( undinic, PXENV_UNDI_CLOSE, &undi_close,
+                      sizeof ( undi_close ) );
 
        /* Disable interrupt and unhook ISR */
        disable_irq ( undinic->irq );
index 2d952aa..091cdcc 100644 (file)
@@ -38,10 +38,13 @@ static int legacy_transmit ( struct net_device *netdev, struct pk_buff *pkb ) {
        return 0;
 }
 
-static void legacy_poll ( struct net_device *netdev ) {
+static void legacy_poll ( struct net_device *netdev, unsigned int rx_quota ) {
        struct nic *nic = netdev->priv;
        struct pk_buff *pkb;
 
+       if ( ! rx_quota )
+               return;
+
        pkb = alloc_pkb ( ETH_FRAME_LEN );
        if ( ! pkb )
                return;
index 09ac92a..c74e7b5 100644 (file)
@@ -112,14 +112,14 @@ static int pnic_api_check ( uint16_t api_version ) {
 /**************************************************************************
 POLL - Wait for a frame
 ***************************************************************************/
-static void pnic_poll ( struct net_device *netdev ) {
+static void pnic_poll ( struct net_device *netdev, unsigned int rx_quota ) {
        struct pnic *pnic = netdev->priv;
        struct pk_buff *pkb;
        uint16_t length;
        uint16_t qlen;
 
        /* Fetch all available packets */
-       while ( 1 ) {
+       while ( rx_quota ) {
                if ( pnic_command ( pnic, PNIC_CMD_RECV_QLEN, NULL, 0,
                                    &qlen, sizeof ( qlen ), NULL )
                     != PNIC_STATUS_OK )
@@ -139,6 +139,7 @@ static void pnic_poll ( struct net_device *netdev ) {
                }
                pkb_put ( pkb, length );
                netdev_rx ( netdev, pkb );
+               --rx_quota;
        }
 }
 
index 4981329..a25755c 100644 (file)
@@ -413,8 +413,9 @@ static int rtl_transmit ( struct net_device *netdev, struct pk_buff *pkb ) {
  * Poll for received packets
  *
  * @v netdev   Network device
+ * @v rx_quota Maximum number of packets to receive
  */
-static void rtl_poll ( struct net_device *netdev ) {
+static void rtl_poll ( struct net_device *netdev, unsigned int rx_quota ) {
        struct rtl8139_nic *rtl = netdev->priv;
        unsigned int status;
        unsigned int tsad;
@@ -441,7 +442,7 @@ static void rtl_poll ( struct net_device *netdev ) {
        }
 
        /* Handle received packets */
-       while ( ! ( inw ( rtl->ioaddr + ChipCmd ) & RxBufEmpty ) ) {
+       while ( rx_quota && ! ( inw ( rtl->ioaddr + ChipCmd ) & RxBufEmpty ) ){
                rx_status = * ( ( uint16_t * )
                                ( rtl->rx.ring + rtl->rx.offset ) );
                rx_len = * ( ( uint16_t * )
index ad1f901..bc08272 100644 (file)
@@ -181,15 +181,16 @@ struct net_device {
        /** Poll for received packet
         *
         * @v netdev    Network device
+        * @v rx_quota  Maximum number of packets to receive
         *
         * This method should cause the hardware to check for received
         * packets.  Any received packets should be delivered via
-        * netdev_rx().
+        * netdev_rx(), up to a maximum of @c rx_quota packets.
         *
         * This method is guaranteed to be called only when the device
         * is open.
         */
-       void ( * poll ) ( struct net_device *netdev );
+       void ( * poll ) ( struct net_device *netdev, unsigned int rx_quota );
 
        /** Link-layer protocol */
        struct ll_protocol *ll_protocol;
@@ -238,7 +239,7 @@ extern int netdev_tx ( struct net_device *netdev, struct pk_buff *pkb );
 void netdev_tx_complete ( struct net_device *netdev, struct pk_buff *pkb );
 void netdev_tx_complete_next ( struct net_device *netdev );
 extern void netdev_rx ( struct net_device *netdev, struct pk_buff *pkb );
-extern int netdev_poll ( struct net_device *netdev );
+extern int netdev_poll ( struct net_device *netdev, unsigned int rx_quota );
 extern struct pk_buff * netdev_rx_dequeue ( struct net_device *netdev );
 extern struct net_device * alloc_netdev ( size_t priv_size );
 extern int register_netdev ( struct net_device *netdev );
index 6da2ddf..40f836d 100644 (file)
@@ -126,16 +126,17 @@ void netdev_rx ( struct net_device *netdev, struct pk_buff *pkb ) {
  * Poll for packet on network device
  *
  * @v netdev           Network device
+ * @v rx_quota         Maximum number of packets to receive
  * @ret True           There are packets present in the receive queue
  * @ret False          There are no packets present in the receive queue
  *
  * Polls the network device for received packets.  Any received
  * packets will be added to the RX packet queue via netdev_rx().
  */
-int netdev_poll ( struct net_device *netdev ) {
+int netdev_poll ( struct net_device *netdev, unsigned int rx_quota ) {
 
        if ( netdev->state & NETDEV_OPEN )
-               netdev->poll ( netdev );
+               netdev->poll ( netdev, rx_quota );
 
        return ( ! list_empty ( &netdev->rx_queue ) );
 }
@@ -351,14 +352,9 @@ int net_rx ( struct pk_buff *pkb, struct net_device *netdev,
  *
  * @v process          Network stack process
  *
- * This polls all interfaces for any received packets, and processes
- * at most one packet from the RX queue.
+ * This polls all interfaces for received packets, and processes
+ * packets from the RX queue.
  *
- * We avoid processing all received packets, because processing the
- * received packet can trigger transmission of a new packet (e.g. an
- * ARP response).  Since TX completions will be processed as part of
- * the poll operation, it is easy to overflow small TX queues if
- * multiple packets are processed per poll.
  */
 static void net_step ( struct process *process ) {
        struct net_device *netdev;
@@ -367,10 +363,28 @@ static void net_step ( struct process *process ) {
        /* Poll and process each network device */
        list_for_each_entry ( netdev, &net_devices, list ) {
 
-               /* Poll for new packets */
-               netdev_poll ( netdev );
-
-               /* Handle at most one received packet per poll */
+               /* Poll for new packets.  Limit RX queue size to a
+                * single packet, because otherwise most drivers are
+                * in serious danger of running out of memory and
+                * having to drop packets.
+                *
+                * This limitation isn't relevant to devices that
+                * preallocate packet buffers (i.e. devices with
+                * descriptor-based RX datapaths).  We might at some
+                * point want to relax the quota for such devices.
+                */
+               netdev_poll ( netdev,
+                             ( list_empty ( &netdev->rx_queue ) ? 1 : 0 ) );
+
+               /* Handle at most one received packet per poll.  We
+                * avoid processing more than one packet per call to
+                * netdev_poll(), because processing the received
+                * packet can trigger transmission of a new packet
+                * (e.g. an ARP response).  Since TX completions will
+                * be processed as part of the poll operation, it is
+                * easy to overflow small TX queues if multiple
+                * packets are processed per poll.
+                */
                if ( ( pkb = netdev_rx_dequeue ( netdev ) ) ) {
                        DBGC ( netdev, "NETDEV %p processing %p\n",
                               netdev, pkb );