From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EFCACA034E; Thu, 24 Feb 2022 14:58:42 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7FE9041156; Thu, 24 Feb 2022 14:58:42 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mails.dpdk.org (Postfix) with ESMTP id 9088D4114D for ; Thu, 24 Feb 2022 14:58:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1645711120; x=1677247120; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=gZQCrSovSSPQPWIIulbxWLC8uEFdVHugBjuhVAZf0pc=; b=XjBkociAmUccZ0jydUFx+abt1WdKQyNQZ3JDkFFtrqvjPl0JvOQFGYFE 63cQXV9zaJcdojfZ8Nf5WEQ+VdMplIzmR80rVDOQ9bBfxiXy7ZiN6SMt7 5iLZNpyIbls2uoUQ+F5Hd9AB4au+2Tiw6glydlmXRVjQyG5bP3p6pgDUY jPWTfdOO76avZgBl8xa57aYzm7MqOliUMjc57+LC6paOKgAs031PhvWhb aak/6ipnp+aToABG9LDcxSWOaeuBoFK7sYNlcutXc6SrtHJcMiOGoKH8/ 6j88nYQ9MxpjwZ3/RjrU+8kNf6ghgYNUNiq1RVDgZ5oWpikwuf/9GgWH2 g==; X-IronPort-AV: E=McAfee;i="6200,9189,10267"; a="338674667" X-IronPort-AV: E=Sophos;i="5.90,134,1643702400"; d="scan'208";a="338674667" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Feb 2022 05:58:39 -0800 X-IronPort-AV: E=Sophos;i="5.90,134,1643702400"; d="scan'208";a="508877448" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.15.61]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 24 Feb 2022 05:58:36 -0800 Date: Thu, 24 Feb 2022 13:58:33 +0000 From: Bruce Richardson To: Thomas Monjalon Cc: Stephen Hemminger , "Medvedkin, Vladimir" , "dev@dpdk.org" , "Morrissey, Sean" , "Ananyev, Konstantin" , honnappa.nagarahalli@arm.com, jerinj@marvell.com, ajit.khaparde@broadcom.com, olivier.matz@6wind.com, maxime.coquelin@redhat.com, david.marchand@redhat.com, ktraynor@redhat.com Subject: Re: [PATCH v5 0/2] Add config file support for l3fwd Message-ID: References: <20220126124459.2469838-1-sean.morrissey@intel.com> <2262420.IPqQCg1nHW@thomas> <2122832.NgBsaNRSFp@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2122832.NgBsaNRSFp@thomas> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Feb 24, 2022 at 02:46:24PM +0100, Thomas Monjalon wrote: > 24/02/2022 12:06, Ananyev, Konstantin: > > > > > > > > > > >> Or have a generic library for reading LPM entries. L3fwd is supposed > > > > > > > > > > >> to be as small as possible (it no longer is), and the real work should > > > > > > > > > > >> be done by libraries to make it easier to build other applications. > > > > > > > > > > > > > > > > > > > > > > I never heard users ask about such thing, > > > > > > > > > > > but if there is a demand for that, then I suppose it could be considered. > > > > > > > > > > > CC-ing LPM/FIB maintainers to comment. > > > > > > > > > > > Though I believe it should be a subject of separate patch and discussion > > > > > > > > > > > (I think many questions will arise - what format should be, how to support > > > > > > > > > > > different types of user-data, to make it generic enough, etc.). > > > > > > > > > > > > > > > > > > > > Agree, it is very application specific, so it could be really difficult > > > > > > > > > > to make it generic. > > > > > > > > > > > > > > > > > > But several other also have LPM tables, so why not have common code for other applications. > > > > > > > > > > > > > > > > > > examples/l3fwd-power/main.c > > > > > > > > > examples/ipsec-secgw/rt.c > > > > > > > > > examples/ip_fragmentation/main.c > > > > > > > > > examples/l3fwd/l3fwd_lpm.c > > > > > > > > > examples/ip_reassembly/main.c > > > > > > > > > > > > > > > > Ah yes, that's good point. > > > > > > > > All these examples (except ipsec-secgw) started as l3fwd clones, > > > > > > > > so all of them have hard-coded LPM (and EM) tables too. > > > > > > > > Yes it would be good thing to address that problem too, > > > > > > > > and have some common code (and common routes file format) for all of them. > > > > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library > > > > > > > > itself, might be better to have something like examples/common/lpm_parse*. > > > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe. > > > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches, > > > > > > > > then later we can consider to make it common and update other examples. > > > > > > > > > > > > > > I don't think this patch is urgent. > > > > > > > I suggest taking time to have common code for all examples > > > > > > > and target a merge in DPDK 22.07. > > > > > > > > > > > > Well, yes, from one perspective it not really a critical one, > > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now. > > > > > > Though l3fwd is one of mostly used examples inside DPDK and > > > > > > it is quite a pain to patch/rebuild it each time someone needs to run > > > > > > l3fwd with a different routing table. > > > > > > Merging this patch will allow people to use l3fwd for more realistic test > > > > > > scenarios in a painless manner. > > > > > > So I believe this patch is really helpful and should be beneficial for the whole community. > > > > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here. > > > > > > Why we can't move one step at a time instead? > > > > > > That would allow to split and effort in terms of development/testing/upstreaming/etc. > > > > > > > > > > When a feature is merged, there is less incentives to rework. > > > > > That's why, when a feature is not urgent, > > > > > it is better to wait for the complete work. > > > > > > > > That's true till some extent, though from other side > > > > even without further rework that patch improves situation > > > > from what we have right now. > > > > So I don't see any harm here. > > > > > > It is adding a lot of code to an example which is already too big. > > > There are a lot of complain about the size of l3fwd. > > > That's why I think it makes sense to require this extra code > > > (not demonstrating anything, but just for testing convenience) > > > outside of the example. > > > > Ok, so your main concern is l3fwd code size increase, right? > > Then would it help if for we'll move file parsing code into a separate file(s) > > (under examples/l3fwd) for now? > > Something like examples/l3fwd/(lpm_em)_route_parse.c. > > Yes it would help to isolate the config file parsing code. > What others think? > I still would like config code for loading an LPM table or FIB table to be put inside the relevant libraries themselves, rather than having it in the examples themselves (even if shared between them). /Bruce