[ovs-dev] [PATCH] ovs-dpctl, ofproto/trace: Show and handle the in_port name in flows.
Gurucharan Shetty
shettyg at nicira.com
Wed Sep 25 07:55:43 UTC 2013
With this commit, whenever the verbosity is enabled with '-m'
option, the ovs-dpctl dump-flows command will display the flows with
in_port field showing the name instead of a port number.
Conversely, one can also use a name in the in_port field with del-flow,
add-flow and mod-flow commands of ovs-dpctl. One should also be able
to use the port name when supplying the datapath flow as an input
to ofproto/trace command.
Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
---
lib/dpif.c | 2 +-
lib/odp-util.c | 75 ++++++++++++++++++++++++++++++++++++++++--------
lib/odp-util.h | 14 ++++++++-
ofproto/ofproto-dpif.c | 65 ++++++++++++++++++++++-------------------
tests/ofproto-dpif.at | 17 +++++++----
tests/test-odp.c | 2 +-
utilities/ovs-dpctl.c | 62 +++++++++++++++++++++++++++++----------
7 files changed, 173 insertions(+), 64 deletions(-)
diff --git a/lib/dpif.c b/lib/dpif.c
index 02cc36a..d62f3b2 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1351,7 +1351,7 @@ log_flow_message(const struct dpif *dpif, int error, const char *operation,
if (error) {
ds_put_format(&ds, "(%s) ", ovs_strerror(error));
}
- odp_flow_format(key, key_len, mask, mask_len, &ds, true);
+ odp_flow_format(key, key_len, mask, mask_len, NULL, &ds, true);
if (stats) {
ds_put_cstr(&ds, ", ");
dpif_flow_stats_format(stats, &ds);
diff --git a/lib/odp-util.c b/lib/odp-util.c
index aec4196..57f7920 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -51,7 +51,8 @@ static const char *delimiters = ", \t\r\n";
static int parse_odp_key_mask_attr(const char *, const struct simap *port_names,
struct ofpbuf *, struct ofpbuf *);
static void format_odp_key_attr(const struct nlattr *a,
- const struct nlattr *ma, struct ds *ds,
+ const struct nlattr *ma,
+ const struct hmap *portno_names, struct ds *ds,
bool verbose);
/* Returns one the following for the action with the given OVS_ACTION_ATTR_*
@@ -401,7 +402,7 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
break;
case OVS_ACTION_ATTR_SET:
ds_put_cstr(ds, "set(");
- format_odp_key_attr(nl_attr_get(a), NULL, ds, true);
+ format_odp_key_attr(nl_attr_get(a), NULL, NULL, ds, true);
ds_put_cstr(ds, ")");
break;
case OVS_ACTION_ATTR_PUSH_VLAN:
@@ -935,10 +936,49 @@ odp_mask_attr_is_exact(const struct nlattr *ma)
return is_exact;
}
+void
+odp_portno_names_set(struct hmap *portno_names, odp_port_t port_no,
+ char *port_name)
+{
+ struct odp_portno_names *odp_portno_names;
+
+ odp_portno_names = xmalloc(sizeof *odp_portno_names);
+ odp_portno_names->port_no = port_no;
+ odp_portno_names->name = xstrdup(port_name);
+ hmap_insert(portno_names, &odp_portno_names->hmap_node,
+ hash_odp_port(port_no));
+}
+
+static char *
+odp_portno_names_get(const struct hmap *portno_names, odp_port_t port_no)
+{
+ struct odp_portno_names *odp_portno_names;
+
+ HMAP_FOR_EACH_IN_BUCKET (odp_portno_names, hmap_node,
+ hash_odp_port(port_no), portno_names) {
+ if (odp_portno_names->port_no == port_no) {
+ return odp_portno_names->name;
+ }
+ }
+ return NULL;
+}
+
+void
+odp_portno_names_destroy(struct hmap *portno_names)
+{
+ struct odp_portno_names *odp_portno_names, *odp_portno_names_next;
+ HMAP_FOR_EACH_SAFE (odp_portno_names, odp_portno_names_next,
+ hmap_node, portno_names) {
+ hmap_remove(portno_names, &odp_portno_names->hmap_node);
+ free(odp_portno_names->name);
+ free(odp_portno_names);
+ }
+}
static void
format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
- struct ds *ds, bool verbose)
+ const struct hmap *portno_names, struct ds *ds,
+ bool verbose)
{
struct flow_tnl tun_key;
enum ovs_key_attr attr = nl_attr_type(a);
@@ -981,10 +1021,11 @@ format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
case OVS_KEY_ATTR_ENCAP:
if (ma && nl_attr_get_size(ma) && nl_attr_get_size(a)) {
odp_flow_format(nl_attr_get(a), nl_attr_get_size(a),
- nl_attr_get(ma), nl_attr_get_size(ma), ds, verbose);
- } else if (nl_attr_get_size(a)) {
- odp_flow_format(nl_attr_get(a), nl_attr_get_size(a), NULL, 0, ds,
+ nl_attr_get(ma), nl_attr_get_size(ma), NULL, ds,
verbose);
+ } else if (nl_attr_get_size(a)) {
+ odp_flow_format(nl_attr_get(a), nl_attr_get_size(a), NULL, 0, NULL,
+ ds, verbose);
}
break;
@@ -1038,9 +1079,19 @@ format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
break;
case OVS_KEY_ATTR_IN_PORT:
- ds_put_format(ds, "%"PRIu32, nl_attr_get_u32(a));
- if (!is_exact) {
- ds_put_format(ds, "/%#"PRIx32, nl_attr_get_u32(ma));
+ if (portno_names && verbose && is_exact) {
+ char *name = odp_portno_names_get(portno_names,
+ u32_to_odp(nl_attr_get_u32(a)));
+ if (name) {
+ ds_put_format(ds, "%s", name);
+ } else {
+ ds_put_format(ds, "%"PRIu32, nl_attr_get_u32(a));
+ }
+ } else {
+ ds_put_format(ds, "%"PRIu32, nl_attr_get_u32(a));
+ if (!is_exact) {
+ ds_put_format(ds, "/%#"PRIx32, nl_attr_get_u32(ma));
+ }
}
break;
@@ -1364,7 +1415,7 @@ generate_all_wildcard_mask(struct ofpbuf *ofp, const struct nlattr *key)
void
odp_flow_format(const struct nlattr *key, size_t key_len,
const struct nlattr *mask, size_t mask_len,
- struct ds *ds, bool verbose)
+ const struct hmap *portno_names, struct ds *ds, bool verbose)
{
if (key_len) {
const struct nlattr *a;
@@ -1398,7 +1449,7 @@ odp_flow_format(const struct nlattr *key, size_t key_len,
if (!first_field) {
ds_put_char(ds, ',');
}
- format_odp_key_attr(a, ma, ds, verbose);
+ format_odp_key_attr(a, ma, portno_names, ds, verbose);
first_field = false;
}
ofpbuf_clear(&ofp);
@@ -1435,7 +1486,7 @@ void
odp_flow_key_format(const struct nlattr *key,
size_t key_len, struct ds *ds)
{
- odp_flow_format(key, key_len, NULL, 0, ds, true);
+ odp_flow_format(key, key_len, NULL, 0, NULL, ds, true);
}
static void
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 192cfa0..a804bd5 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -23,6 +23,7 @@
#include <string.h>
#include <linux/openvswitch.h>
#include "hash.h"
+#include "hmap.h"
#include "openflow/openflow.h"
#include "util.h"
@@ -42,6 +43,16 @@ void format_odp_actions(struct ds *, const struct nlattr *odp_actions,
int odp_actions_from_string(const char *, const struct simap *port_names,
struct ofpbuf *odp_actions);
+/* A map between odp port number and its name. */
+struct odp_portno_names {
+ struct hmap_node hmap_node; /* A node in a port number to name hmap. */
+ odp_port_t port_no; /* Port number in the datapath. */
+ char *name; /* Name associated with the above 'port_no'. */
+};
+
+void odp_portno_names_set(struct hmap *portno_names, odp_port_t port_no,
+ char *port_name);
+void odp_portno_names_destroy(struct hmap *portno_names);
/* The maximum number of bytes that odp_flow_key_from_flow() appends to a
* buffer. This is the upper bound on the length of a nlattr-formatted flow
* key that ovs-vswitchd fully understands.
@@ -94,7 +105,8 @@ enum odp_key_fitness odp_tun_key_from_attr(const struct nlattr *,
void odp_flow_format(const struct nlattr *key, size_t key_len,
const struct nlattr *mask, size_t mask_len,
- struct ds *, bool verbose);
+ const struct hmap *portno_names, struct ds *,
+ bool verbose);
void odp_flow_key_format(const struct nlattr *, size_t, struct ds *);
int odp_flow_from_string(const char *s,
const struct simap *port_names,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 80874b8..2e61515 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5340,12 +5340,13 @@ static void
ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
void *aux OVS_UNUSED)
{
- const struct dpif_backer *backer;
+ const struct dpif_backer *backer = NULL;
struct ofproto_dpif *ofproto;
struct ofpbuf odp_key, odp_mask;
struct ofpbuf *packet;
struct ds result;
struct flow flow;
+ struct simap port_names;
char *s;
packet = NULL;
@@ -5369,37 +5370,43 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
}
}
+ /* odp_flow can have its in_port specified as a name instead of port no.
+ * We do not yet know whether a given flow is a odp_flow or a br_flow.
+ * But, to know whether a flow is odp_flow through odp_flow_from_string(),
+ * we need to create a simap of name to port no. */
+ if (argc == 3) {
+ const char *dp_type;
+ if (!strncmp(argv[1], "ovs-", 4)) {
+ dp_type = argv[1] + 4;
+ } else {
+ dp_type = argv[1];
+ }
+ backer = shash_find_data(&all_dpif_backers, dp_type);
+ } else {
+ struct shash_node *node;
+ if (shash_count(&all_dpif_backers) == 1) {
+ node = shash_first(&all_dpif_backers);
+ backer = node->data;
+ }
+ }
+ simap_init(&port_names);
+ if (backer && backer->dpif) {
+ struct dpif_port dpif_port;
+ struct dpif_port_dump port_dump;
+ DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, backer->dpif) {
+ simap_put(&port_names, dpif_port.name,
+ odp_to_u32(dpif_port.port_no));
+ }
+ }
+
/* Parse the flow and determine whether a datapath or
* bridge is specified. If function odp_flow_key_from_string()
* returns 0, the flow is a odp_flow. If function
* parse_ofp_exact_flow() returns 0, the flow is a br_flow. */
- if (!odp_flow_from_string(argv[argc - 1], NULL, &odp_key, &odp_mask)) {
- /* If the odp_flow is the second argument,
- * the datapath name is the first argument. */
- if (argc == 3) {
- const char *dp_type;
- if (!strncmp(argv[1], "ovs-", 4)) {
- dp_type = argv[1] + 4;
- } else {
- dp_type = argv[1];
- }
- backer = shash_find_data(&all_dpif_backers, dp_type);
- if (!backer) {
- unixctl_command_reply_error(conn, "Cannot find datapath "
- "of this name");
- goto exit;
- }
- } else {
- /* No datapath name specified, so there should be only one
- * datapath. */
- struct shash_node *node;
- if (shash_count(&all_dpif_backers) != 1) {
- unixctl_command_reply_error(conn, "Must specify datapath "
- "name, there is more than one type of datapath");
- goto exit;
- }
- node = shash_first(&all_dpif_backers);
- backer = node->data;
+ if (!odp_flow_from_string(argv[argc - 1], &port_names, &odp_key, &odp_mask)) {
+ if (!backer) {
+ unixctl_command_reply_error(conn, "Cannot find the datapath");
+ goto exit;
}
if (xlate_receive(backer, NULL, odp_key.data, odp_key.size, &flow,
@@ -5884,7 +5891,7 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
}
odp_flow_format(subfacet->key, subfacet->key_len,
- mask.data, mask.size, &ds, false);
+ mask.data, mask.size, NULL, &ds, false);
ds_put_format(&ds, ", packets:%"PRIu64", bytes:%"PRIu64", used:",
subfacet->dp_packet_count, subfacet->dp_byte_count);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index f67c3ab..551c2a2 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1139,8 +1139,15 @@ in_port=2 actions=output:1
])
AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
-odp_flow="in_port(1)"
+odp_flow="in_port(p1)"
br_flow="in_port=1"
+# Test command: ofproto/trace odp_flow with in_port as a name.
+AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [dnl
+Datapath actions: 2
+])
+
+odp_flow="in_port(1)"
# Test command: ofproto/trace odp_flow
AT_CHECK([ovs-appctl ofproto/trace "$odp_flow"], [0], [stdout])
AT_CHECK([tail -1 stdout], [0], [dnl
@@ -1285,7 +1292,7 @@ m4_foreach(
[AT_CHECK([ovs-appctl ofproto/trace wrong_name "$odp_flow" option],
[2], [], [stderr])
AT_CHECK([tail -2 stderr], [0], [dnl
-Cannot find datapath of this name
+Cannot find the datapath
ovs-appctl: ovs-vswitchd: server returned an error
])])
@@ -1298,7 +1305,7 @@ m4_foreach(
[AT_CHECK([ovs-appctl ofproto/trace "" "$odp_flow" option],
[2], [], [stderr])
AT_CHECK([tail -2 stderr], [0], [dnl
-Cannot find datapath of this name
+Cannot find the datapath
ovs-appctl: ovs-vswitchd: server returned an error
])])
@@ -1311,7 +1318,7 @@ m4_foreach(
[AT_CHECK([ovs-appctl ofproto/trace ovs-system "$odp_flow" option],
[2], [], [stderr])
AT_CHECK([tail -2 stderr], [0], [dnl
-Cannot find datapath of this name
+Cannot find the datapath
ovs-appctl: ovs-vswitchd: server returned an error
])])
@@ -1324,7 +1331,7 @@ m4_foreach(
[AT_CHECK([ovs-appctl ofproto/trace br0 "$odp_flow" option],
[2], [], [stderr])
AT_CHECK([tail -2 stderr], [0], [dnl
-Cannot find datapath of this name
+Cannot find the datapath
ovs-appctl: ovs-vswitchd: server returned an error
])])
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 45605e4..183a3b3 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -86,7 +86,7 @@ parse_keys(bool wc_keys)
ds_init(&out);
if (wc_keys) {
odp_flow_format(odp_key.data, odp_key.size,
- odp_mask.data, odp_mask.size, &out, false);
+ odp_mask.data, odp_mask.size, NULL, &out, false);
} else {
odp_flow_key_format(odp_key.data, odp_key.size, &out);
}
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index 98b47b8..268df1e 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -742,9 +742,12 @@ dpctl_dump_flows(int argc, char *argv[])
{
const struct dpif_flow_stats *stats;
const struct nlattr *actions;
- struct dpif_flow_dump dump;
+ struct dpif_flow_dump flow_dump;
const struct nlattr *key;
const struct nlattr *mask;
+ struct dpif_port dpif_port;
+ struct dpif_port_dump port_dump;
+ struct hmap portno_names;
size_t actions_len;
struct dpif *dpif;
size_t key_len;
@@ -756,13 +759,19 @@ dpctl_dump_flows(int argc, char *argv[])
run(parsed_dpif_open(name, false, &dpif), "opening datapath");
free(name);
+ hmap_init(&portno_names);
+ DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
+ odp_portno_names_set(&portno_names, dpif_port.port_no, dpif_port.name);
+ }
+
ds_init(&ds);
- dpif_flow_dump_start(&dump, dpif);
- while (dpif_flow_dump_next(&dump, &key, &key_len,
+ dpif_flow_dump_start(&flow_dump, dpif);
+ while (dpif_flow_dump_next(&flow_dump, &key, &key_len,
&mask, &mask_len,
&actions, &actions_len, &stats)) {
ds_clear(&ds);
- odp_flow_format(key, key_len, mask, mask_len, &ds, verbosity);
+ odp_flow_format(key, key_len, mask, mask_len, &portno_names, &ds,
+ verbosity);
ds_put_cstr(&ds, ", ");
dpif_flow_stats_format(stats, &ds);
@@ -770,7 +779,9 @@ dpctl_dump_flows(int argc, char *argv[])
format_odp_actions(&ds, actions, actions_len);
printf("%s\n", ds_cstr(&ds));
}
- dpif_flow_dump_done(&dump);
+ dpif_flow_dump_done(&flow_dump);
+ odp_portno_names_destroy(&portno_names);
+ hmap_destroy(&portno_names);
ds_destroy(&ds);
dpif_close(dpif);
}
@@ -781,25 +792,37 @@ dpctl_put_flow(int argc, char *argv[], enum dpif_flow_put_flags flags)
const char *key_s = argv[argc - 2];
const char *actions_s = argv[argc - 1];
struct dpif_flow_stats stats;
+ struct dpif_port dpif_port;
+ struct dpif_port_dump port_dump;
struct ofpbuf actions;
struct ofpbuf key;
struct ofpbuf mask;
struct dpif *dpif;
struct ds s;
char *dp_name;
+ struct simap port_names;
+
+ dp_name = argc == 4 ? xstrdup(argv[1]) : get_one_dp();
+ run(parsed_dpif_open(dp_name, false, &dpif), "opening datapath");
+ free(dp_name);
+
+
+ simap_init(&port_names);
+ DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
+ simap_put(&port_names, dpif_port.name, odp_to_u32(dpif_port.port_no));
+ }
ds_init(&s);
ofpbuf_init(&key, 0);
ofpbuf_init(&mask, 0);
- run(odp_flow_from_string(key_s, NULL, &key, &mask), "parsing flow key");
+ run(odp_flow_from_string(key_s, &port_names, &key, &mask),
+ "parsing flow key");
+
+ simap_destroy(&port_names);
ofpbuf_init(&actions, 0);
run(odp_actions_from_string(actions_s, NULL, &actions), "parsing actions");
- dp_name = argc == 4 ? xstrdup(argv[1]) : get_one_dp();
- run(parsed_dpif_open(dp_name, false, &dpif), "opening datapath");
- free(dp_name);
-
run(dpif_flow_put(dpif, flags,
key.data, key.size,
mask.size == 0 ? NULL : mask.data, mask.size,
@@ -848,23 +871,32 @@ dpctl_del_flow(int argc, char *argv[])
{
const char *key_s = argv[argc - 1];
struct dpif_flow_stats stats;
+ struct dpif_port dpif_port;
+ struct dpif_port_dump port_dump;
struct ofpbuf key;
struct ofpbuf mask; /* To be ignored. */
struct dpif *dpif;
char *dp_name;
-
- ofpbuf_init(&key, 0);
- ofpbuf_init(&mask, 0);
- run(odp_flow_from_string(key_s, NULL, &key, &mask), "parsing flow key");
+ struct simap port_names;
dp_name = argc == 3 ? xstrdup(argv[1]) : get_one_dp();
run(parsed_dpif_open(dp_name, false, &dpif), "opening datapath");
free(dp_name);
+ simap_init(&port_names);
+ DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
+ simap_put(&port_names, dpif_port.name, odp_to_u32(dpif_port.port_no));
+ }
+
+ ofpbuf_init(&key, 0);
+ ofpbuf_init(&mask, 0);
+ run(odp_flow_from_string(key_s, &port_names, &key, &mask), "parsing flow key");
+
run(dpif_flow_del(dpif,
key.data, key.size,
print_statistics ? &stats : NULL), "deleting flow");
+ simap_destroy(&port_names);
ofpbuf_uninit(&key);
ofpbuf_uninit(&mask);
@@ -1051,7 +1083,7 @@ dpctl_normalize_actions(int argc, char *argv[])
"odp_flow_key_from_string");
ds_clear(&s);
- odp_flow_format(keybuf.data, keybuf.size, NULL, 0, &s, verbosity);
+ odp_flow_format(keybuf.data, keybuf.size, NULL, 0, NULL, &s, verbosity);
printf("input flow: %s\n", ds_cstr(&s));
run(odp_flow_key_to_flow(keybuf.data, keybuf.size, &flow),
--
1.7.9.5
More information about the dev
mailing list