[ovs-dev] [PATCH ovn v2] ovn-northd: Virtual port add ND/ARP responder flows for IPv6 VIPs.
Numan Siddique
numans at ovn.org
Tue Nov 2 23:57:57 UTC 2021
On Wed, Sep 22, 2021 at 1:58 PM <mheib at redhat.com> wrote:
>
> From: Mohammad Heib <mheib at redhat.com>
>
> currently ovn-northd only handle virtual ports with VIP IPv4 and ignores
> virtual ports with VIP IPv6.
>
> This patch adds support for virtual ports with VIP IPv6 by adding
> lflows to the lsp_in_arp_rsp logical switch pipeline.
> Those lflows handle Neighbor Solicitations and Neighbor Advertisement requests
> that target the virtual port VIPs and bind the virtual port to the desired VIF.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2003091
> Fixes: 054f4c85c413 ("Add a new logical switch port type - 'virtual'")
> Signed-off-by: Mohammad Heib <mheib at redhat.com>
Hi Mohammad,
Thanks for the patch.
I didn't do a thorough review. I have just a few comments.
You need to update the documentation in northd/ovn-northd.8.xml about
the new flows added.
PSB for a couple of small nit comments.
Thanks
Numan
> ---
> northd/northd.c | 80 ++++++++++++++++++++++++++++++++++++++-----------
> tests/ovn.at | 66 ++++++++++++++++++++++++++++++++--------
> 2 files changed, 116 insertions(+), 30 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index cf2467fe1..b934819fc 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -7382,16 +7382,28 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op,
> * - ARP reply from the virtual ip which belongs to a logical
> * port of type 'virtual' and bind that port.
> * */
> - ovs_be32 ip;
> + struct in6_addr ip;
> +
> const char *virtual_ip = smap_get(&op->nbsp->options,
> "virtual-ip");
> const char *virtual_parents = smap_get(&op->nbsp->options,
> "virtual-parents");
> - if (!virtual_ip || !virtual_parents ||
> - !ip_parse(virtual_ip, &ip)) {
> + if (!virtual_ip || !virtual_parents) {
> return;
> }
>
> + bool is_ipv4 = strchr(virtual_ip, '.') ? true : false;
> + if (is_ipv4) {
> + ovs_be32 ipv4;
> + if (!ip_parse(virtual_ip, &ipv4)) {
> + return;
> + }
> + } else{
> + if (!ipv6_parse(virtual_ip, &ip)) {
> + return;
> + }
> + }
> +
> char *tokstr = xstrdup(virtual_parents);
> char *save_ptr = NULL;
> char *vparent;
> @@ -7404,13 +7416,31 @@ build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op,
> continue;
> }
>
> - ds_clear(match);
> - ds_put_format(match, "inport == \"%s\" && "
> - "((arp.op == 1 && arp.spa == %s && "
> - "arp.tpa == %s) || (arp.op == 2 && "
> - "arp.spa == %s))",
> - vparent, virtual_ip, virtual_ip,
> - virtual_ip);
> + if (is_ipv4) {
> + ds_clear(match);
> + ds_put_format(match, "inport == \"%s\" && "
> + "((arp.op == 1 && arp.spa == %s && "
> + "arp.tpa == %s) || (arp.op == 2 && "
> + "arp.spa == %s))",
> + vparent, virtual_ip, virtual_ip,
> + virtual_ip);
> + } else{
> + struct ipv6_netaddr na;
> + /* Find VIP multicast group */
> + in6_addr_solicited_node(&na.sn_addr, &ip);
> + inet_ntop(AF_INET6, &na.sn_addr, na.sn_addr_s, sizeof na.sn_addr_s);
> +
> + ds_clear(match);
> + ds_put_format(match, "inport == \"%s\" && "
> + "((nd_ns && ip6.dst == {%s, %s} && nd.target == %s) ||"
> + "(nd_na && nd.target == %s))",
> + vparent,
> + virtual_ip,
> + na.sn_addr_s,
> + virtual_ip,
> + virtual_ip);
> + }
> +
> ds_clear(actions);
> ds_put_format(actions,
> "bind_vport(%s, inport); "
> @@ -11030,17 +11060,29 @@ build_arp_resolve_flows_for_lrouter_port(
> * 00:00:00:00:00:00 and advance to next table so that ARP is
> * resolved by router pipeline using the arp{} action.
> * The MAC_Binding entry for the virtual ip might be invalid. */
> - ovs_be32 ip;
>
> const char *vip = smap_get(&op->nbsp->options,
> "virtual-ip");
> const char *virtual_parents = smap_get(&op->nbsp->options,
> "virtual-parents");
> - if (!vip || !virtual_parents ||
> - !ip_parse(vip, &ip) || !op->sb) {
> +
> + if (!vip || !virtual_parents || !op->sb) {
> return;
> }
>
> + bool is_ipv4 = strchr(vip, '.') ? true : false;
> + if (is_ipv4) {
> + ovs_be32 ipv4;
> + if (!ip_parse(vip, &ipv4)) {
> + return;
> + }
> + } else{
> + struct in6_addr ipv6;
> + if (!ipv6_parse(vip, &ipv6)) {
> + return;
> + }
> + }
> +
> if (!op->sb->virtual_parent || !op->sb->virtual_parent[0] ||
> !op->sb->chassis) {
> /* The virtual port is not claimed yet. */
> @@ -11054,8 +11096,10 @@ build_arp_resolve_flows_for_lrouter_port(
> if (find_lrp_member_ip(peer, vip)) {
> ds_clear(match);
> ds_put_format(match, "outport == %s && "
> - REG_NEXT_HOP_IPV4 " == %s",
> - peer->json_key, vip);
> + "%s == %s",
> + peer->json_key,
> + is_ipv4? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
Can you please add space before and after the '?' and ':'.
> + vip);
>
> const char *arp_actions =
> "eth.dst = 00:00:00:00:00:00; next;";
> @@ -11093,8 +11137,10 @@ build_arp_resolve_flows_for_lrouter_port(
>
> ds_clear(match);
> ds_put_format(match, "outport == %s && "
> - REG_NEXT_HOP_IPV4 " == %s",
> - peer->json_key, vip);
> + "%s == %s",
> + peer->json_key,
> + is_ipv4? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
Same as above.
Thanks
Numan
> + vip);
>
> ds_clear(actions);
> ds_put_format(actions, "eth.dst = %s; next;", ea_s);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 172b5c713..e2d4a592d 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -17831,6 +17831,11 @@ AT_SETUP([virtual ports])
> AT_KEYWORDS([virtual ports])
> ovn_start
>
> +send_nd_ns() {
> + local hv=$1 inport=$2 eth_src=$3 eth_dst=$4 ip6_src=$5 ip6_dst=$6 tgt_ip=$7
> + local request=${eth_dst}${eth_src}86dd6000000000203aff${ip6_src}${ip6_dst}870005c900000000${tgt_ip}0101${eth_src}
> + as hv$hv ovs-appctl netdev-dummy/receive hv${hv}-vif$inport $request
> +}
> send_garp() {
> local hv=$1 inport=$2 eth_src=$3 eth_dst=$4 spa=$5 tpa=$6
> local request=${eth_dst}${eth_src}08060001080006040001${eth_src}${spa}${eth_dst}${tpa}
> @@ -17899,43 +17904,52 @@ check ovn-nbctl lsp-set-type sw0-vir virtual
> check ovn-nbctl set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10
> check ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents=sw0-p1,sw0-p2,sw0-p3
>
> +
> +check ovn-nbctl lsp-add sw0 sw0-vir6
> +check ovn-nbctl lsp-set-addresses sw0-vir6 "50:54:00:00:00:11 1000::61d1"
> +check ovn-nbctl lsp-set-port-security sw0-vir6 "50:54:00:00:00:11 1000::61d1"
> +check ovn-nbctl lsp-set-type sw0-vir6 virtual
> +check ovn-nbctl set logical_switch_port sw0-vir6 options:virtual-ip=1000::61d1
> +check ovn-nbctl set logical_switch_port sw0-vir6 options:virtual-parents=sw0-p1,sw0-p2,sw0-p3
> +
> check ovn-nbctl lsp-add sw0 sw0-p1
> -check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
> -check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3 10.0.0.10"
> +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3 1000::3"
> +check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3 10.0.0.10 1000::3"
>
> check ovn-nbctl lsp-add sw0 sw0-p2
> -check ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4"
> -check ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4 10.0.0.10"
> +check ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:04 10.0.0.4 1000::4"
> +check ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:04 10.0.0.4 10.0.0.10 1000::4"
>
> check ovn-nbctl lsp-add sw0 sw0-p3
> -check ovn-nbctl lsp-set-addresses sw0-p3 "50:54:00:00:00:05 10.0.0.5"
> -check ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:05 10.0.0.5 10.0.0.10"
> +check ovn-nbctl lsp-set-addresses sw0-p3 "50:54:00:00:00:05 10.0.0.5 1000::5"
> +check ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:05 10.0.0.5 10.0.0.10 1000::5"
>
> # Create the second logical switch with one port
> check ovn-nbctl ls-add sw1
> check ovn-nbctl lsp-add sw1 sw1-p1
> -check ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3"
> -check ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3"
> +check ovn-nbctl lsp-set-addresses sw1-p1 "40:54:00:00:00:03 20.0.0.3 2000::3"
> +check ovn-nbctl lsp-set-port-security sw1-p1 "40:54:00:00:00:03 20.0.0.3 2000::3"
>
> # Create a logical router and attach both logical switches
> check ovn-nbctl lr-add lr0
> -check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::a/64
> check ovn-nbctl lsp-add sw0 sw0-lr0
> check ovn-nbctl lsp-set-type sw0-lr0 router
> check ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
> check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
>
> -check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24
> +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 2000::a/64
> check ovn-nbctl lsp-add sw1 sw1-lr0
> check ovn-nbctl lsp-set-type sw1-lr0 router
> check ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02
> check ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
>
> -# Add an ACL that matches on sw0-vir being bound locally.
> +# Add an ACL that matches on sw0-vir/sw0-vir6 being bound locally.
> check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir") && ip' allow
> +check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir6") && ip' allow
>
> check ovn-nbctl ls-add public
> -check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 2001:db8::1/64
> check ovn-nbctl lsp-add public public-lr0
> check ovn-nbctl lsp-set-type public-lr0 router
> check ovn-nbctl lsp-set-addresses public-lr0 router
> @@ -17951,13 +17965,14 @@ check ovn-nbctl lsp-set-options ln-public network_name=public
> check ovn-nbctl --wait=hv lrp-set-gateway-chassis lr0-public hv1 20
>
> check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.50 10.0.0.10 sw0-vir 10:54:00:00:00:10
> +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 2001:db8::61d1 1000::61d1 sw0-vir6 20:01:db:86:d1:10
>
> OVN_POPULATE_ARP
>
> wait_for_ports_up
> ovn-nbctl --wait=hv sync
>
> -# Check that logical flows are added for sw0-vir in lsp_in_arp_rsp pipeline
> +# Check that logical flows are added for sw0-vir/sw0vir6 in lsp_in_arp_rsp pipeline
> # with bind_vport action.
>
> ovn-sbctl dump-flows sw0 > sw0-flows
> @@ -17965,8 +17980,11 @@ AT_CAPTURE_FILE([sw0-flows])
>
> AT_CHECK([grep ls_in_arp_rsp sw0-flows | grep bind_vport | sed 's/table=../table=??/' | sort], [0], [dnl
> table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "sw0-p1" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
> + table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "sw0-p1" && ((nd_ns && ip6.dst == {1000::61d1, ff02::1:ff00:61d1} && nd.target == 1000::61d1) ||(nd_na && nd.target == 1000::61d1))), action=(bind_vport("sw0-vir6", inport); next;)
> table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "sw0-p2" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
> + table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "sw0-p2" && ((nd_ns && ip6.dst == {1000::61d1, ff02::1:ff00:61d1} && nd.target == 1000::61d1) ||(nd_na && nd.target == 1000::61d1))), action=(bind_vport("sw0-vir6", inport); next;)
> table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "sw0-p3" && ((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2 && arp.spa == 10.0.0.10))), action=(bind_vport("sw0-vir", inport); next;)
> + table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "sw0-p3" && ((nd_ns && ip6.dst == {1000::61d1, ff02::1:ff00:61d1} && nd.target == 1000::61d1) ||(nd_na && nd.target == 1000::61d1))), action=(bind_vport("sw0-vir6", inport); next;)
> ])
>
> ovn-sbctl dump-flows lr0 > lr0-flows
> @@ -17978,6 +17996,10 @@ AT_CHECK([grep lr_in_arp_resolve lr0-flows | grep "reg0 == 10.0.0.10" | sed 's/t
> table=??(lr_in_arp_resolve ), priority=100 , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 00:00:00:00:00:00; next;)
> ])
>
> +AT_CHECK([grep lr_in_arp_resolve lr0-flows | grep "xxreg0 == 1000::61d1" | sed 's/table=../table=??/'], [0], [dnl
> + table=??(lr_in_arp_resolve ), priority=100 , match=(outport == "lr0-sw0" && xxreg0 == 1000::61d1), action=(eth.dst = 00:00:00:00:00:00; next;)
> +])
> +
> hv1_ch_uuid=`ovn-sbctl --bare --columns _uuid find chassis name="hv1"`
> hv2_ch_uuid=`ovn-sbctl --bare --columns _uuid find chassis name="hv2"`
>
> @@ -17986,6 +18008,8 @@ logical_port=sw0-vir) = x], [0], [])
>
> AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \
> logical_port=sw0-vir) = x])
> +AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \
> +logical_port=sw0-vir6) = x])
>
> # Try to bind sw0-vir directly to an OVS port. This should be ignored by
> # ovn-controller.
> @@ -18036,11 +18060,22 @@ spa=$(ip_to_hex 10 0 0 10)
> tpa=$(ip_to_hex 10 0 0 10)
> send_garp 1 1 $eth_src $eth_dst $spa $tpa
>
> +eth_dst=3333ff0061d1
> +ipv6_src=10000000000000000000000000000003
> +ipv6_dst=ff0200000000000000000001ff0061d1
> +ipv6_tgt=100000000000000000000000000061d1
> +send_nd_ns 1 1 $eth_src $eth_dst $ipv6_src $ipv6_dst $ipv6_tgt
> +
> wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$hv1_ch_uuid
> check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1
> wait_for_ports_up sw0-vir
> check ovn-nbctl --wait=hv sync
>
> +wait_row_count Port_Binding 1 logical_port=sw0-vir6 chassis=$hv1_ch_uuid
> +check_row_count Port_Binding 1 logical_port=sw0-vir6 virtual_parent=sw0-p1
> +wait_for_ports_up sw0-vir6
> +check ovn-nbctl --wait=hv sync
> +
> # There should be an arp resolve flow to resolve the virtual_ip with the
> # sw0-p1's MAC.
> ovn-sbctl dump-flows lr0 > lr0-flows2
> @@ -18049,6 +18084,10 @@ AT_CHECK([grep lr_in_arp_resolve lr0-flows2 | grep "reg0 == 10.0.0.10" | sed 's/
> table=??(lr_in_arp_resolve ), priority=100 , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 50:54:00:00:00:03; next;)
> ])
>
> +AT_CHECK([grep lr_in_arp_resolve lr0-flows2 | grep "xxreg0 == 1000::61d1" | sed 's/table=../table=??/'], [0], [dnl
> + table=??(lr_in_arp_resolve ), priority=100 , match=(outport == "lr0-sw0" && xxreg0 == 1000::61d1), action=(eth.dst = 50:54:00:00:00:03; next;)
> +])
> +
> # hv1 should add the flow for the ACL with is_chassis_redirect check for sw0-vir and
> # arp responder flow in lr0 pipeline.
> check_virtual_offlows_present hv1
> @@ -18321,6 +18360,7 @@ wait_row_count nb:Logical_Switch_Port 1 up=false name=sw0-vir
>
> # Clear virtual_ip column of sw0-vir. There should be no bind_vport flows.
> ovn-nbctl --wait=hv remove logical_switch_port sw0-vir options virtual-ip
> +ovn-nbctl --wait=hv remove logical_switch_port sw0-vir6 options virtual-ip
>
> ovn-sbctl dump-flows sw0 > sw0-flows2
> AT_CAPTURE_FILE([sw0-flows2])
> --
> 2.27.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list