| Message ID | 20220720084851.60041-6-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | MAC binding aging mechanism | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
Hi Ales, I have one finding below. On 7/20/22 04:48, Ales Musil wrote: > Reported-at: https://bugzilla.redhat.com/2084668 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > v3: Rebase on top of current main. > Update according to the final conclusion. > --- > tests/ovn.at | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 109 insertions(+) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 7d10610fd..cbacc52c5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -32505,3 +32505,112 @@ AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0]) > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([MAC binding aging]) > +ovn_start > + > +net_add n1 > + > +AT_CHECK([ovn-nbctl ls-add public]) > +AT_CHECK([ovn-nbctl ls-add internal]) > + > +AT_CHECK([ovn-nbctl lsp-add public ln_port]) > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) > + > +AT_CHECK([ovn-nbctl lsp-add public public-gw]) > +AT_CHECK([ovn-nbctl lsp-set-type public-gw router]) > +AT_CHECK([ovn-nbctl lsp-set-addresses public-gw 00:00:00:00:10:00 router]) > +AT_CHECK([ovn-nbctl lsp-set-options public-gw router-port=gw-public]) > + > +AT_CHECK([ovn-nbctl lsp-add internal internal-gw]) > +AT_CHECK([ovn-nbctl lsp-set-type internal-gw router]) > +AT_CHECK([ovn-nbctl lsp-set-addresses internal-gw 00:00:00:00:20:00 router]) > +AT_CHECK([ovn-nbctl lsp-set-options internal-gw router-port=gw-internal]) > + > +AT_CHECK([ovn-nbctl lsp-add internal vif1]) > +AT_CHECK([ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:20:10 192.168.20.10"]) > + > +AT_CHECK([ovn-nbctl lsp-add internal vif2]) > +AT_CHECK([ovn-nbctl lsp-set-addresses vif2 "00:00:00:00:20:20 192.168.20.20"]) > + > +AT_CHECK([ovn-nbctl lr-add gw]) > +AT_CHECK([ovn-nbctl lrp-add gw gw-public 00:00:00:00:10:00 192.168.10.1/24]) > +AT_CHECK([ovn-nbctl lrp-add gw gw-internal 00:00:00:00:20:00 192.168.20.1/24]) > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-underlay > +ovn_attach n1 br-underlay 192.168.0.1 > +ovs-vsctl add-br br-phys > +ovs-vsctl -- add-port br-int vif1 -- \ > + set interface vif1 external-ids:iface-id=vif1 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap \ > + ofport-request=1 > +ovs-vsctl -- add-port br-phys ext1 -- \ > + set interface ext1 \ > + options:tx_pcap=hv1/ext1-tx.pcap \ > + options:rxq_pcap=hv1/ext1-rx.pcap \ > + ofport-request=2 > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > + > +sim_add hv2 > +as hv2 > +ovs-vsctl add-br br-underlay > +ovn_attach n1 br-underlay 192.168.0.2 > +ovs-vsctl add-br br-phys > +ovs-vsctl -- add-port br-int vif2 -- \ > + set interface vif2 external-ids:iface-id=vif2 \ > + options:tx_pcap=hv2/vif2-tx.pcap \ > + options:rxq_pcap=hv2/vif2-rx.pcap \ > + ofport-request=1 > +ovs-vsctl -- add-port br-phys ext2 -- \ > + set interface ext2 \ > + options:tx_pcap=hv2/ext2-tx.pcap \ > + options:rxq_pcap=hv2/ext2-rx.pcap \ > + ofport-request=2 > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > + > +OVN_POPULATE_ARP > + > +send_garp() { > + hv=$1 > + dev=$2 > + mac_byte=$3 > + ip_byte=${4-$3} > + > + mac="0000000010$mac_byte" > + ip=`ip_to_hex 192 168 10 $ip_byte` > + packet=ffffffffffff${mac}08060001080006040002${mac}${ip}${mac}${ip} > + as $hv ovs-appctl netdev-dummy/receive $dev $packet > +} > + > +# Check if the option is not present by default > +AT_CHECK([fetch_column nb:logical_router options name="gw" | grep -q mac_binding_age_threshold], [1]) > + > +# Set the MAC binding aging threshold > +AT_CHECK([ovn-nbctl set logical_router gw options:mac_binding_age_threshold=1]) > +AT_CHECK([fetch_column nb:logical_router options | grep -q mac_binding_age_threshold=1]) > +AT_CHECK([ovn-nbctl --wait=sb sync]) > + > +# Send GARP to populate MAC binding table records > +send_garp hv1 ext1 10 > +send_garp hv2 ext2 20 > + > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.10"]) > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.20"]) I worry that on a slower system, the MAC binding could be added and aged out before you reach the OVS_WAIT_UNTIL() commands above, causing the test to fail. What might be better is to create the MAC bindings, ensure they are present, then set the mac_binding_age_threshold to 1 and make sure they go away. > + > +# Check if the records are removed after 1 sec of inactivity > +OVS_WAIT_UNTIL([ > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.10')" > +]) > +OVS_WAIT_UNTIL([ > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" > +]) > + > +OVN_CLEANUP([hv1], [hv2]) > +AT_CLEANUP > +])
On Wed, Jul 20, 2022 at 4:50 AM Ales Musil <amusil@redhat.com> wrote: > > Reported-at: https://bugzilla.redhat.com/2084668 > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > v3: Rebase on top of current main. > Update according to the final conclusion. > --- > tests/ovn.at | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 109 insertions(+) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 7d10610fd..cbacc52c5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -32505,3 +32505,112 @@ AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0]) > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([MAC binding aging]) > +ovn_start > + > +net_add n1 > + > +AT_CHECK([ovn-nbctl ls-add public]) > +AT_CHECK([ovn-nbctl ls-add internal]) > + > +AT_CHECK([ovn-nbctl lsp-add public ln_port]) > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) > + > +AT_CHECK([ovn-nbctl lsp-add public public-gw]) > +AT_CHECK([ovn-nbctl lsp-set-type public-gw router]) > +AT_CHECK([ovn-nbctl lsp-set-addresses public-gw 00:00:00:00:10:00 router]) > +AT_CHECK([ovn-nbctl lsp-set-options public-gw router-port=gw-public]) > + > +AT_CHECK([ovn-nbctl lsp-add internal internal-gw]) > +AT_CHECK([ovn-nbctl lsp-set-type internal-gw router]) > +AT_CHECK([ovn-nbctl lsp-set-addresses internal-gw 00:00:00:00:20:00 router]) > +AT_CHECK([ovn-nbctl lsp-set-options internal-gw router-port=gw-internal]) > + > +AT_CHECK([ovn-nbctl lsp-add internal vif1]) > +AT_CHECK([ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:20:10 192.168.20.10"]) > + > +AT_CHECK([ovn-nbctl lsp-add internal vif2]) > +AT_CHECK([ovn-nbctl lsp-set-addresses vif2 "00:00:00:00:20:20 192.168.20.20"]) > + > +AT_CHECK([ovn-nbctl lr-add gw]) > +AT_CHECK([ovn-nbctl lrp-add gw gw-public 00:00:00:00:10:00 192.168.10.1/24]) > +AT_CHECK([ovn-nbctl lrp-add gw gw-internal 00:00:00:00:20:00 192.168.20.1/24]) > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-underlay > +ovn_attach n1 br-underlay 192.168.0.1 > +ovs-vsctl add-br br-phys > +ovs-vsctl -- add-port br-int vif1 -- \ > + set interface vif1 external-ids:iface-id=vif1 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap \ > + ofport-request=1 > +ovs-vsctl -- add-port br-phys ext1 -- \ > + set interface ext1 \ > + options:tx_pcap=hv1/ext1-tx.pcap \ > + options:rxq_pcap=hv1/ext1-rx.pcap \ > + ofport-request=2 > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > + > +sim_add hv2 > +as hv2 > +ovs-vsctl add-br br-underlay > +ovn_attach n1 br-underlay 192.168.0.2 > +ovs-vsctl add-br br-phys > +ovs-vsctl -- add-port br-int vif2 -- \ > + set interface vif2 external-ids:iface-id=vif2 \ > + options:tx_pcap=hv2/vif2-tx.pcap \ > + options:rxq_pcap=hv2/vif2-rx.pcap \ > + ofport-request=1 > +ovs-vsctl -- add-port br-phys ext2 -- \ > + set interface ext2 \ > + options:tx_pcap=hv2/ext2-tx.pcap \ > + options:rxq_pcap=hv2/ext2-rx.pcap \ > + ofport-request=2 > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > + > +OVN_POPULATE_ARP > + > +send_garp() { > + hv=$1 > + dev=$2 > + mac_byte=$3 > + ip_byte=${4-$3} > + > + mac="0000000010$mac_byte" > + ip=`ip_to_hex 192 168 10 $ip_byte` > + packet=ffffffffffff${mac}08060001080006040002${mac}${ip}${mac}${ip} > + as $hv ovs-appctl netdev-dummy/receive $dev $packet > +} > + > +# Check if the option is not present by default > +AT_CHECK([fetch_column nb:logical_router options name="gw" | grep -q mac_binding_age_threshold], [1]) > + > +# Set the MAC binding aging threshold > +AT_CHECK([ovn-nbctl set logical_router gw options:mac_binding_age_threshold=1]) > +AT_CHECK([fetch_column nb:logical_router options | grep -q mac_binding_age_threshold=1]) > +AT_CHECK([ovn-nbctl --wait=sb sync]) > + > +# Send GARP to populate MAC binding table records > +send_garp hv1 ext1 10 > +send_garp hv2 ext2 20 > + > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.10"]) > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.20"]) > + > +# Check if the records are removed after 1 sec of inactivity I don't see where the 1 sec promise is checked here. OVS_WAIT_UNTIL may take a lot longer than that, and still the test would pass. > +OVS_WAIT_UNTIL([ > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.10')" > +]) > +OVS_WAIT_UNTIL([ > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" > +]) > + > +OVN_CLEANUP([hv1], [hv2]) > +AT_CLEANUP > +]) > -- > 2.35.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi Ihar, On Fri, Jul 22, 2022 at 8:37 PM Ihar Hrachyshka <ihrachys@redhat.com> wrote: > On Wed, Jul 20, 2022 at 4:50 AM Ales Musil <amusil@redhat.com> wrote: > > > > Reported-at: https://bugzilla.redhat.com/2084668 > > Signed-off-by: Ales Musil <amusil@redhat.com> > > --- > > v3: Rebase on top of current main. > > Update according to the final conclusion. > > --- > > tests/ovn.at | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 109 insertions(+) > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 7d10610fd..cbacc52c5 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -32505,3 +32505,112 @@ AT_CHECK([test $(ovn-sbctl list fdb | grep -c > "00:00:00:00:10:30") = 0]) > > OVN_CLEANUP([hv1]) > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([MAC binding aging]) > > +ovn_start > > + > > +net_add n1 > > + > > +AT_CHECK([ovn-nbctl ls-add public]) > > +AT_CHECK([ovn-nbctl ls-add internal]) > > + > > +AT_CHECK([ovn-nbctl lsp-add public ln_port]) > > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) > > + > > +AT_CHECK([ovn-nbctl lsp-add public public-gw]) > > +AT_CHECK([ovn-nbctl lsp-set-type public-gw router]) > > +AT_CHECK([ovn-nbctl lsp-set-addresses public-gw 00:00:00:00:10:00 > router]) > > +AT_CHECK([ovn-nbctl lsp-set-options public-gw router-port=gw-public]) > > + > > +AT_CHECK([ovn-nbctl lsp-add internal internal-gw]) > > +AT_CHECK([ovn-nbctl lsp-set-type internal-gw router]) > > +AT_CHECK([ovn-nbctl lsp-set-addresses internal-gw 00:00:00:00:20:00 > router]) > > +AT_CHECK([ovn-nbctl lsp-set-options internal-gw > router-port=gw-internal]) > > + > > +AT_CHECK([ovn-nbctl lsp-add internal vif1]) > > +AT_CHECK([ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:20:10 > 192.168.20.10"]) > > + > > +AT_CHECK([ovn-nbctl lsp-add internal vif2]) > > +AT_CHECK([ovn-nbctl lsp-set-addresses vif2 "00:00:00:00:20:20 > 192.168.20.20"]) > > + > > +AT_CHECK([ovn-nbctl lr-add gw]) > > +AT_CHECK([ovn-nbctl lrp-add gw gw-public 00:00:00:00:10:00 > 192.168.10.1/24]) > > +AT_CHECK([ovn-nbctl lrp-add gw gw-internal 00:00:00:00:20:00 > 192.168.20.1/24]) > > + > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-underlay > > +ovn_attach n1 br-underlay 192.168.0.1 > > +ovs-vsctl add-br br-phys > > +ovs-vsctl -- add-port br-int vif1 -- \ > > + set interface vif1 external-ids:iface-id=vif1 \ > > + options:tx_pcap=hv1/vif1-tx.pcap \ > > + options:rxq_pcap=hv1/vif1-rx.pcap \ > > + ofport-request=1 > > +ovs-vsctl -- add-port br-phys ext1 -- \ > > + set interface ext1 \ > > + options:tx_pcap=hv1/ext1-tx.pcap \ > > + options:rxq_pcap=hv1/ext1-rx.pcap \ > > + ofport-request=2 > > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > > + > > +sim_add hv2 > > +as hv2 > > +ovs-vsctl add-br br-underlay > > +ovn_attach n1 br-underlay 192.168.0.2 > > +ovs-vsctl add-br br-phys > > +ovs-vsctl -- add-port br-int vif2 -- \ > > + set interface vif2 external-ids:iface-id=vif2 \ > > + options:tx_pcap=hv2/vif2-tx.pcap \ > > + options:rxq_pcap=hv2/vif2-rx.pcap \ > > + ofport-request=1 > > +ovs-vsctl -- add-port br-phys ext2 -- \ > > + set interface ext2 \ > > + options:tx_pcap=hv2/ext2-tx.pcap \ > > + options:rxq_pcap=hv2/ext2-rx.pcap \ > > + ofport-request=2 > > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > > + > > +OVN_POPULATE_ARP > > + > > +send_garp() { > > + hv=$1 > > + dev=$2 > > + mac_byte=$3 > > + ip_byte=${4-$3} > > + > > + mac="0000000010$mac_byte" > > + ip=`ip_to_hex 192 168 10 $ip_byte` > > + packet=ffffffffffff${mac}08060001080006040002${mac}${ip}${mac}${ip} > > + as $hv ovs-appctl netdev-dummy/receive $dev $packet > > +} > > + > > +# Check if the option is not present by default > > +AT_CHECK([fetch_column nb:logical_router options name="gw" | grep -q > mac_binding_age_threshold], [1]) > > + > > +# Set the MAC binding aging threshold > > +AT_CHECK([ovn-nbctl set logical_router gw > options:mac_binding_age_threshold=1]) > > +AT_CHECK([fetch_column nb:logical_router options | grep -q > mac_binding_age_threshold=1]) > > +AT_CHECK([ovn-nbctl --wait=sb sync]) > > + > > +# Send GARP to populate MAC binding table records > > +send_garp hv1 ext1 10 > > +send_garp hv2 ext2 20 > > + > > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.10"]) > > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.20"]) > > + > > +# Check if the records are removed after 1 sec of inactivity > > I don't see where the 1 sec promise is checked here. OVS_WAIT_UNTIL > may take a lot longer than that, and still the test would pass. > There is no guarantee that it will be exactly 1 sec, it can be later than that. From my point of view that's fine as long as it gets deleted eventually. Also having the test tight to the 1 sec would make it potentially flaky. Maybe the comment can be adjusted to better reflect that. > > > +OVS_WAIT_UNTIL([ > > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.10')" > > +]) > > +OVS_WAIT_UNTIL([ > > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" > > +]) > > + > > +OVN_CLEANUP([hv1], [hv2]) > > +AT_CLEANUP > > +]) > > -- > > 2.35.3 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > Regards, Ales
On Mon, Jul 25, 2022 at 1:25 AM Ales Musil <amusil@redhat.com> wrote: > > Hi Ihar, > > > On Fri, Jul 22, 2022 at 8:37 PM Ihar Hrachyshka <ihrachys@redhat.com> wrote: >> >> On Wed, Jul 20, 2022 at 4:50 AM Ales Musil <amusil@redhat.com> wrote: >> > >> > Reported-at: https://bugzilla.redhat.com/2084668 >> > Signed-off-by: Ales Musil <amusil@redhat.com> >> > --- >> > v3: Rebase on top of current main. >> > Update according to the final conclusion. >> > --- >> > tests/ovn.at | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 109 insertions(+) >> > >> > diff --git a/tests/ovn.at b/tests/ovn.at >> > index 7d10610fd..cbacc52c5 100644 >> > --- a/tests/ovn.at >> > +++ b/tests/ovn.at >> > @@ -32505,3 +32505,112 @@ AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0]) >> > OVN_CLEANUP([hv1]) >> > AT_CLEANUP >> > ]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([MAC binding aging]) >> > +ovn_start >> > + >> > +net_add n1 >> > + >> > +AT_CHECK([ovn-nbctl ls-add public]) >> > +AT_CHECK([ovn-nbctl ls-add internal]) >> > + >> > +AT_CHECK([ovn-nbctl lsp-add public ln_port]) >> > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) >> > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) >> > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) >> > + >> > +AT_CHECK([ovn-nbctl lsp-add public public-gw]) >> > +AT_CHECK([ovn-nbctl lsp-set-type public-gw router]) >> > +AT_CHECK([ovn-nbctl lsp-set-addresses public-gw 00:00:00:00:10:00 router]) >> > +AT_CHECK([ovn-nbctl lsp-set-options public-gw router-port=gw-public]) >> > + >> > +AT_CHECK([ovn-nbctl lsp-add internal internal-gw]) >> > +AT_CHECK([ovn-nbctl lsp-set-type internal-gw router]) >> > +AT_CHECK([ovn-nbctl lsp-set-addresses internal-gw 00:00:00:00:20:00 router]) >> > +AT_CHECK([ovn-nbctl lsp-set-options internal-gw router-port=gw-internal]) >> > + >> > +AT_CHECK([ovn-nbctl lsp-add internal vif1]) >> > +AT_CHECK([ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:20:10 192.168.20.10"]) >> > + >> > +AT_CHECK([ovn-nbctl lsp-add internal vif2]) >> > +AT_CHECK([ovn-nbctl lsp-set-addresses vif2 "00:00:00:00:20:20 192.168.20.20"]) >> > + >> > +AT_CHECK([ovn-nbctl lr-add gw]) >> > +AT_CHECK([ovn-nbctl lrp-add gw gw-public 00:00:00:00:10:00 192.168.10.1/24]) >> > +AT_CHECK([ovn-nbctl lrp-add gw gw-internal 00:00:00:00:20:00 192.168.20.1/24]) >> > + >> > +sim_add hv1 >> > +as hv1 >> > +ovs-vsctl add-br br-underlay >> > +ovn_attach n1 br-underlay 192.168.0.1 >> > +ovs-vsctl add-br br-phys >> > +ovs-vsctl -- add-port br-int vif1 -- \ >> > + set interface vif1 external-ids:iface-id=vif1 \ >> > + options:tx_pcap=hv1/vif1-tx.pcap \ >> > + options:rxq_pcap=hv1/vif1-rx.pcap \ >> > + ofport-request=1 >> > +ovs-vsctl -- add-port br-phys ext1 -- \ >> > + set interface ext1 \ >> > + options:tx_pcap=hv1/ext1-tx.pcap \ >> > + options:rxq_pcap=hv1/ext1-rx.pcap \ >> > + ofport-request=2 >> > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys >> > + >> > +sim_add hv2 >> > +as hv2 >> > +ovs-vsctl add-br br-underlay >> > +ovn_attach n1 br-underlay 192.168.0.2 >> > +ovs-vsctl add-br br-phys >> > +ovs-vsctl -- add-port br-int vif2 -- \ >> > + set interface vif2 external-ids:iface-id=vif2 \ >> > + options:tx_pcap=hv2/vif2-tx.pcap \ >> > + options:rxq_pcap=hv2/vif2-rx.pcap \ >> > + ofport-request=1 >> > +ovs-vsctl -- add-port br-phys ext2 -- \ >> > + set interface ext2 \ >> > + options:tx_pcap=hv2/ext2-tx.pcap \ >> > + options:rxq_pcap=hv2/ext2-rx.pcap \ >> > + ofport-request=2 >> > +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys >> > + >> > +OVN_POPULATE_ARP >> > + >> > +send_garp() { >> > + hv=$1 >> > + dev=$2 >> > + mac_byte=$3 >> > + ip_byte=${4-$3} >> > + >> > + mac="0000000010$mac_byte" >> > + ip=`ip_to_hex 192 168 10 $ip_byte` >> > + packet=ffffffffffff${mac}08060001080006040002${mac}${ip}${mac}${ip} >> > + as $hv ovs-appctl netdev-dummy/receive $dev $packet >> > +} >> > + >> > +# Check if the option is not present by default >> > +AT_CHECK([fetch_column nb:logical_router options name="gw" | grep -q mac_binding_age_threshold], [1]) >> > + >> > +# Set the MAC binding aging threshold >> > +AT_CHECK([ovn-nbctl set logical_router gw options:mac_binding_age_threshold=1]) >> > +AT_CHECK([fetch_column nb:logical_router options | grep -q mac_binding_age_threshold=1]) >> > +AT_CHECK([ovn-nbctl --wait=sb sync]) >> > + >> > +# Send GARP to populate MAC binding table records >> > +send_garp hv1 ext1 10 >> > +send_garp hv2 ext2 20 >> > + >> > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.10"]) >> > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.20"]) >> > + >> > +# Check if the records are removed after 1 sec of inactivity >> >> I don't see where the 1 sec promise is checked here. OVS_WAIT_UNTIL >> may take a lot longer than that, and still the test would pass. > > > There is no guarantee that it will be exactly 1 sec, it can be later than that. > From my point of view that's fine as long as it gets deleted eventually. Also > having the test tight to the 1 sec would make it potentially flaky. Maybe the > comment can be adjusted to better reflect that. > I understand that but OVS_WAIT_UNTIL waits up to 30 seconds. Is there a middle ground between 1 sec and 30 sec we could find? AFAIU setting OVS_CTL_TIMEOUT allows to speed up the bailout. >> >> >> > +OVS_WAIT_UNTIL([ >> > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.10')" >> > +]) >> > +OVS_WAIT_UNTIL([ >> > + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" >> > +]) >> > + >> > +OVN_CLEANUP([hv1], [hv2]) >> > +AT_CLEANUP >> > +]) >> > -- >> > 2.35.3 >> > >> > _______________________________________________ >> > dev mailing list >> > dev@openvswitch.org >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >> > > Regards, > Ales > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA > > amusil@redhat.com IM: amusil
On Mon, Jul 25, 2022 at 6:01 PM Ihar Hrachyshka <ihrachys@redhat.com> wrote: > On Mon, Jul 25, 2022 at 1:25 AM Ales Musil <amusil@redhat.com> wrote: > > > > Hi Ihar, > > > > > > On Fri, Jul 22, 2022 at 8:37 PM Ihar Hrachyshka <ihrachys@redhat.com> > wrote: > >> > >> On Wed, Jul 20, 2022 at 4:50 AM Ales Musil <amusil@redhat.com> wrote: > >> > > >> > Reported-at: https://bugzilla.redhat.com/2084668 > >> > Signed-off-by: Ales Musil <amusil@redhat.com> > >> > --- > >> > v3: Rebase on top of current main. > >> > Update according to the final conclusion. > >> > --- > >> > tests/ovn.at | 109 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 109 insertions(+) > >> > > >> > diff --git a/tests/ovn.at b/tests/ovn.at > >> > index 7d10610fd..cbacc52c5 100644 > >> > --- a/tests/ovn.at > >> > +++ b/tests/ovn.at > >> > @@ -32505,3 +32505,112 @@ AT_CHECK([test $(ovn-sbctl list fdb | grep > -c "00:00:00:00:10:30") = 0]) > >> > OVN_CLEANUP([hv1]) > >> > AT_CLEANUP > >> > ]) > >> > + > >> > +OVN_FOR_EACH_NORTHD([ > >> > +AT_SETUP([MAC binding aging]) > >> > +ovn_start > >> > + > >> > +net_add n1 > >> > + > >> > +AT_CHECK([ovn-nbctl ls-add public]) > >> > +AT_CHECK([ovn-nbctl ls-add internal]) > >> > + > >> > +AT_CHECK([ovn-nbctl lsp-add public ln_port]) > >> > +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > >> > +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > >> > +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) > >> > + > >> > +AT_CHECK([ovn-nbctl lsp-add public public-gw]) > >> > +AT_CHECK([ovn-nbctl lsp-set-type public-gw router]) > >> > +AT_CHECK([ovn-nbctl lsp-set-addresses public-gw 00:00:00:00:10:00 > router]) > >> > +AT_CHECK([ovn-nbctl lsp-set-options public-gw router-port=gw-public]) > >> > + > >> > +AT_CHECK([ovn-nbctl lsp-add internal internal-gw]) > >> > +AT_CHECK([ovn-nbctl lsp-set-type internal-gw router]) > >> > +AT_CHECK([ovn-nbctl lsp-set-addresses internal-gw 00:00:00:00:20:00 > router]) > >> > +AT_CHECK([ovn-nbctl lsp-set-options internal-gw > router-port=gw-internal]) > >> > + > >> > +AT_CHECK([ovn-nbctl lsp-add internal vif1]) > >> > +AT_CHECK([ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:20:10 > 192.168.20.10"]) > >> > + > >> > +AT_CHECK([ovn-nbctl lsp-add internal vif2]) > >> > +AT_CHECK([ovn-nbctl lsp-set-addresses vif2 "00:00:00:00:20:20 > 192.168.20.20"]) > >> > + > >> > +AT_CHECK([ovn-nbctl lr-add gw]) > >> > +AT_CHECK([ovn-nbctl lrp-add gw gw-public 00:00:00:00:10:00 > 192.168.10.1/24]) > >> > +AT_CHECK([ovn-nbctl lrp-add gw gw-internal 00:00:00:00:20:00 > 192.168.20.1/24]) > >> > + > >> > +sim_add hv1 > >> > +as hv1 > >> > +ovs-vsctl add-br br-underlay > >> > +ovn_attach n1 br-underlay 192.168.0.1 > >> > +ovs-vsctl add-br br-phys > >> > +ovs-vsctl -- add-port br-int vif1 -- \ > >> > + set interface vif1 external-ids:iface-id=vif1 \ > >> > + options:tx_pcap=hv1/vif1-tx.pcap \ > >> > + options:rxq_pcap=hv1/vif1-rx.pcap \ > >> > + ofport-request=1 > >> > +ovs-vsctl -- add-port br-phys ext1 -- \ > >> > + set interface ext1 \ > >> > + options:tx_pcap=hv1/ext1-tx.pcap \ > >> > + options:rxq_pcap=hv1/ext1-rx.pcap \ > >> > + ofport-request=2 > >> > +ovs-vsctl set open . > external_ids:ovn-bridge-mappings=physnet1:br-phys > >> > + > >> > +sim_add hv2 > >> > +as hv2 > >> > +ovs-vsctl add-br br-underlay > >> > +ovn_attach n1 br-underlay 192.168.0.2 > >> > +ovs-vsctl add-br br-phys > >> > +ovs-vsctl -- add-port br-int vif2 -- \ > >> > + set interface vif2 external-ids:iface-id=vif2 \ > >> > + options:tx_pcap=hv2/vif2-tx.pcap \ > >> > + options:rxq_pcap=hv2/vif2-rx.pcap \ > >> > + ofport-request=1 > >> > +ovs-vsctl -- add-port br-phys ext2 -- \ > >> > + set interface ext2 \ > >> > + options:tx_pcap=hv2/ext2-tx.pcap \ > >> > + options:rxq_pcap=hv2/ext2-rx.pcap \ > >> > + ofport-request=2 > >> > +ovs-vsctl set open . > external_ids:ovn-bridge-mappings=physnet1:br-phys > >> > + > >> > +OVN_POPULATE_ARP > >> > + > >> > +send_garp() { > >> > + hv=$1 > >> > + dev=$2 > >> > + mac_byte=$3 > >> > + ip_byte=${4-$3} > >> > + > >> > + mac="0000000010$mac_byte" > >> > + ip=`ip_to_hex 192 168 10 $ip_byte` > >> > + > packet=ffffffffffff${mac}08060001080006040002${mac}${ip}${mac}${ip} > >> > + as $hv ovs-appctl netdev-dummy/receive $dev $packet > >> > +} > >> > + > >> > +# Check if the option is not present by default > >> > +AT_CHECK([fetch_column nb:logical_router options name="gw" | grep -q > mac_binding_age_threshold], [1]) > >> > + > >> > +# Set the MAC binding aging threshold > >> > +AT_CHECK([ovn-nbctl set logical_router gw > options:mac_binding_age_threshold=1]) > >> > +AT_CHECK([fetch_column nb:logical_router options | grep -q > mac_binding_age_threshold=1]) > >> > +AT_CHECK([ovn-nbctl --wait=sb sync]) > >> > + > >> > +# Send GARP to populate MAC binding table records > >> > +send_garp hv1 ext1 10 > >> > +send_garp hv2 ext2 20 > >> > + > >> > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q > "192.168.10.10"]) > >> > +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q > "192.168.10.20"]) > >> > + > >> > +# Check if the records are removed after 1 sec of inactivity > >> > >> I don't see where the 1 sec promise is checked here. OVS_WAIT_UNTIL > >> may take a lot longer than that, and still the test would pass. > > > > > > There is no guarantee that it will be exactly 1 sec, it can be later > than that. > > From my point of view that's fine as long as it gets deleted eventually. > Also > > having the test tight to the 1 sec would make it potentially flaky. > Maybe the > > comment can be adjusted to better reflect that. > > > > I understand that but OVS_WAIT_UNTIL waits up to 30 seconds. Is there > a middle ground between 1 sec and 30 sec we could find? AFAIU setting > OVS_CTL_TIMEOUT allows to speed up the bailout. I have reduced it to 5 seconds max. It will be included in v4. > > > >> > >> > >> > +OVS_WAIT_UNTIL([ > >> > + test "0" = "$(ovn-sbctl list mac_binding | grep -c > '192.168.10.10')" > >> > +]) > >> > +OVS_WAIT_UNTIL([ > >> > + test "0" = "$(ovn-sbctl list mac_binding | grep -c > '192.168.10.20')" > >> > +]) > >> > + > >> > +OVN_CLEANUP([hv1], [hv2]) > >> > +AT_CLEANUP > >> > +]) > >> > -- > >> > 2.35.3 > >> > > >> > _______________________________________________ > >> > dev mailing list > >> > dev@openvswitch.org > >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > >> > > > > Regards, > > Ales > > > > -- > > > > Ales Musil > > > > Senior Software Engineer - OVN Core > > > > Red Hat EMEA > > > > amusil@redhat.com IM: amusil > > Thanks, Ales
diff --git a/tests/ovn.at b/tests/ovn.at index 7d10610fd..cbacc52c5 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -32505,3 +32505,112 @@ AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0]) OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([MAC binding aging]) +ovn_start + +net_add n1 + +AT_CHECK([ovn-nbctl ls-add public]) +AT_CHECK([ovn-nbctl ls-add internal]) + +AT_CHECK([ovn-nbctl lsp-add public ln_port]) +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) + +AT_CHECK([ovn-nbctl lsp-add public public-gw]) +AT_CHECK([ovn-nbctl lsp-set-type public-gw router]) +AT_CHECK([ovn-nbctl lsp-set-addresses public-gw 00:00:00:00:10:00 router]) +AT_CHECK([ovn-nbctl lsp-set-options public-gw router-port=gw-public]) + +AT_CHECK([ovn-nbctl lsp-add internal internal-gw]) +AT_CHECK([ovn-nbctl lsp-set-type internal-gw router]) +AT_CHECK([ovn-nbctl lsp-set-addresses internal-gw 00:00:00:00:20:00 router]) +AT_CHECK([ovn-nbctl lsp-set-options internal-gw router-port=gw-internal]) + +AT_CHECK([ovn-nbctl lsp-add internal vif1]) +AT_CHECK([ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:20:10 192.168.20.10"]) + +AT_CHECK([ovn-nbctl lsp-add internal vif2]) +AT_CHECK([ovn-nbctl lsp-set-addresses vif2 "00:00:00:00:20:20 192.168.20.20"]) + +AT_CHECK([ovn-nbctl lr-add gw]) +AT_CHECK([ovn-nbctl lrp-add gw gw-public 00:00:00:00:10:00 192.168.10.1/24]) +AT_CHECK([ovn-nbctl lrp-add gw gw-internal 00:00:00:00:20:00 192.168.20.1/24]) + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-underlay +ovn_attach n1 br-underlay 192.168.0.1 +ovs-vsctl add-br br-phys +ovs-vsctl -- add-port br-int vif1 -- \ + set interface vif1 external-ids:iface-id=vif1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 +ovs-vsctl -- add-port br-phys ext1 -- \ + set interface ext1 \ + options:tx_pcap=hv1/ext1-tx.pcap \ + options:rxq_pcap=hv1/ext1-rx.pcap \ + ofport-request=2 +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys + +sim_add hv2 +as hv2 +ovs-vsctl add-br br-underlay +ovn_attach n1 br-underlay 192.168.0.2 +ovs-vsctl add-br br-phys +ovs-vsctl -- add-port br-int vif2 -- \ + set interface vif2 external-ids:iface-id=vif2 \ + options:tx_pcap=hv2/vif2-tx.pcap \ + options:rxq_pcap=hv2/vif2-rx.pcap \ + ofport-request=1 +ovs-vsctl -- add-port br-phys ext2 -- \ + set interface ext2 \ + options:tx_pcap=hv2/ext2-tx.pcap \ + options:rxq_pcap=hv2/ext2-rx.pcap \ + ofport-request=2 +ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys + +OVN_POPULATE_ARP + +send_garp() { + hv=$1 + dev=$2 + mac_byte=$3 + ip_byte=${4-$3} + + mac="0000000010$mac_byte" + ip=`ip_to_hex 192 168 10 $ip_byte` + packet=ffffffffffff${mac}08060001080006040002${mac}${ip}${mac}${ip} + as $hv ovs-appctl netdev-dummy/receive $dev $packet +} + +# Check if the option is not present by default +AT_CHECK([fetch_column nb:logical_router options name="gw" | grep -q mac_binding_age_threshold], [1]) + +# Set the MAC binding aging threshold +AT_CHECK([ovn-nbctl set logical_router gw options:mac_binding_age_threshold=1]) +AT_CHECK([fetch_column nb:logical_router options | grep -q mac_binding_age_threshold=1]) +AT_CHECK([ovn-nbctl --wait=sb sync]) + +# Send GARP to populate MAC binding table records +send_garp hv1 ext1 10 +send_garp hv2 ext2 20 + +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.10"]) +OVS_WAIT_UNTIL([ovn-sbctl list mac_binding | grep -q "192.168.10.20"]) + +# Check if the records are removed after 1 sec of inactivity +OVS_WAIT_UNTIL([ + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.10')" +]) +OVS_WAIT_UNTIL([ + test "0" = "$(ovn-sbctl list mac_binding | grep -c '192.168.10.20')" +]) + +OVN_CLEANUP([hv1], [hv2]) +AT_CLEANUP +])
Reported-at: https://bugzilla.redhat.com/2084668 Signed-off-by: Ales Musil <amusil@redhat.com> --- v3: Rebase on top of current main. Update according to the final conclusion. --- tests/ovn.at | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+)