From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <adrien.mazarguil@6wind.com> Received: from mail-wr0-f169.google.com (mail-wr0-f169.google.com [209.85.128.169]) by dpdk.org (Postfix) with ESMTP id 82D311B168 for <dev@dpdk.org>; Tue, 16 Jan 2018 15:31:45 +0100 (CET) Received: by mail-wr0-f169.google.com with SMTP id 16so15382834wry.12 for <dev@dpdk.org>; Tue, 16 Jan 2018 06:31:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=a96NUCKUOf0Nu0CgJCtiH+7y2ugkuxQQRbW9HGpTJbA=; b=qJN9fUXuKCZA9Q8uGuB96nka/VBeQr8dpRUl4S4LOBtUjONdRfknOg21MCyJaOFvnu JK5if3xNFZWga87AtoESiLjRk7WnxXd0btp3/2C3fuFWMPzOlXf8zqmWvkAOVNU2530G vERe2lvt2qBphmQJCEKggnj3WV6zzvLivaaWFDRY5z2oH0/66/rPf/KAiRz1a/DCEDZl Jid/LZJbBvuGM4YAtkUf5F8G2ZPWtKbb7OlCuDccXwzHPNYeJkuTWMaYBjrbtOiAXamz q4f3OrBQEQEH6PuiB4lCnJq+sUKj25lmdKRLLj2yy7gmA5ypJ6gBMfS7w3DO3GX9qL+j eDLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=a96NUCKUOf0Nu0CgJCtiH+7y2ugkuxQQRbW9HGpTJbA=; b=FvjE8xySarwxMiJ5h5yrCEI3uzTkn//TwpGoLPW6zq+MwHPescLmy70fGGamTVjwmj B9R6lOPrJV9Mqa/i37U+4/9cZsYdEnnX6iTflhKGHWxICScoT36DnhTzLyEVBTQX6njl qG46oZkR7jw9hsiOHSc/kSpkFKbX6ddoj/35EvSIFsmJHRCfV6wNfve/u+PAbfJOw24i zVb50aMCzjl85UR/ixauYn6BRIp5m1G42pIp+Ayz3W5NYUMHuoevj8PToVhBO8VVkER/ sjjDpL5nglMZfGPG7AqYVnBbL9Ks4LqNeaTXSlcH0VcbrAciGbMm9QzCTQFDf378nOkn mx5A== X-Gm-Message-State: AKwxytdU44y7N8TB7GlXGgNIIZsFwLUIBqxU0DffqoqCEzzSnI2i/x/g dEFoYWE3TPzAKGkF93tEJjW9zg== X-Google-Smtp-Source: ACJfBosdVvFXhZNchvqGpem1NuelI4VHCSfb+1dJEnwES7z2C8VhI/jnRnbo/aA4vnWDSQsYag8f1Q== X-Received: by 10.223.139.218 with SMTP id w26mr14354199wra.67.1516113105219; Tue, 16 Jan 2018 06:31:45 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id k69sm2309721wmg.8.2018.01.16.06.31.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 16 Jan 2018 06:31:44 -0800 (PST) Date: Tue, 16 Jan 2018 15:31:32 +0100 From: Adrien Mazarguil <adrien.mazarguil@6wind.com> To: george.dit@gmail.com Cc: Olivier Matz <olivier.matz@6wind.com>, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>, "dev@dpdk.org" <dev@dpdk.org> Message-ID: <20180116143132.GG4256@6wind.com> References: <1515790887-64502-1-git-send-email-george.dit@gmail.com> <6A0DE07E22DDAD4C9103DF62FEBC09093B710648@shsmsx102.ccr.corp.intel.com> <20180116083958.hk4rtxfoa2y3uydk@platinum> <CAN9HtFA2Rrhx2OqLJLL4F_M1mF8QFksD8EC_XAW+sOZ1QQp-FQ@mail.gmail.com> <20180116092425.h4qecfmki2ih5shq@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180116092425.h4qecfmki2ih5shq@platinum> Subject: Re: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to librte_cmdline X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Tue, 16 Jan 2018 14:31:45 -0000 George, I missed the original RFC thread [1][2] (you should have used it as a cover letter for this series BTW) please see below. On Tue, Jan 16, 2018 at 10:24:25AM +0100, Olivier Matz wrote: > Hi, > > > On Tue, Jan 16, 2018 at 9:40 AM Olivier Matz <olivier.matz@6wind.com> wrote: > > > > On Tue, Jan 16, 2018 at 08:45:32AM +0000, george.dit@gmail.com wrote: > > > Hi Georgios, > > > > > > On Mon, Jan 15, 2018 at 01:30:35AM +0000, Lu, Wenzhuo wrote: > > > > Hi, > > > > > > > > > -----Original Message----- > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Georgios Katsikas > > > > > Sent: Saturday, January 13, 2018 5:01 AM > > > > > To: olivier.matz@6wind.com > > > > > Cc: dev@dpdk.org; Georgios Katsikas <george.dit@gmail.com> > > > > > Subject: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow to > > > > > librte_cmdline > > > > > > > > > > Signed-off-by: Georgios Katsikas <george.dit@gmail.com> > > > > Looks like a good idea to move this code to the lib. > > > > cc Adrien the author of this file, app/test-pmd/cmdline_flow.c. > > > > > > If the command line parsing of rte_flow is something that has some > > > chances to be shared among multiple applications, I agree it makes sense > > > to move it in a library. > > > > > > However, my opinion is that it would be better to have a specific > > > library for it, like librte_flow_cmdline, because I'm not sure that > > > people linking with librte_cmdline always want to pull the rte_flow > > > parsing code. In my opinion the entire flow command parser has nothing to do outside testpmd, it's way too specific, even if another application finds it useful. Code duplication being a bad thing, your application could as well compile or even #include app/test-pmd/cmdline_flow.c directly (not pretty, I know) since it would have the same internal layout as testpmd. Testpmd's Makefile could be modified to spit it out as a separate library if needed. What could make more sense would be to extract the parser engine for librte_cmdline's dynamic tokens (e.g. struct context, struct arg, struct token) and possibly various generic helpers (e.g. parse_string, parse_mac_addr), but not enum index nor token_list[] and friends which define the layout of testpmd's flow command. This would enable other flow-like commands without duplicating the engine every time, however I'm still not sure if making it available outside testpmd is a good idea. Extracting and making it fully generic will require a considerable amount of work. > > Hi Lu, Oliver, > > > > Thanks for your feedback! > > You have a point here, flow commands are only a subset of the parser. > > Do you want me to create this new library and send another patch? > > Let's first wait for Adrien's feedback, he can have counter-arguments. > > > I guess I have to use librte_cmdline as a template/example for creating the > > librte_flow_cmdline library. > > It can be used as an example for Makefile and .map file. I'm not opposed to the idea of exporting the parser engine after making it properly generic, but please reconsider. Testpmd is an application made to validate PMD functionality. The flow command implementation, as neat as it is, remains a complex wrapper on top of the cmdline_parse API which wasn't originally designed for dynamic tokens. Its syntax may evolve without warning if deemed necessary. Making it public will subject it to exported API/ABI constraints and likely impede future evolution. Assuming your application is not dragging testpmd's legacy, I think it would be wiser to re-implement a simpler flow command look-alike on top of a more suitable command-line handling library if you like its syntax. [1] http://dpdk.org/ml/archives/dev/2018-January/086872.html [2] Message-ID: CAN9HtFDz+imqbCKfs6a0NE0W7iF8C+-KiNB0nCRywimspjfEDg@mail.gmail.com -- Adrien Mazarguil 6WIND