diff mbox

validation: pktio: remove octet check from stats test

Message ID 1459873002-31367-1-git-send-email-zoltan.kiss@linaro.org
State Superseded
Headers show

Commit Message

Zoltan Kiss April 5, 2016, 4:16 p.m. UTC 
This test sets up two interface and connect them to each other, so in
theory these two numbers should be the same. However when you use a pktio
which doesn't have full control of the interface, it could happen that
other players, e.g. various services of the operating system start to
send traffic out on the newly created interfaces. It won't be visible
for ODP when going out, but coming in it will increase the counters.
This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does not,
because it checks the system level statistics, not the ODP level ones.

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---

Comments

Zoltan Kiss April 6, 2016, 10:43 a.m. UTC | #1 
On 05/04/16 17:16, Zoltan Kiss wrote:
> This test sets up two interface and connect them to each other, so in

s/interface/interfaces/

Maxim, could you fix that when commit? (assuming the patch is OK)

And an another note: Maxim told me when I brought this up first to 
disable stuff, and I haven't replied then. The problem is you can't 
possibly disable all OS services which tries to hook into the interface 
up notification, because it is a moving target.

> theory these two numbers should be the same. However when you use a pktio
> which doesn't have full control of the interface, it could happen that
> other players, e.g. various services of the operating system start to
> send traffic out on the newly created interfaces. It won't be visible
> for ODP when going out, but coming in it will increase the counters.
> This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does not,
> because it checks the system level statistics, not the ODP level ones.
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
> ---
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index cb403a6..73b702c 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)
>   		CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
>   			  (stats[1].in_ucast_pkts >= (uint64_t)pkts));
>   		CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts);
> -		CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
>   		CU_ASSERT((stats[0].out_octets == 0) ||
>   			  (stats[0].out_octets >=
>   			  (PKT_LEN_NORMAL * (uint64_t)pkts)));
>
Zoltan Kiss April 14, 2016, 11:56 a.m. UTC | #2 
Ping

On 06/04/16 11:43, Zoltan Kiss wrote:
>
>
> On 05/04/16 17:16, Zoltan Kiss wrote:
>> This test sets up two interface and connect them to each other, so in
>
> s/interface/interfaces/
>
> Maxim, could you fix that when commit? (assuming the patch is OK)
>
> And an another note: Maxim told me when I brought this up first to
> disable stuff, and I haven't replied then. The problem is you can't
> possibly disable all OS services which tries to hook into the interface
> up notification, because it is a moving target.
>
>> theory these two numbers should be the same. However when you use a pktio
>> which doesn't have full control of the interface, it could happen that
>> other players, e.g. various services of the operating system start to
>> send traffic out on the newly created interfaces. It won't be visible
>> for ODP when going out, but coming in it will increase the counters.
>> This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does
>> not,
>> because it checks the system level statistics, not the ODP level ones.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>> ---
>> diff --git a/test/validation/pktio/pktio.c
>> b/test/validation/pktio/pktio.c
>> index cb403a6..73b702c 100644
>> --- a/test/validation/pktio/pktio.c
>> +++ b/test/validation/pktio/pktio.c
>> @@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)
>>           CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
>>                 (stats[1].in_ucast_pkts >= (uint64_t)pkts));
>>           CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts);
>> -        CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
>>           CU_ASSERT((stats[0].out_octets == 0) ||
>>                 (stats[0].out_octets >=
>>                 (PKT_LEN_NORMAL * (uint64_t)pkts)));
>>
Bill Fischofer April 14, 2016, 10:12 p.m. UTC | #3 
On Tue, Apr 5, 2016 at 11:16 AM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote:

> This test sets up two interface and connect them to each other, so in

> theory these two numbers should be the same. However when you use a pktio

> which doesn't have full control of the interface, it could happen that

> other players, e.g. various services of the operating system start to

> send traffic out on the newly created interfaces. It won't be visible

> for ODP when going out, but coming in it will increase the counters.

> This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does not,

> because it checks the system level statistics, not the ODP level ones.

>

> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>

>


Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org>



> ---

> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c

> index cb403a6..73b702c 100644

> --- a/test/validation/pktio/pktio.c

> +++ b/test/validation/pktio/pktio.c

> @@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)

>                 CU_ASSERT((stats[1].in_ucast_pkts == 0) ||

>                           (stats[1].in_ucast_pkts >= (uint64_t)pkts));

>                 CU_ASSERT(stats[0].out_ucast_pkts ==

> stats[1].in_ucast_pkts);

> -               CU_ASSERT(stats[0].out_octets == stats[1].in_octets);

>                 CU_ASSERT((stats[0].out_octets == 0) ||

>                           (stats[0].out_octets >=

>                           (PKT_LEN_NORMAL * (uint64_t)pkts)));

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
Maxim Uvarov April 15, 2016, 1:45 p.m. UTC | #4 
On 04/05/16 19:16, Zoltan Kiss wrote:
> This test sets up two interface and connect them to each other, so in
> theory these two numbers should be the same. However when you use a pktio
> which doesn't have full control of the interface, it could happen that
> other players, e.g. various services of the operating system start to
> send traffic out on the newly created interfaces. It won't be visible
> for ODP when going out, but coming in it will increase the counters.
> This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does not,
> because it checks the system level statistics, not the ODP level ones.
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
> ---
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index cb403a6..73b702c 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)
>   		CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
>   			  (stats[1].in_ucast_pkts >= (uint64_t)pkts));
that is strange that if:
>   		CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts);
passes and this:
> -		CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
is not.

number of packet should be linked to number of bytes. Might be skip this 
test in pktio_check_statistics_counters()
if we see some traffic after start?

Maxim.
>   		CU_ASSERT((stats[0].out_octets == 0) ||
>   			  (stats[0].out_octets >=
>   			  (PKT_LEN_NORMAL * (uint64_t)pkts)));
Zoltan Kiss April 21, 2016, 6:43 p.m. UTC | #5 
On 15/04/16 14:45, Maxim Uvarov wrote:
> On 04/05/16 19:16, Zoltan Kiss wrote:
>> This test sets up two interface and connect them to each other, so in
>> theory these two numbers should be the same. However when you use a pktio
>> which doesn't have full control of the interface, it could happen that
>> other players, e.g. various services of the operating system start to
>> send traffic out on the newly created interfaces. It won't be visible
>> for ODP when going out, but coming in it will increase the counters.
>> This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does
>> not,
>> because it checks the system level statistics, not the ODP level ones.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
>> ---
>> diff --git a/test/validation/pktio/pktio.c
>> b/test/validation/pktio/pktio.c
>> index cb403a6..73b702c 100644
>> --- a/test/validation/pktio/pktio.c
>> +++ b/test/validation/pktio/pktio.c
>> @@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)
>>           CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
>>                 (stats[1].in_ucast_pkts >= (uint64_t)pkts));
> that is strange that if:
>>           CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts);
> passes and this:
>> -        CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
> is not.
>
> number of packet should be linked to number of bytes. Might be skip this
> test in pktio_check_statistics_counters()
> if we see some traffic after start?

It's actually because ODP-DPDK sets ucast_pkts counters to 0, because it 
doesn't have a separate counter for unicast packets. I'll resend

>
> Maxim.
>>           CU_ASSERT((stats[0].out_octets == 0) ||
>>                 (stats[0].out_octets >=
>>                 (PKT_LEN_NORMAL * (uint64_t)pkts)));
>
diff mbox

Patch

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index cb403a6..73b702c 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -1256,7 +1256,6 @@  void pktio_test_statistics_counters(void)
 		CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
 			  (stats[1].in_ucast_pkts >= (uint64_t)pkts));
 		CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts);
-		CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
 		CU_ASSERT((stats[0].out_octets == 0) ||
 			  (stats[0].out_octets >=
 			  (PKT_LEN_NORMAL * (uint64_t)pkts)));