| Message ID | 20211103133616.433626-1-mheib@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v4] ovn-northd: Virtual port add ND/ARP responder flows for IPv6 VIPs. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Bleep bloop. Greetings Mohammad Heib, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 82 characters long (recommended limit is 79)
#159 FILE: northd/northd.c:11253:
is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
WARNING: Line is 82 characters long (recommended limit is 79)
#172 FILE: northd/northd.c:11294:
is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
WARNING: Line is 214 characters long (recommended limit is 79)
#192 FILE: northd/ovn-northd.8.xml:1075:
<code>inport == <var>P</var> && && ((nd_ns && ip6.dst == <var>{VIP, NS_MULTICAST_ADDR}</var> && nd.target == <var>VIP</var>) || (nd_na && nd.target == <var>VIP</var>))</code>
Lines checked: 398, Warnings: 3, Errors: 0
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
On Wed, Nov 3, 2021 at 9:37 AM Mohammad Heib <mheib@redhat.com> wrote: > > 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@redhat.com> Thanks for v4. There were a couple of checkpatch warnings related to line length limits. I fixed those and applied the patch to the main branch. Thanks Numan > --- > northd/northd.c | 92 +++++++++++++++++++++++++++++++++-------- > northd/ovn-northd.8.xml | 11 +++-- > tests/ovn.at | 66 +++++++++++++++++++++++------ > 3 files changed, 134 insertions(+), 35 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index da4f9cd14..4e1b6edb1 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -7501,7 +7501,7 @@ build_lswitch_arp_nd_responder_skip_local(struct ovn_port *op, > } > } > > -/* Ingress table 13: ARP/ND responder, reply for known IPs. > +/* Ingress table 16: ARP/ND responder, reply for known IPs. > * (priority 50). */ > static void > build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, > @@ -7519,17 +7519,37 @@ 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. > + * > + * - IPv6 Neighbor Solicitations requests that targets virtual > + * ip which belongs to a logical port of type 'virtual' and > + * bind that port. > + * > + * - IPv6 unsolicited Neighbor Advertisements that targets > + * 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; > @@ -7542,13 +7562,33 @@ 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); " > @@ -11172,17 +11212,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. */ > @@ -11196,8 +11248,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, > + vip); > > const char *arp_actions = > "eth.dst = 00:00:00:00:00:00; next;"; > @@ -11235,8 +11289,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, > + vip); > > ds_clear(actions); > ds_put_format(actions, "eth.dst = %s; next;", ea_s); > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index d219c5618..cf7dc2fda 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -1066,12 +1066,13 @@ > <li> > <p> > If inport <code>V</code> is of type <code>virtual</code> adds a > - priority-100 logical flow for each <var>P</var> configured in the > + priority-100 logical flows for each <var>P</var> configured in the > <ref table="Logical_Switch_Port" column="options:virtual-parents"/> > column with the match > </p> > <pre> > <code>inport == <var>P</var> && && ((arp.op == 1 && arp.spa == <var>VIP</var> && arp.tpa == <var>VIP</var>) || (arp.op == 2 && arp.spa == <var>VIP</var>))</code> > +<code>inport == <var>P</var> && && ((nd_ns && ip6.dst == <var>{VIP, NS_MULTICAST_ADDR}</var> && nd.target == <var>VIP</var>) || (nd_na && nd.target == <var>VIP</var>))</code> > </pre> > > <p> > @@ -1087,7 +1088,9 @@ > > <p> > Where <var>VIP</var> is the virtual ip configured in the column > - <ref table="Logical_Switch_Port" column="options:virtual-ip"/>. > + <ref table="Logical_Switch_Port" column="options:virtual-ip"/> and > + NS_MULTICAST_ADDR is solicited-node multicast address corresponding > + to the VIP. > </p> > </li> > > @@ -3629,7 +3632,7 @@ outport = <var>P</var> > record and the virtual parent with the Ethernet address <var>E</var> > and the virtual ip is reachable via the router port <var>P</var>, a > priority-100 flow with match <code>outport === <var>P</var> > - && reg0 == <var>A</var></code> has actions > + && xxreg0/reg0 == <var>A</var></code> has actions > <code>eth.dst = <var>E</var>; next;</code>. > </p> > > @@ -3641,7 +3644,7 @@ outport = <var>P</var> > record and the virtual ip <var>A</var> is reachable via the > router port <var>P</var>, a > priority-100 flow with match <code>outport === <var>P</var> > - && reg0 == <var>A</var></code> has actions > + && xxreg0/reg0 == <var>A</var></code> has actions > <code>eth.dst = <var>00:00:00:00:00:00</var>; next;</code>. > This flow is added so that the ARP is always resolved for the > virtual ip <var>A</var> by generating ARP request and > diff --git a/tests/ovn.at b/tests/ovn.at > index 5458552d3..ddd0df6f4 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -17950,6 +17950,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} > @@ -18018,43 +18023,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 > @@ -18070,13 +18084,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 > @@ -18084,8 +18099,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 > @@ -18097,6 +18115,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"` > > @@ -18105,6 +18127,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. > @@ -18155,11 +18179,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 > @@ -18168,6 +18203,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 > @@ -18440,6 +18479,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.26.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/northd.c b/northd/northd.c index da4f9cd14..4e1b6edb1 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -7501,7 +7501,7 @@ build_lswitch_arp_nd_responder_skip_local(struct ovn_port *op, } } -/* Ingress table 13: ARP/ND responder, reply for known IPs. +/* Ingress table 16: ARP/ND responder, reply for known IPs. * (priority 50). */ static void build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op, @@ -7519,17 +7519,37 @@ 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. + * + * - IPv6 Neighbor Solicitations requests that targets virtual + * ip which belongs to a logical port of type 'virtual' and + * bind that port. + * + * - IPv6 unsolicited Neighbor Advertisements that targets + * 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; @@ -7542,13 +7562,33 @@ 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); " @@ -11172,17 +11212,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. */ @@ -11196,8 +11248,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, + vip); const char *arp_actions = "eth.dst = 00:00:00:00:00:00; next;"; @@ -11235,8 +11289,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, + vip); ds_clear(actions); ds_put_format(actions, "eth.dst = %s; next;", ea_s); diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index d219c5618..cf7dc2fda 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1066,12 +1066,13 @@ <li> <p> If inport <code>V</code> is of type <code>virtual</code> adds a - priority-100 logical flow for each <var>P</var> configured in the + priority-100 logical flows for each <var>P</var> configured in the <ref table="Logical_Switch_Port" column="options:virtual-parents"/> column with the match </p> <pre> <code>inport == <var>P</var> && && ((arp.op == 1 && arp.spa == <var>VIP</var> && arp.tpa == <var>VIP</var>) || (arp.op == 2 && arp.spa == <var>VIP</var>))</code> +<code>inport == <var>P</var> && && ((nd_ns && ip6.dst == <var>{VIP, NS_MULTICAST_ADDR}</var> && nd.target == <var>VIP</var>) || (nd_na && nd.target == <var>VIP</var>))</code> </pre> <p> @@ -1087,7 +1088,9 @@ <p> Where <var>VIP</var> is the virtual ip configured in the column - <ref table="Logical_Switch_Port" column="options:virtual-ip"/>. + <ref table="Logical_Switch_Port" column="options:virtual-ip"/> and + NS_MULTICAST_ADDR is solicited-node multicast address corresponding + to the VIP. </p> </li> @@ -3629,7 +3632,7 @@ outport = <var>P</var> record and the virtual parent with the Ethernet address <var>E</var> and the virtual ip is reachable via the router port <var>P</var>, a priority-100 flow with match <code>outport === <var>P</var> - && reg0 == <var>A</var></code> has actions + && xxreg0/reg0 == <var>A</var></code> has actions <code>eth.dst = <var>E</var>; next;</code>. </p> @@ -3641,7 +3644,7 @@ outport = <var>P</var> record and the virtual ip <var>A</var> is reachable via the router port <var>P</var>, a priority-100 flow with match <code>outport === <var>P</var> - && reg0 == <var>A</var></code> has actions + && xxreg0/reg0 == <var>A</var></code> has actions <code>eth.dst = <var>00:00:00:00:00:00</var>; next;</code>. This flow is added so that the ARP is always resolved for the virtual ip <var>A</var> by generating ARP request and diff --git a/tests/ovn.at b/tests/ovn.at index 5458552d3..ddd0df6f4 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -17950,6 +17950,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} @@ -18018,43 +18023,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 @@ -18070,13 +18084,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 @@ -18084,8 +18099,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 @@ -18097,6 +18115,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"` @@ -18105,6 +18127,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. @@ -18155,11 +18179,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 @@ -18168,6 +18203,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 @@ -18440,6 +18479,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])
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@redhat.com> --- northd/northd.c | 92 +++++++++++++++++++++++++++++++++-------- northd/ovn-northd.8.xml | 11 +++-- tests/ovn.at | 66 +++++++++++++++++++++++------ 3 files changed, 134 insertions(+), 35 deletions(-)