[ovs-dev] [PATCH v2 1/5] ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.

Jarno Rajahalme jarno at ovn.org
Wed Aug 31 18:32:12 UTC 2016


I’d put the registers and metadata field to the ‘false’ and also maybe non-writeable fields (ether type, frags, nw_proto, etc.) in to OVS_NOT_REACHED() case, just in case.

  Jarno

> On Aug 31, 2016, at 10:38 AM, Jesse Gross <jesse at kernel.org> wrote:
> 
> On Tue, Aug 30, 2016 at 6:47 PM, Daniele Di Proietto
> <diproiettod at vmware.com> wrote:
>> When translating a set action we also unwildcard the field in question.
>> This is done to correctly translate set actions with the value identical
>> to the ingress flow, like in the following example:
>> 
>> flow table:
>> 
>> tcp,actions=set_field:80->tcp_dst,output:5
>> 
>> ingress packet:
>> 
>> ...,tcp,tcp_dst=80
>> 
>> datapath flow
>> 
>> ...,tcp(dst=80) actions:5
>> 
>> The datapath flow must exact match the target field, because the actions
>> do not include a set field. (Otherwise a packet coming in with a
>> different tcp_dst would be matched, and its port wouldn't be altered).
>> 
>> Tunnel attributes behave differently: at the datapath layer, before
>> action processing they're cleared (we do the same at the ofproto layer
>> in xlate_actions()).  Therefore there's no need to unwildcard them,
>> because a set action would always be detected (since we zero them at the
>> beginning of xlate_ations()).
>> 
>> This fixes a problem related to the handling of Geneve options.
>> Unwildcarding non existing Geneve options (as done by a
>> set_field:X->tun_metadata<n> on a packet coming from a non-tunnel
>> interface) would be problematic for the datapaths: the ODP translation
>> functions cannot express a match on non existing Geneve options (unlike
>> on other attributes), and the userspace datapath wouldn't be able to
>> translate them to "userspace datapath format".  In both cases the
>> installed flow would be deleted by revalidation at the first
>> opportunity.
>> 
>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> 
> I think there might be some more obscure ways of triggering this
> problem that still exist. Basically anything that can use a register
> as a target is a potential issue. For example, stack_pop, bundle, and
> multipath all look like they have the same masking behavior.
> 
> I do have a general solution in this patch (look at the bottom of
> xlate_actions() where it is adjusting the wildcards):
> http://openvswitch.org/pipermail/dev/2016-August/078484.html
> 
> I didn't realize that it was addressing an existing issue though and
> that patch certainly isn't suitable for anything other than master.
> 
> I'm also not really a big fan of the way I handled it there since it's
> a pretty coarse way to do it. I would be happy to drop it if we can
> feel comfortable that we got all of the callsites (now and in the
> future) using your method. Perhaps we can just create a helper
> function with this check builtin and then use it everywhere? That
> might be enough to be confident about the future.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list