From owner-freebsd-net@FreeBSD.ORG Fri Dec 4 19:52:56 2009 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 207A3106566B for ; Fri, 4 Dec 2009 19:52:56 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from mail-vw0-f173.google.com (mail-vw0-f173.google.com [209.85.212.173]) by mx1.freebsd.org (Postfix) with ESMTP id B953A8FC08 for ; Fri, 4 Dec 2009 19:52:55 +0000 (UTC) Received: by vws3 with SMTP id 3so1306012vws.3 for ; Fri, 04 Dec 2009 11:52:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:from:date:to:cc :subject:message-id:reply-to:references:mime-version:content-type :content-disposition:in-reply-to:user-agent; bh=E0nlUzWZLOje//vBu31XD4FmCWq6oWg0mBdzGHvrjRc=; b=vnkuwmwrtJMZRBMrd4migFU65S08Zjz3wp6oEoZwD6LsuiR8iNvtDz4ZeadJdqwQ6g gWWxFj+l4jM//KjLpnWFgRYGHQwMPpQclWeQvk4mYelHDDskw4jG+vOhJdRzW9jKDewP q5KzazZ4ucRXca7WqUXH+hyO8ZThQKe83OItM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:date:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=isUVmlEQEkaJ/Iy46Y7d7aSYt1iDGK5gkSzviwG4X/VARuk2B8OENGkOBZ6laP2Po1 0MpQo646bsMZxLE15hNDU45rS29KLxHzpZ/QWoFJtwuV5D4NXQAAQIW/UNK3VTKkuxeY nIFRIWrKt5fENfRbeiMT8frVI5HLgbAtw51GE= Received: by 10.220.126.144 with SMTP id c16mr4647815vcs.43.1259956374917; Fri, 04 Dec 2009 11:52:54 -0800 (PST) Received: from pyunyh@gmail.com ([174.35.1.224]) by mx.google.com with ESMTPS id 20sm6947841vws.8.2009.12.04.11.52.52 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 04 Dec 2009 11:52:53 -0800 (PST) Received: by pyunyh@gmail.com (sSMTP sendmail emulation); Fri, 4 Dec 2009 11:51:40 -0800 From: Pyun YongHyeon Date: Fri, 4 Dec 2009 11:51:40 -0800 To: Igor Sysoev Message-ID: <20091204195140.GH16491@michelle.cdnetworks.com> References: <20091204075440.GH14822@rambler-co.ru> <20091204173243.GC16491@michelle.cdnetworks.com> <20091204191114.GB76992@rambler-co.ru> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="jq0ap7NbKX2Kqbes" Content-Disposition: inline In-Reply-To: <20091204191114.GB76992@rambler-co.ru> User-Agent: Mutt/1.4.2.3i Cc: freebsd-net@freebsd.org Subject: Re: hw.bge.forced_collapse X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: pyunyh@gmail.com List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Dec 2009 19:52:56 -0000 --jq0ap7NbKX2Kqbes Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Dec 04, 2009 at 10:11:14PM +0300, Igor Sysoev wrote: > On Fri, Dec 04, 2009 at 09:32:43AM -0800, Pyun YongHyeon wrote: > > > On Fri, Dec 04, 2009 at 10:54:40AM +0300, Igor Sysoev wrote: > > > I saw commit introducing hw.bge.forced_collapse loader tunable. > > > Just intresting, why it can not be a sysctl ? > > > > I didn't think the sysctl variable would be frequently changed > > in runtime except debugging driver so I took simple path. > > I do not think it's worth to reboot server just to look how various > values affect on bandwidth and CPU usage, expecially in production. > > As I understand the change is trivial: > > - CTLFLAG_RD > + CTLFLAG_RW > > since bge_forced_collapse is used atomically. > I have no problem changing it to RW but that case I may have to create actual sysctl node(e.g. dev.bge.0.forced_collapse) instead of hw.bge.forced_collapse which may affect all bge(4) controllers on system. Attached patch may be what you want. You can change the value at any time. --jq0ap7NbKX2Kqbes Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="bge.collapse.diff" Index: sys/dev/bge/if_bgereg.h =================================================================== --- sys/dev/bge/if_bgereg.h (revision 200104) +++ sys/dev/bge/if_bgereg.h (working copy) @@ -2647,6 +2647,7 @@ int bge_link; /* link state */ int bge_link_evt; /* pending link event */ int bge_timer; + int bge_forced_collapse; struct callout bge_stat_ch; uint32_t bge_rx_discards; uint32_t bge_tx_discards; Index: sys/dev/bge/if_bge.c =================================================================== --- sys/dev/bge/if_bge.c (revision 200104) +++ sys/dev/bge/if_bge.c (working copy) @@ -449,6 +449,7 @@ #endif static void bge_add_sysctls(struct bge_softc *); static int bge_sysctl_stats(SYSCTL_HANDLER_ARGS); +static int bge_sysctl_forced_collapse(SYSCTL_HANDLER_ARGS); static device_method_t bge_methods[] = { /* Device interface */ @@ -483,29 +484,12 @@ DRIVER_MODULE(miibus, bge, miibus_driver, miibus_devclass, 0, 0); static int bge_allow_asf = 1; -/* - * A common design characteristic for many Broadcom client controllers - * is that they only support a single outstanding DMA read operation - * on the PCIe bus. This means that it will take twice as long to fetch - * a TX frame that is split into header and payload buffers as it does - * to fetch a single, contiguous TX frame (2 reads vs. 1 read). For - * these controllers, coalescing buffers to reduce the number of memory - * reads is effective way to get maximum performance(about 940Mbps). - * Without collapsing TX buffers the maximum TCP bulk transfer - * performance is about 850Mbps. However forcing coalescing mbufs - * consumes a lot of CPU cycles, so leave it off by default. - */ -static int bge_forced_collapse = 0; TUNABLE_INT("hw.bge.allow_asf", &bge_allow_asf); -TUNABLE_INT("hw.bge.forced_collapse", &bge_forced_collapse); SYSCTL_NODE(_hw, OID_AUTO, bge, CTLFLAG_RD, 0, "BGE driver parameters"); SYSCTL_INT(_hw_bge, OID_AUTO, allow_asf, CTLFLAG_RD, &bge_allow_asf, 0, "Allow ASF mode if available"); -SYSCTL_INT(_hw_bge, OID_AUTO, forced_collapse, CTLFLAG_RD, &bge_forced_collapse, - 0, "Number of fragmented TX buffers of a frame allowed before " - "forced collapsing"); #define SPARC64_BLADE_1500_MODEL "SUNW,Sun-Blade-1500" #define SPARC64_BLADE_1500_PATH_BGE "/pci@1f,700000/network@2" @@ -3933,17 +3917,17 @@ } if ((m->m_pkthdr.csum_flags & CSUM_TSO) == 0 && - bge_forced_collapse > 0 && (sc->bge_flags & BGE_FLAG_PCIE) != 0 && - m->m_next != NULL) { + sc->bge_forced_collapse > 0 && + (sc->bge_flags & BGE_FLAG_PCIE) != 0 && m->m_next != NULL) { /* * Forcedly collapse mbuf chains to overcome hardware * limitation which only support a single outstanding * DMA read operation. */ - if (bge_forced_collapse == 1) + if (sc->bge_forced_collapse == 1) m = m_defrag(m, M_DONTWAIT); else - m = m_collapse(m, M_DONTWAIT, bge_forced_collapse); + m = m_collapse(m, M_DONTWAIT, sc->bge_forced_collapse); if (m == NULL) { m_freem(*m_head); *m_head = NULL; @@ -4879,6 +4863,7 @@ struct sysctl_ctx_list *ctx; struct sysctl_oid_list *children, *schildren; struct sysctl_oid *tree; + int error; ctx = device_get_sysctl_ctx(sc->bge_dev); children = SYSCTL_CHILDREN(device_get_sysctl_tree(sc->bge_dev)); @@ -4898,6 +4883,32 @@ #endif + /* + * A common design characteristic for many Broadcom client controllers + * is that they only support a single outstanding DMA read operation + * on the PCIe bus. This means that it will take twice as long to fetch + * a TX frame that is split into header and payload buffers as it does + * to fetch a single, contiguous TX frame (2 reads vs. 1 read). For + * these controllers, coalescing buffers to reduce the number of memory + * reads is effective way to get maximum performance(about 940Mbps). + * Without collapsing TX buffers the maximum TCP bulk transfer + * performance is about 850Mbps. However forcing coalescing mbufs + * consumes a lot of CPU cycles, so leave it off by default. + */ + SYSCTL_ADD_PROC(ctx, children, OID_AUTO, "forced_collapse", + CTLTYPE_INT | CTLFLAG_RW, &sc->bge_forced_collapse, 0, + bge_sysctl_forced_collapse, "I", "Number of fragmented TX " + "buffers of a frame allowed before forced collapsing"); + sc->bge_forced_collapse = 0; + error = resource_int_value(device_get_name(sc->bge_dev), + device_get_unit(sc->bge_dev), "forced_collapse", + &sc->bge_forced_collapse); + if (error == 0) { + if (sc->bge_forced_collapse < 0 || + sc->bge_forced_collapse > BGE_NSEG_NEW); + sc->bge_forced_collapse = 0; + } + if (BGE_IS_5705_PLUS(sc)) return; @@ -5143,6 +5154,23 @@ #endif static int +bge_sysctl_forced_collapse(SYSCTL_HANDLER_ARGS) +{ + int error, value; + + if (arg1 == NULL) + return (EINVAL); + value = *(int *)arg1; + error = sysctl_handle_int(oidp, &value, 0, req); + if (error || req->newptr == NULL) + return (error); + if (value < 0 || value > BGE_NSEG_NEW) + return (EINVAL); + *(int *)arg1 = value; + return (0); +} + +static int bge_get_eaddr_fw(struct bge_softc *sc, uint8_t ether_addr[]) { --jq0ap7NbKX2Kqbes--