Re: [ovs-dev] [PATCH v4 4/4] netdev-dpdk: Allow arbitrary eal arguments
On 01/15/2016 10:02 PM, Aaron Conole wrote: A previous change moved some commonly used arguments from commandline to the database. This change allows arbitrary eal arguments to be provided via a new db entry 'other_config:dpdk-extra' which will tokenize the string and add it to the argument list. This patch removes 'defaulting' of socket-mem as it will conflict with attempts to pass --no-huge via the eal command line Does OVS even work at all with --no-huge? I doubt it... In any case, I'd think a sane default is more important than supporting a debugging-only option. - Panu - ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn-tutorial: fix a typo
On 01/16/2016 08:23 PM, William Tu wrote: > switch_in_pre_acl -> switch_out_pre_acl > > Signed-off-by: William Tu > --- > tutorial/OVN-Tutorial.md | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tutorial/OVN-Tutorial.md b/tutorial/OVN-Tutorial.md > index 2e6a08d..1188faa 100644 > --- a/tutorial/OVN-Tutorial.md > +++ b/tutorial/OVN-Tutorial.md > @@ -675,7 +675,7 @@ tracker. This populates the connection state fields so > that we can apply policy > as appropriate. > > table=0(switch_out_pre_acl), priority= 100, match=(ip), > action=(ct_next;) > -table=1(switch_in_pre_acl), priority=0, match=(1), action=(next;) > +table=1(switch_out_pre_acl), priority=0, match=(1), action=(next;) > > In `switch_out_acl`, we allow packets associated with existing connections. > We > drop packets that are deemed to be invalid (such as non-SYN TCP packet not > Thanks for the patch! I pushed this to master and branch-2.5. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v4 4/4] netdev-dpdk: Allow arbitrary eal arguments
Panu Matilainen writes: > On 01/15/2016 10:02 PM, Aaron Conole wrote: >> A previous change moved some commonly used arguments from commandline to >> the database. This change allows arbitrary eal arguments to be provided >> via a new db entry 'other_config:dpdk-extra' which will tokenize the >> string and add it to the argument list. >> >> This patch removes 'defaulting' of socket-mem as it will conflict with >> attempts to pass --no-huge via the eal command line > > Does OVS even work at all with --no-huge? I doubt it... > > In any case, I'd think a sane default is more important than > supporting a debugging-only option. Hrrm I could have sworn I had done a test with --no-huge on my OVS setup, but you're right it doesn't seem to be working. Okay, I'll resend v5 and state that there is no support for the --no-huge option in the dpdk-extra command line. Thanks for the review, Panu! > - Panu - ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Group selection_method and hash
Hi, I would like to use the hash selection_method that OvS supports but with OpenFlow13. Could someone tell me the fields that are used as the input to the hash function for OpenFlow13 in OvS? Thanks, Kash ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] ovn-controller: Move local_datapaths calculation.
Before this patch, physical.c build up the set of local datapaths for its own use. I'd like to use it in another module in a later patch, so pull it out of physical. It's now populated by the bindings module, since that seems like a more appropriate place to do it, and it's also done much earlier in the main loop, making it easier to re-use. Signed-off-by: Russell Bryant --- ovn/controller/binding.c| 25 - ovn/controller/binding.h| 3 ++- ovn/controller/ovn-controller.c | 18 -- ovn/controller/physical.c | 25 +++-- ovn/controller/physical.h | 3 ++- 5 files changed, 47 insertions(+), 27 deletions(-) diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 1854ff4..82cd10b 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -17,6 +17,7 @@ #include "binding.h" #include "lib/bitmap.h" +#include "lib/hmap.h" #include "lib/sset.h" #include "lib/util.h" #include "lib/vswitch-idl.h" @@ -117,10 +118,24 @@ update_ct_zones(struct sset *lports, struct simap *ct_zones, } } +static void +add_local_datapath(struct hmap *local_datapaths, +const struct sbrec_port_binding *binding_rec) +{ +struct hmap_node *ld; +ld = hmap_first_with_hash(local_datapaths, + binding_rec->datapath->tunnel_key); +if (!ld) { +ld = xmalloc(sizeof *ld); +hmap_insert(local_datapaths, ld, +binding_rec->datapath->tunnel_key); +} +} + void binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, const char *chassis_id, struct simap *ct_zones, -unsigned long *ct_zone_bitmap) +unsigned long *ct_zone_bitmap, struct hmap *local_datapaths) { const struct sbrec_chassis *chassis_rec; const struct sbrec_port_binding *binding_rec; @@ -150,6 +165,13 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for '%s'", chassis_id); +/* Clear local_datapaths, as we're going to rebuild it next. */ +struct hmap_node *node; +while ((node = hmap_first(local_datapaths))) { +hmap_remove(local_datapaths, node); +free(node); +} + /* Run through each binding record to see if it is resident on this * chassis and update the binding accordingly. This includes both * directly connected logical ports and children of those ports. */ @@ -161,6 +183,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, /* Add child logical port to the set of all local ports. */ sset_add(&all_lports, binding_rec->logical_port); } +add_local_datapath(local_datapaths, binding_rec); if (binding_rec->chassis == chassis_rec) { continue; } diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h index 3d4a91e..6e19c10 100644 --- a/ovn/controller/binding.h +++ b/ovn/controller/binding.h @@ -20,6 +20,7 @@ #include struct controller_ctx; +struct hmap; struct ovsdb_idl; struct ovsrec_bridge; struct simap; @@ -27,7 +28,7 @@ struct simap; void binding_register_ovs_idl(struct ovsdb_idl *); void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, const char *chassis_id, struct simap *ct_zones, - unsigned long *ct_zone_bitmap); + unsigned long *ct_zone_bitmap, struct hmap *local_datapaths); bool binding_cleanup(struct controller_ctx *, const char *chassis_id); #endif /* ovn/binding.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 02ecb3e..aefe1a5 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -33,6 +33,7 @@ #include "ovn/lib/ovn-sb-idl.h" #include "poll-loop.h" #include "fatal-signal.h" +#include "lib/hmap.h" #include "lib/vswitch-idl.h" #include "smap.h" #include "stream.h" @@ -267,6 +268,10 @@ main(int argc, char *argv[]) unixctl_command_register("ct-zone-list", "", 0, 0, ct_zone_list, &ct_zones); +/* Contains bare "struct hmap_node"s whose hash values are the tunnel_key + * of datapaths with at least one local port binding. */ +struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths); + /* Main loop. */ exiting = false; while (!exiting) { @@ -288,7 +293,8 @@ main(int argc, char *argv[]) if (chassis_id) { chassis_run(&ctx, chassis_id); encaps_run(&ctx, br_int, chassis_id); -binding_run(&ctx, br_int, chassis_id, &ct_zones, ct_zone_bitmap); +binding_run(&ctx, br_int, chassis_id, &ct_zones, ct_zone_bitmap, +&local_datapaths); } if (br_int) { @@ -300,7 +306,8 @@ main(int argc, char *argv[])
[ovs-dev] [PATCH 2/2] ovn: Fix localnet ports on the same chassis.
Multiple logical ports on the same chassis that were connected to the same physical network via localnet ports were not able to send packets to each other. This was because ovn-controller created a single patch port between br-int and the physical network access bridge and used it for all localnet ports. The fix implemented here is to create a separate patch port for every logical port of type=localnet. An optimization is included where these ports are only created if the localnet port is on a logical switch with another logical port with an associated local VIF. A nice side effect of this fix is that the code in physical.c got a lot simpler, as localnet ports are now handled mostly like local VIFs. Fixes: c02819293d52 ("ovn: Add "localnet" logical port type.") Reported-by: Han Zhou Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064413.html Signed-off-by: Russell Bryant --- ovn/controller/ovn-controller.c | 10 +- ovn/controller/patch.c | 80 +- ovn/controller/patch.h | 4 +- ovn/controller/physical.c | 237 +--- ovn/controller/physical.h | 3 +- tests/ovn-controller.at | 39 --- tests/ovn.at| 10 +- 7 files changed, 140 insertions(+), 243 deletions(-) diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index aefe1a5..b4d3fd3 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -285,11 +285,6 @@ main(int argc, char *argv[]) const struct ovsrec_bridge *br_int = get_br_int(&ctx); const char *chassis_id = get_chassis_id(ctx.ovs_idl); -/* Map bridges to local nets from ovn-bridge-mappings */ -if (br_int) { -patch_run(&ctx, br_int); -} - if (chassis_id) { chassis_run(&ctx, chassis_id); encaps_run(&ctx, br_int, chassis_id); @@ -298,6 +293,8 @@ main(int argc, char *argv[]) } if (br_int) { +patch_run(&ctx, br_int, &local_datapaths); + enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int); pinctrl_run(&ctx, br_int); @@ -306,8 +303,7 @@ main(int argc, char *argv[]) lflow_run(&ctx, &flow_table, &ct_zones); if (chassis_id) { physical_run(&ctx, mff_ovn_geneve, - br_int, chassis_id, &ct_zones, &flow_table, - &local_datapaths); + br_int, chassis_id, &ct_zones, &flow_table); } ofctrl_put(&flow_table); hmap_destroy(&flow_table); diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c index 07a3540..a48da6d 100644 --- a/ovn/controller/patch.c +++ b/ovn/controller/patch.c @@ -18,6 +18,7 @@ #include "patch.h" #include "hash.h" +#include "lib/hmap.h" #include "lib/vswitch-idl.h" #include "openvswitch/vlog.h" #include "ovn-controller.h" @@ -95,27 +96,6 @@ create_patch_port(struct controller_ctx *ctx, free(ports); } -/* Creates a pair of patch ports that connect bridges 'b1' and 'b2', using a - * port named 'name1' and 'name2' in each respective bridge. - * external-ids:'key' in each port is initialized to 'value'. - * - * If one or both of the ports already exists, leaves it there and removes it - * from 'existing_ports'. */ -static void -create_patch_ports(struct controller_ctx *ctx, - const char *key, const char *value, - const struct ovsrec_bridge *b1, - const struct ovsrec_bridge *b2, - struct shash *existing_ports) -{ -char *name1 = patch_port_name(b1->name, b2->name); -char *name2 = patch_port_name(b2->name, b1->name); -create_patch_port(ctx, key, value, b1, name1, b2, name2, existing_ports); -create_patch_port(ctx, key, value, b2, name2, b1, name1, existing_ports); -free(name2); -free(name1); -} - static void remove_port(struct controller_ctx *ctx, const struct ovsrec_port *port) @@ -153,7 +133,8 @@ remove_port(struct controller_ctx *ctx, static void add_bridge_mappings(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, -struct shash *existing_ports) +struct shash *existing_ports, +struct hmap *local_datapaths) { /* Get ovn-bridge-mappings. */ const char *mappings_cfg = ""; @@ -166,7 +147,8 @@ add_bridge_mappings(struct controller_ctx *ctx, } } -/* Create patch ports. */ +/* Parse bridge mappings. */ +struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); char *cur, *next, *start; next = start = xstrdup(mappings_cfg); while ((cur = strsep(&next, ",")) && *cur) { @@ -187,10 +169,53 @@ add_bridge_mappings(struct controller_ctx *ctx, continue; } -create_patch_ports(ctx, "ovn-localnet-port", netw
[ovs-dev] [PATCH v5 0/4] Convert DPDK configuration from command line to DB based
Currently, configuration of DPDK parameters is done via the command line through a --dpdk **OPTIONS** -- command line argument. This has a number of challenges, including: * It must be the first option passed to ovs-vswitchd * It breaks from the way most other things are configured in OVS * It doesn't allow an easy way to populate defaults This series brings the following changes to openvswitch: * All DPDK options are taken from the ovs database rather than the command line * DPDK lcores are optionally auto-assigned to a single core based on the bridge coremask. * Updated documentation v2: * Dropped the vhost-user socket configuration options. Those can be re-added as an extension * Incorporated feedback from Kevin Traynor. v3: * Went back to a global dpdk-init * Language cleanup and various minor fixes v4: * Added a way to pass arbitrary eal arguments v5: * Restore the socket-mem default, and fix up the ovs-dev.py script, along with the manpage for ovsdb-server Aaron Conole (4): netdev-dpdk: Restore thread affinity after DPDK init netdev-dpdk: Convert initialization from cmdline to db netdev-dpdk: Autofill lcore coremask if absent netdev-dpdk: Allow arbitrary eal arguments FAQ.md | 6 +- INSTALL.DPDK.md| 86 lib/netdev-dpdk.c | 249 - lib/netdev-dpdk.h | 22 ++-- utilities/ovs-dev.py | 7 +- vswitchd/bridge.c | 3 + vswitchd/ovs-vswitchd.8.in | 5 +- vswitchd/ovs-vswitchd.c| 25 + vswitchd/vswitch.xml | 128 ++- 9 files changed, 426 insertions(+), 105 deletions(-) -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v5 2/4] netdev-dpdk: Convert initialization from cmdline to db
Existing DPDK integration is provided by use of command line options which must be split out and passed to librte in a special manner. However, this forces any configuration to be passed by way of a special DPDK flag, and interferes with ovs+dpdk packaging solutions. This commit delays dpdk initialization until after the OVS database connection is established, at which point ovs initializes librte. It pulls all of the config data from the OVS database, and assembles a new argv/argc pair to be passed along. Signed-off-by: Aaron Conole --- v2: * Removed trailing whitespace * Followed for() loop brace coding style * Automatically enable DPDK when adding a DPDK enabled port * Fixed an issue on startup when DPDK enabled ports are present * Updated the documentation (including vswitch.xml) and documented all new parameters * Dropped the premature initialization test v3: * Improved description language in INSTALL.DPDK.md * Fixed the ovs-vsctl examples for DPDK * Returned to the global dpdk-init (bullet 3 from v2) * Fixed a build error when compiling without dpdk support enabled * converted to xstrdup, for consistency after rebasing v4: * No change v5: * Adjust the ovs-dev script to account for the new dpdk configuration * Update the ovs-vswitchd.8.in pointing to INSTALL.DPDK.md FAQ.md | 6 +- INSTALL.DPDK.md| 81 ++- lib/netdev-dpdk.c | 191 - lib/netdev-dpdk.h | 22 -- utilities/ovs-dev.py | 11 ++- vswitchd/bridge.c | 3 + vswitchd/ovs-vswitchd.8.in | 5 +- vswitchd/ovs-vswitchd.c| 25 +- vswitchd/vswitch.xml | 118 +++- 9 files changed, 346 insertions(+), 116 deletions(-) diff --git a/FAQ.md b/FAQ.md index 29b2e19..c233118 100644 --- a/FAQ.md +++ b/FAQ.md @@ -431,9 +431,9 @@ A: Yes. How you configure it depends on what you mean by "promiscuous A: Firstly, you must have a DPDK-enabled version of Open vSwitch. - If your version is DPDK-enabled it will support the --dpdk - argument on the command line and will display lines with - "EAL:..." during startup when --dpdk is supplied. + If your version is DPDK-enabled it will support the other_config:dpdk-init + configuration in the database and will display lines with + "EAL:..." during startup when other_config:dpdk-init is set to 'true'. Secondly, when adding a DPDK port, unlike a system port, the type for the interface must be specified. For example; diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 96b686c..46bd1a8 100644 --- a/INSTALL.DPDK.md +++ b/INSTALL.DPDK.md @@ -143,22 +143,64 @@ Using the DPDK with ovs-vswitchd: 5. Start vswitchd: - DPDK configuration arguments can be passed to vswitchd via `--dpdk` - argument. This needs to be first argument passed to vswitchd process. - dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter - for dpdk initialization. + DPDK configuration arguments can be passed to vswitchd via Open_vSwitch + other_config database. The recognized configuration options are listed. + Defaults will be provided for all values not explicitly set. + + * dpdk-init + Specifies whether OVS should initialize and support DPDK ports. This is + a boolean, and defaults to false. + + * dpdk-lcore-mask + Specifies the CPU cores on which dpdk lcore threads should be spawned. + The DPDK lcore threads are used for DPDK library tasks, such as + library internal message processing, logging, etc. Value should be in + the form of a hex string (so '0x123') similar to the 'taskset' mask + input. + If not specified, the value will be determined by choosing the lowest + CPU core from initial cpu affinity list. Otherwise, the value will be + passed directly to the DPDK library. + For performance reasons, it is best to set this to a single core on + the system, rather than allow lcore threads to float. + + * dpdk-mem-channels + This sets the number of memory spread channels per CPU socket. It is purely + an optimization flag. + + * dpdk-alloc-mem + This sets the total memory to preallocate from hugepages regardless of + processor socket. It is recommended to use dpdk-socket-mem instead. + + * dpdk-socket-mem + Comma separated list of memory to pre-allocate from hugepages on specific + sockets. + + * dpdk-hugepage-dir + Directory where hugetlbfs is mounted + + * cuse-dev-name + Option to set the vhost_cuse character device name. + + * vhost-sock-dir + Option to set the path to the vhost_user unix socket files. + + NOTE: Changing any of these options requires restarting the ovs-vswitchd + application. + + Open vSwitch can be started as normal. DPDK will not be initialized until + the first DPDK-enabled port is added to the bridge. ``` export DB_SOCK=/usr/local/var/run/openvswitch/db.sock - ovs-vswitchd --dpdk -c 0x1 -n 4 -- unix:$DB_SOCK --pidfi
[ovs-dev] [PATCH v5 3/4] netdev-dpdk: Autofill lcore coremask if absent
The user has control over the DPDK internal lcore coremask, but this parameter can be autofilled with a bit more intelligence. If the user does not fill this parameter in, we use the lowest set bit in the current task CPU affinity. Otherwise, we will reassign the current thread to the specified lcore mask, in addition to the dpdk lcore threads. Signed-off-by: Aaron Conole Cc: Kevin Traynor --- v2: * Fix a conditional branch coding standard issue * When lcore coremask is set, do not reset the affinities as suggested by Kevin Traynor v3: * Fixed grow_argv calls * Fixed an error in argc assignment after 'break;' introduced in v2 * Switched to using xstrdup v4,v5: * No change lib/netdev-dpdk.c | 58 --- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index b7bb3eb..29b3db2 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -65,6 +65,8 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE #define OVS_VPORT_DPDK "ovs_dpdk" +#define MAX_BUFSIZ 256 + /* * need to reserve tons of extra space in the mbufs so we can align the * DMA addresses to 4KB. @@ -2168,7 +2170,8 @@ grow_argv(char ***argv, size_t cur_siz, size_t grow_by) } static int -get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv) +get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv, + int argc) { struct dpdk_options_map { const char *ovs_configuration; @@ -2176,14 +2179,14 @@ get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv) bool default_enabled; const char *default_value; } opts[] = { -{"dpdk-lcore-mask", "-c", true, "0x1"}, +{"dpdk-lcore-mask", "-c", false, NULL}, /* XXX: DPDK 2.2.0 support, the true should become false for -n */ {"dpdk-mem-channels", "-n", true, "4"}, {"dpdk-alloc-mem", "-m", false, NULL}, {"dpdk-socket-mem", "--socket-mem", true, "1024,0"}, {"dpdk-hugepage-dir", "--huge-dir", false, NULL}, }; -int i, ret = 1; +int i, ret = argc; for(i = 0; i < (sizeof(opts) / sizeof(opts[0])); ++i) { const char *lookup = smap_get(&ovs_cfg->other_config, @@ -2226,7 +2229,8 @@ __dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg) { char **argv = NULL; int result; -int argc; +int argc = 0, argc_tmp; +bool auto_determine = true; int err; cpu_set_t cpuset; @@ -2260,12 +2264,41 @@ __dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg) ovs_abort(0, "Thread getaffinity error %d.", err); } -argv = grow_argv(&argv, 0, 1); +argv = grow_argv(&argv, argc, argc+1); if (!argv) { ovs_abort(0, "Unable to allocate an initial argv."); } -argv[0] = xstrdup("ovs"); /* TODO use prctl to get process name */ -argc = get_dpdk_args(ovs_cfg, &argv); +argv[argc++] = xstrdup("ovs"); /* TODO use prctl to get process name */ +argc_tmp = get_dpdk_args(ovs_cfg, &argv, argc); + +while(argc_tmp != argc) { +if (!strcmp("-c", argv[argc++])) { +auto_determine = false; +break; +} +} +argc = argc_tmp; + +/** + * NOTE: This is an unsophisticated mechanism for determining the DPDK + * lcore for the DPDK Master. + */ +if (auto_determine) { +int i; +for (i = 0; i < CPU_SETSIZE; i++) { +if (CPU_ISSET(i, &cpuset)) { +char buf[MAX_BUFSIZ]; +snprintf(buf, MAX_BUFSIZ, "0x%08llX", (1ULL
[ovs-dev] [PATCH v5 1/4] netdev-dpdk: Restore thread affinity after DPDK init
When the DPDK init function is called, it changes the executing thread's CPU affinity to a single core specified in -c. This will result in the userspace bridge configuration thread being rebound, even if that is not the intent. This change fixes that behavior by rebinding to the original thread affinity after calling dpdk_init(). Signed-off-by: Kevin Traynor Signed-off-by: Aaron Conole --- v2: * Removed trailing whitespace v3->v5: * No change lib/netdev-dpdk.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index de7e488..78f013d 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2142,6 +2142,9 @@ dpdk_init(int argc, char **argv) int result; int base = 0; char *pragram_name = argv[0]; +int err; +int isset; +cpu_set_t cpuset; if (argc < 2 || strcmp(argv[1], "--dpdk")) return 0; @@ -2183,6 +2186,14 @@ dpdk_init(int argc, char **argv) base = 2; } +/* Get the main thread affinity */ +CPU_ZERO(&cpuset); +err = pthread_getaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset); +if (err) { +VLOG_ERR("Thread getaffinity error %d.", err); +return err; +} + /* Keep the program name argument as this is needed for call to * rte_eal_init() */ @@ -2194,6 +2205,13 @@ dpdk_init(int argc, char **argv) ovs_abort(result, "Cannot init EAL"); } +/* Set the main thread affinity back to pre rte_eal_init() value */ +err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset); +if (err) { +VLOG_ERR("Thread setaffinity error %d", err); +return err; +} + rte_memzone_dump(stdout); rte_eal_init_ret = 0; -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v5 4/4] netdev-dpdk: Allow arbitrary eal arguments
A previous change moved some commonly used arguments from commandline to the database, and with it the ability to pass arbitrary arguments to EAL. This change allows arbitrary eal arguments to be provided via a new db entry 'other_config:dpdk-extra' which will tokenize the string and add it to the argument list. The only argument which will not be supported with this change is '--no-huge', which appears to break the system in other ways. Signed-off-by: Aaron Conole Suggested-by: Panu Matilainen --- v4: * Added by suggestion of Panu, making socket-mem non-default v5: * Keep the socket-mem as default parameter, and mention that we do not support --no-huge * Update ovs-dev.py with the new mechanism for passing arbitrary dpdk options INSTALL.DPDK.md | 5 + lib/netdev-dpdk.c| 50 +++--- utilities/ovs-dev.py | 6 -- vswitchd/vswitch.xml | 10 ++ 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 46bd1a8..e2d001a 100644 --- a/INSTALL.DPDK.md +++ b/INSTALL.DPDK.md @@ -178,6 +178,11 @@ Using the DPDK with ovs-vswitchd: * dpdk-hugepage-dir Directory where hugetlbfs is mounted + * dpdk-extra + Extra arguments to provide to DPDK EAL, as previously specified on the + command line. Do not pass '--no-huge' to the system in this way. Support + for running the system without hugepages is nonexistent. + * cuse-dev-name Option to set the vhost_cuse character device name. diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 29b3db2..e8b6b4b 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2166,13 +2166,34 @@ static char ** grow_argv(char ***argv, size_t cur_siz, size_t grow_by) { char **new_argv = realloc(*argv, sizeof(char *) * (cur_siz + grow_by)); +if (!new_argv) { +ovs_abort(0, "grow_argv() failed to allocate memory."); +} return new_argv; } static int +extra_dpdk_args(const char *ovs_cfg, char ***argv, int argc) +{ +int ret = argc; +char *release_tok = xstrdup(ovs_cfg); +char *tok = release_tok, *endptr = NULL; + +for(tok = strtok_r(release_tok, " ", &endptr); tok != NULL; +tok = strtok_r(NULL, " ", &endptr)) { +char **newarg = grow_argv(argv, ret, 1); +*argv = newarg; +(*argv)[ret++] = xstrdup(tok); +} +free(release_tok); +return ret; +} + +static int get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv, int argc) { +const char *extra_configuration; struct dpdk_options_map { const char *ovs_configuration; const char *dpdk_option; @@ -2196,17 +2217,17 @@ get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv, if(lookup) { char **newargv = grow_argv(argv, ret, 2); - -if (!newargv) { -ovs_abort(0, "grow_argv() failed to allocate memory."); -} - *argv = newargv; (*argv)[ret++] = xstrdup(opts[i].dpdk_option); (*argv)[ret++] = xstrdup(lookup); } } +extra_configuration = smap_get(&ovs_cfg->other_config, "dpdk-extra"); +if (extra_configuration) { +ret = extra_dpdk_args(extra_configuration, argv, ret); +} + return ret; } @@ -2265,17 +2286,15 @@ __dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg) } argv = grow_argv(&argv, argc, argc+1); -if (!argv) { -ovs_abort(0, "Unable to allocate an initial argv."); -} argv[argc++] = xstrdup("ovs"); /* TODO use prctl to get process name */ argc_tmp = get_dpdk_args(ovs_cfg, &argv, argc); while(argc_tmp != argc) { -if (!strcmp("-c", argv[argc++])) { +if (!strcmp("-c", argv[argc]) || !strcmp("-l", argv[argc])) { auto_determine = false; break; } +argc++; } argc = argc_tmp; @@ -2290,9 +2309,6 @@ __dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg) char buf[MAX_BUFSIZ]; snprintf(buf, MAX_BUFSIZ, "0x%08llX", (1ULL< + + + Specifies additional eal command line arguments for DPDK. + + + The default is empty. + + + -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVS PatchWork Update
Marked committed patches as 'Accepted' Marked duplicates as 'Not Applicable' I have a patch that automates the marking above, https://github.com/yew011/openvswitch.patchwork.mgmt/pull/1 Ben, can I merge it and do it in the weekly cron task? Thanks, Alex Wang, On 17 January 2016 at 16:01, wrote: > 30+ Day Old Patches > === > ID State Date Name > -- - > 548008 New2015-11-24 11:19:01 [ovs-dev,v4,2/4] ovn: New flows for > DHCP tranffic > 548010 New2015-11-24 11:19:02 [ovs-dev,v4,3/4] ovn: Process dhcp > packet-ins and respond through packet-outs > 548011 New2015-11-24 11:24:36 [ovs-dev,v4,4/4] ovn: Add tests for > ovn dhcp > 549307 New2015-11-27 06:07:22 [ovs-dev,1/2] datapath: Provide > this_cpu_{read, inc, dec} wrappers for v2.6.33 > 549306 New2015-11-27 06:07:23 [ovs-dev,2/2] datapath: test for > netlink_set_err returning void > 551762 New2015-12-02 20:54:21 [ovs-dev] Update windows datapath > build system > 554024 New2015-12-08 18:59:19 [ovs-dev,1/2] tests: Add vlog tests > for C implementation to match Python tests. > 554025 New2015-12-08 18:59:20 [ovs-dev,2/2] vlog: Add vlog/close > command. > 554171 New2015-12-09 01:08:08 [ovs-dev,05/14] Add OpenFlow support > for new "arp" action. > 554173 New2015-12-09 01:08:11 [ovs-dev,08/14] actions: Implement > OVN "arp" action. > 554175 New2015-12-09 01:08:13 [ovs-dev,10/14] ovn: Use callback > function instead of simap for logical port number map. > 554176 New2015-12-09 01:08:14 [ovs-dev,11/14] ovn-controller: Add > data structure for indexing lports, multicast groups. > 554177 New2015-12-09 01:08:15 [ovs-dev,12/14] expr: Generalize > wording of error message in expand_symbol(). > 554178 New2015-12-09 01:08:16 [ovs-dev,13/14] ovn: Reorganize > action parsing test. > 554179 New2015-12-09 01:08:17 [ovs-dev,14/14] ovn: Implement basic > ARP support for L3 logical routers. > 555209 New2015-12-10 16:58:46 [ovs-dev,v2] ovsdb-idl: Add support > for column tracking in IDL > 556183 New2015-12-13 14:16:52 [ovs-dev] ovn-northd: Recalculate db > mappings if the txn returns TXN_ERROR > 556572 New2015-12-14 17:35:24 [ovs-dev] INSTALL.DPDK.md: Clarify > DPDK arguments. > 558736 New2015-12-18 04:50:00 [ovs-dev,1/5] datapath: correct > {new,del}link compat code > 558738 New2015-12-18 04:50:02 [ovs-dev,3/5] datapath: provide > dst_init_metrics compat code prior to v2.6.39 > 558739 New2015-12-18 04:50:03 [ovs-dev,4/5] datapath: restrict > usage of tstats to kernels older than 2.6.37 > 558740 New2015-12-18 04:50:04 [ovs-dev,5/5] datapath: provide > dev_get_stats copat code for v2.6.35 > 558762 New2015-12-18 07:14:07 [ovs-dev] When deleting a group, > flows from table other than 0 are not removed. > 558905 New2015-12-18 15:33:35 [ovs-dev,11/14] openvswitch: use > list_for_each_entry > > Duplicate Patches in Patchwork > == > ID State Date Name > -- - > 566221 New2016-01-12 00:23:19 [ovs-dev] datapath: Fix deadlock on > STT device destroy. > 566563 New2016-01-12 13:48:44 [ovs-dev] ipv6 tunneling: Fix for > performance drop introduced by ipv6 tunnel support. > 562708 New2016-01-04 21:46:32 [ovs-dev,v2,1/3] netdev-dpdk: Restore > thread affinity after DPDK init > 562709 New2016-01-04 21:46:33 [ovs-dev,v2,2/3] netdev-dpdk: Convert > initialization from cmdline to db > 562710 New2016-01-04 21:46:34 [ovs-dev,v2,3/3] netdev-dpdk: > Autofill lcore coremask if absent > 567205 New2016-01-14 00:07:16 [ovs-dev] datapath: STT: Fix nf-hook > softlockup. > 566598 New2016-01-12 15:19:41 [ovs-dev,1/2] odp-util: Accept fields > with zero mask > 566599 New2016-01-12 15:19:42 [ovs-dev,2/2] ofproto: Wildcard TTL > on IP tunnels > 566756 New2016-01-12 22:26:03 [ovs-dev] ipv6 tunneling: Fix for > performance drop introduced by ipv6 tunnel support. > 566770 New2016-01-12 23:39:49 [ovs-dev,net] ovs: add recursion > limit to ovs_vport_receive > 567136 New2016-01-13 21:17:36 [ovs-dev,v3,1/3] netdev-dpdk: Restore > thread affinity after DPDK init > 567137 New2016-01-13 21:17:37 [ovs-dev,v3,2/3] netdev-dpdk: Convert > initialization from cmdline to db > 567138 New2016-01-13 21:17:38 [ovs-dev,v3,3/3] netdev-dpdk: > Autofill lcore coremask if absent > 563080 New2016-01-05 13:13:54 [ovs-dev,monitor_cond,01/12] ovsdb: > create column index mapping between ovsdb row to monitor row > 563091 New2016-01-05 13:13:55 [ovs-dev,monitor_cond,02/12] ovsdb: > add conditions utilities to support monitor_cond > 563083 New2016-01-05 13:13:56 [ovs-dev,monitor_cond,03/12] ovsdb: > generate update notifications for monitor_cond session > 563082 New2016-01-05 13:13:57 [ovs-dev,monitor_
[ovs-dev] [PATCH 00/41] Implement "closures".
The purpose of this series is the final patch, which is important for OVN and should also solve some problems that I've heard of in other controllers over the years. Ben Pfaff (41): ofproto: Fix memory leak and memory exhaustion bugs in group_mod. learning-switch: Use "if"s instead of "switch" to reduce maintenance. ofp-msgs: Fix definitions of OF1.4 OFPT_GET_ASYNC_REPLY and OFPT_SET_ASYNC. ofp-msgs: Fix comments. ofp-util: Improve formatting of comment. openflow-1.2: Remove unused struct definition. pinsched: Remove obsolete ofpbuf_trim(). ofp-print: Improve formatting of queue stat requests and port_mods. openflow: Rename OF0.1-1.3 queue property constants. netdev-dummy: Add a dummy queue. ovs-ofctl: Merge dump_stats_transaction() into dump_transaction(). ofp-actions: Append in ofpacts_pull_openflow_actions(), instead of replace. ofp-util: Improve function to emit a bitmap property. ofp-errors: Add extension error codes for OF1.3+ property errors. ofp-prop: New module for working with OpenFlow 1.3+ properties. ofp-prop: Add support for experimenter properties. ofp-prop: Add generic functions for working with 16- and 32-bit properties. ofp-prop: Add helpers for u8, be64/u64, flag, and UUID properties. ofp-prop: New function ofpprop_put_zeros(). openflow: Remove unused (and not useful) property headers. openflow: Implement OF1.4+ OFPMP_QUEUE_DESC multipart message. ofp-util: New function ofputil_async_msg_type_to_string(). ofp-util: Define struct ofputil_async_cfg to hold async message config. ofp-util: Rewrite async config encoding and decoding to be table-driven. ofp-util: Fix OF1.4+ version of ofputil_decode_set_async_config(). ofp-util: Add function to encode OFPT_SET_ASYNC messages. ofp-print: Decode all async config messages the same way. openflow: Get rid of struct ofp13_packet_in. fail-open: Drop some of the weirder special cases. pktbuf: Move from 'ofproto' to 'lib'. openflow: Better abstract handling of packet-in messages. connmgr: Generalize ofproto_packet_in to ofproto_async_msg. hash: New helper functions hash_bytes32() and hash_bytes64(). ofproto-dpif-rid: Use array instead of ofpbuf for recirc_state stack. ofproto-dpif-rid: Use separate pointers for actions and action set. ofproto-dpif-rid: Don't carry actset_output explicitly in metadata. ofproto-dpif-rid: Fix names of recirc_metadata_{hash,equal}(). ofproto-dpif-rid: Use UUID, not pointer, to identify ofprotos for recirc. nx-match: Add functions for raw decoding and encoding of OXM. ofproto-dpif-xlate: Put recirc_state, not recirc_id_node, in xlate_in. [RFC] Implement "closures". NEWS |8 +- OPENFLOW-1.1+.md |5 +- include/openflow/nicira-ext.h | 79 +- include/openflow/openflow-1.0.h| 37 +- include/openflow/openflow-1.2.h|9 - include/openflow/openflow-1.3.h| 104 +- include/openflow/openflow-1.4.h| 128 +-- include/openflow/openflow-1.5.h| 18 - include/openflow/openflow-common.h | 72 +- lib/automake.mk|6 +- lib/flow.h |5 +- lib/hash.h | 14 +- lib/learning-switch.c | 117 +-- lib/meta-flow.c|9 +- lib/meta-flow.h|3 +- lib/netdev-dummy.c | 99 +- lib/nx-match.c | 30 +- lib/nx-match.h |7 +- lib/odp-util.c |5 +- lib/ofp-actions.c | 91 +- lib/ofp-actions.h |9 +- lib/ofp-errors.h | 42 +- lib/ofp-msgs.c | 36 +- lib/ofp-msgs.h | 34 +- lib/ofp-print.c| 277 +++--- lib/ofp-prop.c | 445 + lib/ofp-prop.h | 130 +++ lib/ofp-util.c | 1937 +--- lib/ofp-util.h | 156 ++- lib/ofpbuf.h | 19 +- {ofproto => lib}/pktbuf.c | 44 +- {ofproto => lib}/pktbuf.h |3 +- lib/rconn.c|4 +- ofproto/automake.mk|4 +- ofproto/connmgr.c | 323 ++ ofproto/connmgr.h | 52 +- ofproto/fail-open.c| 29 +- ofproto/ofproto-dpif-rid.c | 68 +- ofproto/ofproto-dpif-rid.h | 25 +- ofproto/ofproto-dpif-upcall.c | 29 +- ofproto/ofproto-dpif-xlate.c | 269 +++-- ofproto/ofproto-dpif-xlate.h |9 +- ofproto/ofproto-dpif.c | 108 +- ofproto/ofproto-dpif.h | 10 +- ofproto/ofproto-provider.h |5 +- ofproto/ofproto.c | 178 ++-- ofproto/pinsched.c | 13 +- ovn/controller/pinctrl.c |4 +- tests/ofp-print.at
[ovs-dev] [PATCH 01/41] ofproto: Fix memory leak and memory exhaustion bugs in group_mod.
In handle_group_mod() cases where adding a group failed, nothing freed the list of buckets, causing a leak. The same was true in every case of modifying a group. This commit fixes the problem by changing add_group() to never steal or free the buckets (modify_group() already acted this way) and then making handle_group_mod() always free the buckets when it's done. This approach might at first raise objections, because it makes add_group() copy the buckets instead of just take the existing ones. But it actually fixes a worse problem too: when OF1.4+ REQUESTFORWARD is enabled, the group_mod is reused for the request forwarding. Until now, for a group_mod that adds a new group and that has some buckets, the previous stealing of buckets in add_group() meant that the group_mod's buckets were no longer valid; in practice, the list of buckets became linked in a way that iteration never terminated, which caused memory to be exhausted while composing the requestforward message. By making add_group() no longer modify the group_mod, we also fix this problem. The requestforward test in the testsuite did not find the latter problem because it only added a group without any buckets. This commit also updates the testsuite to include a bucket in its group_mod, which would have found the problem. Found by pain and suffering. Signed-off-by: Ben Pfaff --- ofproto/ofproto.c | 17 +++-- tests/ofproto.at | 16 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 528d5ac..386009e 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -297,7 +297,8 @@ static bool ofproto_group_exists__(const struct ofproto *ofproto, static bool ofproto_group_exists(const struct ofproto *ofproto, uint32_t group_id) OVS_EXCLUDED(ofproto->groups_rwlock); -static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *); +static enum ofperr add_group(struct ofproto *, + const struct ofputil_group_mod *); static void handle_openflow(struct ofconn *, const struct ofpbuf *); static enum ofperr ofproto_flow_mod_start(struct ofproto *, struct ofproto_flow_mod *) @@ -6280,7 +6281,7 @@ handle_queue_get_config_request(struct ofconn *ofconn, } static enum ofperr -init_group(struct ofproto *ofproto, struct ofputil_group_mod *gm, +init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm, struct ofgroup **ofgroup) { enum ofperr error; @@ -6306,7 +6307,9 @@ init_group(struct ofproto *ofproto, struct ofputil_group_mod *gm, *CONST_CAST(long long int *, &((*ofgroup)->modified)) = now; ovs_refcount_init(&(*ofgroup)->ref_count); -list_move(&(*ofgroup)->buckets, &gm->buckets); +list_init(&(*ofgroup)->buckets); +ofputil_bucket_clone_list(&(*ofgroup)->buckets, &gm->buckets, NULL); + *CONST_CAST(uint32_t *, &(*ofgroup)->n_buckets) = list_size(&(*ofgroup)->buckets); @@ -6326,7 +6329,7 @@ init_group(struct ofproto *ofproto, struct ofputil_group_mod *gm, * 'ofproto''s group table. Returns 0 on success or an OpenFlow error code on * failure. */ static enum ofperr -add_group(struct ofproto *ofproto, struct ofputil_group_mod *gm) +add_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm) { struct ofgroup *ofgroup; enum ofperr error; @@ -6474,7 +6477,7 @@ copy_buckets_for_remove_bucket(const struct ofgroup *ofgroup, * ofproto's ofgroup hash map. Thus, the group is never altered while users of * the xlate module hold a pointer to the group. */ static enum ofperr -modify_group(struct ofproto *ofproto, struct ofputil_group_mod *gm) +modify_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm) { struct ofgroup *ofgroup, *new_ofgroup, *retiring; enum ofperr error; @@ -6643,7 +6646,7 @@ handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh) VLOG_INFO_RL(&rl, "%s: Invalid group_mod command type %d", ofproto->name, gm.command); } -return OFPERR_OFPGMFC_BAD_COMMAND; +error = OFPERR_OFPGMFC_BAD_COMMAND; } if (!error) { @@ -6653,6 +6656,8 @@ handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh) rf.group_mod = &gm; connmgr_send_requestforward(ofproto->connmgr, ofconn, &rf); } +ofputil_bucket_list_destroy(&gm.buckets); + return error; } diff --git a/tests/ofproto.at b/tests/ofproto.at index b3b8a0f..787def1 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -3144,25 +3144,25 @@ check_async () { shift # OFPGC_ADD -ovs-appctl -t `pwd`/c2 ofctl/send 050f00120001 +ovs-appctl -t `pwd`/c2 ofctl/send "050f00220001 0010 " if test X"$1" = X"OFPGC_ADD"; then shift; echo >>expout2 "send: OFPT_GROUP_MO
[ovs-dev] [PATCH 02/41] learning-switch: Use "if"s instead of "switch" to reduce maintenance.
This code only cares about a very few kinds of OpenFlow messages, and it's unlikely that it will care about new ones, so replace the "switch" by "if" statements so that GCC won't complain about every new message. Signed-off-by: Ben Pfaff --- lib/learning-switch.c | 99 +-- 1 file changed, 9 insertions(+), 90 deletions(-) diff --git a/lib/learning-switch.c b/lib/learning-switch.c index b3f79ea..2b764f6 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -354,12 +354,9 @@ lswitch_process_packet(struct lswitch *sw, const struct ofpbuf *msg) return; } -switch (type) { -case OFPTYPE_ECHO_REQUEST: +if (type == OFPTYPE_ECHO_REQUEST) { process_echo_request(sw, msg->data); -break; - -case OFPTYPE_FEATURES_REPLY: +} else if (type == OFPTYPE_FEATURES_REPLY) { if (sw->state == S_FEATURES_REPLY) { if (!process_switch_features(sw, msg->data)) { sw->state = S_SWITCHING; @@ -367,93 +364,15 @@ lswitch_process_packet(struct lswitch *sw, const struct ofpbuf *msg) rconn_disconnect(sw->rconn); } } -break; - -case OFPTYPE_PACKET_IN: +} else if (type == OFPTYPE_PACKET_IN) { process_packet_in(sw, msg->data); -break; - -case OFPTYPE_FLOW_REMOVED: +} else if (type == OFPTYPE_FLOW_REMOVED) { /* Nothing to do. */ -break; - -case OFPTYPE_HELLO: -case OFPTYPE_ERROR: -case OFPTYPE_ECHO_REPLY: -case OFPTYPE_FEATURES_REQUEST: -case OFPTYPE_GET_CONFIG_REQUEST: -case OFPTYPE_GET_CONFIG_REPLY: -case OFPTYPE_SET_CONFIG: -case OFPTYPE_PORT_STATUS: -case OFPTYPE_PACKET_OUT: -case OFPTYPE_FLOW_MOD: -case OFPTYPE_GROUP_MOD: -case OFPTYPE_PORT_MOD: -case OFPTYPE_TABLE_MOD: -case OFPTYPE_BARRIER_REQUEST: -case OFPTYPE_BARRIER_REPLY: -case OFPTYPE_QUEUE_GET_CONFIG_REQUEST: -case OFPTYPE_QUEUE_GET_CONFIG_REPLY: -case OFPTYPE_DESC_STATS_REQUEST: -case OFPTYPE_DESC_STATS_REPLY: -case OFPTYPE_FLOW_STATS_REQUEST: -case OFPTYPE_FLOW_STATS_REPLY: -case OFPTYPE_AGGREGATE_STATS_REQUEST: -case OFPTYPE_AGGREGATE_STATS_REPLY: -case OFPTYPE_TABLE_STATS_REQUEST: -case OFPTYPE_TABLE_STATS_REPLY: -case OFPTYPE_PORT_STATS_REQUEST: -case OFPTYPE_PORT_STATS_REPLY: -case OFPTYPE_QUEUE_STATS_REQUEST: -case OFPTYPE_QUEUE_STATS_REPLY: -case OFPTYPE_PORT_DESC_STATS_REQUEST: -case OFPTYPE_PORT_DESC_STATS_REPLY: -case OFPTYPE_ROLE_REQUEST: -case OFPTYPE_ROLE_REPLY: -case OFPTYPE_ROLE_STATUS: -case OFPTYPE_REQUESTFORWARD: -case OFPTYPE_SET_FLOW_FORMAT: -case OFPTYPE_FLOW_MOD_TABLE_ID: -case OFPTYPE_SET_PACKET_IN_FORMAT: -case OFPTYPE_FLOW_AGE: -case OFPTYPE_SET_CONTROLLER_ID: -case OFPTYPE_FLOW_MONITOR_STATS_REQUEST: -case OFPTYPE_FLOW_MONITOR_STATS_REPLY: -case OFPTYPE_FLOW_MONITOR_CANCEL: -case OFPTYPE_FLOW_MONITOR_PAUSED: -case OFPTYPE_FLOW_MONITOR_RESUMED: -case OFPTYPE_GET_ASYNC_REQUEST: -case OFPTYPE_GET_ASYNC_REPLY: -case OFPTYPE_SET_ASYNC_CONFIG: -case OFPTYPE_METER_MOD: -case OFPTYPE_GROUP_STATS_REQUEST: -case OFPTYPE_GROUP_STATS_REPLY: -case OFPTYPE_GROUP_DESC_STATS_REQUEST: -case OFPTYPE_GROUP_DESC_STATS_REPLY: -case OFPTYPE_GROUP_FEATURES_STATS_REQUEST: -case OFPTYPE_GROUP_FEATURES_STATS_REPLY: -case OFPTYPE_METER_STATS_REQUEST: -case OFPTYPE_METER_STATS_REPLY: -case OFPTYPE_METER_CONFIG_STATS_REQUEST: -case OFPTYPE_METER_CONFIG_STATS_REPLY: -case OFPTYPE_METER_FEATURES_STATS_REQUEST: -case OFPTYPE_METER_FEATURES_STATS_REPLY: -case OFPTYPE_TABLE_FEATURES_STATS_REQUEST: -case OFPTYPE_TABLE_FEATURES_STATS_REPLY: -case OFPTYPE_TABLE_DESC_REQUEST: -case OFPTYPE_TABLE_DESC_REPLY: -case OFPTYPE_BUNDLE_CONTROL: -case OFPTYPE_BUNDLE_ADD_MESSAGE: -case OFPTYPE_NXT_TLV_TABLE_MOD: -case OFPTYPE_NXT_TLV_TABLE_REQUEST: -case OFPTYPE_NXT_TLV_TABLE_REPLY: -default: -if (VLOG_IS_DBG_ENABLED()) { -char *s = ofp_to_string(msg->data, msg->size, 2); -VLOG_DBG_RL(&rl, "%016llx: OpenFlow packet ignored: %s", -sw->datapath_id, s); -free(s); -} +} else if (VLOG_IS_DBG_ENABLED()) { +char *s = ofp_to_string(msg->data, msg->size, 2); +VLOG_DBG_RL(&rl, "%016llx: OpenFlow packet ignored: %s", +sw->datapath_id, s); +free(s); } } -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 05/41] ofp-util: Improve formatting of comment.
Signed-off-by: Ben Pfaff --- lib/ofp-util.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ofp-util.h b/lib/ofp-util.h index f51f9b8..f411651 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -1317,8 +1317,7 @@ void ofputil_uninit_tlv_table(struct ovs_list *mappings); enum ofputil_async_msg_type { OAM_PACKET_IN, /* OFPT_PACKET_IN or NXT_PACKET_IN. */ OAM_PORT_STATUS,/* OFPT_PORT_STATUS. */ -OAM_FLOW_REMOVED, /* OFPT_FLOW_REMOVED or - * NXT_FLOW_REMOVED. */ +OAM_FLOW_REMOVED, /* OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED. */ OAM_ROLE_STATUS,/* OFPT_ROLE_STATUS. */ OAM_TABLE_STATUS, /* OFPT_TABLE_STATUS. */ OAM_REQUESTFORWARD, /* OFPT_REQUESTFORWARD. */ -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 07/41] pinsched: Remove obsolete ofpbuf_trim().
This call to ofpbuf_trim() comes from a time when the packets passed to pinsched came directly from a dpif. For some time now that's no longer true--now they are messages generated by ofputil_encode_packet_in(), which generally are well sized and do not benefit from trimming. This is not a bug fix--the code is equally correct either way, it's only the rationale for trimming that's obsolete. Signed-off-by: Ben Pfaff --- ofproto/pinsched.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/ofproto/pinsched.c b/ofproto/pinsched.c index d81c9b3..c7118a6 100644 --- a/ofproto/pinsched.c +++ b/ofproto/pinsched.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -195,18 +195,11 @@ pinsched_send(struct pinsched *ps, ofp_port_t port_no, list_push_back(txq, &packet->list_node); } else { /* Otherwise queue it up for the periodic callback to drain out. */ -struct pinqueue *q; - -/* We might be called with a buffer obtained from dpif_recv() that has - * much more allocated space than actual content most of the time. - * Since we're going to store the packet for some time, free up that - * otherwise wasted space. */ -ofpbuf_trim(packet); - if (ps->n_queued * 1000 >= ps->token_bucket.burst) { drop_packet(ps); } -q = pinqueue_get(ps, port_no); + +struct pinqueue *q = pinqueue_get(ps, port_no); list_push_back(&q->packets, &packet->list_node); q->n++; ps->n_queued++; -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 03/41] ofp-msgs: Fix definitions of OF1.4 OFPT_GET_ASYNC_REPLY and OFPT_SET_ASYNC.
The structures declared in ofp-msgs.h for messages definitions should not include an OpenFlow header (its presence is implied), but the definition of these messages did. This commit fixes the definitions. The visible bug was really minor here: messages of these kinds without any TLVs would be rejected by the OpenFlow parser. But OVS never sends these messages without TLVs, so probably no one ever noticed this. (Also, the OVS support for OF1.4 is still incomplete and experimental.) Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.4.h | 11 --- lib/ofp-msgs.h | 6 +++--- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h index e600cff..daf6cf4 100644 --- a/include/openflow/openflow-1.4.h +++ b/include/openflow/openflow-1.4.h @@ -245,17 +245,6 @@ struct ofp14_async_config_prop_header { ovs_be16length; /* Length in bytes of this property. */ }; OFP_ASSERT(sizeof(struct ofp14_async_config_prop_header) == 4); - -/* Asynchronous message configuration. - * OFPT_GET_ASYNC_REPLY or OFPT_SET_ASYNC. - */ -struct ofp14_async_config { -struct ofp_header header; -/* Async config Property list - 0 or more */ -struct ofp14_async_config_prop_header properties[0]; -}; -OFP_ASSERT(sizeof(struct ofp14_async_config) == 8); - /* Request forward reason */ enum ofp14_requestforward_reason { OFPRFR_GROUP_MOD = 0, /* Forward group mod requests. */ diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 6770fa4..2c4a916 100644 --- a/lib/ofp-msgs.h +++ b/lib/ofp-msgs.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -232,13 +232,13 @@ enum ofpraw { OFPRAW_OFPT14_GET_ASYNC_REQUEST, /* OFPT 1.3 (27): struct ofp13_async_config. */ OFPRAW_OFPT13_GET_ASYNC_REPLY, -/* OFPT 1.4+ (27): struct ofp14_async_config, uint8_t[8][]. */ +/* OFPT 1.4+ (27): uint8_t[8][]. */ OFPRAW_OFPT14_GET_ASYNC_REPLY, /* OFPT 1.3 (28): struct ofp13_async_config. */ OFPRAW_OFPT13_SET_ASYNC, /* NXT 1.0+ (19): struct nx_async_config. */ OFPRAW_NXT_SET_ASYNC_CONFIG, -/* OFPT 1.4+ (28): struct ofp14_async_config, uint8_t[8][]. */ +/* OFPT 1.4+ (28): uint8_t[8][]. */ OFPRAW_OFPT14_SET_ASYNC, /* OFPT 1.3+ (29): struct ofp13_meter_mod, uint8_t[8][]. */ -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 04/41] ofp-msgs: Fix comments.
ofpbuf used to have members named 'frame' and 'l3' but now they're 'header' and 'msg'. Signed-off-by: Ben Pfaff --- lib/ofp-msgs.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c index 3a120e2..cb27f79 100644 --- a/lib/ofp-msgs.c +++ b/lib/ofp-msgs.c @@ -381,9 +381,9 @@ ofpraw_decode_assert(const struct ofp_header *oh) * * In addition to setting '*rawp', this function pulls off the OpenFlow header * (including the stats headers, vendor header, and any subtype header) with - * ofpbuf_pull(). It also sets 'msg->frame' to the start of the OpenFlow - * header and 'msg->msg' just beyond the headers (that is, to the final value of - * msg->data). */ + * ofpbuf_pull(). It also sets 'msg->header' to the start of the OpenFlow + * header and 'msg->msg' just beyond the headers (that is, to the final value + * of msg->data). */ enum ofperr ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg) { @@ -512,8 +512,8 @@ static void ofpraw_put__(enum ofpraw, uint8_t version, ovs_be32 xid, * Each 'raw' value is valid only for certain OpenFlow versions. The caller * must specify a valid (raw, version) pair. * - * In the returned ofpbuf, 'frame' points to the beginning of the - * OpenFlow header and 'l3' points just after it, to where the + * In the returned ofpbuf, 'header' points to the beginning of the + * OpenFlow header and 'msg' points just after it, to where the * message's body will start. The caller must actually allocate the * body into the space reserved for it, e.g. with ofpbuf_put_uninit(). * @@ -559,8 +559,8 @@ ofpraw_alloc_reply(enum ofpraw raw, const struct ofp_header *request, * value. Every stats request has a corresponding reply, so the (raw, version) * pairing pitfalls of the other ofpraw_alloc_*() functions don't apply here. * - * In the returned ofpbuf, 'frame' points to the beginning of the - * OpenFlow header and 'l3' points just after it, to where the + * In the returned ofpbuf, 'header' points to the beginning of the + * OpenFlow header and 'msg' points just after it, to where the * message's body will start. The caller must actually allocate the * body into the space reserved for it, e.g. with ofpbuf_put_uninit(). * @@ -592,9 +592,9 @@ ofpraw_alloc_stats_reply(const struct ofp_header *request, * Each 'raw' value is valid only for certain OpenFlow versions. The caller * must specify a valid (raw, version) pair. * - * Upon return, 'buf->frame' points to the beginning of the OpenFlow header and - * 'buf->msg' points just after it, to where the message's body will start. The - * caller must actually allocating the body into the space reserved for it, + * Upon return, 'buf->header' points to the beginning of the OpenFlow header + * and 'buf->msg' points just after it, to where the message's body will start. + * The caller must actually allocating the body into the space reserved for it, * e.g. with ofpbuf_put_uninit(). */ void ofpraw_put(enum ofpraw raw, uint8_t version, struct ofpbuf *buf) @@ -632,8 +632,8 @@ ofpraw_put_reply(enum ofpraw raw, const struct ofp_header *request, * value. Every stats request has a corresponding reply, so the (raw, version) * pairing pitfalls of the other ofpraw_alloc_*() functions don't apply here. * - * In the returned ofpbuf, 'frame' points to the beginning of the - * OpenFlow header and 'l3' points just after it, to where the + * In the returned ofpbuf, 'header' points to the beginning of the + * OpenFlow header and 'msg' points just after it, to where the * message's body will start. The caller must actually allocate the * body into the space reserved for it, e.g. with ofpbuf_put_uninit(). * @@ -801,9 +801,9 @@ ofptype_decode(enum ofptype *typep, const struct ofp_header *oh) * * In addition to setting '*typep', this function pulls off the OpenFlow header * (including the stats headers, vendor header, and any subtype header) with - * ofpbuf_pull(). It also sets 'msg->frame' to the start of the OpenFlow - * header and 'msg->msg' just beyond the headers (that is, to the final value of - * msg->data). */ + * ofpbuf_pull(). It also sets 'msg->header' to the start of the OpenFlow + * header and 'msg->msg' just beyond the headers (that is, to the final value + * of msg->data). */ enum ofperr ofptype_pull(enum ofptype *typep, struct ofpbuf *buf) { -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 06/41] openflow-1.2: Remove unused struct definition.
Experimenter stats are handled by code in ofp-msgs, and this struct isn't good for anything. Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.2.h | 9 - 1 file changed, 9 deletions(-) diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h index 65417d6..30e220c 100644 --- a/include/openflow/openflow-1.2.h +++ b/include/openflow/openflow-1.2.h @@ -172,15 +172,6 @@ enum ofp12_group_capabilities { OFPGFC12_CHAINING_CHECKS = 1 << 3, /* Check chaining for loops and delete */ }; -/* Body for ofp12_stats_request/reply of type OFPST_EXPERIMENTER. */ -struct ofp12_experimenter_stats_header { -ovs_be32 experimenter;/* Experimenter ID which takes the same form - as in struct ofp_experimenter_header. */ -ovs_be32 exp_type;/* Experimenter defined. */ -/* Experimenter-defined arbitrary additional data. */ -}; -OFP_ASSERT(sizeof(struct ofp12_experimenter_stats_header) == 8); - /* Role request and reply message. */ struct ofp12_role_request { ovs_be32 role;/* One of OFPCR12_ROLE_*. */ -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 15/41] ofp-prop: New module for working with OpenFlow 1.3+ properties.
Several OpenFlow 1.3+ messages use TLV-based properties that take a common form. Until now, ofp-util has had some static functions for dealing with properties. Because properties will start to be needed outside of ofp-util, this commit breaks them out into a new library, renaming them to begin with ofpprop_. The following commit will add a few new interfaces that add new functionality. Signed-off-by: Ben Pfaff --- include/openflow/openflow-common.h | 28 + lib/automake.mk| 2 + lib/ofp-prop.c | 125 +++ lib/ofp-prop.h | 71 +++ lib/ofp-util.c | 246 +++-- 5 files changed, 300 insertions(+), 172 deletions(-) create mode 100644 lib/ofp-prop.c create mode 100644 lib/ofp-prop.h diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h index f152718..d2efa5f 100644 --- a/include/openflow/openflow-common.h +++ b/include/openflow/openflow-common.h @@ -209,6 +209,34 @@ enum ofp_port_features { OFPPF_10GB_FD= 1 << 6, /* 10 Gb full-duplex rate support. */ }; +/* Generic OpenFlow property header, as used by various messages in OF1.3+, and + * especially in OF1.4. + * + * The OpenFlow specs prefer to define a new structure with a specialized name + * each time this property structure comes up: struct + * ofp_port_desc_prop_header, struct ofp_controller_status_prop_header, struct + * ofp_table_mod_prop_header, and more. They're all the same, so it's easier + * to unify them. + */ +struct ofp_prop_header { +ovs_be16 type; +ovs_be16 len; +}; +OFP_ASSERT(sizeof(struct ofp_prop_header) == 4); + +/* Generic OpenFlow experimenter property header. + * + * Again the OpenFlow specs define this over and over again and it's easier to + * unify them. */ +struct ofp_prop_experimenter { +ovs_be16 type; /* Generally 0x (in one case 0xfffe). */ +ovs_be16 len; /* Length in bytes of this property. */ +ovs_be32 experimenter; /* Experimenter ID which takes the same form as + * in struct ofp_experimenter_header. */ +ovs_be32 exp_type; /* Experimenter defined. */ +}; +OFP_ASSERT(sizeof(struct ofp_prop_experimenter) == 12); + /* Switch features. */ struct ofp_switch_features { ovs_be64 datapath_id; /* Datapath unique ID. The lower 48-bits are for diff --git a/lib/automake.mk b/lib/automake.mk index 4ff9fc0..39a652b 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -150,6 +150,8 @@ lib_libopenvswitch_la_SOURCES = \ lib/ofp-parse.h \ lib/ofp-print.c \ lib/ofp-print.h \ + lib/ofp-prop.c \ + lib/ofp-prop.h \ lib/ofp-util.c \ lib/ofp-util.h \ lib/ofp-version-opt.h \ diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c new file mode 100644 index 000..2d90e1b --- /dev/null +++ b/lib/ofp-prop.c @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2014, 2015 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "ofp-prop.h" + +#include "byte-order.h" +#include "ofpbuf.h" +#include "ofp-errors.h" +#include "util.h" + +/* Pulls a property, beginning with struct ofp_prop_header, from the beginning + * of 'msg'. Stores the type of the property in '*typep' and, if 'property' is + * nonnull, the entire property, including the header, in '*property'. Returns + * 0 if successful, otherwise an OpenFlow error code. + * + * This function pulls the property's stated size padded out to a multiple of + * 'alignment' bytes. The common case in OpenFlow is an 'alignment' of 8, so + * you can use ofpprop_pull() for that case. */ +enum ofperr +ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property, + unsigned int alignment, uint16_t *typep) +{ +struct ofp_prop_header *oph; +unsigned int padded_len; +unsigned int len; + +if (msg->size < sizeof *oph) { +return OFPERR_OFPBPC_BAD_LEN; +} + +oph = msg->data; +len = ntohs(oph->len); +padded_len = ROUND_UP(len, alignment); +if (len < sizeof *oph || padded_len > msg->size) { +return OFPERR_OFPBPC_BAD_LEN; +} + +*typep = ntohs(oph->type); +if (property) { +ofpbuf_use_const(property, msg->data, len); +} +ofpbuf_pull(msg, padded_len); +return 0; +} + +/* Pulls a property, beginning with struct ofp_prop_header, from the beginning + * of 'msg'. Stores
[ovs-dev] [PATCH 10/41] netdev-dummy: Add a dummy queue.
Until now it's been pretty hard to properly test any of the queue support, because the dummy network device doesn't have any queues. By adding one queue to the dummy network device (queue 0), we can get slightly higher confidence that OVS queue support works correctly. I suppose we could do even better if we made the dummy network device support modifying the queues, but that's a job for another day. Signed-off-by: Ben Pfaff --- lib/netdev-dummy.c | 99 ++ tests/ofproto.at | 83 +++-- 2 files changed, 136 insertions(+), 46 deletions(-) diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index df319de..c39f36c 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -1058,6 +1058,93 @@ netdev_dummy_get_stats(const struct netdev *netdev, struct netdev_stats *stats) } static int +netdev_dummy_get_queue(const struct netdev *netdev OVS_UNUSED, + unsigned int queue_id, struct smap *details) +{ +if (queue_id == 0) { +smap_add(details, "key", "value"); +return 0; +} else { +return EINVAL; +} +} + +static void +netdev_dummy_init_queue_stats(struct netdev_queue_stats *stats) +{ +*stats = (struct netdev_queue_stats) { +.tx_bytes = UINT64_MAX, +.tx_packets = UINT64_MAX, +.tx_errors = UINT64_MAX, +.created = LLONG_MIN, +}; +} + +static int +netdev_dummy_get_queue_stats(const struct netdev *netdev OVS_UNUSED, + unsigned int queue_id, + struct netdev_queue_stats *stats) +{ +if (queue_id == 0) { +netdev_dummy_init_queue_stats(stats); +return 0; +} else { +return EINVAL; +} +} + +struct netdev_dummy_queue_state { +unsigned int next_queue; +}; + +static int +netdev_dummy_queue_dump_start(const struct netdev *netdev OVS_UNUSED, + void **statep) +{ +struct netdev_dummy_queue_state *state = xmalloc(sizeof *state); +state->next_queue = 0; +*statep = state; +return 0; +} + +static int +netdev_dummy_queue_dump_next(const struct netdev *netdev OVS_UNUSED, + void *state_, + unsigned int *queue_id, struct smap *details) +{ +struct netdev_dummy_queue_state *state = state_; +if (state->next_queue == 0) { +*queue_id = 0; +smap_add(details, "key", "value"); +state->next_queue++; +return 0; +} else { +return EOF; +} +} + +static int +netdev_dummy_queue_dump_done(const struct netdev *netdev OVS_UNUSED, + void *state) +{ +free(state); +return 0; +} + +static int +netdev_dummy_dump_queue_stats(const struct netdev *netdev OVS_UNUSED, + void (*cb)(unsigned int queue_id, + struct netdev_queue_stats *, + void *aux), + void *aux) +{ +struct netdev_queue_stats stats; +netdev_dummy_init_queue_stats(&stats); +cb(0, &stats, aux); +return 0; +} + +static int netdev_dummy_get_ifindex(const struct netdev *netdev) { struct netdev_dummy *dev = netdev_dummy_cast(netdev); @@ -1147,14 +1234,14 @@ static const struct netdev_class dummy_class = { NULL, /* get_qos_capabilities */ NULL, /* get_qos */ NULL, /* set_qos */ -NULL, /* get_queue */ +netdev_dummy_get_queue, NULL, /* set_queue */ NULL, /* delete_queue */ -NULL, /* get_queue_stats */ -NULL, /* queue_dump_start */ -NULL, /* queue_dump_next */ -NULL, /* queue_dump_done */ -NULL, /* dump_queue_stats */ +netdev_dummy_get_queue_stats, +netdev_dummy_queue_dump_start, +netdev_dummy_queue_dump_next, +netdev_dummy_queue_dump_done, +netdev_dummy_dump_queue_stats, netdev_dummy_get_in4, /* get_in4 */ NULL, /* set_in4 */ diff --git a/tests/ofproto.at b/tests/ofproto.at index f29543e..a4064d4 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -178,58 +178,53 @@ OFPST_PORT_DESC reply (OF1.5): OVS_VSWITCHD_STOP AT_CLEANUP -dnl This is really bare-bones. -dnl It at least checks request and reply serialization and deserialization. -AT_SETUP([ofproto - queue stats - (OpenFlow 1.0)]) +dnl CHECK_QUEUE_STATS(label, option, format) +m4_define([CHECK_QUEUE_STATS], [ +AT_SETUP([ofproto - queue stats - (OpenFlow $1)]) OVS_VSWITCHD_START -AT_CHECK([ovs-ofctl -vwarn queue-stats br0], [0], [stdout]) -AT_CHECK([STRIP_XIDS stdout], [0], [dnl -OFPST_QUEUE reply: 0 queues -]) -AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ANY 5], [0], -
[ovs-dev] [PATCH 16/41] ofp-prop: Add support for experimenter properties.
Signed-off-by: Ben Pfaff --- lib/ofp-prop.c | 98 +++-- lib/ofp-prop.h | 57 lib/ofp-util.c | 134 - 3 files changed, 160 insertions(+), 129 deletions(-) diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c index 2d90e1b..83d72ab 100644 --- a/lib/ofp-prop.c +++ b/lib/ofp-prop.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2015 Nicira, Inc. + * Copyright (c) 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,8 +21,21 @@ #include "byte-order.h" #include "ofpbuf.h" #include "ofp-errors.h" +#include "openvswitch/vlog.h" #include "util.h" +static uint32_t +ofpprop_type_to_experimenter(uint64_t type) +{ +return type >> 32; +} + +static uint32_t +ofpprop_type_to_exp_id(uint64_t type) +{ +return type & UINT32_MAX; +} + /* Pulls a property, beginning with struct ofp_prop_header, from the beginning * of 'msg'. Stores the type of the property in '*typep' and, if 'property' is * nonnull, the entire property, including the header, in '*property'. Returns @@ -33,7 +46,8 @@ * you can use ofpprop_pull() for that case. */ enum ofperr ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property, - unsigned int alignment, uint16_t *typep) + unsigned int alignment, unsigned int min_exp, + uint64_t *typep) { struct ofp_prop_header *oph; unsigned int padded_len; @@ -50,9 +64,31 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property, return OFPERR_OFPBPC_BAD_LEN; } -*typep = ntohs(oph->type); +uint16_t type = ntohs(oph->type); +if (type < min_exp) { +*typep = type; +} else { +struct ofp_prop_experimenter *ope = msg->data; +if (len < sizeof *ope) { +return OFPERR_OFPBPC_BAD_LEN; +} + +if (!ope->experimenter) { +/* Reject experimenter 0 because it yields ambiguity with standard + * property types. */ +return OFPERR_OFPBPC_BAD_EXPERIMENTER; +} + +*typep = OFPPROP_EXP(ntohl(ope->experimenter), ntohl(ope->exp_type)); +} + if (property) { ofpbuf_use_const(property, msg->data, len); +msg->header = msg->data; +msg->msg = ((uint8_t *) msg->data ++ (type < min_exp + ? sizeof(struct ofp_prop_header) + : sizeof(struct ofp_prop_experimenter))); } ofpbuf_pull(msg, padded_len); return 0; @@ -66,15 +102,15 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property, * This function pulls the property's stated size padded out to a multiple of * 8 bytes, which is the common case for OpenFlow properties. */ enum ofperr -ofpprop_pull(struct ofpbuf *msg, struct ofpbuf *property, uint16_t *typep) +ofpprop_pull(struct ofpbuf *msg, struct ofpbuf *property, uint64_t *typep) { -return ofpprop_pull__(msg, property, 8, typep); +return ofpprop_pull__(msg, property, 8, 0x, typep); } /* Adds a property with the given 'type' and 'len'-byte contents 'value' to * 'msg', padding the property out to a multiple of 8 bytes. */ void -ofpprop_put(struct ofpbuf *msg, uint16_t type, const void *value, size_t len) +ofpprop_put(struct ofpbuf *msg, uint64_t type, const void *value, size_t len) { size_t start_ofs = ofpprop_start(msg, type); ofpbuf_put(msg, value, len); @@ -84,7 +120,7 @@ ofpprop_put(struct ofpbuf *msg, uint16_t type, const void *value, size_t len) /* Appends a property to 'msg' whose type is 'type' and whose contents is a * series of property headers, one for each 1-bit in 'bitmap'. */ void -ofpprop_put_bitmap(struct ofpbuf *msg, uint16_t type, uint64_t bitmap) +ofpprop_put_bitmap(struct ofpbuf *msg, uint64_t type, uint64_t bitmap) { size_t start_ofs = ofpprop_start(msg, type); for (; bitmap; bitmap = zero_rightmost_1bit(bitmap)) { @@ -98,14 +134,21 @@ ofpprop_put_bitmap(struct ofpbuf *msg, uint16_t type, uint64_t bitmap) * ofpprop_end(). Returns the offset of the beginning of the property (to pass * to ofpprop_end() later). */ size_t -ofpprop_start(struct ofpbuf *msg, uint16_t type) +ofpprop_start(struct ofpbuf *msg, uint64_t type) { size_t start_ofs = msg->size; -struct ofp_prop_header *oph; - -oph = ofpbuf_put_uninit(msg, sizeof *oph); -oph->type = htons(type); -oph->len = htons(4);/* May be updated later by ofpprop_end(). */ +if (!ofpprop_is_experimenter(type)) { +struct ofp_prop_header *oph = ofpbuf_put_uninit(msg, sizeof *oph); +oph->type = htons(type); +oph->len = htons(4); +} else { +struct ofp_prop_experimenter *ope += ofpbuf_put_uninit(msg, sizeof *ope); +ope->type = htons(0x); +ope->len = htons(12); +ope->experimenter
[ovs-dev] [PATCH 12/41] ofp-actions: Append in ofpacts_pull_openflow_actions(), instead of replace.
An upcoming commit will have a need to parse actions incrementally, so this change makes that easier to do. Signed-off-by: Ben Pfaff --- lib/ofp-actions.c | 20 lib/ofp-util.c| 2 ++ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index becf02d..ff3bc12 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -5631,12 +5631,9 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow, enum ofpact_type outer_action) { const struct ofp_action_header *actions; +size_t orig_size = ofpacts->size; enum ofperr error; -if (!outer_action) { -ofpbuf_clear(ofpacts); -} - if (actions_len % OFP_ACTION_ALIGN != 0) { VLOG_WARN_RL(&rl, "OpenFlow message actions length %u is not a " "multiple of %d", actions_len, OFP_ACTION_ALIGN); @@ -5653,21 +5650,21 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow, error = ofpacts_decode(actions, actions_len, version, ofpacts); if (error) { -ofpbuf_clear(ofpacts); +ofpacts->size = orig_size; return error; } error = ofpacts_verify(ofpacts->data, ofpacts->size, allowed_ovsinsts, outer_action); if (error) { -ofpbuf_clear(ofpacts); +ofpacts->size = orig_size; } return error; } -/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the - * front of 'openflow' into ofpacts. On success, replaces any existing content - * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'. +/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the front + * of 'openflow' into ofpacts. On success, appends the converted actions to + * 'ofpacts'; on failure, 'ofpacts' is unchanged (but might be reallocated) . * Returns 0 if successful, otherwise an OpenFlow error. * * Actions are processed according to their OpenFlow version which @@ -6229,6 +6226,7 @@ ofpacts_pull_openflow_instructions(struct ofpbuf *openflow, const struct ofp11_instruction *insts[N_OVS_INSTRUCTIONS]; enum ofperr error; +ofpbuf_clear(ofpacts); if (version == OFP10_VERSION) { return ofpacts_pull_openflow_actions__(openflow, instructions_len, version, @@ -6236,8 +6234,6 @@ ofpacts_pull_openflow_instructions(struct ofpbuf *openflow, ofpacts, 0); } -ofpbuf_clear(ofpacts); - if (instructions_len % OFP11_INSTRUCTION_ALIGN != 0) { VLOG_WARN_RL(&rl, "OpenFlow message instructions length %u is not a " "multiple of %d", diff --git a/lib/ofp-util.c b/lib/ofp-util.c index c1cbcea..58f7e23 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -3672,6 +3672,7 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, ofpbuf_use_const(&b, oh, ntohs(oh->length)); raw = ofpraw_pull_assert(&b); +ofpbuf_clear(ofpacts); if (raw == OFPRAW_OFPT11_PACKET_OUT) { enum ofperr error; const struct ofp11_packet_out *opo = ofpbuf_pull(&b, sizeof *opo); @@ -6265,6 +6266,7 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update, ofpraw_pull_assert(msg); } +ofpbuf_clear(ofpacts); if (!msg->size) { return EOF; } -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 13/41] ofp-util: Improve function to emit a bitmap property.
The callers had some common code that could be reasonably encapsulated, so this commit does so. Signed-off-by: Ben Pfaff --- lib/ofp-util.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 58f7e23..0c0b571 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -209,12 +209,16 @@ end_property(struct ofpbuf *msg, size_t start_ofs) ofpbuf_padto(msg, ROUND_UP(msg->size, 8)); } +/* Appends a property to 'msg' whose type is 'type' and whose contents is a + * series of property headers, one for each 1-bit in 'bitmap'. */ static void -put_bitmap_properties(struct ofpbuf *msg, uint64_t bitmap) +put_bitmap_property(struct ofpbuf *msg, uint16_t type, uint64_t bitmap) { +size_t start_ofs = start_property(msg, type); for (; bitmap; bitmap = zero_rightmost_1bit(bitmap)) { start_property(msg, rightmost_1bit_idx(bitmap)); } +end_property(msg, start_ofs); } /* Given the wildcard bit count in the least-significant 6 of 'wcbits', returns @@ -4932,14 +4936,9 @@ put_table_action_features(struct ofpbuf *reply, enum ofp13_table_feature_prop_type set_fields_type, int miss_offset, enum ofp_version version) { -size_t start_ofs; - -start_ofs = start_property(reply, actions_type + miss_offset); -put_bitmap_properties(reply, - ntohl(ofpact_bitmap_to_openflow(taf->ofpacts, - version))); -end_property(reply, start_ofs); - +put_bitmap_property(reply, actions_type + miss_offset, +ntohl(ofpact_bitmap_to_openflow(taf->ofpacts, +version))); put_fields_property(reply, &taf->set_fields, NULL, set_fields_type + miss_offset, version); } @@ -4952,11 +4951,9 @@ put_table_instruction_features( size_t start_ofs; uint8_t table_id; -start_ofs = start_property(reply, OFPTFPT13_INSTRUCTIONS + miss_offset); -put_bitmap_properties(reply, - ntohl(ovsinst_bitmap_to_openflow(tif->instructions, - version))); -end_property(reply, start_ofs); +put_bitmap_property(reply, OFPTFPT13_INSTRUCTIONS + miss_offset, +ntohl(ovsinst_bitmap_to_openflow(tif->instructions, + version))); start_ofs = start_property(reply, OFPTFPT13_NEXT_TABLES + miss_offset); BITMAP_FOR_EACH_1 (table_id, 255, tif->next) { -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 08/41] ofp-print: Improve formatting of queue stat requests and port_mods.
Without this, OFPST_QUEUE requests are formatted as: OFPST_QUEUE request:port=LOCAL queue=5 With this commit, OFPST_QUEUE requests are formatted as: OFPST_QUEUE request: port=LOCAL queue=5 which looks better. Similarly for OFPT_PORT_MOD. Signed-off-by: Ben Pfaff --- lib/ofp-print.c| 4 ++-- tests/ofp-print.at | 20 ++-- tests/ofproto.at | 12 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 5525aee..af56e9b 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -934,7 +934,7 @@ ofp_print_port_mod(struct ds *string, const struct ofp_header *oh) return; } -ds_put_cstr(string, "port: "); +ds_put_cstr(string, " port: "); ofputil_format_port(pm.port_no, string); ds_put_format(string, ": addr:"ETH_ADDR_FMT"\n", ETH_ADDR_ARGS(pm.hw_addr)); @@ -1714,7 +1714,7 @@ ofp_print_ofpst_queue_request(struct ds *string, const struct ofp_header *oh) return; } -ds_put_cstr(string, "port="); +ds_put_cstr(string, " port="); ofputil_format_port(oqsr.port_no, string); ds_put_cstr(string, " queue="); diff --git a/tests/ofp-print.at b/tests/ofp-print.at index ed9ffdb..6fae7f0 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -1037,7 +1037,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 01 0f 00 20 00 00 00 03 00 03 50 54 00 00 00 01 \ 00 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 \ " 3], [0], [dnl -OFPT_PORT_MOD (xid=0x3):port: 3: addr:50:54:00:00:00:01 +OFPT_PORT_MOD (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: UNCHANGED @@ -1051,7 +1051,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ 00 00 00 00 00 00 00 00 \ " 3], [0], [dnl -OFPT_PORT_MOD (OF1.1) (xid=0x3):port: 3: addr:50:54:00:00:00:01 +OFPT_PORT_MOD (OF1.1) (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: UNCHANGED @@ -1065,7 +1065,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ 00 00 00 00 00 00 00 00 \ " 3], [0], [dnl -OFPT_PORT_MOD (OF1.2) (xid=0x3):port: 3: addr:50:54:00:00:00:01 +OFPT_PORT_MOD (OF1.2) (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: UNCHANGED @@ -1079,7 +1079,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ 00 00 00 00 00 00 00 00 \ " 3], [0], [dnl -OFPT_PORT_MOD (OF1.3) (xid=0x3):port: 3: addr:50:54:00:00:00:01 +OFPT_PORT_MOD (OF1.3) (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: UNCHANGED @@ -1093,7 +1093,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ 00 00 00 08 00 00 00 01 " 3], [0], [dnl -OFPT_PORT_MOD (OF1.4) (xid=0x3):port: 3: addr:50:54:00:00:00:01 +OFPT_PORT_MOD (OF1.4) (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: 10MB-HD @@ -1755,7 +1755,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 01 10 00 14 00 00 00 01 00 05 00 00 ff fc 00 00 \ ff ff ff ff \ "], [0], [dnl -OFPST_QUEUE request (xid=0x1):port=ANY queue=ALL +OFPST_QUEUE request (xid=0x1): port=ANY queue=ALL ]) AT_CLEANUP @@ -1765,7 +1765,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 02 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \ ff ff ff ff ff ff ff ff \ "], [0], [dnl -OFPST_QUEUE request (OF1.1) (xid=0x2):port=ANY queue=ALL +OFPST_QUEUE request (OF1.1) (xid=0x2): port=ANY queue=ALL ]) AT_CLEANUP @@ -1775,7 +1775,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 03 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \ ff ff ff ff ff ff ff ff \ "], [0], [dnl -OFPST_QUEUE request (OF1.2) (xid=0x2):port=ANY queue=ALL +OFPST_QUEUE request (OF1.2) (xid=0x2): port=ANY queue=ALL ]) AT_CLEANUP @@ -1785,7 +1785,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ 04 12 00 18 00 00 00 02 00 05 00 00 00 00 00 00 \ ff ff ff ff ff ff ff ff \ "], [0], [dnl -OFPST_QUEUE request (OF1.3) (xid=0x2):port=ANY queue=ALL +OFPST_QUEUE request (OF1.3) (xid=0x2): port=ANY queue=ALL ]) AT_CLEANUP @@ -3383,7 +3383,7 @@ AT_CHECK([ovs-ofctl ofp-print "\ "], [0], [dnl OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x3): bundle_id=0x1 flags=atomic -OFPT_PORT_MOD (OF1.4) (xid=0x3):port: 3: addr:50:54:00:00:00:01 +OFPT_PORT_MOD (OF1.4) (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: 10MB-HD diff --git a/tests/ofproto.at b/tests/ofproto.at index 787def1..eceff58 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -188,11 +188,11 @@ OFPST_QUEUE reply: 0 queues ]) AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ANY 5], [0], [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_QUEUE -OFPST_QUEUE request (xid=0x2):port=ANY queue=5 +OFPST_QUEUE request (xid=0x2): port=ANY queue=5 ]) AT_CHECK([ovs-ofctl -vwarn queue-stats br0 10], [0], [OFPT_ER
[ovs-dev] [PATCH 11/41] ovs-ofctl: Merge dump_stats_transaction() into dump_transaction().
The callers call dump_stats_transaction() for OFPST_* messages and dump_transaction() for other messages, but the callee can easily distinguish the two types, so this commit eliminates the difference for the callers to simplify use. This will be more valuable in an upcoming commit in which a single ofputil_encode_*() function can produce an OFPT_* request for some OpenFlow versions and an OFPST_* request for others. (Specifically, OF1.4 changes OFPT_QUEUE_GET_CONFIG_REQUEST into OFPST_QUEUE_DESC.) Also merges dump_trivial_stats_transaction() into dump_trivial_transaction() for the same reason. Signed-off-by: Ben Pfaff --- utilities/ovs-ofctl.c | 123 ++ 1 file changed, 54 insertions(+), 69 deletions(-) diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 332a99d..4dbd4ed 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -532,78 +532,64 @@ send_openflow_buffer(struct vconn *vconn, struct ofpbuf *buffer) static void dump_transaction(struct vconn *vconn, struct ofpbuf *request) { -struct ofpbuf *reply; - -run(vconn_transact(vconn, request, &reply), "talking to %s", -vconn_get_name(vconn)); -ofp_print(stdout, reply->data, reply->size, verbosity + 1); -ofpbuf_delete(reply); -} - -static void -dump_trivial_transaction(const char *vconn_name, enum ofpraw raw) -{ -struct ofpbuf *request; -struct vconn *vconn; - -open_vconn(vconn_name, &vconn); -request = ofpraw_alloc(raw, vconn_get_version(vconn), 0); -dump_transaction(vconn, request); -vconn_close(vconn); -} +const struct ofp_header *oh = request->data; +if (ofpmsg_is_stat_request(oh)) { +ovs_be32 send_xid = oh->xid; +enum ofpraw request_raw; +enum ofpraw reply_raw; +bool done = false; -static void -dump_stats_transaction(struct vconn *vconn, struct ofpbuf *request) -{ -const struct ofp_header *request_oh = request->data; -ovs_be32 send_xid = request_oh->xid; -enum ofpraw request_raw; -enum ofpraw reply_raw; -bool done = false; - -ofpraw_decode_partial(&request_raw, request->data, request->size); -reply_raw = ofpraw_stats_request_to_reply(request_raw, - request_oh->version); +ofpraw_decode_partial(&request_raw, request->data, request->size); +reply_raw = ofpraw_stats_request_to_reply(request_raw, oh->version); -send_openflow_buffer(vconn, request); -while (!done) { -ovs_be32 recv_xid; -struct ofpbuf *reply; +send_openflow_buffer(vconn, request); +while (!done) { +ovs_be32 recv_xid; +struct ofpbuf *reply; -run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive failed"); -recv_xid = ((struct ofp_header *) reply->data)->xid; -if (send_xid == recv_xid) { -enum ofpraw raw; +run(vconn_recv_block(vconn, &reply), +"OpenFlow packet receive failed"); +recv_xid = ((struct ofp_header *) reply->data)->xid; +if (send_xid == recv_xid) { +enum ofpraw raw; -ofp_print(stdout, reply->data, reply->size, verbosity + 1); +ofp_print(stdout, reply->data, reply->size, verbosity + 1); -ofpraw_decode(&raw, reply->data); -if (ofptype_from_ofpraw(raw) == OFPTYPE_ERROR) { -done = true; -} else if (raw == reply_raw) { -done = !ofpmp_more(reply->data); +ofpraw_decode(&raw, reply->data); +if (ofptype_from_ofpraw(raw) == OFPTYPE_ERROR) { +done = true; +} else if (raw == reply_raw) { +done = !ofpmp_more(reply->data); +} else { +ovs_fatal(0, "received bad reply: %s", + ofp_to_string(reply->data, reply->size, +verbosity + 1)); +} } else { -ovs_fatal(0, "received bad reply: %s", - ofp_to_string(reply->data, reply->size, -verbosity + 1)); +VLOG_DBG("received reply with xid %08"PRIx32" " + "!= expected %08"PRIx32, recv_xid, send_xid); } -} else { -VLOG_DBG("received reply with xid %08"PRIx32" " - "!= expected %08"PRIx32, recv_xid, send_xid); +ofpbuf_delete(reply); } +} else { +struct ofpbuf *reply; + +run(vconn_transact(vconn, request, &reply), "talking to %s", +vconn_get_name(vconn)); +ofp_print(stdout, reply->data, reply->size, verbosity + 1); ofpbuf_delete(reply); } } static void -dump_trivial_stats_transaction(const char *vconn_name, enum ofpraw raw) +dump_trivial_transaction(const char *vconn_
[ovs-dev] [PATCH 09/41] openflow: Rename OF0.1-1.3 queue property constants.
At first glance, OF1.4 queue properties look a lot like those for OF1.0 to OF1.3, but in fact their different padding makes them incompatible. In addition, OF1.4 switches from using regular OpenFlow messages to request queue properties, to using multipart messages. Thus, we really need to use separate code to deal with OF1.4 queues. OF1.0, OF1.1, and OF1.2 all have slightly different queue config reply messages, but only OF1.0 and OF1.2 had tests, so this adds tests. (There is no test for OF1.3 because it's the same as OF1.2.) Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.0.h| 37 - include/openflow/openflow-common.h | 23 --- lib/ofp-util.c | 27 ++- tests/ofp-print.at | 15 +++ tests/ofproto.at | 14 ++ 5 files changed, 79 insertions(+), 37 deletions(-) diff --git a/include/openflow/openflow-1.0.h b/include/openflow/openflow-1.0.h index 54b15f2..db629f7 100644 --- a/include/openflow/openflow-1.0.h +++ b/include/openflow/openflow-1.0.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -137,6 +137,41 @@ struct ofp10_packet_queue { }; OFP_ASSERT(sizeof(struct ofp10_packet_queue) == 8); +/* Queue properties for OF1.0 to OF1.3. + * + * OF1.4+ use the same numbers but rename them and change the property formats + * in incompatible ways, so there's not much benefit to sharing the names. */ +enum ofp10_queue_properties { +/* Introduced in OF1.0. */ +OFPQT10_MIN_RATE = 1, /* Minimum datarate guaranteed. */ + +/* Introduced in OF1.1. */ +OFPQT11_MAX_RATE = 2, /* Maximum guaranteed rate. */ +OFPQT11_EXPERIMENTER = 0x, /* Experimenter defined property. */ +}; + +/* Description for a queue in OpenFlow 1.0 to 1.3. + * + * OF1.4+ also use a TLV format but an incompatible one. */ +struct ofp10_queue_prop_header { +ovs_be16 property; /* One of OFPQT*. */ +ovs_be16 len; /* Length of property, including this header. */ +uint8_t pad[4];/* 64-bit alignemnt. */ +}; +OFP_ASSERT(sizeof(struct ofp10_queue_prop_header) == 8); + +/* Min-Rate and Max-Rate queue property description (OFPQT10_MIN and + * OFPQT11_MAX). + * + * OF1.4+ use similar TLVs but they are incompatible due to different padding. + */ +struct ofp10_queue_prop_rate { +struct ofp10_queue_prop_header prop_header; +ovs_be16 rate;/* In 1/10 of a percent; >1000 -> disabled. */ +uint8_t pad[6]; /* 64-bit alignment */ +}; +OFP_ASSERT(sizeof(struct ofp10_queue_prop_rate) == 16); + /* Query for port queue configuration. */ struct ofp10_queue_get_config_request { ovs_be16 port; /* Port to be queried. Should refer diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h index 81f9120..f152718 100644 --- a/include/openflow/openflow-common.h +++ b/include/openflow/openflow-common.h @@ -209,29 +209,6 @@ enum ofp_port_features { OFPPF_10GB_FD= 1 << 6, /* 10 Gb full-duplex rate support. */ }; -enum ofp_queue_properties { -OFPQT_MIN_RATE = 1, /* Minimum datarate guaranteed. */ -OFPQT_MAX_RATE = 2, /* Maximum guaranteed rate. */ -OFPQT_EXPERIMENTER = 0x, /* Experimenter defined property. */ -}; - -/* Common description for a queue. */ -struct ofp_queue_prop_header { -ovs_be16 property; /* One of OFPQT_. */ -ovs_be16 len; /* Length of property, including this header. */ -uint8_t pad[4];/* 64-bit alignemnt. */ -}; -OFP_ASSERT(sizeof(struct ofp_queue_prop_header) == 8); - -/* Min-Rate and Max-Rate queue property description (OFPQT_MIN and - * OFPQT_MAX). */ -struct ofp_queue_prop_rate { -struct ofp_queue_prop_header prop_header; -ovs_be16 rate;/* In 1/10 of a percent; >1000 -> disabled. */ -uint8_t pad[6]; /* 64-bit alignment */ -}; -OFP_ASSERT(sizeof(struct ofp_queue_prop_rate) == 16); - /* Switch features. */ struct ofp_switch_features { ovs_be64 datapath_id; /* Datapath unique ID. The lower 48-bits are for diff --git a/lib/ofp-util.c b/lib/ofp-util.c index effd96a..c1cbcea 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -2554,11 +2554,11 @@ ofputil_encode_queue_get_config_reply(const struct ofp_header *oh) } static void -put_queue_rate(struct ofpbuf *reply, enum ofp_queue_properties property, - uint16_t rate) +put_ofp11_queue_rate(struct ofpbuf *reply, + enum ofp10_queue_properties property, uint16_t rate) { if (rate != UINT16_MAX) { -struct ofp_queue_prop_rate *oqpr; +struct ofp10_queue_prop_rate *oqpr; oqpr = ofpbuf_put_zeros(reply
[ovs-dev] [PATCH 17/41] ofp-prop: Add generic functions for working with 16- and 32-bit properties.
These will see increasing use in upcoming commits. Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.4.h | 27 - include/openflow/openflow-1.5.h | 18 lib/ofp-prop.c | 126 ++- lib/ofp-prop.h | 9 ++ lib/ofp-util.c | 218 ++-- lib/ofpbuf.h| 11 +- 6 files changed, 192 insertions(+), 217 deletions(-) diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h index daf6cf4..b65eeb8 100644 --- a/include/openflow/openflow-1.4.h +++ b/include/openflow/openflow-1.4.h @@ -94,15 +94,6 @@ enum ofp14_port_mod_prop_type { OFPPMPT14_EXPERIMENTER = 0x, /* Experimenter property. */ }; -/* Ethernet port mod property. */ -struct ofp14_port_mod_prop_ethernet { -ovs_be16 type; /* OFPPMPT14_ETHERNET. */ -ovs_be16 length; /* Length in bytes of this property. */ -ovs_be32 advertise; /* Bitmap of OFPPF_*. Zero all bits to prevent - any action taking place. */ -}; -OFP_ASSERT(sizeof(struct ofp14_port_mod_prop_ethernet) == 8); - struct ofp14_port_mod { ovs_be32 port_no; uint8_t pad[4]; @@ -137,13 +128,6 @@ enum ofp14_table_reason { OFPTR_N_REASONS/* Denotes number of reasons. */ }; -struct ofp14_table_mod_prop_eviction { -ovs_be16 type;/* OFPTMPT14_EVICTION. */ -ovs_be16 length; /* Length in bytes of this property. */ -ovs_be32 flags; /* Bitmap of OFPTMPEF14_* flags */ -}; -OFP_ASSERT(sizeof(struct ofp14_table_mod_prop_eviction) == 8); - struct ofp14_table_mod_prop_vacancy { ovs_be16 type; /* OFPTMPT14_VACANCY. */ ovs_be16 length; /* Length in bytes of this property. */ @@ -273,17 +257,6 @@ enum ofp14_async_config_prop_type { OFPTFPT_EXPERIMENTER_MASTER = 0x, /* Experimenter for master. */ }; -/* Various reason based properties */ -struct ofp14_async_config_prop_reasons { -/* 'type' is one of OFPACPT_PACKET_IN_*, OFPACPT_PORT_STATUS_*, - * OFPACPT_FLOW_REMOVED_*, OFPACPT_ROLE_STATUS_*, - * OFPACPT_TABLE_STATUS_*, OFPACPT_REQUESTFORWARD_*. */ -ovs_be16type; -ovs_be16length; /* Length in bytes of this property. */ -ovs_be32mask; /* Bitmasks of reason values. */ -}; -OFP_ASSERT(sizeof(struct ofp14_async_config_prop_reasons) == 8); - /* Experimenter async config property */ struct ofp14_async_config_prop_experimenter { ovs_be16type; /* One of OFPTFPT_EXPERIMENTER_SLAVE, diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h index b3deb2d..0c478d1 100644 --- a/include/openflow/openflow-1.5.h +++ b/include/openflow/openflow-1.5.h @@ -69,24 +69,6 @@ enum ofp15_group_bucket_prop_type { OFPGBPT15_EXPERIMENTER = 0x, /* Experimenter defined. */ }; -/* Group bucket weight property, for select groups only. */ -struct ofp15_group_bucket_prop_weight { -ovs_be16 type;/* OFPGBPT15_WEIGHT. */ -ovs_be16 length; /* 8. */ -ovs_be16 weight; /* Relative weight of bucket. */ -uint8_t pad[2]; /* Pad to 64 bits. */ -}; -OFP_ASSERT(sizeof(struct ofp15_group_bucket_prop_weight) == 8); - -/* Group bucket watch port or watch group property, for fast failover groups - * only. */ -struct ofp15_group_bucket_prop_watch { -ovs_be16 type;/* OFPGBPT15_WATCH_PORT or OFPGBPT15_WATCH_GROUP. */ -ovs_be16 length; /* 8. */ -ovs_be32 watch; /* The port or the group. */ -}; -OFP_ASSERT(sizeof(struct ofp15_group_bucket_prop_watch) == 8); - /* Bucket for use in groups. */ struct ofp15_bucket { ovs_be16 len; /* Length the bucket in bytes, including diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c index 83d72ab..4215e40 100644 --- a/lib/ofp-prop.c +++ b/lib/ofp-prop.c @@ -24,6 +24,21 @@ #include "openvswitch/vlog.h" #include "util.h" +struct ofp_prop_be16 { +ovs_be16 type; +ovs_be16 len; +ovs_be16 value; +uint8_t pad[2]; +}; +BUILD_ASSERT_DECL(sizeof(struct ofp_prop_be16) == 8); + +struct ofp_prop_be32 { +ovs_be16 type; +ovs_be16 len; +ovs_be32 value; +}; +BUILD_ASSERT_DECL(sizeof(struct ofp_prop_be32) == 8); + static uint32_t ofpprop_type_to_experimenter(uint64_t type) { @@ -84,11 +99,11 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property, if (property) { ofpbuf_use_const(property, msg->data, len); -msg->header = msg->data; -msg->msg = ((uint8_t *) msg->data -+ (type < min_exp - ? sizeof(struct ofp_prop_header) - : sizeof(struct ofp_prop_experimenter))); +property->header = property->data; +property->msg = ((uint8_t *) property->data + + (type < min_exp +? sizeof(struct ofp_prop_head
[ovs-dev] [PATCH 14/41] ofp-errors: Add extension error codes for OF1.3+ property errors.
Upcoming commits will introduce uses of the "property" message formats, which are used in OF1.3 and especially in OF1.4+, in Nicira extension messages for earlier versions of OpenFlow. Thus, it's best to also support the appropriate error codes in those versions of OpenFlow, so that errors can be reported in a useful way. Signed-off-by: Ben Pfaff --- lib/ofp-errors.h | 29 +++-- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h index 9aedb7c..4f59acf 100644 --- a/lib/ofp-errors.h +++ b/lib/ofp-errors.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -590,37 +590,46 @@ enum ofperr { /* ## OFPET_BAD_PROPERTY ## */ /* ## -- ## */ -/* OF1.3(13,2), OF1.4+(14,0). Unknown property type. +/* NX1.0-1.1(13,2), NX1.2(25), OF1.3(13,2), OF1.4+(14,0). Unknown property + * type. * * [Known as OFPTFFC_BAD_TYPE in OF1.3.] */ OFPERR_OFPBPC_BAD_TYPE, -/* OF1.3(13,3), OF1.4+(14,1). Length problem in property. +/* NX1.0-1.1(13,3), NX1.2(26), OF1.3(13,3), OF1.4+(14,1). Length problem + * in property. * * [Known as OFPTFFC_BAD_LEN in OF1.3.] */ OFPERR_OFPBPC_BAD_LEN, -/* OF1.3(13,4), OF1.4+(14,2). Unsupported property value. +/* NX1.0-1.1(13,4), NX1.2(27), OF1.3(13,4), OF1.4+(14,2). Unsupported + * property value. * * [Known as OFPTFFC_BAD_ARGUMENT in OF1.3.] */ OFPERR_OFPBPC_BAD_VALUE, -/* ONF1.3(4443), OF1.4+(14,3). Can't handle this many properties. */ +/* NX1.0-1.1(14,3), NX1.2(28), ONF1.3(4443), OF1.4+(14,3). Can't handle + * this many properties. */ OFPERR_OFPBPC_TOO_MANY, -/* ONF1.3(), OF1.4+(14,4). A property type was duplicated. */ +/* NX1.0-1.1(14,4), NX1.2(29), ONF1.3(), OF1.4+(14,4). A property type + * was duplicated. */ OFPERR_OFPBPC_DUP_TYPE, -/* ONF1.3(4445), OF1.4+(14,5). Unknown experimenter id specified. */ +/* NX1.0-1.1(14,5), NX1.2(30), ONF1.3(4445), OF1.4+(14,5). Unknown + * experimenter id specified. */ OFPERR_OFPBPC_BAD_EXPERIMENTER, -/* ONF1.3(4446), OF1.4+(14,6). Unknown exp_type for experimenter id. */ +/* NX1.0-1.1(14,6), NX1.2(31), ONF1.3(4446), OF1.4+(14,6). Unknown + * exp_type for experimenter id. */ OFPERR_OFPBPC_BAD_EXP_TYPE, -/* ONF1.3(4447), OF1.4+(14,7). Unknown value for experimenter id. */ +/* NX1.0-1.1(14,7), NX1.2(32), ONF1.3(4447), OF1.4+(14,7). Unknown value + * for experimenter id. */ OFPERR_OFPBPC_BAD_EXP_VALUE, -/* ONF1.3(4448), OF1.4+(14,8). Permissions error. */ +/* NX1.0-1.1(14,8), NX1.2(33), ONF1.3(4448), OF1.4+(14,8). Permissions + * error. */ OFPERR_OFPBPC_EPERM, /* ## -- ## */ -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 20/41] openflow: Remove unused (and not useful) property headers.
These are all just copies of the otherwise generic ofp_prop_header or ofp_prop_experimenter. Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.3.h | 83 - include/openflow/openflow-1.4.h | 45 +- 2 files changed, 1 insertion(+), 127 deletions(-) diff --git a/include/openflow/openflow-1.3.h b/include/openflow/openflow-1.3.h index cf93429..252e08e 100644 --- a/include/openflow/openflow-1.3.h +++ b/include/openflow/openflow-1.3.h @@ -215,13 +215,6 @@ struct ofp13_table_stats { }; OFP_ASSERT(sizeof(struct ofp13_table_stats) == 24); -/* Common header for all Table Feature Properties */ -struct ofp13_table_feature_prop_header { -ovs_be16type; /* One of OFPTFPT_*. */ -ovs_be16length; /* Length in bytes of this property. */ -}; -OFP_ASSERT(sizeof(struct ofp13_table_feature_prop_header) == 4); - /* Body for ofp_multipart_request of type OFPMP_TABLE_FEATURES./ * Body of reply to OFPMP_TABLE_FEATURES request. */ struct ofp13_table_features { @@ -269,82 +262,6 @@ enum ofp13_table_feature_prop_type { OFPTFPT13_EXPERIMENTER_MISS= 0x, /* Experimenter for table-miss. */ }; -/* Instructions property */ -struct ofp13_table_feature_prop_instructions { -ovs_be16type;/* One of OFPTFPT13_INSTRUCTIONS, -OFPTFPT13_INSTRUCTIONS_MISS. */ -ovs_be16length; /* Length in bytes of this property. */ -/* Followed by: - * - Exactly (length - 4) bytes containing the instruction ids, then - * - Exactly (length + 7)/8*8 - (length) (between 0 and 7) - * bytes of all-zero bytes */ -/* struct ofp11_instruction instruction_ids[0]; List of instructions - without any data */ -}; -OFP_ASSERT(sizeof(struct ofp13_table_feature_prop_instructions) == 4); - -/* Next Tables property */ -struct ofp13_table_feature_prop_next_tables { -ovs_be16type; /* One of OFPTFPT13_NEXT_TABLES, - OFPTFPT13_NEXT_TABLES_MISS. */ -ovs_be16length; /* Length in bytes of this property. */ -/* Followed by: - * - Exactly (length - 4) bytes containing the table_ids, then - * - Exactly (length + 7)/8*8 - (length) (between 0 and 7) - * bytes of all-zero bytes */ -/* uint8_t next_table_ids[0]; */ -}; -OFP_ASSERT(sizeof(struct ofp13_table_feature_prop_next_tables) == 4); - -/* Actions property */ -struct ofp13_table_feature_prop_actions { -ovs_be16type; /* One of OFPTFPT13_WRITE_ACTIONS, - OFPTFPT13_WRITE_ACTIONS_MISS, - OFPTFPT13_APPLY_ACTIONS, - OFPTFPT13_APPLY_ACTIONS_MISS. */ -ovs_be16length; /* Length in bytes of this property. */ -/* Followed by: - * - Exactly (length - 4) bytes containing the action_ids, then - * - Exactly (length + 7)/8*8 - (length) (between 0 and 7) - * bytes of all-zero bytes */ -/* struct ofp_action_header action_ids[0]; List of actions - without any data */ -}; -OFP_ASSERT(sizeof(struct ofp13_table_feature_prop_actions) == 4); - - -/* Match, Wildcard or Set-Field property */ -struct ofp13_table_feature_prop_oxm { -ovs_be16type; /* One of OFPTFPT13_MATCH, OFPTFPT13_WILDCARDS, - OFPTFPT13_WRITE_SETFIELD, - OFPTFPT13_WRITE_SETFIELD_MISS, - OFPTFPT13_APPLY_SETFIELD, - OFPTFPT13_APPLY_SETFIELD_MISS. */ -ovs_be16length; /* Length in bytes of this property. */ -/* Followed by: - * - Exactly (length - 4) bytes containing the oxm_ids, then - * - Exactly (length + 7)/8*8 - (length) (between 0 and 7) - * bytes of all-zero bytes */ -/* ovs_be32oxm_ids[0]; Array of OXM headers */ -}; -OFP_ASSERT(sizeof(struct ofp13_table_feature_prop_oxm) == 4); - -/* Experimenter table feature property */ -struct ofp13_table_feature_prop_experimenter { -ovs_be16type; /* One of OFPTFPT13_EXPERIMENTER, - OFPTFPT13_EXPERIMENTER_MISS. */ -ovs_be16length; /* Length in bytes of this property. */ -ovs_be32experimenter; /* Experimenter ID which takes the same form - as in struct ofp_experimenter_header. */ -ovs_be32exp_type; /* Experimenter defined. */ -/* Followed by: - * - Exactly (length - 12) bytes containing the experimenter data, then - * - Exactly (length + 7)/8*8 - (length) (between 0 and 7) - * bytes of all-zero bytes */ -/* ovs_be32experimenter_data[0]; */ -}; -OFP_ASSERT(sizeof(struct ofp13_table_feature_prop_experimenter) == 12); - /* Body of reply to OFPMP13_PORT request. If a counter is unsupported, set * the field to all ones. */ struct ofp13_port_stats { diff --git a/include/openflow/openflow-1.4.h
[ovs-dev] [PATCH 18/41] ofp-prop: Add helpers for u8, be64/u64, flag, and UUID properties.
These will have users in upcoming commits. Unlike the previously added helpers, there isn't any existing code that can immediately use them. Signed-off-by: Ben Pfaff --- lib/ofp-prop.c | 116 +++-- lib/ofp-prop.h | 10 + lib/ofpbuf.h | 8 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c index 4215e40..76031b6 100644 --- a/lib/ofp-prop.c +++ b/lib/ofp-prop.c @@ -23,6 +23,7 @@ #include "ofp-errors.h" #include "openvswitch/vlog.h" #include "util.h" +#include "uuid.h" struct ofp_prop_be16 { ovs_be16 type; @@ -111,8 +112,10 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf *property, /* Pulls a property, beginning with struct ofp_prop_header, from the beginning * of 'msg'. Stores the type of the property in '*typep' and, if 'property' is - * nonnull, the entire property, including the header, in '*property'. Returns - * 0 if successful, otherwise an error code. + * nonnull, the entire property, including the header, in '*property'. Points + * 'property->msg' to just past the property header (which could be + * ofp_prop_header or ofp_prop_experimenter) in 'property'. Returns 0 if + * successful, otherwise an error code. * * This function pulls the property's stated size padded out to a multiple of * 8 bytes, which is the common case for OpenFlow properties. */ @@ -153,6 +156,40 @@ ofpprop_parse_be32(const struct ofpbuf *property, ovs_be32 *value) return 0; } +/* Attempts to parse 'property' as a property containing a 64-bit value. If + * successful, stores the value into '*value' and returns 0; otherwise returns + * an OpenFlow error. */ +enum ofperr +ofpprop_parse_be64(const struct ofpbuf *property, ovs_be64 *value) +{ +size_t be64_offset = ROUND_UP(ofpbuf_headersize(property), 8); +if (property->size != be64_offset + 8) { +return OFPERR_OFPBPC_BAD_LEN; +} + +ovs_be64 *p = ALIGNED_CAST(ovs_be64 *, (char *) property->data + 8); +*value = *p; +return 0; +} + +/* Attempts to parse 'property' as a property containing a 8-bit value. If + * successful, stores the value into '*value' and returns 0; otherwise returns + * an OpenFlow error. */ +enum ofperr +ofpprop_parse_u8(const struct ofpbuf *property, uint8_t *value) +{ +/* OpenFlow 1.5 and earlier don't have any 8-bit properties, but it uses + * 8-byte properties for 16-bit values, which doesn't really make sense. + * Be forgiving by allowing any size payload as long as it's at least big + * enough. */ +uint8_t *p = property->msg; +if (ofpbuf_msgsize(property) < sizeof *p) { +return OFPERR_OFPBPC_BAD_LEN; +} +*value = *p; +return 0; +} + /* Attempts to parse 'property' as a property containing a 16-bit value. If * successful, stores the value into '*value' and returns 0; otherwise returns * an OpenFlow error. */ @@ -184,6 +221,36 @@ ofpprop_parse_u32(const struct ofpbuf *property, uint32_t *value) return 0; } +/* Attempts to parse 'property' as a property containing a 64-bit value. If + * successful, stores the value into '*value' and returns 0; otherwise returns + * an OpenFlow error. */ +enum ofperr +ofpprop_parse_u64(const struct ofpbuf *property, uint64_t *value) +{ +size_t be64_offset = ROUND_UP(ofpbuf_headersize(property), 8); +if (property->size != be64_offset + 8) { +return OFPERR_OFPBPC_BAD_LEN; +} + +ovs_be64 *p = ALIGNED_CAST(ovs_be64 *, (char *) property->data + 8); +*value = ntohll(*p); +return 0; +} + +/* Attempts to parse 'property' as a property containing a UUID. If + * successful, stores the value into '*uuid' and returns 0; otherwise returns + * an OpenFlow error. */ +enum ofperr +ofpprop_parse_uuid(const struct ofpbuf *property, struct uuid *uuid) +{ +struct uuid *p = property->msg; +if (ofpbuf_msgsize(property) != sizeof *p) { +return OFPERR_OFPBPC_BAD_LEN; +} +*uuid = *p; +return 0; +} + /* Adds a property with the given 'type' and 'len'-byte contents 'value' to * 'msg', padding the property out to a multiple of 8 bytes. */ void @@ -218,6 +285,25 @@ ofpprop_put_be32(struct ofpbuf *msg, uint64_t type, ovs_be32 value) ofpprop_put(msg, type, &value, sizeof value); } +/* Adds a property with the given 'type' and 64-bit 'value' to 'msg'. */ +void +ofpprop_put_be64(struct ofpbuf *msg, uint64_t type, ovs_be64 value) +{ +size_t start = ofpprop_start(msg, type); +ofpbuf_put_zeros(msg, 4); +ofpbuf_put(msg, &value, sizeof value); +ofpprop_end(msg, start); +} + +/* Adds a property with the given 'type' and 8-bit 'value' to 'msg'. */ +void +ofpprop_put_u8(struct ofpbuf *msg, uint64_t type, uint8_t value) +{ +/* There's no precedent for 8-bit properties in OpenFlow 1.5 and earlier + * but let's assume they're done sanely. */ +ofpprop_put(msg, type, &value, 1); +} + /* Adds a property with the given 'type' and 16-
[ovs-dev] [PATCH 21/41] openflow: Implement OF1.4+ OFPMP_QUEUE_DESC multipart message.
OpenFlow 1.0 through 1.3 have a message OFPT_QUEUE_GET_CONFIG_REQUEST and its corresponding reply, for fetching a description of the queues configured on a given port. OpenFlow 1.4 changes this message to a multipart message OFPMP_QUEUE_DESC, which Open vSwitch has not until now implemented. This commit adds an implemntation of that message. Because the message is a replacement for the former one, this commit implements it using the same ofp-util functions as the former message, so that the client code doesn't have to distinguish a difference between versions. The ovs-ofctl command queue-get-config was previously undocumented (due only to an oversight). This commit corrects that and documents the new feature available with OpenFlow 1.4. Signed-off-by: Ben Pfaff --- NEWS| 4 +- OPENFLOW-1.1+.md| 4 +- include/openflow/openflow-1.4.h | 25 lib/ofp-errors.h| 3 + lib/ofp-msgs.h | 15 +- lib/ofp-print.c | 32 +++-- lib/ofp-util.c | 305 +++- lib/ofp-util.h | 16 +-- ofproto/ofproto.c | 116 +-- tests/ofp-print.at | 44 ++ tests/ofproto.at| 46 -- utilities/ovs-ofctl.8.in| 12 +- utilities/ovs-ofctl.c | 20 +-- 13 files changed, 453 insertions(+), 189 deletions(-) diff --git a/NEWS b/NEWS index 4433329..5c18867 100644 --- a/NEWS +++ b/NEWS @@ -4,7 +4,9 @@ Post-v2.5.0 * New "monitor2" and "update2" extensions to RFC 7047. - OpenFlow: * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY. - + * OpenFlow 1.4+ OFPMP_QUEUE_DESC is now supported. + - ovs-ofctl: + * queue-get-config command now allows a queue ID to be specified. v2.5.0 - xx xxx - diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md index 0b2f0f9..537f660 100644 --- a/OPENFLOW-1.1+.md +++ b/OPENFLOW-1.1+.md @@ -194,8 +194,8 @@ OpenFlow 1.4 features are listed in the previous section. Many on-wire structures got TLVs. Already implemented: port desc properties, port mod properties, port stats properties, table mod properties, - queue stats, unified property errors. -Remaining required: set-async, queue desc + queue stats, unified property errors, queue desc. +Remaining required: set-async Remaining optional: table desc, table-status [EXT-262] [required for OF1.4+] diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h index bbdd54f..bce1598 100644 --- a/include/openflow/openflow-1.4.h +++ b/include/openflow/openflow-1.4.h @@ -219,6 +219,31 @@ struct ofp14_queue_stats { OFP_ASSERT(sizeof(struct ofp14_queue_stats) == 48); +/* ## ## */ +/* ## ofp14_queue_desc ## */ +/* ## ## */ + +struct ofp14_queue_desc_request { +ovs_be32 port; /* All ports if OFPP_ANY. */ +ovs_be32 queue; /* All queues if OFPQ_ALL. */ +}; +OFP_ASSERT(sizeof(struct ofp14_queue_desc_request) == 8); + +/* Body of reply to OFPMP_QUEUE_DESC request. */ +struct ofp14_queue_desc { +ovs_be32 port_no; /* Port this queue is attached to. */ +ovs_be32 queue_id; /* ID for the specific queue. */ +ovs_be16 len; /* Length in bytes of this queue desc. */ +uint8_t pad[6]; /* 64-bit alignment. */ +}; +OFP_ASSERT(sizeof(struct ofp14_queue_desc) == 16); + +enum ofp14_queue_desc_prop_type { +OFPQDPT14_MIN_RATE = 1, +OFPQDPT14_MAX_RATE = 2, +OFPQDPT14_EXPERIMENTER = 0x +}; + /* ## -- ## */ /* ## Miscellaneous. ## */ /* ## -- ## */ diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h index 4f59acf..8e13873 100644 --- a/lib/ofp-errors.h +++ b/lib/ofp-errors.h @@ -504,6 +504,9 @@ enum ofperr { /* OF1.0(5,2), OF1.1+(9,2). Permissions error. */ OFPERR_OFPQOFC_EPERM, +/* NX1.4+(23). System error retrieving queue details. */ +OFPERR_NXQOFC_QUEUE_ERROR, + /* ## -- ## */ /* ## OFPET_SWITCH_CONFIG_FAILED ## */ /* ## -- ## */ diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 2c4a916..54018b4 100644 --- a/lib/ofp-msgs.h +++ b/lib/ofp-msgs.h @@ -208,12 +208,12 @@ enum ofpraw { /* OFPT 1.0 (20): struct ofp10_queue_get_config_request. */ OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST, -/* OFPT 1.1+ (22): struct ofp11_queue_get_config_request. */ +/* OFPT 1.1-1.3 (22): struct ofp11_queue_get_config_request. */ OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST, /* OFPT 1.0 (21): struct ofp10_queue_get_config_reply, uint8_t[8][]. */ OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY, -/* OFPT 1.1+ (23): struct ofp11_queue_get_config_reply, uint8_t[8][]. */ +/* OFPT 1.1-1.3 (23): struct ofp11_queue_get_config_reply,
[ovs-dev] [PATCH 19/41] ofp-prop: New function ofpprop_put_zeros().
This will have additional users in later commits. Signed-off-by: Ben Pfaff --- lib/ofp-prop.c | 22 ++ lib/ofp-prop.h | 1 + lib/ofp-util.c | 20 ++-- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c index 76031b6..1641a39 100644 --- a/lib/ofp-prop.c +++ b/lib/ofp-prop.c @@ -261,6 +261,28 @@ ofpprop_put(struct ofpbuf *msg, uint64_t type, const void *value, size_t len) ofpprop_end(msg, start_ofs); } +/* Adds a property with the given 'type' to 'msg', consisting of a struct + * ofp_prop_header followed by enough zero bytes to total 'len' bytes, followed + * by padding to bring the property up to a multiple of 8 bytes. Returns the + * property header. */ +void * +ofpprop_put_zeros(struct ofpbuf *msg, uint64_t type, size_t len) +{ +void *header = ofpbuf_put_zeros(msg, ROUND_UP(len, 8)); +if (!ofpprop_is_experimenter(type)) { +struct ofp_prop_header *oph = header; +oph->type = htons(type); +oph->len = htons(len); +} else { +struct ofp_prop_experimenter *ope = header; +ope->type = htons(0x); +ope->len = htons(12); +ope->experimenter = htonl(ofpprop_type_to_experimenter(type)); +ope->exp_type = htonl(ofpprop_type_to_exp_id(type)); +} +return header; +} + /* Adds a property with the given 'type' and 16-bit 'value' to 'msg'. */ void ofpprop_put_be16(struct ofpbuf *msg, uint64_t type, ovs_be16 value) diff --git a/lib/ofp-prop.h b/lib/ofp-prop.h index 7cd107c..db29650 100644 --- a/lib/ofp-prop.h +++ b/lib/ofp-prop.h @@ -85,6 +85,7 @@ enum ofperr ofpprop_parse_uuid(const struct ofpbuf *, struct uuid *); /* Serializing properties. */ void ofpprop_put(struct ofpbuf *, uint64_t type, const void *value, size_t len); +void *ofpprop_put_zeros(struct ofpbuf *, uint64_t type, size_t len); void ofpprop_put_be16(struct ofpbuf *, uint64_t type, ovs_be16 value); void ofpprop_put_be32(struct ofpbuf *, uint64_t type, ovs_be32 value); void ofpprop_put_be64(struct ofpbuf *, uint64_t type, ovs_be64 value); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index c813afd..1225664 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -3868,9 +3868,7 @@ ofputil_put_ofp14_port(const struct ofputil_phy_port *pp, op->config = htonl(pp->config & OFPPC11_ALL); op->state = htonl(pp->state & OFPPS11_ALL); -eth = ofpbuf_put_zeros(b, sizeof *eth); -eth->type = htons(OFPPDPT14_ETHERNET); -eth->length = htons(sizeof *eth); +eth = ofpprop_put_zeros(b, OFPPDPT14_ETHERNET, sizeof *eth); eth->curr = netdev_port_features_to_ofp11(pp->curr); eth->advertised = netdev_port_features_to_ofp11(pp->advertised); eth->supported = netdev_port_features_to_ofp11(pp->supported); @@ -5013,9 +5011,7 @@ ofputil_append_table_desc_reply(const struct ofputil_table_desc *td, if (td->vacancy == OFPUTIL_TABLE_VACANCY_ON) { struct ofp14_table_mod_prop_vacancy *otv; -otv = ofpbuf_put_zeros(reply, sizeof *otv); -otv->type = htons(OFPTMPT14_VACANCY); -otv->length = htons(sizeof *otv); +otv = ofpprop_put_zeros(reply, OFPTMPT14_VACANCY, sizeof *otv); otv->vacancy_down = td->table_vacancy.vacancy_down; otv->vacancy_up = td->table_vacancy.vacancy_up; otv->vacancy = td->table_vacancy.vacancy; @@ -5279,7 +5275,6 @@ ofputil_encode_table_mod(const struct ofputil_table_mod *tm, case OFP14_VERSION: case OFP15_VERSION: { struct ofp14_table_mod *otm; -struct ofp14_table_mod_prop_vacancy *otv; b = ofpraw_alloc(OFPRAW_OFPT14_TABLE_MOD, ofp_version, 0); otm = ofpbuf_put_zeros(b, sizeof *otm); @@ -5291,9 +5286,9 @@ ofputil_encode_table_mod(const struct ofputil_table_mod *tm, ofpprop_put_u32(b, OFPTMPT14_EVICTION, tm->eviction_flags); } if (tm->vacancy == OFPUTIL_TABLE_VACANCY_ON) { -otv = ofpbuf_put_zeros(b, sizeof *otv); -otv->type = htons(OFPTMPT14_VACANCY); -otv->length = htons(sizeof *otv); +struct ofp14_table_mod_prop_vacancy *otv; + +otv = ofpprop_put_zeros(b, OFPTMPT14_VACANCY, sizeof *otv); otv->vacancy_down = tm->table_vacancy.vacancy_down; otv->vacancy_up = tm->table_vacancy.vacancy_up; } @@ -6961,10 +6956,7 @@ ofputil_append_ofp14_port_stats(const struct ofputil_port_stats *ops, ps14->rx_errors = htonll(ops->stats.rx_errors); ps14->tx_errors = htonll(ops->stats.tx_errors); -eth = ofpbuf_put_uninit(reply, sizeof *eth); -eth->type = htons(OFPPSPT14_ETHERNET); -eth->length = htons(sizeof *eth); -memset(eth->pad, 0, sizeof eth->pad); +eth = ofpprop_put_zeros(reply, OFPPSPT14_ETHERNET, sizeof *eth); eth->rx_frame_err = htonll(ops->stats.rx_frame_errors); eth->rx_over_err = htonll(ops->stats.rx_over_errors); eth->rx_crc_err = htonll(ops->stats.rx_crc_er
[ovs-dev] [PATCH 24/41] ofp-util: Rewrite async config encoding and decoding to be table-driven.
The encoding and decoding of the OpenFlow and Open vSwitch async config messages was, until now, a collection of disjoint code that had a lot of redundancy. This commit changes it all to be driven using a single central table. This rewrite fixes a bug in the OF1.4+ version of the code, which until now assumed that every TLV in an OF1.4+ asynchronous configuration message was exactly 8 bytes long, and reported an error if any was a different length. This invariant is true of all the standard TLVs already defined, but it won't be true of any experimenter TLVs (and won't necessarily be true of any new standard TLVs), so this commit changes it to be more tolerant. The OFPACPT_* constants are no longer useful (they are encoded directly in the table and do not need to be anywhere else), so this removes them. This commit also adds support for experimenter async config messages. We don't have any yet but an upcoming commit will add one. Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.4.h | 24 +-- lib/ofp-print.c | 1 - lib/ofp-util.c | 389 lib/ofp-util.h | 16 -- tests/ofp-print.at | 6 + 5 files changed, 206 insertions(+), 230 deletions(-) diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h index bce1598..49a4f7b 100644 --- a/include/openflow/openflow-1.4.h +++ b/include/openflow/openflow-1.4.h @@ -125,7 +125,7 @@ enum ofp14_table_mod_prop_eviction_flag { enum ofp14_table_reason { OFPTR_VACANCY_DOWN = 3,/* Vacancy down threshold event. */ OFPTR_VACANCY_UP = 4,/* Vacancy up threshold event. */ -OFPTR_N_REASONS/* Denotes number of reasons. */ +#define OFPTR_BITS ((1u << OFPTR_VACANCY_DOWN) | (1u << OFPTR_VACANCY_UP)) }; struct ofp14_table_mod_prop_vacancy { @@ -255,28 +255,6 @@ enum ofp14_requestforward_reason { OFPRFR_N_REASONS /* Denotes number of reasons. */ }; -/* Async Config property types. - -* Low order bit cleared indicates a property for the slave role. -* Low order bit set indicates a property for the master/equal role. -*/ -enum ofp14_async_config_prop_type { -OFPACPT_PACKET_IN_SLAVE = 0, /* Packet-in mask for slave. */ -OFPACPT_PACKET_IN_MASTER = 1, /* Packet-in mask for master. */ -OFPACPT_PORT_STATUS_SLAVE = 2, /* Port-status mask for slave. */ -OFPACPT_PORT_STATUS_MASTER= 3, /* Port-status mask for master. */ -OFPACPT_FLOW_REMOVED_SLAVE= 4, /* Flow removed mask for slave. */ -OFPACPT_FLOW_REMOVED_MASTER = 5, /* Flow removed mask for master. */ -OFPACPT_ROLE_STATUS_SLAVE = 6, /* Role status mask for slave. */ -OFPACPT_ROLE_STATUS_MASTER= 7, /* Role status mask for master. */ -OFPACPT_TABLE_STATUS_SLAVE= 8, /* Table status mask for slave. */ -OFPACPT_TABLE_STATUS_MASTER = 9, /* Table status mask for master. */ -OFPACPT_REQUESTFORWARD_SLAVE = 10, /* RequestForward mask for slave. */ -OFPACPT_REQUESTFORWARD_MASTER = 11, /* RequestForward mask for master. */ -OFPTFPT_EXPERIMENTER_SLAVE= 0xFFFE, /* Experimenter for slave. */ -OFPTFPT_EXPERIMENTER_MASTER = 0x, /* Experimenter for master. */ -}; - /* Role status event message. */ struct ofp14_role_status { ovs_be32 role; /* One of OFPCR_ROLE_*. */ diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 36a6301..accde8e 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2003,7 +2003,6 @@ ofp_table_reason_to_string(enum ofp14_table_reason reason, case OFPTR_VACANCY_UP: return "vacancy_up"; -case OFPTR_N_REASONS: default: snprintf(reasonbuf, bufsize, "%d", (int) reason); return reasonbuf; diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 40c84e4..19d3775 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -63,56 +63,6 @@ static ovs_be32 ofputil_encode_table_config(enum ofputil_table_miss, enum ofputil_table_vacancy, enum ofp_version); -static enum ofperr -ofputil_check_mask(uint16_t type, uint32_t mask) -{ -switch (type) { -case OFPACPT_PACKET_IN_SLAVE: -case OFPACPT_PACKET_IN_MASTER: -if (mask > MAXIMUM_MASK_PACKET_IN) { -return OFPERR_OFPACFC_INVALID; -} -break; - -case OFPACPT_FLOW_REMOVED_SLAVE: -case OFPACPT_FLOW_REMOVED_MASTER: -if (mask > MAXIMUM_MASK_FLOW_REMOVED) { -return OFPERR_OFPACFC_INVALID; -} -break; - -case OFPACPT_PORT_STATUS_SLAVE: -case OFPACPT_PORT_STATUS_MASTER: -if (mask > MAXIMUM_MASK_PORT_STATUS) { -return OFPERR_OFPACFC_INVALID; -} -break; - -case OFPACPT_ROLE_STATUS_SLAVE: -case OFPACPT_ROLE_STATUS_MASTER: -if (mask > MAXIMUM_MASK_ROLE_STATUS) { -return OFPERR_OFPACFC_INVALID; -} -break
[ovs-dev] [PATCH 23/41] ofp-util: Define struct ofputil_async_cfg to hold async message config.
This seems a little better than a pair of bare arrays. Signed-off-by: Ben Pfaff --- include/openflow/openflow-common.h | 24 ++- lib/ofp-print.c| 22 ++ lib/ofp-util.c | 79 +- lib/ofp-util.h | 20 -- ofproto/connmgr.c | 134 - ofproto/connmgr.h | 9 +-- ofproto/ofproto.c | 17 ++--- 7 files changed, 126 insertions(+), 179 deletions(-) diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h index d2efa5f..3985705 100644 --- a/include/openflow/openflow-common.h +++ b/include/openflow/openflow-common.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2011, 2012, 2013, 2014 The Board of Trustees of The Leland Stanford +/* Copyright (c) 2008, 2011, 2012, 2013, 2014, 2016 The Board of Trustees of The Leland Stanford * Junior University * * We are making the OpenFlow specification and associated documentation @@ -280,6 +280,13 @@ enum ofp_packet_in_reason { OFPR_ACTION_SET,/* Output to controller in action set */ OFPR_GROUP, /* Output to controller in group bucket */ OFPR_PACKET_OUT,/* Output to controller in packet-out */ + +#define OFPR10_BITS \ +((1u << OFPR_NO_MATCH) | (1u << OFPR_ACTION) | (1u << OFPR_INVALID_TTL)) +#define OFPR14_BITS \ +(OFPR10_BITS | \ + (1u << OFPR_ACTION_SET) | (1u << OFPR_GROUP) | (1u << OFPR_PACKET_OUT)) + OFPR_N_REASONS }; @@ -306,6 +313,16 @@ enum ofp_flow_removed_reason { OFPRR_METER_DELETE, /* Meter was removed. */ OFPRR_EVICTION, /* Switch eviction to free resources. */ +#define OFPRR10_BITS\ +((1u << OFPRR_IDLE_TIMEOUT) | \ + (1u << OFPRR_HARD_TIMEOUT) | \ + (1u << OFPRR_DELETE)) +#define OFPRR14_BITS\ +(OFPRR10_BITS | \ + (1u << OFPRR_GROUP_DELETE) | \ + (1u << OFPRR_METER_DELETE) | \ + (1u << OFPRR_EVICTION)) + OVS_OFPRR_NONE /* OVS internal_use only, keep last!. */ }; @@ -314,6 +331,11 @@ enum ofp_port_reason { OFPPR_ADD, /* The port was added. */ OFPPR_DELETE, /* The port was removed. */ OFPPR_MODIFY, /* Some attribute of the port has changed. */ + +#define OFPPR_BITS ((1u << OFPPR_ADD) | \ +(1u << OFPPR_DELETE) | \ +(1u << OFPPR_MODIFY)) + OFPPR_N_REASONS /* Denotes number of reasons. */ }; diff --git a/lib/ofp-print.c b/lib/ofp-print.c index bf97b04..36a6301 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2131,31 +2131,23 @@ ofp_print_nxt_set_async_config(struct ds *string, } } else if (raw == OFPRAW_OFPT14_SET_ASYNC || raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { -enum ofperr error = 0; -uint32_t role[2][OAM_N_TYPES] = {{0}}; -uint32_t type; - -if (raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { -error = ofputil_decode_set_async_config(oh, role[0], role[1], true); -} -else if (raw == OFPRAW_OFPT14_SET_ASYNC) { -error = ofputil_decode_set_async_config(oh, role[0], role[1], -false); -} +struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT; +bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY; +enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, &ac); if (error) { ofp_print_error(string, error); return; } for (i = 0; i < 2; i++) { - ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave"); -for (type = 0; type < OAM_N_TYPES; type++) { +for (uint32_t type = 0; type < OAM_N_TYPES; type++) { ds_put_format(string, "%16s:", ofputil_async_msg_type_to_string(type)); +uint32_t role = i == 0 ? ac.master[type] : ac.slave[type]; for (j = 0; j < 32; j++) { -if (role[i][type] & (1u << j)) { +if (role & (1u << j)) { char reasonbuf[OFP_ASYNC_CONFIG_REASON_BUFSIZE]; const char *reason; @@ -2165,7 +2157,7 @@ ofp_print_nxt_set_async_config(struct ds *string, ds_put_format(string, " %s", reason); } } -if (!role[i][type]) { +if (!role) { ds_put_cstr(string, " (off)"); } ds_put_char(string, '\n'); diff --git a/lib/ofp-util.c
[ovs-dev] [PATCH 22/41] ofp-util: New function ofputil_async_msg_type_to_string().
An upcoming commit will add another user. Signed-off-by: Ben Pfaff --- lib/ofp-print.c | 27 ++- lib/ofp-util.c | 17 + lib/ofp-util.h | 1 + 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 42e822b..bf97b04 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2151,31 +2151,8 @@ ofp_print_nxt_set_async_config(struct ds *string, ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave"); for (type = 0; type < OAM_N_TYPES; type++) { -switch (type) { -case OAM_PACKET_IN: -ds_put_cstr(string, " PACKET_IN:"); -break; - -case OAM_PORT_STATUS: -ds_put_cstr(string, " PORT_STATUS:"); -break; - -case OAM_FLOW_REMOVED: -ds_put_cstr(string, "FLOW_REMOVED:"); -break; - -case OAM_ROLE_STATUS: -ds_put_cstr(string, " ROLE_STATUS:"); -break; - -case OAM_TABLE_STATUS: -ds_put_cstr(string, "TABLE_STATUS:"); -break; - -case OAM_REQUESTFORWARD: -ds_put_cstr(string, " REQUESTFORWARD:"); -break; -} +ds_put_format(string, "%16s:", + ofputil_async_msg_type_to_string(type)); for (j = 0; j < 32; j++) { if (role[i][type] & (1u << j)) { diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 6f9aeb7..722f033 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -9467,6 +9467,23 @@ ofputil_uninit_tlv_table(struct ovs_list *mappings) } } +const char * +ofputil_async_msg_type_to_string(enum ofputil_async_msg_type type) +{ +switch (type) { +case OAM_PACKET_IN: return "PACKET_IN"; +case OAM_PORT_STATUS:return "PORT_STATUS"; +case OAM_FLOW_REMOVED: return "FLOW_REMOVED"; +case OAM_ROLE_STATUS:return "ROLE_STATUS"; +case OAM_TABLE_STATUS: return "TABLE_STATUS"; +case OAM_REQUESTFORWARD: return "REQUESTFORWARD"; + +case OAM_N_TYPES: +default: +OVS_NOT_REACHED(); +} +} + /* Decodes the OpenFlow "set async config" request and "get async config * reply" message in '*oh' into an abstract form in 'master' and 'slave'. * diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 89f3d95..1fd013d 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -1323,6 +1323,7 @@ enum ofputil_async_msg_type { OAM_REQUESTFORWARD, /* OFPT_REQUESTFORWARD. */ OAM_N_TYPES }; +const char *ofputil_async_msg_type_to_string(enum ofputil_async_msg_type); enum ofperr ofputil_decode_set_async_config(const struct ofp_header *, uint32_t master[OAM_N_TYPES], -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 26/41] ofp-util: Add function to encode OFPT_SET_ASYNC messages.
This isn't used yet but it will be in future commits. This also looks forward to supporting Open vSwitch extensions to OAM_*, which will be coming up soon. Signed-off-by: Ben Pfaff --- lib/ofp-msgs.h| 3 +++ lib/ofp-util.c| 32 +--- lib/ofp-util.h| 9 - ofproto/ofproto.c | 2 +- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 54018b4..4f95607 100644 --- a/lib/ofp-msgs.h +++ b/lib/ofp-msgs.h @@ -238,6 +238,8 @@ enum ofpraw { OFPRAW_OFPT13_SET_ASYNC, /* NXT 1.0+ (19): struct nx_async_config. */ OFPRAW_NXT_SET_ASYNC_CONFIG, +/* NXT 1.0-1.3 (27): uint8_t[8][]. */ +OFPRAW_NXT_SET_ASYNC_CONFIG2, /* OFPT 1.4+ (28): uint8_t[8][]. */ OFPRAW_OFPT14_SET_ASYNC, @@ -560,6 +562,7 @@ enum ofptype { OFPTYPE_GET_ASYNC_REPLY, /* OFPRAW_OFPT13_GET_ASYNC_REPLY. * OFPRAW_OFPT14_GET_ASYNC_REPLY. */ OFPTYPE_SET_ASYNC_CONFIG, /* OFPRAW_NXT_SET_ASYNC_CONFIG. + * OFPRAW_NXT_SET_ASYNC_CONFIG2. * OFPRAW_OFPT13_SET_ASYNC. * OFPRAW_OFPT14_SET_ASYNC. */ diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 5bb0b74..4850aa5 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -9612,7 +9612,8 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose, decode_legacy_async_masks(msg->flow_removed_mask, OAM_FLOW_REMOVED, oh->version, ac); } else if (raw == OFPRAW_OFPT14_SET_ASYNC || - raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { + raw == OFPRAW_OFPT14_GET_ASYNC_REPLY || + raw == OFPRAW_NXT_SET_ASYNC_CONFIG2) { *ac = *basis; while (b.size > 0) { struct ofpbuf property; @@ -9694,8 +9695,8 @@ ofputil_put_async_config__(const struct ofputil_async_cfg *ac, /* Encodes and returns a reply to the OFPT_GET_ASYNC_REQUEST in 'oh' that * states that the asynchronous message configuration is 'ac'. */ struct ofpbuf * -ofputil_encode_get_async_config(const struct ofp_header *oh, -const struct ofputil_async_cfg *ac) +ofputil_encode_get_async_reply(const struct ofp_header *oh, + const struct ofputil_async_cfg *ac) { struct ofpbuf *buf; @@ -9711,6 +9712,31 @@ ofputil_encode_get_async_config(const struct ofp_header *oh, return buf; } +/* Encodes and returns a message, in a format appropriate for OpenFlow version + * 'ofp_version', that sets the asynchronous message configuration to 'ac'. + * + * Specify 'oams' as a bitmap of OAM_* that indicate the asynchronous messages + * to configure. OF1.0 through OF1.3 can't natively configure a subset of + * messages, so more messages than requested may be configured. OF1.0 through + * OF1.3 also can't configure OVS extension OAM_* values, so if 'oam' includes + * any extensions then this function encodes an Open vSwitch extension message + * that does support configuring OVS extension OAM_*. */ +struct ofpbuf * +ofputil_encode_set_async_config(const struct ofputil_async_cfg *ac, +uint32_t oams, enum ofp_version ofp_version) +{ +enum ofpraw raw = (ofp_version >= OFP14_VERSION ? OFPRAW_OFPT14_SET_ASYNC + : oams & OAM_EXTENSIONS ? OFPRAW_NXT_SET_ASYNC_CONFIG2 + : ofp_version >= OFP13_VERSION ? OFPRAW_OFPT13_SET_ASYNC + : OFPRAW_NXT_SET_ASYNC_CONFIG); +struct ofpbuf *request = ofpraw_alloc(raw, ofp_version, 0); +ofputil_put_async_config__(ac, request, + (raw == OFPRAW_OFPT14_SET_ASYNC || +raw == OFPRAW_NXT_SET_ASYNC_CONFIG2), + ofp_version, oams); +return request; +} + struct ofputil_async_cfg ofputil_async_cfg_default(enum ofp_version version) { diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 88c67f9..1c359b3 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -1299,12 +1299,17 @@ enum ofperr ofputil_decode_tlv_table_reply(const struct ofp_header *, void ofputil_uninit_tlv_table(struct ovs_list *mappings); enum ofputil_async_msg_type { +/* Standard asynchronous messages. */ OAM_PACKET_IN, /* OFPT_PACKET_IN or NXT_PACKET_IN. */ OAM_PORT_STATUS,/* OFPT_PORT_STATUS. */ OAM_FLOW_REMOVED, /* OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED. */ OAM_ROLE_STATUS,/* OFPT_ROLE_STATUS. */ OAM_TABLE_STATUS, /* OFPT_TABLE_STATUS. */ OAM_REQUESTFORWARD, /* OFPT_REQUESTFORWARD. */ + +/* Extension asynchronous messages (none yet--coming soon!). */ +#define OAM_EXTENSIONS 0/* Bitmap of all extensions. */ + OAM_N_TYPES }; const char *ofputil_async_msg_type_to_string(enum ofputil_async_msg_type); @@ -1320,8 +13
[ovs-dev] [PATCH 27/41] ofp-print: Decode all async config messages the same way.
We have a single function to decode all of these messages, so there's no reason to do it two different ways for printing. Signed-off-by: Ben Pfaff --- lib/ofp-print.c | 117 +++--- tests/ofp-print.at| 26 +-- tests/ofproto-dpif.at | 6 +++ tests/ofproto.at | 6 +++ 4 files changed, 60 insertions(+), 95 deletions(-) diff --git a/lib/ofp-print.c b/lib/ofp-print.c index f36335b..bedfc94 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2063,108 +2063,42 @@ ofp_async_config_reason_to_string(uint32_t reason, #define OFP_ASYNC_CONFIG_REASON_BUFSIZE (INT_STRLEN(int) + 1) static void -ofp_print_nxt_set_async_config(struct ds *string, - const struct ofp_header *oh) +ofp_print_set_async_config(struct ds *string, const struct ofp_header *oh, + enum ofptype type) { -int i, j; -enum ofpraw raw; - -ofpraw_decode(&raw, oh); - -if (raw == OFPRAW_OFPT13_SET_ASYNC || -raw == OFPRAW_NXT_SET_ASYNC_CONFIG || -raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) { -const struct nx_async_config *nac = ofpmsg_body(oh); - -for (i = 0; i < 2; i++) { +struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT; +struct ofputil_async_cfg ac; -ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave"); - -ds_put_cstr(string, " PACKET_IN:"); -for (j = 0; j < 32; j++) { -if (nac->packet_in_mask[i] & htonl(1u << j)) { -char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE]; -const char *reason; - -reason = ofputil_packet_in_reason_to_string(j, reasonbuf, -sizeof reasonbuf); -ds_put_format(string, " %s", reason); -} -} -if (!nac->packet_in_mask[i]) { -ds_put_cstr(string, " (off)"); -} -ds_put_char(string, '\n'); - -ds_put_cstr(string, " PORT_STATUS:"); -for (j = 0; j < 32; j++) { -if (nac->port_status_mask[i] & htonl(1u << j)) { -char reasonbuf[OFP_PORT_REASON_BUFSIZE]; -const char *reason; +bool is_reply = type == OFPTYPE_GET_ASYNC_REPLY; +enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, +&basis, &ac); +if (error) { +ofp_print_error(string, error); +return; +} -reason = ofp_port_reason_to_string(j, reasonbuf, - sizeof reasonbuf); -ds_put_format(string, " %s", reason); -} -} -if (!nac->port_status_mask[i]) { -ds_put_cstr(string, " (off)"); -} -ds_put_char(string, '\n'); +for (int i = 0; i < 2; i++) { +ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave"); +for (uint32_t type = 0; type < OAM_N_TYPES; type++) { +ds_put_format(string, "%16s:", + ofputil_async_msg_type_to_string(type)); -ds_put_cstr(string, "FLOW_REMOVED:"); -for (j = 0; j < 32; j++) { -if (nac->flow_removed_mask[i] & htonl(1u << j)) { -char reasonbuf[OFP_FLOW_REMOVED_REASON_BUFSIZE]; +uint32_t role = i == 0 ? ac.master[type] : ac.slave[type]; +for (int j = 0; j < 32; j++) { +if (role & (1u << j)) { +char reasonbuf[OFP_ASYNC_CONFIG_REASON_BUFSIZE]; const char *reason; -reason = ofp_flow_removed_reason_to_string(j, reasonbuf, - sizeof reasonbuf); +reason = ofp_async_config_reason_to_string( +j, type, reasonbuf, sizeof reasonbuf); ds_put_format(string, " %s", reason); } } -if (!nac->flow_removed_mask[i]) { +if (!role) { ds_put_cstr(string, " (off)"); } ds_put_char(string, '\n'); } -} else if (raw == OFPRAW_OFPT14_SET_ASYNC || - raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { -struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT; -struct ofputil_async_cfg ac; - -bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY; -enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, -&basis, &ac); -if (error) { -ofp_print_error(string, error); -return; -} - -for (i = 0; i < 2; i++) { -ds_put_format(string, "\n %s:\n", i == 0 ? "master" : "slave"); -
[ovs-dev] [PATCH 25/41] ofp-util: Fix OF1.4+ version of ofputil_decode_set_async_config().
The OF1.0 through OF1.3 "set async config" set the whole configuration, OF1.4+ only update parts of it piecemeal, but the decoding function always set the whole configuration. This commit fixes the problem by changing the interface to require the caller to provide an initial state. (It would be possible to simply make it mutate existing state in-place, but that interface seems a little more error-prone.) Found by inspection. Signed-off-by: Ben Pfaff --- OPENFLOW-1.1+.md | 5 + lib/ofp-print.c | 7 +-- lib/ofp-util.c| 8 lib/ofp-util.h| 1 + ofproto/ofproto.c | 5 +++-- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md index 537f660..62ebddc 100644 --- a/OPENFLOW-1.1+.md +++ b/OPENFLOW-1.1+.md @@ -192,10 +192,7 @@ OpenFlow 1.4 features are listed in the previous section. * More extensible wire protocol Many on-wire structures got TLVs. -Already implemented: port desc properties, port mod properties, - port stats properties, table mod properties, - queue stats, unified property errors, queue desc. -Remaining required: set-async +All required features are now supported. Remaining optional: table desc, table-status [EXT-262] [required for OF1.4+] diff --git a/lib/ofp-print.c b/lib/ofp-print.c index accde8e..f36335b 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -2130,9 +2130,12 @@ ofp_print_nxt_set_async_config(struct ds *string, } } else if (raw == OFPRAW_OFPT14_SET_ASYNC || raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { -struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT; +struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT; +struct ofputil_async_cfg ac; + bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY; -enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, &ac); +enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, +&basis, &ac); if (error) { ofp_print_error(string, error); return; diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 19d3775..5bb0b74 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -9570,6 +9570,11 @@ decode_legacy_async_masks(const ovs_be32 masks[2], /* Decodes the OpenFlow "set async config" request and "get async config * reply" message in '*oh' into an abstract form in 'ac'. * + * Some versions of the "set async config" request change only some of the + * settings and leave the others alone. This function uses 'basis' as the + * initial state for decoding these. Other versions of the request change all + * the settings; this function ignores 'basis' when decoding these. + * * If 'loose' is true, this function ignores properties and values that it does * not understand, as a controller would want to do when interpreting * capabilities provided by a switch. If 'loose' is false, this function @@ -9585,6 +9590,7 @@ decode_legacy_async_masks(const ovs_be32 masks[2], * supported.*/ enum ofperr ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose, +const struct ofputil_async_cfg *basis, struct ofputil_async_cfg *ac) { enum ofpraw raw; @@ -9598,6 +9604,7 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose, raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) { const struct nx_async_config *msg = ofpmsg_body(oh); +*ac = OFPUTIL_ASYNC_CFG_INIT; decode_legacy_async_masks(msg->packet_in_mask, OAM_PACKET_IN, oh->version, ac); decode_legacy_async_masks(msg->port_status_mask, OAM_PORT_STATUS, @@ -9606,6 +9613,7 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose, oh->version, ac); } else if (raw == OFPRAW_OFPT14_SET_ASYNC || raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { +*ac = *basis; while (b.size > 0) { struct ofpbuf property; enum ofperr error; diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 52268d8..88c67f9 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -1317,6 +1317,7 @@ struct ofputil_async_cfg { enum ofperr ofputil_decode_set_async_config(const struct ofp_header *, bool loose, +const struct ofputil_async_cfg *, struct ofputil_async_cfg *); struct ofpbuf *ofputil_encode_get_async_config( diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index fbaf7dd..957e323 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -5403,10 +5403,11 @@ handle_nxt_set_packet_in_format(struct ofconn *ofconn, static enum ofperr handle_nxt_set_async_config(struct ofconn *ofconn, const
[ovs-dev] [PATCH 28/41] openflow: Get rid of struct ofp13_packet_in.
It's actually harder to parse OF1.2/OF1.3 "packet-in" messages when ofp13_packet_in is involved than when the code just realizes that ofp13_packet_in = ofp12_packet_in + cookie. Signed-off-by: Ben Pfaff --- include/openflow/openflow-1.3.h | 21 +-- lib/ofp-msgs.h | 2 +- lib/ofp-util.c | 79 - 3 files changed, 33 insertions(+), 69 deletions(-) diff --git a/include/openflow/openflow-1.3.h b/include/openflow/openflow-1.3.h index 252e08e..a521995 100644 --- a/include/openflow/openflow-1.3.h +++ b/include/openflow/openflow-1.3.h @@ -46,7 +46,7 @@ * - new field: auxiliary_id * - removed: ofp_ports at the end * - * OFPT_PACKET_IN = 10 (ofp13_packet_in) new field: cookie + * OFPT_PACKET_IN = 10 (appends an ovs_be64 to ofp12_packet_in) * * OpenFlow 1.3 adds following new message types: * @@ -352,23 +352,4 @@ struct ofp13_async_config { }; OFP_ASSERT(sizeof(struct ofp13_async_config) == 24); - -/* Packet received on port (datapath -> controller). */ -struct ofp13_packet_in { -struct ofp12_packet_in pi; -ovs_be64 cookie; /* Cookie of the flow entry that was looked up */ -/* Followed by: - * - Match - * - Exactly 2 all-zero padding bytes, then - * - An Ethernet frame whose length is inferred from header.length. - * The padding bytes preceding the Ethernet frame ensure that the IP - * header (if any) following the Ethernet header is 32-bit aligned. - */ -/* struct ofp12_match match; */ -/* uint8_t pad[2]; Align to 64 bit + 16 bit */ -/* uint8_t data[0];Ethernet frame */ -}; -OFP_ASSERT(sizeof(struct ofp13_packet_in) == 16); - - #endif /* openflow/openflow-1.3.h */ diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 4f95607..a15efb6 100644 --- a/lib/ofp-msgs.h +++ b/lib/ofp-msgs.h @@ -148,7 +148,7 @@ enum ofpraw { OFPRAW_OFPT11_PACKET_IN, /* OFPT 1.2 (10): struct ofp12_packet_in, uint8_t[]. */ OFPRAW_OFPT12_PACKET_IN, -/* OFPT 1.3+ (10): struct ofp13_packet_in, uint8_t[]. */ +/* OFPT 1.3+ (10): struct ofp12_packet_in, ovs_be64, uint8_t[]. */ OFPRAW_OFPT13_PACKET_IN, /* NXT 1.0+ (17): struct nx_packet_in, uint8_t[]. */ OFPRAW_NXT_PACKET_IN, diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 4850aa5..347cb61 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -3324,18 +3324,11 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin, ofpbuf_use_const(&b, oh, ntohs(oh->length)); raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT13_PACKET_IN || raw == OFPRAW_OFPT12_PACKET_IN) { -const struct ofp13_packet_in *opi; -int error; -size_t packet_in_size; - -if (raw == OFPRAW_OFPT12_PACKET_IN) { -packet_in_size = sizeof (struct ofp12_packet_in); -} else { -packet_in_size = sizeof (struct ofp13_packet_in); -} - -opi = ofpbuf_pull(&b, packet_in_size); -error = oxm_pull_match_loose(&b, &pin->flow_metadata); +const struct ofp12_packet_in *opi = ofpbuf_pull(&b, sizeof *opi); +const ovs_be64 *cookie = (raw == OFPRAW_OFPT13_PACKET_IN + ? ofpbuf_pull(&b, sizeof *cookie) + : NULL); +enum ofperr error = oxm_pull_match_loose(&b, &pin->flow_metadata); if (error) { return error; } @@ -3344,13 +3337,13 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin, return OFPERR_OFPBRC_BAD_LEN; } -pin->reason = opi->pi.reason; -pin->table_id = opi->pi.table_id; -pin->buffer_id = ntohl(opi->pi.buffer_id); -pin->total_len = ntohs(opi->pi.total_len); +pin->reason = opi->reason; +pin->table_id = opi->table_id; +pin->buffer_id = ntohl(opi->buffer_id); +pin->total_len = ntohs(opi->total_len); -if (raw == OFPRAW_OFPT13_PACKET_IN) { -pin->cookie = opi->cookie; +if (cookie) { +pin->cookie = *cookie; } pin->packet = b.data; @@ -3489,41 +3482,31 @@ static struct ofpbuf * ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin, enum ofputil_protocol protocol) { -struct ofp13_packet_in *opi; -enum ofpraw packet_in_raw; -enum ofp_version packet_in_version; -size_t packet_in_size; -struct ofpbuf *packet; - -if (protocol == OFPUTIL_P_OF12_OXM) { -packet_in_raw = OFPRAW_OFPT12_PACKET_IN; -packet_in_version = OFP12_VERSION; -packet_in_size = sizeof (struct ofp12_packet_in); -} else { -packet_in_raw = OFPRAW_OFPT13_PACKET_IN; -packet_in_version = ofputil_protocol_to_ofp_version(protocol); -packet_in_size = sizeof (struct ofp13_packet_in); -} +enum ofp_version version = o
[ovs-dev] [PATCH 30/41] pktbuf: Move from 'ofproto' to 'lib'.
An upcoming commit will use this library from ofp-util instead of ofproto. Signed-off-by: Ben Pfaff --- lib/automake.mk | 4 +++- {ofproto => lib}/pktbuf.c | 0 {ofproto => lib}/pktbuf.h | 0 ofproto/automake.mk | 4 +--- 4 files changed, 4 insertions(+), 4 deletions(-) rename {ofproto => lib}/pktbuf.c (100%) rename {ofproto => lib}/pktbuf.h (100%) diff --git a/lib/automake.mk b/lib/automake.mk index 39a652b..27a1669 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -1,4 +1,4 @@ -# Copyright (C) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. +# Copyright (C) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. # # Copying and distribution of this file, with or without modification, # are permitted in any medium without royalty provided the copyright @@ -195,6 +195,8 @@ lib_libopenvswitch_la_SOURCES = \ lib/pcap-file.h \ lib/perf-counter.h \ lib/perf-counter.c \ + lib/pktbuf.c \ + lib/pktbuf.h \ lib/poll-loop.c \ lib/poll-loop.h \ lib/process.c \ diff --git a/ofproto/pktbuf.c b/lib/pktbuf.c similarity index 100% rename from ofproto/pktbuf.c rename to lib/pktbuf.c diff --git a/ofproto/pktbuf.h b/lib/pktbuf.h similarity index 100% rename from ofproto/pktbuf.h rename to lib/pktbuf.h diff --git a/ofproto/automake.mk b/ofproto/automake.mk index 50e7507..7486f2b 100644 --- a/ofproto/automake.mk +++ b/ofproto/automake.mk @@ -1,4 +1,4 @@ -# Copyright (C) 2009, 2010, 2011, 2012, 2014 Nicira, Inc. +# Copyright (C) 2009, 2010, 2011, 2012, 2014, 2016 Nicira, Inc. # # Copying and distribution of this file, with or without modification, # are permitted in any medium without royalty provided the copyright @@ -43,8 +43,6 @@ ofproto_libofproto_la_SOURCES = \ ofproto/ofproto-dpif-xlate.c \ ofproto/ofproto-dpif-xlate.h \ ofproto/ofproto-provider.h \ - ofproto/pktbuf.c \ - ofproto/pktbuf.h \ ofproto/pinsched.c \ ofproto/pinsched.h \ ofproto/tunnel.c \ -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 29/41] fail-open: Drop some of the weirder special cases.
I don't have any real evidence that these special cases make a difference in real-world cases. The messages for the commits that add them are not clear about the reasons for them. I seem to recall that they were only tested with the dummy controller inside OVS, which isn't very good evidence for real controllers. Finally, they cut across layers in nasty ways and make it hard to better abstract packet-ins and packet buffering. If these solve real problems then we can reconsider after some reports come in. Signed-off-by: Ben Pfaff --- ofproto/connmgr.c | 3 --- ofproto/ofproto-dpif-upcall.c | 25 ofproto/ofproto-dpif-xlate.c | 2 -- ofproto/ofproto-dpif-xlate.h | 3 +-- ofproto/pktbuf.c | 44 +-- ofproto/pktbuf.h | 3 +-- 6 files changed, 7 insertions(+), 73 deletions(-) diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 295c03c..c3e486c 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -1756,7 +1756,6 @@ static void schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin, enum ofp_packet_in_reason wire_reason) { -struct connmgr *mgr = ofconn->connmgr; uint16_t controller_max_len; struct ovs_list txq; @@ -1774,8 +1773,6 @@ schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin, * unbuffered. This behaviour doesn't violate prior versions, too. */ if (controller_max_len == UINT16_MAX) { pin.up.buffer_id = UINT32_MAX; -} else if (mgr->fail_open && fail_open_is_active(mgr->fail_open)) { -pin.up.buffer_id = pktbuf_get_null(); } else if (!ofconn->pktbuf) { pin.up.buffer_id = UINT32_MAX; } else { diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 9dd7895..b505206 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1090,31 +1090,6 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, xlate_actions(&xin, &upcall->xout); upcall->xout_initialized = true; -/* Special case for fail-open mode. - * - * If we are in fail-open mode, but we are connected to a controller too, - * then we should send the packet up to the controller in the hope that it - * will try to set up a flow and thereby allow us to exit fail-open. - * - * See the top-level comment in fail-open.c for more information. - * - * Copy packets before they are modified by execution. */ -if (upcall->xout.fail_open) { -const struct dp_packet *packet = upcall->packet; -struct ofproto_packet_in *pin; - -pin = xmalloc(sizeof *pin); -pin->up.packet = xmemdup(dp_packet_data(packet), dp_packet_size(packet)); -pin->up.packet_len = dp_packet_size(packet); -pin->up.reason = OFPR_NO_MATCH; -pin->up.table_id = 0; -pin->up.cookie = OVS_BE64_MAX; -flow_get_metadata(upcall->flow, &pin->up.flow_metadata); -pin->send_len = 0; /* Not used for flow table misses. */ -pin->miss_type = OFPROTO_PACKET_IN_NO_MISS; -ofproto_dpif_send_packet_in(upcall->ofproto, pin); -} - if (!upcall->xout.slow) { ofpbuf_use_const(&upcall->put_actions, odp_actions->data, odp_actions->size); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 57d877f..e2ca960 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5033,7 +5033,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) { *xout = (struct xlate_out) { .slow = 0, -.fail_open = false, .recircs = RECIRC_REFS_EMPTY_INITIALIZER, }; @@ -5229,7 +5228,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) ctx.xin->resubmit_hook(ctx.xin, ctx.rule, 0); } } -xout->fail_open = ctx.rule && rule_dpif_is_fail_open(ctx.rule); /* Get the proximate input port of the packet. (If xin->recirc, * flow->in_port is the ultimate input port of the packet.) */ diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h index 02067a7..a135d8f 100644 --- a/ofproto/ofproto-dpif-xlate.h +++ b/ofproto/ofproto-dpif-xlate.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,7 +39,6 @@ struct xlate_cache; struct xlate_out { enum slow_path_reason slow; /* 0 if fast path may be used. */ -bool fail_open; /* Initial rule is fail open? */ struct recirc_refs recircs; /* Recirc action IDs on which references are * held. */ diff --git a/ofproto/pktbuf.c b/ofproto/pktbuf.c index def0c92..0ff2c6f 1
[ovs-dev] [PATCH 35/41] ofproto-dpif-rid: Use separate pointers for actions and action set.
During translation it makes some sense to concatenate these in a single array, but in my opinion it's conceptually better to separate them for the recirc_state; they are not naturally the same thing. Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-rid.c | 21 + ofproto/ofproto-dpif-rid.h | 8 ofproto/ofproto-dpif-xlate.c | 21 +++-- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index f4dc21f..f2d3c96 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -148,6 +148,10 @@ recirc_metadata_hash(const struct recirc_state *state) } hash = hash_int(state->mirrors, hash); hash = hash_int(state->action_set_len, hash); +if (state->action_set_len) { +hash = hash_bytes64(ALIGNED_CAST(const uint64_t *, state->action_set), +state->action_set_len, hash); +} if (state->ofpacts_len) { hash = hash_bytes64(ALIGNED_CAST(const uint64_t *, state->ofpacts), state->ofpacts_len, hash); @@ -168,9 +172,10 @@ recirc_metadata_equal(const struct recirc_state *a, && !memcmp(a->stack, b->stack, a->n_stack * sizeof *a->stack) && a->mirrors == b->mirrors && a->conntracked == b->conntracked -&& a->action_set_len == b->action_set_len && ofpacts_equal(a->ofpacts, a->ofpacts_len, - b->ofpacts, b->ofpacts_len)); + b->ofpacts, b->ofpacts_len) +&& ofpacts_equal(a->action_set, a->action_set_len, + b->action_set, b->action_set_len)); } /* Lockless RCU protected lookup. If node is needed accross RCU quiescent @@ -210,14 +215,21 @@ recirc_state_clone(struct recirc_state *new, const struct recirc_state *old, flow_tnl_copy__(tunnel, old->metadata.tunnel); new->metadata.tunnel = tunnel; -if (new->n_stack) { -new->stack = xmemdup(new->stack, new->n_stack * sizeof *new->stack); +if (new->stack) { +new->stack = (new->n_stack + ? xmemdup(new->stack, new->n_stack * sizeof *new->stack) + : NULL); } if (new->ofpacts) { new->ofpacts = (new->ofpacts_len ? xmemdup(new->ofpacts, new->ofpacts_len) : NULL); } +if (new->action_set) { +new->action_set = (new->action_set_len + ? xmemdup(new->action_set, new->action_set_len) + : NULL); +} } static void @@ -225,6 +237,7 @@ recirc_state_free(struct recirc_state *state) { free(state->stack); free(state->ofpacts); +free(state->action_set); } /* Allocate a unique recirculation id for the given set of flow metadata. diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 101bd6e..ba968ba 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -146,10 +146,10 @@ struct recirc_state { bool conntracked; /* Conntrack occurred prior to recirc. */ /* Actions to be translated on recirculation. */ -uint32_t action_set_len; /* How much of 'ofpacts' consists of an - * action set? */ -uint32_t ofpacts_len; /* Size of 'ofpacts', in bytes. */ -struct ofpact *ofpacts; /* Sequence of "struct ofpacts". */ +struct ofpact *ofpacts; +size_t ofpacts_len; /* Size of 'ofpacts', in bytes. */ +struct ofpact *action_set; +size_t action_set_len;/* Size of 'action_set', in bytes. */ }; /* This maps a recirculation ID to saved state that flow translation can diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 6d67e91..1140652 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3630,9 +3630,11 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table) .n_stack = ctx->stack.size / sizeof(union mf_subvalue), .mirrors = ctx->mirrors, .conntracked = ctx->conntracked, +.ofpacts = ((struct ofpact *) ctx->action_set.data ++ ctx->recirc_action_offset / sizeof(struct ofpact)), +.ofpacts_len = ctx->action_set.size - ctx->recirc_action_offset, +.action_set = ctx->action_set.data, .action_set_len = ctx->recirc_action_offset, -.ofpacts_len = ctx->action_set.size, -.ofpacts = ctx->action_set.data, }; /* Allocate a unique recirc id for the given metadata state in the @@ -5176,11 +5178,12 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) const struct ofpact *a; xlate_report_actions(&ctx, "- Restoring action set", - state->ofpacts, state->action_set_len); + state->action_set, stat
[ovs-dev] [PATCH 31/41] openflow: Better abstract handling of packet-in messages.
Packet-in messages have been a bit of a mess. First, their abstraction in the form of struct ofputil_packet_in has some fields that are used in a clear way for incoming and outgoing packet-ins, and others (packet_len, total_len, buffer_id) have have confusing meanings or usage pattern depending on their direction. Second, it's very confusing how a packet-in has both a reason (OFPR_*) and a miss type (OFPROTO_PACKET_IN_*) and how those add up to the actual reason that is used "on the wire" for each OpenFlow version (and even whether the packet-in is sent at all!). Finally, there's all kind of low-level detail randomly scattered between connmgr, ofproto-dpif-xlate, and ofp-util. This commit attempts to clear up some of the confusion. It simplifies the struct ofputil_packet_in abstraction by removing the members that didn't have a clear and consistent meaning between incoming and outgoing packet-ins. It gets rid of OFPROTO_PACKET_IN_*, instead adding a couple of nonstandard OFPR_* reasons that add up to what OFPROTO_PACKET_IN_* was meant to say (in what I hope is a clearer way). And it consolidates the tricky parts into ofp-util, where I hope it will be easier to understand all in one place. Signed-off-by: Ben Pfaff --- include/openflow/openflow-common.h | 5 + lib/learning-switch.c | 18 +-- lib/ofp-print.c| 27 +++-- lib/ofp-util.c | 233 + lib/ofp-util.h | 40 --- ofproto/connmgr.c | 140 -- ofproto/connmgr.h | 22 +--- ofproto/fail-open.c| 23 ++-- ofproto/ofproto-dpif-xlate.c | 47 ofproto/ofproto-dpif.c | 2 +- ofproto/ofproto.c | 2 +- ovn/controller/pinctrl.c | 4 +- 12 files changed, 270 insertions(+), 293 deletions(-) diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h index 3985705..da2b7a5 100644 --- a/include/openflow/openflow-common.h +++ b/include/openflow/openflow-common.h @@ -274,6 +274,7 @@ enum ofp_capabilities { /* Why is this packet being sent to the controller? */ enum ofp_packet_in_reason { +/* Standard reasons. */ OFPR_NO_MATCH, /* No matching flow. */ OFPR_ACTION,/* Action explicitly output to controller. */ OFPR_INVALID_TTL, /* Packet has invalid TTL. */ @@ -287,6 +288,10 @@ enum ofp_packet_in_reason { (OFPR10_BITS | \ (1u << OFPR_ACTION_SET) | (1u << OFPR_GROUP) | (1u << OFPR_PACKET_OUT)) +/* Nonstandard reason--not exposed via OpenFlow. */ +OFPR_EXPLICIT_MISS, +OFPR_IMPLICIT_MISS, + OFPR_N_REASONS }; diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 2b764f6..35c3fef 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -512,6 +512,8 @@ static void process_packet_in(struct lswitch *sw, const struct ofp_header *oh) { struct ofputil_packet_in pi; +size_t total_len; +uint32_t buffer_id; uint32_t queue_id; ofp_port_t out_port; @@ -524,7 +526,7 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) struct dp_packet pkt; struct flow flow; -error = ofputil_decode_packet_in(&pi, oh); +error = ofputil_decode_packet_in(oh, &pi, &total_len, &buffer_id); if (error) { VLOG_WARN_RL(&rl, "failed to decode packet-in: %s", ofperr_to_string(error)); @@ -538,8 +540,8 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) return; } -/* Extract flow data from 'opi' into 'flow'. */ -dp_packet_use_const(&pkt, pi.packet, pi.packet_len); +/* Extract flow data from 'pi' into 'flow'. */ +dp_packet_use_const(&pkt, pi.packet, pi.len); flow_extract(&pkt, &flow); flow.in_port.ofp_port = pi.flow_metadata.flow.in_port.ofp_port; flow.tunnel.tun_id = pi.flow_metadata.flow.tunnel.tun_id; @@ -562,8 +564,8 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) } /* Prepare packet_out in case we need one. */ -po.buffer_id = pi.buffer_id; -if (po.buffer_id == UINT32_MAX) { +po.buffer_id = buffer_id; +if (buffer_id == UINT32_MAX) { po.packet = dp_packet_data(&pkt); po.packet_len = dp_packet_size(&pkt); } else { @@ -583,7 +585,7 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) .table_id = 0xff, .command = OFPFC_ADD, .idle_timeout = sw->max_idle, -.buffer_id = pi.buffer_id, +.buffer_id = buffer_id, .out_port = OFPP_NONE, .ofpacts = ofpacts.data, .ofpacts_len = ofpacts.size, @@ -596,13 +598,13 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) queue_tx(sw, buffer); /* If the switch didn't b
[ovs-dev] [PATCH 37/41] ofproto-dpif-rid: Fix names of recirc_metadata_{hash, equal}().
These functions actually hash or compare recirc_state structs, so they should be named that way; recirc_metadata is only a small subset of recirc_state. Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-rid.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index f2d3c96..eab5baa 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -126,7 +126,7 @@ recirc_id_node_find(uint32_t id) } static uint32_t -recirc_metadata_hash(const struct recirc_state *state) +recirc_state_hash(const struct recirc_state *state) { uint32_t hash; @@ -160,7 +160,7 @@ recirc_metadata_hash(const struct recirc_state *state) } static bool -recirc_metadata_equal(const struct recirc_state *a, +recirc_state_equal(const struct recirc_state *a, const struct recirc_state *b) { return (a->table_id == b->table_id @@ -186,7 +186,7 @@ recirc_find_equal(const struct recirc_state *target, uint32_t hash) struct recirc_id_node *node; CMAP_FOR_EACH_WITH_HASH (node, metadata_node, hash, &metadata_map) { -if (recirc_metadata_equal(&node->state, target)) { +if (recirc_state_equal(&node->state, target)) { return node; } } @@ -283,7 +283,7 @@ recirc_alloc_id__(const struct recirc_state *state, uint32_t hash) uint32_t recirc_find_id(const struct recirc_state *target) { -uint32_t hash = recirc_metadata_hash(target); +uint32_t hash = recirc_state_hash(target); struct recirc_id_node *node = recirc_find_equal(target, hash); return node ? node->id : 0; } @@ -293,7 +293,7 @@ recirc_find_id(const struct recirc_state *target) uint32_t recirc_alloc_id_ctx(const struct recirc_state *state) { -uint32_t hash = recirc_metadata_hash(state); +uint32_t hash = recirc_state_hash(state); struct recirc_id_node *node = recirc_ref_equal(state, hash); if (!node) { node = recirc_alloc_id__(state, hash); @@ -313,7 +313,7 @@ recirc_alloc_id(struct ofproto_dpif *ofproto) .ofproto = ofproto, .metadata = { .tunnel = &tunnel, .in_port = OFPP_NONE }, }; -return recirc_alloc_id__(&state, recirc_metadata_hash(&state))->id; +return recirc_alloc_id__(&state, recirc_state_hash(&state))->id; } static void -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 32/41] connmgr: Generalize ofproto_packet_in to ofproto_async_msg.
An upcoming commit will add another kind of asynchronous message that should be handled in the same way as packet-ins. Signed-off-by: Ben Pfaff --- ofproto/connmgr.c| 29 ofproto/connmgr.h| 20 +- ofproto/fail-open.c | 28 +++- ofproto/ofproto-dpif-xlate.c | 32 +++--- ofproto/ofproto-dpif.c | 63 +--- ofproto/ofproto-dpif.h | 6 ++--- 6 files changed, 98 insertions(+), 80 deletions(-) diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index fb8f251..5161a15 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -1653,31 +1653,31 @@ connmgr_send_flow_removed(struct connmgr *mgr, * * The caller doesn't need to fill in pin->buffer_id or pin->total_len. */ void -connmgr_send_packet_in(struct connmgr *mgr, - const struct ofproto_packet_in *pin) +connmgr_send_async_msg(struct connmgr *mgr, + const struct ofproto_async_msg *am) { struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { enum ofputil_protocol protocol = ofconn_get_protocol(ofconn); if (protocol == OFPUTIL_P_NONE || !rconn_is_connected(ofconn->rconn) -|| ofconn->controller_id != pin->controller_id -|| !ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, - pin->up.reason)) { +|| ofconn->controller_id != am->controller_id +|| !ofconn_receives_async_msg(ofconn, am->oam, + am->pin.up.reason)) { continue; } struct ofpbuf *msg = ofputil_encode_packet_in( -&pin->up, protocol, ofconn->packet_in_format, -pin->max_len >= 0 ? pin->max_len : ofconn->miss_send_len, +&am->pin.up, protocol, ofconn->packet_in_format, +am->pin.max_len >= 0 ? am->pin.max_len : ofconn->miss_send_len, ofconn->pktbuf); struct ovs_list txq; -bool is_miss = (pin->up.reason == OFPR_NO_MATCH || -pin->up.reason == OFPR_EXPLICIT_MISS || -pin->up.reason == OFPR_IMPLICIT_MISS); +bool is_miss = (am->pin.up.reason == OFPR_NO_MATCH || +am->pin.up.reason == OFPR_EXPLICIT_MISS || +am->pin.up.reason == OFPR_IMPLICIT_MISS); pinsched_send(ofconn->schedulers[is_miss], - pin->up.flow_metadata.flow.in_port.ofp_port /* XXX */, + am->pin.up.flow_metadata.flow.in_port.ofp_port /* XXX */, msg, &txq); do_send_packet_ins(ofconn, &txq); } @@ -2243,3 +2243,10 @@ ofmonitor_wait(struct connmgr *mgr) } ovs_mutex_unlock(&ofproto_mutex); } + +void +ofproto_async_msg_free(struct ofproto_async_msg *am) +{ +free(CONST_CAST(void *, am->pin.up.packet)); +free(am); +} diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index ced6a68..fb7573e 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -54,13 +54,21 @@ enum ofconn_type { OFCONN_SERVICE /* A service connection, e.g. "ovs-ofctl". */ }; -/* A packet_in, with extra members to assist in queuing and routing it. */ -struct ofproto_packet_in { -struct ofputil_packet_in up; +/* An asynchronous message that might need to be queued between threads. */ +struct ofproto_async_msg { struct ovs_list list_node; /* For queuing. */ uint16_t controller_id; /* Controller ID to send to. */ -int max_len;/* From action, or -1 if none. */ + +enum ofputil_async_msg_type oam; +union { +/* OAM_PACKET_IN. */ +struct { +struct ofputil_packet_in up; +int max_len;/* From action, or -1 if none. */ +} pin; +}; }; +void ofproto_async_msg_free(struct ofproto_async_msg *); /* Basics. */ struct connmgr *connmgr_create(struct ofproto *ofproto, @@ -140,8 +148,8 @@ void connmgr_send_port_status(struct connmgr *, struct ofconn *source, const struct ofputil_phy_port *, uint8_t reason); void connmgr_send_flow_removed(struct connmgr *, const struct ofputil_flow_removed *); -void connmgr_send_packet_in(struct connmgr *, -const struct ofproto_packet_in *); +void connmgr_send_async_msg(struct connmgr *, +const struct ofproto_async_msg *); void ofconn_send_role_status(struct ofconn *ofconn, uint32_t role, uint8_t reason); diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c index 5a692bf..a28acef 100644 --- a/ofproto/fail-open.c +++ b/ofproto/fail-open.c @@ -125,19 +125,23 @@ send_bogus_packet_ins(struct fail_open *fo) eth_addr_nicira_random(&mac); compose_rarp(&b, mac); -struct ofproto_packet_in pin = { -
[ovs-dev] [PATCH 36/41] ofproto-dpif-rid: Don't carry actset_output explicitly in metadata.
Instead reconstruct it using the action set, since we already have the logic to do that. This seems a little nicer because we don't have to "trust" the metadata as much. Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-rid.h | 3 --- ofproto/ofproto-dpif-xlate.c | 40 ++-- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index ba968ba..654c95c 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -101,7 +101,6 @@ struct recirc_metadata { ovs_be64 metadata;/* OpenFlow Metadata. */ uint64_t regs[FLOW_N_XREGS]; /* Registers. */ ofp_port_t in_port; /* Incoming port. */ -ofp_port_t actset_output; /* Output port in action set. */ }; static inline void @@ -113,7 +112,6 @@ recirc_metadata_from_flow(struct recirc_metadata *md, md->metadata = flow->metadata; memcpy(md->regs, flow->regs, sizeof md->regs); md->in_port = flow->in_port.ofp_port; -md->actset_output = flow->actset_output; } static inline void @@ -128,7 +126,6 @@ recirc_metadata_to_flow(const struct recirc_metadata *md, flow->metadata = md->metadata; memcpy(flow->regs, md->regs, sizeof flow->regs); flow->in_port.ofp_port = md->in_port; -flow->actset_output = md->actset_output; } /* State that flow translation can save, to restore when recirculation diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 1140652..e3b46ab 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -4074,12 +4074,9 @@ may_receive(const struct xport *xport, struct xlate_ctx *ctx) } static void -xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a) +xlate_write_actions__(struct xlate_ctx *ctx, + const struct ofpact *ofpacts, size_t ofpacts_len) { -const struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a); -size_t on_len = ofpact_nest_get_action_len(on); -const struct ofpact *inner; - /* Maintain actset_output depending on the contents of the action set: * * - OFPP_UNSET, if there is no "output" action. @@ -4090,10 +4087,11 @@ xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a) * - OFPP_UNSET, if there is a "group" action. */ if (!ctx->action_set_has_group) { -OFPACT_FOR_EACH (inner, on->actions, on_len) { -if (inner->type == OFPACT_OUTPUT) { -ctx->xin->flow.actset_output = ofpact_get_OUTPUT(inner)->port; -} else if (inner->type == OFPACT_GROUP) { +const struct ofpact *a; +OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { +if (a->type == OFPACT_OUTPUT) { +ctx->xin->flow.actset_output = ofpact_get_OUTPUT(a)->port; +} else if (a->type == OFPACT_GROUP) { ctx->xin->flow.actset_output = OFPP_UNSET; ctx->action_set_has_group = true; break; @@ -4101,7 +4099,13 @@ xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact *a) } } -ofpbuf_put(&ctx->action_set, on->actions, on_len); +ofpbuf_put(&ctx->action_set, ofpacts, ofpacts_len); +} + +static void +xlate_write_actions(struct xlate_ctx *ctx, const struct ofpact_nest *a) +{ +xlate_write_actions__(ctx, a->actions, ofpact_nest_get_action_len(a)); } static void @@ -4723,7 +4727,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, break; case OFPACT_WRITE_ACTIONS: -xlate_write_actions(ctx, a); +xlate_write_actions(ctx, ofpact_get_WRITE_ACTIONS(a)); break; case OFPACT_WRITE_METADATA: @@ -5175,20 +5179,12 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) /* Restore action set, if any. */ if (state->action_set_len) { -const struct ofpact *a; - xlate_report_actions(&ctx, "- Restoring action set", state->action_set, state->action_set_len); -ofpbuf_put(&ctx.action_set, - state->action_set, state->action_set_len); - -OFPACT_FOR_EACH (a, state->action_set, state->action_set_len) { -if (a->type == OFPACT_GROUP) { -ctx.action_set_has_group = true; -break; -} -} +flow->actset_output = OFPP_UNSET; +xlate_write_actions__(&ctx, state->action_set, + state->action_set_len); } /* Restore recirculation actions. If there are no actions, processing -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 33/41] hash: New helper functions hash_bytes32() and hash_bytes64().
All of the callers of hash_words() and hash_words64() actually find it easier to pass in the number of bytes instead of the number of 32-bit or 64-bit words. These new functions allow the callers to be a little simpler. Signed-off-by: Ben Pfaff --- lib/flow.h | 5 ++--- lib/hash.h | 14 +- lib/odp-util.c | 5 ++--- lib/ofp-msgs.c | 6 +++--- ofproto/ofproto-dpif-rid.c | 19 --- 5 files changed, 28 insertions(+), 21 deletions(-) diff --git a/lib/flow.h b/lib/flow.h index 5d78615..dc7130d 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -263,8 +263,7 @@ flow_equal(const struct flow *a, const struct flow *b) static inline size_t flow_hash(const struct flow *flow, uint32_t basis) { -return hash_words64((const uint64_t *)flow, -sizeof *flow / sizeof(uint64_t), basis); +return hash_bytes64((const uint64_t *)flow, sizeof *flow, basis); } static inline uint16_t diff --git a/lib/hash.h b/lib/hash.h index 65c52b9..114a419 100644 --- a/lib/hash.h +++ b/lib/hash.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -323,6 +323,18 @@ hash_words64(const uint64_t p[], size_t n_words, uint32_t basis) } #endif +static inline uint32_t +hash_bytes32(const uint32_t p[], size_t n_bytes, uint32_t basis) +{ +return hash_words(p, n_bytes / 4, basis); +} + +static inline uint32_t +hash_bytes64(const uint64_t p[], size_t n_bytes, uint32_t basis) +{ +return hash_words64(p, n_bytes / 8, basis); +} + static inline uint32_t hash_string(const char *s, uint32_t basis) { return hash_bytes(s, strlen(s), basis); diff --git a/lib/odp-util.c b/lib/odp-util.c index f16e113..6271601 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -4532,8 +4532,7 @@ uint32_t odp_flow_key_hash(const struct nlattr *key, size_t key_len) { BUILD_ASSERT_DECL(!(NLA_ALIGNTO % sizeof(uint32_t))); -return hash_words(ALIGNED_CAST(const uint32_t *, key), - key_len / sizeof(uint32_t), 0); +return hash_bytes32(ALIGNED_CAST(const uint32_t *, key), key_len, 0); } static void diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c index cb27f79..944ab33 100644 --- a/lib/ofp-msgs.c +++ b/lib/ofp-msgs.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -118,8 +118,8 @@ alloc_xid(void) static uint32_t ofphdrs_hash(const struct ofphdrs *hdrs) { -BUILD_ASSERT_DECL(sizeof *hdrs == 12); -return hash_words((const uint32_t *) hdrs, 3, 0); +BUILD_ASSERT_DECL(sizeof *hdrs % 4 == 0); +return hash_bytes32((const uint32_t *) hdrs, sizeof *hdrs, 0); } static bool diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index d142933..cb00301 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -135,25 +135,22 @@ recirc_metadata_hash(const struct recirc_state *state) if (flow_tnl_dst_is_set(state->metadata.tunnel)) { /* We may leave remainder bytes unhashed, but that is unlikely as * the tunnel is not in the datapath format. */ -hash = hash_words64((const uint64_t *) state->metadata.tunnel, -flow_tnl_size(state->metadata.tunnel) -/ sizeof(uint64_t), hash); +hash = hash_bytes64((const uint64_t *) state->metadata.tunnel, +flow_tnl_size(state->metadata.tunnel), hash); } hash = hash_boolean(state->conntracked, hash); -hash = hash_words64((const uint64_t *) &state->metadata.metadata, -(sizeof state->metadata - sizeof state->metadata.tunnel) -/ sizeof(uint64_t), +hash = hash_bytes64((const uint64_t *) &state->metadata.metadata, +sizeof state->metadata - sizeof state->metadata.tunnel, hash); if (state->stack && state->stack->size != 0)
[ovs-dev] [PATCH 38/41] ofproto-dpif-rid: Use UUID, not pointer, to identify ofprotos for recirc.
An upcoming commit will make it possible to essentially serialize the recirculation state into an OpenFlow message. For that purpose, we can't sensibly pass a "struct ofproto *", but a randomly generated UUID works just as well. Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-rid.c | 9 + ofproto/ofproto-dpif-rid.h | 5 +++-- ofproto/ofproto-dpif-xlate.c | 22 +++--- ofproto/ofproto-dpif.c | 11 +++ ofproto/ofproto-dpif.h | 4 +++- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index eab5baa..ba6237d 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -130,7 +130,7 @@ recirc_state_hash(const struct recirc_state *state) { uint32_t hash; -hash = hash_pointer(state->ofproto, 0); +hash = uuid_hash(&state->ofproto_uuid); hash = hash_int(state->table_id, hash); if (flow_tnl_dst_is_set(state->metadata.tunnel)) { /* We may leave remainder bytes unhashed, but that is unlikely as @@ -164,7 +164,7 @@ recirc_state_equal(const struct recirc_state *a, const struct recirc_state *b) { return (a->table_id == b->table_id -&& a->ofproto == b->ofproto +&& uuid_equals(&a->ofproto_uuid, &b->ofproto_uuid) && flow_tnl_equal(a->metadata.tunnel, b->metadata.tunnel) && !memcmp(&a->metadata.metadata, &b->metadata.metadata, sizeof a->metadata - sizeof a->metadata.tunnel) @@ -310,7 +310,7 @@ recirc_alloc_id(struct ofproto_dpif *ofproto) tunnel.ipv6_dst = in6addr_any; struct recirc_state state = { .table_id = TBL_INTERNAL, -.ofproto = ofproto, +.ofproto_uuid = *ofproto_dpif_get_uuid(ofproto), .metadata = { .tunnel = &tunnel, .in_port = OFPP_NONE }, }; return recirc_alloc_id__(&state, recirc_state_hash(&state))->id; @@ -363,8 +363,9 @@ recirc_free_ofproto(struct ofproto_dpif *ofproto, const char *ofproto_name) { struct recirc_id_node *n; +const struct uuid *ofproto_uuid = ofproto_dpif_get_uuid(ofproto); CMAP_FOR_EACH (n, metadata_node, &metadata_map) { -if (n->state.ofproto == ofproto) { +if (uuid_equals(&n->state.ofproto_uuid, ofproto_uuid)) { VLOG_ERR("recirc_id %"PRIu32 " left allocated when ofproto (%s)" " is destructed", n->id, ofproto_name); diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 654c95c..04ec037 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2015 Nicira, Inc. + * Copyright (c) 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ #include "ofp-actions.h" #include "ofproto-dpif-mirror.h" #include "ovs-thread.h" +#include "uuid.h" struct ofproto_dpif; struct rule; @@ -135,7 +136,7 @@ struct recirc_state { uint8_t table_id; /* Pipeline context for post-recirculation processing. */ -struct ofproto_dpif *ofproto; /* Post-recirculation bridge. */ +struct uuid ofproto_uuid; /* Post-recirculation bridge. */ struct recirc_metadata metadata; /* Flow metadata. */ union mf_subvalue *stack; /* Stack if any. */ size_t n_stack; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index e3b46ab..8ab4b2a 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -506,6 +506,8 @@ static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port, static struct xbridge *xbridge_lookup(struct xlate_cfg *, const struct ofproto_dpif *); +static struct xbridge *xbridge_lookup_by_uuid(struct xlate_cfg *, + const struct uuid *); static struct xbundle *xbundle_lookup(struct xlate_cfg *, const struct ofbundle *); static struct xport *xport_lookup(struct xlate_cfg *, @@ -1233,6 +1235,19 @@ xbridge_lookup(struct xlate_cfg *xcfg, const struct ofproto_dpif *ofproto) return NULL; } +static struct xbridge * +xbridge_lookup_by_uuid(struct xlate_cfg *xcfg, const struct uuid *uuid) +{ +struct xbridge *xbridge; + +HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) { +if (uuid_equals(ofproto_dpif_get_uuid(xbridge->ofproto), uuid)) { +return xbridge; +} +} +return NULL; +} + static struct xbundle * xbundle_lookup(struct xlate_cfg *xcfg, const struct ofbundle *ofbundle) { @@ -3624,7 +3639,7 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table) struct recirc_state state = { .table_id = table, -.ofproto = ctx->xbridge->ofproto, +.ofproto_uuid = *ofproto_dpif_get_uuid(ctx->xbridge
[ovs-dev] [PATCH 34/41] ofproto-dpif-rid: Use array instead of ofpbuf for recirc_state stack.
In my opinion, this makes better sense for the stack, because it's not a packet or a collection of bytes, it's an array of struct mf_subvalue. (I left it as an ofpbuf for accumulating stack entries during translation, because the automatic reallocation and especially the stub support there is helpful.) Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-rid.c | 17 - ofproto/ofproto-dpif-rid.h | 3 ++- ofproto/ofproto-dpif-xlate.c | 6 -- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index cb00301..f4dc21f 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -142,9 +142,9 @@ recirc_metadata_hash(const struct recirc_state *state) hash = hash_bytes64((const uint64_t *) &state->metadata.metadata, sizeof state->metadata - sizeof state->metadata.tunnel, hash); -if (state->stack && state->stack->size != 0) { -hash = hash_bytes64((const uint64_t *) state->stack->data, -state->stack->size, hash); +if (state->stack && state->n_stack) { +hash = hash_bytes64((const uint64_t *) state->stack, +state->n_stack * sizeof *state->stack, hash); } hash = hash_int(state->mirrors, hash); hash = hash_int(state->action_set_len, hash); @@ -164,9 +164,8 @@ recirc_metadata_equal(const struct recirc_state *a, && flow_tnl_equal(a->metadata.tunnel, b->metadata.tunnel) && !memcmp(&a->metadata.metadata, &b->metadata.metadata, sizeof a->metadata - sizeof a->metadata.tunnel) -&& (((!a->stack || !a->stack->size) && - (!b->stack || !b->stack->size)) -|| (a->stack && b->stack && ofpbuf_equal(a->stack, b->stack))) +&& a->n_stack == b->n_stack +&& !memcmp(a->stack, b->stack, a->n_stack * sizeof *a->stack) && a->mirrors == b->mirrors && a->conntracked == b->conntracked && a->action_set_len == b->action_set_len @@ -211,8 +210,8 @@ recirc_state_clone(struct recirc_state *new, const struct recirc_state *old, flow_tnl_copy__(tunnel, old->metadata.tunnel); new->metadata.tunnel = tunnel; -if (new->stack) { -new->stack = new->stack->size ? ofpbuf_clone(new->stack) : NULL; +if (new->n_stack) { +new->stack = xmemdup(new->stack, new->n_stack * sizeof *new->stack); } if (new->ofpacts) { new->ofpacts = (new->ofpacts_len @@ -224,7 +223,7 @@ recirc_state_clone(struct recirc_state *new, const struct recirc_state *old, static void recirc_state_free(struct recirc_state *state) { -ofpbuf_delete(state->stack); +free(state->stack); free(state->ofpacts); } diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 4bbc7bf..101bd6e 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -140,7 +140,8 @@ struct recirc_state { /* Pipeline context for post-recirculation processing. */ struct ofproto_dpif *ofproto; /* Post-recirculation bridge. */ struct recirc_metadata metadata; /* Flow metadata. */ -struct ofpbuf *stack; /* Stack if any. */ +union mf_subvalue *stack; /* Stack if any. */ +size_t n_stack; mirror_mask_t mirrors;/* Mirrors already output. */ bool conntracked; /* Conntrack occurred prior to recirc. */ diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 2ee647b..6d67e91 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3626,7 +3626,8 @@ compose_recirculate_action__(struct xlate_ctx *ctx, uint8_t table) .table_id = table, .ofproto = ctx->xbridge->ofproto, .metadata = md, -.stack = &ctx->stack, +.stack = ctx->stack.data, +.n_stack = ctx->stack.size / sizeof(union mf_subvalue), .mirrors = ctx->mirrors, .conntracked = ctx->conntracked, .action_set_len = ctx->recirc_action_offset, @@ -5163,7 +5164,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) /* Restore stack, if any. */ if (state->stack) { -ofpbuf_put(&ctx.stack, state->stack->data, state->stack->size); +ofpbuf_put(&ctx.stack, state->stack, + state->n_stack * sizeof *state->stack); } /* Restore mirror state. */ -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 41/41] [RFC] Implement "closures".
One purpose of OpenFlow packet-in messages is to allow a controller to interpose on the path of a packet through the flow tables. If, for example, the controller needs to modify a packet in some way that the switch doesn't directly support, the controller should be able to program the switch to send it the packet, then modify the packet and send it back to the switch to continue through the flow table. That's the theory. In practice, this doesn't work with any but the simplest flow tables. Packet-in messages simply don't include enough context to allow the flow table traversal to continue. For example: * Via "resubmit" actions, an Open vSwitch packet can have an effective "call stack", but a packet-in can't describe it, and so it would be lost. * Via "patch ports", an Open vSwitch packet can traverse multiple OpenFlow logical switches. A packet-in can't describe or resume this context. * A packet-in can't preserve the stack used by NXAST_PUSH and NXAST_POP actions. * A packet-in can't preserve the OpenFlow 1.1+ action set. * A packet-in can't preserve the state of Open vSwitch mirroring or connection tracking. This commit introduces a solution called "closures". A closure is the state of a packet's traversal through OpenFlow flow tables. A new NXAST_PAUSE action generates a closure and sends it to the OpenFlow controller as an NXT_CLOSURE asynchronous message (which must be enabled through an extension to OFPT_SET_ASYNC or NXT_SET_ASYNC2). The controller processes the closure, possibly modifying some of its publicly accessible data (for now, the packet and its metadata), and sends it back to the switch with an NXT_RESUME request, which causes flow table traversal to continue. In principle, a single packet can be paused and resumed multiple times. This RFC patch has a number of caveats: * No real tests yet. * Needs more and better documentation. * Flow statistics are not yet correctly updated. Signed-off-by: Ben Pfaff --- NEWS | 4 + include/openflow/nicira-ext.h | 79 +- lib/meta-flow.c | 9 +- lib/meta-flow.h | 3 +- lib/ofp-actions.c | 73 - lib/ofp-actions.h | 9 +- lib/ofp-errors.h | 10 +- lib/ofp-msgs.h| 8 + lib/ofp-print.c | 68 + lib/ofp-util.c| 341 ++ lib/ofp-util.h| 56 ++- lib/rconn.c | 4 +- ofproto/connmgr.c | 65 ++-- ofproto/connmgr.h | 3 + ofproto/ofproto-dpif-xlate.c | 112 -- ofproto/ofproto-dpif-xlate.h | 4 + ofproto/ofproto-dpif.c| 32 ofproto/ofproto-provider.h| 5 +- ofproto/ofproto.c | 25 tests/ofp-print.at| 6 + tests/ofproto-dpif.at | 24 +++ tests/ofproto.at | 2 + utilities/ovs-ofctl.c | 59 +++- vswitchd/vswitch.xml | 17 ++- 24 files changed, 965 insertions(+), 53 deletions(-) diff --git a/NEWS b/NEWS index 5c18867..b264701 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,10 @@ Post-v2.5.0 - OpenFlow: * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY. * OpenFlow 1.4+ OFPMP_QUEUE_DESC is now supported. + * New extension message NXT_SET_ASYNC_CONFIG2 to allow OpenFlow 1.4-like + control over asynchronous messages in earlier versions of OpenFlow. + * New "closure" extension to pause and resume flow table traversal. + See NXAST_PAUSE, NXT_CLOSURE, and NXT_RESUME for more information. - ovs-ofctl: * queue-get-config command now allows a queue ID to be specified. diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index dad8707..78e0a56 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -246,6 +246,83 @@ struct nx_packet_in { }; OFP_ASSERT(sizeof(struct nx_packet_in) == 24); +/* NXT_CLOSURE. + * + * A closure serializes the state of a packet's trip through Open vSwitch flow + * tables. This permits an OpenFlow controller to interpose on a packet midway + * through processing in Open vSwitch. + * + * Closures fit into packet processing this way: + * + * 1. A packet ingresses into Open vSwitch, which runs it through the OpenFlow + *tables. + * + * 2. An OpenFlow flow executes a NXAST_PAUSE action. Open vSwitch serializes + *the packet processing state into a closure and sends it (as an + *NXT_CLOSURE) to the OpenFlow controller. + * + * 3. The controller
[ovs-dev] [PATCH 39/41] nx-match: Add functions for raw decoding and encoding of OXM.
The existing functions either assumed that we were working with NXM (instead of OXM), or they added or parsed a header before OXM headers. These aren't needed when OXM is encapsulated inside a property, as will be the case in an upcoming commit. Signed-off-by: Ben Pfaff --- lib/nx-match.c | 30 -- lib/nx-match.h | 7 ++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/nx-match.c b/lib/nx-match.c index 11bcd95..0f695f0 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -630,6 +630,18 @@ oxm_pull_match_loose(struct ofpbuf *b, struct match *match) return oxm_pull_match__(b, false, match); } +/* Parses the OXM match description in the 'oxm_len' bytes in 'oxm'. Stores + * the result in 'match'. + * + * Fails with an error when encountering unknown OXM headers. + * + * Returns 0 if successful, otherwise an OpenFlow error code. */ +enum ofperr +oxm_decode_match(const void *oxm, size_t oxm_len, struct match *match) +{ +return nx_pull_raw(oxm, oxm_len, true, match, NULL, NULL); +} + /* Verify an array of OXM TLVs treating value of each TLV as a mask, * disallowing masks in each TLV and ignoring pre-requisites. */ enum ofperr @@ -1099,7 +,7 @@ nx_put_match(struct ofpbuf *b, const struct match *match, } /* Appends to 'b' an struct ofp11_match_header followed by the OXM format that - * expresses 'cr', plus enough zero bytes to pad the data appended out to a + * expresses 'match', plus enough zero bytes to pad the data appended out to a * multiple of 8. * * OXM differs slightly among versions of OpenFlow. Specify the OpenFlow @@ -1130,6 +1142,20 @@ oxm_put_match(struct ofpbuf *b, const struct match *match, return match_len; } +/* Appends to 'b' the OXM formats that expresses 'match', without header or + * padding. + * + * OXM differs slightly among versions of OpenFlow. Specify the OpenFlow + * version in use as 'version'. + * + * This function can cause 'b''s data to be reallocated. */ +void +oxm_put_raw(struct ofpbuf *b, const struct match *match, +enum ofp_version version) +{ +nx_put_raw(b, version, match, 0, 0); +} + /* Appends to 'b' the nx_match format that expresses the tlv corresponding * to 'id'. If mask is not all-ones then it is also formated as the value * of the tlv. */ diff --git a/lib/nx-match.h b/lib/nx-match.h index 2625c63..c663e54 100644 --- a/lib/nx-match.h +++ b/lib/nx-match.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -47,6 +47,7 @@ char *mf_parse_subfield__(struct mf_subfield *sf, const char **s) char *mf_parse_subfield(struct mf_subfield *, const char *s) OVS_WARN_UNUSED_RESULT; +/* Decoding matches. */ enum ofperr nx_pull_match(struct ofpbuf *, unsigned int match_len, struct match *, ovs_be64 *cookie, ovs_be64 *cookie_mask); @@ -55,11 +56,15 @@ enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int match_len, ovs_be64 *cookie_mask); enum ofperr oxm_pull_match(struct ofpbuf *, struct match *); enum ofperr oxm_pull_match_loose(struct ofpbuf *, struct match *); +enum ofperr oxm_decode_match(const void *, size_t, struct match *); enum ofperr oxm_pull_field_array(const void *, size_t fields_len, struct field_array *); + +/* Encoding matches. */ int nx_put_match(struct ofpbuf *, const struct match *, ovs_be64 cookie, ovs_be64 cookie_mask); int oxm_put_match(struct ofpbuf *, const struct match *, enum ofp_version); +void oxm_put_raw(struct ofpbuf *, const struct match *, enum ofp_version); void oxm_format_field_array(struct ds *, const struct field_array *); int oxm_put_field_array(struct ofpbuf *, const struct field_array *, enum ofp_version version); -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 40/41] ofproto-dpif-xlate: Put recirc_state, not recirc_id_node, in xlate_in.
This will make it possible, in an upcoming commit, to construct a recirc_state locally on the stack to pass to xlate_actions(). It would also be possible to construct and pass a recirc_id_node on the stack, but the translation process only uses the recirc_state anyway. The alternative here of having upcall_xlate() know that it can recover the recirc_id_node from the recirc_state isn't great either; it's debatable which is the better approach. Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-rid.h| 6 ++ ofproto/ofproto-dpif-upcall.c | 4 ++-- ofproto/ofproto-dpif-xlate.c | 11 --- ofproto/ofproto-dpif-xlate.h | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 04ec037..85ec24a 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -184,6 +184,12 @@ void recirc_free_ofproto(struct ofproto_dpif *, const char *ofproto_name); const struct recirc_id_node *recirc_id_node_find(uint32_t recirc_id); +static inline struct recirc_id_node * +recirc_id_node_from_state(const struct recirc_state *state) +{ +return CONTAINER_OF(state, struct recirc_id_node, state); +} + static inline bool recirc_id_node_try_ref_rcu(const struct recirc_id_node *n_) { struct recirc_id_node *node = CONST_CAST(struct recirc_id_node *, n_); diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index b505206..240bd92 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1074,8 +1074,8 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, * upcalls using recirculation ID for which no context can be * found). We may still execute the flow's actions even if we * don't install the flow. */ -upcall->recirc = xin.recirc; -upcall->have_recirc_ref = recirc_id_node_try_ref_rcu(xin.recirc); +upcall->recirc = recirc_id_node_from_state(xin.recirc); +upcall->have_recirc_ref = recirc_id_node_try_ref_rcu(upcall->recirc); } } else { /* For non-miss upcalls, we are either executing actions (one of which diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 8ab4b2a..184eb46 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -4828,9 +4828,14 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto, xin->odp_actions = odp_actions; /* Do recirc lookup. */ -xin->recirc = flow->recirc_id -? recirc_id_node_find(flow->recirc_id) -: NULL; +xin->recirc = NULL; +if (flow->recirc_id) { +const struct recirc_id_node *node += recirc_id_node_find(flow->recirc_id); +if (node) { +xin->recirc = &node->state; +} +} } void diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h index a135d8f..3b06285 100644 --- a/ofproto/ofproto-dpif-xlate.h +++ b/ofproto/ofproto-dpif-xlate.h @@ -142,7 +142,7 @@ struct xlate_in { /* The recirculation context related to this translation, as returned by * xlate_lookup. */ -const struct recirc_id_node *recirc; +const struct recirc_state *recirc; }; void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *, -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto: Fix memory leak and memory exhaustion bugs in group_mod.
I reposted this as the first patch in a new series: https://patchwork.ozlabs.org/patch/569826/ Please review it there (although it hasn't changed). ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Mail System Error - Returned Mail
The original message was received at Sun, 1 Jan 2006 01:58:42 +0530 from openvswitch.org [207.4.66.82] - The following addresses had permanent fatal errors - ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev