[ovs-dev] [PATCH 2/3] ofp-parse: Accept port names in "output" and "enqueue" actions.

[Ben Pfaff]

The "output" and "enqueue" actions did not accept port names.  The
ovs-ofctl manpage claimed that "enqueue" did accept them.  This fixes
the problem.

As a side effect, this change begins validating numeric port numbers used
in "output" and "enqueue" actions.  We are trying to get users to use
keywords such as "local" instead of the corresponding numeric values for
OpenFlow 1.1+ compatibility.  As NEWS for 1.9.0 says:

- ovs-ofctl:
  - Commands and actions that accept port numbers now also accept keywords
    that represent those ports (such as LOCAL, NONE, and ALL).  This is
    also the recommended way to specify these ports, for compatibility
    with OpenFlow 1.1 and later (which use the OpenFlow 1.0 numbers
    for these ports for different purposes).

I had overlooked these actions in previous versions.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-parse.c       |   10 +++++++---
 tests/ofproto-dpif.at |    6 +++---
 tests/ovs-ofctl.at    |    4 ++--
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 6282c0b..5204c78 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -136,7 +136,9 @@ parse_enqueue(char *arg, struct ofpbuf *ofpacts)
     }
 
     enqueue = ofpact_put_ENQUEUE(ofpacts);
-    enqueue->port = str_to_u32(port);
+    if (!ofputil_port_from_string(port, &enqueue->port)) {
+        ovs_fatal(0, "%s: invalid port", port);
+    }
     enqueue->queue = str_to_u32(queue);
 }
 
@@ -153,7 +155,9 @@ parse_output(char *arg, struct ofpbuf *ofpacts)
         struct ofpact_output *output;
 
         output = ofpact_put_OUTPUT(ofpacts);
-        output->port = str_to_u32(arg);
+        if (!ofputil_port_from_string(arg, &output->port)) {
+            ovs_fatal(0, "%s: invalid port", arg);
+        }
         output->max_len = output->port == OFPP_CONTROLLER ? UINT16_MAX : 0;
     }
 }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 1fdbac3..80f7975 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -166,7 +166,7 @@ AT_SETUP([ofproto-dpif - DSCP])
 OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy])
 ADD_OF_PORTS([br0], [9])
 AT_DATA([flows.txt], [dnl
-actions=output:65534,enqueue:1:1,enqueue:1:2,enqueue:1:2,enqueue:1:1,output:1,mod_nw_tos:0,output:1,output:65534
+actions=local,enqueue(1,1),enqueue:1:2,enqueue:1:2,enqueue:1:1,output:1,mod_nw_tos:0,output:1,local
 ])
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-vsctl -- \
@@ -197,8 +197,8 @@ AT_DATA([flows.txt], [dnl
 in_port=local actions=local,flood
 in_port=1 actions=flood
 in_port=2 actions=all
-in_port=3 actions=output:65534,output:1,output:2,output:3,output:4,output:5,output:6,output:7
-in_port=4 actions=enqueue:65534:1,enqueue:1:1,enqueue:2:1,enqueue:3:2,enqueue:4:1,enqueue:5:1,enqueue:6:1,enqueue:7:1
+in_port=3 actions=local,output:1,output:2,output:3,output:4,output:5,output:6,output:7
+in_port=4 actions=enqueue:local:1,enqueue:1:1,enqueue:2:1,enqueue:3:2,enqueue:4:1,enqueue:5:1,enqueue:6:1,enqueue:7:1
 ])
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-ofctl mod-port br0 5 noforward])
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index db19e01..2c99bb2 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -11,7 +11,7 @@ udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1
 cookie=0x123456789abcdef hard_timeout=10 priority=60000 actions=controller
 actions=note:41.42.43,note:00.01.02.03.04.05.06.07,note
 actions=set_field:fe80:0123:4567:890a:a6ba:dbff:fefe:59fa->ipv6_src
-in_port=0 actions=resubmit:0
+in_port=0 actions=resubmit:0,enqueue:local:5,enqueue:123:456
 actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
 ]])
 
@@ -28,7 +28,7 @@ OFPT_FLOW_MOD: ADD udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1
 OFPT_FLOW_MOD: ADD priority=60000 cookie:0x123456789abcdef hard:10 actions=CONTROLLER:65535
 OFPT_FLOW_MOD: ADD actions=note:41.42.43.00.00.00,note:00.01.02.03.04.05.06.07.00.00.00.00.00.00,note:00.00.00.00.00.00
 OFPT_FLOW_MOD: ADD actions=load:0xa6badbfffefe59fa->NXM_NX_IPV6_SRC[0..63],load:0xfe8001234567890a->NXM_NX_IPV6_SRC[64..127]
-OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0
+OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0,enqueue:LOCAL:5,enqueue:123:456
 OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
 ]])
 AT_CLEANUP
-- 
1.7.2.5