[PATCH 1/2] doc: announce ABI change for cmdline buffer size

[PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Nelio Laranjeiro]

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(+)

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.
-- 
2.1.4

[PATCH 2/2] doc: announce ABI change for RETA configuration

[Nelio Laranjeiro]

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

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index c75b4b4..8554c53 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -52,3 +52,8 @@ Deprecation Notices
   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.
+
+* ABI changes is planned for the reta field in struct rte_eth_rss_reta_entry64
+  which handles at most 256 entries (8 bits) while newer NICs support larger
+  tables (512 entries).
+  It should be integrated in release 2.3.
-- 
2.1.4

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Mcnamara, John]

> -----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>

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Olivier MATZ]

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

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Bruce Richardson]

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

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Mcnamara, John]

> -----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.
-- 

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Thomas Monjalon]

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>

Re: [PATCH 2/2] doc: announce ABI change for RETA configuration

[Thomas Monjalon]

2015-11-09 17:48, Nelio Laranjeiro:
> +* ABI changes is planned for the reta field in struct rte_eth_rss_reta_entry64
> +  which handles at most 256 entries (8 bits) while newer NICs support larger
> +  tables (512 entries).
> +  It should be integrated in release 2.3.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Olga Shern]

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>

Re: [PATCH 2/2] doc: announce ABI change for RETA configuration

[Olga Shern]

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:25 PM
To: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 2/2] doc: announce ABI change for RETA configuration

2015-11-09 17:48, Nelio Laranjeiro:
> +* ABI changes is planned for the reta field in struct rte_eth_rss_reta_entry64
> +  which handles at most 256 entries (8 bits) while newer NICs support larger
> +  tables (512 entries).
> +  It should be integrated in release 2.3.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Re: [PATCH 2/2] doc: announce ABI change for RETA configuration

[Lu, Wenzhuo]

Hi,

> -----Original Message-----
> From: Nelio Laranjeiro [mailto:nelio.laranjeiro@6wind.com]
> Sent: Tuesday, November 10, 2015 12:48 AM
> To: dev@dpdk.org
> Cc: olivier.matz@6wind.com; thomas.monjalon@6wind.com; Mcnamara,
> John <john.mcnamara@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [PATCH 2/2] doc: announce ABI change for RETA configuration
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [PATCH 1/2] doc: announce ABI change for cmdline buffer size

[Thomas Monjalon]

> > 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

Re: [PATCH 2/2] doc: announce ABI change for RETA configuration

[Thomas Monjalon]

> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Acked-by: Olga Shern <olgas@mellanox.com>

Applied, thanks