[OPNFLWPLUG-222] MD-SAL app cannot create a flow entry which adds a VLAN tag with the specified VLAN ID into untagged frame. Created: 24/Jul/14 Updated: 27/Sep/21 Resolved: 11/Sep/14 |
|
| Status: | Resolved |
| Project: | OpenFlowPlugin |
| Component/s: | General |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | ||
| Reporter: | Hideyuki Tai | Assignee: | Michal Rehak |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Operating System: All |
||
| Issue Links: |
|
||||||||
| External issue ID: | 1421 | ||||||||
| Description |
|
MD-SAL applications cannot correctly create a flow entry which adds a VLAN tag with the specified VLAN ID into untagged ethernet frame. If a MD-SAL application uses set-vlan-id-action, the MD-SAL OF plugin translates it into SET_VLAN_ID action for OF 1.0 switch (this is OK), and translates it SET_FIELD(VLAN_VID) action for OF 1.3 switch (this is NG). (Please note that OF 1.3 flow entry that adds a VLAN tag with the specified VLAN ID into untagged ethernet frame needs to contain PUSH_VLAN action before SET_FIELD(VLAN_VID) action.) I pointed out this issue in the mail on openflowplugin-dev ML. Consequently VTN Manager does not work correctly with OF 1.3 switches. VTN project requests that this issue is fixed by Milestone 5 of Helium release. |
| Comments |
| Comment by Hideyuki Tai [ 24/Jul/14 ] |
|
One possible solution for this problem is that the MD-SAL OF plugin handles a MD-SAL action as a semantic equivalent to SET_VLAN_VID action of OF 1.0. (The semantic of SET_VLAN_VID of OF 1.0 action is explained in the first line of the table in page 7 of OpenFlow spec v1.0.0. Other possible solution is that the MD-SAL OF plugin provides information of supported actions for switches. |
| Comment by Kamal Rameshan [ 19/Aug/14 ] |
|
The scenario which I am trying to implement is as below: Use-case: Configure openflow 1.3+ switch as a openflow 1.0 switch using openflow 1.0 instructions (e.g existing AD-SAL application using compatibility layer ) A. Client creates flow OFP-1.3 Flow-1: (to take care of pkts with no vlan header) Flow-2: (to take care of pkts with vlan header) When a set-vlan-id is executed for OF1.3, we need to apply 2 flows, Flow-1 and Flow-2. Now in the code, the stack trace of a flow mod is as follows: ModelDrivenSwitchImpl.addFlow(AddFlowInput input) Its in ActionConvertor.getActions() , we convert a set-vlan-id (OF1.3) to PushVlan+SetField. This would take care of Flow-1 . Converted Flow-1 will get returned to OFRpcTaskFactory.createAddFlowTask() which would then sent to switch. But to generate Flow-2, I could see only the following options. 1. Somehow let OFRpcTaskFactory.createAddFlowTask() know that it has to generate a flow2. One way is to embed a flag in the returned converted flow, but that would be polluting it. 2. In FlowConverter , check to see if the input flow has a SetVlanIdActionCase. If yes, add a flag to Flow input (by augmenting the SetVlanIdActionCase in OFP) when initiating the 2nd flow. Check for the flag in ActionConverter to convert to pushvlan/setfield with the masks. With the 2nd option, the FlowConverter will return 2 flows (Flow-1 and Flow-2) to the OFRpcTaskFactory, which will then perform flowMods for both (with different xids). But it would/could return RpcResult to the caller only for one of the two. If you can think of better/simpler design , let me know. |
| Comment by Kamal Rameshan [ 26/Aug/14 ] |
|
Gerrit review submitted: https://git.opendaylight.org/gerrit/#/c/10280/ |
| Comment by Michal Rehak [ 26/Aug/14 ] |
|
Sorry to react so late - after merge. I noticed 2 things:
|
| Comment by Kamal Rameshan [ 26/Aug/14 ] |
|
Hi Michal, I guess i missed updateFlow, although i had it in my radar. I will submit a patch for that asap. I do have to improve the tests as well. I will also look into future result chaining. Thanks |
| Comment by Kamal Rameshan [ 27/Aug/14 ] |
|
https://git.opendaylight.org/gerrit/#/c/10348/ submitted for review |
| Comment by Hideyuki Tai [ 29/Aug/14 ] |
|
It seems that the patch has a problem. At FlowConvertor.java, you can see how the OpenFlow plugin handles SET_VLAN_ID action. public static List<FlowModInputBuilder> toFlowModInputs(Flow srcFlow, short version, BigInteger datapathId) { if (version >= OFConstants.OFP_VERSION_1_3 && isSetVlanIdActionCasePresent(srcFlow)) { list.addAll(handleSetVlanIdForOF13(srcFlow, version, datapathId)); }else { list.add(toFlowModInput(srcFlow, version, datapathId)); } return list; handleSetVlanIdForOF13() method ignores original match condition and change the match condition incorrectly. For example, when VTN Manager tries to install a flow entry which matches untagged frame, the OF plugin incorrectly deletes in_port match field, source/destination MAC Address match field, and change VLAN match field. [Flow entry which VTN Mamanager tries to install]
[Flow entry which the OF Plugin set to a switch]
My colleague said that a similar issue occurs when VTN Manager tries to install a flow entry which has set-vlan-id action to frames with a VLAN tag. |
| Comment by Hideyuki Tai [ 03/Sep/14 ] |
|
We've found another issue. When a controller with the OF plugin connected to a OpenFlow 1.0 switch and I tried to install a flow entry with push-vlan action into the switch, NullPointerException occured. I used the latest code on September 2nd, 2014. You can reproduce this issue using RESTCONF API. URL: http://192.168.2.56:8080/restconf/config/opendaylight-inventory:nodes/node/openflow:1/table/0/flow/9 Method: PUT Body: The output what I saw: 2014-09-02 22:13:08.662 EDT [nioEventLoopGroup-11-1] WARN o.o.o.protocol.impl.core.OFEncoder - Message serialization failed When I applied the following patch, the above NPE didn't occur. diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ActionConvertor.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/ActionConvertor.java PushVlanActionCase pushVlanActionCase = (PushVlanActionCase) action; |
| Comment by Hideyuki Tai [ 05/Sep/14 ] |
|
(In reply to Hideyuki Tai from comment #8) I've filed the issue I mentioned comment #8 as |
| Comment by Hideyuki Tai [ 05/Sep/14 ] |
|
(In reply to Hideyuki Tai from comment #7) I've filed the issue which I mentioned in comment #7 as |
| Comment by Michal Rehak [ 08/Sep/14 ] |
|
https://git.opendaylight.org/gerrit/#/c/10916/ |
| Comment by Hideyuki Tai [ 09/Sep/14 ] |
|
I've tested the patch set 1 of Gerrit 10916, and I've found out that it's getting better, but the patch doesn't fix the entire problem. Firstly, I tried to install a flow entry with a vlan match (vlan-id=10) and actions (set-vlan-id and output) for an OpenFlow 1.3 switch via RESTCONF of a OpenDaylight controller. Method: PUT URL: http://192.168.2.56:8080/restconf/config/opendaylight-inventory:nodes/node/openflow:1/table/0/flow/2 Body: <?xml version="1.0" encoding="UTF-8" standalone="no"?> The flow entry installed in the switch was as follows: cookie=0x0, duration=5.588s, table=0, n_packets=0, n_bytes=0, idle_timeout=300, hard_timeout=600, send_flow_rem priority=2,in_port=10,dl_vlan=10,dl_src=3a:72:f1:02:1a:d0,dl_dst=16:58:21:81:bc:43 actions=set_field:4151->vlan_vid,output:2 That did not contain a match field for VLAN PCP. Next, I tried to install a flow entry with a vlan match (for untagged frame) and actions (push-vlan, set-vlan-id, and output) for an OpenFlow 1.3 switch via RESTCONF of a OpenDaylight controller. <?xml version="1.0" encoding="UTF-8" standalone="no"?> I checked the flow entry table of the switch, but no flow entry was newly added. |
| Comment by Michal Rehak [ 09/Sep/14 ] |
|
I pushed PS2 which fixed the missing vlan-pcp match part. I also inspected the flowMod message in wireshark - looks ok. OVS reports unfortunately "clean" value of vlan-id in match and flagged value (cfi) in action/set-field. But good news is that in statistics - both values are reported as flagged form and we translate those to clean values. So in statistics you can see correct match and action. The second flow works only on cpqd. Ovs (2.1.3 an 2.3.0) responded with error message containing "unsupported tag/encap". I guess this might be matter of input validation. No idea which virtual switch is doing it better. Could you please retest current PS2 at least for vlan-pcp? Regarding the second flow it is a topic for discussion probably. Anyway - can be tested on cpqd switch. Michal |
| Comment by Hideyuki Tai [ 09/Sep/14 ] |
|
(In reply to michal rehak from comment #13) Hi Michal, Thank you for updating the patch. |
| Comment by Hideyuki Tai [ 09/Sep/14 ] |
|
(In reply to michal rehak from comment #13) (snip) > Could you please retest current PS2 at least for vlan-pcp? Regarding the Hi Michal, I retested the patch set 2 for vlan-pcp. I sent the following PUT request (which is the same request I mentioned in comment 12) to a OpenDaylight controller and checked the OFPT_FLOW_MOD. Method: PUT URL: http://192.168.2.56:8080/restconf/config/opendaylight-inventory:nodes/node/openflow:1/table/0/flow/2 Body: <?xml version="1.0" encoding="UTF-8" standalone="no"?> |
| Comment by Hideyuki Tai [ 09/Sep/14 ] |
|
(In reply to michal rehak from comment #13) Hi Michal, I retested the send flow request with Wireshark. The OFPT_FLOW_MOD message sent by the OpenDaylight controller was wrong.
It meant that the OFPT_FLOW_MOD message tried to install a flow entry which pushed a VLAN tag to frames which had already VLAN tag. It seems to me that FlowConvertor.java does not take "vlan match is OFPVID_NONE" case into consideration. Method: PUT Body: |
| Comment by Hideyuki Tai [ 10/Sep/14 ] |
|
(In reply to Hideyuki Tai from comment #16) (snip) > Hi Michal, Hi Michal, I retested the patch set 3 of Gerrit 10916. I've confirmed that I can successfully install a flow entry with a VLAN match for untagged frames ans actions (push-vlan, set-vlan-id, and output) for OpenFlow 1.3 which is the test case I mentioned in comment #13. I'm going to test the patch set with VTN Manager. |
| Comment by Michal Rehak [ 10/Sep/14 ] |
| Comment by Sam Hague [ 10/Sep/14 ] |
|
I tested ps3 and it seems to work, but I have add the OFPVID_PRESENT value and set it to false. Is that the expected behaviour? My test case is to add a tag for packets that are currently untagged. Here is the JAVA code I am using to set the value. Notice I also had to set a vid of 0 for it to take. public static MatchBuilder createVlanIdPresentMatch(MatchBuilder matchBuilder, boolean present) { VlanMatchBuilder vlanMatchBuilder = new VlanMatchBuilder(); VlanIdBuilder vlanIdBuilder = new VlanIdBuilder(); vlanIdBuilder.setVlanIdPresent(present); vlanIdBuilder.setVlanId(new VlanId(0)); vlanMatchBuilder.setVlanId(vlanIdBuilder.build()); matchBuilder.setVlanMatch(vlanMatchBuilder.build()); return matchBuilder; } |
| Comment by Hideyuki Tai [ 10/Sep/14 ] |
|
(In reply to Hideyuki Tai from comment #17) (snip) > I've tested the patch set 3 with VTN Manager, and confirmed that the patch set works fine. When I deleted flow entries, I saw the following ERROR message. 2014-09-10 13:24:39.874 EDT [CommitFutures-5] ERROR o.o.c.m.s.b.i.ForwardedBackwardsCompatibleDataBroker - Transaction DOM-207 failed to complete I put all messages I saw when I deleted a flow entry at gist: |