On 12/14/21 10:55, Amber, Kumar wrote: > Hi Ilya, > > Reviewed and Tested the Patch looks good. Thanks for the Fixes . > >> -----Original Message----- >> From: Ilya Maximets <[email protected]> >> Sent: Tuesday, December 14, 2021 3:49 AM >> To: [email protected] >> Cc: Stokes, Ian <[email protected]>; Amber, Kumar >> <[email protected]>; Ferriter, Cian <[email protected]>; Van >> Haaren, Harry <[email protected]>; Ilya Maximets >> <[email protected]> >> Subject: [PATCH] dpif-netdev: Fix the autovalidator output for the miniflow >> extract. >> >> The autovalidator uses incorrect block count while printing the miniflow >> buffer >> from a tested implementation. This results in not printing fields that was >> incorrectly added to the miniflow or printing more of a buffer even if not >> needed. >> >> Fix that by requesting and using the correct block count. >> >> Also fixed the output formatting issues: extra spaces, characters, unclear >> relations between names and numbers due to mixed up delimiters, '%u' used for >> uint16_t, '\t' in the output, etc. >> >> Fixes: dd3f5d86d983 ("dpif-netdev: Add auto validation function for miniflow >> extract") >> Signed-off-by: Ilya Maximets <[email protected]> >> --- >> lib/dpif-netdev-private-extract.c | 26 +++++++++++++++----------- >> 1 file changed, 15 insertions(+), 11 deletions(-) >> >> diff --git a/lib/dpif-netdev-private-extract.c >> b/lib/dpif-netdev-private-extract.c >> index 7a06dbf6f..5f29861b8 100644 >> --- a/lib/dpif-netdev-private-extract.c >> +++ b/lib/dpif-netdev-private-extract.c >> @@ -295,8 +295,8 @@ dpif_miniflow_extract_autovalidator(struct >> dp_packet_batch *packets, >> if ((keys[i].mf.map.bits[0] != test_keys[i].mf.map.bits[0]) || >> (keys[i].mf.map.bits[1] != test_keys[i].mf.map.bits[1])) { >> ds_put_format(&log_msg, "Autovalidation map failed\n" >> - "Good 0x%llx 0x%llx\tTest 0x%llx" >> - " 0x%llx\n", keys[i].mf.map.bits[0], >> + "Good: 0x%llx 0x%llx Test: 0x%llx >> 0x%llx\n", >> + keys[i].mf.map.bits[0], >> keys[i].mf.map.bits[1], >> test_keys[i].mf.map.bits[0], >> test_keys[i].mf.map.bits[1]); @@ -305,13 >> +305,15 @@ >> dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, >> >> if (!miniflow_equal(&keys[i].mf, &test_keys[i].mf)) { >> uint32_t block_cnt = miniflow_n_values(&keys[i].mf); >> + uint32_t test_block_cnt = >> + miniflow_n_values(&test_keys[i].mf); >> + >> ds_put_format(&log_msg, "Autovalidation blocks failed\n" >> - "nGood hex:\n"); >> + "Good hex:\n"); >> ds_put_hex_dump(&log_msg, &keys[i].buf, block_cnt * 8, 0, >> false); >> ds_put_format(&log_msg, "Test hex:\n"); >> - ds_put_hex_dump(&log_msg, &test_keys[i].buf, block_cnt * 8, >> 0, >> - false); >> + ds_put_hex_dump(&log_msg, &test_keys[i].buf, >> + test_block_cnt * 8, 0, false); >> failed = 1; >> } >> >> @@ -320,14 +322,16 @@ dpif_miniflow_extract_autovalidator(struct >> dp_packet_batch *packets, >> (packet->l2_5_ofs != good_l2_5_ofs[i]) || >> (packet->l3_ofs != good_l3_ofs[i]) || >> (packet->l4_ofs != good_l4_ofs[i])) { >> - ds_put_format(&log_msg, "Autovalidation packet offsets >> failed" >> - "\n"); >> - ds_put_format(&log_msg, "Good offsets: l2_pad_size %u," >> - " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n", >> + ds_put_format(&log_msg, >> + "Autovalidation packet offsets failed\n"); >> + ds_put_format(&log_msg, "Good offsets: " >> + "l2_pad_size: %"PRIu16", l2_5_ofs: %"PRIu16", >> " >> + "l3_ofs: %"PRIu16", l4_ofs: %"PRIu16"\n", >> good_l2_pad_size[i], good_l2_5_ofs[i], >> good_l3_ofs[i], good_l4_ofs[i]); >> - ds_put_format(&log_msg, " Test offsets: l2_pad_size %u," >> - " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n", >> + ds_put_format(&log_msg, "Test offsets: " >> + "l2_pad_size: %"PRIu16", l2_5_ofs: %"PRIu16", >> " >> + "l3_ofs: %"PRIu16", l4_ofs: %"PRIu16"\n", >> packet->l2_pad_size, packet->l2_5_ofs, >> packet->l3_ofs, packet->l4_ofs); >> failed = 1; >> -- >> 2.31.1 > > Acked-by: Kumar Amber <[email protected]> >
Applied. Thanks! best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
