[ovs-dev] [PATCH v4 ovn 0/2] Forwarding group to load balance l2 traffic with liveness detection

Numan Siddique numans at ovn.org
Thu Jan 23 19:02:17 UTC 2020


On Thu, Jan 23, 2020 at 3:01 AM Manoj Sharma <manoj.sharma at nutanix.com> wrote:
>
> *** BLURB HERE ***
>
> Manoj Sharma (2):
>   Re-apply the v3 to resolve the failure reported by 0-day robot.
>   A forwarding group is an aggregation of logical switch ports of a
>     logical switch to load balance traffic across the ports. It also
>     detects     the liveness if the logical switch ports are realized as
>     OVN tunnel ports     on the physical topology.
>
>  NEWS                      |   1 +
>  controller/lflow.c        |  20 ++++
>  controller/physical.c     |  13 +++
>  controller/physical.h     |   4 +
>  include/ovn/actions.h     |  19 +++-
>  lib/actions.c             | 142 ++++++++++++++++++++++++++
>  northd/ovn-northd.8.xml   |  49 +++++++++
>  northd/ovn-northd.c       |  64 ++++++++++++
>  ovn-nb.ovsschema          |  18 +++-
>  ovn-nb.xml                |  35 +++++++
>  ovn-sb.xml                |  19 ++++
>  tests/ovn-nbctl.at        |  37 +++++++
>  tests/ovn.at              | 124 +++++++++++++++++++++++
>  utilities/ovn-nbctl.8.xml |  38 +++++++
>  utilities/ovn-nbctl.c     | 253 ++++++++++++++++++++++++++++++++++++++++++++++
>  utilities/ovn-trace.c     |   3 +
>  16 files changed, 836 insertions(+), 3 deletions(-)

Hi Manoj,

Thanks for the patches.. I applied this series to master with the below changes.
Patch 1 had code which belonged to patch 2. So I moved that to patch 2.
Without this the tests were failing after applying the first patch.
Test cases for fwd_group action were missing in the "action parsing".
I added few cases for that.

Patch 1 changes
************************
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index bcb320be9..3628d352b 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -766,45 +766,6 @@ output;
         </p>
       </li>

-      <li>
-        <p>
-          For each <var>VIP</var> configured in the table
-          <ref table="Forwarding_Group" db="OVN_Northbound"/>
-          a priority-50 logical flow is added with the match
-          <code>arp.tpa == <var>vip</var> && && arp.op == 1
-          </code> and applies the action
-        </p>
-
-        <pre>
-eth.dst = eth.src;
-eth.src = <var>E</var>;
-arp.op = 2; /* ARP reply. */
-arp.tha = arp.sha;
-arp.sha = <var>E</var>;
-arp.tpa = arp.spa;
-arp.spa = <var>A</var>;
-outport = inport;
-flags.loopback = 1;
-output;
-        </pre>
-
-        <p>
-          where <var>E</var> is the forwarding group's mac defined in
-          the <ref column="vmac" table="Forwarding_Group"
-          db="OVN_Northbound"/>.
-        </p>
-
-        <p>
-          <var>A</var> is used as either the destination ip for load balancing
-          traffic to child ports or as nexthop to hosts behind the child ports.
-        </p>
-
-        <p>
-          These flows are required to respond to an ARP request if an ARP
-          request is sent for the IP <var>vip</var>.
-        </p>
-      </li>
-
       <li>
         One priority-0 fallback flow that matches all packets and advances to
         the next table.
@@ -1192,16 +1153,6 @@ output;
             address is only programmed on the <code>redirect-chassis</code>.
           </li>
         </ul>
-
-        <p>
-          For each forwarding group configured on the logical switch datapath,
-          a priority-50 flow that matches on <code>eth.dst == <var>VIP</var>
-          </code> with an action of <code>fwd_group(childports=<var>args
-          </var>)</code>, where <var>args</var> contains comma separated
-          logical switch child ports to load balance to.
-          If <code>liveness</code> is enabled, then action also includes
-          <code> liveness=true</code>.
-        </p>
       </li>

       <li>
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 99b62859f..7d969fb58 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Northbound",
-    "version": "5.18.0",
-    "cksum": "63300136 24879",
+    "version": "5.19.0",
+    "cksum": "4258826789 24879",
     "tables": {
         "NB_Global": {
             "columns": {
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 93bbb8618..9635dcc1d 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -1957,25 +1957,6 @@
             </dd>
           </dl>
         </dd>
-
-        <dt><code>fwd_group(<var>P</var>);</code></dt>
-        <dd>
-          <p>
-            <b>Parameters</b>: liveness, list of child ports <var>P</var>.
-          </p>
-
-          <p>
-            It load balances traffic to one or more child ports in a
-            logical switch. <code>ovn-controller</code> translates the
-            <code>fwd_group</code> into openflow group with one bucket
-            for each child port. If liveness is set to true, it also
-            integrates the bucket selection with BFD status on the tunnel
-            interface corresponding to child port.
-          </p>
-
-          <p><b>Example:</b> <code>fwd_group(liveness=true, childports=p1,p2
-            </code></p>
-        </dd>
       </dl>

       <dl>
diff --git a/tests/ovn.at b/tests/ovn.at
index b02d3768d..f8ea88d27 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -17502,127 +17502,3 @@ done
 OVN_CLEANUP([hv1])

 AT_CLEANUP
-
-AT_SETUP([ovn -- forwarding group: 3 HVs, 1 LR, 2 LS])
-AT_KEYWORDS([forwarding-group])
-ovn_start
-
-# Logical network:
-# One LR - R1 has a logical switch ls1 and ls2 connected to it.
-# Logical switch ls1 has one port while ls2 has two logical switch ports as
-# child ports.
-ovn-nbctl lr-add R1
-ovn-nbctl ls-add ls1
-ovn-nbctl ls-add ls2
-
-# Logical switch ls1 to R1 connectivity
-ovn-nbctl lrp-add R1 R1-ls1 00:00:00:01:02:f1 192.168.1.1/24
-ovn-nbctl lsp-add ls1 ls1-R1 -- set Logical_Switch_Port ls1-R1 \
-    type=router options:router-port=R1-ls1 -- lsp-set-addresses ls1-R1 router
-ovn-nbctl lsp-add ls1 lsp11 \
-    -- lsp-set-addresses lsp11 "00:00:00:01:02:01 192.168.1.2"
-
-# Logical switch ls2 to R1 connectivity
-ovn-nbctl lrp-add R1 R1-ls2 00:00:00:01:02:f2 172.16.1.1/24
-ovn-nbctl lsp-add ls2 ls2-R1 -- set Logical_Switch_Port ls2-R1 \
-    type=router options:router-port=R1-ls2 -- lsp-set-addresses ls2-R1 router
-ovn-nbctl lsp-add ls2 lsp21 \
-    -- lsp-set-addresses lsp21 "00:00:00:01:02:01 172.16.1.2"
-ovn-nbctl lsp-add ls2 lsp22 \
-    -- lsp-set-addresses lsp22 "00:00:00:01:02:02 172.16.1.3"
-
-# Create a network
-net_add n1
-
-# Create hypervisor hv1 connected to n1
-sim_add hv1
-as hv1
-ovs-vsctl add-br br-phys
-ovn_attach n1 br-phys 192.168.0.1
-ovs-vsctl add-port br-int vif1 -- set Interface vif1
external-ids:iface-id=lsp11 options:tx_pcap=hv1/vif1-tx.pcap
options:rxq_pcap=hv1/vif1-rx.pcap ofport-request=1
-
-# Create hypervisor hv2 connected to n1
-sim_add hv2
-as hv2
-ovs-vsctl add-br br-phys
-ovn_attach n1 br-phys 192.168.0.2
-ovs-vsctl add-port br-int vif2 -- set Interface vif2
external-ids:iface-id=lsp21 options:tx_pcap=hv2/vif2-tx.pcap
options:rxq_pcap=hv2/vif2-rx.pcap ofport-request=1
-
-# Create hypervisor hv3 connected to n1
-sim_add hv3
-as hv3
-ovs-vsctl add-br br-phys
-ovn_attach n1 br-phys 192.168.0.3
-ovs-vsctl add-port br-int vif3 -- set Interface vif3
external-ids:iface-id=lsp22 options:tx_pcap=hv3/vif3-tx.pcap
options:rxq_pcap=hv3/vif3-rx.pcap ofport-request=1
-
-# Add a forwarding group on ls2 with lsp21 and lsp22 as child ports
-# virtual IP - 172.16.1.11, virtual MAC - 00:11:de:ad:be:ef
-ovn-nbctl fwd-group-add fwd_grp1 ls2 172.16.1.11 00:11:de:ad:be:ef lsp21 lsp22
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-sleep 1
-
-# Check logical flow
-AT_CHECK([ovn-sbctl dump-flows | grep ls_in_l2_lkup | grep fwd_group
| wc -l], [0], [dnl
-1
-])
-
-# Check openflow rule with "group" on hypervisor
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | \
-    grep "dl_dst=00:11:de:ad:be:ef actions=group" | wc -l], [0], [dnl
-1
-])
-
-# Verify openflow group members
-for child_port in lsp21 lsp22; do
-    tunnel_key=`ovn-sbctl --bare --column tunnel_key find
port_binding logical_port=$child_port`
-    AT_CHECK([as hv1 ovs-ofctl -O OpenFlow13 dump-groups br-int | \
-        grep "bucket=actions=load:0x"$tunnel_key | wc -l], [0], [dnl
-1
-])
-done
-
-# Send a packet to virtual IP
-src_mac=00:00:00:01:02:01
-dst_mac=00:00:00:01:02:f1
-src_ip=192.168.1.2
-dst_ip=172.16.1.11
-packet="inport==\"lsp11\" && eth.src==$src_mac && eth.dst==$dst_mac &&
-        ip4 && ip.ttl==64 && ip4.src==$src_ip && ip4.dst==$dst_ip &&
-        udp && udp.src==53 && udp.dst==4369"
-as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"
-
-# Check if the packet hit the forwarding group policy
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | \
-    grep "dl_dst=00:11:de:ad:be:ef actions=group" | \
-    grep "n_packets=1" | wc -l], [0], [dnl
-1
-])
-
-# Delete the forwarding group
-ovn-nbctl fwd-group-del fwd_grp1
-
-# Add a forwarding group with liveness on ls2 with lsp21 and lsp22 as child
-# ports virtual IP - 172.16.1.11, virtual MAC - 00:11:de:ad:be:ef
-ovn-nbctl --liveness fwd-group-add fwd_grp1 ls2 172.16.1.11
00:11:de:ad:be:ef lsp21 lsp22
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-sleep 1
-
-# Verify openflow group members
-ofport_lsp21=$(as hv1 ovs-vsctl --bare --columns ofport find
Interface name=ovn-hv2-0)
-tunnel_key=`ovn-sbctl --bare --column tunnel_key find port_binding
logical_port=lsp21`
-AT_CHECK([as hv1 ovs-ofctl -O OpenFlow13 dump-groups br-int | \
-    grep "bucket=watch_port:$ofport_lsp21,actions=load:0x"$tunnel_key
| wc -l], [0], [dnl
-1
-])
-
-ofport_lsp22=$(as hv1 ovs-vsctl --bare --columns ofport find
Interface name=ovn-hv3-0)
-tunnel_key=`ovn-sbctl --bare --column tunnel_key find port_binding
logical_port=lsp22`
-AT_CHECK([as hv1 ovs-ofctl -O OpenFlow13 dump-groups br-int | \
-    grep "bucket=watch_port:$ofport_lsp22,actions=load:0x"$tunnel_key
| wc -l], [0], [dnl
-1
-])
-
-OVN_CLEANUP([hv1], [hv2], [hv3])
-AT_CLEANUP
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 5d1370adb..f5d58cc42 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4840,7 +4840,7 @@ fwd_group_by_name_or_uuid(struct ctl_context
*ctx, const char *id)
     }

     if (!fwd_group) {
-        NBREC_FORWARDING_GROUP_FOR_EACH(fwd_group, ctx->idl) {
+        NBREC_FORWARDING_GROUP_FOR_EACH (fwd_group, ctx->idl) {
             if (!strcmp(fwd_group->name, id)) {
                 break;
             }
@@ -5036,7 +5036,7 @@ fwd_group_list_all(struct ctl_context *ctx,
const char *ls_name)
     ds_put_format(s, "%-16.16s%-14.16s%-16.7s%-22.21s%s\n",
                   "FWD_GROUP", "LS", "VIP", "VMAC", "CHILD_PORTS");

-    NBREC_FORWARDING_GROUP_FOR_EACH(fwd_group, ctx->idl) {
+    NBREC_FORWARDING_GROUP_FOR_EACH (fwd_group, ctx->idl) {
         ls = fwd_group_to_logical_switch(ctx, fwd_group);
         if (!ls) {
             continue;
******************************



Patch 2 changes

****************************
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 3628d352b..bcb320be9 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -766,6 +766,45 @@ output;
         </p>
       </li>

+      <li>
+        <p>
+          For each <var>VIP</var> configured in the table
+          <ref table="Forwarding_Group" db="OVN_Northbound"/>
+          a priority-50 logical flow is added with the match
+          <code>arp.tpa == <var>vip</var> && && arp.op == 1
+          </code> and applies the action
+        </p>
+
+        <pre>
+eth.dst = eth.src;
+eth.src = <var>E</var>;
+arp.op = 2; /* ARP reply. */
+arp.tha = arp.sha;
+arp.sha = <var>E</var>;
+arp.tpa = arp.spa;
+arp.spa = <var>A</var>;
+outport = inport;
+flags.loopback = 1;
+output;
+        </pre>
+
+        <p>
+          where <var>E</var> is the forwarding group's mac defined in
+          the <ref column="vmac" table="Forwarding_Group"
+          db="OVN_Northbound"/>.
+        </p>
+
+        <p>
+          <var>A</var> is used as either the destination ip for load balancing
+          traffic to child ports or as nexthop to hosts behind the child ports.
+        </p>
+
+        <p>
+          These flows are required to respond to an ARP request if an ARP
+          request is sent for the IP <var>vip</var>.
+        </p>
+      </li>
+
       <li>
         One priority-0 fallback flow that matches all packets and advances to
         the next table.
@@ -1153,6 +1192,16 @@ output;
             address is only programmed on the <code>redirect-chassis</code>.
           </li>
         </ul>
+
+        <p>
+          For each forwarding group configured on the logical switch datapath,
+          a priority-50 flow that matches on <code>eth.dst == <var>VIP</var>
+          </code> with an action of <code>fwd_group(childports=<var>args
+          </var>)</code>, where <var>args</var> contains comma separated
+          logical switch child ports to load balance to.
+          If <code>liveness</code> is enabled, then action also includes
+          <code> liveness=true</code>.
+        </p>
       </li>

       <li>
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 9635dcc1d..93bbb8618 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -1957,6 +1957,25 @@
             </dd>
           </dl>
         </dd>
+
+        <dt><code>fwd_group(<var>P</var>);</code></dt>
+        <dd>
+          <p>
+            <b>Parameters</b>: liveness, list of child ports <var>P</var>.
+          </p>
+
+          <p>
+            It load balances traffic to one or more child ports in a
+            logical switch. <code>ovn-controller</code> translates the
+            <code>fwd_group</code> into openflow group with one bucket
+            for each child port. If liveness is set to true, it also
+            integrates the bucket selection with BFD status on the tunnel
+            interface corresponding to child port.
+          </p>
+
+          <p><b>Example:</b> <code>fwd_group(liveness=true, childports=p1,p2
+            </code></p>
+        </dd>
       </dl>

       <dl>
diff --git a/tests/ovn.at b/tests/ovn.at
index f8ea88d27..b02d3768d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -17502,3 +17502,127 @@ done
 OVN_CLEANUP([hv1])

 AT_CLEANUP
+
+AT_SETUP([ovn -- forwarding group: 3 HVs, 1 LR, 2 LS])
+AT_KEYWORDS([forwarding-group])
+ovn_start
+
+# Logical network:
+# One LR - R1 has a logical switch ls1 and ls2 connected to it.
+# Logical switch ls1 has one port while ls2 has two logical switch ports as
+# child ports.
+ovn-nbctl lr-add R1
+ovn-nbctl ls-add ls1
+ovn-nbctl ls-add ls2
+
+# Logical switch ls1 to R1 connectivity
+ovn-nbctl lrp-add R1 R1-ls1 00:00:00:01:02:f1 192.168.1.1/24
+ovn-nbctl lsp-add ls1 ls1-R1 -- set Logical_Switch_Port ls1-R1 \
+    type=router options:router-port=R1-ls1 -- lsp-set-addresses ls1-R1 router
+ovn-nbctl lsp-add ls1 lsp11 \
+    -- lsp-set-addresses lsp11 "00:00:00:01:02:01 192.168.1.2"
+
+# Logical switch ls2 to R1 connectivity
+ovn-nbctl lrp-add R1 R1-ls2 00:00:00:01:02:f2 172.16.1.1/24
+ovn-nbctl lsp-add ls2 ls2-R1 -- set Logical_Switch_Port ls2-R1 \
+    type=router options:router-port=R1-ls2 -- lsp-set-addresses ls2-R1 router
+ovn-nbctl lsp-add ls2 lsp21 \
+    -- lsp-set-addresses lsp21 "00:00:00:01:02:01 172.16.1.2"
+ovn-nbctl lsp-add ls2 lsp22 \
+    -- lsp-set-addresses lsp22 "00:00:00:01:02:02 172.16.1.3"
+
+# Create a network
+net_add n1
+
+# Create hypervisor hv1 connected to n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl add-port br-int vif1 -- set Interface vif1
external-ids:iface-id=lsp11 options:tx_pcap=hv1/vif1-tx.pcap
options:rxq_pcap=hv1/vif1-rx.pcap ofport-request=1
+
+# Create hypervisor hv2 connected to n1
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+ovs-vsctl add-port br-int vif2 -- set Interface vif2
external-ids:iface-id=lsp21 options:tx_pcap=hv2/vif2-tx.pcap
options:rxq_pcap=hv2/vif2-rx.pcap ofport-request=1
+
+# Create hypervisor hv3 connected to n1
+sim_add hv3
+as hv3
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.3
+ovs-vsctl add-port br-int vif3 -- set Interface vif3
external-ids:iface-id=lsp22 options:tx_pcap=hv3/vif3-tx.pcap
options:rxq_pcap=hv3/vif3-rx.pcap ofport-request=1
+
+# Add a forwarding group on ls2 with lsp21 and lsp22 as child ports
+# virtual IP - 172.16.1.11, virtual MAC - 00:11:de:ad:be:ef
+ovn-nbctl fwd-group-add fwd_grp1 ls2 172.16.1.11 00:11:de:ad:be:ef lsp21 lsp22
+
+# Allow some time for ovn-northd and ovn-controller to catch up.
+sleep 1
+
+# Check logical flow
+AT_CHECK([ovn-sbctl dump-flows | grep ls_in_l2_lkup | grep fwd_group
| wc -l], [0], [dnl
+1
+])
+
+# Check openflow rule with "group" on hypervisor
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | \
+    grep "dl_dst=00:11:de:ad:be:ef actions=group" | wc -l], [0], [dnl
+1
+])
+
+# Verify openflow group members
+for child_port in lsp21 lsp22; do
+    tunnel_key=`ovn-sbctl --bare --column tunnel_key find
port_binding logical_port=$child_port`
+    AT_CHECK([as hv1 ovs-ofctl -O OpenFlow13 dump-groups br-int | \
+        grep "bucket=actions=load:0x"$tunnel_key | wc -l], [0], [dnl
+1
+])
+done
+
+# Send a packet to virtual IP
+src_mac=00:00:00:01:02:01
+dst_mac=00:00:00:01:02:f1
+src_ip=192.168.1.2
+dst_ip=172.16.1.11
+packet="inport==\"lsp11\" && eth.src==$src_mac && eth.dst==$dst_mac &&
+        ip4 && ip.ttl==64 && ip4.src==$src_ip && ip4.dst==$dst_ip &&
+        udp && udp.src==53 && udp.dst==4369"
+as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"
+
+# Check if the packet hit the forwarding group policy
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | \
+    grep "dl_dst=00:11:de:ad:be:ef actions=group" | \
+    grep "n_packets=1" | wc -l], [0], [dnl
+1
+])
+
+# Delete the forwarding group
+ovn-nbctl fwd-group-del fwd_grp1
+
+# Add a forwarding group with liveness on ls2 with lsp21 and lsp22 as child
+# ports virtual IP - 172.16.1.11, virtual MAC - 00:11:de:ad:be:ef
+ovn-nbctl --liveness fwd-group-add fwd_grp1 ls2 172.16.1.11
00:11:de:ad:be:ef lsp21 lsp22
+
+# Allow some time for ovn-northd and ovn-controller to catch up.
+sleep 1
+
+# Verify openflow group members
+ofport_lsp21=$(as hv1 ovs-vsctl --bare --columns ofport find
Interface name=ovn-hv2-0)
+tunnel_key=`ovn-sbctl --bare --column tunnel_key find port_binding
logical_port=lsp21`
+AT_CHECK([as hv1 ovs-ofctl -O OpenFlow13 dump-groups br-int | \
+    grep "bucket=watch_port:$ofport_lsp21,actions=load:0x"$tunnel_key
| wc -l], [0], [dnl
+1
+])
+
+ofport_lsp22=$(as hv1 ovs-vsctl --bare --columns ofport find
Interface name=ovn-hv3-0)
+tunnel_key=`ovn-sbctl --bare --column tunnel_key find port_binding
logical_port=lsp22`
+AT_CHECK([as hv1 ovs-ofctl -O OpenFlow13 dump-groups br-int | \
+    grep "bucket=watch_port:$ofport_lsp22,actions=load:0x"$tunnel_key
| wc -l], [0], [dnl
+1
+])
+
+OVN_CLEANUP([hv1], [hv2], [hv3])
+AT_CLEANUP
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 14a615b2b..d094587a6 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5436,7 +5436,7 @@ build_fwd_group_lflows(struct ovn_datapath *od,
struct hmap *lflows)
     for (int i = 0; i < od->nbs->n_forwarding_groups; ++i) {
         const struct nbrec_forwarding_group *fwd_group = NULL;
         fwd_group = od->nbs->forwarding_groups[i];
-        if (!fwd_group || (fwd_group->n_child_port == 0)) {
+        if (!fwd_group->n_child_port) {
             continue;
         }

@@ -5480,6 +5480,9 @@ build_fwd_group_lflows(struct ovn_datapath *od,
struct hmap *lflows)
         ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 50,
                       ds_cstr(&match), ds_cstr(&actions));
     }
+
+    ds_destroy(&match);
+    ds_destroy(&actions);
 }

 static void
diff --git a/lib/actions.c b/lib/actions.c
index 6cde9ea24..a0f6aeb81 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -3052,15 +3052,16 @@ format_FWD_GROUP(const struct ovnact_fwd_group
*fwd_group, struct ds *s)
 {
     ds_put_cstr(s, "fwd_group(");
     if (fwd_group->liveness) {
-        ds_put_cstr(s, "liveness=true,");
+        ds_put_cstr(s, "liveness=\"true\", ");
     }
     if (fwd_group->n_child_ports) {
+        ds_put_cstr(s, "childports=");
         for (size_t i = 0; i < fwd_group->n_child_ports; i++) {
             if (i) {
                 ds_put_cstr(s, ", ");
             }

-            ds_put_format(s, "childports=%s", fwd_group->child_ports[i]);
+            ds_put_format(s, "\"%s\"", fwd_group->child_ports[i]);
         }
     }
     ds_put_cstr(s, ");");
diff --git a/tests/ovn.at b/tests/ovn.at
index b02d3768d..89e2b837f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1508,6 +1508,23 @@ ip.proto = select(1, 2, 3);
 reg0[0..14] = select(1, 2, 3);
     cannot use 15-bit field reg0[0..14] for "select", which requires
at least 16 bits.

+fwd_group(liveness="true", childports="eth0", "lsp1");
+    encodes as group:6
+    uses group: id(6),
name(type=select,selection_method=dp_hash,bucket=watch_port:5,load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=watch_port:17,load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
+
+fwd_group(childports="eth0", "lsp1");
+    encodes as group:7
+    uses group: id(7),
name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[0..15],resubmit(,64),bucket=load=0x17->NXM_NX_REG15[0..15],resubmit(,64))
+
+fwd_group(childports=eth0);
+    Syntax error at `eth0' expecting logical switch port.
+
+fwd_group();
+    Syntax error at `)' expecting `;'.
+
+fwd_group(liveness="false", childports="eth0", "lsp1");
+    Syntax error at `"false"' expecting `,'.
+
 # Miscellaneous negative tests.
 ;
     Syntax error at `;'.
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index 119a1adef..8ef886d6e 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -251,6 +251,19 @@ lookup_port_cb(const void *ports_, const char
*port_name, unsigned int *portp)
     return true;
 }

+static bool
+lookup_tunnel_ofport(const void *ports_, const char *port_name,
+                     ofp_port_t *ofport)
+{
+    const struct simap *ports = ports_;
+    const struct simap_node *node = simap_find(ports, port_name);
+    if (!node) {
+        return false;
+    }
+    *ofport = (OVS_FORCE ofp_port_t ) node->data;
+    return true;
+}
+
 static bool
 is_chassis_resident_cb(const void *ports_, const char *port_name)
 {
@@ -1311,6 +1324,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
OVS_UNUSED)
             /* Encode the actions into OpenFlow and print. */
             const struct ovnact_encode_params ep = {
                 .lookup_port = lookup_port_cb,
+                .tunnel_ofport = lookup_tunnel_ofport,
                 .aux = &ports,
                 .is_switch = true,
                 .group_table = &group_table,
***********************************************
>
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list