From owner-freebsd-net@FreeBSD.ORG Thu Dec 5 21:05:21 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id F1DB2297; Thu, 5 Dec 2013 21:05:20 +0000 (UTC) Received: from mail-n.franken.de (drew.ipv6.franken.de [IPv6:2001:638:a02:a001:20e:cff:fe4a:feaa]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 83B6C1C7A; Thu, 5 Dec 2013 21:05:20 +0000 (UTC) Received: from [192.168.1.102] (p508F016D.dip0.t-ipconnect.de [80.143.1.109]) (Authenticated sender: macmic) by mail-n.franken.de (Postfix) with ESMTP id 778861C0C0695; Thu, 5 Dec 2013 22:05:18 +0100 (CET) Content-Type: text/plain; charset=iso-8859-1 Mime-Version: 1.0 (Mac OS X Mail 6.6 \(1510\)) Subject: Re: A small fix for if_em.c, if_igb.c, if_ixgbe.c From: Michael Tuexen In-Reply-To: Date: Thu, 5 Dec 2013 22:05:16 +0100 Content-Transfer-Encoding: 7bit Message-Id: References: <521B9C2A-EECC-4412-9F68-2235320EF324@lurchi.franken.de> <20131202022338.GA3500@michelle.cdnetworks.com> <20131203021658.GC2981@michelle.cdnetworks.com> To: Adrian Chadd X-Mailer: Apple Mail (2.1510) Cc: Yong-Hyeon Pyun , Jack F Vogel , "freebsd-net@freebsd.org list" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Dec 2013 21:05:21 -0000 On Dec 5, 2013, at 7:29 PM, Adrian Chadd wrote: > Hi, > > Yes. Looking at the ixgbe code, ixgbe_mq_start_locked() returns an > error from ixgbe_xmit() but if it fails, it puts the buffer back. But > it's already successfully queued a frame to the driver, so in this > instance it shouldn't return the error from ixgbe_mq_start_locked(). > > The same deal in if_em.c and igb.c > > Now, drbr_putback() used to fail and now it doesn't, as you've said. > So we should change the xxx_mq_start_locked() to set err=0 if we go > via the drbr_putback() routine, as it hasn't actually failed to > transmit. > > Now the very dirty thing is this - the error from xxx_transmit() is > for the mbuf being queued at the end; but xxx_mq_start_locked() > failures are for transmitting from the front. If there's only packet > in the queue and that fails then they're the same thing and returning > the error from xxx_mq_start_locked() matches the current mbuf being > queued. But otherwise, they're referring to totally different packets. > For TCP this may hurt; the TCP stack treats ENOBUFS a certain way and > kicks off a timer to schedule a retransmit. I don't think we can fix > _this_ right now. Just to be clear: This would mean that xxx_transmit() would return an error even if the packet provided in the call xxx_transmit() is enqueued and not dropped? This would also be problem with the current SCTP stack. Best regards Michael > > So Michael - can you redo your patch to set err=0 if drbr_putback() is > called, and retest? > > Thanks! > > > > > -adrian >