DPDK patches and discussions
 help / color / mirror / Atom feed
From: Matthew Hall <mhall@mhcomputing.net>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: dev@dpdk.org, Nikita Kozlov <nikita@gandi.net>
Subject: Re: [dpdk-dev] LPM6 next hop size
Date: Wed, 21 Sep 2016 10:29:05 -0700	[thread overview]
Message-ID: <20160921172905.GA7158@mhcomputing.net> (raw)
In-Reply-To: <3263960.cPWMKkvuZx@xps13>

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.

  reply	other threads:[~2016-09-21 17:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19 20:04 Shyam Sundar Govindaraj
2016-09-19 21:18 ` Nikita Kozlov
2016-09-19 21:22   ` Matthew Hall
2016-09-20 20:11     ` Thomas Monjalon
2016-09-21 17:29       ` Matthew Hall [this message]
2016-09-21 19:37         ` Thomas Monjalon
2016-09-21 23:42         ` Stephen Hemminger
2016-09-22  1:50           ` Matthew Hall
2016-09-22  5:07             ` Stephen Hemminger
2016-09-22 17:55               ` Matthew Hall
2016-09-22 18:09                 ` Stephen Hemminger
2016-09-22 20:32         ` Nikita Kozlov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160921172905.GA7158@mhcomputing.net \
    --to=mhall@mhcomputing.net \
    --cc=dev@dpdk.org \
    --cc=nikita@gandi.net \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).