net: microchip: vcap: Add typegroup table terminators in kunit tests

[RESEND PATCH] net: microchip: vcap: Add typegroup table terminators in kunit tests

Posted by Guenter Roeck 1 year, 2 months ago

VCAP API unit tests fail randomly with errors such as

   # vcap_api_iterator_init_test: EXPECTATION FAILED at drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:387
   Expected 134 + 7 == iter.offset, but
       134 + 7 == 141 (0x8d)
       iter.offset == 17214 (0x433e)
   # vcap_api_iterator_init_test: EXPECTATION FAILED at drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:388
   Expected 5 == iter.reg_idx, but
       iter.reg_idx == 702 (0x2be)
   # vcap_api_iterator_init_test: EXPECTATION FAILED at drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:389
   Expected 11 == iter.reg_bitpos, but
       iter.reg_bitpos == 15 (0xf)
   # vcap_api_iterator_init_test: pass:0 fail:1 skip:0 total:1

Comments in the code state that "A typegroup table ends with an all-zero
terminator". Add the missing terminators.

Some of the typegroups did have a terminator of ".offset = 0, .width = 0,
.value = 0,". Replace those terminators with "{ }" (no trailing ',') for
consistency and to excplicitly state "this is a terminator".

Fixes: 67d637516fa9 ("net: microchip: sparx5: Adding KUNIT test for the VCAP API")
Cc: Steen Hegelund <steen.hegelund@microchip.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
resend: forgot to copy netdev@.

 .../ethernet/microchip/vcap/vcap_api_kunit.c    | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
index 7251121ab196..16eb3de60eb6 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
@@ -366,12 +366,13 @@ static void vcap_api_iterator_init_test(struct kunit *test)
 	struct vcap_typegroup typegroups[] = {
 		{ .offset = 0, .width = 2, .value = 2, },
 		{ .offset = 156, .width = 1, .value = 0, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 	struct vcap_typegroup typegroups2[] = {
 		{ .offset = 0, .width = 3, .value = 4, },
 		{ .offset = 49, .width = 2, .value = 0, },
 		{ .offset = 98, .width = 2, .value = 0, },
+		{ }
 	};
 
 	vcap_iter_init(&iter, 52, typegroups, 86);
@@ -399,6 +400,7 @@ static void vcap_api_iterator_next_test(struct kunit *test)
 		{ .offset = 147, .width = 3, .value = 0, },
 		{ .offset = 196, .width = 2, .value = 0, },
 		{ .offset = 245, .width = 1, .value = 0, },
+		{ }
 	};
 	int idx;
 
@@ -433,7 +435,7 @@ static void vcap_api_encode_typegroups_test(struct kunit *test)
 		{ .offset = 147, .width = 3, .value = 5, },
 		{ .offset = 196, .width = 2, .value = 2, },
 		{ .offset = 245, .width = 5, .value = 27, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 
 	vcap_encode_typegroups(stream, 49, typegroups, false);
@@ -463,6 +465,7 @@ static void vcap_api_encode_bit_test(struct kunit *test)
 		{ .offset = 147, .width = 3, .value = 5, },
 		{ .offset = 196, .width = 2, .value = 2, },
 		{ .offset = 245, .width = 1, .value = 0, },
+		{ }
 	};
 
 	vcap_iter_init(&iter, 49, typegroups, 44);
@@ -489,7 +492,7 @@ static void vcap_api_encode_field_test(struct kunit *test)
 		{ .offset = 147, .width = 3, .value = 5, },
 		{ .offset = 196, .width = 2, .value = 2, },
 		{ .offset = 245, .width = 5, .value = 27, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 	struct vcap_field rf = {
 		.type = VCAP_FIELD_U32,
@@ -538,7 +541,7 @@ static void vcap_api_encode_short_field_test(struct kunit *test)
 		{ .offset = 0, .width = 3, .value = 7, },
 		{ .offset = 21, .width = 2, .value = 3, },
 		{ .offset = 42, .width = 1, .value = 1, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 	struct vcap_field rf = {
 		.type = VCAP_FIELD_U32,
@@ -608,7 +611,7 @@ static void vcap_api_encode_keyfield_test(struct kunit *test)
 	struct vcap_typegroup tgt[] = {
 		{ .offset = 0, .width = 2, .value = 2, },
 		{ .offset = 156, .width = 1, .value = 1, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 
 	vcap_test_api_init(&admin);
@@ -671,7 +674,7 @@ static void vcap_api_encode_max_keyfield_test(struct kunit *test)
 	struct vcap_typegroup tgt[] = {
 		{ .offset = 0, .width = 2, .value = 2, },
 		{ .offset = 156, .width = 1, .value = 1, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 	u32 keyres[] = {
 		0x928e8a84,
@@ -732,7 +735,7 @@ static void vcap_api_encode_actionfield_test(struct kunit *test)
 		{ .offset = 0, .width = 2, .value = 2, },
 		{ .offset = 21, .width = 1, .value = 1, },
 		{ .offset = 42, .width = 1, .value = 0, },
-		{ .offset = 0, .width = 0, .value = 0, },
+		{ }
 	};
 
 	vcap_encode_actionfield(&rule, &caf, &rf, tgt);
-- 
2.45.2

Re: [RESEND PATCH] net: microchip: vcap: Add typegroup table terminators in kunit tests

Posted by Jacob Keller 1 year, 2 months ago


On 11/19/2024 1:32 PM, Guenter Roeck wrote:
> VCAP API unit tests fail randomly with errors such as
> 
>    # vcap_api_iterator_init_test: EXPECTATION FAILED at drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:387
>    Expected 134 + 7 == iter.offset, but
>        134 + 7 == 141 (0x8d)
>        iter.offset == 17214 (0x433e)
>    # vcap_api_iterator_init_test: EXPECTATION FAILED at drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:388
>    Expected 5 == iter.reg_idx, but
>        iter.reg_idx == 702 (0x2be)
>    # vcap_api_iterator_init_test: EXPECTATION FAILED at drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c:389
>    Expected 11 == iter.reg_bitpos, but
>        iter.reg_bitpos == 15 (0xf)
>    # vcap_api_iterator_init_test: pass:0 fail:1 skip:0 total:1
> 
> Comments in the code state that "A typegroup table ends with an all-zero
> terminator". Add the missing terminators.
> 
> Some of the typegroups did have a terminator of ".offset = 0, .width = 0,
> .value = 0,". Replace those terminators with "{ }" (no trailing ',') for
> consistency and to excplicitly state "this is a terminator".
> 
> Fixes: 67d637516fa9 ("net: microchip: sparx5: Adding KUNIT test for the VCAP API")
> Cc: Steen Hegelund <steen.hegelund@microchip.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [RESEND PATCH] net: microchip: vcap: Add typegroup table terminators in kunit tests

Posted by Daniel Machon 1 year, 2 months ago

Hi Guenter,

> Comments in the code state that "A typegroup table ends with an all-zero
> terminator". Add the missing terminators.
> 
> Some of the typegroups did have a terminator of ".offset = 0, .width = 0,
> .value = 0,". Replace those terminators with "{ }" (no trailing ',') for
> consistency and to excplicitly state "this is a terminator".
> 
> Fixes: 67d637516fa9 ("net: microchip: sparx5: Adding KUNIT test for the VCAP API")
> Cc: Steen Hegelund <steen.hegelund@microchip.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> resend: forgot to copy netdev@.

You are missing the target tree in the subject - in this case it should be
'net'

Apart from that, I think the fix looks correct. In the drivers utilizing the VCAP
API, all the typegroups are "{ }" terminated - also with no trailing ','.

Thanks for fixing this!

/Daniel

Reviewed-by: Daniel Machon <daniel.machon@microchip.com>

Re: [RESEND PATCH] net: microchip: vcap: Add typegroup table terminators in kunit tests

Posted by Guenter Roeck 1 year, 2 months ago

On Wed, Nov 20, 2024 at 10:52:02AM +0000, Daniel Machon wrote:
> Hi Guenter,
> 
> > Comments in the code state that "A typegroup table ends with an all-zero
> > terminator". Add the missing terminators.
> > 
> > Some of the typegroups did have a terminator of ".offset = 0, .width = 0,
> > .value = 0,". Replace those terminators with "{ }" (no trailing ',') for
> > consistency and to excplicitly state "this is a terminator".
> > 
> > Fixes: 67d637516fa9 ("net: microchip: sparx5: Adding KUNIT test for the VCAP API")
> > Cc: Steen Hegelund <steen.hegelund@microchip.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > resend: forgot to copy netdev@.
> 
> You are missing the target tree in the subject - in this case it should be
> 'net'

Sorry, I seem to be missing something. The subject starts with
"net: microchip: vcap: Add ...". How should it look like instead ?

Thanks,
Guenter

Re: [RESEND PATCH] net: microchip: vcap: Add typegroup table terminators in kunit tests

Posted by Paolo Abeni 1 year, 2 months ago

On 11/20/24 15:58, Guenter Roeck wrote:
> On Wed, Nov 20, 2024 at 10:52:02AM +0000, Daniel Machon wrote:
>> Hi Guenter,
>>
>>> Comments in the code state that "A typegroup table ends with an all-zero
>>> terminator". Add the missing terminators.
>>>
>>> Some of the typegroups did have a terminator of ".offset = 0, .width = 0,
>>> .value = 0,". Replace those terminators with "{ }" (no trailing ',') for
>>> consistency and to excplicitly state "this is a terminator".
>>>
>>> Fixes: 67d637516fa9 ("net: microchip: sparx5: Adding KUNIT test for the VCAP API")
>>> Cc: Steen Hegelund <steen.hegelund@microchip.com>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> resend: forgot to copy netdev@.
>>
>> You are missing the target tree in the subject - in this case it should be
>> 'net'
> 
> Sorry, I seem to be missing something. The subject starts with
> "net: microchip: vcap: Add ...". How should it look like instead ?

The correct subject should have been:

[PATCH net RESEND] net: microchip: vcap: Add typegroup table terminators
in kunit tests

the target tree is identified by the subj prefix - whatever is enclosed
by []:

https://elixir.bootlin.com/linux/v6.11.8/source/Documentation/process/maintainer-netdev.rst#L12

Please don't resent this patch just to address the above, but please
keep in mind for future submissions.

Thanks,

Paolo