On Wed, Apr 16, 2014 at 11:49 PM, Andy Zhou <[email protected]> wrote:
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 82cfd2d..820075f 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> +static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
> +{
> + struct sw_flow_key *key = OVS_CB(skb)->pkt_key;
> + u32 hash = 0;
> +
> + /* OVS_HASH_ALG_L4 is the only possible hash algorithm. */
> + hash = skb_get_rxhash(skb);
> + if (!hash)
> + hash = 0x1;
> +
> + key->ovs_flow_hash = hash;
I just realized that this ignores the basis. Isn't that a problem?
We could potentially rehash the hash - we do this in
flow_table.c:find_bucket() although it's not a perfect solution.
> @@ -511,7 +524,7 @@ static int do_execute_actions(struct datapath *dp, struct
> sk_buff *skb,
> const struct nlattr *attr, int len, bool keep_skb)
> {
> /* Every output action needs a separate clone of 'skb', but the common
> - * case is just a single output action, so that doing a clone and
> + * case is just a single output action, so that doin a clone and
This introduces a typo.
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index 5c32cd0..f464a5b 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -123,6 +123,7 @@ static bool match_validate(const struct sw_flow_match
> *match,
> | (1ULL << OVS_KEY_ATTR_ICMP)
> | (1ULL << OVS_KEY_ATTR_ICMPV6)
> | (1ULL << OVS_KEY_ATTR_ARP)
> + | (1ULL << OVS_KEY_ATTR_DP_HASH)
I don't think this should be in the list, since these are for things
that require additional validation.
> @@ -130,7 +131,6 @@ static bool match_validate(const struct sw_flow_match
> *match,
> | (1ULL << OVS_KEY_ATTR_IN_PORT)
> | (1ULL << OVS_KEY_ATTR_ETHERTYPE));
>
> - /* Check key attributes. */
I think this comment should remain.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
