[ovs-dev] [PATCH 4/4] dpif-netdev: Initialize packet RSS hash in dpif_netdev_execute().

[Daniele Di Proietto]

On 19/05/2016 21:02, "Ben Pfaff" <blp at ovn.org> wrote:

>On Tue, May 17, 2016 at 10:31:25PM -0700, Daniele Di Proietto wrote:
>> The datapath code expects the RSS hash to always be initialized.  This
>> is enforced by checking in emc_processing() that the hash is valid, and
>> eventually by computing a new one.
>> 
>> Unfortunately, there is another entry point to the datapath,
>> dpif_netdev_execute().  A packet generated by OVS (BFD frame,
>> packet-out from controller) doesn't have a valid RSS hash and so is
>> allowed to enter the datapath with an uninitialized hash value.
>> 
>> This commit recomputes the hash (if not valid) in dpif_netdev_execute().
>> 
>> The only place where we would use an invalid hash is netdev-vport, in
>> push_udp_header().  This caused an uninitialized memory read, and a
>> random value to be assigned to the outer tunnel header source port.
>> 
>> Reported-by: William Tu <u9012063 at gmail.com>
>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>
>This is well reasoned.
>
>Would there be any value in adding a comment explaining when the hash
>might not already be valid?

Sure, I added:

/* The action processing expects the RSS hash to be valid, because
 * it's always initialized at the beginning of datapath processing.
 * In this case, though, 'execute->packet' may not have gone through
 * the datapath at all, it may have been generated by the upper layer
 * (OpenFlow packet-out, BFD frame, ...). */

>Acked-by: Ben Pfaff <blp at ovn.org>

and pushed this to master.

Thanks,

Daniele