Operating System: All
Platform: All

(In reply to Seungho Chung from comment #0)
> This is related to OVSDB-150
>
>
> I added debug code like below for debugging(line 285),
>
> 276 if (isPortDeleted) {
> 277 // Iterate through all actions in the modified list and
> adjust the order of
> 278 // the actions left behind. With that, all actions that have
> order higher than
> 279 // the action removed gets their value decremented by 1.
> Note that this iteration
> 280 // visits all entries to account for cases where the list
> order is not the same
> 281 // as the action's order.
> 282 //
> 283 for (int i = 0; i < actionList.size(); i++) {
> 284 Action action = actionList.get;
> 285 logger.debug("################# i = {}, action = {},
> order = {}, key = {}, rmo = {}", i, action.getAction(), action.getOrder(),
> action.getKey(), removedActionOrder );
>
>
> and debug log likes below
>
> 2015-05-22 19:01:48,318 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | removeOutputPortFromInstructions() Node Connector ID is - Type=openflow:
> DPID=77207755810117 port=6
> existingInstructions=[Instruction{getInstruction=ApplyActionsCase{getApplyAct
> ions=ApplyActions{getAction=[Action{getAction=OutputActionCase{getOutputActio
> n=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:9], augmentations={}}, augmentations={}},
> getOrder=6, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:8], augmentations={}},
> augmentations={}}, getOrder=5, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:7], augmentations={}},
> augmentations={}}, getOrder=4, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:6], augmentations={}},
> augmentations={}}, getOrder=3, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:5], augmentations={}},
> augmentations={}}, getOrder=2, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:4], augmentations={}},
> augmentations={}}, getOrder=1, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:3], augmentations={}},
> augmentations={}}, getOrder=0, augmentations={}}], augmentations={}},
> augmentations={}}, getOrder=0, augmentations={}}]
> 2015-05-22 19:01:48,319 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 0, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:9], augmentations={}}, augmentations={}},
> order = 6, key = ActionKey [_order=6], rmo = 3
> 2015-05-22 19:01:48,320 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 1, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:8], augmentations={}}, augmentations={}},
> order = 5, key = ActionKey [_order=5], rmo = 3
> 2015-05-22 19:01:48,320 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 2, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:7], augmentations={}}, augmentations={}},
> order = 4, key = ActionKey [_order=4], rmo = 3
> 2015-05-22 19:01:48,321 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 3, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:5], augmentations={}}, augmentations={}},
> order = 2, key = ActionKey [_order=2], rmo = 3
> 2015-05-22 19:01:48,321 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 4, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:4], augmentations={}}, augmentations={}},
> order = 1, key = ActionKey [_order=1], rmo = 3
> 2015-05-22 19:01:48,322 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 5, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:3], augmentations={}}, augmentations={}},
> order = 0, key = ActionKey [_order=0], rmo = 3
> 2015-05-22 19:01:48,322 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | removeOutputPortFromInstructions() : applyAction ApplyActions
> [_action=[Action
> [_action=OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector
> =Uri [_value=openflow:77207755810117:9], augmentations={}},
> augmentations={}}, _key=ActionKey [_order=0], _order=0, augmentation=[]],
> Action
> [_action=OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector
> =Uri [_value=openflow:77207755810117:8], augmentations={}},
> augmentations={}}, _key=ActionKey [_order=1], _order=1, augmentation=[]],
> Action
> [_action=OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector
> =Uri [_value=openflow:77207755810117:7], augmentations={}},
> augmentations={}}, _key=ActionKey [_order=2], _order=2, augmentation=[]],
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:5], augmentations={}},
> augmentations={}}, getOrder=2, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:4], augmentations={}},
> augmentations={}}, getOrder=1, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:3], augmentations={}},
> augmentations={}}, getOrder=0, augmentations={}}], augmentation=[]]
> 2015-05-22 19:01:48,826 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | removeOutputPortFromInstructions() Node Connector ID is - Type=openflow:
> DPID=77207755810117 port=6
> existingInstructions=[Instruction{getInstruction=ApplyActionsCase{getApplyAct
> ions=ApplyActions{getAction=[Action{getAction=OutputActionCase{getOutputActio
> n=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:8], augmentations={}}, augmentations={}},
> getOrder=7, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:9], augmentations={}},
> augmentations={}}, getOrder=8, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:7], augmentations={}},
> augmentations={}}, getOrder=6, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:6], augmentations={}},
> augmentations={}}, getOrder=5, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:5], augmentations={}},
> augmentations={}}, getOrder=4, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:4], augmentations={}},
> augmentations={}}, getOrder=3, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:2], augmentations={}},
> augmentations={}}, getOrder=2, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:1], augmentations={}},
> augmentations={}}, getOrder=1, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:3], augmentations={}},
> augmentations={}}, getOrder=0, augmentations={}}], augmentations={}},
> augmentations={}}, getOrder=0, augmentations={}}]
> 2015-05-22 19:01:48,826 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 0, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:8], augmentations={}}, augmentations={}},
> order = 7, key = ActionKey [_order=7], rmo = 5
> 2015-05-22 19:01:48,827 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 1, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:9], augmentations={}}, augmentations={}},
> order = 8, key = ActionKey [_order=8], rmo = 5
> 2015-05-22 19:01:48,827 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 2, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:7], augmentations={}}, augmentations={}},
> order = 6, key = ActionKey [_order=6], rmo = 5
> 2015-05-22 19:01:48,828 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 3, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:5], augmentations={}}, augmentations={}},
> order = 4, key = ActionKey [_order=4], rmo = 5
> 2015-05-22 19:01:48,828 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 4, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:4], augmentations={}}, augmentations={}},
> order = 3, key = ActionKey [_order=3], rmo = 5
> 2015-05-22 19:01:48,828 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | ################# i = 5, action =
> OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector=Uri
> [_value=openflow:77207755810117:2], augmentations={}}, augmentations={}},
> order = 2, key = ActionKey [_order=2], rmo = 5
> 2015-05-22 19:01:48,829 | DEBUG | ool-176-thread-1 | InstructionUtils
> | 364 - org.opendaylight.ovsdb.openstack.net-virt-providers - 1.0.4.SNAPSHOT
> | removeOutputPortFromInstructions() : applyAction ApplyActions
> [_action=[Action
> [_action=OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector
> =Uri [_value=openflow:77207755810117:8], augmentations={}},
> augmentations={}}, _key=ActionKey [_order=0], _order=0, augmentation=[]],
> Action
> [_action=OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector
> =Uri [_value=openflow:77207755810117:9], augmentations={}},
> augmentations={}}, _key=ActionKey [_order=1], _order=1, augmentation=[]],
> Action
> [_action=OutputActionCase{getOutputAction=OutputAction{getOutputNodeConnector
> =Uri [_value=openflow:77207755810117:7], augmentations={}},
> augmentations={}}, _key=ActionKey [_order=2], _order=2, augmentation=[]],
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:5], augmentations={}},
> augmentations={}}, getOrder=4, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:4], augmentations={}},
> augmentations={}}, getOrder=3, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:2], augmentations={}},
> augmentations={}}, getOrder=2, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:1], augmentations={}},
> augmentations={}}, getOrder=1, augmentations={}},
> Action{getAction=OutputActionCase{getOutputAction=OutputAction{getOutputNodeC
> onnector=Uri [_value=openflow:77207755810117:3], augmentations={}},
> augmentations={}}, getOrder=0, augmentations={}}], augmentation=[]]
>
>
> ab.setKey(new ActionKey); // this should be changed like
> below
> ab.setKey(new ActionKey(action.getOrder() -1));
>
>
> if not orders will be all messed up.

Hi Seungho,

Can you please submit a gerrit with you r proposed fix? I'm not sure I understand what the bug is, as well as how to reproduce it.

Thanks.

I already submit to gerrit.

https://git.opendaylight.org/gerrit/#/c/21608/

only one line is changed.

line 290 is changed, it used i(variable) for setKey, my proposal is use "action.getOrder() -1" for setKey.

you can find order is over wrapped at log, it caused setKey used i(variable).

1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. code Block ##############################
                                                276 if (isPortDeleted) {
                                                277 // Iterate through all actions in the modified list and adjust the order of
                                                278 // the actions left behind. With that, all actions that have order higher than
                                                279 // the action removed gets their value decremented by 1. Note that this iteration
                                                280 // visits all entries to account for cases where the list order is not the same
                                                281 // as the action's order.
                                                282 //
                                                283 for (int i = 0; i < actionList.size(); i++) {
                                                284 Action action = actionList.get;
                                                285 if (action.getOrder() > removedActionOrder) { 286 /* Shift the action by rebuilding action, using adjusted order */ 287 ab = new ActionBuilder(); 288 ab.setAction(action.getAction()); 289 ab.setOrder(action.getOrder() - 1); 290 ab.setKey(new ActionKey(action.getOrder() -1)); 291 Action actionNewOrder = ab.build(); 292 actionList.remove(action); 293 actionList.add(i, actionNewOrder); 294 }

                                                else if (action.getOrder() == removedActionOrder) {
                                                295 // Sanity: implementation assumes no two actions have the same order
                                                296 //
                                                297 logger.error("Found action with same order as the action removed for {}, order {} index {}: {}",
                                                298 ncid, removedActionOrder, i, action);
                                                299 }
                                                300
                                                301 // If action refers to a port output, then flow should be preserved.
                                                302 // We do this, so that if we are only left with non-output port actions,
                                                303 // we still remove the flow
                                                304 //
                                                305 if (action.getAction() instanceof OutputActionCase)

                                                { 306 removeFlow = false; 307 }

                                                308 }
                                                ############################################################

(In reply to Seungho Chung from comment #2)
> I already submit to gerrit.
>
> https://git.opendaylight.org/gerrit/#/c/21608/
>
> only one line is changed.
>
>
> line 290 is changed, it used i(variable) for setKey, my proposal is use
> "action.getOrder() -1" for setKey.
>
> you can find order is over wrapped at log, it caused setKey used i(variable).
>
>
>
> ################ code Block ##############################
> 276 if (isPortDeleted) {
> 277 // Iterate through all actions in the modified list and
> adjust the order of
> 278 // the actions left behind. With that, all actions that
> have order higher than
> 279 // the action removed gets their value decremented by 1.
> Note that this iteration
> 280 // visits all entries to account for cases where the list
> order is not the same
> 281 // as the action's order.
> 282 //
> 283 for (int i = 0; i < actionList.size(); i++) {
> 284 Action action = actionList.get;
> 285 if (action.getOrder() > removedActionOrder)

else if (action.getOrder() == removedActionOrder) {
> 295 // Sanity: implementation assumes no two actions
> have the same order
> 296 //
> 297 logger.error("Found action with same order as the
> action removed for {}, order {} index {}: {}",
> 298 ncid, removedActionOrder, i, action);
> 299 }
> 300
> 301 // If action refers to a port output, then flow should
> be preserved.
> 302 // We do this, so that if we are only left with
> non-output port actions,
> 303 // we still remove the flow
> 304 //
> 305 if (action.getAction() instanceof OutputActionCase)

> 308 }
> ############################################################

hmm... so order and key must have same value? I will try to find out why we
need two attributes if they are expected to be same value. Meanwhile, please add
missing space to the gerrit.

Thanks,

– flavio

Gerrits:

master: https://git.opendaylight.org/gerrit/#/c/21688/
lithium: https://git.opendaylight.org/gerrit/#/c/21687/
helium: https://git.opendaylight.org/gerrit/#/c/21608/

Thanks Seungho!