[dpdk-dev,1/2] doc: announce ABI change for cmdline buffer size

Message ID 1447087700-20921-1-git-send-email-nelio.laranjeiro@6wind.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Nélio Laranjeiro Nov. 9, 2015, 4:48 p.m. UTC
  Current buffer size are not enough for a few testpmd commands.

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)
  

Comments

John McNamara Nov. 10, 2015, 5:29 p.m. UTC | #1
> -----Original Message-----
> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> Sent: Monday, November 9, 2015 4:48 PM
> To: dev@dpdk.org
> Cc: olivier.matz@6wind.com; thomas.monjalon@6wind.com; Mcnamara, John; Lu,
> Wenzhuo
> Subject: [PATCH 1/2] doc: announce ABI change for cmdline buffer size
> 
> Current buffer size are not enough for a few testpmd commands.
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Acked-by: John McNamara <john.mcnamara@intel.com>
  
Olivier Matz Nov. 20, 2015, 4:28 p.m. UTC | #2
Hi Nélio,

On 11/10/2015 06:29 PM, Mcnamara, John wrote:
> 
> 
>> -----Original Message-----
>> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
>> Sent: Monday, November 9, 2015 4:48 PM
>> To: dev@dpdk.org
>> Cc: olivier.matz@6wind.com; thomas.monjalon@6wind.com; Mcnamara, John; Lu,
>> Wenzhuo
>> Subject: [PATCH 1/2] doc: announce ABI change for cmdline buffer size
>>
>> Current buffer size are not enough for a few testpmd commands.
>>
>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>
> 

While I'm not fundamentally opposed to change the buffer size,
I'm wondering if the impacted commands shouldn't be reworked to
have smaller lines. 256 is already a quite big value for a line:

0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345

For instance, we could change some commands to use contexts.
Dummy example with reta config:

testpmd> port config 0 rss reta
testpmd-reta-config-0> add hash1 queue1
testpmd-reta-config-0> add hash2 queue2
testpmd-reta-config-0> del hash1 queue1
testpmd-reta-config-0> show
testpmd-reta-config-0> commit
testpmd>

What do you think?

Regards,
Olivier
  
Bruce Richardson Nov. 20, 2015, 4:33 p.m. UTC | #3
On Fri, Nov 20, 2015 at 05:28:43PM +0100, Olivier MATZ wrote:
> Hi Nélio,
> 
> On 11/10/2015 06:29 PM, Mcnamara, John wrote:
> > 
> > 
> >> -----Original Message-----
> >> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> >> Sent: Monday, November 9, 2015 4:48 PM
> >> To: dev@dpdk.org
> >> Cc: olivier.matz@6wind.com; thomas.monjalon@6wind.com; Mcnamara, John; Lu,
> >> Wenzhuo
> >> Subject: [PATCH 1/2] doc: announce ABI change for cmdline buffer size
> >>
> >> Current buffer size are not enough for a few testpmd commands.
> >>
> >> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > 
> > Acked-by: John McNamara <john.mcnamara@intel.com>
> > 
> 
> While I'm not fundamentally opposed to change the buffer size,
> I'm wondering if the impacted commands shouldn't be reworked to
> have smaller lines. 256 is already a quite big value for a line:
> 
> 0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345
> 
> For instance, we could change some commands to use contexts.
> Dummy example with reta config:
> 
> testpmd> port config 0 rss reta
> testpmd-reta-config-0> add hash1 queue1
> testpmd-reta-config-0> add hash2 queue2
> testpmd-reta-config-0> del hash1 queue1
> testpmd-reta-config-0> show
> testpmd-reta-config-0> commit
> testpmd>
> 
> What do you think?
> 
+1

multiple shorter commands are much less error prone than a single long one.

/Bruce
  
John McNamara Dec. 3, 2015, 4:32 p.m. UTC | #4
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Friday, November 20, 2015 4:29 PM
> To: Mcnamara, John; Nelio Laranjeiro; dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; Lu, Wenzhuo
> Subject: Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size
> 
> Hi Nélio,
> 
> On 11/10/2015 06:29 PM, Mcnamara, John wrote:
> >
> >
> >> -----Original Message-----
> >> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> >> Sent: Monday, November 9, 2015 4:48 PM
> >> To: dev@dpdk.org
> >> Cc: olivier.matz@6wind.com; thomas.monjalon@6wind.com; Mcnamara,
> >> John; Lu, Wenzhuo
> >> Subject: [PATCH 1/2] doc: announce ABI change for cmdline buffer size
> >>
> >> Current buffer size are not enough for a few testpmd commands.
> >>
> >> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >
> > Acked-by: John McNamara <john.mcnamara@intel.com>
> >
> 
> While I'm not fundamentally opposed to change the buffer size, I'm
> wondering if the impacted commands shouldn't be reworked to have smaller
> lines. 256 is already a quite big value for a line:
> 
> 01234567890123456789012345678901234567890123456789012345678901234567890123
> 45678901234567890123456789012345678901234567890123456789012345678901234567
> 89012345678901234567890123456789012345678901234567890123456789012345678901
> 2345678901234567890123456789012345
> 
> For instance, we could change some commands to use contexts.
> Dummy example with reta config:
> 
> testpmd> port config 0 rss reta
> testpmd-reta-config-0> add hash1 queue1
> testpmd-reta-config-0> add hash2 queue2
> testpmd-reta-config-0> del hash1 queue1
> testpmd-reta-config-0> show
> testpmd-reta-config-0> commit
> testpmd>
> 
> What do you think?

Hi,

I think it is a good idea but my concern is that it won't get done unless someone commits to doing it.

And if they do it will be a non-trivial change since the commandline/runtime parsing in testpmd is a little crufty and it isn't set up to do this kind of sub-command parsing.

Also, we will still have to maintain backward compatibility (for users and testers) with the existing single line versions of the commands.

So, I'd like to make sure that this change isn't blocked on the assumption that it will be fixed with a more elegant solution if that solution is unlikely to happen.

However, I do think that we should avoid bolting on every increasing options to existing testpmd commands and should instead create new commands where it makes sense.

John.
--
  
Thomas Monjalon Dec. 14, 2015, 2:13 p.m. UTC | #5
2015-11-10 17:29, Mcnamara, John:
> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> > Current buffer size are not enough for a few testpmd commands.
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
  
Olga Shern Dec. 14, 2015, 2:54 p.m. UTC | #6
Acked-by: Olga Shern <olgas@mellanox.com>

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
Sent: Monday, December 14, 2015 4:13 PM
To: Mcnamara, John <john.mcnamara@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] doc: announce ABI change for cmdline buffer size

2015-11-10 17:29, Mcnamara, John:
> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> > Current buffer size are not enough for a few testpmd commands.
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
  
Thomas Monjalon Dec. 15, 2015, 6:11 a.m. UTC | #7
> > Current buffer size are not enough for a few testpmd commands.
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Acked-by: John McNamara <john.mcnamara@intel.com>
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Acked-by: Olga Shern <olgas@mellanox.com>

Applied, thanks
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 730c3b7..c75b4b4 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -47,3 +47,8 @@  Deprecation Notices
   and table action handlers will be updated:
   the pipeline parameter will be added, the packets mask parameter will be
   either removed (for input port action handler) or made input-only.
+
+* ABI changes are planned in cmdline buffer size to allow the use of long
+  commands (such as RETA update in testpmd).  This should impact
+  CMDLINE_PARSE_RESULT_BUFSIZE, STR_TOKEN_SIZE and RDLINE_BUF_SIZE.
+  It should be integrated in release 2.3.