From owner-freebsd-net@FreeBSD.ORG Thu Dec 5 18:29:52 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 1709778F; Thu, 5 Dec 2013 18:29:52 +0000 (UTC) Received: from mail-qe0-x232.google.com (mail-qe0-x232.google.com [IPv6:2607:f8b0:400d:c02::232]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id B69FB11D2; Thu, 5 Dec 2013 18:29:51 +0000 (UTC) Received: by mail-qe0-f50.google.com with SMTP id 1so15503204qec.23 for ; Thu, 05 Dec 2013 10:29:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=G0HpOnWOZgQvdeO5/FtHLsKbi5gBRJlunrlZSQE22TM=; b=bzUxqf/p7idy6hQlp6lmK6lcJ7yE7OI8fH2oSe1XGsowXlAApXqZlHnpvdD+r1l7aQ SzKWgdUgoUuGxlea7QluVlCo5ySlCy4GHzZfS61pXJ+ksXrYlVB/igAKtc9HzRUNewYg wRkJGO6y5yXApYCnGEx0JYRuoxWXkSqh3qa9rkVgffmRzcdOUphtadGS6ZRrrX9KUM/r 0fDJ32Z84Lc4WJEuOzDejnpWLe604pp3dfHcuR+bTmiyjATgd0oq8VxHHVjHT3v3EvvG /zkeyAlzTmRYDPD72f7qiqTbxiaDNKcZZtpyf0sX6XAx+qx+NMPdo5iHVxGyA5l+/hQv OyXQ== MIME-Version: 1.0 X-Received: by 10.49.24.163 with SMTP id v3mr87399765qef.78.1386268190994; Thu, 05 Dec 2013 10:29:50 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.224.53.200 with HTTP; Thu, 5 Dec 2013 10:29:50 -0800 (PST) In-Reply-To: <20131203021658.GC2981@michelle.cdnetworks.com> References: <521B9C2A-EECC-4412-9F68-2235320EF324@lurchi.franken.de> <20131202022338.GA3500@michelle.cdnetworks.com> <20131203021658.GC2981@michelle.cdnetworks.com> Date: Thu, 5 Dec 2013 10:29:50 -0800 X-Google-Sender-Auth: GgzO7v3Q3v1O6TEyiMPh6f7avAU Message-ID: Subject: Re: A small fix for if_em.c, if_igb.c, if_ixgbe.c From: Adrian Chadd To: Yong-Hyeon Pyun Content-Type: text/plain; charset=ISO-8859-1 Cc: Jack F Vogel , Michael Tuexen , "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 18:29:52 -0000 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. So Michael - can you redo your patch to set err=0 if drbr_putback() is called, and retest? Thanks! -adrian