From: Gurucharan Shetty Date: Tue, 24 Sep 2013 05:58:46 +0000 (-0700) Subject: ovs-dpctl, ofproto/trace: Show and handle the in_port name in flows. X-Git-Tag: sliver-openvswitch-2.0.90-1~11^2~13 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=0a37839c03307cc4cc726d27a7348c05d469c5eb;p=sliver-openvswitch.git ovs-dpctl, ofproto/trace: Show and handle the in_port name in flows. 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 Acked-by: Ben Pfaff --- diff --git a/lib/dpif.c b/lib/dpif.c index add2a0bdf..b66e3bc03 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1349,7 +1349,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 0785c6a46..5c7ccfb63 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 4abf54344..2712cb007 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -23,6 +23,7 @@ #include #include #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 from odp port number to 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 037ee4b8e..80e97e04c 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -5220,12 +5220,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; @@ -5233,6 +5234,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[], ds_init(&result); ofpbuf_init(&odp_key, 0); ofpbuf_init(&odp_mask, 0); + simap_init(&port_names); /* Handle "-generate" or a hex string as the last argument. */ if (!strcmp(argv[argc - 1], "-generate")) { @@ -5249,37 +5251,42 @@ 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; + } + } + 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, @@ -5332,6 +5339,7 @@ exit: ofpbuf_delete(packet); ofpbuf_uninit(&odp_key); ofpbuf_uninit(&odp_mask); + simap_destroy(&port_names); } static void @@ -5746,7 +5754,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 205044364..ea3602016 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -1161,8 +1161,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 @@ -1307,7 +1314,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 ])]) @@ -1320,7 +1327,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 ])]) @@ -1333,7 +1340,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 ])]) @@ -1346,7 +1353,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 45605e4b9..183a3b3cd 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 af4fc1f51..4fb02dda4 100644 --- a/utilities/ovs-dpctl.c +++ b/utilities/ovs-dpctl.c @@ -740,9 +740,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; @@ -754,13 +757,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); @@ -768,7 +777,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); } @@ -779,25 +790,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, @@ -846,23 +869,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); @@ -1049,7 +1081,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),