[ovs-dev] [PATCH 1/3] ofproto-dpif-rid: Include action length as part of hash.
Justin Pettit
jpettit at ovn.org
Thu Jul 27 23:21:42 UTC 2017
> On Jul 27, 2017, at 4:17 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> On Thu, Jul 27, 2017 at 02:05:22PM -0700, Justin Pettit wrote:
>>
>>> On Jul 27, 2017, at 1:54 PM, Ben Pfaff <blp at ovn.org> wrote:
>>>
>>> On Thu, Jul 13, 2017 at 11:30:49PM -0700, Justin Pettit wrote:
>>>> Signed-off-by: Justin Pettit <jpettit at ovn.org>
>>>> ---
>>>> ofproto/ofproto-dpif-rid.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
>>>> index d546b150b938..26c2357007b2 100644
>>>> --- a/ofproto/ofproto-dpif-rid.c
>>>> +++ b/ofproto/ofproto-dpif-rid.c
>>>> @@ -146,6 +146,7 @@ frozen_state_hash(const struct frozen_state *state)
>>>> hash = hash_bytes64(ALIGNED_CAST(const uint64_t *, state->action_set),
>>>> state->action_set_len, hash);
>>>> }
>>>> + hash = hash_int(state->ofpacts_len, hash);
>>>> if (state->ofpacts_len) {
>>>> hash = hash_bytes64(ALIGNED_CAST(const uint64_t *, state->ofpacts),
>>>> state->ofpacts_len, hash);
>>>
>>> Can you explain the benefit of this change? hash_bytes64() already uses
>>> the number of bytes hashed as one of the inputs to the hash.
>>
>> hash_bytes64() is only called if the action length is non-zero.
>
> Yes, but even so, the hash should still distinguish states with no
> actions from states with actions.
Yes, I agree.
>> However, I was on the fence about making this change, since it wasn't
>> clear if it would be that valuable. The main reason was just to make
>> it consistent with how "action_set" is handled right above it.
>>
>> I'm happy to drop the patch if you prefer.
>
> I'd be more inclined to remove the hash of action_set_len, because it is
> unnecessary for the same reason that hash of ofpacts_len is unnecessary.
I'll go ahead and do that. I'll send out a v2, since I need to respin the next patch in the series due to a build issue on some versions of gcc.
--Justin
More information about the dev
mailing list