| Message ID | 20240118145426.271860-1-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] northd: Use proper field for lookup_nd | 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 |
Hi Ales The patch looks good to me, Thanks! Acked-by: Xavier Simonart <xsimonar@redhat.com> Thanks Xavier On Thu, Jan 18, 2024 at 3:54 PM Ales Musil <amusil@redhat.com> wrote: > We are intentionally skipping ND NA with LLA as source. > However, this doesn't work when the ND NA has LLA source, > but the target address is global one. In that case we > would skip update of already existing entry when > always_learn_from_arp_request is set to false. > > Use ND target address in the check instead of source. > > Fixes: 5e0cb03605ea ("northd: Add logical flow to skip GARP with LLA") > Reported-at: https://issues.redhat.com/browse/FDP-283 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > northd/northd.c | 4 ++-- > tests/ovn.at | 25 ++++++++++++++++++++++--- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 952f8200d..de15ca101 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -13171,9 +13171,9 @@ build_neigh_learning_flows_for_lrouter( > * address, the all-nodes multicast address. */ > ds_clear(actions); > ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT > - " = lookup_nd(inport, ip6.src, nd.tll); " > + " = lookup_nd(inport, nd.target, nd.tll); " > REGBIT_LOOKUP_NEIGHBOR_IP_RESULT > - " = lookup_nd_ip(inport, ip6.src); next;"); > + " = lookup_nd_ip(inport, nd.target); > next;"); > ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 110, > "nd_na && ip6.src == fe80::/10 && ip6.dst == > ff00::/8", > ds_cstr(actions)); > diff --git a/tests/ovn.at b/tests/ovn.at > index 2dd46fd79..26c516ef9 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -5385,10 +5385,11 @@ test_arp() { > } > > test_na() { > - local inport=$1 sha=$2 spa=$3 > + local inport=$1 sha=$2 spa=$3 src=${4-$3} > local request=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', > src='${sha}')/ \ > - IPv6(dst='ff01::1', src='${spa}')/ \ > - ICMPv6ND_NA(tgt='${spa}')") > + IPv6(dst='ff01::1', src='${src}')/ \ > + ICMPv6ND_NA(tgt='${spa}')/ \ > + ICMPv6NDOptDstLLAddr(lladdr='${sha}')") > > hv=hv`vif_to_hv $inport` > as $hv ovs-appctl netdev-dummy/receive vif$inport $request > @@ -5464,6 +5465,24 @@ for i in 1 2; do > done > done > > +# Make sure that we can update existing entry with > +# "always_learn_from_arp_request=false" even when the source of NA src is > LLA. > +ovn-sbctl --all destroy mac_binding > +ovn-nbctl --wait=hv set logical_router lr0 > options:always_learn_from_arp_request=false > + > +sha="f0:00:00:00:00:11" > +spa6="fd00::abcd:1" > + > +test_na 11 $sha $spa6 > +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\" > + > +sha="f0:00:00:00:00:12" > +lla6="fe80::abcd:1" > + > +test_na 11 $sha $spa6 $lla6 > +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\" > +check_row_count MAC_Binding 0 ip=\"$lla6\" > + > # Gracefully terminate daemons > OVN_CLEANUP([hv1], [hv2]) > > -- > 2.43.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On 1/19/24 10:28, Xavier Simonart wrote: > Hi Ales > > The patch looks good to me, Thanks! > Acked-by: Xavier Simonart <xsimonar@redhat.com> > > Thanks > Xavier > > On Thu, Jan 18, 2024 at 3:54 PM Ales Musil <amusil@redhat.com> wrote: > >> We are intentionally skipping ND NA with LLA as source. >> However, this doesn't work when the ND NA has LLA source, >> but the target address is global one. In that case we >> would skip update of already existing entry when >> always_learn_from_arp_request is set to false. >> >> Use ND target address in the check instead of source. >> >> Fixes: 5e0cb03605ea ("northd: Add logical flow to skip GARP with LLA") >> Reported-at: https://issues.redhat.com/browse/FDP-283 >> Signed-off-by: Ales Musil <amusil@redhat.com> >> --- >> northd/northd.c | 4 ++-- >> tests/ovn.at | 25 ++++++++++++++++++++++--- >> 2 files changed, 24 insertions(+), 5 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index 952f8200d..de15ca101 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -13171,9 +13171,9 @@ build_neigh_learning_flows_for_lrouter( >> * address, the all-nodes multicast address. */ >> ds_clear(actions); >> ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT >> - " = lookup_nd(inport, ip6.src, nd.tll); " >> + " = lookup_nd(inport, nd.target, nd.tll); " >> REGBIT_LOOKUP_NEIGHBOR_IP_RESULT >> - " = lookup_nd_ip(inport, ip6.src); next;"); >> + " = lookup_nd_ip(inport, nd.target); >> next;"); >> ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 110, >> "nd_na && ip6.src == fe80::/10 && ip6.dst == >> ff00::/8", >> ds_cstr(actions)); >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 2dd46fd79..26c516ef9 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -5385,10 +5385,11 @@ test_arp() { >> } >> >> test_na() { >> - local inport=$1 sha=$2 spa=$3 >> + local inport=$1 sha=$2 spa=$3 src=${4-$3} >> local request=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', >> src='${sha}')/ \ >> - IPv6(dst='ff01::1', src='${spa}')/ \ >> - ICMPv6ND_NA(tgt='${spa}')") >> + IPv6(dst='ff01::1', src='${src}')/ \ >> + ICMPv6ND_NA(tgt='${spa}')/ \ >> + ICMPv6NDOptDstLLAddr(lladdr='${sha}')") >> >> hv=hv`vif_to_hv $inport` >> as $hv ovs-appctl netdev-dummy/receive vif$inport $request >> @@ -5464,6 +5465,24 @@ for i in 1 2; do >> done >> done >> >> +# Make sure that we can update existing entry with >> +# "always_learn_from_arp_request=false" even when the source of NA src is >> LLA. >> +ovn-sbctl --all destroy mac_binding >> +ovn-nbctl --wait=hv set logical_router lr0 >> options:always_learn_from_arp_request=false Thanks, Ales and Xavier! I added the missing "check" calls here and pushed this to main and backported it all the way down to 22.03. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 952f8200d..de15ca101 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -13171,9 +13171,9 @@ build_neigh_learning_flows_for_lrouter( * address, the all-nodes multicast address. */ ds_clear(actions); ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT - " = lookup_nd(inport, ip6.src, nd.tll); " + " = lookup_nd(inport, nd.target, nd.tll); " REGBIT_LOOKUP_NEIGHBOR_IP_RESULT - " = lookup_nd_ip(inport, ip6.src); next;"); + " = lookup_nd_ip(inport, nd.target); next;"); ovn_lflow_add(lflows, od, S_ROUTER_IN_LOOKUP_NEIGHBOR, 110, "nd_na && ip6.src == fe80::/10 && ip6.dst == ff00::/8", ds_cstr(actions)); diff --git a/tests/ovn.at b/tests/ovn.at index 2dd46fd79..26c516ef9 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -5385,10 +5385,11 @@ test_arp() { } test_na() { - local inport=$1 sha=$2 spa=$3 + local inport=$1 sha=$2 spa=$3 src=${4-$3} local request=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \ - IPv6(dst='ff01::1', src='${spa}')/ \ - ICMPv6ND_NA(tgt='${spa}')") + IPv6(dst='ff01::1', src='${src}')/ \ + ICMPv6ND_NA(tgt='${spa}')/ \ + ICMPv6NDOptDstLLAddr(lladdr='${sha}')") hv=hv`vif_to_hv $inport` as $hv ovs-appctl netdev-dummy/receive vif$inport $request @@ -5464,6 +5465,24 @@ for i in 1 2; do done done +# Make sure that we can update existing entry with +# "always_learn_from_arp_request=false" even when the source of NA src is LLA. +ovn-sbctl --all destroy mac_binding +ovn-nbctl --wait=hv set logical_router lr0 options:always_learn_from_arp_request=false + +sha="f0:00:00:00:00:11" +spa6="fd00::abcd:1" + +test_na 11 $sha $spa6 +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\" + +sha="f0:00:00:00:00:12" +lla6="fe80::abcd:1" + +test_na 11 $sha $spa6 $lla6 +wait_row_count MAC_Binding 1 ip=\"$spa6\" mac=\"$sha\" +check_row_count MAC_Binding 0 ip=\"$lla6\" + # Gracefully terminate daemons OVN_CLEANUP([hv1], [hv2])
We are intentionally skipping ND NA with LLA as source. However, this doesn't work when the ND NA has LLA source, but the target address is global one. In that case we would skip update of already existing entry when always_learn_from_arp_request is set to false. Use ND target address in the check instead of source. Fixes: 5e0cb03605ea ("northd: Add logical flow to skip GARP with LLA") Reported-at: https://issues.redhat.com/browse/FDP-283 Signed-off-by: Ales Musil <amusil@redhat.com> --- northd/northd.c | 4 ++-- tests/ovn.at | 25 ++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-)