netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] netdevsim: Two small fixes
@ 2020-05-21 11:46 Ido Schimmel
  2020-05-21 11:46 ` [PATCH net 1/2] netdevsim: Ensure policer drop counter always increases Ido Schimmel
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ido Schimmel @ 2020-05-21 11:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

Fix two bugs observed while analyzing regression failures.

Patch #1 fixes a bug where sometimes the drop counter of a packet trap
policer would not increase.

Patch #2 adds a missing initialization of a variable in a related
selftest.

Ido Schimmel (2):
  netdevsim: Ensure policer drop counter always increases
  selftests: netdevsim: Always initialize 'RET' variable

 drivers/net/netdevsim/dev.c                                   | 3 +--
 tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh | 4 ++++
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 1/2] netdevsim: Ensure policer drop counter always increases
  2020-05-21 11:46 [PATCH net 0/2] netdevsim: Two small fixes Ido Schimmel
@ 2020-05-21 11:46 ` Ido Schimmel
  2020-05-21 11:46 ` [PATCH net 2/2] selftests: netdevsim: Always initialize 'RET' variable Ido Schimmel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ido Schimmel @ 2020-05-21 11:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

In case the policer drop counter is retrieved when the jiffies value is
a multiple of 64, the counter will not be incremented.

This randomly breaks a selftest [1] the reads the counter twice and
checks that it was incremented:

```
TEST: Trap policer                                                  [FAIL]
	Policer drop counter was not incremented
```

Fix by always incrementing the counter by 1.

[1] tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh

Fixes: ad188458d012 ("netdevsim: Add devlink-trap policer support")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 68668a22b9dd..dc3ff0e20944 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -858,8 +858,7 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink *devlink,
 		return -EINVAL;
 
 	cnt = &nsim_dev->trap_data->trap_policers_cnt_arr[policer->id - 1];
-	*p_drops = *cnt;
-	*cnt += jiffies % 64;
+	*p_drops = (*cnt)++;
 
 	return 0;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 2/2] selftests: netdevsim: Always initialize 'RET' variable
  2020-05-21 11:46 [PATCH net 0/2] netdevsim: Two small fixes Ido Schimmel
  2020-05-21 11:46 ` [PATCH net 1/2] netdevsim: Ensure policer drop counter always increases Ido Schimmel
@ 2020-05-21 11:46 ` Ido Schimmel
  2020-05-21 19:20 ` [PATCH net 0/2] netdevsim: Two small fixes Jakub Kicinski
  2020-05-22 23:05 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Ido Schimmel @ 2020-05-21 11:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

The variable is used by log_test() to check if the test case completely
successfully or not. In case it is not initialized at the start of a
test case, it is possible for the test case to fail despite not
encountering any errors.

Example:

```
...
TEST: Trap group statistics                                         [ OK ]
TEST: Trap policer                                                  [FAIL]
	Policer drop counter was not incremented
TEST: Trap policer binding                                          [FAIL]
	Policer drop counter was not incremented
```

Failure of trap_policer_test() caused trap_policer_bind_test() to fail
as well.

Fix by adding missing initialization of the variable.

Fixes: 5fbff58e27a1 ("selftests: netdevsim: Add test cases for devlink-trap policers")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
index dbd1e014ba17..da49ad2761b5 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
@@ -264,6 +264,8 @@ trap_policer_test()
 	local packets_t0
 	local packets_t1
 
+	RET=0
+
 	if [ $(devlink_trap_policers_num_get) -eq 0 ]; then
 		check_err 1 "Failed to dump policers"
 	fi
@@ -328,6 +330,8 @@ trap_group_check_policer()
 
 trap_policer_bind_test()
 {
+	RET=0
+
 	devlink trap group set $DEVLINK_DEV group l2_drops policer 1
 	check_err $? "Failed to bind a valid policer"
 	if [ $(devlink_trap_group_policer_get "l2_drops") -ne 1 ]; then
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net 0/2] netdevsim: Two small fixes
  2020-05-21 11:46 [PATCH net 0/2] netdevsim: Two small fixes Ido Schimmel
  2020-05-21 11:46 ` [PATCH net 1/2] netdevsim: Ensure policer drop counter always increases Ido Schimmel
  2020-05-21 11:46 ` [PATCH net 2/2] selftests: netdevsim: Always initialize 'RET' variable Ido Schimmel
@ 2020-05-21 19:20 ` Jakub Kicinski
  2020-05-22 23:05 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2020-05-21 19:20 UTC (permalink / raw)
  To: Ido Schimmel, Ido Schimmel; +Cc: netdev, davem, jiri, mlxsw

On Thu, 21 May 2020 14:46:15 +0300 Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> Fix two bugs observed while analyzing regression failures.
> 
> Patch #1 fixes a bug where sometimes the drop counter of a packet trap
> policer would not increase.
> 
> Patch #2 adds a missing initialization of a variable in a related
> selftest.

Acked-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net 0/2] netdevsim: Two small fixes
  2020-05-21 11:46 [PATCH net 0/2] netdevsim: Two small fixes Ido Schimmel
                   ` (2 preceding siblings ...)
  2020-05-21 19:20 ` [PATCH net 0/2] netdevsim: Two small fixes Jakub Kicinski
@ 2020-05-22 23:05 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2020-05-22 23:05 UTC (permalink / raw)
  To: idosch; +Cc: netdev, kuba, jiri, mlxsw, idosch

From: Ido Schimmel <idosch@idosch.org>
Date: Thu, 21 May 2020 14:46:15 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> Fix two bugs observed while analyzing regression failures.
> 
> Patch #1 fixes a bug where sometimes the drop counter of a packet trap
> policer would not increase.
> 
> Patch #2 adds a missing initialization of a variable in a related
> selftest.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-05-22 23:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 11:46 [PATCH net 0/2] netdevsim: Two small fixes Ido Schimmel
2020-05-21 11:46 ` [PATCH net 1/2] netdevsim: Ensure policer drop counter always increases Ido Schimmel
2020-05-21 11:46 ` [PATCH net 2/2] selftests: netdevsim: Always initialize 'RET' variable Ido Schimmel
2020-05-21 19:20 ` [PATCH net 0/2] netdevsim: Two small fixes Jakub Kicinski
2020-05-22 23:05 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).