z.archive.openflowjava-dev@lists.opendaylight.org | [controller-dev] Flow actions were reordered.

Home Messages Hashtags Subgroups

This group is locked. No changes can be made to the group while it is locked.

Date Date 1 - 11 of 11

[controller-dev] Flow actions were reordered.

Ed Warnicke (eaw) <eaw@...> #345 Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: toggle quoted message Show quoted text Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Tai, Hideyuki <hideyuki.tai@...> #349 Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai toggle quoted message Show quoted text From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Ed Warnicke (eaw) <eaw@...> #350 Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: toggle quoted message Show quoted text Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Tai, Hideyuki <hideyuki.tai@...> #351 Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai toggle quoted message Show quoted text From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Colin Dixon #358 It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin toggle quoted message Show quoted text On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Ed Warnicke (eaw) <eaw@...> #359 The natural ordering would be for the openflowplugin to respect the ordering field. Here’s the model currently: grouping action-list { list action { key "order"; leaf order { type int32; } uses action; } } It has an explicit leaf, order, that indicates the order of the actions. The openflowplugin isn’t applying them in that order. Ed On Jul 21, 2014, at 10:23 PM, Colin Dixon <colin@...> wrote: toggle quoted message Show quoted text It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Ed Warnicke (eaw) <eaw@...> #360 Here’s a patch that shows what I mean: https://git.opendaylight.org/gerrit/9214 Ed P.S. The only thing wrong with the patch (the only reason its marked DO NOT MERGE) is because I haven’t had a chance to test it yet. On Jul 21, 2014, at 10:29 PM, Ed Warnicke (eaw) <eaw@...> wrote: toggle quoted message Show quoted text The natural ordering would be for the openflowplugin to respect the ordering field. Here’s the model currently: grouping action-list { list action { key "order"; leaf order { type int32; } uses action; } } It has an explicit leaf, order, that indicates the order of the actions. The openflowplugin isn’t applying them in that order. Ed On Jul 21, 2014, at 10:23 PM, Colin Dixon <colin@...> wrote: It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Tai, Hideyuki <hideyuki.tai@...> #364 Hi all, As Colin pointed out, I think there is no good reason not to take the following patch. https://git.opendaylight.org/gerrit/#/c/8842/ And, Shigeru has already verified that Bug 1338 is fixed by the patch. To remove the Bug 1338, could a committer of controller project review and approve the patch? https://git.opendaylight.org/gerrit/#/c/8842/ Regards, Hideyuki Tai toggle quoted message Show quoted text From: controller-dev-bounces@... [mailto:controller-dev-bounces@...] On Behalf Of Ed Warnicke (eaw) Sent: Monday, July 21, 2014 21:26 To: Colin Dixon Cc: openflowjava-dev@...; controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Here’s a patch that shows what I mean: https://git.opendaylight.org/gerrit/9214 Ed P.S. The only thing wrong with the patch (the only reason its marked DO NOT MERGE) is because I haven’t had a chance to test it yet. On Jul 21, 2014, at 10:29 PM, Ed Warnicke (eaw) <eaw@...> wrote: The natural ordering would be for the openflowplugin to respect the ordering field. Here’s the model currently: grouping action-list { list action { key "order"; leaf order { type int32; } uses action; } } It has an explicit leaf, order, that indicates the order of the actions. The openflowplugin isn’t applying them in that order. Ed On Jul 21, 2014, at 10:23 PM, Colin Dixon <colin@...> wrote: It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Ed Warnicke (eaw) <eaw@...> #365 Hideyuki, The model itself specifies the means for how flows should be ordered ( the order field) The openflowplugin is currently not respecting it. The patch you point to does not resolve that underlying issue. So it will still produce incorrect results. Ed On Jul 23, 2014, at 5:59 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: toggle quoted message Show quoted text Hi all, As Colin pointed out, I think there is no good reason not to take the following patch. https://git.opendaylight.org/gerrit/#/c/8842/ And, Shigeru has already verified that Bug 1338 is fixed by the patch. To remove the Bug 1338, could a committer of controller project review and approve the patch? https://git.opendaylight.org/gerrit/#/c/8842/ Regards, Hideyuki Tai From: controller-dev-bounces@... [mailto:controller-dev-bounces@...] On Behalf Of Ed Warnicke (eaw) Sent: Monday, July 21, 2014 21:26 To: Colin Dixon Cc: openflowjava-dev@...; controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Here’s a patch that shows what I mean: https://git.opendaylight.org/gerrit/9214 Ed P.S. The only thing wrong with the patch (the only reason its marked DO NOT MERGE) is because I haven’t had a chance to test it yet. On Jul 21, 2014, at 10:29 PM, Ed Warnicke (eaw) <eaw@...> wrote: The natural ordering would be for the openflowplugin to respect the ordering field. Here’s the model currently: grouping action-list { list action { key "order"; leaf order { type int32; } uses action; } } It has an explicit leaf, order, that indicates the order of the actions. The openflowplugin isn’t applying them in that order. Ed On Jul 21, 2014, at 10:23 PM, Colin Dixon <colin@...> wrote: It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Tai, Hideyuki <hideyuki.tai@...> #366 Hi Ed, Thank you for explaining it. I understand that. So should the patch 8842 be abandoned? Regards, Hideyuki Tai toggle quoted message Show quoted text From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Wednesday, July 23, 2014 18:39 To: Tai, Hideyuki Cc: Colin Dixon; controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, The model itself specifies the means for how flows should be ordered ( the order field) The openflowplugin is currently not respecting it. The patch you point to does not resolve that underlying issue. So it will still produce incorrect results. Ed On Jul 23, 2014, at 5:59 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi all, As Colin pointed out, I think there is no good reason not to take the following patch. https://git.opendaylight.org/gerrit/#/c/8842/ And, Shigeru has already verified that Bug 1338 is fixed by the patch. To remove the Bug 1338, could a committer of controller project review and approve the patch? https://git.opendaylight.org/gerrit/#/c/8842/ Regards, Hideyuki Tai From: controller-dev-bounces@... [mailto:controller-dev-bounces@...] On Behalf Of Ed Warnicke (eaw) Sent: Monday, July 21, 2014 21:26 To: Colin Dixon Cc: openflowjava-dev@...; controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Here’s a patch that shows what I mean: https://git.opendaylight.org/gerrit/9214 Ed P.S. The only thing wrong with the patch (the only reason its marked DO NOT MERGE) is because I haven’t had a chance to test it yet. On Jul 21, 2014, at 10:29 PM, Ed Warnicke (eaw) <eaw@...> wrote: The natural ordering would be for the openflowplugin to respect the ordering field. Here’s the model currently: grouping action-list { list action { key "order"; leaf order { type int32; } uses action; } } It has an explicit leaf, order, that indicates the order of the actions. The openflowplugin isn’t applying them in that order. Ed On Jul 21, 2014, at 10:23 PM, Colin Dixon <colin@...> wrote: It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev
Ed Warnicke (eaw) <eaw@...> #367 Not until we demonstrate a real fix to you :) Ed On Jul 23, 2014, at 9:42 PM, "Tai, Hideyuki" <hideyuki.tai@...> wrote: toggle quoted message Show quoted text Hi Ed, Thank you for explaining it. I understand that. So should the patch 8842 be abandoned? Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Wednesday, July 23, 2014 18:39 To: Tai, Hideyuki Cc: Colin Dixon; controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, The model itself specifies the means for how flows should be ordered ( the order field) The openflowplugin is currently not respecting it. The patch you point to does not resolve that underlying issue. So it will still produce incorrect results. Ed On Jul 23, 2014, at 5:59 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi all, As Colin pointed out, I think there is no good reason not to take the following patch. https://git.opendaylight.org/gerrit/#/c/8842/ And, Shigeru has already verified that Bug 1338 is fixed by the patch. To remove the Bug 1338, could a committer of controller project review and approve the patch? https://git.opendaylight.org/gerrit/#/c/8842/ Regards, Hideyuki Tai From: controller-dev-bounces@... [mailto:controller-dev-bounces@...] On Behalf Of Ed Warnicke (eaw) Sent: Monday, July 21, 2014 21:26 To: Colin Dixon Cc: openflowjava-dev@...; controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Here’s a patch that shows what I mean: https://git.opendaylight.org/gerrit/9214 Ed P.S. The only thing wrong with the patch (the only reason its marked DO NOT MERGE) is because I haven’t had a chance to test it yet. On Jul 21, 2014, at 10:29 PM, Ed Warnicke (eaw) <eaw@...> wrote: The natural ordering would be for the openflowplugin to respect the ordering field. Here’s the model currently: grouping action-list { list action { key "order"; leaf order { type int32; } uses action; } } It has an explicit leaf, order, that indicates the order of the actions. The openflowplugin isn’t applying them in that order. Ed On Jul 21, 2014, at 10:23 PM, Colin Dixon <colin@...> wrote: It seems like we should use the tools of our modeling language when it helps us do things in a less cumbersome way. Is there any good reason not to take the patch? My gut reaction is that the broader fix would be to remove the code needed to handle the order field and instead use the natural ordering of the YANG rather than to bend over backwards to do something which we get for free. --Colin On Mon, Jul 21, 2014 at 2:14 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, It seems that when DataBrokerServer passes MD-SAL data to DataProviderService, they do serialization/deserialization. The issue that reordering of actions occurs during the serialization/deserialization. Therefore, I think openflowplugin does not cause this issue. And, adding 'ordered-by user' statement to MD-SAL action list solves the issue. Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 11:59 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Yep… which means the bug is actually (I suspect) in the openflowplugin not respecting the order field ;) Ed On Jul 21, 2014, at 1:42 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi Ed, The 'order' field has been already used. However, as Shigeru mentioned in the previous mail, when MD-SAL actions are deserialized, the 'order' field is ignored and actions are reordered unexpectedly, because 'ordered-by user' is not there. https://lists.opendaylight.org/pipermail/controller-dev/2014-July/005487.html To sort MD-SAL actions according to the 'order' field, we need 'ordered-by user'. http://tools.ietf.org/html/rfc6020#section-7.7.5.2 Regards, Hideyuki Tai From: Ed Warnicke (eaw) [mailto:eaw@...] Sent: Monday, July 21, 2014 10:06 To: Tai, Hideyuki Cc: controller-dev@...; openflowplugin-dev@...; openflowjava-dev@...; vtn-dev@... Subject: Re: [controller-dev] Flow actions were reordered. Hideyuki, Wouldn’t it make more sense to simply set the ‘order’ field to indicate the order? (there is if memory serves a numerical order field) Ed On Jul 18, 2014, at 4:06 PM, Tai, Hideyuki <hideyuki.tai@...> wrote: Hi committers of Controller project, Could you review and approve the following patch? https://git.opendaylight.org/gerrit/8842 This is a patch for Bug 1338. Due to this bug, flow actions in a flow entry is unexpectedly reordered when using the MD-SAL OF plugin (OF13 plugin), so that an OF switch does not correctly handle packets as intended. Regards, Hideyuki Tai -----Original Message----- From: openflowjava-dev-bounces@... [mailto:openflowjava-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Wednesday, July 09, 2014 00:46 To: colin@... Cc: openflowjava-dev@...; controller-dev@...; kkoushik@...; openflowplugin-dev@...; vtn-dev@... Subject: Re: [openflowjava-dev] [openflowplugin-dev] [controller-dev] Flow actions were reordered. Thanks Colin, But action list I mentioned is in MD-SAL flow, not OF flow. So I pushed a patch that adds "ordered-by user" to MD-SAL action list. https://git.opendaylight.org/gerrit/8842 I verified that above patch preserves the order of actions in MD-SAL flow. I think Colin's patch should also be merged because the order of OF action list should be preserved. Regards, -- Shigeru Yasuda On [Tue, 8 Jul 2014 17:58:24 -0500], Colin Dixon <colin@...> wrote: My guess is that this is a bug in openflowjava's (adding openflowjava-dev) openflow-action.yang where they should specify that the list of actions in actions-grouping is ordered-by-user. Right now it is not: grouping actions-grouping { list action { config false; leaf type { type identityref { base oft:action-base; } } } } It should be a simple fix. I've opened a bug here: https://bugs.opendaylight.org/show_bug.cgi?id=1338 I've proposed a simple fix here: https://git.opendaylight.org/gerrit/8830 You can try pulling that and building to see if it fixes your problem. Cheers, --Colin On Tue, Jul 8, 2014 at 5:16 PM, Kiran Agrahara Sreenivasa < kkoushik@...> wrote: If you have the YANG data model, have you tried specifying "ordered-by-user" in the list? Thanks Kiran -----Original Message----- From: controller-dev-bounces@... [mailto: controller-dev-bounces@...] On Behalf Of Shigeru Yasuda Sent: Tuesday, July 08, 2014 11:18 AM To: controller-dev@...; openflowplugin-dev@...; vtn-dev@... Subject: [controller-dev] Flow actions were reordered. Hi folks, I observed that flow actions in a flow entry were reordered unexpectedly when I tested VTN with OF13 plugin. I configured 3 actions in a flow entry (via sal-compatibility): action[0]: PUSH_VLAN action[1]: SET_FIELD (VLAN_VID) action[2]: OUTPUT But above actions were encoded into an OF13 FLOW_MOD as follows: action[0]: OUTPUT action[1]: PUSH_VLAN action[2]: SET_FIELD (VLAN_VID) I embedded some trace logs into the controller source, then I observed that flow actions in MD-SAL flow were already reordered when FlowChangeListener.add() was called. _applyActions=ApplyActions [ _action=[ Action [ _order=2, _key=ActionKey [_order=2], _action=OutputActionCase [ _outputAction=OutputAction [ _outputNodeConnector=Uri [_value=openflow:4:1], augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=0, _key=ActionKey [_order=0], _action=PushVlanActionCase [ _pushVlanAction=PushVlanAction [ _ethernetType=33024, augmentation=[] ], augmentation=[] ], augmentation=[] ], Action [ _order=1, _key=ActionKey [_order=1], _action=SetVlanIdActionCase [ _setVlanIdAction=SetVlanIdAction [ _vlanId=VlanId [_value=10], augmentation=[] ], augmentation=[] ], augmentation=[] ] ], augmentation=[] ], ... And I enabled trace logging for BindingToNormalizedNodeCodec class in sal-binding-broker, then I observed that MD-SAL actions were deserialized out of order. 2014-07-09 00:37:40.849 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)output- action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)set-vla n-id-action 2014-07-09 00:37:40.851 GMT+09:00 [pool-17-thread-1] TRACE \ o.o.c.m.s.b.i.BindingToNormalizedNodeCodec - \ InstanceIdentifier Path Deserialization: [... snipped ..] \ action/(urn:opendaylight:flow:inventory?revision=2013-08-19)push-vl an-action The order of elements in a YANG data list seems to be unspecified because it may be randomized by deserialization. My understanding is correct? If so, I think openflowplugin should sort MD-SAL actions according to action order (Action.getOrder()) when it converts MD-SAL actions into OF actions. Regards, -- Shigeru Yasuda _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ openflowplugin-dev mailing list openflowplugin-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev _______________________________________________ openflowjava-dev mailing list openflowjava-dev@... https://lists.opendaylight.org/mailman/listinfo/openflowjava-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev _______________________________________________ controller-dev mailing list controller-dev@... https://lists.opendaylight.org/mailman/listinfo/controller-dev

1 - 11 of 11

1

Previous Topic Next Topic

Home Hashtags Subgroups Terms