[ovs-dev] [PATCH v5 13/13] datapath: Relax match_validate.

[Jesse Gross]

On Mon, Sep 22, 2014 at 3:49 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>
> On Sep 22, 2014, at 2:26 PM, Jesse Gross <jesse at nicira.com> wrote:
>
>> On Fri, Sep 5, 2014 at 4:05 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>>> When userspace inserts masked flows, it is not necessary to demand
>>> that flows matching in a known ethertype also must have the
>>> corresponding key, as a missing key will be automatically wildcarded.
>>>
>>> For example, if a drop flow dropping all UDP packets is installed,
>>> this patch allows the userspace application to not specify the
>>> OVS_KEY_ATTR_UDP key.
>>>
>>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>>
>> I guess I'm not sure that I quite understand the rationale for this
>> patch or at least how it is planned to be used.
>>
>> Since this is the reflection of the original flow that was hit, it
>> should always be possible to fully specify the key. This doesn't
>> affect the ability to mask out those fields so this change shouldn't
>> save on flow setups or anything like that. I think the only new case
>> that this enables is proactively installing flows. Is that the goal?
>
> Even though we might not do proactive flow setups now, we should not make it harder than it needs to be. I came up with this trying to test simple datapath flows with ovs-dpctl.

This would be a pretty big shift, so at a minimum I'm not really in
favor of lumping this patch in at the end of a patch series that I'm
not sure it is directly related to.

Even within the context of this patch, I'm not sure that this is
really complete - what happens when flows are serialized back up to
userspace? I think with this it would return the full flow key but
with the last elements filled with zeros. That doesn't really seem
like the correct thing to do. I also see that Ben had some questions
about the userspace implications in the patch to ovs-dpctl.

Overall though, if we go in the direction of allowing flows to be
proactively installed I would like to be a little more deliberate
about it rather than just enabling it because we can.