[ovs-dev] [PATCH v3 2/3] ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action.

[Guru Shetty]

On 22 March 2016 at 12:55, Russell Bryant <russell at ovn.org> wrote:

>
>
> On Tue, Mar 22, 2016 at 11:49 AM, Guru Shetty <guru at ovn.org> wrote:
>>
>> Macro was probably wrong use of word. I mean to say, something like (very
>> crude):  ct_commit(ct_label=MARK_FOR_DELETION)
>>
>> And you are only allowed to set certain values to ct_label and those
>> values only set certain bits. This will likely mean that we can share the
>> ct_mark and ct_label better across different features.
>>
>
> I thought this had to do with playing nicely with your LB series, but now
> it sounds like putting more structure around the values we use for
> ct_label.  Is that right?  Do you see other uses for ct_label coming up?
>

So with the LB series, I need to store the value of ct_label and ct_mark in
a register and then load it from there when I finally do ct_commit.
ct_label is 32 bits or one register and ct_mark is 128 bits or 4 registers
for a total of 5 registers. We do not have that many registers at all. With
this patch, one can set ct_mark or ct_label to any value in logical flows
which means that I am forced to use 5 registers. What I am saying is that
if we get rid of ct_mark from this patch and make ct_label to only be set
to one value, I can confidently use a single register for all of this.

I guess what you are saying is that even though this patch is general
purpose, we only use one bit of ct_label for ACL. But in the code of
ovn-controller, it no longer will be general purpose and I will only have
to read one bit from the set logical flow for ct_label and ignore ct_mark.
That would look odd, no?




>
> As for playing nicely with your LB series, it seems like we could already
> set a bit in a register and then use that to set ct_label in the next
> table.  I don't think that would require any changes to these patches,
> unless they go in after your LB series.
>
> Let me know if I'm missing the point.
>



>
> --
> Russell Bryant
>