From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Subject: Re: [PATCH v2 1/3] cmdline: increase command line buffer Date: Fri, 26 Feb 2016 16:16:51 +0100 Message-ID: <20160226151651.GE2105@autoinstall.dev.6wind.com> References: <1452090774-10650-1-git-send-email-nelio.laranjeiro@6wind.com> <1452595749-11297-1-git-send-email-nelio.laranjeiro@6wind.com> <1452595749-11297-2-git-send-email-nelio.laranjeiro@6wind.com> <5694F58F.1040105@redhat.com> <20160115084439.GW13678@autoinstall.dev.6wind.com> <5698B51B.60603@redhat.com> <569CF8F3.2020908@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org To: john.mcnamara@intel.com Return-path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id 70F9556B7 for ; Fri, 26 Feb 2016 16:17:07 +0100 (CET) Received: by mail-wm0-f54.google.com with SMTP id a4so74079846wme.1 for ; Fri, 26 Feb 2016 07:17:07 -0800 (PST) Content-Disposition: inline In-Reply-To: <569CF8F3.2020908@6wind.com> 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" On Mon, Jan 18, 2016 at 03:38:43PM +0100, Olivier MATZ wrote: > Hi, >=20 > On 01/15/2016 10:00 AM, Panu Matilainen wrote: > >>>> diff --git a/lib/librte_cmdline/cmdline_rdline.h > >>>> b/lib/librte_cmdline/cmdline_rdline.h > >>>> index b9aad9b..72e2dad 100644 > >>>> --- a/lib/librte_cmdline/cmdline_rdline.h > >>>> +++ b/lib/librte_cmdline/cmdline_rdline.h > >>>> @@ -93,7 +93,7 @@ extern "C" { > >>>> #endif > >>>> > >>>> /* configuration */ > >>>> -#define RDLINE_BUF_SIZE 256 > >>>> +#define RDLINE_BUF_SIZE 512 > >>>> #define RDLINE_PROMPT_SIZE 32 > >>>> #define RDLINE_VT100_BUF_SIZE 8 > >>>> #define RDLINE_HISTORY_BUF_SIZE BUFSIZ > >>> > >>> Having to break a library ABI for a change like this is a bit > >>> ridiculous. > >> > >> Sure, but John McNamara needed it to handle flow director with IPv6[= 1]. > >> > >> For my part, I was needing it to manipulate the RETA table, but as I > >> wrote in the cover letter, it ends by breaking other commands. > >> Olivier Matz, has proposed another way to handle long commands lines= [2], > >> it could be a good idea to go on this direction. > >> > >> For RETA situation, we already discussed on a new API, but for now, = I > >> do not have time for it (and as it is another ABI breakage it could = only > >> be done for 16.07 or 2.4)[3]. > >> > >> If this patch is no more needed we can just drop it, for that I woul= d > >> like to have the point of view from John. > >=20 > > Note that I was not objecting to the patch as such, I can easily see = 256 > > characters not being enough for commandline buffer. > >=20 > > I was merely noting that having to break an ABI to increase an > > effectively internal buffer size is a sign of a, um, less-than-optima= l > > library design. >=20 > You are right about the cmdline ABI. Changing this buffer size > should not imply an ABI change. I'll try to find some time to > investigate this issue. >=20 > Another question we could raise is: should we export the API of > librte_cmdline to external applications? Now that baremetal dpdk is > not supported, having this library in dpdk is probably useless as > we can surely find standard replacements for it. A first step could > be to mark it as "internal". >=20 > About the patch N=E9lio's patch itself, I'm not so convinced having mor= e > than 256 characters is absolutely required, and I would prefer to see > the commands beeing reworked to be more human-readable. On the other > hand, the ABI breakage was announced so there is no reason to nack > this patch now. >=20 > Regards, > Olivier John, What is your position about this patch? Is it still needed? Regards, --=20 N=E9lio Laranjeiro 6WIND