|
Bugzilla – Full Text Bug Listing |
| Summary: | OpenflowSwitchNetDevice flooding bug | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Tom Henderson <tomh> |
| Component: | openflow | Assignee: | ns-bugs <ns-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | P5 | ||
| Version: | pre-release | ||
| Hardware: | PC | ||
| OS: | Linux | ||
pushed in changeset 11619:0b8c6e883208 with the following commit message When you have the action OFPP_FLOOD on a flow, and the received packet is not coming into the switch through port 0, then the packet will be flooded on all ports, including the receiving port! This is especially bad if you have broadcasts as packets will get replicated indefinitely (ARP req for e.g.). This is caused by missing ntohs() that fails to convert port numbers to their correct values. The switch works correctly for broadcast packets received on port zero. Example: The ReceivePacketOut() gets 0 if the input port is 0 and it works. But will get 512 if the input port is 1 and since 512 is not our real input port the check will fail and the packet will also be replicated on the input port. |
Reported by Ovidiu Poncea on the ns-developers list: ----- So, if you have the action OFPP_FLOOD and the received packet is not coming into port 0 of the switch then the packet will be flooded on all ports including the receiving port - which is very bad specially if you have broadcasts as packets will get replicated indefinitely (ARP req for e.g.). This is caused by a missing ntohs(). The ReceivePacketOut() gets 0 if the input port is 0 and it works but 512 if the input port is 1 and we don't have this port. Oh, and I'm using 3.20 but it's still present in ns3-dev. Here is my patch: Fix flooding of packets on input port, for packets received on in!=0 Packets that were not received on port zero were also flooded on input. Because htons(0) will return 0 it works for packets coming in on port 0. Seems that someone encountered some issues and left a trail of commented modifications - I left some of them unmodified as they seems unrelated to this issue. ns-3.20/src/openflow/model/openflow-switch-net-device.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ns-3.20/src/openflow/model/openflow-switch-net-device.cc b/ns-3.20/src/openflow/model/openflow-switch-net-device.cc index 9071ed1..65c4711 100644 --- a/ns-3.20/src/openflow/model/openflow-switch-net-device.cc +++ b/ns-3.20/src/openflow/model/openflow-switch-net-device.cc @@ -1164,7 +1164,7 @@ OpenFlowSwitchNetDevice::ReceivePacketOut (const void *msg) } sw_flow_key key; - flow_extract (buffer, opo->in_port, &key.flow); // ntohs(opo->in_port) + flow_extract (buffer, ntohs(opo->in_port), &key.flow); // ntohs(opo->in_port) uint16_t v_code = ofi::ValidateActions (&key, opo->actions, actions_len); if (v_code != ACT_VALIDATION_OK) @@ -1307,7 +1307,7 @@ OpenFlowSwitchNetDevice::AddFlow (const ofp_flow_mod *ofm) { sw_flow_key key; flow_used (flow, buffer); - flow_extract (buffer, ofm->match.in_port, &key.flow); // ntohs(ofm->match.in_port); + flow_extract (buffer, ntohs(ofm->match.in_port), &key.flow); // ntohs(ofm->match.in_port); ofi::ExecuteActions (this, ofm->buffer_id, buffer, &key, ofm->actions, actions_len, false); ofpbuf_delete (buffer); } @@ -1348,7 +1348,7 @@ OpenFlowSwitchNetDevice::ModFlow (const ofp_flow_mod *ofm) if (buffer) { sw_flow_key skb_key; - flow_extract (buffer, ofm->match.in_port, &skb_key.flow); // ntohs(ofm->match.in_port); + flow_extract (buffer, ntohs(ofm->match.in_port), &skb_key.flow); // ntohs(ofm->match.in_port); ofi::ExecuteActions (this, ofm->buffer_id, buffer, &skb_key, ofm->actions, actions_len, false); ofpbuf_delete (buffer); }