On 5/26/20 5:23 AM, Han Zhou wrote:
>
>
> On Mon, May 25, 2020 at 2:55 PM Lorenzo Bianconi
> <lorenzo.bianc...@redhat.com <mailto:lorenzo.bianc...@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.bianc...@redhat.com
> <mailto:lorenzo.bianc...@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
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
