[ovs-dev] [recirc datapath V2 2/3] datapath: add hash action

[Andy Zhou]

On Wed, Apr 16, 2014 at 2:37 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Wed, Apr 16, 2014 at 1:54 PM, Andy Zhou <azhou at nicira.com> wrote:
>> On Wed, Apr 16, 2014 at 1:34 PM, Jesse Gross <jesse at nicira.com> wrote:
>>> On Tue, Apr 15, 2014 at 6:21 PM, Andy Zhou <azhou at nicira.com> wrote:
>>>> diff --git a/datapath/actions.c b/datapath/actions.c
>>>> index 0b66e7c..cb239c8 100644
>>>> --- a/datapath/actions.c
>>>> +++ b/datapath/actions.c
>>>> +               dp_hash = skb_get_rxhash(skb);
>>>> +               if (!dp_hash)
>>>> +                       dp_hash = 0x1;
>>>
>>> I don't know if we want to rely on this but I think the check for a
>>> zero hash is already done in skb_get_rxhash.
>> I could be wrong on this. It seems some earlier kernel may return 0 on
>> error, Recent kernels may
>> return whatever is in skb->rxhash on error.
>
> OK, I guess that's fine.
>
>>>> diff --git a/datapath/flow.h b/datapath/flow.h
>>>> index 1bb6ce0..bcc36d2 100644
>>>> --- a/datapath/flow.h
>>>> +++ b/datapath/flow.h
>>>> @@ -74,6 +74,7 @@ struct sw_flow_key {
>>>>                 u32     skb_mark;       /* SKB mark. */
>>>>                 u16     in_port;        /* Input switch port (or DP_MAX_PORTS). */
>>>>         } __packed phy; /* Safe when right after 'tun_key'. */
>>>> +       u32 dp_hash;                    /* Datapath computed hash value.  */
>>>
>>> The name "dp_hash" sounds like it is somehow a property of the
>>> datapath. What about "flow_hash" or just "hash"?
>> I would like to indicate this value is computed in datapath. I don't
>> mind changing it to
>> a better name, but flow_hash or hash sounds too generic to me.
>
> "ovs_flow_hash"?

I am fine with this. Should we use the same name for user space flow
or 'dp_hash' is fine there?