From 092e2daa707655119a695a9c30d6f50824b79570 Mon Sep 17 00:00:00 2001 From: maquresh Date: Fri, 18 Jul 2014 16:17:46 -0700 Subject: [PATCH] Flow request containing an invalid action should be rejected When ODL Controller receives a request to install a flow, it is processed by FlowConfig class in forwardingrulesmanager bundle. There is method named "validate" which validates the matches and actions specified in the flow request. If this method encounters an invalid action, it simply ignores it and moves to the next action. The result of this behavior is that if flow request contains only one action and it happens to be invalid, the FlowMod request will be sent to the switch without any action. An OVS conforming switch will install this flow and set the action to drop. However, ideally Controller should reject this request (or any request containing an invalid action). Few invalid unit test cases were also escaping due to this error, which have been corrected. Change-Id: I4df3226d6ec9c663476cae1991c194bc36cb2f99 Signed-off-by: maquresh --- .../forwardingrulesmanager/FlowConfig.java | 53 ++++++++++++++++++- .../forwardingrulesmanager/frmTest.java | 6 +-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java index c89cfc1b9e..e2b6642501 100644 --- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java +++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java @@ -867,6 +867,7 @@ public class FlowConfig extends ConfigurationObject implements Serializable { } continue; } + sstr = Pattern.compile(ActionType.SET_NEXT_HOP.toString() + "=(.*)").matcher(actiongrp); if (sstr.matches()) { if (!NetUtils.isIPAddressValid(sstr.group(1))) { @@ -875,7 +876,57 @@ public class FlowConfig extends ConfigurationObject implements Serializable { } continue; } - } + + sstr = Pattern.compile(ActionType.OUTPUT + "=(.*)").matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.DROP.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.LOOPBACK.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.FLOOD.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.FLOOD_ALL.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.SW_PATH.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.HW_PATH.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.CONTROLLER.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + sstr = Pattern.compile(ActionType.POP_VLAN.toString()).matcher(actiongrp); + if (sstr.matches()) { + continue; + } + + // If execution reached here, it means there is an Action + // which is not recognized by the controller. Return error + + return new Status(StatusCode.BADREQUEST, String.format("%s is an UnSupported Action", actiongrp)); + } } catch (NumberFormatException e) { return new Status(StatusCode.BADREQUEST, String.format("Invalid number format %s", e.getMessage())); } diff --git a/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java b/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java index 685ccdb7c4..3c367b9af4 100644 --- a/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java +++ b/opendaylight/forwardingrulesmanager/api/src/test/java/org/opendaylight/controller/forwardingrulesmanager/frmTest.java @@ -760,9 +760,9 @@ public class frmTest { actions.add(ActionType.SET_NW_SRC.toString() + "=1.1.1.1"); actions.add(ActionType.SET_NW_DST.toString() + "=2.2.2.2"); actions.add(ActionType.CONTROLLER.toString()); - actions.add(ActionType.SET_NW_TOS.toString() + "1"); - actions.add(ActionType.SET_TP_SRC.toString() + "60"); - actions.add(ActionType.SET_TP_DST.toString() + "8080"); + actions.add(ActionType.SET_NW_TOS.toString() + "=1"); + actions.add(ActionType.SET_TP_SRC.toString() + "=60"); + actions.add(ActionType.SET_TP_DST.toString() + "=8080"); actions.add(ActionType.SET_NEXT_HOP.toString() + "=1.1.1.1"); return actions; -- 2.36.6