From owner-freebsd-net@FreeBSD.ORG Fri Apr 19 17:50:04 2013 Return-Path: Delivered-To: freebsd-net@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 279E1BC6 for ; Fri, 19 Apr 2013 17:50:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) by mx1.freebsd.org (Postfix) with ESMTP id F267F918 for ; Fri, 19 Apr 2013 17:50:03 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.6/8.14.6) with ESMTP id r3JHo3aP083304 for ; Fri, 19 Apr 2013 17:50:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.6/8.14.6/Submit) id r3JHo3Ur083303; Fri, 19 Apr 2013 17:50:03 GMT (envelope-from gnats) Date: Fri, 19 Apr 2013 17:50:03 GMT Message-Id: <201304191750.r3JHo3Ur083303@freefall.freebsd.org> To: freebsd-net@FreeBSD.org Cc: From: John Baldwin Subject: Re: kern/176446: [netinet] [patch] Concurrency in ixgbe driving out-of-order packet process and spurious RST X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: John Baldwin List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Apr 2013 17:50:04 -0000 The following reply was made to PR kern/176446; it has been noted by GNATS. From: John Baldwin To: freebsd-net@freebsd.org Cc: Jack Vogel , bug-followup@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