diff mbox series

[4/4] test/lpm: improve coverage on tbl8

Message ID 20210108082127.1061538-5-ruifeng.wang@arm.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series lpm lookupx4 fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Ruifeng Wang Jan. 8, 2021, 8:21 a.m. UTC 
  Existing test cases create 256 tbl8 groups for testing. The number covers
only 8 bit next_hop/group field. Since the next_hop/group field had been
extended to 24-bits, creating more than 256 groups in tests can improve
the coverage.

Coverage was not expanded to reach the max supported group number, because
it would take too much time to run for this fast-test.

Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test/test_lpm.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

David Christensen Jan. 11, 2021, 9:29 p.m. UTC | #1 
On 1/8/21 12:21 AM, Ruifeng Wang wrote:
> Existing test cases create 256 tbl8 groups for testing. The number covers
> only 8 bit next_hop/group field. Since the next_hop/group field had been
> extended to 24-bits, creating more than 256 groups in tests can improve
> the coverage.
> 
> Coverage was not expanded to reach the max supported group number, because
> it would take too much time to run for this fast-test.
> 
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>   app/test/test_lpm.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test/test_lpm.c b/app/test/test_lpm.c
> index 258b2f67c..ee6c4280b 100644
> --- a/app/test/test_lpm.c
> +++ b/app/test/test_lpm.c
> @@ -993,7 +993,7 @@ test13(void)
>   }
> 
>   /*
> - * Fore TBL8 extension exhaustion. Add 256 rules that require a tbl8 extension.
> + * For TBL8 extension exhaustion. Add 512 rules that require a tbl8 extension.
>    * No more tbl8 extensions will be allowed. Now add one more rule that required
>    * a tbl8 extension and get fail.
>    * */
> @@ -1008,28 +1008,34 @@ test14(void)
>   	struct rte_lpm_config config;
> 
>   	config.max_rules = 256 * 32;
> -	config.number_tbl8s = NUMBER_TBL8S;
> +	config.number_tbl8s = 512;
>   	config.flags = 0;
> -	uint32_t ip, next_hop_add, next_hop_return;
> +	uint32_t ip, next_hop_base, next_hop_return;
>   	uint8_t depth;
>   	int32_t status = 0;
> +	xmm_t ipx4;
> +	uint32_t hop[4];
> 
>   	/* Add enough space for 256 rules for every depth */
>   	lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config);
>   	TEST_LPM_ASSERT(lpm != NULL);
> 
>   	depth = 32;
> -	next_hop_add = 100;
> +	next_hop_base = 100;
>   	ip = RTE_IPV4(0, 0, 0, 0);
> 
>   	/* Add 256 rules that require a tbl8 extension */
> -	for (; ip <= RTE_IPV4(0, 0, 255, 0); ip += 256) {
> -		status = rte_lpm_add(lpm, ip, depth, next_hop_add);
> +	for (; ip <= RTE_IPV4(0, 1, 255, 0); ip += 256) {
> +		status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
>   		TEST_LPM_ASSERT(status == 0);
> 
>   		status = rte_lpm_lookup(lpm, ip, &next_hop_return);
>   		TEST_LPM_ASSERT((status == 0) &&
> -				(next_hop_return == next_hop_add));
> +				(next_hop_return == next_hop_base + ip));
> +
> +		ipx4 = vect_set_epi32(ip + 3, ip + 2, ip + 1, ip);
> +		rte_lpm_lookupx4(lpm, ipx4, hop, UINT32_MAX);
> +		TEST_LPM_ASSERT(hop[0] == next_hop_base + ip);
>   	}
> 
>   	/* All tbl8 extensions have been used above. Try to add one more and
> @@ -1037,7 +1043,7 @@ test14(void)
>   	ip = RTE_IPV4(1, 0, 0, 0);
>   	depth = 32;
> 
> -	status = rte_lpm_add(lpm, ip, depth, next_hop_add);
> +	status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
>   	TEST_LPM_ASSERT(status < 0);
> 
>   	rte_lpm_free(lpm);
> 

Tested-by: David Christensen <drc@linux.vnet.ibm.com>
Vladimir Medvedkin Jan. 13, 2021, 6:51 p.m. UTC | #2 
Hi Ruifeng,

Please find comment inlined. Apart from that looks good.

On 08/01/2021 08:21, Ruifeng Wang wrote:
> Existing test cases create 256 tbl8 groups for testing. The number covers
> only 8 bit next_hop/group field. Since the next_hop/group field had been
> extended to 24-bits, creating more than 256 groups in tests can improve
> the coverage.
> 
> Coverage was not expanded to reach the max supported group number, because
> it would take too much time to run for this fast-test.
> 
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>   app/test/test_lpm.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test/test_lpm.c b/app/test/test_lpm.c
> index 258b2f67c..ee6c4280b 100644
> --- a/app/test/test_lpm.c
> +++ b/app/test/test_lpm.c
> @@ -993,7 +993,7 @@ test13(void)
>   }
>   
>   /*
> - * Fore TBL8 extension exhaustion. Add 256 rules that require a tbl8 extension.
> + * For TBL8 extension exhaustion. Add 512 rules that require a tbl8 extension.
>    * No more tbl8 extensions will be allowed. Now add one more rule that required
>    * a tbl8 extension and get fail.
>    * */
> @@ -1008,28 +1008,34 @@ test14(void)
>   	struct rte_lpm_config config;
>   
>   	config.max_rules = 256 * 32;
> -	config.number_tbl8s = NUMBER_TBL8S;
> +	config.number_tbl8s = 512;
>   	config.flags = 0;
> -	uint32_t ip, next_hop_add, next_hop_return;
> +	uint32_t ip, next_hop_base, next_hop_return;
>   	uint8_t depth;
>   	int32_t status = 0;
> +	xmm_t ipx4;
> +	uint32_t hop[4];
>   
>   	/* Add enough space for 256 rules for every depth */
>   	lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config);
>   	TEST_LPM_ASSERT(lpm != NULL);
>   
>   	depth = 32;
> -	next_hop_add = 100;
> +	next_hop_base = 100;
>   	ip = RTE_IPV4(0, 0, 0, 0);
>   
>   	/* Add 256 rules that require a tbl8 extension */
> -	for (; ip <= RTE_IPV4(0, 0, 255, 0); ip += 256) {
> -		status = rte_lpm_add(lpm, ip, depth, next_hop_add);
> +	for (; ip <= RTE_IPV4(0, 1, 255, 0); ip += 256) {
> +		status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
>   		TEST_LPM_ASSERT(status == 0);
>   
>   		status = rte_lpm_lookup(lpm, ip, &next_hop_return);
>   		TEST_LPM_ASSERT((status == 0) &&
> -				(next_hop_return == next_hop_add));
> +				(next_hop_return == next_hop_base + ip));
> +
> +		ipx4 = vect_set_epi32(ip + 3, ip + 2, ip + 1, ip);
> +		rte_lpm_lookupx4(lpm, ipx4, hop, UINT32_MAX);
> +		TEST_LPM_ASSERT(hop[0] == next_hop_base + ip);

I think it is worth to check all 4 returned next hops here.

>   	}
>   
>   	/* All tbl8 extensions have been used above. Try to add one more and
> @@ -1037,7 +1043,7 @@ test14(void)
>   	ip = RTE_IPV4(1, 0, 0, 0);
>   	depth = 32;
>   
> -	status = rte_lpm_add(lpm, ip, depth, next_hop_add);
> +	status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
>   	TEST_LPM_ASSERT(status < 0);
>   
>   	rte_lpm_free(lpm);
>
Ruifeng Wang Jan. 14, 2021, 6:38 a.m. UTC | #3 
> -----Original Message-----
> From: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> Sent: Thursday, January 14, 2021 2:52 AM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; Bruce Richardson
> <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; jerinj@marvell.com;
> drc@linux.vnet.ibm.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [PATCH 4/4] test/lpm: improve coverage on tbl8
> 
> Hi Ruifeng,
> 
> Please find comment inlined. Apart from that looks good.
> 
> On 08/01/2021 08:21, Ruifeng Wang wrote:
> > Existing test cases create 256 tbl8 groups for testing. The number
> > covers only 8 bit next_hop/group field. Since the next_hop/group field
> > had been extended to 24-bits, creating more than 256 groups in tests
> > can improve the coverage.
> >
> > Coverage was not expanded to reach the max supported group number,
> > because it would take too much time to run for this fast-test.
> >
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >   app/test/test_lpm.c | 22 ++++++++++++++--------
> >   1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/app/test/test_lpm.c b/app/test/test_lpm.c index
> > 258b2f67c..ee6c4280b 100644
> > --- a/app/test/test_lpm.c
> > +++ b/app/test/test_lpm.c
> > @@ -993,7 +993,7 @@ test13(void)
> >   }
> >
> >   /*
> > - * Fore TBL8 extension exhaustion. Add 256 rules that require a tbl8
> extension.
> > + * For TBL8 extension exhaustion. Add 512 rules that require a tbl8
> extension.
> >    * No more tbl8 extensions will be allowed. Now add one more rule that
> required
> >    * a tbl8 extension and get fail.
> >    * */
> > @@ -1008,28 +1008,34 @@ test14(void)
> >   	struct rte_lpm_config config;
> >
> >   	config.max_rules = 256 * 32;
> > -	config.number_tbl8s = NUMBER_TBL8S;
> > +	config.number_tbl8s = 512;
> >   	config.flags = 0;
> > -	uint32_t ip, next_hop_add, next_hop_return;
> > +	uint32_t ip, next_hop_base, next_hop_return;
> >   	uint8_t depth;
> >   	int32_t status = 0;
> > +	xmm_t ipx4;
> > +	uint32_t hop[4];
> >
> >   	/* Add enough space for 256 rules for every depth */
> >   	lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config);
> >   	TEST_LPM_ASSERT(lpm != NULL);
> >
> >   	depth = 32;
> > -	next_hop_add = 100;
> > +	next_hop_base = 100;
> >   	ip = RTE_IPV4(0, 0, 0, 0);
> >
> >   	/* Add 256 rules that require a tbl8 extension */
> > -	for (; ip <= RTE_IPV4(0, 0, 255, 0); ip += 256) {
> > -		status = rte_lpm_add(lpm, ip, depth, next_hop_add);
> > +	for (; ip <= RTE_IPV4(0, 1, 255, 0); ip += 256) {
> > +		status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
> >   		TEST_LPM_ASSERT(status == 0);
> >
> >   		status = rte_lpm_lookup(lpm, ip, &next_hop_return);
> >   		TEST_LPM_ASSERT((status == 0) &&
> > -				(next_hop_return == next_hop_add));
> > +				(next_hop_return == next_hop_base + ip));
> > +
> > +		ipx4 = vect_set_epi32(ip + 3, ip + 2, ip + 1, ip);
> > +		rte_lpm_lookupx4(lpm, ipx4, hop, UINT32_MAX);
> > +		TEST_LPM_ASSERT(hop[0] == next_hop_base + ip);
> 
> I think it is worth to check all 4 returned next hops here.

Agree. I will send out v2.

> 
> >   	}
> >
> >   	/* All tbl8 extensions have been used above. Try to add one more
> > and @@ -1037,7 +1043,7 @@ test14(void)
> >   	ip = RTE_IPV4(1, 0, 0, 0);
> >   	depth = 32;
> >
> > -	status = rte_lpm_add(lpm, ip, depth, next_hop_add);
> > +	status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
> >   	TEST_LPM_ASSERT(status < 0);
> >
> >   	rte_lpm_free(lpm);
> >
> 
> 
> --
> Regards,
> Vladimir
diff mbox series

Patch

diff --git a/app/test/test_lpm.c b/app/test/test_lpm.c
index 258b2f67c..ee6c4280b 100644
--- a/app/test/test_lpm.c
+++ b/app/test/test_lpm.c
@@ -993,7 +993,7 @@  test13(void)
 }
 
 /*
- * Fore TBL8 extension exhaustion. Add 256 rules that require a tbl8 extension.
+ * For TBL8 extension exhaustion. Add 512 rules that require a tbl8 extension.
  * No more tbl8 extensions will be allowed. Now add one more rule that required
  * a tbl8 extension and get fail.
  * */
@@ -1008,28 +1008,34 @@  test14(void)
 	struct rte_lpm_config config;
 
 	config.max_rules = 256 * 32;
-	config.number_tbl8s = NUMBER_TBL8S;
+	config.number_tbl8s = 512;
 	config.flags = 0;
-	uint32_t ip, next_hop_add, next_hop_return;
+	uint32_t ip, next_hop_base, next_hop_return;
 	uint8_t depth;
 	int32_t status = 0;
+	xmm_t ipx4;
+	uint32_t hop[4];
 
 	/* Add enough space for 256 rules for every depth */
 	lpm = rte_lpm_create(__func__, SOCKET_ID_ANY, &config);
 	TEST_LPM_ASSERT(lpm != NULL);
 
 	depth = 32;
-	next_hop_add = 100;
+	next_hop_base = 100;
 	ip = RTE_IPV4(0, 0, 0, 0);
 
 	/* Add 256 rules that require a tbl8 extension */
-	for (; ip <= RTE_IPV4(0, 0, 255, 0); ip += 256) {
-		status = rte_lpm_add(lpm, ip, depth, next_hop_add);
+	for (; ip <= RTE_IPV4(0, 1, 255, 0); ip += 256) {
+		status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
 		TEST_LPM_ASSERT(status == 0);
 
 		status = rte_lpm_lookup(lpm, ip, &next_hop_return);
 		TEST_LPM_ASSERT((status == 0) &&
-				(next_hop_return == next_hop_add));
+				(next_hop_return == next_hop_base + ip));
+
+		ipx4 = vect_set_epi32(ip + 3, ip + 2, ip + 1, ip);
+		rte_lpm_lookupx4(lpm, ipx4, hop, UINT32_MAX);
+		TEST_LPM_ASSERT(hop[0] == next_hop_base + ip);
 	}
 
 	/* All tbl8 extensions have been used above. Try to add one more and
@@ -1037,7 +1043,7 @@  test14(void)
 	ip = RTE_IPV4(1, 0, 0, 0);
 	depth = 32;
 
-	status = rte_lpm_add(lpm, ip, depth, next_hop_add);
+	status = rte_lpm_add(lpm, ip, depth, next_hop_base + ip);
 	TEST_LPM_ASSERT(status < 0);
 
 	rte_lpm_free(lpm);