From owner-freebsd-net@FreeBSD.ORG Wed Jan 24 13:46:40 2007 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 51AAF16A403 for ; Wed, 24 Jan 2007 13:46:40 +0000 (UTC) (envelope-from rrs@cisco.com) Received: from sj-iport-5.cisco.com (sj-iport-5.cisco.com [171.68.10.87]) by mx1.freebsd.org (Postfix) with ESMTP id 2E71113C465 for ; Wed, 24 Jan 2007 13:46:40 +0000 (UTC) (envelope-from rrs@cisco.com) Received: from sj-dkim-4.cisco.com ([171.71.179.196]) by sj-iport-5.cisco.com with ESMTP; 24 Jan 2007 05:46:39 -0800 Received: from sj-core-2.cisco.com (sj-core-2.cisco.com [171.71.177.254]) by sj-dkim-4.cisco.com (8.12.11/8.12.11) with ESMTP id l0ODkc61014932; Wed, 24 Jan 2007 05:46:38 -0800 Received: from xbh-sjc-211.amer.cisco.com (xbh-sjc-211.cisco.com [171.70.151.144]) by sj-core-2.cisco.com (8.12.10/8.12.6) with ESMTP id l0ODkaGk024347; Wed, 24 Jan 2007 05:46:36 -0800 (PST) Received: from xfe-sjc-212.amer.cisco.com ([171.70.151.187]) by xbh-sjc-211.amer.cisco.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 24 Jan 2007 05:46:36 -0800 Received: from [127.0.0.1] ([171.68.225.134]) by xfe-sjc-212.amer.cisco.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 24 Jan 2007 05:46:36 -0800 Message-ID: <45B7631A.3070001@cisco.com> Date: Wed, 24 Jan 2007 08:46:02 -0500 From: Randall Stewart User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.8) Gecko/20061029 FreeBSD/i386 SeaMonkey/1.0.6 MIME-Version: 1.0 To: Luigi Rizzo References: <45B679F3.3080407@cisco.com> <20070124051050.A56064@xorpc.icir.org> In-Reply-To: <20070124051050.A56064@xorpc.icir.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 24 Jan 2007 13:46:36.0302 (UTC) FILETIME=[10E35AE0:01C73FBE] DKIM-Signature: v=0.5; a=rsa-sha256; q=dns/txt; l=2187; t=1169646398; x=1170510398; c=relaxed/simple; s=sjdkim4002; h=Content-Type:From:Subject:Content-Transfer-Encoding:MIME-Version; d=cisco.com; i=rrs@cisco.com; z=From:=20Randall=20Stewart=20 |Subject:=20Re=3A=20mbuf=20patch=20with=20sysctl=20suggestions=20too |Sender:=20; bh=JmglI4vnlLBiSC8bBUrCbGhFcjG3/jRYvA6w2BLMY4M=; b=WbMAIWPKujbtVHEk7az6r83ujNtIvnYVx8GlIDdKkk9yk+ybMbFfJrr7SxnzxBpBYSorO47F fCJTO/WA7F6Lxw/12TmDkfzf5FD4zJSDgknMtRTZP0MwaT/fHxZsPcY8; Authentication-Results: sj-dkim-4; header.From=rrs@cisco.com; dkim=pass (sig from cisco.com/sjdkim4002 verified; ); Cc: freebsd-net Subject: Re: mbuf patch with sysctl suggestions too X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2007 13:46:40 -0000 Luigi Rizzo wrote: > On Tue, Jan 23, 2007 at 04:11:15PM -0500, Randall Stewart wrote: >> Hi all: >> >> Here is iteration 2 of the mbuf patch with limits I >> proposed. >> >> Also note the changes for sysctl stuff that Lugi suggested. >> Please let me know what you think :-) > > ... >> + newnmbjclusters = nmbjumbop; >> + error = sysctl_int_checked(oidp, &newnmbjclusters, nmbjumbop, >> + SYSCTL_NO_LIMIT, req); > > A few things here: > - i don't see much of a point in defining SYSCTL_NO_LIMIT; > UINT32_MAX would do perfectly there, and i think it is easier > to understand than SYSCTL_NO_LIMIT (which looks like a flag). > ok > - here and in other places you do not allow decresaing the value > (by putting min = nmbjumbop etc.), and i am not sure why. > I understand a reasonable lower bound, but i guess the worst > that can happen, when you reduce the limit to something above > the current allocation, is that nothing is allocated until > you go again below the limit, right ? Well.. no I believe someone (was in Lin) mentioned that you can get a live-lock if you allow a reduction.. and thus the mbuf clusters were NOT allowed to be reduced.. > > - given that you don't use the new value if error != 0, perhaps > you can save the assignment newnmbjclusters = nmbjumbop; > ok > And below: > >> +/* >> + * Handle an int, unsigned, but limited >> + * between min and max (unsigned) >> + * Two cases: >> + * a variable: point arg1 at it. >> + * a constant: pass it in arg2. >> + * >> + */ >> + >> +extern int nmbjumbo9; >> + >> +int >> +sysctl_int_checked(struct sysctl_oid *oidp, void *val, uint32_t min, uint32_t max, struct sysctl_req *req) >> +{ > > the comment refers to something else and should be fixed e.g. > > Handle an unsigned int variable with bound checking. > Returns 0 (and updates *val) if min <= v <= max; > returns EINVAL otherwhise (in which case *val is unchanged) > Cool.. but as I said, Andre wants me to wait on this.. so I will :-) R -- Randall Stewart NSSTG - Cisco Systems Inc. 803-345-0369 803-317-4952 (cell)