| Message ID | 6554c8f94a5572b52bd7e3267b8602f4f1a8cf85.1600704955.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | fix localnet QoS configuration after I-P | expand |
On Mon, Sep 21, 2020 at 9:56 PM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > After the I-P has been introduced in commit 354bdba51a ("ovn-controller: > I-P for SB port binding and OVS interface in runtime_data"), the QoS on > localnet ports is not properly configured if the ovs interface is marked > with "ovn-egress-iface" flag after the related record in the > logical_switch_port table is set. The issue can be triggered with the > following reproducer: > > $ovn-nbctl set Logical_Switch_Port ln-public options:qos_burst=1000 > $ovs-vsctl set interface eth0 external-ids:ovn-egress-iface=true > > Fix the issue triggering a recomputation after qos is configured for ovs > interface > > Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS > interface in runtime_data") > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Hi Lorenzo, This approach of having the same shash for representing both the local interfaces (interfaces connected to the integration bridge - br-int) and the egress interfaces (interfaces connected to provider bridges) doesn't seem right to me. They are not related at all. We already maintain an sset - egress_ifaces [1] and we add an interface to this set if 'ovn-egress-iface' is set to true in external_ids here [2]. I think a simple approach to fix this issue would be to do the following in the function binding_handle_ovs_interface_changes() - If an ovs interface is deleted, check if this interface name is present in the sset 'egress_ifaces', if so, return false so to trigger a full recompute. - If an ovs interface is added/updated check if 'ovn-egress-iface' key is present in the external_ids. 1. If it is present, then return false to trigger a full recompute. 2. If it is not present, then check if the interface is in the 'egress_ifaces' sset. If so, return false to trigger a full recompute. Thanks Numan > controller/binding.c | 26 +++++++++++++++++ > controller/binding.h | 1 + > tests/ovn-performance.at | 13 +++++++++ > tests/system-ovn.at | 60 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 100 insertions(+) > > diff --git a/controller/binding.c b/controller/binding.c > index 107bc28a6..26542ae66 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1353,6 +1353,13 @@ build_local_bindings(struct binding_ctx_in > *b_ctx_in, > struct local_iface_node *iface_node = > xmalloc(sizeof *iface_node); > iface_node->iface_id = xstrdup(iface_id); > + struct local_iface_node *old_iface_node = shash_find_data( > + b_ctx_out->local_ifaces, > + iface_rec->name); > + if (old_iface_node) { > + iface_node->qos_egress_iface = > + old_iface_node->qos_egress_iface; > + } > iface_node = shash_replace(b_ctx_out->local_ifaces, > iface_rec->name, iface_node); > if (iface_node) { > @@ -1700,6 +1707,12 @@ consider_iface_claim(const struct ovsrec_interface > *iface_rec, > > struct local_iface_node *iface_node = xmalloc(sizeof *iface_node); > iface_node->iface_id = xstrdup(iface_id); > + struct local_iface_node *old_iface_node = shash_find_data( > + b_ctx_out->local_ifaces, > + iface_rec->name); > + if (old_iface_node) { > + iface_node->qos_egress_iface = old_iface_node->qos_egress_iface; > + } > iface_node = shash_replace(b_ctx_out->local_ifaces, > iface_rec->name, iface_node); > if (iface_node) { > @@ -1928,6 +1941,19 @@ binding_handle_ovs_interface_changes(struct > binding_ctx_in *b_ctx_in, > continue; > } > > + struct local_iface_node *node = shash_find_data( > + b_ctx_out->local_ifaces, iface_rec->name); > + bool old_qos_egress = node ? node->qos_egress_iface : false; > + bool qos_egress = smap_get_bool(&iface_rec->external_ids, > + "ovn-egress-iface", false); > + if (qos_egress != old_qos_egress) { > + if (node) { > + node->qos_egress_iface = qos_egress; > + } > + handled = false; > + break; > + } > + > const char *iface_id = smap_get(&iface_rec->external_ids, > "iface-id"); > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > if (iface_id && ofport > 0) { > diff --git a/controller/binding.h b/controller/binding.h > index a1e0f1ccf..9e3e1f512 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -56,6 +56,7 @@ struct binding_ctx_in { > > struct local_iface_node { > char *iface_id; > + bool qos_egress_iface; > }; > > struct binding_ctx_out { > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > index 3010860d5..6cc5b2174 100644 > --- a/tests/ovn-performance.at > +++ b/tests/ovn-performance.at > @@ -247,6 +247,8 @@ for i in 1 2 3; do > ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex" > j=$((i + 2)) > ovn_attach n1 br-phys 192.168.0.$j > + ip link add vgw$i type dummy > + ovs-vsctl add-port br-ex vgw$i > done > > # Wait for the tunnel ports to be created and up. > @@ -477,6 +479,17 @@ OVN_CONTROLLER_EXPECT_HIT( > [ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw3 10 && > ovn-nbctl --wait=hv sync] > ) > > +# create QoS rule > +OVN_CONTROLLER_EXPECT_NO_HIT( > + [hv1 hv2 gw1 gw2 gw3], [lflow_run], > + [ovn-nbctl --wait=hv set Logical_Switch_Port ln-public > options:qos_burst=1000] > +) > + > +OVN_CONTROLLER_EXPECT_HIT( > + [gw1], [lflow_run], > + [as gw1 ovs-vsctl set interface vgw1 > external-ids:ovn-egress-iface=true] > +) > + > # Make gw2 master. There is remote possibility that full recompute > # triggers for gw2 after it becomes master. Most of the time > # there will be no recompute. > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 4f72754c6..f779947f4 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -5397,3 +5397,63 @@ as > OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d > /.*terminating with signal 15.*/d"]) > AT_CLEANUP > + > +AT_SETUP([ovn -- egress qos]) > +AT_KEYWORDS([ovn-egress-qos]) > + > +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 \ > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > + -- set Open_vSwitch . > external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > + -- set bridge br-int fail-mode=secure > other-config:disable-in-band=true > + > +# Start ovn-controller > +start_daemon ovn-controller > + > +ovn-nbctl ls-add sw0 > + > +ADD_NAMESPACES(sw01) > +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03") > +ovn-nbctl lsp-add sw0 sw01 \ > + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" > + > +ADD_NAMESPACES(public) > +ADD_VETH(public, public, br-ext, "192.168.2.2/24", "f0:00:00:01:02:05") > + > +AT_CHECK([ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=phynet:br-ext]) > +ovn-nbctl lsp-add sw0 public \ > + -- lsp-set-addresses public unknown \ > + -- lsp-set-type public localnet \ > + -- lsp-set-options public network_name=phynet > + > +AT_CHECK([ovn-nbctl set Logical_Switch_Port public > options:qos_burst=1000]) > +AT_CHECK([ovs-vsctl set interface ovs-public > external-ids:ovn-egress-iface=true]) > +AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public']) > + > +AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options > qos_burst=1000]) > +AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public'],[1]) > + > +kill $(pidof ovn-controller) > + > +as ovn-sb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as ovn-nb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as northd > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) > + > +as > +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d > +/.*terminating with signal 15.*/d"]) > +AT_CLEANUP > -- > 2.26.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Wed, Sep 23, 2020 at 12:26 PM Numan Siddique <numans@ovn.org> wrote: > > > On Mon, Sep 21, 2020 at 9:56 PM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> wrote: > >> After the I-P has been introduced in commit 354bdba51a ("ovn-controller: >> I-P for SB port binding and OVS interface in runtime_data"), the QoS on >> localnet ports is not properly configured if the ovs interface is marked >> with "ovn-egress-iface" flag after the related record in the >> logical_switch_port table is set. The issue can be triggered with the >> following reproducer: >> >> $ovn-nbctl set Logical_Switch_Port ln-public options:qos_burst=1000 >> $ovs-vsctl set interface eth0 external-ids:ovn-egress-iface=true >> >> Fix the issue triggering a recomputation after qos is configured for ovs >> interface >> >> Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS >> interface in runtime_data") >> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> --- >> > > Hi Lorenzo, > > This approach of having the same shash for representing both the local > interfaces (interfaces connected > to the integration bridge - br-int) and the egress interfaces (interfaces > connected to provider bridges) > doesn't seem right to me. They are not related at all. > > We already maintain an sset - egress_ifaces [1] and we add an interface to > this set if 'ovn-egress-iface' is set to true > in external_ids here [2]. I think a simple approach to fix this issue > would be to do the following in the function > binding_handle_ovs_interface_changes() > - If an ovs interface is deleted, check if this interface name is > present in the sset 'egress_ifaces', if so, return false so > to trigger a full recompute. > > - If an ovs interface is added/updated check if 'ovn-egress-iface' key > is present in the external_ids. > 1. If it is present, then return false to trigger a full recompute. > 2. If it is not present, then check if the interface is in the > 'egress_ifaces' sset. If so, return false to trigger a full recompute. > > > [1] - https://github.com/ovn-org/ovn/blob/master/controller/ovn-controller.c#L1035 https://github.com/ovn-org/ovn/blob/master/controller/ovn-controller.c#L1235 [2] - https://github.com/ovn-org/ovn/blob/master/controller/binding.c#L457 > Thanks > Numan > > > > >> controller/binding.c | 26 +++++++++++++++++ >> controller/binding.h | 1 + >> tests/ovn-performance.at | 13 +++++++++ >> tests/system-ovn.at | 60 ++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 100 insertions(+) >> >> diff --git a/controller/binding.c b/controller/binding.c >> index 107bc28a6..26542ae66 100644 >> --- a/controller/binding.c >> +++ b/controller/binding.c >> @@ -1353,6 +1353,13 @@ build_local_bindings(struct binding_ctx_in >> *b_ctx_in, >> struct local_iface_node *iface_node = >> xmalloc(sizeof *iface_node); >> iface_node->iface_id = xstrdup(iface_id); >> + struct local_iface_node *old_iface_node = >> shash_find_data( >> + b_ctx_out->local_ifaces, >> + iface_rec->name); >> + if (old_iface_node) { >> + iface_node->qos_egress_iface = >> + old_iface_node->qos_egress_iface; >> + } >> iface_node = shash_replace(b_ctx_out->local_ifaces, >> iface_rec->name, iface_node); >> if (iface_node) { >> @@ -1700,6 +1707,12 @@ consider_iface_claim(const struct ovsrec_interface >> *iface_rec, >> >> struct local_iface_node *iface_node = xmalloc(sizeof *iface_node); >> iface_node->iface_id = xstrdup(iface_id); >> + struct local_iface_node *old_iface_node = shash_find_data( >> + b_ctx_out->local_ifaces, >> + iface_rec->name); >> + if (old_iface_node) { >> + iface_node->qos_egress_iface = old_iface_node->qos_egress_iface; >> + } >> iface_node = shash_replace(b_ctx_out->local_ifaces, >> iface_rec->name, iface_node); >> if (iface_node) { >> @@ -1928,6 +1941,19 @@ binding_handle_ovs_interface_changes(struct >> binding_ctx_in *b_ctx_in, >> continue; >> } >> >> + struct local_iface_node *node = shash_find_data( >> + b_ctx_out->local_ifaces, iface_rec->name); >> + bool old_qos_egress = node ? node->qos_egress_iface : false; >> + bool qos_egress = smap_get_bool(&iface_rec->external_ids, >> + "ovn-egress-iface", false); >> + if (qos_egress != old_qos_egress) { >> + if (node) { >> + node->qos_egress_iface = qos_egress; >> + } >> + handled = false; >> + break; >> + } >> + >> const char *iface_id = smap_get(&iface_rec->external_ids, >> "iface-id"); >> int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; >> if (iface_id && ofport > 0) { >> diff --git a/controller/binding.h b/controller/binding.h >> index a1e0f1ccf..9e3e1f512 100644 >> --- a/controller/binding.h >> +++ b/controller/binding.h >> @@ -56,6 +56,7 @@ struct binding_ctx_in { >> >> struct local_iface_node { >> char *iface_id; >> + bool qos_egress_iface; >> }; >> >> struct binding_ctx_out { >> diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at >> index 3010860d5..6cc5b2174 100644 >> --- a/tests/ovn-performance.at >> +++ b/tests/ovn-performance.at >> @@ -247,6 +247,8 @@ for i in 1 2 3; do >> ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex" >> j=$((i + 2)) >> ovn_attach n1 br-phys 192.168.0.$j >> + ip link add vgw$i type dummy >> + ovs-vsctl add-port br-ex vgw$i >> done >> >> # Wait for the tunnel ports to be created and up. >> @@ -477,6 +479,17 @@ OVN_CONTROLLER_EXPECT_HIT( >> [ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw3 10 && >> ovn-nbctl --wait=hv sync] >> ) >> >> +# create QoS rule >> +OVN_CONTROLLER_EXPECT_NO_HIT( >> + [hv1 hv2 gw1 gw2 gw3], [lflow_run], >> + [ovn-nbctl --wait=hv set Logical_Switch_Port ln-public >> options:qos_burst=1000] >> +) >> + >> +OVN_CONTROLLER_EXPECT_HIT( >> + [gw1], [lflow_run], >> + [as gw1 ovs-vsctl set interface vgw1 >> external-ids:ovn-egress-iface=true] >> +) >> + >> # Make gw2 master. There is remote possibility that full recompute >> # triggers for gw2 after it becomes master. Most of the time >> # there will be no recompute. >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 4f72754c6..f779947f4 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -5397,3 +5397,63 @@ as >> OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d >> /.*terminating with signal 15.*/d"]) >> AT_CLEANUP >> + >> +AT_SETUP([ovn -- egress qos]) >> +AT_KEYWORDS([ovn-egress-qos]) >> + >> +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 \ >> + -- set Open_vSwitch . external-ids:system-id=hv1 \ >> + -- set Open_vSwitch . >> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ >> + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ >> + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ >> + -- set bridge br-int fail-mode=secure >> other-config:disable-in-band=true >> + >> +# Start ovn-controller >> +start_daemon ovn-controller >> + >> +ovn-nbctl ls-add sw0 >> + >> +ADD_NAMESPACES(sw01) >> +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03") >> +ovn-nbctl lsp-add sw0 sw01 \ >> + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" >> + >> +ADD_NAMESPACES(public) >> +ADD_VETH(public, public, br-ext, "192.168.2.2/24", "f0:00:00:01:02:05") >> + >> +AT_CHECK([ovs-vsctl set Open_vSwitch . >> external-ids:ovn-bridge-mappings=phynet:br-ext]) >> +ovn-nbctl lsp-add sw0 public \ >> + -- lsp-set-addresses public unknown \ >> + -- lsp-set-type public localnet \ >> + -- lsp-set-options public network_name=phynet >> + >> +AT_CHECK([ovn-nbctl set Logical_Switch_Port public >> options:qos_burst=1000]) >> +AT_CHECK([ovs-vsctl set interface ovs-public >> external-ids:ovn-egress-iface=true]) >> +AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public']) >> + >> +AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options >> qos_burst=1000]) >> +AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public'],[1]) >> + >> +kill $(pidof ovn-controller) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([ovn-northd]) >> + >> +as >> +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d >> +/.*terminating with signal 15.*/d"]) >> +AT_CLEANUP >> -- >> 2.26.2 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >>
diff --git a/controller/binding.c b/controller/binding.c index 107bc28a6..26542ae66 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1353,6 +1353,13 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, struct local_iface_node *iface_node = xmalloc(sizeof *iface_node); iface_node->iface_id = xstrdup(iface_id); + struct local_iface_node *old_iface_node = shash_find_data( + b_ctx_out->local_ifaces, + iface_rec->name); + if (old_iface_node) { + iface_node->qos_egress_iface = + old_iface_node->qos_egress_iface; + } iface_node = shash_replace(b_ctx_out->local_ifaces, iface_rec->name, iface_node); if (iface_node) { @@ -1700,6 +1707,12 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec, struct local_iface_node *iface_node = xmalloc(sizeof *iface_node); iface_node->iface_id = xstrdup(iface_id); + struct local_iface_node *old_iface_node = shash_find_data( + b_ctx_out->local_ifaces, + iface_rec->name); + if (old_iface_node) { + iface_node->qos_egress_iface = old_iface_node->qos_egress_iface; + } iface_node = shash_replace(b_ctx_out->local_ifaces, iface_rec->name, iface_node); if (iface_node) { @@ -1928,6 +1941,19 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, continue; } + struct local_iface_node *node = shash_find_data( + b_ctx_out->local_ifaces, iface_rec->name); + bool old_qos_egress = node ? node->qos_egress_iface : false; + bool qos_egress = smap_get_bool(&iface_rec->external_ids, + "ovn-egress-iface", false); + if (qos_egress != old_qos_egress) { + if (node) { + node->qos_egress_iface = qos_egress; + } + handled = false; + break; + } + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; if (iface_id && ofport > 0) { diff --git a/controller/binding.h b/controller/binding.h index a1e0f1ccf..9e3e1f512 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -56,6 +56,7 @@ struct binding_ctx_in { struct local_iface_node { char *iface_id; + bool qos_egress_iface; }; struct binding_ctx_out { diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 3010860d5..6cc5b2174 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -247,6 +247,8 @@ for i in 1 2 3; do ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex" j=$((i + 2)) ovn_attach n1 br-phys 192.168.0.$j + ip link add vgw$i type dummy + ovs-vsctl add-port br-ex vgw$i done # Wait for the tunnel ports to be created and up. @@ -477,6 +479,17 @@ OVN_CONTROLLER_EXPECT_HIT( [ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw3 10 && ovn-nbctl --wait=hv sync] ) +# create QoS rule +OVN_CONTROLLER_EXPECT_NO_HIT( + [hv1 hv2 gw1 gw2 gw3], [lflow_run], + [ovn-nbctl --wait=hv set Logical_Switch_Port ln-public options:qos_burst=1000] +) + +OVN_CONTROLLER_EXPECT_HIT( + [gw1], [lflow_run], + [as gw1 ovs-vsctl set interface vgw1 external-ids:ovn-egress-iface=true] +) + # Make gw2 master. There is remote possibility that full recompute # triggers for gw2 after it becomes master. Most of the time # there will be no recompute. diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 4f72754c6..f779947f4 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -5397,3 +5397,63 @@ as OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d /.*terminating with signal 15.*/d"]) AT_CLEANUP + +AT_SETUP([ovn -- egress qos]) +AT_KEYWORDS([ovn-egress-qos]) + +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 \ + -- set Open_vSwitch . external-ids:system-id=hv1 \ + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true + +# Start ovn-controller +start_daemon ovn-controller + +ovn-nbctl ls-add sw0 + +ADD_NAMESPACES(sw01) +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03") +ovn-nbctl lsp-add sw0 sw01 \ + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" + +ADD_NAMESPACES(public) +ADD_VETH(public, public, br-ext, "192.168.2.2/24", "f0:00:00:01:02:05") + +AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext]) +ovn-nbctl lsp-add sw0 public \ + -- lsp-set-addresses public unknown \ + -- lsp-set-type public localnet \ + -- lsp-set-options public network_name=phynet + +AT_CHECK([ovn-nbctl set Logical_Switch_Port public options:qos_burst=1000]) +AT_CHECK([ovs-vsctl set interface ovs-public external-ids:ovn-egress-iface=true]) +AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public']) + +AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_burst=1000]) +AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public'],[1]) + +kill $(pidof ovn-controller) + +as ovn-sb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +as ovn-nb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +as northd +OVS_APP_EXIT_AND_WAIT([ovn-northd]) + +as +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d +/.*terminating with signal 15.*/d"]) +AT_CLEANUP
After the I-P has been introduced in commit 354bdba51a ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data"), the QoS on localnet ports is not properly configured if the ovs interface is marked with "ovn-egress-iface" flag after the related record in the logical_switch_port table is set. The issue can be triggered with the following reproducer: $ovn-nbctl set Logical_Switch_Port ln-public options:qos_burst=1000 $ovs-vsctl set interface eth0 external-ids:ovn-egress-iface=true Fix the issue triggering a recomputation after qos is configured for ovs interface Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data") Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- controller/binding.c | 26 +++++++++++++++++ controller/binding.h | 1 + tests/ovn-performance.at | 13 +++++++++ tests/system-ovn.at | 60 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+)