On 19/05/2016 21:02, "Ben Pfaff" <b...@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 <u9012...@gmail.com>
>> Signed-off-by: Daniele Di Proietto <diproiet...@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 <b...@ovn.org>
and pushed this to master.
Thanks,
Daniele
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
