On 12 April 2016 at 21:19, Ben Pfaff <b...@ovn.org> wrote:
> On Tue, Apr 12, 2016 at 02:23:47PM -0700, Joe Stringer wrote:
>> On 4 April 2016 at 14:56, Joe Stringer <j...@ovn.org> wrote:
>> > Previously, whenever a set_field() action was executed, the entire field
>> > would become masked and the entire field replaced, regardless of the
>> > mask specified in the set_field() action.
>> >
>> > In most cases this is fine, although it may lead to more specific
>> > wildcards than strictly necessary. However, in a particular case with
>> > connection tracking actions it could lead to the wrong behaviour.
>> >
>> > Unlike most OpenFlow fields, the ct_{mark,labels} fields are typically
>> > unknown until the ct(...,recirc_table=N,...) action is executed however
>> > the packet may actually belong to a connection which has a nonzero value
>> > for one of these fields. This can lead to the wrong behaviour with flows
>> > such as the following:
>> >
>> > in_port=1,ip,actions=ct(commit,exec(set_field(0x1/0x1->ct_mark))),2
>> > in_port=2,ip,actions=ct(commit,exec(set_field(0x2/0x2->ct_mark))),1
>> >
>> > Connections flowing through these actions will always update the ct_mark
>> > field stored within the conntrack table. However, rather than modifying
>> > only the specified bits (0x1 in one direction, 0x2 in the other), the
>> > entire ct_mark field will be replaced. Such connections will constantly
>> > toggle the value of ct_mark between 0x1 and 0x2, rather than becoming
>> > 0x3 and keeping that value.
>> >
>> > This commit fixes the issue by ensuring that set_field actions only
>> > modify the modified bits in the wildcards, rather than masking the
>> > entire field.
>> >
>> > Fixes: 8e53fe8cf7a1 ("Add connection tracking mark support.")
>> > Fixes: 9daf23484fb1 ("Add connection tracking label support.")
>> > Signed-off-by: Joe Stringer <j...@ovn.org>
>>
>> A couple of questions came up around this offline, which I looked into:
>>
>> * How does this work for OpenFlow versions that don't support masked
>> set_field?
>
> Is this a real issue? Open vSwitch effectively supports masked
> set_field on older versions of OpenFlow by encoding them as an
> equivalent series of one or more NXAST_REG_LOAD operations.
Not really; I think the concern was more: if a set_field mask is not
expressible in the OF protocol version, then is the mask populated
correctly with this change - ie full mask? (Yes, it is).
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
