From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <mhall@mhcomputing.net>
Received: from mail.mhcomputing.net (master.mhcomputing.net [74.208.228.170])
 by dpdk.org (Postfix) with ESMTP id D76D4558C
 for <dev@dpdk.org>; Wed, 21 Sep 2016 19:29:05 +0200 (CEST)
Received: by mail.mhcomputing.net (Postfix, from userid 1000)
 id 61EC3BE; Wed, 21 Sep 2016 10:29:05 -0700 (PDT)
Date: Wed, 21 Sep 2016 10:29:05 -0700
From: Matthew Hall <mhall@mhcomputing.net>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: dev@dpdk.org, Nikita Kozlov <nikita@gandi.net>
Message-ID: <20160921172905.GA7158@mhcomputing.net>
References: <d28c22febc564e45a4c2f5b4066223bc@Hq1wp-exmb11.corp.brocade.com>
 <c57b485f-ff12-7b7c-9ad8-8e9131647069@gandi.net>
 <20160919212257.GA27713@mhcomputing.net> <3263960.cPWMKkvuZx@xps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3263960.cPWMKkvuZx@xps13>
User-Agent: Mutt/1.5.21 (2010-09-15)
Subject: Re: [dpdk-dev] LPM6 next hop size
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 21 Sep 2016 17:29:06 -0000

On Tue, Sep 20, 2016 at 10:11:04PM +0200, Thomas Monjalon wrote:
> Please, will you help reviewing this patch?

Sure.

1. It adds a dependency on libbsd on Linux: bsd/sys/tree.h. Is this an 
expected dependency of DPDK already? I use it in my code but not sure it's 
expected for everybody else.

2. Some of the tabbing of the new code seems not right. Probably need to 
recheck the indentation settings.

3. The comment formatting of the new code is a bit odd. Continued lines in /* 
... */ in DPDK should have * on them I think.

4. It also contains some vendor specific comments like "/* Vyatta change to 
use red-black tree */". That's probably not needed if it's going into normal 
DPDK.

5. It uses "malloc" instead of standard DPDK allocators. That's bad for me 
because I don't want to use libc malloc in my code. Only DPDK allocators and 
jemalloc.

6. I don't see any updates to the unit test stuff. Don't we need new tests for 
the changes to deletion and re-insertion of rules and the changes in the tbl8 
allocation.

7. Some of us previous submitted code to expand LPM4 and LPM6 to 24 bit next 
hop but the current code is only doing 16 bit. This is not pleasant for me 
because I really need the 24-bit support. When can we make this happen?

Matthew.