diff mbox

[ovs-dev,ovn-ipv6,07/26] ovn: Add xxreg[01] symbols.

Message ID 1468306616-125783-8-git-send-email-jpettit@ovn.org
State Accepted
Headers show

Commit Message

Justin Pettit July 12, 2016, 6:56 a.m. UTC
Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 ovn/controller/lflow.c |  3 +++
 ovn/ovn-sb.xml         | 12 ++++++++++++
 tests/test-ovn.c       |  3 +++
 3 files changed, 18 insertions(+)

Comments

Ben Pfaff July 12, 2016, 11:26 p.m. UTC | #1
On Mon, Jul 11, 2016 at 11:56:37PM -0700, Justin Pettit wrote:
> Signed-off-by: Justin Pettit <jpettit@ovn.org>

Acked-by: Ben Pfaff <blp@ovn.org>
Zong Kai LI July 13, 2016, 2:53 a.m. UTC | #2
> +      <p>
> +        The <code>reg</code><var>X</var> symbols are 32-bit integers.
> +        The <code>xxreg</code><var>X</var> symbols are 128-bit integers,
> +        which overlay four of the 32-bit registers: <code>xxreg0</code>
> +        overlays <code>reg0</code> through <code>reg3</code>, with
> +        <code>reg0</code> supplying the most-significant bits of
> +        <code>xxreg0</code> and <code>reg3</code> the least-signficant.
> +        <code>xxreg1</code> similarly overlays <code>reg4</code> through
> +        <code>reg7</code>.
> +      </p>
> +

I think REGBIT_CONNTRACK_** defined in ovn-northd will be covered by xxreg0.
Since you renumbered MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPUT from
MFF_REG3...MFF_REG7 to MFF_REG11...MFF_REG15. REGBIT_CONNTRACK_**
which are using MFF_REG0 should be renumbered too.

Thanks.
Zong Kai, LI
Justin Pettit July 13, 2016, 3:24 a.m. UTC | #3
> On Jul 12, 2016, at 7:53 PM, Zong Kai Li <zealokii@gmail.com> wrote:
> 
>> +      <p>
>> +        The <code>reg</code><var>X</var> symbols are 32-bit integers.
>> +        The <code>xxreg</code><var>X</var> symbols are 128-bit integers,
>> +        which overlay four of the 32-bit registers: <code>xxreg0</code>
>> +        overlays <code>reg0</code> through <code>reg3</code>, with
>> +        <code>reg0</code> supplying the most-significant bits of
>> +        <code>xxreg0</code> and <code>reg3</code> the least-signficant.
>> +        <code>xxreg1</code> similarly overlays <code>reg4</code> through
>> +        <code>reg7</code>.
>> +      </p>
>> +
> 
> I think REGBIT_CONNTRACK_** defined in ovn-northd will be covered by xxreg0.
> Since you renumbered MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPUT from
> MFF_REG3...MFF_REG7 to MFF_REG11...MFF_REG15. REGBIT_CONNTRACK_**
> which are using MFF_REG0 should be renumbered too.

I'm not sure I understand.  Those registers are that I renumbered are physical (used by ovn-controller) and not accessible by logical flows.  The REGBIT_CONNTRACK_* are only used by logical flows (in ovn-northd), so they need to be in the logical registers that are accessible by xxregX and xregX.

--Justin
Zong Kai LI July 13, 2016, 5:56 a.m. UTC | #4
>On Wed, Jul 13, 2016 at 11:24 AM, Justin Pettit <jpettit@ovn.org> wrote:
>
>> On Jul 12, 2016, at 7:53 PM, Zong Kai Li <zealokii@gmail.com> wrote:
>>
>>> +      <p>
>>> +        The <code>reg</code><var>X</var> symbols are 32-bit integers.
>>> +        The <code>xxreg</code><var>X</var> symbols are 128-bit integers,
>>> +        which overlay four of the 32-bit registers: <code>xxreg0</code>
>>> +        overlays <code>reg0</code> through <code>reg3</code>, with
>>> +        <code>reg0</code> supplying the most-significant bits of
>>> +        <code>xxreg0</code> and <code>reg3</code> the least-signficant.
>>> +        <code>xxreg1</code> similarly overlays <code>reg4</code> through
>>> +        <code>reg7</code>.
>>> +      </p>
>>> +
>>
>> I think REGBIT_CONNTRACK_** defined in ovn-northd will be covered by xxreg0.
>> Since you renumbered MFF_LOG_DNAT_ZONE...MFF_LOG_OUTPUT from
>> MFF_REG3...MFF_REG7 to MFF_REG11...MFF_REG15. REGBIT_CONNTRACK_**
>> which are using MFF_REG0 should be renumbered too.
>
> I'm not sure I understand.  Those registers are that I renumbered are physical (used by ovn-controller) and not accessible by logical flows.  The REGBIT_CONNTRACK_* are only used by logical flows (in ovn-northd), so they need to be in the logical registers that are accessible by xxregX and xregX.
>
> --Justin
>
>

Yes, REGBIT_CONNTRACK_* are only used by logical flows, just like
"inport", "outport".
But things are little different, since "inport" and "outport" are
symbols and they will be translated, while REGBIT_CONNTRACK_* are not,
they are "reg0[..]" already, ovn-controller will directly submit them
to ovs.

If a logical flow both use REGBIT_CONNTRACK_* and xxreg0, reg0 will be
called twice, the directly way and the symbol translation way.

Thanks,
Zong Kai, LI
diff mbox

Patch

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 00d1d6e..b77b364 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -66,6 +66,9 @@  lflow_init(void)
     MFF_LOG_REGS;
 #undef MFF_LOG_REG
 
+    expr_symtab_add_field(&symtab, "xxreg0", MFF_XXREG0, NULL, false);
+    expr_symtab_add_field(&symtab, "xxreg1", MFF_XXREG1, NULL, false);
+
     /* Connection tracking state. */
     expr_symtab_add_field(&symtab, "ct_mark", MFF_CT_MARK, NULL, false);
     expr_symtab_add_field(&symtab, "ct_label", MFF_CT_LABEL, NULL, false);
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 00a92e0..7b45bbb 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -738,8 +738,20 @@ 
         symbols, only names within the flow's logical datapath may be used.
       </p>
 
+      <p>
+        The <code>reg</code><var>X</var> symbols are 32-bit integers.
+        The <code>xxreg</code><var>X</var> symbols are 128-bit integers,
+        which overlay four of the 32-bit registers: <code>xxreg0</code>
+        overlays <code>reg0</code> through <code>reg3</code>, with
+        <code>reg0</code> supplying the most-significant bits of
+        <code>xxreg0</code> and <code>reg3</code> the least-signficant.
+        <code>xxreg1</code> similarly overlays <code>reg4</code> through
+        <code>reg7</code>.
+      </p>
+
       <ul>
         <li><code>reg0</code>...<code>reg9</code></li>
+        <li><code>xxreg0</code> <code>xxreg1</code></li>
         <li><code>inport</code> <code>outport</code></li>
         <li><code>eth.src</code> <code>eth.dst</code> <code>eth.type</code></li>
         <li><code>vlan.tci</code> <code>vlan.vid</code> <code>vlan.pcp</code> <code>vlan.present</code></li>
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index a3357a3..fd004c9 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -146,6 +146,9 @@  create_symtab(struct shash *symtab)
     expr_symtab_add_string(symtab, "inport", MFF_REG14, NULL);
     expr_symtab_add_string(symtab, "outport", MFF_REG15, NULL);
 
+    expr_symtab_add_field(symtab, "xxreg0", MFF_XXREG0, NULL, false);
+    expr_symtab_add_field(symtab, "xxreg1", MFF_XXREG1, NULL, false);
+
     expr_symtab_add_field(symtab, "xreg0", MFF_XREG0, NULL, false);
     expr_symtab_add_field(symtab, "xreg1", MFF_XREG1, NULL, false);
     expr_symtab_add_field(symtab, "xreg2", MFF_XREG2, NULL, false);