On 4 April 2016 at 11:20, Joe Stringer <j...@ovn.org> wrote:
> On 30 March 2016 at 19:55, Ben Pfaff <b...@ovn.org> wrote:
>> On Thu, Mar 31, 2016 at 12:21:13AM +1300, Joe Stringer wrote:
>>> Using the action ct(commit,set_field:0x1/0x1->ct_mark), ie, specifying a
>>> mask, would previously overwrite the entire ct_mark field rather than
>>> modifying only the specified bits. Fix the issue.
>>>
>>> Fixes: 8e53fe8cf7a1 ("Add connection tracking mark support.")
>>> Signed-off-by: Joe Stringer <j...@ovn.org>
>>> ---
>>> v2: Remove wildcards from put_ct_mark().
>>
>> I didn't test this but it looks good and compiles cleanly.
>>
>> A possible (though unnecessary) enhancement to the test would be to
>> verify that setting a bit to 0 also works.
>>
>> Acked-by: Ben Pfaff <b...@ovn.org>
>
> Thanks for the review.
>
> Actually, the "set-0" test is necessary and reveals that this series
> isn't really targeting the core issue.
>
> I was a bit confused about how 'wc' were being used and what's the
> 'correct' way to handle these. After discussion with Jarno I
> understand that the 'wc' represents both the incoming match on a given
> field, but also the bits that have been modified in a 'set_field'
> action. Furthermore, as soon as a 'set_field' action is translated,
> the field is fully masked. This is actually what leads to the
> incorrect behaviour for setting the ct_mark/ct_labels. Even when
> modifying just a single bit of a field, the entire field is marked to
> change in the 'wc'.
>
> I'm looking at preparing a patch which will only mask the bits which
> are modified by the set_field action.I should add that this bug only affects bitmask generation when performing ct(...,exec(set_field...)) on packets that haven't yet been through the connection tracker, eg: in_port=1,actions=ct(commit,exec(set_field(0x1/0x1->ct_mark))),2 in_port=2,actions=ct(commit,exec(set_field(0x2/0x2->ct_mark))),1 Connections flowing through this pipeline will constantly flip between having ct_mark=0x1 and ct_mark=0x2, rather than having ct_mark=0x3. I intend to drop this series in favour of a different fix. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
