From: Alessandro Boch Date: Fri, 20 Sep 2013 17:10:40 +0000 (+0000) Subject: Merge "Fix: IPProtocols not parsing correctly" X-Git-Tag: releasepom-0.1.0~47 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=65ab4ccadb216b12d773ec4113ae241be30d3eb7;hp=b8657e0aa3bbb8b30ef316d849c73adf5d841e64 Merge "Fix: IPProtocols not parsing correctly" --- diff --git a/opendaylight/containermanager/api/src/main/java/org/opendaylight/controller/containermanager/ContainerFlowConfig.java b/opendaylight/containermanager/api/src/main/java/org/opendaylight/controller/containermanager/ContainerFlowConfig.java index 488f8928de..2520172d89 100644 --- a/opendaylight/containermanager/api/src/main/java/org/opendaylight/controller/containermanager/ContainerFlowConfig.java +++ b/opendaylight/containermanager/api/src/main/java/org/opendaylight/controller/containermanager/ContainerFlowConfig.java @@ -479,12 +479,12 @@ public class ContainerFlowConfig implements Serializable { } /** - * Returns the protocol + * Get the IP protocol value * * @return the protocol */ public Short getProtoNum() { - return protocol == null ? IPProtocols.ANY.shortValue() : IPProtocols.getProtocolNumberShort(protocol); + return protocol == null ? null : IPProtocols.getProtocolNumberShort(protocol); } /** @@ -578,16 +578,15 @@ public class ContainerFlowConfig implements Serializable { /** * Validate the protocol field. Either it can be a enum defined in IPProtocols.java - * or a value between 1 and 255 + * or a valid IP proto value between 0 and 255, see: + * http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml + * for more details. * * @return true if a valid protocol value */ private boolean hasValidProtocol() { - if (protocol != null && !protocol.isEmpty()) { - short proto = this.getProtoNum(); - return (((proto != 0) && (proto > 0) && (proto < 256)) || protocol.equalsIgnoreCase("any")); - } - return true; + IPProtocols p = IPProtocols.fromString(protocol); + return p != null; } /** @@ -664,9 +663,8 @@ public class ContainerFlowConfig implements Serializable { mask = NetUtils.getInetNetworkMask(maskLen, ip instanceof Inet6Address); match.setField(MatchType.NW_DST, ip, mask); } - if (this.protocol != null && !this.protocol.trim().isEmpty() && !this.protocol.equalsIgnoreCase("any")) { - match.setField(MatchType.NW_PROTO, IPProtocols - .getProtocolNumberByte(this.protocol)); + if (IPProtocols.fromString(this.protocol) != IPProtocols.ANY) { + match.setField(MatchType.NW_PROTO, IPProtocols.getProtocolNumberByte(this.protocol)); } if (this.tpSrc != null && !this.tpSrc.trim().isEmpty()) { match.setField(MatchType.TP_SRC, Integer.valueOf(tpSrc).shortValue()); 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 62d6855e10..e0b8e9a786 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 @@ -656,11 +656,8 @@ public class FlowConfig implements Serializable { } public boolean isProtocolValid(String protocol) { - int protocol_number = IPProtocols.getProtocolNumberInt(protocol); - if (protocol_number < 1 || protocol_number > 255) { - return false; - } - return true; + IPProtocols proto = IPProtocols.fromString(protocol); + return (proto != null); } private Status conflictWithContainerFlow(IContainer container) { @@ -1033,7 +1030,7 @@ public class FlowConfig implements Serializable { mask = NetUtils.getInetNetworkMask(maskLen, ip instanceof Inet6Address); match.setField(MatchType.NW_DST, ip, mask); } - if (this.protocol != null) { + if (IPProtocols.fromString(this.protocol) != IPProtocols.ANY) { match.setField(MatchType.NW_PROTO, IPProtocols.getProtocolNumberByte(this.protocol)); } if (this.tosBits != null) { diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowConverter.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowConverter.java index 2b8f6553d6..daf1aa8b71 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowConverter.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowConverter.java @@ -557,6 +557,7 @@ public class FlowConverter { byte tos = (byte) (dscp >> 2); salMatch.setField(MatchType.NW_TOS, tos); } + //TODO: NW protocol 0 is a valid protocol if (ofMatch.getNetworkProtocol() != 0) { salMatch.setField(MatchType.NW_PROTO, ofMatch.getNetworkProtocol()); diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/IPProtocols.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/IPProtocols.java index 66e6e65706..d3d3142cdf 100644 --- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/IPProtocols.java +++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/IPProtocols.java @@ -13,24 +13,25 @@ import java.util.ArrayList; import java.util.List; /** - * It represents the most common IP protocols numbers - * It provides the binding between IP protocol names and numbers - * and provides APIs to read and parse them in either of the two forms - * + * Enum represents the most common IP protocols numbers It provides the binding + * between IP protocol names and numbers and provides APIs to read and parse + * them in either of the two forms * + * NOTE: Openflow 1.0 supports the IP Proto match only for ICMP, TCP and UDP * + * references: + * http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml */ -// Openflow 1.0 supports the IP Proto match only for ICMP, TCP and UDP public enum IPProtocols { - ANY("any", 0), - /* HOPOPT("HOPOPT",0), - */ICMP("ICMP", 1), - /* IGMP("IGMP",2), + ANY("any", -1), + HOPOPT("HOPOPT",0), + ICMP("ICMP", 1), + IGMP("IGMP",2), GGP("GGP",3), IPV4("IPv4",4), ST("ST",5), - */TCP("TCP", 6), - /* CBT("CBT",7), + TCP("TCP", 6), + CBT("CBT",7), EGP("EGP",8), IGP("IGP",9), BBNRCCMON("BBN-RCC-MON",10), @@ -40,8 +41,8 @@ public enum IPProtocols { EMCON("EMCON",14), XNET("XNET",15), CHAOS("CHAOS",16), - */UDP("UDP", 17), - /* MUX("MUX",18), + UDP("UDP", 17), + MUX("MUX",18), DCNMEAS("DCN-MEAS",19), HMP("HMP",20), PRM("PRM",21), @@ -81,8 +82,8 @@ public enum IPProtocols { MOBILE("MOBILE",55), TLSP("TLSP",56), SKIP("SKIP",57), - */IPV6ICMP("IPv6-ICMP", 58); - /* IPV6NoNxt("IPv6-NoNxt",59), + IPV6ICMP("IPv6-ICMP", 58), + IPV6NoNxt("IPv6-NoNxt",59), IPV6Opts("IPv6-Opts",60), ANYHOST("ANY-HOST",61), CFTP("CFTP",62), @@ -166,10 +167,15 @@ public enum IPProtocols { HIP("HIP",139), SHIM6("Shim6",140), WESP("WESP",141), - ROHC("ROHC",142); - */ - private static final String regexDecimalString = "^[0-9]{3}$"; - private static final String regexHexString = "^(0(x|X))[0-9a-fA-F]{2}$"; + ROHC("ROHC",142), + /*143-252 Unassigned by IANA*/ + + //Experimebtal protocol numbers (http://tools.ietf.org/html/rfc3692) + EXP1("Experimental1", 253), + EXP2("Experimental2", 254), + + RESERVED("RESERVED",255); + private String protocolName; private int protocolNumber; @@ -190,6 +196,7 @@ public enum IPProtocols { return ((Integer) protocolNumber).byteValue(); } + @Override public String toString() { return protocolName; } @@ -199,11 +206,11 @@ public enum IPProtocols { } public static String getProtocolName(short number) { - return getProtocolNameInternal((int) number & 0xffff); + return getProtocolNameInternal(number & 0xffff); } public static String getProtocolName(byte number) { - return getProtocolNameInternal((int) number & 0xff); + return getProtocolNameInternal(number & 0xff); } private static String getProtocolNameInternal(int number) { @@ -212,52 +219,35 @@ public enum IPProtocols { return proto.toString(); } } + //TODO: this is for backwards compatibility return "0x" + Integer.toHexString(number); } public static short getProtocolNumberShort(String name) { - if (name.matches(regexHexString)) { - return Short.valueOf(Short.decode(name)); - } - if (name.matches(regexDecimalString)) { - return Short.valueOf(name); + IPProtocols p = fromString(name); + if (p != null) { + return p.shortValue(); } - for (IPProtocols proto : IPProtocols.values()) { - if (proto.protocolName.equalsIgnoreCase(name)) { - return proto.shortValue(); - } - } - return 0; + //This method should be called after validation only + throw new IllegalArgumentException("Illegal IP protocol value: " + name); } public static int getProtocolNumberInt(String name) { - if (name.matches(regexHexString)) { - return Integer.valueOf(Integer.decode(name)); - } - if (name.matches(regexDecimalString)) { - return Integer.valueOf(name); - } - for (IPProtocols proto : IPProtocols.values()) { - if (proto.protocolName.equalsIgnoreCase(name)) { - return proto.intValue(); - } + IPProtocols p = fromString(name); + if (p != null) { + return p.intValue(); } - return 0; + //This method should be called after validation only + throw new IllegalArgumentException("Illegal IP protocol value: " + name); } public static byte getProtocolNumberByte(String name) { - if (name.matches(regexHexString)) { - return Integer.valueOf(Integer.decode(name)).byteValue(); + IPProtocols p = fromString(name); + if (p != null) { + return p.byteValue(); } - if (name.matches(regexDecimalString)) { - return Integer.valueOf(name).byteValue(); - } - for (IPProtocols proto : IPProtocols.values()) { - if (proto.protocolName.equalsIgnoreCase(name)) { - return proto.byteValue(); - } - } - return 0; + //This method should be called after validation only + throw new IllegalArgumentException("Illegal IP protocol value: " + name); } public static List getProtocolNameList() { @@ -267,4 +257,47 @@ public enum IPProtocols { } return protoList; } + + /** + * Method to parse an IPProtocol from a numeric string + * (see: {@link Java.Lang.Integer.decode(java.lang.String)} for parsable strings), + * or this enum's name string. + * + * @param s + * The IP protocol string to be parsed + * @return The IP protocol Enum, or null if invalid protocol string is passed + */ + public static IPProtocols fromString(String s) { + // null/empty/any/* evaluates to ANY + if (s == null || s.isEmpty() || s.equalsIgnoreCase("any") || s.equals("*")) { + return IPProtocols.ANY; + } + + // Try parsing numeric and find the related ENUM + try { + int protoNum = Integer.decode(s); + for (IPProtocols protoEnum : IPProtocols.values()) { + if (protoEnum.protocolNumber == protoNum) { + return protoEnum; + } + } + // At this point it's an invalid number (i.e. out of range or not a valid proto num) + return null; + } catch (NumberFormatException nfe) { + // numeric failed try by NAME + try { + return valueOf(s); + } catch(IllegalArgumentException e) { + // Neither numeric nor enum NAME, attempt human readable name + for (IPProtocols protoEnum : IPProtocols.values()) { + if (protoEnum.toString().equalsIgnoreCase(s)) { + return protoEnum; + } + } + //couldn't parse, signifies an invalid proto field! + return null; + } + + } + } } diff --git a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/IPProtocolsTest.java b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/IPProtocolsTest.java new file mode 100644 index 0000000000..fc6d78c260 --- /dev/null +++ b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/utils/IPProtocolsTest.java @@ -0,0 +1,107 @@ +/** + * + */ +package org.opendaylight.controller.sal.utils; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +/** + * @author ykhodork + * + */ +public class IPProtocolsTest { + + static short shortVal = 1; + static int intVal = 1; + static byte byteVal = 1; + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#getProtocolName(int)}. + */ + @Test + public void testGetProtocolNameInt() { + assertEquals("ICMP", IPProtocols.getProtocolName(1)); + assertEquals("0x4d2", IPProtocols.getProtocolName(1234)); + } + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#getProtocolName(short)}. + */ + @Test + public void testGetProtocolNameShort() { + assertEquals("ICMP", IPProtocols.getProtocolName(shortVal)); + } + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#getProtocolName(byte)}. + */ + @Test + public void testGetProtocolNameByte() { + assertEquals("ICMP", IPProtocols.getProtocolName(byteVal)); + } + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#getProtocolNumberShort(java.lang.String)}. + */ + @Test + public void testGetProtocolNumberShort() { + assertEquals(shortVal, IPProtocols.getProtocolNumberShort("ICMP")); + } + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#getProtocolNumberInt(java.lang.String)}. + */ + @Test + public void testGetProtocolNumberInt() { + assertEquals(intVal, IPProtocols.getProtocolNumberInt("ICMP")); + } + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#getProtocolNumberByte(java.lang.String)}. + */ + @Test + public void testGetProtocolNumberByte() { + assertEquals(byteVal, IPProtocols.getProtocolNumberByte("ICMP")); + } + + /** + * Test method for {@link org.opendaylight.controller.sal.utils.IPProtocols#fromString(java.lang.String)}. + */ + @Test + public void testFromString() { + assertTrue(null == IPProtocols.fromString("Not a protocol")); + assertTrue(null == IPProtocols.fromString("0xFFF")); + assertTrue(null == IPProtocols.fromString("-2")); + + assertTrue(IPProtocols.ANY == IPProtocols.fromString("any")); + assertTrue(IPProtocols.ANY == IPProtocols.fromString("ANY")); + assertTrue(IPProtocols.ANY == IPProtocols.fromString("*")); + assertTrue(IPProtocols.ANY == IPProtocols.fromString(null)); + + assertTrue(IPProtocols.TCP == IPProtocols.fromString("TCP")); + assertTrue(IPProtocols.TCP == IPProtocols.fromString("tcp")); + assertTrue(IPProtocols.UDP == IPProtocols.fromString("0x11")); + assertTrue(IPProtocols.UDP == IPProtocols.fromString("0X11")); + + } + +} + + + + + + + + + + + + + + + +