[ovs-dev] [recirc datapath V4 4/5] datapath: add hash action

Andy Zhou azhou at nicira.com
Fri Apr 18 19:39:35 UTC 2014


The incremental change follows:

diff --git a/datapath/actions.c b/datapath/actions.c
index fdcd576..921310a 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -468,10 +468,11 @@ static void execute_hash(struct sk_buff *skb,
const struct nlattr *attr)

     /* OVS_HASH_ALG_L4 is the only possible hash algorithm.  */
     hash = skb_get_rxhash(skb);
+    hash = jhash_1word(hash, hash_act->hash_basis);
     if (!hash)
         hash = 0x1;

-    key->ovs_flow_hash = jhash_1word(hash, hash_act->hash_basis);
+    key->ovs_flow_hash = hash;
 }

 static int execute_set_action(struct sk_buff *skb,
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 3bee04d..873c42b 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -128,7 +128,6 @@ static bool match_validate(const struct
sw_flow_match *match,
     /* Always allowed mask fields. */
     mask_allowed |= ((1ULL << OVS_KEY_ATTR_TUNNEL)
                | (1ULL << OVS_KEY_ATTR_IN_PORT)
-               | (1ULL << OVS_KEY_ATTR_DP_HASH)
                | (1ULL << OVS_KEY_ATTR_RECIRC_ID)
                | (1ULL << OVS_KEY_ATTR_ETHERTYPE));


On Fri, Apr 18, 2014 at 11:42 AM, Andy Zhou <azhou at nicira.com> wrote:
> On Fri, Apr 18, 2014 at 10:52 AM, Jesse Gross <jesse at nicira.com> wrote:
>> On Fri, Apr 18, 2014 at 10:39 AM, Andy Zhou <azhou at nicira.com> wrote:
>>> On Fri, Apr 18, 2014 at 9:28 AM, Jesse Gross <jesse at nicira.com> wrote:
>>>> I don't understand the other change about accepting a hash mask even
>>>> when there isn't a hash value. Can you explain this?
>>> It is not necessary now. I was thinking of the case Pravin suggested that
>>> we do a  masked action.  A hash value may be zero after applying a mask.
>>> In this case, the hash value won't be exported on a post recirculation miss, but
>>> its mask can be supplied on flow installation. We can remove it if this is not
>>> the direction we want to go.
>>
>> I don't know if Pravin was actually suggesting a maskable action - his
>> questions seemed more on the match side. I can't really come up with a
>> use case either.
>>
> Thanks for clarifying. I will remove it from this patch since there is
> not a clear
> use case to allow dp_hash mask alone.
>
>> I think that the value being special/disallowed in the hash should be
>> entirely internal to the kernel. In this case it's fine to do it
>> because the hash is opaque anyways, so who's so say that we just don't
>> have a hash with this property. However, I think if we started getting
>> into cases like this where a zero value might come up then we should
>> probably either use a bit to indicate that the hash is valid or always
>> send it.
>
> That's not a bad idea. I some how don't like sending 'new' attributes
> to an older
> user space, even if it will not cause harm because of the
> compatibility layer as your
> previous email suggested. We could go down this road if you think this
> is significantly
> better than removing it.



More information about the dev mailing list