> -----Original Message-----
> From: Amber, Kumar <kumar.am...@intel.com>
> Sent: Tuesday, May 24, 2022 12:39 PM
> To: ovs-dev@openvswitch.org
> Cc: f...@sysclose.org; echau...@redhat.com; i.maxim...@ovn.org; Van Haaren,
> Harry <harry.van.haa...@intel.com>; Ferriter, Cian <cian.ferri...@intel.com>;
> Stokes, Ian <ian.sto...@intel.com>; Amber, Kumar <kumar.am...@intel.com>
> 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 <kumar.am...@intel.com>
> Signed-off-by: Cian Ferriter <cian.ferri...@intel.com>
> Co-authored-by: Cian Ferriter <cian.ferri...@intel.com>

<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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to