| Message ID | 20241119213202.2884639-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RESEND] net: microchip: vcap: Add typegroup table terminators in kunit tests | expand |
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>
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
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
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);
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(-)