[ovs-dev] [PATCHv2 1/2] ofproto-dpif-xlate: Fix bitwise ops on ct_mark.

Joe Stringer joe at ovn.org
Mon Apr 4 18:20:18 UTC 2016


On 30 March 2016 at 19:55, Ben Pfaff <blp at 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 <joe at 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 <blp at 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.



More information about the dev mailing list