From owner-freebsd-net@FreeBSD.ORG Fri Jun 14 09:51:26 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id F16CF186; Fri, 14 Jun 2013 09:51:26 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) by mx1.freebsd.org (Postfix) with ESMTP id 657DF12BE; Fri, 14 Jun 2013 09:51:26 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.7/8.14.7) with ESMTP id r5E9pPXE028187; Fri, 14 Jun 2013 13:51:25 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.7/8.14.7/Submit) id r5E9pPCm028186; Fri, 14 Jun 2013 13:51:25 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Fri, 14 Jun 2013 13:51:25 +0400 From: Gleb Smirnoff To: Ermal Lu?i Subject: Re: [PATH] ALTQ(9) codel algorithm implementation Message-ID: <20130614095125.GQ12443@FreeBSD.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-net X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Jun 2013 09:51:27 -0000 Ermal, On Mon, Jun 10, 2013 at 03:43:12PM +0200, Ermal Lu?i wrote: E> at location [1] can be found a patch for Codel[3] algorithm implementation. E> E> Triggered by a mail to the mailing lists[2] of OpenBSD i completed the E> implementation for FreeBSD. E> E> It allows to use codel as the single configured discipline on an interface. E> Also it can be used as a sub discipline of existing queueing disciplines E> already present. E> E> The work has been tested and confirmed working without issues in pfSense. E> E> Any objections on pushing this into FreeBSD? E> E> [1] E> https://github.com/pfsense/pfsense-tools/blob/master/patches/RELENG_10_0/altq_codel.diff E> [2] http://permalink.gmane.org/gmane.os.openbsd.tech/29745 E> [3] http://www.bufferbloat.net/projects/codel/wiki I'm afraid we can't grow mbuf packet header with 8 bytes just to satisfy the ALTQ codel algo, which would definitely have a limited usage among FreeBSD users. Thus, "enqueue_time" should go into mbuf_tags(9) not into mbuf packet header. Smaller problems noticed: - codel_alloc() doesn't check return from malloc(). And fairq_class_create(), priq_class_create(), rmc_newclass() and hfsc_class_create() don't check return from codel_alloc(). - Need newline before function name in codel_Newton_step(). - Spaces instead of tab in declaration of struct codel_ifstats and struct codel_opts. - Diff chunk to fairq_add_altq() is okay, but is entirely unrelated to overall patch. Should be committed separately and old MALLOC() macro should be removed. Assuming all above is taken into account, IMHO, the patch is okay to be included into FreeBSD. Thanks! Some grumbling: the altq_codel.c is polluted with spl(9) and has a lot of ifdef __NetBSD__, uses legacy u_intXX_t types instead of C99, uses strange macro m_pktlen() and so on. Same thing as the rest of ALTQ code. If someone can mechanically polish altq, that would be nice. -- Totus tuus, Glebius.