Fix bug in IP wildcarding.

[jnfoster]

This pull requests fixes a bug in the calculation of IP masks in OpenFlow 0x01.

[seliopou]

Whitespace.

[seliopou]

The line-wrapping introduced more whitespace errors. There are also tabs:

$ git diff --check master..HEAD
lib/SDN_OpenFlow0x01.ml:54: tab in indent.
+        if m = 32l then
lib/SDN_OpenFlow0x01.ml:55: tab in indent.
+          None
lib/SDN_OpenFlow0x01.ml:56: tab in indent.
+        else
lib/SDN_OpenFlow0x01.ml:57: tab in indent.
+          Some (Int32.sub 32l m) in
lib/SDN_OpenFlow0x01.ml:63: tab in indent.
+        if m = 32l then
lib/SDN_OpenFlow0x01.ml:64: tab in indent.
+          None
lib/SDN_OpenFlow0x01.ml:65: tab in indent.
+        else
lib/SDN_OpenFlow0x01.ml:66: tab in indent.
+          Some (Int32.sub 32l m) in
lib/SDN_OpenFlow0x01.ml:132: trailing whitespace.
+      | AL.Modify (AL.SetEthTyp _) -> 
lib/SDN_OpenFlow0x01.ml:133: tab in indent.
+        raise (Invalid_argument "cannot set Ethernet type")
lib/SDN_OpenFlow0x01.ml:134: trailing whitespace.
+      | AL.Modify (AL.SetIPProto _) -> 
lib/SDN_OpenFlow0x01.ml:135: tab in indent.
+        raise (Invalid_argument "cannot set IP protocol")
lib/SDN_OpenFlow0x01.ml:147: trailing whitespace.
+  : Core.action list = 
lib/SDN_OpenFlow0x01.ml:151: trailing whitespace.
+  | _ -> 

Adding the pre-commit hook will notify you of stuff like this before you commit:

$ cp .git/hooks/pre-commit{.sample,}

[jnfoster]

By the way: @mcanini @fugitifduck I only fixed 1.0 since it's the code I'm most familiar with. Do you know if 1.3 and 1.4 might also have the same bug?

-N

[fugitifduck]

@jnfoster I think, line 20 of OpenFlow0x0(4|5)_Core.ml is code like in 0x01.

But, starting from 0x02, nwsrc is include in Oxm, and the mask is a CIDR mask (where /32 is exact and /0 is wildcard).
It can also be arbitrary masking... (currently not implemented i think)

[jnfoster]

Cool. So, just to make sure I understand, they switched to CIDR conventions so there shouldn't be a bug in our 1.3/1.4 code?

[fugitifduck]

I think (p52 of OF1.3.3 spec)

[jnfoster]

Excellent. Thanks!

[mcanini]

@jnfoster

Actually I am not sure that this won't be a bug in 1.3/1.4 too.

Quoting the specs (1.3.3): "Each 1-bit in oxm_mask constrains the OXM TLV to match only packets in which the corresponding bit of the field equals the the corresponding bit in oxm_value. Each 0-bit in oxm_mask places no constraint on the corresponding bit in the field."

So, in my interpretation, something link 192.168.1.0/24 means that the mask has to be
24 MSB set to 1 followed by 8 LSB set to 0.

[mcanini]

So, after a bit of investigation, indeed it seems that the bug affects OF1.3/1.4 too.
@fugitifduck is implementing the fix. Thanks!

[jnfoster]

Fantastic! Great to hear.

-N

On Fri, Sep 5, 2014 at 10:58 AM, Marco Canini notifications@github.com
wrote:

So, after a bit of investigation, indeed it seems that the bug affects
OF1.3/1.4 too.
@fugitifduck https://github.com/fugitifduck is implementing the fix.
Thanks!

—
Reply to this email directly or view it on GitHub
#172 (comment)
.