On Tue, Jun 6, 2023 at 11:37 PM Ihar Hrachyshka <ihrac...@redhat.com> wrote:
> On Tue, May 30, 2023 at 6:00 AM Ales Musil <amu...@redhat.com> wrote: > >> Skip GARP packet with link-local address being advertised >> when "always_learn_from_arp_request=false", this should >> prevent huge grow of MAC Binding table. To keep the option >> consistent overwrite the previous MAC with LLA if it was >> already stored in DB. >> >> Signed-off-by: Ales Musil <amu...@redhat.com> >> --- >> v2: Remove leftover from previous tests. >> --- >> northd/northd.c | 11 +++++++ >> tests/ovn.at | 78 ++++++++++++++++++++++++++++++++----------------- >> 2 files changed, 63 insertions(+), 26 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index a6eca916b..ff305fc67 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -11951,6 +11951,17 @@ build_neigh_learning_flows_for_lrouter( >> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 100, "nd_ns", >> ds_cstr(actions)); >> >> + if (!learn_from_arp_request) { >> + /* Add flow to skip LLA only if we don't know it already. */ >> > > Sorry for pedantry, but this "only" made me confused for a bit. With it, > the comment may be (mis?)read as saying that "if we don't know it already - > then we skip, OTHERWISE - we don't skip." And while in effect it's true > (combined with the other flow for nd_na slightly above), the "OTHERWISE" > part is not implemented by this new flow. > > I think I would be slightly less confused if this comment said > > "Add flow to skip LLA if we don't know it already." (without `only`). > > Again, sorry for pedantry. > No worries, it sounds perfectly reasonable. > > --- > > I also wonder if moving this flow setup closer to the other flow to handle > nd_na would help with reading the code (to have all the effects related to > nd_na in a single place). > Done in v3. > > >> + ds_clear(actions); >> + ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT >> + " = lookup_nd(inport, ip6.src, nd.tll); " >> + REGBIT_LOOKUP_NEIGHBOR_IP_RESULT >> + " = lookup_nd_ip(inport, ip6.src); next;"); >> + ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 110, >> + "nd_na && ip6.src == fe80::/10", ds_cstr(actions)); >> + } >> + >> /* For other packet types, we can skip neighbor learning. >> * So set REGBIT_LOOKUP_NEIGHBOR_RESULT to 1. */ >> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 0, "1", >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 6f9fbbfd2..7a2658f40 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -5073,6 +5073,7 @@ AT_CLEANUP >> >> OVN_FOR_EACH_NORTHD([ >> AT_SETUP([IP relocation using GARP request]) >> +AT_SKIP_IF([test $HAVE_SCAPY = no]) >> > > Thank you!! > > >> ovn_start >> >> # Logical network: >> @@ -5172,7 +5173,9 @@ done >> test_ip() { >> # This packet has bad checksums but logical L3 routing doesn't check. >> local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 >> - local >> packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 >> + local packet=$(fmt_pkt "Ether(dst='${dst_mac}', src='${src_mac}')/ \ >> + IP(dst='${dst_ip}', src='${src_ip}')/ \ >> + UDP(sport=53, dport=4369)") >> shift; shift; shift; shift; shift >> hv=hv`vif_to_hv $inport` >> as $hv ovs-appctl netdev-dummy/receive vif$inport $packet >> @@ -5187,7 +5190,9 @@ test_ip() { >> # Routing decrements TTL and updates source and dest MAC >> # (and checksum). >> out_lrp=`vif_to_lrp $outport` >> - echo >> f000000000${outport}00000000ff0${out_lrp}08004500001c00000000"3f1101"00${src_ip}${dst_ip}0035111100080000 >> + echo $(fmt_pkt "Ether(dst='f0:00:00:00:00:${outport}', >> src='00:00:00:00:ff:${out_lrp}')/ \ >> + IP(src='${src_ip}', dst='${dst_ip}', >> ttl=63)/ \ >> + UDP(sport=53, dport=4369)") >> fi >> $outport.expected >> done >> } >> @@ -5203,8 +5208,10 @@ test_ip() { >> # SHA and REPLY_HA are each 12 hex digits. >> # SPA and TPA are each 8 hex digits. >> test_arp() { >> - local inport=$1 sha=$2 spa=$3 tpa=$4 reply_ha=$5 >> - local >> request=ffffffffffff${sha}08060001080006040001${sha}${spa}ffffffffffff${tpa} >> + local inport=$1 sha=$2 spa=$3 tpa=$3 >> + local request=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', >> src='${sha}')/ \ >> + ARP(hwsrc='${sha}', >> hwdst='ff:ff:ff:ff:ff:ff', psrc='${spa}', pdst='${tpa}')") >> + >> hv=hv`vif_to_hv $inport` >> as $hv ovs-appctl netdev-dummy/receive vif$inport $request >> >> @@ -5217,53 +5224,72 @@ test_arp() { >> echo $request >> $i$j$k.expected >> fi >> done >> +} >> >> - # Expect to receive the reply, if any. >> - if test X$reply_ha != X; then >> - lrp=`vif_to_lrp $inport` >> - local >> reply=${sha}00000000ff0${lrp}08060001080006040002${reply_ha}${tpa}${sha}${spa} >> - echo $reply >> $inport.expected >> - fi >> +test_na() { >> + local inport=$1 sha=$2 spa=$3 >> + local request=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', >> src='${sha}')/ \ >> + IPv6(dst='fe80::1', src='${spa}')/ \ >> + ICMPv6ND_NA(tgt='${spa}')") >> + >> + hv=hv`vif_to_hv $inport` >> + as $hv ovs-appctl netdev-dummy/receive vif$inport $request >> + >> + # Expect to receive the broadcast ARP on the other logical switch >> ports if >> + # IP address is not configured to the switch patch port. >> + local i=`vif_to_ls $inport` >> + local j >> + for j in 1 2; do >> + if test $i$j != $inport; then >> + echo $request >> $i$j$k.expected >> + fi >> + done >> } >> >> -# lp11 send GARP request to announce ownership of 192.168.1.100. >> +# lp11 send GARP request to announce ownership of 192.168.1.100 and >> fe80::abcd:1. >> >> -sha=f00000000011 >> -spa=`ip_to_hex 192 168 1 100` >> -tpa=$spa >> +sha="f0:00:00:00:00:11" >> +spa="192.168.1.100" >> +spa6="fe80::abcd:1" >> >> # When always_learn_from_arp_request=false, the new mac-binding will not >> be learned >> # through GARP request. >> ovn-nbctl --wait=hv set logical_router lr0 >> options:always_learn_from_arp_request=false >> >> -test_arp 11 $sha $spa $tpa >> +test_arp 11 $sha $spa >> +test_na 11 $sha $spa6 >> sleep 1 >> -check_row_count MAC_Binding 0 ip="192.168.1.100" >> +check_row_count MAC_Binding 0 ip="$spa" >> +check_row_count MAC_Binding 0 ip="$spa6" >> >> # When always_learn_from_arp_request=true, the new mac-binding will be >> learned. >> ovn-nbctl --wait=hv set logical_router lr0 >> options:always_learn_from_arp_request=true >> >> -test_arp 11 $sha $spa $tpa >> -OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding ip="192.168.1.100" | wc >> -l` -gt 0]) >> +test_arp 11 $sha $spa >> +test_na 11 $sha $spa6 >> +wait_row_count MAC_Binding 1 ip="$spa" mac=\"$sha\" >> +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\" >> ovn-nbctl --wait=hv sync >> >> # Send an IP packet from lp21 to 192.168.1.100, which should go to lp11. >> >> -smac=f00000000021 >> -dmac=00000000ff02 >> -sip=`ip_to_hex 192 168 2 11` >> -dip=`ip_to_hex 192 168 1 100` >> +smac="f0:00:00:00:00:21" >> +dmac="00:00:00:00:ff:02" >> +sip="192.168.2.11" >> +dip="192.168.1.100" >> test_ip 21 $smac $dmac $sip $dip 11 >> >> -# lp12 send GARP request to announce ownership of 192.168.1.100. >> +# lp12 send GARP request to announce ownership of 192.168.1.100 and >> fe80::abcd:1. >> >> # Even when always_learn_from_arp_request=false, the existing >> mac-binding should be >> # updated through GARP request. >> ovn-nbctl --wait=hv set logical_router lr0 >> options:always_learn_from_arp_request=false >> >> -sha=f00000000012 >> -test_arp 12 $sha $spa $tpa >> -wait_row_count MAC_Binding 1 ip="192.168.1.100" mac='"f0:00:00:00:00:12"' >> +sha="f0:00:00:00:00:12" >> +test_arp 12 $sha $spa >> +test_na 11 $sha $spa6 >> +wait_row_count MAC_Binding 1 ip="$spa" mac=\"$sha\" >> +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\" >> ovn-nbctl --wait=hv sync >> # give to the hv the time to send queued ip packets >> sleep 1 >> -- >> 2.40.1 >> >> _______________________________________________ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> amu...@redhat.com IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev