> From: Kumar Amber <[email protected]>
>
> This patch adds error checking of packet hashes to the mfex
> autovalidator infrastructure, ensuring that hashes calculated by
> optimized mfex implementations is identical to the scalar code.
>
> This patch avoids calculating the software hash of the packet again
> if the optimized miniflow-extract hit and has already calculated the
> packet hash. In cases of scalar miniflow extract, the normal hashing
> calculation is performed.
>
> Signed-off-by: Kumar Amber <[email protected]>
> Signed-off-by: Harry van Haaren <[email protected]>
Thanks for the patch Harry/Amber, few queries below.
>
> ---
>
> v5:
> - Always use SW hashing to validate optimized hash implementations
> ---
> lib/dpif-netdev-avx512.c | 6 +++---
> lib/dpif-netdev-private-extract.c | 19 +++++++++++++++++++
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/lib/dpif-netdev-avx512.c b/lib/dpif-netdev-avx512.c
> index b7131ba3f..c68b79f6b 100644
> --- a/lib/dpif-netdev-avx512.c
> +++ b/lib/dpif-netdev-avx512.c
> @@ -212,15 +212,15 @@ dp_netdev_input_outer_avx512(struct
> dp_netdev_pmd_thread *pmd,
> if (!mfex_hit) {
> /* Do a scalar miniflow extract into keys. */
> miniflow_extract(packet, &key->mf);
> + key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
> + key->hash = dpif_netdev_packet_get_rss_hash_orig_pkt(packet,
> + &key->mf);
So I'm not sure, but has there been any investigation into the effect of only
storing this info when !mfex_hit occurs?
Prior to this these values were stored regardless. My concern here is that is
there a case where this info is needed even if the mfex_hit is true?
> }
>
> /* Cache TCP and byte values for all packets. */
> pkt_meta[i].bytes = dp_packet_size(packet);
> pkt_meta[i].tcp_flags = miniflow_get_tcp_flags(&key->mf);
>
> - key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
> - key->hash = dpif_netdev_packet_get_rss_hash_orig_pkt(packet, &key-
> >mf);
> -
> if (emc_enabled) {
> f = emc_lookup(&cache->emc_cache, key);
>
> diff --git a/lib/dpif-netdev-private-extract.c
> b/lib/dpif-netdev-private-extract.c
> index a29bdcfa7..2957c0172 100644
> --- a/lib/dpif-netdev-private-extract.c
> +++ b/lib/dpif-netdev-private-extract.c
> @@ -252,8 +252,15 @@ dpif_miniflow_extract_autovalidator(struct
> dp_packet_batch *packets,
>
> /* Run scalar miniflow_extract to get default result. */
> DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
> +
> + /* remove the NIC RSS bit to force SW hashing for validation. */
Minor, Capitalize Remove.
> + dp_packet_reset_offload(packet);
> +
Is there any performance penalty for forcing this reset each time?
Ian
> pkt_metadata_init(&packet->md, in_port);
> miniflow_extract(packet, &keys[i].mf);
> + keys[i].len = netdev_flow_key_size(miniflow_n_values(&keys[i].mf));
> + keys[i].hash = dpif_netdev_packet_get_rss_hash_orig_pkt(packet,
> + &keys[i].mf);
>
> /* Store known good metadata to compare with optimized metadata. */
> good_l2_5_ofs[i] = packet->l2_5_ofs;
> @@ -271,7 +278,10 @@ dpif_miniflow_extract_autovalidator(struct
> dp_packet_batch *packets,
> /* Reset keys and offsets before each implementation. */
> memset(test_keys, 0, keys_size * sizeof(struct netdev_flow_key));
> DP_PACKET_BATCH_FOR_EACH (i, packet, packets) {
> + /* Ensure offsets is set by the opt impl. */
> dp_packet_reset_offsets(packet);
> + /* Ensure packet hash is re-calculated by opt impl. */
> + dp_packet_reset_offload(packet);
> }
> /* Call optimized miniflow for each batch of packet. */
> uint32_t hit_mask = mfex_impls[j].extract_func(packets, test_keys,
> @@ -303,6 +313,15 @@ dpif_miniflow_extract_autovalidator(struct
> dp_packet_batch *packets,
> failed = 1;
> }
>
> + /* Check hashes are equal. */
> + if ((keys[i].hash != test_keys[i].hash) ||
> + (keys[i].len != test_keys[i].len)) {
> + ds_put_format(&log_msg, "Good hash: %d len: %d\tTest hash:%d"
> + " len:%d\n", keys[i].hash, keys[i].len,
> + test_keys[i].hash, test_keys[i].len);
> + failed = 1;
> + }
> +
> 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);
> --
> 2.25.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
