kern/176446: [netinet] [patch] Concurrency in ixgbe driving out-of-order packet process and spurious RST

[John Baldwin]

The following reply was made to PR kern/176446; it has been noted by GNATS.

From: John Baldwin <jhb at freebsd.org>
To: freebsd-net at freebsd.org
Cc: Jack Vogel <jfvogel at gmail.com>,
 bug-followup at freebsd.org
Subject: Re: kern/176446: [netinet] [patch] Concurrency in ixgbe driving out-of-order packet process and spurious RST
Date: Fri, 19 Apr 2013 12:09:11 -0400

 I want to make some progress on this, so let's break this up into smaller 
 parts.
 
 First, I think both calls to rearm_queues() should be removed.  In the case of 
 the local timer, this can only re-enable interrupts if the interrupt handler 
 is already scheduled or running or its associated task is running.  In the 
 last case this means the ithread can run concurrently with the interrupt 
 handler causing out-of-order processing.  The rxeof case has the same issue.  
 Normally the code calling rxeof is going to re-enable the interrupt if rxeof 
 runs to completion, and if not it is going to schedule the taskqueue.  The 
 effect of the rxeof change was to always re-enable interrupts before 
 scheduling the taskqueue which can result in those running concurrently.
 
 Index: /home/jhb/work/freebsd/svn/head/sys/dev/ixgbe/ixgbe.c
 ===================================================================
 --- /home/jhb/work/freebsd/svn/head/sys/dev/ixgbe/ixgbe.c	(revision 249553)
 +++ /home/jhb/work/freebsd/svn/head/sys/dev/ixgbe/ixgbe.c	(working copy)
 @@ -1386,23 +1386,6 @@
  	}
  }
  
 -static inline void
 -ixgbe_rearm_queues(struct adapter *adapter, u64 queues)
 -{
 -	u32 mask;
 -
 -	if (adapter->hw.mac.type == ixgbe_mac_82598EB) {
 -		mask = (IXGBE_EIMS_RTX_QUEUE & queues);
 -		IXGBE_WRITE_REG(&adapter->hw, IXGBE_EICS, mask);
 -	} else {
 -		mask = (queues & 0xFFFFFFFF);
 -		IXGBE_WRITE_REG(&adapter->hw, IXGBE_EICS_EX(0), mask);
 -		mask = (queues >> 32);
 -		IXGBE_WRITE_REG(&adapter->hw, IXGBE_EICS_EX(1), mask);
 -	}
 -}
 -
 -
  static void
  ixgbe_handle_que(void *context, int pending)
  {
 @@ -2069,7 +2055,6 @@
                  goto watchdog;
  
  out:
 -	ixgbe_rearm_queues(adapter, adapter->que_mask);
  	callout_reset(&adapter->timer, hz, ixgbe_local_timer, adapter);
  	return;
  
 @@ -4596,14 +4577,8 @@
  
  	/*
  	** We still have cleaning to do?
 -	** Schedule another interrupt if so.
  	*/
 -	if ((staterr & IXGBE_RXD_STAT_DD) != 0) {
 -		ixgbe_rearm_queues(adapter, (u64)(1 << que->msix));
 -		return (TRUE);
 -	}
 -
 -	return (FALSE);
 +	return ((staterr & IXGBE_RXD_STAT_DD) != 0);
  }
  
  
 -- 
 John Baldwin