| Message ID | 20220829092720.1574621-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] controller: fix potential segmentation violation when removing ports | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Aug 29, 2022 at 2:27 AM Xavier Simonart <xsimonar@redhat.com> wrote: > > If a logical switch port is added and connected to a logical router > port (through options: router-port) before the router port is > created, then this might cause further issues such as segmentation > violation when the switch and router ports are deleted. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > --- > v2: - handled Han's comments (avoid wasting CPU cycles searching for peer_ld) > v3: - handled additional case causing potential crash > - add test case covering this additional potential crash > - remove ofport-request from test case > - rebased on origin/main > --- > controller/local_data.c | 38 +++++++----------- > controller/pinctrl.c | 16 ++++++-- > tests/ovn.at | 89 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 117 insertions(+), 26 deletions(-) > > diff --git a/controller/local_data.c b/controller/local_data.c > index 7f874fc19..9eee568d1 100644 > --- a/controller/local_data.c > +++ b/controller/local_data.c > @@ -34,7 +34,7 @@ > > VLOG_DEFINE_THIS_MODULE(ldata); > > -static void add_local_datapath__( > +static struct local_datapath *add_local_datapath__( > struct ovsdb_idl_index *sbrec_datapath_binding_by_key, > struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > struct ovsdb_idl_index *sbrec_port_binding_by_name, > @@ -194,17 +194,7 @@ add_local_datapath_peer_port( > return; > } > > - bool present = false; > - for (size_t i = 0; i < ld->n_peer_ports; i++) { > - if (ld->peer_ports[i].local == pb) { > - present = true; > - break; > - } > - } > - > - if (!present) { > - local_datapath_peer_port_add(ld, pb, peer); > - } > + local_datapath_peer_port_add(ld, pb, peer); > > struct local_datapath *peer_ld = > get_local_datapath(local_datapaths, > @@ -218,12 +208,6 @@ add_local_datapath_peer_port( > return; > } > > - for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { > - if (peer_ld->peer_ports[i].local == peer) { > - return; > - } > - } > - > local_datapath_peer_port_add(peer_ld, peer, pb); > } > > @@ -541,7 +525,7 @@ chassis_tunnel_find(const struct hmap *chassis_tunnels, const char *chassis_id, > } > > /* static functions. */ > -static void > +static struct local_datapath * > add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, > struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > struct ovsdb_idl_index *sbrec_port_binding_by_name, > @@ -553,7 +537,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, > uint32_t dp_key = dp->tunnel_key; > struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); > if (ld) { > - return; > + return ld; > } > > ld = local_datapath_alloc(dp); > @@ -568,7 +552,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, > if (depth >= 100) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > VLOG_WARN_RL(&rl, "datapaths nested too deep"); > - return; > + return ld; > } > > struct sbrec_port_binding *target = > @@ -589,19 +573,22 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, > if (peer && peer->datapath) { > if (need_add_patch_peer_to_local( > sbrec_port_binding_by_name, pb, chassis)) { > - add_local_datapath__(sbrec_datapath_binding_by_key, > + struct local_datapath *peer_ld = > + add_local_datapath__(sbrec_datapath_binding_by_key, > sbrec_port_binding_by_datapath, > sbrec_port_binding_by_name, > depth + 1, peer->datapath, > chassis, local_datapaths, > tracked_datapaths); > + local_datapath_peer_port_add(peer_ld, peer, pb); > + local_datapath_peer_port_add(ld, pb, peer); > } > - local_datapath_peer_port_add(ld, pb, peer); > } > } > } > } > sbrec_port_binding_index_destroy_row(target); > + return ld; > } > > static struct tracked_datapath * > @@ -622,6 +609,11 @@ local_datapath_peer_port_add(struct local_datapath *ld, > const struct sbrec_port_binding *local, > const struct sbrec_port_binding *remote) > { > + for (size_t i = 0; i < ld->n_peer_ports; i++) { > + if (ld->peer_ports[i].local == local) { > + return; > + } > + } > ld->n_peer_ports++; > if (ld->n_peer_ports > ld->n_allocated_peer_ports) { > size_t old_n_ports = ld->n_allocated_peer_ports; > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index eeb6f7527..3f5d0af79 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -181,6 +181,7 @@ static void init_buffered_packets_map(void); > static void destroy_buffered_packets_map(void); > static void > run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, > + struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > const struct hmap *local_datapaths) > OVS_REQUIRES(pinctrl_mutex); > > @@ -3584,6 +3585,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > sbrec_igmp_groups, > sbrec_ip_multicast_opts); > run_buffered_binding(sbrec_mac_binding_by_lport_ip, > + sbrec_port_binding_by_datapath, > local_datapaths); > sync_svc_monitors(ovnsb_idl_txn, svc_mon_table, sbrec_port_binding_by_name, > chassis); > @@ -4354,6 +4356,7 @@ run_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn, > > static void > run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, > + struct ovsdb_idl_index *sbrec_port_binding_by_datapath, > const struct hmap *local_datapaths) > OVS_REQUIRES(pinctrl_mutex) > { > @@ -4369,9 +4372,15 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, > continue; > } > > - for (size_t i = 0; i < ld->n_peer_ports; i++) { > - > - const struct sbrec_port_binding *pb = ld->peer_ports[i].local; > + struct sbrec_port_binding *target = > + sbrec_port_binding_index_init_row(sbrec_port_binding_by_datapath); > + sbrec_port_binding_index_set_datapath(target, ld->datapath); > + const struct sbrec_port_binding *pb; > + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, > + sbrec_port_binding_by_datapath) { > + if (strcmp(pb->type, "patch")) { > + continue; > + } > struct buffered_packets *cur_qp; > HMAP_FOR_EACH_SAFE (cur_qp, hmap_node, &buffered_packets_map) { > struct ds ip_s = DS_EMPTY_INITIALIZER; > @@ -4388,6 +4397,7 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, > ds_destroy(&ip_s); > } > } > + sbrec_port_binding_index_destroy_row(target); > } > buffered_packets_map_gc(); > > diff --git a/tests/ovn.at b/tests/ovn.at > index 5d73c3379..c6ac9e5ca 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -32710,3 +32710,92 @@ done > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([router port add then remove - lsp first]) > +ovn_start > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +ovs-vsctl -- add-port br-int hv1-vif1 -- \ > + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lr-add ro0 > +check ovn-nbctl lsp-add sw0 sw0-p1 > +check ovn-nbctl lsp-add sw0 lsp > +check ovn-nbctl lsp-set-type lsp router > +check ovn-nbctl lsp-set-options lsp router-port=lrp > +check ovn-nbctl lsp-set-addresses lsp 00:00:00:00:00:1 > +check ovn-nbctl --wait=hv lrp-add ro0 lrp 00:00:00:00:00:1 aef0:0:0:0:0:0:0:1/64 > +check ovn-nbctl --wait=hv lsp-del lsp > +check ovn-nbctl lrp-del lrp > +check ovn-nbctl --wait=hv sync > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([router port add then remove - distributed router gateway port]) > +ovn_start > + > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +ovs-vsctl -- add-port br-int hv1-vif1 -- \ > + set interface hv1-vif1 external-ids:iface-id=sw0-p0 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap > + > +sim_add hv2 > +as hv2 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.2 > +ovs-vsctl -- add-port br-int hv2-vif1 -- \ > + set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ > + options:tx_pcap=hv2/vif1-tx.pcap \ > + options:rxq_pcap=hv2/vif1-rx.pcap > + > +ovs-vsctl -- add-port br-int hv2-vif2 -- \ > + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ > + options:tx_pcap=hv2/vif2-tx.pcap \ > + options:rxq_pcap=hv2/vif2-rx.pcap > + > +ovn-nbctl create Logical_Router name=lr0 > +ovn-nbctl ls-add sw0 > +ovn-nbctl ls-add sw1 > + > +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:01:01:02:03 192.168.1.1/24 > +ovn-nbctl lsp-add sw0 sw0-lr0 -- set Logical_Switch_Port sw0-lr0 \ > + type=router options:router-port=lr0-sw0 \ > + -- lsp-set-addresses sw0-lr0 router > + > +ovn-nbctl lrp-add lr0 lr0-sw1 00:00:02:01:02:03 172.16.1.1/24 \ > + -- lrp-set-gateway-chassis lr0-sw1 hv2 > +ovn-nbctl lsp-add sw1 sw1-lr0 -- set Logical_Switch_Port sw1-lr0 \ > + type=router options:router-port=lr0-sw1 \ > + -- lsp-set-addresses sw1-lr0 router > + > +ovn-nbctl lsp-add sw0 sw0-p0 \ > + -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2" > + > +ovn-nbctl lsp-add sw0 sw0-p1 \ > + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3" > + > +ovn-nbctl --wait=hv lsp-add sw1 sw1-p0 \ > + -- lsp-set-addresses sw1-p0 unknown > + > +check ovn-nbctl --wait=hv lsp-del sw1-lr0 > +check ovn-nbctl lrp-del lr0-sw1 > + > +OVN_CLEANUP([hv1],[hv2]) > +AT_CLEANUP > +]) > -- > 2.31.1 > Thanks again Xavier. I applied to main and backported to 22.06 and 22.03. Han
On 8/29/22 22:49, Han Zhou wrote: > On Mon, Aug 29, 2022 at 2:27 AM Xavier Simonart <xsimonar@redhat.com> wrote: >> >> If a logical switch port is added and connected to a logical router >> port (through options: router-port) before the router port is >> created, then this might cause further issues such as segmentation >> violation when the switch and router ports are deleted. >> >> Signed-off-by: Xavier Simonart <xsimonar@redhat.com> >> >> --- >> v2: - handled Han's comments (avoid wasting CPU cycles searching for > peer_ld) >> v3: - handled additional case causing potential crash >> - add test case covering this additional potential crash >> - remove ofport-request from test case >> - rebased on origin/main >> --- >> controller/local_data.c | 38 +++++++----------- >> controller/pinctrl.c | 16 ++++++-- >> tests/ovn.at | 89 +++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 117 insertions(+), 26 deletions(-) > > Thanks again Xavier. I applied to main and backported to 22.06 and 22.03. > > Han Hey, Han and Xavier. Not sure if it is the same or similar issue, but I caught this today: ../../controller/binding.c:2515:42: runtime error: member access within null pointer of type 'struct sbrec_datapath_binding' #0 0x4dd10f in handle_deleted_lport /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../controller/binding.c:2515:42 #1 0x4dcc74 in handle_deleted_vif_lport /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../controller/binding.c:2587:5 #2 0x4da9aa in binding_handle_port_binding_changes /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../controller/binding.c:2908:19 #3 0x5be01b in runtime_data_sb_port_binding_handler /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../controller/ovn-controller.c:1622:10 #4 0x67b9fe in engine_compute /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../lib/inc-proc-eng.c:403:28 #5 0x67a260 in engine_run_node /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../lib/inc-proc-eng.c:465:14 #6 0x6798f1 in engine_run /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../lib/inc-proc-eng.c:490:9 #7 0x5ac053 in main /home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/../../controller/ovn-controller.c #8 0x7f92c25a6082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) #9 0x41fe7d in _start (/home/runner/work/ovn/ovn/ovn-22.09.90/_build/sub/controller/ovn-controller+0x41fe7d) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../controller/binding.c:2515:42 in Here is a failed job: https://github.com/igsilya/ovn/runs/8162460040?check_suite_focus=true#step:13:5493 The branch is a latest main branch with the fix already applied. There are some northd changes, but only pure performance, no logical changes. And no ovn-controller changes. Best regards, Ilya Maximets.
diff --git a/controller/local_data.c b/controller/local_data.c index 7f874fc19..9eee568d1 100644 --- a/controller/local_data.c +++ b/controller/local_data.c @@ -34,7 +34,7 @@ VLOG_DEFINE_THIS_MODULE(ldata); -static void add_local_datapath__( +static struct local_datapath *add_local_datapath__( struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, @@ -194,17 +194,7 @@ add_local_datapath_peer_port( return; } - bool present = false; - for (size_t i = 0; i < ld->n_peer_ports; i++) { - if (ld->peer_ports[i].local == pb) { - present = true; - break; - } - } - - if (!present) { - local_datapath_peer_port_add(ld, pb, peer); - } + local_datapath_peer_port_add(ld, pb, peer); struct local_datapath *peer_ld = get_local_datapath(local_datapaths, @@ -218,12 +208,6 @@ add_local_datapath_peer_port( return; } - for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { - if (peer_ld->peer_ports[i].local == peer) { - return; - } - } - local_datapath_peer_port_add(peer_ld, peer, pb); } @@ -541,7 +525,7 @@ chassis_tunnel_find(const struct hmap *chassis_tunnels, const char *chassis_id, } /* static functions. */ -static void +static struct local_datapath * add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, @@ -553,7 +537,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, uint32_t dp_key = dp->tunnel_key; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); if (ld) { - return; + return ld; } ld = local_datapath_alloc(dp); @@ -568,7 +552,7 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, if (depth >= 100) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "datapaths nested too deep"); - return; + return ld; } struct sbrec_port_binding *target = @@ -589,19 +573,22 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, if (peer && peer->datapath) { if (need_add_patch_peer_to_local( sbrec_port_binding_by_name, pb, chassis)) { - add_local_datapath__(sbrec_datapath_binding_by_key, + struct local_datapath *peer_ld = + add_local_datapath__(sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, depth + 1, peer->datapath, chassis, local_datapaths, tracked_datapaths); + local_datapath_peer_port_add(peer_ld, peer, pb); + local_datapath_peer_port_add(ld, pb, peer); } - local_datapath_peer_port_add(ld, pb, peer); } } } } sbrec_port_binding_index_destroy_row(target); + return ld; } static struct tracked_datapath * @@ -622,6 +609,11 @@ local_datapath_peer_port_add(struct local_datapath *ld, const struct sbrec_port_binding *local, const struct sbrec_port_binding *remote) { + for (size_t i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == local) { + return; + } + } ld->n_peer_ports++; if (ld->n_peer_ports > ld->n_allocated_peer_ports) { size_t old_n_ports = ld->n_allocated_peer_ports; diff --git a/controller/pinctrl.c b/controller/pinctrl.c index eeb6f7527..3f5d0af79 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -181,6 +181,7 @@ static void init_buffered_packets_map(void); static void destroy_buffered_packets_map(void); static void run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, + struct ovsdb_idl_index *sbrec_port_binding_by_datapath, const struct hmap *local_datapaths) OVS_REQUIRES(pinctrl_mutex); @@ -3584,6 +3585,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, sbrec_igmp_groups, sbrec_ip_multicast_opts); run_buffered_binding(sbrec_mac_binding_by_lport_ip, + sbrec_port_binding_by_datapath, local_datapaths); sync_svc_monitors(ovnsb_idl_txn, svc_mon_table, sbrec_port_binding_by_name, chassis); @@ -4354,6 +4356,7 @@ run_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn, static void run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, + struct ovsdb_idl_index *sbrec_port_binding_by_datapath, const struct hmap *local_datapaths) OVS_REQUIRES(pinctrl_mutex) { @@ -4369,9 +4372,15 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, continue; } - for (size_t i = 0; i < ld->n_peer_ports; i++) { - - const struct sbrec_port_binding *pb = ld->peer_ports[i].local; + struct sbrec_port_binding *target = + sbrec_port_binding_index_init_row(sbrec_port_binding_by_datapath); + sbrec_port_binding_index_set_datapath(target, ld->datapath); + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, + sbrec_port_binding_by_datapath) { + if (strcmp(pb->type, "patch")) { + continue; + } struct buffered_packets *cur_qp; HMAP_FOR_EACH_SAFE (cur_qp, hmap_node, &buffered_packets_map) { struct ds ip_s = DS_EMPTY_INITIALIZER; @@ -4388,6 +4397,7 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, ds_destroy(&ip_s); } } + sbrec_port_binding_index_destroy_row(target); } buffered_packets_map_gc(); diff --git a/tests/ovn.at b/tests/ovn.at index 5d73c3379..c6ac9e5ca 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -32710,3 +32710,92 @@ done OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([router port add then remove - lsp first]) +ovn_start +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap + +check ovn-nbctl ls-add sw0 +check ovn-nbctl lr-add ro0 +check ovn-nbctl lsp-add sw0 sw0-p1 +check ovn-nbctl lsp-add sw0 lsp +check ovn-nbctl lsp-set-type lsp router +check ovn-nbctl lsp-set-options lsp router-port=lrp +check ovn-nbctl lsp-set-addresses lsp 00:00:00:00:00:1 +check ovn-nbctl --wait=hv lrp-add ro0 lrp 00:00:00:00:00:1 aef0:0:0:0:0:0:0:1/64 +check ovn-nbctl --wait=hv lsp-del lsp +check ovn-nbctl lrp-del lrp +check ovn-nbctl --wait=hv sync +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([router port add then remove - distributed router gateway port]) +ovn_start + +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw0-p0 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap + +sim_add hv2 +as hv2 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.2 +ovs-vsctl -- add-port br-int hv2-vif1 -- \ + set interface hv2-vif1 external-ids:iface-id=sw1-p0 \ + options:tx_pcap=hv2/vif1-tx.pcap \ + options:rxq_pcap=hv2/vif1-rx.pcap + +ovs-vsctl -- add-port br-int hv2-vif2 -- \ + set interface hv2-vif2 external-ids:iface-id=sw0-p1 \ + options:tx_pcap=hv2/vif2-tx.pcap \ + options:rxq_pcap=hv2/vif2-rx.pcap + +ovn-nbctl create Logical_Router name=lr0 +ovn-nbctl ls-add sw0 +ovn-nbctl ls-add sw1 + +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:01:01:02:03 192.168.1.1/24 +ovn-nbctl lsp-add sw0 sw0-lr0 -- set Logical_Switch_Port sw0-lr0 \ + type=router options:router-port=lr0-sw0 \ + -- lsp-set-addresses sw0-lr0 router + +ovn-nbctl lrp-add lr0 lr0-sw1 00:00:02:01:02:03 172.16.1.1/24 \ + -- lrp-set-gateway-chassis lr0-sw1 hv2 +ovn-nbctl lsp-add sw1 sw1-lr0 -- set Logical_Switch_Port sw1-lr0 \ + type=router options:router-port=lr0-sw1 \ + -- lsp-set-addresses sw1-lr0 router + +ovn-nbctl lsp-add sw0 sw0-p0 \ + -- lsp-set-addresses sw0-p0 "f0:00:00:01:02:03 192.168.1.2" + +ovn-nbctl lsp-add sw0 sw0-p1 \ + -- lsp-set-addresses sw0-p1 "f0:00:00:11:02:03 192.168.1.3" + +ovn-nbctl --wait=hv lsp-add sw1 sw1-p0 \ + -- lsp-set-addresses sw1-p0 unknown + +check ovn-nbctl --wait=hv lsp-del sw1-lr0 +check ovn-nbctl lrp-del lr0-sw1 + +OVN_CLEANUP([hv1],[hv2]) +AT_CLEANUP +])
If a logical switch port is added and connected to a logical router port (through options: router-port) before the router port is created, then this might cause further issues such as segmentation violation when the switch and router ports are deleted. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- v2: - handled Han's comments (avoid wasting CPU cycles searching for peer_ld) v3: - handled additional case causing potential crash - add test case covering this additional potential crash - remove ofport-request from test case - rebased on origin/main --- controller/local_data.c | 38 +++++++----------- controller/pinctrl.c | 16 ++++++-- tests/ovn.at | 89 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 26 deletions(-)