From owner-freebsd-current@FreeBSD.ORG  Wed Apr  2 05:14:31 2014
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115])
 (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTPS id E32242B1;
 Wed,  2 Apr 2014 05:14:31 +0000 (UTC)
Received: from ns.kevlo.org (220-135-115-6.HINET-IP.hinet.net [220.135.115.6])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits))
 (Client CN "ns.kevlo.org", Issuer "ns.kevlo.org" (not verified))
 by mx1.freebsd.org (Postfix) with ESMTPS id 84618923;
 Wed,  2 Apr 2014 05:14:31 +0000 (UTC)
Received: from srg.kevlo.org (220-135-115-6.HINET-IP.hinet.net [220.135.115.6])
 by ns.kevlo.org (8.14.8/8.14.8) with ESMTP id s325Dt4i042936
 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO);
 Wed, 2 Apr 2014 13:13:56 +0800 (CST)
 (envelope-from kevlo@FreeBSD.org)
Message-ID: <533B9C9C.3050504@FreeBSD.org>
Date: Wed, 02 Apr 2014 13:14:04 +0800
From: Kevin Lo <kevlo@FreeBSD.org>
User-Agent: Mozilla/5.0 (X11; FreeBSD amd64;
 rv:24.0) Gecko/20100101 Thunderbird/24.4.0
MIME-Version: 1.0
To: Adrian Chadd <adrian@freebsd.org>
Subject: Re: UDP Lite support
References: <CA+WntOugdnjM1SvFcEtWb8=heQCOBs2-c97EUKsfEHSmi1HRSw@mail.gmail.com>	<201403261122.43541.jhb@freebsd.org>	<5333F020.8000200@FreeBSD.org>	<201403271221.29864.jhb@freebsd.org>	<533A2285.2030705@FreeBSD.org>
 <CAJ-VmondoAHF2YJtS_Zt11aP00oc91=AO02qdOHDABoJM7PAEw@mail.gmail.com>
In-Reply-To: <CAJ-VmondoAHF2YJtS_Zt11aP00oc91=AO02qdOHDABoJM7PAEw@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Joe Nosay <superbisquit@gmail.com>, Brooks Davis <brooks@freebsd.org>,
 freebsd-current <freebsd-current@freebsd.org>, Xin LI <d@delphij.net>
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
 <freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-current>, 
 <mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current/>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
 <mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 02 Apr 2014 05:14:32 -0000

On 2014/04/02 04:53, Adrian Chadd wrote:
> Hi!

Hi Adrian,

>
> On 31 March 2014 19:20, Kevin Lo <kevlo@freebsd.org> wrote:
>
>
>> Thank you John.  glebius@ suggests we don't need to have two absolutely
>> equal uma zones since most systems don't run UDP-Lite.
>> If practice shows that a differentiation at zone level between UDP and
>> UDP-Lite PCBs is important, then it could be done later.
>>
>> Following up with a fourth version of the udp-lite patch.
>> http://people.freebsd.org/~kevlo/udplite.diff
>>
>> On top of the previous versions, this:
>>          - removes a uma zone for udp-lite
>>          - udp_common_ctlinput() belongs under #ifdef INET
>>          - removes sysctl nodes for udp-lite.
>>          - bumps version and adds my copyright.
> I've just briefly review this.
>
> I recommend turning the places where you do this:
>
> + pcbinfo = (pr == IPPROTO_UDP) ? &V_udbinfo : &V_ulitecbinfo;
>
> .. into some inline function which returns the correct pcbinfo based
> on what 'pr' is.
>
> That way if someone wants to add another derivative UDP handler they
> won't have to go and change those conditionals to yet another set of
> nested conditionals.
>
> Same for:
>
> + pcblist = (pr == IPPROTO_UDP) ? &V_udb : &V_ulitecb;
>
> Other than that, it looks good.

Thanks for the review.  I added two inline functions get_inpcbinfo() and
get_pcblist() which return the correct pcbinfo and pcblist respectively.

The current version of the patch is in the same location, thanks.

>
>
> -a

     Kevin