Message ID | 20200122213118.65988-1-manoj.sharma@nutanix.com |
---|---|
Headers | show |
Series | Forwarding group to load balance l2 traffic with liveness detection | expand |
On Thu, Jan 23, 2020 at 3:01 AM Manoj Sharma <manoj.sharma@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thank you Numan! On 1/23/20, 11:02 AM, "Numan Siddique" <numans@ovn.org> wrote: On Thu, Jan 23, 2020 at 3:01 AM Manoj Sharma <manoj.sharma@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@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=9SN4tlEcxQzh-CvuC81YSi9I6ERNBOWQbg-05pnXlNw&m=Mwhcw_ywerAjnoz8yYQVTzJMtVkMFOUXRcMzohPzfVA&s=wS_6OFFH-5eolkapo5R8CbnAsR77EfiCRzIMhR7AEXk&e= >