On Mon, Jan 18, 2016 at 03:38:43PM +0100, Olivier MATZ wrote: > Hi, > > 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 would > >> like to have the point of view from John. > > > > Note that I was not objecting to the patch as such, I can easily see 256 > > characters not being enough for commandline buffer. > > > > 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-optimal > > library design. > > 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. > > 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". > > About the patch N?lio's patch itself, I'm not so convinced having more > 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. > > Regards, > Olivier
John, What is your position about this patch? Is it still needed? Regards, -- N?lio Laranjeiro 6WIND