On 3/24/23 10:11, Ales Musil wrote:
> On Wed, Mar 22, 2023 at 2:44 PM Enrique Llorente <ellor...@redhat.com>
> wrote:
>
>> The localnet LSPs skip responding ARP/NDP [1], this change relax this
>> restriction by allowing responding from localnet LSPs when owner
>> switch has a router LSP with a proper arp_proxy configuration.
>>
>> [1] ovn-northd.8.xml
>>
>> Signed-off-by: Enrique Llorente <ellor...@redhat.com>
>> ---
>> northd/northd.c | 19 ++++++++++++-------
>> northd/northd.h | 1 +
>> northd/ovn-northd.8.xml | 7 ++++---
>> tests/system-ovn.at | 34 ++++++++++++++++++++++++++++++----
>> 4 files changed, 47 insertions(+), 14 deletions(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 5f0b436c2..ee44a2e8b 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -2721,12 +2721,17 @@ join_logical_ports(struct northd_input *input_data,
>> */
>> const char *arp_proxy =
>> smap_get(&op->nbsp->options,"arp_proxy");
>> int ofs = 0;
>> - if (arp_proxy &&
>> - !extract_addresses(arp_proxy, &op->proxy_arp_addrs, &ofs)
>> &&
>> - !extract_ip_addresses(arp_proxy, &op->proxy_arp_addrs)) {
>> - static struct vlog_rate_limit rl =
>> VLOG_RATE_LIMIT_INIT(1, 5);
>> - VLOG_WARN_RL(&rl, "Invalid arp_proxy option: '%s' at lsp
>> '%s'",
>> - arp_proxy, op->nbsp->name);
>> + if (arp_proxy) {
>> + if (extract_addresses(arp_proxy, &op->proxy_arp_addrs,
>> &ofs) ||
>> + extract_ip_addresses(arp_proxy,
>> &op->proxy_arp_addrs)) {
>> + op->od->has_arp_proxy_port = true;
>> + } else {
>> + static struct vlog_rate_limit rl =
>> + VLOG_RATE_LIMIT_INIT(1, 5);
>> + VLOG_WARN_RL(&rl,
>> + "Invalid arp_proxy option: '%s' at lsp '%s'",
>> + arp_proxy, op->nbsp->name);
>> + }
>> }
>> } else if (op->nbrp && op->nbrp->peer && !op->l3dgw_port) {
>> struct ovn_port *peer = ovn_port_find(ports, op->nbrp->peer);
>> @@ -8428,7 +8433,7 @@ build_lswitch_arp_nd_responder_skip_local(struct
>> ovn_port *op,
>> struct hmap *lflows,
>> struct ds *match)
>> {
>> - if (op->nbsp && lsp_is_localnet(op->nbsp)) {
>> + if (op->nbsp && lsp_is_localnet(op->nbsp) &&
>> !op->od->has_arp_proxy_port) {
>>
>
> This is checked at single place so we can do probably do !(op->od>
>
>
>> ds_clear(match);
>> ds_put_format(match, "inport == %s", op->json_key);
>> ovn_lflow_add_with_lport_and_hint(lflows, op->od,
>> diff --git a/northd/northd.h b/northd/northd.h
>> index 4d9055296..5a666bbd3 100644
>> --- a/northd/northd.h
>> +++ b/northd/northd.h
>> @@ -214,6 +214,7 @@ struct ovn_datapath {
>> bool has_unknown;
>> bool has_acls;
>> bool has_vtep_lports;
>> + bool has_arp_proxy_port;
>>
>> /* IPAM data. */
>> struct ipam_info ipam_info;
>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>> index 5d513e65a..7d400a7b8 100644
>> --- a/northd/ovn-northd.8.xml
>> +++ b/northd/ovn-northd.8.xml
>> @@ -1269,9 +1269,10 @@
>>
>> <ul>
>> <li>
>> - Priority-100 flows to skip the ARP responder if inport is of type
>> - <code>localnet</code> advances directly to the next table. ARP
>> - requests sent to <code>localnet</code> ports can be received by
>> + If the logical switch has no router ports with options:arp_proxy
>> + configured add a priority-100 flows to skip the ARP responder if
>> inport
>> + is of type <code>localnet</code> advances directly to the next
>> table.
>> + ARP requests sent to <code>localnet</code> ports can be received
>> by
>> multiple hypervisors. Now, because the same mac binding rules are
>> downloaded to all hypervisors, each of the multiple hypervisors
>> will
>> respond. This will confuse L2 learning on the source of the ARP
>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>> index ad1188078..f820a5206 100644
>> --- a/tests/system-ovn.at
>> +++ b/tests/system-ovn.at
>> @@ -10431,6 +10431,8 @@ AT_SKIP_IF([test $HAVE_TCPDUMP = no])
>> ovn_start
>> OVS_TRAFFIC_VSWITCHD_START()
>> ADD_BR([br-int])
>> +ADD_BR([br-ext])
>> +ovs-ofctl add-flow br-ext action=normal
>>
>> # Set external-ids in br-int needed for ovn-controller
>> ovs-vsctl \
>> @@ -10447,7 +10449,9 @@ start_daemon ovn-controller
>> # One LR - R1 and two LSs - foo and bar, R1 has switches foo (
>> 192.168.1.0/24) and
>> # bar (192.168.2.0/24) connected to it
>> #
>> -# foo -- R1 -- bar
>> +# br-ext -- localnet -- foo -- R1 -- bar
>> +
>> +check ovs-vsctl set open .
>> external-ids:ovn-bridge-mappings=provider:br-ext
>>
>> check ovn-nbctl lr-add R1
>> check ovn-nbctl ls-add foo
>> @@ -10456,13 +10460,14 @@ check ovn-nbctl ls-add bar
>> # Connect foo to R1
>> check ovn-nbctl lrp-add R1 foo 00:00:01:01:02:03 192.168.1.1/24
>> check ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \
>> - type=router options:arp_proxy="0a:58:a9:fe:01:01 169.254.239.254
>> 169.254.239.2 169.254.238.0/24 " options:router-port=foo
>> addresses='"router"'
>> + type=router options:arp_proxy="0a:58:a9:fe:01:01 169.254.239.254
>> 169.254.239.2 169.254.238.0/24 192.168.1.100" options:router-port=foo
>> addresses='"router"'
>>
>> # Connect bar to R1
>> check ovn-nbctl lrp-add R1 bar 00:00:01:01:02:04 192.168.2.1/24
>> check ovn-nbctl lsp-add bar rp-bar -- set Logical_Switch_Port rp-bar \
>> type=router options:arp_proxy="169.254.239.253"
>> options:router-port=bar addresses='"router"'
>>
>> +
>> # Logical port 'foo1' in switch 'foo'.
>> ADD_NAMESPACES(foo1)
>> ADD_VETH(foo1, foo1, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \
>> @@ -10484,12 +10489,20 @@ ADD_VETH(foo3, foo3, br-int, "192.168.1.4/24",
>> "f0:00:00:01:02:05", \
>> check ovn-nbctl lsp-add foo foo3 \
>> -- lsp-set-addresses foo3 "f0:00:00:01:02:05 192.168.1.4"
>>
>> +# Logical port 'ext1' in switch 'foo'
>> +ADD_NAMESPACES(ext1)
>> +ADD_VETH(ext1, ext1, br-ext, "192.168.1.5/24", "f0:00:00:01:02:06", \
>> + "192.168.1.100",)
>> +check ovn-nbctl lsp-add foo ln -- lsp-set-options ln network_name=provider
>> +check ovn-nbctl lsp-set-type ln localnet
>> +check ovn-nbctl lsp-set-addresses ln unknown
>> +
>> # Logical port 'bar1' in switch 'bar'.
>> ADD_NAMESPACES(bar1)
>> -ADD_VETH(bar1, bar1, br-int, "192.168.2.2/24", "f0:00:00:01:02:06", \
>> +ADD_VETH(bar1, bar1, br-int, "192.168.2.2/24", "f0:00:00:01:02:07", \
>> "169.254.239.253")
>> check ovn-nbctl lsp-add bar bar1 \
>> --- lsp-set-addresses bar1 "f0:00:00:01:02:06 192.168.2.2"
>> +-- lsp-set-addresses bar1 "f0:00:00:01:02:07 192.168.2.2"
>>
>> # wait for ovn-controller to catch up.
>> check ovn-nbctl --wait=hv sync
>> @@ -10533,6 +10546,19 @@ OVS_WAIT_UNTIL([
>> test "${total_pkts}" = "3"
>> ])
>>
>> +NETNS_DAEMONIZE([ext1], [tcpdump -l -nn -e -i ext1 'ether dst
>> 0a:58:a9:fe:01:01 and icmp' > ext1-icmp.pcap 2>ext1-tcpdump.stderr],
>> [ext1-icmp-tcpdump.pid])
>> +OVS_WAIT_UNTIL([grep "listening" ext1-tcpdump.stderr])
>> +
>> +# 'ext1' should be able to ping 'bar1'
>> +NS_CHECK_EXEC([ext1], [ping -q -c 3 -i 0.3 -w 2 192.168.2.2 |
>> FORMAT_PING], \
>> +[0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +OVS_WAIT_UNTIL([
>> + total_pkts=$(cat ext1-icmp.pcap| wc -l)
>> + test "${total_pkts}" = "3"
>> +])
>> +
>>
>> OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>
>> --
>> 2.32.0
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Looks good to me, thanks.
>
> Acked-by: Ales Musil <amu...@redhat.com>
>
Thanks, Enrique and Ales! I checked with Ales offline and his comment
above was just a leftover, no change was needed. I rebased this and
applied it to the main branch.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
