search

GlobalNOC / FlowSpaceFirewall

FlowSpace Firewall Application a floodlight based controller allowing multiple controllers to talk to a single switch, but can not interact with each others flow space (hence FlowSpace Firewall)
http://globalnoc.iu.edu/sdn/fsfw.html
Apache License 2.0
14 stars 14 forks source link

Managed tag mode sets an incorrect total_len in the ofp_packet_in header #116

Closed gth828r closed 9 years ago

gth828r commented 9 years ago

When using managed tag mode, the total_len field in packet-in messages is incorrect when the packet-in reaches the upstream controller. Note that the length field (tracking the size of the OF packet) is set correctly, and just the total_len (tracking the size of the data frame encapsulated in the OF packet) is incorrect.

As an example, I took a packet capture of a packet-in as it came in from the switch and as it went out to the controller with the switch under managed tag mode for the slice.

Here it is coming in: total_len_correct

Note that the byte count in the lower left matches the total_len field in the ofp_packet_in header.

Now, here is a snapshot of a packet leaving: total_len_incorrect

Again, note the total_len field as compared to the byte count in the lower left. This time, they are off by four.

gth828r commented 9 years ago

In my testing, I found that things were not always off by 4 (although it seems to consistently be off by more than 4 bytes). For example, I was doing packet captures at my controller, and when not running in managed tag mode, I saw some padding bytes at the end of an arp packet. When running in managed tag mode, those bytes were gone, but I don't think that total_len had been updated to account for that either. I'll capture a snapshot of that as well and post it here. I'll also try to get the full packet captures up so you all can look at them yourself.

gth828r commented 9 years ago

Note that this problem is really an issue for controllers that validate this sort of thing. I am using a modified version of the pox forwarding.l2_learning, and part of what it does calls an underlying pox library that verifies that the packet-in data is present in-full. When it detects that the length of the data does not match what is in the header total_len field, it bails out. That is a bit extreme, but that is just what it does.

ajragusa commented 9 years ago

Thanks for the report we'll make sure we take care of this in the 1.0.5 release. GlobalNOC ticket 11135 is tracking this and we will make sure to update this with with any questions and status updates

gth828r commented 9 years ago

Thanks!

For completeness, here are the snapshots of the arp situation with padding that I mentioned before. This pcap was taken at the slicer, and the correct one is on the southbound, with the incorrect one on the northbound:

Correct: arp_total_len_correct

Incorrect: arp_total_len_incorrect

Note that the "trailer" in the southbound packet is 14 bytes long. The total_len is off by 18 bytes, so that is consistent with the 4 bytes missing in the previously attached snapshots + the 14 byte trailers.

I decided not to upload the full packet captures unless you all tell me you really want them, since I'll need to describe a bit more of my setup.

ajragusa commented 9 years ago

I think this is more than enough data for us to play with, we will attempt to replicate and fix for the next FSFW release

gth828r commented 9 years ago

FYI, based on our timeline, I need to run some tests with this fix applied. I haven't run anything through the motions yet (doing that tomorrow), but I patched master to deal with this, and I got something that appears to be working OK based on some basic tests. If you all want, I can apply that change to the 1.0.5-dev branch and issue a pull request, or you all are welcome to just steal what I have and drop it in: https://github.com/gth828r/FlowSpaceFirewall/commit/68121da28976e5e9f474eeac346f775000de443a

Its a pretty small change.

ajragusa commented 9 years ago

Please send a pull request to the 1.0.5-dev branch with that.

Thanks!

gth828r commented 9 years ago

Ok, I cherry-picked my commit in master into the 1.0.5-dev branch and sent a pull request using that.

ajragusa commented 9 years ago

Yep, already pulled in. Thanks!