-
Notifications
You must be signed in to change notification settings - Fork 103
Description
Describe the bug
In bridge service a deadlock may occur when deleting a port while a packet is being processed in the slowpath.
Mutex ports_mutex_ is acquired when removing a port
polycube/src/services/pcn-bridge/src/Bridge.cpp
Lines 154 to 158 in 2e4f4e2
| void Bridge::delPorts(const std::string &name) { | |
| std::lock_guard<std::mutex> guard(ports_mutex_); | |
| BridgeBase::delPorts(name); | |
| } |
This function then calls function
Cube<PortType>::remove_port() that acquires cube_mutex polycube/src/libs/polycube/include/polycube/services/cube.h
Lines 153 to 157 in 2e4f4e2
| template <class PortType> | |
| void Cube<PortType>::remove_port(const std::string &port_name) { | |
| // avoid deleting a port while processing a packet_in event | |
| std::lock_guard<std::mutex> guard(cube_mutex); | |
Packets coming from the fast path are processed acquiring those mutex in reverse order.
The callback acquires cube_mutex
polycube/src/libs/polycube/include/polycube/services/cube.h
Lines 69 to 86 in 2e4f4e2
| handle_packet_in = [&](const PacketIn *md, | |
| const std::vector<uint8_t> &packet) -> void { | |
| // This lock guarantees: | |
| // - port is not deleted while processing it | |
| // - service implementation is not deleted wile processing it | |
| std::lock_guard<std::mutex> guard(cube_mutex); | |
| if (dismounted_) | |
| return; | |
| auto &p = *ports_by_id_.at(md->port_id); | |
| PacketInMetadata md_; | |
| md_.traffic_class = md->traffic_class; | |
| md_.reason = md->reason; | |
| md_.metadata[0] = md->metadata[0]; | |
| md_.metadata[1] = md->metadata[1]; | |
| md_.metadata[2] = md->metadata[2]; | |
| packet_in(p, md_, packet); | |
| }; |
and method
broadcast() of the bridge then acquires ports_mutex_ polycube/src/services/pcn-bridge/src/Bridge.cpp
Lines 111 to 120 in 2e4f4e2
| void Bridge::broadcastPacket(Ports &port, | |
| polycube::service::PacketInMetadata &md, | |
| const std::vector<uint8_t> &packet) { | |
| logger()->trace("Broadcasting packet.."); | |
| bool tagged = md.metadata[1]; | |
| uint16_t vlan = md.metadata[0]; | |
| logger()->trace("metadata vlan:{0} tagged:{1}", vlan, tagged); | |
| std::lock_guard<std::mutex> guard(ports_mutex_); |
Causing a deadlock.
To Reproduce
This is difficult to reproduce since it depends on timing.
I found it while running test pcn-bridge/tests/vlan/test1.sh on my PR #298
Expected behavior
No deadlock should occur.
Please tell us about your environment:
- OS details: Ubuntu 18.04
- Kernel details: 5.3.0
Additional context
I think ports_mutex_ can be safely removed.
As far as I understand both the mutex have the same function: avoid deleting a port while processing a packet from the fastpath