From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v2 2/2] lib/lpm:fix an initialization issue of valid_group in the delete_depth_small() Date: Fri, 30 Oct 2015 15:31:19 +0100 Message-ID: <2760784.4fBWi2jOBS@xps13> References: <1446210879-14242-1-git-send-email-jijiang.liu@intel.com> <20151030142227.GB10520@bricha3-MOBL3> <20151030142425.GC10520@bricha3-MOBL3> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: Bruce Richardson Return-path: Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id 0739E919D for ; Fri, 30 Oct 2015 15:32:31 +0100 (CET) Received: by wmeg8 with SMTP id g8so13386897wme.1 for ; Fri, 30 Oct 2015 07:32:30 -0700 (PDT) In-Reply-To: <20151030142425.GC10520@bricha3-MOBL3> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2015-10-30 14:24, Bruce Richardson: > On Fri, Oct 30, 2015 at 02:22:27PM +0000, Bruce Richardson wrote: > > On Fri, Oct 30, 2015 at 09:14:39PM +0800, Jijiang Liu wrote: > > > > Title can be shortened to: "lpm: fix initialization of valid_group field" > > > > > Fixes an initialization issue of 'valid_group' in the delete_depth_small function. > > > > > > In this function, use new rte_lpm_tbl8_entry we call A to replace the old rte_lpm_tbl8_entry. But the valid_group do not set VALID, so it > > > will be INVALID. > > > > > > Then when adding a new route which depth is > 24,the tbl8_alloc() function will search the rte_lpm_tbl8_entrys to find INVALID > > > valid_group, and it will return the A to the add_depth_big function, so A's data is overridden. > > > > > > > Not sure this message is entirely clear. > > How about: > > When adding an entry to a tbl8, the .valid_group field should always be set, > > so that future adds do not accidently find and use this table, thinking it is > > currently invalid, i.e. unused, and thereby overwrite existing entries. > > > > > Signed-off-by: NaNa > > > > Assuming we get a little cleanup on commit title and log message (Thomas, perhaps > just a rewrite on commit?): Giving the name of a field in the title is not really useful for the overview. It's better to talk about the use case which is fixed.