From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua0-f173.google.com (mail-ua0-f173.google.com [209.85.217.173]) by dpdk.org (Postfix) with ESMTP id C08151B1C3 for ; Tue, 16 Jan 2018 15:54:59 +0100 (CET) Received: by mail-ua0-f173.google.com with SMTP id x16so10932547uaj.11 for ; Tue, 16 Jan 2018 06:54:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=MY5KwT/4Jxp8iA38k7AF3XOuUwvq5hxC87Jn4piDfKU=; b=biI8XzYtTnMh8XN50JwtM6A6vvW+Lk+29ZIXEBQZ6iUvup1+AnayybDuoCziMmKgZC PNlgm8Xchz9vfciVjktZQvxxxA3z+CxxuGheAz7muk9W1WG3TzLJzMQqayCkZm+5RmGM aUlaGlfxDvjwaOsJG0g0kMoOm6RPMmAhTe+78gYqKwvEWzYjFZPCdkpkz4wlSWMhd6WY Ya0qUO6LbZYUxgRor9YtAEfxGU7eZYZyLJbOPsdq5vTrIra/0Kr8ycGuCboakDFB9cMF kwBYMRElBZqH+Z5/KVERZ8//ri6WgTDOqujx0R8CtIuGCZ+Cc4F7XWoC/k9ZGpX2vtjr 3UOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=MY5KwT/4Jxp8iA38k7AF3XOuUwvq5hxC87Jn4piDfKU=; b=BsYfyShecJzHOgQYc1Uj1bswI+CdlregEr5vyM1EiBreAh+heTWdL8OkTw+qQM+s18 oIHxRvcPqOKRQblyJLQzJ9re+WJLJsr9SNXvrlecdsDEBB4n6R8bmM9P69888nWC4P2R 0M3vK02+GL2oCk9OZ6Wnt2/5dbZNGxNozPXoQ5cVIZO7ei4KBe/jY8z9ktjL2mZd/hJY CSeMXUBvChkSslLuq8FFn5uaPuFww0chWgbcrB0iqk6r2T6PrTj12Ms5wgYRnzDkEkUj Ki2qj40ETCFXTNGcdHCX7IFSONnTS6s6Uz2ytjAgPMT5C5kzqr0Sz+q1sE2cMZvMygj3 RtaQ== X-Gm-Message-State: AKwxytdIyTYyQ0fnlF7DpseMMzSPqSGcV2D8GA/hosP/EYUZ9VQUznDE jl4Z2hgBqEp0Oxzjc+GDgxtemKpVKuhV1qvS0s1Lnw== X-Google-Smtp-Source: ACJfBouu4T+wfVQQvliL00z2owIh9F50wLSQ9MZzfqZ33bIfSy+58z9G8h6fiagRZem/cP8MShXP1bmpxrQOaqTvh/E= X-Received: by 10.176.2.241 with SMTP id 104mr28047062uah.23.1516114498741; Tue, 16 Jan 2018 06:54:58 -0800 (PST) MIME-Version: 1.0 Received: by 10.103.10.198 with HTTP; Tue, 16 Jan 2018 06:54:57 -0800 (PST) In-Reply-To: <20180116143132.GG4256@6wind.com> References: <1515790887-64502-1-git-send-email-george.dit@gmail.com> <6A0DE07E22DDAD4C9103DF62FEBC09093B710648@shsmsx102.ccr.corp.intel.com> <20180116083958.hk4rtxfoa2y3uydk@platinum> <20180116092425.h4qecfmki2ih5shq@platinum> <20180116143132.GG4256@6wind.com> From: george.dit@gmail.com Date: Tue, 16 Jan 2018 15:54:57 +0100 Message-ID: To: Adrien Mazarguil Cc: Olivier Matz , "Lu, Wenzhuo" , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jan 2018 14:55:00 -0000 Hi Adrien, Thanks for your insights and sorry for omitting the cover letter (this is my first patch in DPDK). I understand your concerns. The reason I proposed this patch is to facilitate a more high level vendor agnostic API for configuring and monitoring DPDK-based NICs. To avoid just copying thousands of lines of code into my application, do you think it is feasible to move at least the components (struct context, struct arg, struct token and the parse_* helpers) you mentioned and restructure testpmd in a way that allows applications to re-use all of its parsing features? I could give it a try and come back with a new patch, otherwise I am perfectly ok if you want to do it instead. Best regards, Georgios On Tue, Jan 16, 2018 at 3:31 PM, Adrien Mazarguil < adrien.mazarguil@6wind.com> wrote: > 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 > 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 > > > > > > Subject: [dpdk-dev] [PATCH 1/3] app/testpmd: Moved cmdline_flow > to > > > > > > librte_cmdline > > > > > > > > > > > > Signed-off-by: Georgios Katsikas > > > > > 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 > -- Georgios Katsikas Industrial Ph.D. Student Network Intelligence Group Decision, Networks, and Analytics (DNA) Lab RISE SICS E-Mail: georgios.katsikas@ri.se