Message ID | c763a91110de22311f7b1fae514df6d9707e4008.1590443439.git.lorenzo.bianconi@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | fix ARP processing for FIP traffic | expand |
On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > > Introduce 150-priority logical flows for each NAT rule that can > be handled in a distributed manner to manage ARP request locally > for FIP traffic. In particular set reg1 and eth.src to NAT external > ip and NAT external mac respectivelly and do not distribute ARP > traffic using FIP > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > northd/ovn-northd.8.xml | 12 ++++++++++++ > northd/ovn-northd.c | 18 +++++++++++++++++- > tests/ovn.at | 28 +++++++++++++++++++++++++--- > tests/system-ovn.at | 28 ++++++++++++++++++++++++++++ > 4 files changed, 82 insertions(+), 4 deletions(-) > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 9423cbfd1..5e37d9758 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -2875,6 +2875,18 @@ icmp4 { > </p> > > <ul> > + <li> > + For each NAT rule in the OVN Northbound database that can > + be handled in a distributed manner, a priorirty-150 logical > + flow with match <code>ip.src == <var>A</var> && > + is_chassis_resident(<var>B</var>)</code>, where <var>A</var> > + is NAT logical ip and <var>B</var> is NAT logical port. > + IP traffic matching the above rule will be managed locally > + setting <code>reg1</code> to <var>C</var> and <code>eth.src</code> > + to <var>D</var>, where <var>C</var> is NAT external ip and > + <var>D</var> is NAT external mac. > + </li> > + > <li> > For each NAT rule in the OVN Northbound database that can > be handled in a distributed manner, a priority-100 logical > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 7ae5e45da..20f0ee2e4 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > /* Ingress Gateway Redirect Table: For NAT on a distributed > * router, add flows that are specific to a NAT rule. These > * flows indicate the presence of an applicable NAT rule that > - * can be applied in a distributed manner. */ > + * can be applied in a distributed manner. > + * Moreover add logical flows to avoid ARP distributing when the > + * chassis has a direct connection to the underlay network through > + * a localnet port I think the two flows may be combined, right? Firstly, the match "outport == DGW" is missing from the new flow. Secondly, the "is_chassis_resident()" seems to be missing from the original flow. If this understanding is correct, the both flows just have same match condition, so the actions should be combined as well. In addition, I'd suggest to add more clear comment to state the reason why setting reg1 and eth.src. For example: it is done to make sure when ARP request is triggered (in next stage), the ARP generated can have the eth.src = xxx and src IP as yyy, so that it can bypass the flow that redirect the ARP request to GW node ... Thanks, Han > + */ > if (distributed) { > + ds_clear(&match); > + ds_clear(&actions); > + ds_put_format(&match, > + "ip%s.src == %s && is_chassis_resident(\"%s\")", > + is_v6 ? "6" : "4", nat->logical_ip, > + nat->logical_port); > + ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; next;", > + nat->external_mac, is_v6 ? "xx" : "", > + nat->external_ip); > + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 150, > + ds_cstr(&match), ds_cstr(&actions)); > + > ds_clear(&match); > ds_put_format(&match, "ip%s.src == %s && outport == %s", > is_v6 ? "6" : "4", > diff --git a/tests/ovn.at b/tests/ovn.at > index 8fa1a7e1b..15b40ca1e 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ > options:tx_pcap=hv2/vif1-tx.pcap \ > options:rxq_pcap=hv2/vif1-rx.pcap \ > - ofport-request=1 > + ofport-request=2 > +ovs-vsctl -- add-port br-int hv2-vif2 -- \ > + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ > + options:tx_pcap=hv2/vif2-tx.pcap \ > + options:rxq_pcap=hv2/vif2-rx.pcap \ > + ofport-request=3 > > -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 > +ovn-nbctl create Logical_Router name=lr0 > ovn-nbctl ls-add sw0 > ovn-nbctl ls-add sw1 > > @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set Logical_Switch_Port rp-sw0 \ > type=router options:router-port=sw0 \ > -- lsp-set-addresses rp-sw0 router > > -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002:0:0:0:0:0:0:1/64 > +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002:0:0:0:0:0:0:1/64 \ > + -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" > ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ > type=router options:router-port=sw1 \ > -- lsp-set-addresses rp-sw1 router > > ovn-nbctl lsp-add sw0 sw0-p0 \ > -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 2001::2" > +ovn-nbctl lsp-add sw0 sw0-p1 \ > + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 2001::3" > > ovn-nbctl lsp-add sw1 sw1-p0 \ > -- lsp-set-addresses sw1-p0 unknown > @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 $router_ip6 > > OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > > +# Create FIP on sw0-p0, add a route on logical router pipeline and > +# ARP request for a unkwon destination is sent using FIP MAC/IP > +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 f0:00:00:01:02:04 > +ovn-nbctl lr-route-add lr0 172.16.2.0/24 172.16.1.11 > + > +dst_ip=$(ip_to_hex 172 16 2 10) > +fip_ip=$(ip_to_hex 172 16 1 2) > +src_ip=$(ip_to_hex 192 168 1 3) > +gw_router=$(ip_to_hex 172 16 1 11) > +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 $data > +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected > + > +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > + > OVN_CLEANUP([hv1],[hv2]) > AT_CLEANUP > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 9ae6c6b1f..99a0ee07b 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24", "f0:00:00:01:02:05", \ > ovn-nbctl lsp-add alice alice1 \ > -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" > > +# Add external network > +ADD_NAMESPACES(ext-net) > +ip link add alice-ext netns alice1 type veth peer name ext-veth netns ext-net > +ip -n ext-net link set dev ext-veth up > +ip -n ext-net addr add 10.0.0.1/24 dev ext-veth > +ip -n ext-net route add default via 10.0.0.2 > + > +ip -n alice1 link set dev alice-ext up > +ip -n alice1 addr add 10.0.0.2/24 dev alice-ext > +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 > + > # Add DNAT rules > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:00:02:02:03:04]) > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:00:02:02:03:05]) > @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:0 > # Add a SNAT rule > AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) > > +# Add default route to ext-net > +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 172.16.1.2]) > + > ovn-nbctl --wait=hv sync > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=172.16.1.1)']) > > @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > ]) > > +# Try to ping external network > +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 and icmp > ext-net.pcap &]) > +sleep 1 > +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) > +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | FORMAT_PING], \ > +[0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +OVS_WAIT_UNTIL([ > + total_pkts=$(cat ext-net.pcap | wc -l) > + test "${total_pkts}" = "3" > +]) > + > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > as ovn-sb > -- > 2.26.2 >
On 5/26/20 5:23 AM, Han Zhou wrote: > > > On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi > <lorenzo.bianconi@redhat.com <mailto:lorenzo.bianconi@redhat.com>> wrote: >> >> Introduce 150-priority logical flows for each NAT rule that can >> be handled in a distributed manner to manage ARP request locally >> for FIP traffic. In particular set reg1 and eth.src to NAT external >> ip and NAT external mac respectivelly and do not distribute ARP >> traffic using FIP >> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com > <mailto:lorenzo.bianconi@redhat.com>> >> --- >> northd/ovn-northd.8.xml | 12 ++++++++++++ >> northd/ovn-northd.c | 18 +++++++++++++++++- >> tests/ovn.at <http://ovn.at> | 28 +++++++++++++++++++++++++--- >> tests/system-ovn.at <http://system-ovn.at> | 28 > ++++++++++++++++++++++++++++ >> 4 files changed, 82 insertions(+), 4 deletions(-) >> >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml >> index 9423cbfd1..5e37d9758 100644 >> --- a/northd/ovn-northd.8.xml >> +++ b/northd/ovn-northd.8.xml >> @@ -2875,6 +2875,18 @@ icmp4 { >> </p> >> >> <ul> >> + <li> >> + For each NAT rule in the OVN Northbound database that can >> + be handled in a distributed manner, a priorirty-150 logical >> + flow with match <code>ip.src == <var>A</var> && >> + is_chassis_resident(<var>B</var>)</code>, where <var>A</var> >> + is NAT logical ip and <var>B</var> is NAT logical port. >> + IP traffic matching the above rule will be managed locally >> + setting <code>reg1</code> to <var>C</var> and > <code>eth.src</code> >> + to <var>D</var>, where <var>C</var> is NAT external ip and >> + <var>D</var> is NAT external mac. >> + </li> >> + >> <li> >> For each NAT rule in the OVN Northbound database that can >> be handled in a distributed manner, a priority-100 logical >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index 7ae5e45da..20f0ee2e4 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, > struct hmap *ports, >> /* Ingress Gateway Redirect Table: For NAT on a distributed >> * router, add flows that are specific to a NAT rule. These >> * flows indicate the presence of an applicable NAT rule that >> - * can be applied in a distributed manner. */ >> + * can be applied in a distributed manner. >> + * Moreover add logical flows to avoid ARP distributing > when the >> + * chassis has a direct connection to the underlay > network through >> + * a localnet port > > I think the two flows may be combined, right? Firstly, the match > "outport == DGW" is missing from the new flow. Secondly, the > "is_chassis_resident()" seems to be missing from the original flow. If > this understanding is correct, the both flows just have same match > condition, so the actions should be combined as well. > Hi Lorenzo, Han, I agree that both flows should have the same match and that actions should be combined. I'm not sure however if we can ever hit these flows if is_chassis_resident(logical_port) == false. If I understand correctly we can't, but just to be safe it's probably fine to add the is_chassis_resident(logical_port) check. > In addition, I'd suggest to add more clear comment to state the reason > why setting reg1 and eth.src. For example: it is done to make sure when > ARP request is triggered (in next stage), the ARP generated can have the > eth.src = xxx and src IP as yyy, so that it can bypass the flow that > redirect the ARP request to GW node ... > > Thanks, > Han > >> + */ >> if (distributed) { >> + ds_clear(&match); >> + ds_clear(&actions); >> + ds_put_format(&match, >> + "ip%s.src == %s && > is_chassis_resident(\"%s\")", >> + is_v6 ? "6" : "4", nat->logical_ip, >> + nat->logical_port); >> + ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; > next;", >> + nat->external_mac, is_v6 ? "xx" : "", >> + nat->external_ip); >> + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 150, >> + ds_cstr(&match), ds_cstr(&actions)); >> + >> ds_clear(&match); >> ds_put_format(&match, "ip%s.src == %s && outport == %s", >> is_v6 ? "6" : "4", >> diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at> >> index 8fa1a7e1b..15b40ca1e 100644 >> --- a/tests/ovn.at <http://ovn.at> >> +++ b/tests/ovn.at <http://ovn.at> >> @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ >> set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ >> options:tx_pcap=hv2/vif1-tx.pcap \ >> options:rxq_pcap=hv2/vif1-rx.pcap \ >> - ofport-request=1 >> + ofport-request=2 >> +ovs-vsctl -- add-port br-int hv2-vif2 -- \ >> + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ >> + options:tx_pcap=hv2/vif2-tx.pcap \ >> + options:rxq_pcap=hv2/vif2-rx.pcap \ >> + ofport-request=3 >> >> -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 >> +ovn-nbctl create Logical_Router name=lr0 >> ovn-nbctl ls-add sw0 >> ovn-nbctl ls-add sw1 >> >> @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set > Logical_Switch_Port rp-sw0 \ >> type=router options:router-port=sw0 \ >> -- lsp-set-addresses rp-sw0 router >> >> -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 >> +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 \ >> + -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" >> ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ >> type=router options:router-port=sw1 \ >> -- lsp-set-addresses rp-sw1 router >> >> ovn-nbctl lsp-add sw0 sw0-p0 \ >> -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 2001::2" >> +ovn-nbctl lsp-add sw0 sw0-p1 \ >> + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 2001::3" >> >> ovn-nbctl lsp-add sw1 sw1-p0 \ >> -- lsp-set-addresses sw1-p0 unknown >> @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 > $router_ip6 >> >> OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) >> >> +# Create FIP on sw0-p0, add a route on logical router pipeline and >> +# ARP request for a unkwon destination is sent using FIP MAC/IP >> +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 > f0:00:00:01:02:04 >> +ovn-nbctl lr-route-add lr0 172.16.2.0/24 <http://172.16.2.0/24> > 172.16.1.11 >> + >> +dst_ip=$(ip_to_hex 172 16 2 10) >> +fip_ip=$(ip_to_hex 172 16 1 2) >> +src_ip=$(ip_to_hex 192 168 1 3) >> +gw_router=$(ip_to_hex 172 16 1 11) >> +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 $data >> +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected >> + >> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) >> + >> OVN_CLEANUP([hv1],[hv2]) >> AT_CLEANUP >> >> diff --git a/tests/system-ovn.at <http://system-ovn.at> > b/tests/system-ovn.at <http://system-ovn.at> >> index 9ae6c6b1f..99a0ee07b 100644 >> --- a/tests/system-ovn.at <http://system-ovn.at> >> +++ b/tests/system-ovn.at <http://system-ovn.at> >> @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24 > <http://172.16.1.2/24>", "f0:00:00:01:02:05", \ >> ovn-nbctl lsp-add alice alice1 \ >> -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" >> >> +# Add external network >> +ADD_NAMESPACES(ext-net) >> +ip link add alice-ext netns alice1 type veth peer name ext-veth netns > ext-net >> +ip -n ext-net link set dev ext-veth up >> +ip -n ext-net addr add 10.0.0.1/24 <http://10.0.0.1/24> dev ext-veth >> +ip -n ext-net route add default via 10.0.0.2 >> + >> +ip -n alice1 link set dev alice-ext up >> +ip -n alice1 addr add 10.0.0.2/24 <http://10.0.0.2/24> dev alice-ext >> +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 >> + Shouldn't we use NS_CHECK_EXEC() instead of "ip -n"/"ip netns"? Regards, Dumitru >> # Add DNAT rules >> AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 > 192.168.1.2 foo1 00:00:02:02:03:04]) >> AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 > 192.168.1.3 foo2 00:00:02:02:03:05]) >> @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > 172.16.1.4 192.168.1.3 foo2 00:0 >> # Add a SNAT rule >> AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16 > <http://192.168.0.0/16>]) >> >> +# Add default route to ext-net >> +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 <http://10.0.0.0/24> > 172.16.1.2]) >> + >> ovn-nbctl --wait=hv sync >> OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep > 'nat(src=172.16.1.1)']) >> >> @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], > [dnl >> > icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> >> ]) >> >> +# Try to ping external network >> +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 > and icmp > ext-net.pcap &]) >> +sleep 1 >> +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) >> +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | > FORMAT_PING], \ >> +[0], [dnl >> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >> +]) >> + >> +OVS_WAIT_UNTIL([ >> + total_pkts=$(cat ext-net.pcap | wc -l) >> + test "${total_pkts}" = "3" >> +]) >> + >> OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> -- >> 2.26.2 >>
> On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> wrote: > > > > Introduce 150-priority logical flows for each NAT rule that can > > be handled in a distributed manner to manage ARP request locally > > for FIP traffic. In particular set reg1 and eth.src to NAT external > > ip and NAT external mac respectivelly and do not distribute ARP > > traffic using FIP > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > northd/ovn-northd.8.xml | 12 ++++++++++++ > > northd/ovn-northd.c | 18 +++++++++++++++++- > > tests/ovn.at | 28 +++++++++++++++++++++++++--- > > tests/system-ovn.at | 28 ++++++++++++++++++++++++++++ > > 4 files changed, 82 insertions(+), 4 deletions(-) > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > index 9423cbfd1..5e37d9758 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -2875,6 +2875,18 @@ icmp4 { > > </p> > > > > <ul> > > + <li> > > + For each NAT rule in the OVN Northbound database that can > > + be handled in a distributed manner, a priorirty-150 logical > > + flow with match <code>ip.src == <var>A</var> && > > + is_chassis_resident(<var>B</var>)</code>, where <var>A</var> > > + is NAT logical ip and <var>B</var> is NAT logical port. > > + IP traffic matching the above rule will be managed locally > > + setting <code>reg1</code> to <var>C</var> and > <code>eth.src</code> > > + to <var>D</var>, where <var>C</var> is NAT external ip and > > + <var>D</var> is NAT external mac. > > + </li> > > + > > <li> > > For each NAT rule in the OVN Northbound database that can > > be handled in a distributed manner, a priority-100 logical > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 7ae5e45da..20f0ee2e4 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, struct > hmap *ports, > > /* Ingress Gateway Redirect Table: For NAT on a distributed > > * router, add flows that are specific to a NAT rule. These > > * flows indicate the presence of an applicable NAT rule that > > - * can be applied in a distributed manner. */ > > + * can be applied in a distributed manner. > > + * Moreover add logical flows to avoid ARP distributing when > the > > + * chassis has a direct connection to the underlay network > through > > + * a localnet port > > I think the two flows may be combined, right? Firstly, the match "outport > == DGW" is missing from the new flow. Secondly, the "is_chassis_resident()" > seems to be missing from the original flow. If this understanding is > correct, the both flows just have same match condition, so the actions > should be combined as well. > > In addition, I'd suggest to add more clear comment to state the reason why > setting reg1 and eth.src. For example: it is done to make sure when ARP > request is triggered (in next stage), the ARP generated can have the > eth.src = xxx and src IP as yyy, so that it can bypass the flow that > redirect the ARP request to GW node ... Hi Han, ack, thx for the review. I will fix it in v2. Regards, Lorenzo > > Thanks, > Han > > > + */ > > if (distributed) { > > + ds_clear(&match); > > + ds_clear(&actions); > > + ds_put_format(&match, > > + "ip%s.src == %s && > is_chassis_resident(\"%s\")", > > + is_v6 ? "6" : "4", nat->logical_ip, > > + nat->logical_port); > > + ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; > next;", > > + nat->external_mac, is_v6 ? "xx" : "", > > + nat->external_ip); > > + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 150, > > + ds_cstr(&match), ds_cstr(&actions)); > > + > > ds_clear(&match); > > ds_put_format(&match, "ip%s.src == %s && outport == %s", > > is_v6 ? "6" : "4", > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 8fa1a7e1b..15b40ca1e 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > > set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ > > options:tx_pcap=hv2/vif1-tx.pcap \ > > options:rxq_pcap=hv2/vif1-rx.pcap \ > > - ofport-request=1 > > + ofport-request=2 > > +ovs-vsctl -- add-port br-int hv2-vif2 -- \ > > + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ > > + options:tx_pcap=hv2/vif2-tx.pcap \ > > + options:rxq_pcap=hv2/vif2-rx.pcap \ > > + ofport-request=3 > > > > -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 > > +ovn-nbctl create Logical_Router name=lr0 > > ovn-nbctl ls-add sw0 > > ovn-nbctl ls-add sw1 > > > > @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set > Logical_Switch_Port rp-sw0 \ > > type=router options:router-port=sw0 \ > > -- lsp-set-addresses rp-sw0 router > > > > -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > 2002:0:0:0:0:0:0:1/64 > > +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > 2002:0:0:0:0:0:0:1/64 \ > > + -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" > > ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ > > type=router options:router-port=sw1 \ > > -- lsp-set-addresses rp-sw1 router > > > > ovn-nbctl lsp-add sw0 sw0-p0 \ > > -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 2001::2" > > +ovn-nbctl lsp-add sw0 sw0-p1 \ > > + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 2001::3" > > > > ovn-nbctl lsp-add sw1 sw1-p0 \ > > -- lsp-set-addresses sw1-p0 unknown > > @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 > $router_ip6 > > > > OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > > > > +# Create FIP on sw0-p0, add a route on logical router pipeline and > > +# ARP request for a unkwon destination is sent using FIP MAC/IP > > +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 > f0:00:00:01:02:04 > > +ovn-nbctl lr-route-add lr0 172.16.2.0/24 172.16.1.11 > > + > > +dst_ip=$(ip_to_hex 172 16 2 10) > > +fip_ip=$(ip_to_hex 172 16 1 2) > > +src_ip=$(ip_to_hex 192 168 1 3) > > +gw_router=$(ip_to_hex 172 16 1 11) > > +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 $data > > +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected > > + > > +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > > + > > OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 9ae6c6b1f..99a0ee07b 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24", > "f0:00:00:01:02:05", \ > > ovn-nbctl lsp-add alice alice1 \ > > -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" > > > > +# Add external network > > +ADD_NAMESPACES(ext-net) > > +ip link add alice-ext netns alice1 type veth peer name ext-veth netns > ext-net > > +ip -n ext-net link set dev ext-veth up > > +ip -n ext-net addr add 10.0.0.1/24 dev ext-veth > > +ip -n ext-net route add default via 10.0.0.2 > > + > > +ip -n alice1 link set dev alice-ext up > > +ip -n alice1 addr add 10.0.0.2/24 dev alice-ext > > +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 > > + > > # Add DNAT rules > > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 > foo1 00:00:02:02:03:04]) > > AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 > foo2 00:00:02:02:03:05]) > > @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > 172.16.1.4 192.168.1.3 foo2 00:0 > > # Add a SNAT rule > > AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) > > > > +# Add default route to ext-net > > +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 172.16.1.2]) > > + > > ovn-nbctl --wait=hv sync > > OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep > 'nat(src=172.16.1.1)']) > > > > @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], > [dnl > > > icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > > ]) > > > > +# Try to ping external network > > +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 and > icmp > ext-net.pcap &]) > > +sleep 1 > > +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) > > +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | FORMAT_PING], > \ > > +[0], [dnl > > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > > +]) > > + > > +OVS_WAIT_UNTIL([ > > + total_pkts=$(cat ext-net.pcap | wc -l) > > + test "${total_pkts}" = "3" > > +]) > > + > > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > > > as ovn-sb > > -- > > 2.26.2 > >
> On 5/26/20 5:23 AM, Han Zhou wrote: > > > > > > On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi > > <lorenzo.bianconi@redhat.com <mailto:lorenzo.bianconi@redhat.com>> wrote: > >> > >> Introduce 150-priority logical flows for each NAT rule that can > >> be handled in a distributed manner to manage ARP request locally > >> for FIP traffic. In particular set reg1 and eth.src to NAT external > >> ip and NAT external mac respectivelly and do not distribute ARP > >> traffic using FIP > >> > >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com > > <mailto:lorenzo.bianconi@redhat.com>> > >> --- > >> northd/ovn-northd.8.xml | 12 ++++++++++++ > >> northd/ovn-northd.c | 18 +++++++++++++++++- > >> tests/ovn.at <http://ovn.at> | 28 +++++++++++++++++++++++++--- > >> tests/system-ovn.at <http://system-ovn.at> | 28 > > ++++++++++++++++++++++++++++ > >> 4 files changed, 82 insertions(+), 4 deletions(-) > >> > >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > >> index 9423cbfd1..5e37d9758 100644 > >> --- a/northd/ovn-northd.8.xml > >> +++ b/northd/ovn-northd.8.xml > >> @@ -2875,6 +2875,18 @@ icmp4 { > >> </p> > >> > >> <ul> > >> + <li> > >> + For each NAT rule in the OVN Northbound database that can > >> + be handled in a distributed manner, a priorirty-150 logical > >> + flow with match <code>ip.src == <var>A</var> && > >> + is_chassis_resident(<var>B</var>)</code>, where <var>A</var> > >> + is NAT logical ip and <var>B</var> is NAT logical port. > >> + IP traffic matching the above rule will be managed locally > >> + setting <code>reg1</code> to <var>C</var> and > > <code>eth.src</code> > >> + to <var>D</var>, where <var>C</var> is NAT external ip and > >> + <var>D</var> is NAT external mac. > >> + </li> > >> + > >> <li> > >> For each NAT rule in the OVN Northbound database that can > >> be handled in a distributed manner, a priority-100 logical > >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > >> index 7ae5e45da..20f0ee2e4 100644 > >> --- a/northd/ovn-northd.c > >> +++ b/northd/ovn-northd.c > >> @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, > > struct hmap *ports, > >> /* Ingress Gateway Redirect Table: For NAT on a distributed > >> * router, add flows that are specific to a NAT rule. These > >> * flows indicate the presence of an applicable NAT rule that > >> - * can be applied in a distributed manner. */ > >> + * can be applied in a distributed manner. > >> + * Moreover add logical flows to avoid ARP distributing > > when the > >> + * chassis has a direct connection to the underlay > > network through > >> + * a localnet port > > > > I think the two flows may be combined, right? Firstly, the match > > "outport == DGW" is missing from the new flow. Secondly, the > > "is_chassis_resident()" seems to be missing from the original flow. If > > this understanding is correct, the both flows just have same match > > condition, so the actions should be combined as well. > > > > Hi Lorenzo, Han, > > I agree that both flows should have the same match and that actions > should be combined. > > I'm not sure however if we can ever hit these flows if > is_chassis_resident(logical_port) == false. If I understand correctly we > can't, but just to be safe it's probably fine to add the > is_chassis_resident(logical_port) check. > > > In addition, I'd suggest to add more clear comment to state the reason > > why setting reg1 and eth.src. For example: it is done to make sure when > > ARP request is triggered (in next stage), the ARP generated can have the > > eth.src = xxx and src IP as yyy, so that it can bypass the flow that > > redirect the ARP request to GW node ... > > > > Thanks, > > Han > > > >> + */ > >> if (distributed) { > >> + ds_clear(&match); > >> + ds_clear(&actions); > >> + ds_put_format(&match, > >> + "ip%s.src == %s && > > is_chassis_resident(\"%s\")", > >> + is_v6 ? "6" : "4", nat->logical_ip, > >> + nat->logical_port); > >> + ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; > > next;", > >> + nat->external_mac, is_v6 ? "xx" : "", > >> + nat->external_ip); > >> + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 150, > >> + ds_cstr(&match), ds_cstr(&actions)); > >> + > >> ds_clear(&match); > >> ds_put_format(&match, "ip%s.src == %s && outport == %s", > >> is_v6 ? "6" : "4", > >> diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at> > >> index 8fa1a7e1b..15b40ca1e 100644 > >> --- a/tests/ovn.at <http://ovn.at> > >> +++ b/tests/ovn.at <http://ovn.at> > >> @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > >> set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ > >> options:tx_pcap=hv2/vif1-tx.pcap \ > >> options:rxq_pcap=hv2/vif1-rx.pcap \ > >> - ofport-request=1 > >> + ofport-request=2 > >> +ovs-vsctl -- add-port br-int hv2-vif2 -- \ > >> + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ > >> + options:tx_pcap=hv2/vif2-tx.pcap \ > >> + options:rxq_pcap=hv2/vif2-rx.pcap \ > >> + ofport-request=3 > >> > >> -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 > >> +ovn-nbctl create Logical_Router name=lr0 > >> ovn-nbctl ls-add sw0 > >> ovn-nbctl ls-add sw1 > >> > >> @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set > > Logical_Switch_Port rp-sw0 \ > >> type=router options:router-port=sw0 \ > >> -- lsp-set-addresses rp-sw0 router > >> > >> -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > > <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 > >> +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > > <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 \ > >> + -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" > >> ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ > >> type=router options:router-port=sw1 \ > >> -- lsp-set-addresses rp-sw1 router > >> > >> ovn-nbctl lsp-add sw0 sw0-p0 \ > >> -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 2001::2" > >> +ovn-nbctl lsp-add sw0 sw0-p1 \ > >> + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 2001::3" > >> > >> ovn-nbctl lsp-add sw1 sw1-p0 \ > >> -- lsp-set-addresses sw1-p0 unknown > >> @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 > > $router_ip6 > >> > >> OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > >> > >> +# Create FIP on sw0-p0, add a route on logical router pipeline and > >> +# ARP request for a unkwon destination is sent using FIP MAC/IP > >> +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 > > f0:00:00:01:02:04 > >> +ovn-nbctl lr-route-add lr0 172.16.2.0/24 <http://172.16.2.0/24> > > 172.16.1.11 > >> + > >> +dst_ip=$(ip_to_hex 172 16 2 10) > >> +fip_ip=$(ip_to_hex 172 16 1 2) > >> +src_ip=$(ip_to_hex 192 168 1 3) > >> +gw_router=$(ip_to_hex 172 16 1 11) > >> +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 $data > >> +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected > >> + > >> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > >> + > >> OVN_CLEANUP([hv1],[hv2]) > >> AT_CLEANUP > >> > >> diff --git a/tests/system-ovn.at <http://system-ovn.at> > > b/tests/system-ovn.at <http://system-ovn.at> > >> index 9ae6c6b1f..99a0ee07b 100644 > >> --- a/tests/system-ovn.at <http://system-ovn.at> > >> +++ b/tests/system-ovn.at <http://system-ovn.at> > >> @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24 > > <http://172.16.1.2/24>", "f0:00:00:01:02:05", \ > >> ovn-nbctl lsp-add alice alice1 \ > >> -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" > >> > >> +# Add external network > >> +ADD_NAMESPACES(ext-net) > >> +ip link add alice-ext netns alice1 type veth peer name ext-veth netns > > ext-net > >> +ip -n ext-net link set dev ext-veth up > >> +ip -n ext-net addr add 10.0.0.1/24 <http://10.0.0.1/24> dev ext-veth > >> +ip -n ext-net route add default via 10.0.0.2 > >> + > >> +ip -n alice1 link set dev alice-ext up > >> +ip -n alice1 addr add 10.0.0.2/24 <http://10.0.0.2/24> dev alice-ext > >> +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 > >> + > > Shouldn't we use NS_CHECK_EXEC() instead of "ip -n"/"ip netns"? Hi Dumitru, thx for the review. I guess we can use NS_EXEC instead, right? Regards, Lorenzo > > Regards, > Dumitru > > >> # Add DNAT rules > >> AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 > > 192.168.1.2 foo1 00:00:02:02:03:04]) > >> AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 > > 192.168.1.3 foo2 00:00:02:02:03:05]) > >> @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > > 172.16.1.4 192.168.1.3 foo2 00:0 > >> # Add a SNAT rule > >> AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16 > > <http://192.168.0.0/16>]) > >> > >> +# Add default route to ext-net > >> +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 <http://10.0.0.0/24> > > 172.16.1.2]) > >> + > >> ovn-nbctl --wait=hv sync > >> OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep > > 'nat(src=172.16.1.1)']) > >> > >> @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], > > [dnl > >> > > icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > >> ]) > >> > >> +# Try to ping external network > >> +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 > > and icmp > ext-net.pcap &]) > >> +sleep 1 > >> +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) > >> +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | > > FORMAT_PING], \ > >> +[0], [dnl > >> +3 packets transmitted, 3 received, 0% packet loss, time 0ms > >> +]) > >> + > >> +OVS_WAIT_UNTIL([ > >> + total_pkts=$(cat ext-net.pcap | wc -l) > >> + test "${total_pkts}" = "3" > >> +]) > >> + > >> OVS_APP_EXIT_AND_WAIT([ovn-controller]) > >> > >> as ovn-sb > >> -- > >> 2.26.2 > >> >
On 5/26/20 5:39 PM, Lorenzo Bianconi wrote: >> On 5/26/20 5:23 AM, Han Zhou wrote: >>> >>> >>> On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi >>> <lorenzo.bianconi@redhat.com <mailto:lorenzo.bianconi@redhat.com>> wrote: >>>> >>>> Introduce 150-priority logical flows for each NAT rule that can >>>> be handled in a distributed manner to manage ARP request locally >>>> for FIP traffic. In particular set reg1 and eth.src to NAT external >>>> ip and NAT external mac respectivelly and do not distribute ARP >>>> traffic using FIP >>>> >>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com >>> <mailto:lorenzo.bianconi@redhat.com>> >>>> --- >>>>  northd/ovn-northd.8.xml | 12 ++++++++++++ >>>>  northd/ovn-northd.c   | 18 +++++++++++++++++- >>>>  tests/ovn.at <http://ovn.at>       | 28 +++++++++++++++++++++++++--- >>>>  tests/system-ovn.at <http://system-ovn.at>   | 28 >>> ++++++++++++++++++++++++++++ >>>>  4 files changed, 82 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml >>>> index 9423cbfd1..5e37d9758 100644 >>>> --- a/northd/ovn-northd.8.xml >>>> +++ b/northd/ovn-northd.8.xml >>>> @@ -2875,6 +2875,18 @@ icmp4 { >>>>    </p> >>>> >>>>    <ul> >>>> +    <li> >>>> +     For each NAT rule in the OVN Northbound database that can >>>> +     be handled in a distributed manner, a priorirty-150 logical >>>> +     flow with match <code>ip.src == <var>A</var> && >>>> +     is_chassis_resident(<var>B</var>)</code>, where <var>A</var> >>>> +     is NAT logical ip and <var>B</var> is NAT logical port. >>>> +     IP traffic matching the above rule will be managed locally >>>> +     setting <code>reg1</code> to <var>C</var> and >>> <code>eth.src</code> >>>> +     to <var>D</var>, where <var>C</var> is NAT external ip and >>>> +     <var>D</var> is NAT external mac. >>>> +    </li> >>>> + >>>>     <li> >>>>      For each NAT rule in the OVN Northbound database that can >>>>      be handled in a distributed manner, a priority-100 logical >>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>>> index 7ae5e45da..20f0ee2e4 100644 >>>> --- a/northd/ovn-northd.c >>>> +++ b/northd/ovn-northd.c >>>> @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, >>> struct hmap *ports, >>>>        /* Ingress Gateway Redirect Table: For NAT on a distributed >>>>        * router, add flows that are specific to a NAT rule. These >>>>        * flows indicate the presence of an applicable NAT rule that >>>> -       * can be applied in a distributed manner. */ >>>> +       * can be applied in a distributed manner. >>>> +       * Moreover add logical flows to avoid ARP distributing >>> when the >>>> +       * chassis has a direct connection to the underlay >>> network through >>>> +       * a localnet port >>> >>> I think the two flows may be combined, right? Firstly, the match >>> "outport == DGW" is missing from the new flow. Secondly, the >>> "is_chassis_resident()" seems to be missing from the original flow. If >>> this understanding is correct, the both flows just have same match >>> condition, so the actions should be combined as well. >>> >> >> Hi Lorenzo, Han, >> >> I agree that both flows should have the same match and that actions >> should be combined. >> >> I'm not sure however if we can ever hit these flows if >> is_chassis_resident(logical_port) == false. If I understand correctly we >> can't, but just to be safe it's probably fine to add the >> is_chassis_resident(logical_port) check. >> >>> In addition, I'd suggest to add more clear comment to state the reason >>> why setting reg1 and eth.src. For example: it is done to make sure when >>> ARP request is triggered (in next stage), the ARP generated can have the >>> eth.src = xxx and src IP as yyy, so that it can bypass the flow that >>> redirect the ARP request to GW node ... >>> >>> Thanks, >>> Han >>> >>>> +       */ >>>>        if (distributed) { >>>> +         ds_clear(&match); >>>> +         ds_clear(&actions); >>>> +         ds_put_format(&match, >>>> +                "ip%s.src == %s && >>> is_chassis_resident(\"%s\")", >>>> +                is_v6 ? "6" : "4", nat->logical_ip, >>>> +                nat->logical_port); >>>> +         ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; >>> next;", >>>> +                nat->external_mac, is_v6 ? "xx" : "", >>>> +                nat->external_ip); >>>> +         ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 150, >>>> +                ds_cstr(&match), ds_cstr(&actions)); >>>> + >>>>          ds_clear(&match); >>>>          ds_put_format(&match, "ip%s.src == %s && outport == %s", >>>>                 is_v6 ? "6" : "4", >>>> diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at> >>>> index 8fa1a7e1b..15b40ca1e 100644 >>>> --- a/tests/ovn.at <http://ovn.at> >>>> +++ b/tests/ovn.at <http://ovn.at> >>>> @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ >>>>    set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ >>>>    options:tx_pcap=hv2/vif1-tx.pcap \ >>>>    options:rxq_pcap=hv2/vif1-rx.pcap \ >>>> -   ofport-request=1 >>>> +   ofport-request=2 >>>> +ovs-vsctl -- add-port br-int hv2-vif2 -- \ >>>> +   set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ >>>> +   options:tx_pcap=hv2/vif2-tx.pcap \ >>>> +   options:rxq_pcap=hv2/vif2-rx.pcap \ >>>> +   ofport-request=3 >>>> >>>> -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 >>>> +ovn-nbctl create Logical_Router name=lr0 >>>>  ovn-nbctl ls-add sw0 >>>>  ovn-nbctl ls-add sw1 >>>> >>>> @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set >>> Logical_Switch_Port rp-sw0 \ >>>>    type=router options:router-port=sw0 \ >>>>    -- lsp-set-addresses rp-sw0 router >>>> >>>> -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 >>> <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 >>>> +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 >>> <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 \ >>>> +   -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" >>>>  ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ >>>>    type=router options:router-port=sw1 \ >>>>    -- lsp-set-addresses rp-sw1 router >>>> >>>>  ovn-nbctl lsp-add sw0 sw0-p0 \ >>>>    -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 2001::2" >>>> +ovn-nbctl lsp-add sw0 sw0-p1 \ >>>> +   -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 2001::3" >>>> >>>>  ovn-nbctl lsp-add sw1 sw1-p0 \ >>>>    -- lsp-set-addresses sw1-p0 unknown >>>> @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 >>> $router_ip6 >>>> >>>>  OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) >>>> >>>> +# Create FIP on sw0-p0, add a route on logical router pipeline and >>>> +# ARP request for a unkwon destination is sent using FIP MAC/IP >>>> +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 >>> f0:00:00:01:02:04 >>>> +ovn-nbctl lr-route-add lr0 172.16.2.0/24 <http://172.16.2.0/24> >>> 172.16.1.11 >>>> + >>>> +dst_ip=$(ip_to_hex 172 16 2 10) >>>> +fip_ip=$(ip_to_hex 172 16 1 2) >>>> +src_ip=$(ip_to_hex 192 168 1 3) >>>> +gw_router=$(ip_to_hex 172 16 1 11) >>>> +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 $data >>>> +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected >>>> + >>>> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) >>>> + >>>>  OVN_CLEANUP([hv1],[hv2]) >>>>  AT_CLEANUP >>>> >>>> diff --git a/tests/system-ovn.at <http://system-ovn.at> >>> b/tests/system-ovn.at <http://system-ovn.at> >>>> index 9ae6c6b1f..99a0ee07b 100644 >>>> --- a/tests/system-ovn.at <http://system-ovn.at> >>>> +++ b/tests/system-ovn.at <http://system-ovn.at> >>>> @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24 >>> <http://172.16.1.2/24>", "f0:00:00:01:02:05", \ >>>>  ovn-nbctl lsp-add alice alice1 \ >>>>  -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" >>>> >>>> +# Add external network >>>> +ADD_NAMESPACES(ext-net) >>>> +ip link add alice-ext netns alice1 type veth peer name ext-veth netns >>> ext-net >>>> +ip -n ext-net link set dev ext-veth up >>>> +ip -n ext-net addr add 10.0.0.1/24 <http://10.0.0.1/24> dev ext-veth >>>> +ip -n ext-net route add default via 10.0.0.2 >>>> + >>>> +ip -n alice1 link set dev alice-ext up >>>> +ip -n alice1 addr add 10.0.0.2/24 <http://10.0.0.2/24> dev alice-ext >>>> +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 >>>> + >> >> Shouldn't we use NS_CHECK_EXEC() instead of "ip -n"/"ip netns"? > > Hi Dumitru, > > thx for the review. I guess we can use NS_EXEC instead, right? > I guess we could but we don't use NS_EXEC() anywhere in the system-ovn.at tests. Also I don't see any harm in the additional checks done by NS_CHECK_EXEC(). Thanks, Dumitru > Regards, > Lorenzo > >> >> Regards, >> Dumitru >> >>>>  # Add DNAT rules >>>>  AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 >>> 192.168.1.2 foo1 00:00:02:02:03:04]) >>>>  AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 >>> 192.168.1.3 foo2 00:00:02:02:03:05]) >>>> @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat >>> 172.16.1.4 192.168.1.3 foo2 00:0 >>>>  # Add a SNAT rule >>>>  AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16 >>> <http://192.168.0.0/16>]) >>>> >>>> +# Add default route to ext-net >>>> +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 <http://10.0.0.0/24> >>> 172.16.1.2]) >>>> + >>>>  ovn-nbctl --wait=hv sync >>>>  OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep >>> 'nat(src=172.16.1.1)']) >>>> >>>> @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], >>> [dnl >>>> >>>  icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> >>>>  ]) >>>> >>>> +# Try to ping external network >>>> +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 >>> and icmp > ext-net.pcap &]) >>>> +sleep 1 >>>> +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) >>>> +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | >>> FORMAT_PING], \ >>>> +[0], [dnl >>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >>>> +]) >>>> + >>>> +OVS_WAIT_UNTIL([ >>>> +   total_pkts=$(cat ext-net.pcap | wc -l) >>>> +   test "${total_pkts}" = "3" >>>> +]) >>>> + >>>>  OVS_APP_EXIT_AND_WAIT([ovn-controller]) >>>> >>>>  as ovn-sb >>>> -- >>>> 2.26.2 >>>> >>
On Tue, May 26, 2020 at 9:16 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 5/26/20 5:39 PM, Lorenzo Bianconi wrote: > >> On 5/26/20 5:23 AM, Han Zhou wrote: > >>> > >>> > >>> On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi > >>> <lorenzo.bianconi@redhat.com <mailto:lorenzo.bianconi@redhat.com>> > wrote: > >>>> > >>>> Introduce 150-priority logical flows for each NAT rule that can > >>>> be handled in a distributed manner to manage ARP request locally > >>>> for FIP traffic. In particular set reg1 and eth.src to NAT external > >>>> ip and NAT external mac respectivelly and do not distribute ARP > >>>> traffic using FIP > >>>> > >>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com > >>> <mailto:lorenzo.bianconi@redhat.com>> > >>>> --- > >>>> northd/ovn-northd.8.xml | 12 ++++++++++++ > >>>> northd/ovn-northd.c | 18 +++++++++++++++++- > >>>> tests/ovn.at <http://ovn.at> | 28 > +++++++++++++++++++++++++--- > >>>> tests/system-ovn.at <http://system-ovn.at> | 28 > >>> ++++++++++++++++++++++++++++ > >>>> 4 files changed, 82 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > >>>> index 9423cbfd1..5e37d9758 100644 > >>>> --- a/northd/ovn-northd.8.xml > >>>> +++ b/northd/ovn-northd.8.xml > >>>> @@ -2875,6 +2875,18 @@ icmp4 { > >>>> </p> > >>>> > >>>> <ul> > >>>> + <li> > >>>> + For each NAT rule in the OVN Northbound database that can > >>>> + be handled in a distributed manner, a priorirty-150 logical > >>>> + flow with match <code>ip.src == <var>A</var> && > >>>> + is_chassis_resident(<var>B</var>)</code>, where <var>A</var> > >>>> + is NAT logical ip and <var>B</var> is NAT logical port. > >>>> + IP traffic matching the above rule will be managed locally > >>>> + setting <code>reg1</code> to <var>C</var> and > >>> <code>eth.src</code> > >>>> + to <var>D</var>, where <var>C</var> is NAT external ip and > >>>> + <var>D</var> is NAT external mac. > >>>> + </li> > >>>> + > >>>> <li> > >>>> For each NAT rule in the OVN Northbound database that can > >>>> be handled in a distributed manner, a priority-100 logical > >>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > >>>> index 7ae5e45da..20f0ee2e4 100644 > >>>> --- a/northd/ovn-northd.c > >>>> +++ b/northd/ovn-northd.c > >>>> @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, > >>> struct hmap *ports, > >>>> /* Ingress Gateway Redirect Table: For NAT on a > distributed > >>>> * router, add flows that are specific to a NAT rule. > These > >>>> * flows indicate the presence of an applicable NAT rule > that > >>>> - * can be applied in a distributed manner. */ > >>>> + * can be applied in a distributed manner. > >>>> + * Moreover add logical flows to avoid ARP distributing > >>> when the > >>>> + * chassis has a direct connection to the underlay > >>> network through > >>>> + * a localnet port > >>> > >>> I think the two flows may be combined, right? Firstly, the match > >>> "outport == DGW" is missing from the new flow. Secondly, the > >>> "is_chassis_resident()" seems to be missing from the original flow. If > >>> this understanding is correct, the both flows just have same match > >>> condition, so the actions should be combined as well. > >>> > >> > >> Hi Lorenzo, Han, > >> > >> I agree that both flows should have the same match and that actions > >> should be combined. > >> > >> I'm not sure however if we can ever hit these flows if > >> is_chassis_resident(logical_port) == false. If I understand correctly we > >> can't, but just to be safe it's probably fine to add the > >> is_chassis_resident(logical_port) check. > >> > >>> In addition, I'd suggest to add more clear comment to state the reason > >>> why setting reg1 and eth.src. For example: it is done to make sure when > >>> ARP request is triggered (in next stage), the ARP generated can have > the > >>> eth.src = xxx and src IP as yyy, so that it can bypass the flow that > >>> redirect the ARP request to GW node ... > >>> > >>> Thanks, > >>> Han > >>> > >>>> + */ > >>>> if (distributed) { > >>>> + ds_clear(&match); > >>>> + ds_clear(&actions); > >>>> + ds_put_format(&match, > >>>> + "ip%s.src == %s && > >>> is_chassis_resident(\"%s\")", > >>>> + is_v6 ? "6" : "4", nat->logical_ip, > >>>> + nat->logical_port); > >>>> + ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; > >>> next;", > >>>> + nat->external_mac, is_v6 ? "xx" : "", > >>>> + nat->external_ip); > >>>> + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, > 150, > >>>> + ds_cstr(&match), ds_cstr(&actions)); > >>>> + > >>>> ds_clear(&match); > >>>> ds_put_format(&match, "ip%s.src == %s && outport == > %s", > >>>> is_v6 ? "6" : "4", > >>>> diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at < > http://ovn.at> > >>>> index 8fa1a7e1b..15b40ca1e 100644 > >>>> --- a/tests/ovn.at <http://ovn.at> > >>>> +++ b/tests/ovn.at <http://ovn.at> > >>>> @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ > >>>> set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ > >>>> options:tx_pcap=hv2/vif1-tx.pcap \ > >>>> options:rxq_pcap=hv2/vif1-rx.pcap \ > >>>> - ofport-request=1 > >>>> + ofport-request=2 > >>>> +ovs-vsctl -- add-port br-int hv2-vif2 -- \ > >>>> + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ > >>>> + options:tx_pcap=hv2/vif2-tx.pcap \ > >>>> + options:rxq_pcap=hv2/vif2-rx.pcap \ > >>>> + ofport-request=3 > >>>> > >>>> -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 > >>>> +ovn-nbctl create Logical_Router name=lr0 > >>>> ovn-nbctl ls-add sw0 > >>>> ovn-nbctl ls-add sw1 > >>>> > >>>> @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set > >>> Logical_Switch_Port rp-sw0 \ > >>>> type=router options:router-port=sw0 \ > >>>> -- lsp-set-addresses rp-sw0 router > >>>> > >>>> -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > >>> <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 > >>>> +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 > >>> <http://172.16.1.1/24> 2002:0:0:0:0:0:0:1/64 \ > >>>> + -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" > >>>> ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ > >>>> type=router options:router-port=sw1 \ > >>>> -- lsp-set-addresses rp-sw1 router > >>>> > >>>> ovn-nbctl lsp-add sw0 sw0-p0 \ > >>>> -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 > 2001::2" > >>>> +ovn-nbctl lsp-add sw0 sw0-p1 \ > >>>> + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 > 2001::3" > >>>> > >>>> ovn-nbctl lsp-add sw1 sw1-p0 \ > >>>> -- lsp-set-addresses sw1-p0 unknown > >>>> @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 > >>> $router_ip6 > >>>> > >>>> OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > >>>> > >>>> +# Create FIP on sw0-p0, add a route on logical router pipeline and > >>>> +# ARP request for a unkwon destination is sent using FIP MAC/IP > >>>> +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 > >>> f0:00:00:01:02:04 > >>>> +ovn-nbctl lr-route-add lr0 172.16.2.0/24 <http://172.16.2.0/24> > >>> 172.16.1.11 > >>>> + > >>>> +dst_ip=$(ip_to_hex 172 16 2 10) > >>>> +fip_ip=$(ip_to_hex 172 16 1 2) > >>>> +src_ip=$(ip_to_hex 192 168 1 3) > >>>> +gw_router=$(ip_to_hex 172 16 1 11) > >>>> +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 > $data > >>>> +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected > >>>> + > >>>> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > >>>> + > >>>> OVN_CLEANUP([hv1],[hv2]) > >>>> AT_CLEANUP > >>>> > >>>> diff --git a/tests/system-ovn.at <http://system-ovn.at> > >>> b/tests/system-ovn.at <http://system-ovn.at> > >>>> index 9ae6c6b1f..99a0ee07b 100644 > >>>> --- a/tests/system-ovn.at <http://system-ovn.at> > >>>> +++ b/tests/system-ovn.at <http://system-ovn.at> > >>>> @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, " > 172.16.1.2/24 > >>> <http://172.16.1.2/24>", "f0:00:00:01:02:05", \ > >>>> ovn-nbctl lsp-add alice alice1 \ > >>>> -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" > >>>> > >>>> +# Add external network > >>>> +ADD_NAMESPACES(ext-net) > >>>> +ip link add alice-ext netns alice1 type veth peer name ext-veth netns > >>> ext-net > >>>> +ip -n ext-net link set dev ext-veth up > >>>> +ip -n ext-net addr add 10.0.0.1/24 <http://10.0.0.1/24> dev ext-veth > >>>> +ip -n ext-net route add default via 10.0.0.2 > >>>> + > >>>> +ip -n alice1 link set dev alice-ext up > >>>> +ip -n alice1 addr add 10.0.0.2/24 <http://10.0.0.2/24> dev alice-ext > >>>> +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 > >>>> + > >> > >> Shouldn't we use NS_CHECK_EXEC() instead of "ip -n"/"ip netns"? > > > > Hi Dumitru, > > > > thx for the review. I guess we can use NS_EXEC instead, right? > > > > I guess we could but we don't use NS_EXEC() anywhere in the > system-ovn.at tests. Also I don't see any harm in the additional checks > done by NS_CHECK_EXEC(). > I'm not sure in this particular case, but in this patch [1] which I submitted a while back, I couldn't use NS_CHECK_EXEC() and hence used "ip netns " directly. I wanted to do "nc 10.0.0.4 84 2> r" using NS_CHECK_EXEC() but it failed for some reason. [1] - https://github.com/ovn-org/ovn/commit/f792b1a00b439a949e3b7aae4951f8513340c1a1#diff-eaa18a11c5b23c901f9a4dd849c58db6R3802 > Thanks, > Dumitru > > > Regards, > > Lorenzo > > > >> > >> Regards, > >> Dumitru > >> > >>>> # Add DNAT rules > >>>> AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 > >>> 192.168.1.2 foo1 00:00:02:02:03:04]) > >>>> AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 > >>> 192.168.1.3 foo2 00:00:02:02:03:05]) > >>>> @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat > >>> 172.16.1.4 192.168.1.3 foo2 00:0 > >>>> # Add a SNAT rule > >>>> AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16 > >>> <http://192.168.0.0/16>]) > >>>> > >>>> +# Add default route to ext-net > >>>> +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 <http://10.0.0.0/24> > >>> 172.16.1.2]) > >>>> + > >>>> ovn-nbctl --wait=hv sync > >>>> OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep > >>> 'nat(src=172.16.1.1)']) > >>>> > >>>> @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], > >>> [dnl > >>>> > >>> > icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > >>>> ]) > >>>> > >>>> +# Try to ping external network > >>>> +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 > >>> and icmp > ext-net.pcap &]) > >>>> +sleep 1 > >>>> +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) > >>>> +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | > >>> FORMAT_PING], \ > >>>> +[0], [dnl > >>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms > >>>> +]) > >>>> + > >>>> +OVS_WAIT_UNTIL([ > >>>> + total_pkts=$(cat ext-net.pcap | wc -l) > >>>> + test "${total_pkts}" = "3" > >>>> +]) > >>>> + > >>>> OVS_APP_EXIT_AND_WAIT([ovn-controller]) > >>>> > >>>> as ovn-sb > >>>> -- > >>>> 2.26.2 > >>>> > >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 9423cbfd1..5e37d9758 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2875,6 +2875,18 @@ icmp4 { </p> <ul> + <li> + For each NAT rule in the OVN Northbound database that can + be handled in a distributed manner, a priorirty-150 logical + flow with match <code>ip.src == <var>A</var> && + is_chassis_resident(<var>B</var>)</code>, where <var>A</var> + is NAT logical ip and <var>B</var> is NAT logical port. + IP traffic matching the above rule will be managed locally + setting <code>reg1</code> to <var>C</var> and <code>eth.src</code> + to <var>D</var>, where <var>C</var> is NAT external ip and + <var>D</var> is NAT external mac. + </li> + <li> For each NAT rule in the OVN Northbound database that can be handled in a distributed manner, a priority-100 logical diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 7ae5e45da..20f0ee2e4 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -9181,8 +9181,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, /* Ingress Gateway Redirect Table: For NAT on a distributed * router, add flows that are specific to a NAT rule. These * flows indicate the presence of an applicable NAT rule that - * can be applied in a distributed manner. */ + * can be applied in a distributed manner. + * Moreover add logical flows to avoid ARP distributing when the + * chassis has a direct connection to the underlay network through + * a localnet port + */ if (distributed) { + ds_clear(&match); + ds_clear(&actions); + ds_put_format(&match, + "ip%s.src == %s && is_chassis_resident(\"%s\")", + is_v6 ? "6" : "4", nat->logical_ip, + nat->logical_port); + ds_put_format(&actions, "eth.src = %s; %sreg1 = %s; next;", + nat->external_mac, is_v6 ? "xx" : "", + nat->external_ip); + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 150, + ds_cstr(&match), ds_cstr(&actions)); + ds_clear(&match); ds_put_format(&match, "ip%s.src == %s && outport == %s", is_v6 ? "6" : "4", diff --git a/tests/ovn.at b/tests/ovn.at index 8fa1a7e1b..15b40ca1e 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -14943,9 +14943,14 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \ set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ options:tx_pcap=hv2/vif1-tx.pcap \ options:rxq_pcap=hv2/vif1-rx.pcap \ - ofport-request=1 + ofport-request=2 +ovs-vsctl -- add-port br-int hv2-vif2 -- \ + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv2/vif2-tx.pcap \ + options:rxq_pcap=hv2/vif2-rx.pcap \ + ofport-request=3 -ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 +ovn-nbctl create Logical_Router name=lr0 ovn-nbctl ls-add sw0 ovn-nbctl ls-add sw1 @@ -14954,13 +14959,16 @@ ovn-nbctl lsp-add sw0 rp-sw0 -- set Logical_Switch_Port rp-sw0 \ type=router options:router-port=sw0 \ -- lsp-set-addresses rp-sw0 router -ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002:0:0:0:0:0:0:1/64 +ovn-nbctl lrp-add lr0 sw1 00:00:02:01:02:03 172.16.1.1/24 2002:0:0:0:0:0:0:1/64 \ + -- set Logical_Router_Port sw1 options:redirect-chassis="hv2" ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \ type=router options:router-port=sw1 \ -- lsp-set-addresses rp-sw1 router ovn-nbctl lsp-add sw0 sw0-p0 \ -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2 2001::2" +ovn-nbctl lsp-add sw0 sw0-p1 \ + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3 2001::3" ovn-nbctl lsp-add sw1 sw1-p0 \ -- lsp-set-addresses sw1-p0 unknown @@ -15006,6 +15014,20 @@ send_na 2 1 $dst_mac $router_mac1 $dst_ip6 $router_ip6 OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) +# Create FIP on sw0-p0, add a route on logical router pipeline and +# ARP request for a unkwon destination is sent using FIP MAC/IP +ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.16.1.2 192.168.1.3 sw0-p1 f0:00:00:01:02:04 +ovn-nbctl lr-route-add lr0 172.16.2.0/24 172.16.1.11 + +dst_ip=$(ip_to_hex 172 16 2 10) +fip_ip=$(ip_to_hex 172 16 1 2) +src_ip=$(ip_to_hex 192 168 1 3) +gw_router=$(ip_to_hex 172 16 1 11) +send_icmp_packet 2 2 f00000110203 $router_mac0 $src_ip $dst_ip 0000 $data +echo $(get_arp_req f00000010204 $fip_ip $gw_router) >> expected + +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) + OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 9ae6c6b1f..99a0ee07b 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -2747,6 +2747,17 @@ ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24", "f0:00:00:01:02:05", \ ovn-nbctl lsp-add alice alice1 \ -- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2" +# Add external network +ADD_NAMESPACES(ext-net) +ip link add alice-ext netns alice1 type veth peer name ext-veth netns ext-net +ip -n ext-net link set dev ext-veth up +ip -n ext-net addr add 10.0.0.1/24 dev ext-veth +ip -n ext-net route add default via 10.0.0.2 + +ip -n alice1 link set dev alice-ext up +ip -n alice1 addr add 10.0.0.2/24 dev alice-ext +ip netns exec alice1 sysctl -w net.ipv4.conf.all.forwarding=1 + # Add DNAT rules AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.3 192.168.1.2 foo1 00:00:02:02:03:04]) AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:00:02:02:03:05]) @@ -2754,6 +2765,9 @@ AT_CHECK([ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.1.4 192.168.1.3 foo2 00:0 # Add a SNAT rule AT_CHECK([ovn-nbctl lr-nat-add R1 snat 172.16.1.1 192.168.0.0/16]) +# Add default route to ext-net +AT_CHECK([ovn-nbctl lr-route-add R1 10.0.0.0/24 172.16.1.2]) + ovn-nbctl --wait=hv sync OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep 'nat(src=172.16.1.1)']) @@ -2797,6 +2811,20 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> ]) +# Try to ping external network +NS_CHECK_EXEC([ext-net], [tcpdump -n -c 3 -i ext-veth dst 172.16.1.3 and icmp > ext-net.pcap &]) +sleep 1 +AT_CHECK([ovn-nbctl lr-nat-del R1 snat]) +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.1 | FORMAT_PING], \ +[0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +OVS_WAIT_UNTIL([ + total_pkts=$(cat ext-net.pcap | wc -l) + test "${total_pkts}" = "3" +]) + OVS_APP_EXIT_AND_WAIT([ovn-controller]) as ovn-sb
Introduce 150-priority logical flows for each NAT rule that can be handled in a distributed manner to manage ARP request locally for FIP traffic. In particular set reg1 and eth.src to NAT external ip and NAT external mac respectivelly and do not distribute ARP traffic using FIP Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- northd/ovn-northd.8.xml | 12 ++++++++++++ northd/ovn-northd.c | 18 +++++++++++++++++- tests/ovn.at | 28 +++++++++++++++++++++++++--- tests/system-ovn.at | 28 ++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 4 deletions(-)