[CONTROLLER-28] Incorrect data types for certain Flow attributes Created: 20/Aug/13  Updated: 17/Jun/20  Resolved: 19/May/16

Status: Resolved
Project: controller
Component/s: adsal
Affects Version/s: 0.4.0
Fix Version/s: None

Type: Bug
Reporter: Tejas Nevrekar Assignee: Unassigned
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All

Attachments: PNG File OF1.0.0_Sec5.3.3_FlowFields.png    
External issue ID: 57

 Description   

The following data types used in the ODL code base for the Flow class seem incorrect looking at what is required by the OF1.0.0 standard. Any boundary tests will fail using these data types.

Field ODL Data Type ODL Range OF 1.0.0 Data Type OF 1.0.0 Range
idle_timeout short -32767 to +32767 uint16_t 0 to +65535
hard_timeout short -32767 to +32767 uint16_t 0 to +65535
priority short -32767 to +32767 uint16_t 0 to +65535

I see the same data types used in the SAL as well as the plugin layer for these fields. The fix for this needs to be done to both these data structures. A probable solution could be to move to the java int, in spite of its known costs.

 Comments   
Comment by Tejas Nevrekar [ 20/Aug/13 ]

Attachment OF1.0.0_Sec5.3.3_FlowFields.png has been added with description: OF1.0.0 Sec 5.3.3 for Flow fields

Comment by Rajani Srivastava [ 14/Jan/14 ]

Can you tell me the steps to reproduce errors in reference to boundary tests.

As unsigned primitive data types are not supported in java,as solution we can use 'int' in place of 'short'.But the change will have impact on other modules. Moreover , i found that there are other variables such as 'table_id'in FlowOnNode.java in package org.opendaylight.controller.sal.reader;

Field ODL Data Type ODL Range OF 1.0.0 Data Type OF 1.0.0 Range
table_id byte -127 to 127 uint8_t 0 to +255

Will this also fail the boundary tests? What are the expected changes according to you?

Comment by Tony Tkacik [ 06/Mar/14 ]

Google Guava provides unsigned types implementation, also other approach is to use larger types /eg. java.int for uint16, long for uint32 and do the range checks.

Comment by Tony Tkacik [ 06/Mar/14 ]

Accidentally closed, reopening.

Comment by Tejas Nevrekar [ 06/Mar/14 ]

The Openflow library provides for such conversion classes. The OFFlowMod may be updated as follows:

1. Change all short to int for command, idleTimeout, hardTimeout, priority, bufferId, outPort, flags
2. Update the readFrom writeTo methods
@Override
public void readFrom(ChannelBuffer data)

{ super.readFrom(data); if (this.match == null) this.match = new OFMatch(); this.match.readFrom(data); this.cookie = data.readLong(); this.command = U16.f(data.readShort()); this.idleTimeout = U16.f(data.readShort()); this.hardTimeout = U16.f(data.readShort()); this.priority = U16.f(data.readShort()); this.bufferId = data.readInt(); this.outPort = U16.f(data.readShort()); this.flags = U16.f(data.readShort()); if (this.actionFactory == null) throw new RuntimeException("OFActionFactory not set"); this.actions = this.actionFactory.parseActions(data, getLengthU() - MINIMUM_LENGTH); }

@Override
public void writeTo(ChannelBuffer data) {
super.writeTo(data);
this.match.writeTo(data);
data.writeLong(cookie);
data.writeShort(U16.t(command));
data.writeShort(U16.t(idleTimeout));
data.writeShort(U16.t(hardTimeout));
data.writeShort(U16.t(priority));
data.writeInt(bufferId);
data.writeShort(U16.t(outPort));
data.writeShort(U16.t(flags));
if (actions != null) {
for (OFAction action : actions)

{ action.writeTo(data); }

}
}

3. Update the data types for Flow to be int for these variables.

Comment by Carol Sanders [ 04/May/15 ]

This bug is part of the project to Move all ADSAL associated component bugs to ADSAL

Comment by Robert Varga [ 19/May/16 ]

AD-SAL has been removed, this issue will not be fixed.

Generated at Wed Feb 07 19:52:00 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.