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]>

Regards
Amber

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to