diff mbox

[dpdk-dev,v4,17/23] test_table_pipeline: repair munged indirection level

Message ID 152627465307.53156.9964743394274041446.stgit@localhost.localdomain (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Andy Green May 14, 2018, 5:10 a.m. UTC 
  Signed-off-by: Andy Green <andy@warmcat.com>
---
 test/test/test_table_pipeline.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Jasvinder Singh May 14, 2018, 10:35 a.m. UTC | #1 
> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green

> Sent: Monday, May 14, 2018 6:11 AM

> To: dev@dpdk.org

> Subject: [dpdk-dev] [PATCH v4 17/23] test_table_pipeline: repair munged

> indirection level

> 

> Signed-off-by: Andy Green <andy@warmcat.com>

> ---

>  test/test/test_table_pipeline.c |   12 ++++++------

>  1 file changed, 6 insertions(+), 6 deletions(-)

> 

> diff --git a/test/test/test_table_pipeline.c b/test/test/test_table_pipeline.c

> index 5ec4c5244..70dbd25f4 100644

> --- a/test/test/test_table_pipeline.c

> +++ b/test/test/test_table_pipeline.c

> @@ -63,21 +63,21 @@ rte_pipeline_port_out_action_handler

> port_action_stub(struct rte_mbuf **pkts,

> 

>  rte_pipeline_table_action_handler_hit

>  table_action_0x00(struct rte_pipeline *p, struct rte_mbuf **pkts,

> -	uint64_t pkts_mask, struct rte_pipeline_table_entry **entry, void

> *arg);

> +	uint64_t pkts_mask, struct rte_pipeline_table_entry *entry, void

> +*arg);


In my opinion , this is wrong fix. Here, table action is meant to be applied on any number of packets (maximum 64), therefore,
entry parameter should be array of up to 64 pointers to rte_pipeline_table_entry structure. BTW, Dave has already sent
fix for gcc 8.1 build error.  http://www.dpdk.org/ml/archives/dev/2018-May/101113.html
Andy Green May 14, 2018, 10:46 a.m. UTC | #2 
On 05/14/2018 06:35 PM, Singh, Jasvinder wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
>> Sent: Monday, May 14, 2018 6:11 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v4 17/23] test_table_pipeline: repair munged
>> indirection level
>>
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> ---
>>   test/test/test_table_pipeline.c |   12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/test/test/test_table_pipeline.c b/test/test/test_table_pipeline.c
>> index 5ec4c5244..70dbd25f4 100644
>> --- a/test/test/test_table_pipeline.c
>> +++ b/test/test/test_table_pipeline.c
>> @@ -63,21 +63,21 @@ rte_pipeline_port_out_action_handler
>> port_action_stub(struct rte_mbuf **pkts,
>>
>>   rte_pipeline_table_action_handler_hit
>>   table_action_0x00(struct rte_pipeline *p, struct rte_mbuf **pkts,
>> -	uint64_t pkts_mask, struct rte_pipeline_table_entry **entry, void
>> *arg);
>> +	uint64_t pkts_mask, struct rte_pipeline_table_entry *entry, void
>> +*arg);
> 
> In my opinion , this is wrong fix. Here, table action is meant to be applied on any number of packets (maximum 64), therefore,
> entry parameter should be array of up to 64 pointers to rte_pipeline_table_entry structure. BTW, Dave has already sent
> fix for gcc 8.1 build error.  http://www.dpdk.org/ml/archives/dev/2018-May/101113.html


Well, no worries... I saw something was broken (the * vs the **) and 
simply guessed the resolution.

If it's wrong, just drop my patch about this.

-Andy
diff mbox

Patch

diff --git a/test/test/test_table_pipeline.c b/test/test/test_table_pipeline.c
index 5ec4c5244..70dbd25f4 100644
--- a/test/test/test_table_pipeline.c
+++ b/test/test/test_table_pipeline.c
@@ -63,21 +63,21 @@  rte_pipeline_port_out_action_handler port_action_stub(struct rte_mbuf **pkts,
 
 rte_pipeline_table_action_handler_hit
 table_action_0x00(struct rte_pipeline *p, struct rte_mbuf **pkts,
-	uint64_t pkts_mask, struct rte_pipeline_table_entry **entry, void *arg);
+	uint64_t pkts_mask, struct rte_pipeline_table_entry *entry, void *arg);
 
 rte_pipeline_table_action_handler_hit
 table_action_stub_hit(struct rte_pipeline *p, struct rte_mbuf **pkts,
-	uint64_t pkts_mask, struct rte_pipeline_table_entry **entry, void *arg);
+	uint64_t pkts_mask, struct rte_pipeline_table_entry *entry, void *arg);
 
 static int
 table_action_stub_miss(struct rte_pipeline *p, struct rte_mbuf **pkts,
-	uint64_t pkts_mask, struct rte_pipeline_table_entry **entry, void *arg);
+	uint64_t pkts_mask, struct rte_pipeline_table_entry *entry, void *arg);
 
 rte_pipeline_table_action_handler_hit
 table_action_0x00(__attribute__((unused)) struct rte_pipeline *p,
 	__attribute__((unused)) struct rte_mbuf **pkts,
 	uint64_t pkts_mask,
-	__attribute__((unused)) struct rte_pipeline_table_entry **entry,
+	__attribute__((unused)) struct rte_pipeline_table_entry *entry,
 	__attribute__((unused)) void *arg)
 {
 	printf("Table Action, setting pkts_mask to 0x00\n");
@@ -90,7 +90,7 @@  rte_pipeline_table_action_handler_hit
 table_action_stub_hit(__attribute__((unused)) struct rte_pipeline *p,
 	__attribute__((unused)) struct rte_mbuf **pkts,
 	uint64_t pkts_mask,
-	__attribute__((unused)) struct rte_pipeline_table_entry **entry,
+	__attribute__((unused)) struct rte_pipeline_table_entry *entry,
 	__attribute__((unused)) void *arg)
 {
 	printf("STUB Table Action Hit - doing nothing\n");
@@ -105,7 +105,7 @@  static int
 table_action_stub_miss(struct rte_pipeline *p,
 	__attribute__((unused)) struct rte_mbuf **pkts,
 	uint64_t pkts_mask,
-	__attribute__((unused)) struct rte_pipeline_table_entry **entry,
+	__attribute__((unused)) struct rte_pipeline_table_entry *entry,
 	__attribute__((unused)) void *arg)
 {
 	printf("STUB Table Action Miss - setting mask to 0x%"PRIx64"\n",