> -----Original Message-----
> From: Amber, Kumar <[email protected]>
> Sent: Tuesday, May 24, 2022 12:39 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; Van Haaren,
> Harry <[email protected]>; Ferriter, Cian <[email protected]>;
> Stokes, Ian <[email protected]>; Amber, Kumar <[email protected]>
> Subject: [PATCH v2 2/5] dpif-netdev: Refactor hash function to own header.
>
> The refactor allows us to use hash function accross
> multiple files which was earlier restricted to
> dpif-netdev.c only.
>
> Signed-off-by: Kumar Amber <[email protected]>
> Signed-off-by: Cian Ferriter <[email protected]>
> Co-authored-by: Cian Ferriter <[email protected]>
<snip>
> -static inline uint32_t
> -dpif_netdev_packet_get_rss_hash(struct dp_packet *packet,
> - const struct miniflow *mf)
> -{
> - uint32_t hash, recirc_depth;
> -
> - if (OVS_LIKELY(dp_packet_rss_valid(packet))) {
> - hash = dp_packet_get_rss_hash(packet);
> - } else {
> - hash = miniflow_hash_5tuple(mf, 0);
> - dp_packet_set_rss_hash(packet, hash);
> - }
> -
> - /* The RSS hash must account for the recirculation depth to avoid
> - * collisions in the exact match cache */
> - recirc_depth = *recirc_depth_get_unsafe();
> - if (OVS_UNLIKELY(recirc_depth)) {
> - hash = hash_finish(hash, recirc_depth);
> - }
> - return hash;
> -}
Somehow, UBSan builds flag a "NULL pointer load" here, however it is not
reproducible
in a normal (no -fsanitize enabled) build, nor does OVS segfault due to a NULL
ptr load.
Code inspection shows that the recirc_depth value is always written to before
executing the above
code from the dpif-netdev datapath. So I think the Asan/UBSan tools are getting
confused around static
variables, TLS, and the various tricks done to optimize the access, but test
and verify please.
> -
> struct packet_batch_per_flow {
> unsigned int byte_count;
> uint16_t tcp_flags;
> @@ -8500,11 +8476,12 @@ dp_netdev_input(struct dp_netdev_pmd_thread
> *pmd,
> return 0;
> }
>
> -static void
> +int32_t
> dp_netdev_recirculate(struct dp_netdev_pmd_thread *pmd,
> struct dp_packet_batch *packets)
> {
> dp_netdev_input__(pmd, packets, true, 0);
> + return 0;
> }
Perhaps rework this function signature change into the later patch:
"dpif-netdev: Add function pointer for dpif re-circulate"?
This change is not related to refactoring hash functions, so is not expected
here.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev