Re: [ovs-dev] [PATCH] nf_nat_packet: Clear skb hash after modifying packet headers.
On Wed, Apr 20, 2016 at 02:31:10PM -0700, Jarno Rajahalme wrote:
> Clear the skb hash when it does not reflect the actual header values
> any more.
>
> Signed-off-by: Jarno Rajahalme
> ---
> net/netfilter/nf_nat_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 06a9f45..3c2302f 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -505,6 +505,7 @@ unsigned int nf_nat_packet(struct nf_conn *ct,
> if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype))
> return NF_DROP;
> }
> + skb_clear_hash(skb);
> return NF_ACCEPT;
> }
Cc'ing Florian.
This seems to affect the new tracing infrastructure for nf_tables:
31 static int trace_fill_id(struct sk_buff *nlskb, struct sk_buff
*skb)
32 {
33 __be32 id;
34
35 /* using skb address as ID results in a limited number of
36 * values (and quick reuse).
37 *
38 * So we attempt to use as many skb members that will not
39 * change while skb is with netfilter.
40 */
41 id = (__be32)jhash_2words(hash32_ptr(skb), skb_get_hash(skb),
42 skb->skb_iif);
43
44 return nla_put_be32(nlskb, NFTA_TRACE_ID, id);
45 }
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] nf_nat_packet: Clear skb hash after modifying packet headers.
Pablo Neira Ayuso wrote: > On Wed, Apr 20, 2016 at 02:31:10PM -0700, Jarno Rajahalme wrote: > > Clear the skb hash when it does not reflect the actual header values > > any more. > > > > Signed-off-by: Jarno Rajahalme > > --- > > net/netfilter/nf_nat_core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c > > index 06a9f45..3c2302f 100644 > > --- a/net/netfilter/nf_nat_core.c > > +++ b/net/netfilter/nf_nat_core.c > > @@ -505,6 +505,7 @@ unsigned int nf_nat_packet(struct nf_conn *ct, > > if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype)) > > return NF_DROP; > > } > > + skb_clear_hash(skb); > > return NF_ACCEPT; > > } > > Cc'ing Florian. > > This seems to affect the new tracing infrastructure for nf_tables: Right. Jarno, what is your patch trying to fix? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v12 2/6] netdev-dpdk: Convert initialization from cmdline to db
Daniele Di Proietto writes:
> Hi Aaron,
>
> thanks (again!) for this patch. Few comments:
>
> * As I mentioned on my previous round of review, I don't think it's necessary
> to pass the whole Open_vSwitch table to dpdk_init(). I.e., instead of doing
>
> dpdk_init(&cfg);
>
> I'd prefer
>
> if (cfg) {
> dpdk_init(&cfg->other_config);
> }
>
> This way we don't have to include "vswitch-idl.h".
>
>
> * I suggested 'dpdk-mem-channels', because I think people who want to use
> that will be comfortable passing '-n' in dpdk-extra. What do you think? Is
> there any reason why you think it's worth keeping?
D'oh! I had done both of these changes, but they were dropped during the
rebase. I'll cook a fix asap.
> * Sorry for not noticing this before: it seems that netdev_dpdk_register()
> now always registers the netdev classes, even though they cannot be created.
> The registered classes end up in the database in the iface_type column of the
> Open_vSwitch table, so controllers might think that they're available. I
> think we should register the classes only when DPDK is initialized.
I had an issue doing this, back when the I had the lazy
initialization. I don't remember the details, though. I'll try it again,
and see what happens.
> Two minor nits inline,
>
> Thanks
Thanks so much for the review, Daniele!
-Aaron
> On 26/04/2016 12:42, "Aaron Conole" wrote:
>
>>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
>>Acked-by: Kevin Traynor
>>---
>>Previous: http://openvswitch.org/pipermail/dev/2016-April/069028.html
>>
>>v12:
>>* Squashed NEWS change into this commit
>>
>> FAQ.md | 6 +-
>> INSTALL.DPDK.md| 82 +---
>> NEWS | 5 +
>> lib/automake.mk| 4 +
>> lib/netdev-dpdk.c | 304
>> +
>> lib/netdev-dpdk.h | 14 +--
>> lib/netdev-nodpdk.c| 21
>> tests/ofproto-macros.at| 3 +-
>> utilities/ovs-dev.py | 13 +-
>> vswitchd/bridge.c | 3 +
>> vswitchd/ovs-vswitchd.8.in | 6 +-
>> vswitchd/ovs-vswitchd.c| 27 +---
>> vswitchd/vswitch.xml | 120 ++
>> 13 files changed, 463 insertions(+), 145 deletions(-)
>> create mode 100644 lib/netdev-nodpdk.c
>>
>
> [...]
>
>>
>>@@ -2732,22 +2734,20 @@ static const struct dpdk_qos_ops egress_policer_ops =
>>{
>>
>> static int
>> process_vhost_flags(char *flag, char *default_val, int size,
>>-char **argv, char **new_val)
>>+const struct ovsrec_open_vswitch *ovs_cfg,
>>+char **new_val)
>> {
>>+const char *val;
>> int changed = 0;
>>
>>+val = smap_get(&ovs_cfg->other_config, flag);
>>+
>> /* Depending on which version of vhost is in use, process the
>> vhost-specific
>>- * flag if it is provided on the vswitchd command line, otherwise resort
>>to
>>- * a default value.
>>- *
>>- * For vhost-user: Process "-vhost_sock_dir" to set the custom location
>>of
>>- * the vhost-user socket(s).
>>- * For vhost-cuse: Process "-cuse_dev_name" to set the custom name of the
>>- * vhost-cuse character device.
>>+ * flag if it is provided, otherwise resort to default value.
>> */
>>-if (!strcmp(argv[1], flag) && (strlen(argv[2]) <= size)) {
>>+if (val && (strlen(val) <= size)) {
>> changed = 1;
>>-*new_val = xstrdup(argv[2]);
>>+*new_val = xstrdup(val);
>> VLOG_INFO("User-provided %s in use: %s", flag, *new_val);
>> } else {
>> VLOG_INFO("No %s provided - defaulting to %s", flag, default_val);
>>@@ -2757,68 +2757,186 @@ process_vhost_flags(char *flag, char *default_val,
>>int size,
>> return changed;
>> }
>>
>>-int
>>-dpdk_init(int argc, char **argv)
>>+static char **
>>+grow_argv(char ***argv, size_t cur_siz, size_t grow_by)
>> {
>>-int result;
>>-int base = 0;
>>-char *pragram_name = argv[0];
>>-int err;
>>-int isset;
>>-cpu_set_t cpuset;
>>+return xrealloc(*argv, sizeof(char *) * (cur_siz + grow_by));
>>+}
>>
>>-if (argc < 2 || strcmp(argv[1], "--dpdk"))
>>-return 0;
>>+static void
>>+dpdk_option_extend(char ***argv, int argc, const char *option,
>>+ const char *value)
>>+{
>>+char **newargv = grow_argv(argv, argc, 2);
>>+*argv = newargv;
>>+newargv[argc] = xstrdup(option);
>>+newargv[argc+1] = xstrdup(value);
>>+}
>>
>>-
Re: [ovs-dev] [PATCH v12 3/6] netdev-dpdk: Restrict vhost_sock_dir
Daniele Di Proietto writes:
> If vhost-sock-dir is empty, vhost_sock_dir will be NULL, because
> process_vhost_flags() will return 0.
D'oh! Missed this in testing. I'll fix it, and make sure I test this
case more thoroughly.
Thanks so much for the review, Daniele!
-Aaron
> On 26/04/2016 12:42, "Aaron Conole" wrote:
>
>>Since the vhost-user sockets directory now comes from the database, it is
>>possible for any user with database access to program an arbitrary filesystem
>>location for the sockets directory. This could result in unprivileged users
>>creating or deleting arbitrary filesystem files by using specially crafted
>>names. To prevent this, use the introduced ovs_realpath function to resolve
>>the filesystem location and ensure that it resolves under the ovs_rundir.
>>
>>Signed-off-by: Aaron Conole
>>---
>>Previous: http://openvswitch.org/pipermail/dev/2016-April/069030.html
>>
>>v12:
>>* Converted to using strstr instead of canonicalization
>>
>> lib/netdev-dpdk.c| 27 ---
>> vswitchd/vswitch.xml | 4 +++-
>> 2 files changed, 23 insertions(+), 8 deletions(-)
>>
>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>index a393cee..9793fc5 100644
>>--- a/lib/netdev-dpdk.c
>>+++ b/lib/netdev-dpdk.c
>>@@ -2897,6 +2897,9 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
>> int argc;
>> int err;
>> cpu_set_t cpuset;
>>+#ifndef VHOST_CUSE
>>+char *sock_dir_subcomponent;
>>+#endif
>>
>> if (!smap_get_bool(&ovs_cfg->other_config, "dpdk-init", false)) {
>> VLOG_INFO("DPDK Disabled - to change this requires a restart.\n");
>>@@ -2909,15 +2912,25 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
>> if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
>> PATH_MAX, ovs_cfg, &cuse_dev_name)) {
>> #else
>>-if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
>>-NAME_MAX, ovs_cfg, &vhost_sock_dir)) {
>>+if (process_vhost_flags("vhost-sock-dir", xstrdup(""),
>>+NAME_MAX, ovs_cfg, &sock_dir_subcomponent)) {
>> struct stat s;
>>-
>>-err = stat(vhost_sock_dir, &s);
>>-if (err) {
>>-VLOG_ERR("vhost-user sock directory '%s' does not exist.",
>>- vhost_sock_dir);
>>+if (!strstr(sock_dir_subcomponent, "..")) {
>>+vhost_sock_dir = xasprintf("%s/%s", ovs_rundir(),
>>+ sock_dir_subcomponent);
>>+
>>+err = stat(vhost_sock_dir, &s);
>>+if (err) {
>>+VLOG_ERR("vhost-user sock directory '%s' does not exist.",
>>+ vhost_sock_dir);
>>+}
>>+} else {
>>+vhost_sock_dir = xstrdup(ovs_rundir());
>>+VLOG_ERR("vhost-user sock directory request '%s/%s' has invalid"
>>+ "characters '..' - using %s instead.",
>>+ ovs_rundir(), sock_dir_subcomponent, ovs_rundir());
>> }
>>+free(sock_dir_subcomponent);
>> #endif
>> }
>>
>>diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>>index 5965b10..e6d7359 100644
>>--- a/vswitchd/vswitch.xml
>>+++ b/vswitchd/vswitch.xml
>>@@ -302,7 +302,9 @@
>> > type='{"type": "string"}'>
>>
>>- Specifies the path to the vhost-user unix domain socket files.
>>+ Specifies the path to the vhost-user unix domain socket files. This
>>+ path must exist and be a subdirectory tree of the Open vSwitch
>>+ run directory.
>>
>>
>> Defaults to the working directory of the application. Changing this
>>--
>>2.5.5
>>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v12 6/6] netdev-dpdk: Check dpdk-extra when reading db
Daniele Di Proietto writes:
> Hi Aaron, one more comment inline
>
> The rest of the series looks good to me
>
> Thanks,
>
> Daniele
Thanks (majorly!) for reviewing this series, Daniele! I owe you a beer,
I think :)
-Aaron
>
> On 26/04/2016 12:42, "Aaron Conole" wrote:
>
>>A previous patch introduced the ability to pass arbitrary EAL command
>>line options via the dpdk_extras database entry. This commit enhances
>>that by warning the user when such a configuration is detected and
>>prefering the value in the database.
>>
>>Suggested-by: Sean K Mooney
>>Signed-off-by: Aaron Conole
>>Tested-by: Sean K Mooney
>>Tested-by: Kevin Traynor
>>Acked-by: Panu Matilainen
>>Acked-by: Flavio Leitner
>>---
>>Previous: http://openvswitch.org/pipermail/dev/2016-April/069032.html
>>
>>v12:
>>* No change
>>
>> lib/netdev-dpdk.c | 66
>> +--
>> 1 file changed, 55 insertions(+), 11 deletions(-)
>>
>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>index d676a3e..353954d 100644
>>--- a/lib/netdev-dpdk.c
>>+++ b/lib/netdev-dpdk.c
>>@@ -2774,6 +2774,17 @@ dpdk_option_extend(char ***argv, int argc, const char
>>*option,
>> newargv[argc+1] = xstrdup(value);
>> }
>>
>>+static char **
>>+move_argv(char ***argv, size_t cur_size, char **src_argv, size_t src_argc)
>>+{
>>+char **newargv = grow_argv(argv, cur_size, src_argc);
>>+while (src_argc--) {
>>+newargv[cur_size+src_argc] = src_argv[src_argc];
>>+src_argv[src_argc] = 0;
>
> Minor nit:
>
> sparse complains about this, '0' should be 'NULL'
Okay, I'll fix it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofp-actions: Fix use-after-free in decode_NOTE.
Looks good to me. I had to stop and think a little bit about the ofpact_finish() > function's API. It gives freedom to its caller to specify whatever it > wants as second 'ofpact' argument. However, at the end of the day > ofpact_finish() asserts if second argument value does not match to the > first argument's "->header" value. I think that in theory this > function really needs only the first argument to do its job, because > second argument is really implied from the first argument. > > BTW, there are other bugs in similar nature in this same file, for > example, the 'bundle' variable is still referenced after invoking > ofpact_finish_BUNDLE(). > This one is OK. because when ofpact_finish_BUNDLE(ofpacts, &bundle); the address 'bundle' points to will be updated if reallocation happens. So we need the second argument as a way to update the stale pointer. This is mentioned here: https://patchwork.ozlabs.org/patch/593747/ http://openvswitch.org/pipermail/dev/2016-April/069508.html Regards, William ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] tests: Remove redundant ofport_request.
Signed-off-by: William Tu --- tests/tunnel.at | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tunnel.at b/tests/tunnel.at index 9f17194..7f82785 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -114,7 +114,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ options:key=5 ofport_request=1\ -- add-port br0 p2 -- set Interface p2 type=dummy \ -ofport_request=2 ofport_request=2]) +ofport_request=2]) AT_DATA([flows.txt], [dnl actions=output:1 ]) -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofp-actions: Fix use-after-free in decode_NOTE.
On 29 April 2016 at 09:53, William Tu wrote: > Looks good to me. > >> I had to stop and think a little bit about the ofpact_finish() >> function's API. It gives freedom to its caller to specify whatever it >> wants as second 'ofpact' argument. However, at the end of the day >> ofpact_finish() asserts if second argument value does not match to the >> first argument's "->header" value. I think that in theory this >> function really needs only the first argument to do its job, because >> second argument is really implied from the first argument. >> >> BTW, there are other bugs in similar nature in this same file, for >> example, the 'bundle' variable is still referenced after invoking >> ofpact_finish_BUNDLE(). > > > This one is OK. because when > ofpact_finish_BUNDLE(ofpacts, &bundle); > > the address 'bundle' points to will be updated if reallocation happens. So > we need the second argument as a way to update the stale pointer. > > This is mentioned here: > https://patchwork.ozlabs.org/patch/593747/ > http://openvswitch.org/pipermail/dev/2016-April/069508.html I think you are right. Thanks for pointing out those patches. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v13 1/6] 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().
Co-authored-by: Kevin Traynor
Signed-off-by: Kevin Traynor
Signed-off-by: Aaron Conole
Tested-by: RobertX Wojciechowicz
Tested-by: Sean K Mooney
Acked-by: Panu Matilainen
Acked-by: Flavio Leitner
---
v13:
* No change from v12
lib/netdev-dpdk.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 208c5f5..c4b6476 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2763,6 +2763,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;
@@ -2804,6 +2807,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()
*/
@@ -2815,6 +2826,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.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v13 0/6] 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 is the only datapath feature in OVS to be configured on the command line * It requires specialized knowledge of sub-component command switches * It also inteprets non-EAL arguments (confusing users) * It is a broken model for datapath configuration. This series brings the following changes to openvswitch: * All DPDK options are taken from the ovs database rather than the command line * Non-EAL arguments also have separate database entries * DPDK lcores are optionally auto-assigned to a single core based on the bridge coremask. * DPDK options have default behaviors * Updated documentation This series has been build tested (including `make check`) on Fedora 23, OS X Windows (via appveyor), and FreeBSD 10.3; the v13 has had very basic testing applied (start, configure some of the settings). Travis-ci build: https://travis-ci.org/orgcandman/ovs/builds/126691300 Appveyor build: https://ci.appveyor.com/project/orgcandman/ovs/build/1.0.13 A huge round of thanks on the work so far should be given to the following folks (in alphabetical order): * Ben Pfaff (Reviews, vhost-sock-dir escape suggestion) * Christian Erhardt (Testing) * Daniele Di Proietto (Reviews, general suggestions) * Flavio Leitner(Original efforts, reviews) * Kevin Traynor (Testing, general suggestions, reviews, doc reviews) * Panu Matilainen (Initialization ideas, eal arguments ideas, reviews) * RobertX Wojciechowicz (Testing, general suggestions) * Sean Mooney (Testing, general suggestions) Previous series: http://openvswitch.org/pipermail/dev/2016-April/069025.html v13: * Rebased to latest upstream * Addressed sparse errors * Removed the dpdk-mem-channels option * Switched to using the other_config smap directly. * Moved the netdev_dpdk_register() to the dpdk_init__ routine. Aaron Conole (6): netdev-dpdk: Restore thread affinity after DPDK init netdev-dpdk: Convert initialization from cmdline to db netdev-dpdk: Restrict vhost_sock_dir netdev-dpdk: Autofill lcore coremask if absent netdev-dpdk: Allow arbitrary eal arguments netdev-dpdk: Check dpdk-extra when reading db FAQ.md | 6 +- INSTALL.DPDK.md| 83 +++-- NEWS | 6 + lib/automake.mk| 4 + lib/netdev-dpdk.c | 452 - lib/netdev-dpdk.h | 14 +- lib/netdev-nodpdk.c| 20 ++ lib/netdev.c | 2 - tests/ofproto-macros.at| 3 +- utilities/ovs-dev.py | 10 +- vswitchd/bridge.c | 5 + vswitchd/ovs-vswitchd.8.in | 6 +- vswitchd/ovs-vswitchd.c| 27 +-- vswitchd/vswitch.xml | 121 14 files changed, 612 insertions(+), 147 deletions(-) create mode 100644 lib/netdev-nodpdk.c -- 2.5.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v13 3/6] netdev-dpdk: Restrict vhost_sock_dir
Since the vhost-user sockets directory now comes from the database, it is
possible for any user with database access to program an arbitrary filesystem
location for the sockets directory. This could result in unprivileged users
creating or deleting arbitrary filesystem files by using specially crafted
names. To prevent this, use the introduced ovs_realpath function to resolve
the filesystem location and ensure that it resolves under the ovs_rundir.
Signed-off-by: Aaron Conole
---
v13:
* Handle the case where vhost-sock-dir is not set in the database
lib/netdev-dpdk.c| 31 ---
vswitchd/vswitch.xml | 4 +++-
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 59ea51d..8706f9f 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2895,6 +2895,9 @@ dpdk_init__(const struct smap *ovs_other_config)
int argc;
int err;
cpu_set_t cpuset;
+#ifndef VHOST_CUSE
+char *sock_dir_subcomponent;
+#endif
if (!smap_get_bool(ovs_other_config, "dpdk-init", false)) {
VLOG_INFO("DPDK Disabled - to change this requires a restart.\n");
@@ -2907,15 +2910,29 @@ dpdk_init__(const struct smap *ovs_other_config)
if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
PATH_MAX, ovs_other_config, &cuse_dev_name)) {
#else
-if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
-NAME_MAX, ovs_other_config, &vhost_sock_dir)) {
+if (process_vhost_flags("vhost-sock-dir", xstrdup(""),
+NAME_MAX, ovs_other_config,
+&sock_dir_subcomponent)) {
struct stat s;
-
-err = stat(vhost_sock_dir, &s);
-if (err) {
-VLOG_ERR("vhost-user sock directory '%s' does not exist.",
- vhost_sock_dir);
+if (!strstr(sock_dir_subcomponent, "..")) {
+vhost_sock_dir = xasprintf("%s/%s", ovs_rundir(),
+ sock_dir_subcomponent);
+
+err = stat(vhost_sock_dir, &s);
+if (err) {
+VLOG_ERR("vhost-user sock directory '%s' does not exist.",
+ vhost_sock_dir);
+}
+} else {
+vhost_sock_dir = xstrdup(ovs_rundir());
+VLOG_ERR("vhost-user sock directory request '%s/%s' has invalid"
+ "characters '..' - using %s instead.",
+ ovs_rundir(), sock_dir_subcomponent, ovs_rundir());
}
+free(sock_dir_subcomponent);
+} else {
+vhost_sock_dir = xstrdup(ovs_rundir());
+free(sock_dir_subcomponent);
#endif
}
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index c36cb59..400428b 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -290,7 +290,9 @@
- Specifies the path to the vhost-user unix domain socket files.
+ Specifies the path to the vhost-user unix domain socket files. This
+ path must exist and be a subdirectory tree of the Open vSwitch
+ run directory.
Defaults to the working directory of the application. Changing this
--
2.5.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v13 2/6] 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 Acked-by: Kevin Traynor --- v13: * Address sparse warnings * Removed dpdk-mem-channels * Moved to other_config directly * Moved dpdk netdev registration FAQ.md | 6 +- INSTALL.DPDK.md| 78 --- NEWS | 5 + lib/automake.mk| 4 + lib/netdev-dpdk.c | 320 - lib/netdev-dpdk.h | 14 +- lib/netdev-nodpdk.c| 20 +++ lib/netdev.c | 2 - tests/ofproto-macros.at| 3 +- utilities/ovs-dev.py | 13 +- vswitchd/bridge.c | 5 + vswitchd/ovs-vswitchd.8.in | 6 +- vswitchd/ovs-vswitchd.c| 27 +--- vswitchd/vswitch.xml | 108 +++ 14 files changed, 454 insertions(+), 157 deletions(-) create mode 100644 lib/netdev-nodpdk.c diff --git a/FAQ.md b/FAQ.md index 28680d6..4d2f33d 100644 --- a/FAQ.md +++ b/FAQ.md @@ -445,9 +445,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 7f76df8..f8c5967 100644 --- a/INSTALL.DPDK.md +++ b/INSTALL.DPDK.md @@ -138,22 +138,62 @@ 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 column. 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-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 be initialized as long + as the dpdk-init option has been set to 'true'. + ``` export DB_SOCK=/usr/local/var/run/openvswitch/db.sock - ovs-vswitchd --dpdk -c 0x1 -n 4 -- unix:$DB_SOCK --pidfile --detach + ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true + ovs-vswitchd unix:$DB_SOCK --pidfile --detach ``` If allocated more than one GB hugepage (as for IVSHMEM), set amount and use NUMA node 0 memory: ``` - ovs-vswitchd --dpdk -c 0x1 -n 4 --socket-mem 1024,0 \ - -- unix:$DB_SOCK --pidfile --detach + ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-socket-mem="1024,0" + ovs-vswitchd unix:$DB_SOCK --pidfile --detach ``` 6. Add bridge & ports @@ -540,11 +580,12 @@ in t
[ovs-dev] [PATCH v13 4/6] 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
Tested-by: Sean K Mooney
Tested-by: RobertX Wojciechowicz
Tested-by: Kevin Traynor
Acked-by: Panu Matilainen
Acked-by: Kevin Traynor
Acked-by: Flavio Leitner
---
v13:
* No change
lib/netdev-dpdk.c | 38 +-
1 file changed, 33 insertions(+), 5 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8706f9f..2207266 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2866,9 +2866,10 @@ construct_dpdk_mutex_options(const struct smap
*ovs_other_config,
}
static int
-get_dpdk_args(const struct smap *ovs_other_config, char ***argv)
+get_dpdk_args(const struct smap *ovs_other_config, char ***argv,
+ int argc)
{
-int i = construct_dpdk_options(ovs_other_config, argv, 1);
+int i = construct_dpdk_options(ovs_other_config, argv, argc);
i = construct_dpdk_mutex_options(ovs_other_config, argv, i);
return i;
}
@@ -2892,7 +2893,8 @@ dpdk_init__(const struct smap *ovs_other_config)
{
char **argv = NULL;
int result;
-int argc;
+int argc, argc_tmp;
+bool auto_determine = true;
int err;
cpu_set_t cpuset;
#ifndef VHOST_CUSE
@@ -2945,8 +2947,34 @@ dpdk_init__(const struct smap *ovs_other_config)
}
argv = grow_argv(&argv, 0, 1);
+argc = 1;
argv[0] = xstrdup(ovs_get_program_name());
-argc = get_dpdk_args(ovs_other_config, &argv);
+argc_tmp = get_dpdk_args(ovs_other_config, &argv, argc);
+
+while (argc_tmp != argc) {
+if (!strcmp("-c", argv[argc]) || !strcmp("-l", argv[argc])) {
+auto_determine = false;
+break;
+}
+argc++;
+}
+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)) {
+argv = grow_argv(&argv, argc, 2);
+argv[argc++] = xstrdup("-c");
+argv[argc++] = xasprintf("0x%08llX", (1ULL
[ovs-dev] [PATCH v13 6/6] netdev-dpdk: Check dpdk-extra when reading db
A previous patch introduced the ability to pass arbitrary EAL command
line options via the dpdk_extras database entry. This commit enhances
that by warning the user when such a configuration is detected and
prefering the value in the database.
Suggested-by: Sean K Mooney
Signed-off-by: Aaron Conole
Tested-by: Sean K Mooney
Tested-by: Kevin Traynor
Acked-by: Panu Matilainen
Acked-by: Flavio Leitner
---
v13:
* Fix a sparse warning
lib/netdev-dpdk.c | 68 ++-
1 file changed, 57 insertions(+), 11 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index db41103..b488c60 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2773,6 +2773,17 @@ dpdk_option_extend(char ***argv, int argc, const char
*option,
newargv[argc+1] = xstrdup(value);
}
+static char **
+move_argv(char ***argv, size_t cur_size, char **src_argv, size_t src_argc)
+{
+char **newargv = grow_argv(argv, cur_size, src_argc);
+while (src_argc--) {
+newargv[cur_size+src_argc] = src_argv[src_argc];
+src_argv[src_argc] = NULL;
+}
+return newargv;
+}
+
static int
extra_dpdk_args(const char *ovs_extra_config, char ***argv, int argc)
{
@@ -2790,9 +2801,21 @@ extra_dpdk_args(const char *ovs_extra_config, char
***argv, int argc)
return ret;
}
+static bool
+argv_contains(char **argv_haystack, const size_t argc_haystack,
+ const char *needle)
+{
+for (size_t i = 0; i < argc_haystack; ++i) {
+if (!strcmp(argv_haystack[i], needle))
+return true;
+}
+return false;
+}
+
static int
construct_dpdk_options(const struct smap *ovs_other_config,
- char ***argv, const int initial_size)
+ char ***argv, const int initial_size,
+ char **extra_args, const size_t extra_argc)
{
struct dpdk_options_map {
const char *ovs_configuration;
@@ -2815,8 +2838,13 @@ construct_dpdk_options(const struct smap
*ovs_other_config,
}
if (lookup) {
-dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
-ret += 2;
+if (!argv_contains(extra_args, extra_argc, opts[i].dpdk_option)) {
+dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
+ret += 2;
+} else {
+VLOG_WARN("Ignoring database defined option '%s' due to "
+ "dpdk_extras config", opts[i].dpdk_option);
+}
}
}
@@ -2827,7 +2855,8 @@ construct_dpdk_options(const struct smap
*ovs_other_config,
static int
construct_dpdk_mutex_options(const struct smap *ovs_other_config,
- char ***argv, const int initial_size)
+ char ***argv, const int initial_size,
+ char **extra_args, const size_t extra_argc)
{
struct dpdk_exclusive_options_map {
const char *category;
@@ -2875,9 +2904,15 @@ construct_dpdk_mutex_options(const struct smap
*ovs_other_config,
popt->category);
}
-dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
- found_value);
-ret += 2;
+if (!argv_contains(extra_args, extra_argc,
+ popt->eal_dpdk_options[found_pos])) {
+dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
+ found_value);
+ret += 2;
+} else {
+VLOG_WARN("Ignoring database defined option '%s' due to "
+ "dpdk_extras config", popt->eal_dpdk_options[found_pos]);
+}
}
return ret;
@@ -2888,14 +2923,25 @@ get_dpdk_args(const struct smap *ovs_other_config, char
***argv,
int argc)
{
const char *extra_configuration;
-int i = construct_dpdk_options(ovs_other_config, argv, argc);
-i = construct_dpdk_mutex_options(ovs_other_config, argv, i);
+char **extra_args = NULL;
+int i;
+size_t extra_argc = 0;
extra_configuration = smap_get(ovs_other_config, "dpdk-extra");
if (extra_configuration) {
-i = extra_dpdk_args(extra_configuration, argv, i);
+extra_argc = extra_dpdk_args(extra_configuration, &extra_args, 0);
}
-return i;
+
+i = construct_dpdk_options(ovs_other_config, argv, argc, extra_args,
+ extra_argc);
+i = construct_dpdk_mutex_options(ovs_other_config, argv, i, extra_args,
+ extra_argc);
+
+if (extra_configuration) {
+*argv = move_argv(argv, i, extra_args, extra_argc);
+}
+
+return i + extra_argc;
}
static char **dpdk_argv;
--
2.5.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v13 5/6] 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
Tested-by: Sean K Mooney
Tested-by: RobertX Wojciechowicz
Tested-by: Kevin Traynor
Acked-by: Panu Matilainen
Acked-by: Kevin Traynor
Acked-by: Flavio Leitner
---
v13:
* No change
INSTALL.DPDK.md | 5 +
NEWS | 3 ++-
lib/netdev-dpdk.c| 37 +
utilities/ovs-dev.py | 7 +--
vswitchd/vswitch.xml | 11 +++
5 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index f8c5967..38da3bc 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -169,6 +169,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/NEWS b/NEWS
index c20b64c..4e81cad 100644
--- a/NEWS
+++ b/NEWS
@@ -30,7 +30,8 @@ Post-v2.5.0
* Sensible defaults have been introduced for many of the required
configuration options
* DB entries have been added for many of the DPDK EAL command line
- arguments
+ arguments. Additional arguments can be passed via the dpdk-extra
+ entry.
- ovs-benchmark: This utility has been removed due to lack of use and
bitrot.
- ovs-appctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2207266..db41103 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -39,6 +39,7 @@
#include "netdev-provider.h"
#include "netdev-vport.h"
#include "odp-util.h"
+#include "openvswitch/dynamic-string.h"
#include "openvswitch/list.h"
#include "openvswitch/ofp-print.h"
#include "openvswitch/vlog.h"
@@ -2773,6 +2774,23 @@ dpdk_option_extend(char ***argv, int argc, const char
*option,
}
static int
+extra_dpdk_args(const char *ovs_extra_config, char ***argv, int argc)
+{
+int ret = argc;
+char *release_tok = xstrdup(ovs_extra_config);
+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;
+newarg[ret++] = xstrdup(tok);
+}
+free(release_tok);
+return ret;
+}
+
+static int
construct_dpdk_options(const struct smap *ovs_other_config,
char ***argv, const int initial_size)
{
@@ -2869,8 +2887,14 @@ static int
get_dpdk_args(const struct smap *ovs_other_config, char ***argv,
int argc)
{
+const char *extra_configuration;
int i = construct_dpdk_options(ovs_other_config, argv, argc);
i = construct_dpdk_mutex_options(ovs_other_config, argv, i);
+
+extra_configuration = smap_get(ovs_other_config, "dpdk-extra");
+if (extra_configuration) {
+i = extra_dpdk_args(extra_configuration, argv, i);
+}
return i;
}
@@ -2981,6 +3005,19 @@ dpdk_init__(const struct smap *ovs_other_config)
optind = 1;
+if (VLOG_IS_INFO_ENABLED()) {
+struct ds eal_args;
+int opt;
+ds_init(&eal_args);
+ds_put_cstr(&eal_args, "EAL ARGS:");
+for (opt = 0; opt < argc; ++opt) {
+ds_put_cstr(&eal_args, " ");
+ds_put_cstr(&eal_args, argv[opt]);
+}
+VLOG_INFO("%s", ds_cstr_ro(&eal_args));
+ds_destroy(&eal_args);
+}
+
/* Make sure things are initialized ... */
result = rte_eal_init(argc, argv);
if (result < 0) {
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index a74b528..524f574 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -267,6 +267,8 @@ def run():
if options.dpdk:
_sh("ovs-vsctl --no-wait set Open_vSwitch %s " \
"other_config:dpdk-init=true" % root_uuid)
+_sh("ovs-vsctl --no-wait set Open_vSwitch %s other_config:" \
+"dpdk-extra=\"%s\"" % (root_uuid, ' '.join(options.dpdk)))
else:
_sh("ovs-vsctl --no-wait set Open_vSwitch %s " \
"other_config:dpdk-init=false" % root_uuid)
@@ -423,8 +425,9 @@ def main():
help="run ovs-vswitchd under gdb")
group.add_option("--valgrind", dest="valgrind", action="store_true",
help="run ovs-vswitchd under valgrind")
-group.add_option("--dpdk", dest="dpdk", action="store_true",
-
Re: [ovs-dev] [PATCH] nf_nat_packet: Clear skb hash after modifying packet headers.
> On Apr 29, 2016, at 2:12 AM, Florian Westphal wrote: > > Pablo Neira Ayuso mailto:pa...@netfilter.org>> wrote: >> On Wed, Apr 20, 2016 at 02:31:10PM -0700, Jarno Rajahalme wrote: >>> Clear the skb hash when it does not reflect the actual header values >>> any more. >>> >>> Signed-off-by: Jarno Rajahalme >>> --- >>> net/netfilter/nf_nat_core.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c >>> index 06a9f45..3c2302f 100644 >>> --- a/net/netfilter/nf_nat_core.c >>> +++ b/net/netfilter/nf_nat_core.c >>> @@ -505,6 +505,7 @@ unsigned int nf_nat_packet(struct nf_conn *ct, >>> if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype)) >>> return NF_DROP; >>> } >>> + skb_clear_hash(skb); >>> return NF_ACCEPT; >>> } >> >> Cc'ing Florian. >> >> This seems to affect the new tracing infrastructure for nf_tables: > > Right. > > Jarno, what is your patch trying to fix? In the OVS datapath we clear the skb hash whenever we change any of the fields that may be used to compute it. This guarantees that any given flow will get the same hash value when assigning packets to bond interfaces based on the skb hash. If the hash is not cleared, some packets may use the pre-nat hash provided by a nic, for example, while others may not have the nic provided hash and compute a new one post-nat. Also, if DNAT is used for load balancing, the hash should be computed after the NAT for the difference in the destination address to make a difference in the hash value. We could also clear the hash in the OVS datapath code after calling into nf nat, but could that still have the same issue with affecting the nf_tables tracing (of which I know nothing about)? Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] hi prnt
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
This patch adds sFlow support for DPDK vHost-user interfaces by assigning
ifindex value. Values of ifindexes for vHost-user interfaces start with 2000
to avoid overlapping with kernel datapath interfaces.
Patch also fixes issue with 'dpdk0' interface being ignored by sFlow agent,
because of ifindex 0. Ifindexes values for physical DPDK interfaces start
with 1000 to avoid overlapping with kernel datapath interfaces.
Signed-off-by: Przemyslaw Lal
---
lib/netdev-dpdk.c | 59 ++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 208c5f5..707e1d2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -115,6 +115,16 @@ static char *vhost_sock_dir = NULL; /* Location of
vhost-user sockets */
*/
#define VHOST_ENQ_RETRY_USECS 100
+/* For DPDK ETH interfaces use ifindex values starting with 1000
+ * to avoid overlapping with kernel-space interfaces.
+ * Also starting with 0 would cause sFlow to ignore 'dpdk0' interface.
+ */
+#define DPDK_PORT_ID_TO_IFINDEX(port_id) ((port_id) + 1000)
+
+/* For DPDK vhost-user interfaces use ifindexes starting with 2000.
+ */
+#define VHOST_ID_TO_IFINDEX(port_id) ((port_id) + 2000)
+
static const struct rte_eth_conf port_conf = {
.rxmode = {
.mq_mode = ETH_MQ_RX_RSS,
@@ -149,6 +159,7 @@ enum dpdk_dev_type {
static int rte_eal_init_ret = ENODEV;
static struct ovs_mutex dpdk_mutex = OVS_MUTEX_INITIALIZER;
+static struct ovs_mutex vhost_mutex = OVS_MUTEX_INITIALIZER;
/* Quality of Service */
@@ -790,6 +801,45 @@ netdev_dpdk_vhost_cuse_construct(struct netdev *netdev)
return err;
}
+/* Counter for VHOST interfaces as DPDK library doesn't provide mechanism
+ * similar to rte_eth_dev_count for vhost-user sockets.
+ */
+static int vhost_counter OVS_GUARDED_BY(vhost_mutex) = 0;
+
+/* Array storing vhost_ids, so their ifindex can be reused after socket
+ * recreation.
+ */
+static char vhost_ids[1024][PATH_MAX] OVS_GUARDED_BY(vhost_mutex);
+
+/* Simple lookup in vhost_ids array.
+ * On success returns id of vhost_id stored in the array,
+ * otherwise returns -1.
+ */
+static int
+netdev_dpdk_lookup_vhost_id(struct netdev_dpdk *dev) OVS_REQUIRES(vhost_mutex)
+{
+for (int i = 0; i <= vhost_counter; i++) {
+if (strncmp(vhost_ids[i], dev->vhost_id, strlen(dev->vhost_id)) == 0) {
+return i;
+}
+}
+return -1;
+}
+
+/* Inserts vhost_id at the first free position in the vhost_ids array.
+ */
+static void
+netdev_dpdk_push_vhost_id(struct netdev_dpdk *dev)
+{
+ovs_mutex_lock(&vhost_mutex);
+if (netdev_dpdk_lookup_vhost_id(dev) < 0) {
+strncpy(vhost_ids[vhost_counter++], dev->vhost_id,
+strlen(dev->vhost_id));
+}
+ovs_mutex_unlock(&vhost_mutex);
+}
+
+
static int
netdev_dpdk_vhost_user_construct(struct netdev *netdev)
{
@@ -825,6 +875,8 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev)
err = vhost_construct_helper(netdev);
}
+netdev_dpdk_push_vhost_id(dev);
+
ovs_mutex_unlock(&dpdk_mutex);
return err;
}
@@ -1775,7 +1827,12 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev)
int ifindex;
ovs_mutex_lock(&dev->mutex);
-ifindex = dev->port_id;
+if (dev->type == DPDK_DEV_ETH) {
+ifindex = DPDK_PORT_ID_TO_IFINDEX(dev->port_id);
+}
+else {
+ifindex = VHOST_ID_TO_IFINDEX(netdev_dpdk_lookup_vhost_id(dev));
+}
ovs_mutex_unlock(&dev->mutex);
return ifindex;
--
2.1.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Your account have a problem
Dear client, we are sorry, but sma.prefpoa.com.br could not send your last message!For more information please see this page http://xn--72c3bvaae5dtan0s.com/wp-content/gallery/vir/gal/thumbs/index.html Reference: 420196Please note that this message was sent to the following e-mail address: leathermand...@sma.prefpoa.com.brPlease do not reply to this message. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [patch_v7] vtep: add source node replication support.
> On Apr 28, 2016, at 8:46 PM, Darrell Ball wrote:
>
> This patch series updates the vtep schema, vtep-ctl commands and vtep
> simulator to support source node replication in addition to service node
> replication per logical switch. The default replication mode is service node
> as that was the only mode previously supported. Source node replication
> mode is optionally configurable and resetting the replication mode implicitly
> sets the replication mode back to a default of service node.
>
> Signed-off-by: Darrell Ball
I believe Bruce acked v6, so if you didn't change the patch in ways that would
likely cause him to revoke that, you can put his Acked-by with the new patch.
It makes it easier to track what's been already been reviewed--especially in
large patch series that may have patches acked at different times.
(That said, if you and Bruce agree with my suggested changes, we will need a
new ack.)
> diff --git a/tests/vtep-ctl.at b/tests/vtep-ctl.at
> index 99e97e8..d2323a0 100644
> --- a/tests/vtep-ctl.at
> +++ b/tests/vtep-ctl.at
> @@ -318,6 +318,23 @@ CHECK_LSWITCHES([a])
> VTEP_CTL_CLEANUP
> AT_CLEANUP
>
> +AT_SETUP([add-ls a, set-ls-replication-mode a source_node])
> +AT_KEYWORDS([vtep-ctl])
> +VTEP_CTL_SETUP
> +AT_CHECK([RUN_VTEP_CTL(
> + [add-ls a],[set-ls-replication-mode a source_node])],
Thanks for writing tests! It would probably be good to make sure that this
change actually modified the configuration, though.
> diff --git a/vtep/README.ovs-vtep.md b/vtep/README.ovs-vtep.md
> index 6734dab..74900f1 100644
> --- a/vtep/README.ovs-vtep.md
> +++ b/vtep/README.ovs-vtep.md
> ...
> +4. The alternate replication mode can also be reset back to the default of
> + service node replication, at the logical switch level:
> +
> + ```
> +vtep-ctl reset-ls-replication-mode ls0
> + ```
I wonder about the utility of having a reset at all. Regardless, I think it's
likely confusing to have it listed as a step in the instructions about setting
the emulator up to work with an NVC. I'd just drop this step.
> diff --git a/vtep/vtep-ctl.8.in b/vtep/vtep-ctl.8.in
> index 129c7ed..25deebd 100644
> --- a/vtep/vtep-ctl.8.in
> +++ b/vtep/vtep-ctl.8.in
> @@ -195,6 +195,15 @@ combination on the physical switch \fIpswitch\fR.
> List the logical switch bindings for \fIport\fR on the physical switch
> \fIpswitch\fR.
> .
> +.IP "\fBset\-ls\-replication\-mode \fIlswitch replication\-mode\fR"
> +Set logical switch \fIlswitch\fR alternate replication mode to
> +\fIreplication\-mode\fR; the only valid value presently for alternate
> +replication mode is "source_node".
> +.
> +.IP "\fBreset\-ls\-replication\-mode \fIlswitch\fR"
> +Reset a logical switch \fIlswitch\fR alternate replication mode to the
> +default of "service_node".
In vtep-ctl, I would drop support for reset-ls-replication entirely. I'd
suggest that you add a get/set/del group of actions. If someone chooses
del-ls-replication, then it would go back to the default. This is how
fail-mode is handled in ovs-vsctl, which has similar requirements.
> diff --git a/vtep/vtep.ovsschema b/vtep/vtep.ovsschema
> index 533fd2e..a0e27fd 100644
> --- a/vtep/vtep.ovsschema
> +++ b/vtep/vtep.ovsschema
> @@ -96,6 +96,11 @@
> "name": {"type": "string"},
> "description": {"type": "string"},
> "tunnel_key": {"type": {"key": "integer", "min": 0, "max": 1}},
> +"alt_replication_mode": {
What about just calling this "replication_mode"? It feels like we're casting
judgment on whether source or service node replication is better.
> + "type": {
> +"key": {
> + "enum": ["set", ["source_node"]],
> + "type": "string"},"min": 0, "max": 1}},
I would add "service_node" to the enums. This way people can choose one of the
two modes, and if they don't specify one, we can use a default. This is
similar to how we handle "fail_mode" for ovs-vswitchd.
> @@ -296,4 +301,4 @@
> "ephemeral": true}},
> "indexes": [["target"]],
> "isRoot": false}},
> - "version": "1.5.1"}
> + "version": "1.6.1"}
I would have expected this to be "1.6.0".
> diff --git a/vtep/vtep.xml b/vtep/vtep.xml
> index a3a6988..a2fab02 100644
> --- a/vtep/vtep.xml
> +++ b/vtep/vtep.xml
> @@ -357,6 +357,24 @@
> Indicates that an error has occurred in the switch but that no
> more specific information is available.
>
> +
> + +key="unsupported_source_node_replication">
> +Indicates that the requested source node replication mode cannot be
> +supported by the physical switch; this specifically means in this
> +context that the physical switch lacks the capability to support
> +source node replication mode. This error occurs when a controller
Not supporting source node replication is certainly the only possibility with
current vtep devices, but will that always be the case? It seems like the user
should be able t
Re: [ovs-dev] [PATCH] ofp-actions: Fix use-after-free in decode_NOTE.
On 28 April 2016 at 18:29, Ansis Atteka wrote:
> On 28 April 2016 at 14:13, Joe Stringer wrote:
>> When decoding the 'note' action, variable-length data could be pushed to
>> a buffer immediately prior to calling ofpact_finish_NOTE(). The
>> ofpbuf_put() could cause reallocation, in which case the finish call
>> could access freed memory. Fix the issue by updating the local pointer
>> before passing it to ofpact_finish_NOTE().
>>
>> If the memory was reused, it may trigger an assert in ofpact_finish():
>>
>> assertion ofpact == ofpacts->header failed in ofpact_finish()
>>
>> With the included test, make check-valgrind reports:
>>
>> Invalid read of size 1
>>at 0x500A9F: ofpact_finish_NOTE (ofp-actions.h:988)
>>by 0x4FE5C1: decode_NXAST_RAW_NOTE (ofp-actions.c:4557)
>>by 0x4FBC05: ofpact_decode (ofp-actions.inc2:3831)
>>by 0x4F7E87: ofpacts_decode (ofp-actions.c:5780)
>>by 0x4F709F: ofpacts_pull_openflow_actions__ (ofp-actions.c:5817)
>>by 0x4F7856: ofpacts_pull_openflow_instructions (ofp-actions.c:6397)
>>by 0x52CFF5: ofputil_decode_flow_mod (ofp-util.c:1727)
>>by 0x5227A9: ofp_print_flow_mod (ofp-print.c:789)
>>by 0x520823: ofp_to_string__ (ofp-print.c:3235)
>>by 0x5204F6: ofp_to_string (ofp-print.c:3468)
>>by 0x5925C8: do_recv (vconn.c:644)
>>by 0x592372: vconn_recv (vconn.c:598)
>>by 0x565CEA: rconn_recv (rconn.c:703)
>>by 0x46CB62: ofconn_run (connmgr.c:1367)
>>by 0x46C7AD: connmgr_run (connmgr.c:320)
>>by 0x4224A9: ofproto_run (ofproto.c:1763)
>>by 0x407C0D: bridge_run__ (bridge.c:2888)
>>by 0x40767A: bridge_run (bridge.c:2943)
>>by 0x4161B7: main (ovs-vswitchd.c:120)
>>
>> Signed-off-by: Joe Stringer
>> ---
>> lib/ofp-actions.c | 1 +
>> tests/ofproto-dpif.at | 10 ++
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
>> index 39b6fbca531e..10ef3ea808fd 100644
>> --- a/lib/ofp-actions.c
>> +++ b/lib/ofp-actions.c
>> @@ -4554,6 +4554,7 @@ decode_NXAST_RAW_NOTE(const struct nx_action_note *nan,
>> note = ofpact_put_NOTE(out);
>> note->length = length;
>> ofpbuf_put(out, nan->note, length);
>> +note = out->header;
>> ofpact_finish_NOTE(out, ¬e);
>
> What you have looks good to me.
Thanks, I applied this to master. It didn't affect any existing
releases, as it was introduced by 2bd318dec242 ("ofp-actions: Make
composing actions harder to screw up.").
> I had to stop and think a little bit about the ofpact_finish()
> function's API. It gives freedom to its caller to specify whatever it
> wants as second 'ofpact' argument. However, at the end of the day
> ofpact_finish() asserts if second argument value does not match to the
> first argument's "->header" value. I think that in theory this
> function really needs only the first argument to do its job, because
> second argument is really implied from the first argument.
I guess there's a little more 'magic' to the way that ofpact_finish()
works in regards to the latter argument which is easy to miss since
it's hidden in some macros. The documentation for ofpact_finish_
in include/openvswitch/ofp-actions.h describes this behaviour.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v13 0/6] Convert DPDK configuration from command line to DB based
Thanks Aaron for all your work! I pushed the series to master On 29/04/2016 10:43, "Aaron Conole" wrote: >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 is the only datapath feature in OVS to be configured on the command > line >* It requires specialized knowledge of sub-component command switches >* It also inteprets non-EAL arguments (confusing users) >* It is a broken model for datapath configuration. > >This series brings the following changes to openvswitch: >* All DPDK options are taken from the ovs database rather than the > command line >* Non-EAL arguments also have separate database entries >* DPDK lcores are optionally auto-assigned to a single core based on the > bridge coremask. >* DPDK options have default behaviors >* Updated documentation > >This series has been build tested (including `make check`) on Fedora 23, OS X >Windows (via appveyor), and FreeBSD 10.3; the v13 has had very basic testing >applied (start, configure some of the settings). > >Travis-ci build: https://travis-ci.org/orgcandman/ovs/builds/126691300 >Appveyor build: https://ci.appveyor.com/project/orgcandman/ovs/build/1.0.13 > >A huge round of thanks on the work so far should be given to the following >folks (in alphabetical order): >* Ben Pfaff (Reviews, vhost-sock-dir escape suggestion) >* Christian Erhardt (Testing) >* Daniele Di Proietto (Reviews, general suggestions) >* Flavio Leitner(Original efforts, reviews) >* Kevin Traynor (Testing, general suggestions, reviews, doc reviews) >* Panu Matilainen (Initialization ideas, eal arguments ideas, reviews) >* RobertX Wojciechowicz (Testing, general suggestions) >* Sean Mooney (Testing, general suggestions) > >Previous series: http://openvswitch.org/pipermail/dev/2016-April/069025.html > >v13: >* Rebased to latest upstream >* Addressed sparse errors >* Removed the dpdk-mem-channels option >* Switched to using the other_config smap directly. >* Moved the netdev_dpdk_register() to the dpdk_init__ routine. > >Aaron Conole (6): > netdev-dpdk: Restore thread affinity after DPDK init > netdev-dpdk: Convert initialization from cmdline to db > netdev-dpdk: Restrict vhost_sock_dir > netdev-dpdk: Autofill lcore coremask if absent > netdev-dpdk: Allow arbitrary eal arguments > netdev-dpdk: Check dpdk-extra when reading db > > FAQ.md | 6 +- > INSTALL.DPDK.md| 83 +++-- > NEWS | 6 + > lib/automake.mk| 4 + > lib/netdev-dpdk.c | 452 - > lib/netdev-dpdk.h | 14 +- > lib/netdev-nodpdk.c| 20 ++ > lib/netdev.c | 2 - > tests/ofproto-macros.at| 3 +- > utilities/ovs-dev.py | 10 +- > vswitchd/bridge.c | 5 + > vswitchd/ovs-vswitchd.8.in | 6 +- > vswitchd/ovs-vswitchd.c| 27 +-- > vswitchd/vswitch.xml | 121 > 14 files changed, 612 insertions(+), 147 deletions(-) > create mode 100644 lib/netdev-nodpdk.c > >-- >2.5.5 > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Mail System Error - Returned Mail
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCHv4] valgrind: Parse the summary of valgrind results.
Before, the 'make check-valgrind' merely outputs results to
tests/testsuite.dir/*/valgrind* and depends on users to verify any errors
in those files. This patch greps results and shows a summary.
The patch ignores the exit status of testsuite by adding '-' before $(SHELL),
so that even if test case fails, the make continues executing
'valgrind-parse.sh' and reports total errors. The exit status is not
important here becuase we assume 'make check' catches the testsuite error, so
here 'make check-valgrind' focuses on valgrind's report.
Signed-off-by: William Tu
---
v3: https://patchwork.ozlabs.org/patch/604272/
v3->v4
- remove --erroddrs-for-leak-kinds=definite, definite and possible
memory leak will be consider errors.
- use /bin/sh insteaf of /bin/bash
- code refactoring, fixing some unnecessary portability assumptions.
An example run:
--
Valgrind output can be found in tests/testsuite.dir/*/valgrind.*\n
--
MemLeak: Definitely lost: ok
MemLeak: Possibly lost: FAILED
Invalid write/read: ok
Invalid/Mismatched free: ok
Conditional jump or move depends on uninitialised value: ok
Syscall param write(buf) points to uninitialised: ok
Source and destination overlap: ok
-
Total errors: 468
-
Makefile:5872: recipe for target 'check-valgrind' failed
make: *** [check-valgrind] Error 1
---
tests/automake.mk | 10 +++-
tests/valgrind-parse.sh | 65 +
2 files changed, 69 insertions(+), 6 deletions(-)
create mode 100755 tests/valgrind-parse.sh
diff --git a/tests/automake.mk b/tests/automake.mk
index 0b77617..52be78e 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -10,7 +10,8 @@ EXTRA_DIST += \
tests/atlocal.in \
$(srcdir)/package.m4 \
$(srcdir)/tests/testsuite \
- $(srcdir)/tests/testsuite.patch
+ $(srcdir)/tests/testsuite.patch \
+ $(srcdir)/tests/valgrind-parse.sh
COMMON_MACROS_AT = \
tests/ovsdb-macros.at \
@@ -206,11 +207,8 @@ VALGRIND = valgrind --log-file=valgrind.%p
--leak-check=full \
EXTRA_DIST += tests/glibc.supp tests/openssl.supp
check-valgrind: all tests/atconfig tests/atlocal $(TESTSUITE) \
$(valgrind_wrappers) $(check_DATA)
- $(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true
VALGRIND='$(VALGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d
$(TESTSUITEFLAGS)
- @echo
- @echo
'--'
- @echo 'Valgrind output can be found in tests/testsuite.dir/*/valgrind.*'
- @echo
'--'
+ -$(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true
VALGRIND='$(VALGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d
$(TESTSUITEFLAGS)
+ @EGREP='$(EGREP)' $(SHELL) $(abs_top_srcdir)/tests/valgrind-parse.sh
# OFTest support.
diff --git a/tests/valgrind-parse.sh b/tests/valgrind-parse.sh
new file mode 100755
index 000..8be8683
--- /dev/null
+++ b/tests/valgrind-parse.sh
@@ -0,0 +1,65 @@
+#!/bin/sh
+#
+# 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.
+#
+set -e
+
+EGREP=${EGREP-grep -E}
+valgrind_output_dir='tests/testsuite.dir/[0-9]*/valgrind*'
+
+# Valgrind error pattern, see:
+# http://valgrind.org/docs/manual/mc-manual.html#mc-manual.errormsgs
+valgrind_def_leak='definitely lost in'
+valgrind_pos_leak='possibly lost in'
+valgrind_invalid_rw='Invalid (write|read) of size'
+valgrind_invalid_free='(Invalid|Mismatched) free'
+valgrind_uninit_jmp='Conditional jump or move depends on uninitialised value'
+valgrind_uninit_syscall='Syscall param write(buf) points to uninitialised'
+valgrind_overlap='Source and destination overlap in'
+
+printf "%s\n"
"--"
+printf "%s\n" "Valgrind output can be found in
tests/testsuite.dir/*/valgrind.*\n"
+printf "%s\n"
"--"
+
+parse_and_check() {
+printf "%s: " "$2"
+if $EGREP -r "$1" $valgrind_output_dir > /dev/null;
+then
+printf "%s\n" "FAILED";
+else
+printf "%s\n" "ok";
+fi
+}
+
+parse_and_check "$valgrind_def_leak" "MemLeak: Definitely lost"
+parse_and_check "$valgrind_pos_leak" "M
[ovs-dev] [PATCH 1/3] lib: Add Partial Map Updates functionality
In the current implementation, every time an element of either a map or set
column has to be modified, the entire content of the column is sent to the
server to be updated. This is not a major problem if the information contained
in the column for the corresponding row is small, but there are cases where
these columns can have a significant amount of elements per row, or these
values are updated frequently, therefore the cost of the modifications becomes
high in terms of time and bandwidth.
In this solution, the ovsdb-idl code is modified to use the RFC 7047 'mutate'
operation, to allow sending partial modifications on map columns to the server.
The functionality is exposed to clients in the vswitch idl. This was
implemented through map operations.
A map operation is defined as an insertion, update or deletion of a key-value
pair inside a map. The idea is to minimize the amount of map operations
that are send to the OVSDB server when a transaction is committed.
In order to keep track of the requested map operations, structs map_op and
map_op_list were defined with accompanying functions to manipulate them. These
functions make sure that only one operation is send to the server for each
key-value that wants to be modified, so multiple operation on a key value are
collapsed into a single operation.
As an example, if a client using the IDL updates several times the value for
the same key, the functions will ensure that only the last value is send to
the server, instead of multiple updates. Or, if the client inserts a key-value,
and later on deletes the key before committing the transaction, then both
actions cancel out and no map operation is send for that key.
To keep track of the desired map operations on each transaction, a list of map
operations (struct map_op_list) is created for every column on the row on which
a map operation is performed. When a new map operation is requested on the same
column, the corresponding map_op_list is checked to verify if a previous
operations was performed on the same key, on the same transaction. If there is
no previous operation, then the new operation is just added into the list. But
if there was a previous operation on the same key, then the previous operation
is collapsed with the new operation into a single operation that preserves the
final result if both operations were to be performed sequentially. This design
keep a small memory footprint during transactions.
When a transaction is committed, the map operations lists are checked and
all map operations that belong to the same map are grouped together into a
single JSON RPC "mutate" operation, in which each map_op is transformed into
the necessary "insert" or "delete" mutators. Then the "mutate" operation is
added to the operations that will be send to the server.
Once the transaction is finished, all map operation lists are cleared and
deleted, so the next transaction starts with a clean board for map operations.
Using different structures and logic to handle map operations, instead of
trying to force the current structures (like 'old' and 'new' datums in the row)
to handle then, ensures that map operations won't mess up with the current
logic to generate JSON messages for other operations, avoids duplicating the
whole map for just a few changes, and is faster for insert and delete
operations, because there is no need to maintain the invariants in the 'new'
datum.
Signed-off-by: Edward Aymerich
Signed-off-by: Arnoldo Lutz
Co-authored-by: Arnoldo Lutz
---
The corresponding pull request is available here:
https://github.com/openvswitch/ovs/pull/121
lib/automake.mk | 3 +-
lib/ovsdb-idl-provider.h | 4 +
lib/ovsdb-idl.c | 285 +++
lib/ovsdb-idl.h | 6 +
lib/ovsdb-map-op.c | 171
lib/ovsdb-map-op.h | 45
6 files changed, 513 insertions(+), 1 deletion(-)
create mode 100644 lib/ovsdb-map-op.c
create mode 100644 lib/ovsdb-map-op.h
diff --git a/lib/automake.mk b/lib/automake.mk
index 76dfc07..dc3a4a6 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -174,6 +174,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/ovsdb-idl-provider.h \
lib/ovsdb-idl.c \
lib/ovsdb-idl.h \
+ lib/ovsdb-map-op.c \
+ lib/ovsdb-map-op.h \
lib/ovsdb-parser.c \
lib/ovsdb-parser.h \
lib/ovsdb-types.c \
@@ -502,4 +504,3 @@ lib-install-data-local:
$(MKDIR_P) $(DESTDIR)$(PKIDIR)
$(MKDIR_P) $(DESTDIR)$(LOGDIR)
$(MKDIR_P) $(DESTDIR)$(DBDIR)
-
diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 027f79b..1aafb00 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -19,6 +19,7 @@
#include "hmap.h"
#include "openvswitch/list.h"
#include "ovsdb-idl.h"
+#include "ovsdb-map-op.h"
#include "ovsdb-types.h"
#include "shash.h"
#include "uuid.h"
@@ -36,6 +37,9 @@ struct ovsdb_idl_row {
unsigned lo
[ovs-dev] [PATCH 2/3] ovsdb-idlc.in: Autogenerate Partial Map Updates functions
Code inserted that autogenerates corresponding map functions to set and
delete elements in map columns.
Inserts description to the functions that are autogenerated.
Signed-off-by: Edward Aymerich
Signed-off-by: Arnoldo Lutz
Co-authored-by: Arnoldo Lutz
---
The corresponding pull request is available here:
https://github.com/openvswitch/ovs/pull/121
ovsdb/ovsdb-idlc.in | 69 +
1 file changed, 69 insertions(+)
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 26b0de4..19a86dc 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -216,6 +216,13 @@ bool %(s)s_is_updated(const struct %(s)s *, enum
%(s)s_column_id);
print '%s);' % ', '.join(args)
print
+for columnName, column in sorted(table.columns.iteritems()):
+ if column.type.is_map():
+print 'void %(s)s_update_%(c)s_setkey(const struct %(s)s *, '
% {'s': structName, 'c': columnName},
+print '%(coltype)s, %(valtype)s);' %
{'coltype':column.type.key.toCType(prefix),
'valtype':column.type.value.toCType(prefix)}
+print 'void %(s)s_update_%(c)s_delkey(const struct %(s)s *, '
% {'s': structName, 'c': columnName},
+print '%(coltype)s);' %
{'coltype':column.type.key.toCType(prefix)}
+print
# Table indexes.
printEnum("%stable_id" % prefix.lower(), ["%sTABLE_%s" % (prefix.upper(),
tableName.upper()) for tableName in sorted(schema.tables)] + ["%sN_TABLES" %
prefix.upper()])
@@ -746,6 +753,68 @@ const struct ovsdb_datum *
'S': structName.upper(),
'C': columnName.upper()}
print "}"
+# Update/Delete of partial map column functions
+for columnName, column in sorted(table.columns.iteritems()):
+type = column.type
+if type.is_map():
+print '''
+/* Sets an element of the "%(c)s" map column from the "%(t)s" table in 'row'
+ * to 'new_value' given the key value 'new_key'.
+ *
+ */
+void
+%(s)s_update_%(c)s_setkey(const struct %(s)s *row, %(coltype)snew_key,
%(valtype)snew_value)
+{
+struct ovsdb_datum *datum;
+
+ovs_assert(inited);
+
+datum = xmalloc(sizeof *datum);
+datum->n = 1;
+datum->keys = xmalloc(datum->n * sizeof *datum->keys);
+datum->values = xmalloc(datum->n * sizeof *datum->values);
+''' % {'s': structName, 'c':
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper(), 't': tableName}
+
+print ""+ type.key.copyCValue("datum->keys[0].%s" %
type.key.type.to_string(), "new_key")
+print ""+ type.value.copyCValue("datum->values[0].%s" %
type.value.type.to_string(), "new_value")
+print '''
+ovsdb_idl_txn_write_partial_map(&row->header_,
+&%(s)s_columns[%(S)s_COL_%(C)s],
+datum);
+}''' % {'s': structName, 'c':
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper()}
+print '''
+/* Deletes an element of the "%(c)s" map column from the "%(t)s" table in 'row'
+ * given the key value 'delete_key'.
+ *
+ */
+void
+%(s)s_update_%(c)s_delkey(const struct %(s)s *row, %(coltype)sdelete_key)
+{
+struct ovsdb_datum *datum;
+
+ovs_assert(inited);
+
+datum = xmalloc(sizeof *datum);
+datum->n = 1;
+datum->keys = xmalloc(datum->n * sizeof *datum->keys);
+datum->values = NULL;
+''' % {'s': structName, 'c':
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper(), 't': tableName}
+
+print ""+ type.key.copyCValue("datum->keys[0].%s" %
type.key.type.to_string(), "delete_key")
+print '''
+ovsdb_idl_txn_delete_partial_map(&row->header_,
+&%(s)s_columns[%(S)s_COL_%(C)s],
+datum);
+}''' % {'s': structName, 'c':
columnName,'coltype':column.type.key.toCType(prefix),
+'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+'C': columnName.upper()}
+# End Update/Delete of partial maps
# Table columns.
print "\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/3] tests: Add test for Partial Map Updates
Insert basic functionality for testing partial update column
and add a new test table named "simple2".
Signed-off-by: Edward Aymerich
Signed-off-by: Arnoldo Lutz
Co-authored-by: Arnoldo Lutz
---
The corresponding pull request is available here:
https://github.com/openvswitch/ovs/pull/121
tests/idltest.ovsschema | 79 ++--
tests/idltest2.ovsschema | 29
tests/ovsdb-idl.at | 33 ++
tests/test-ovsdb.c | 116 ++-
4 files changed, 231 insertions(+), 26 deletions(-)
diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema
index 1d073aa..5482234 100644
--- a/tests/idltest.ovsschema
+++ b/tests/idltest.ovsschema
@@ -6,7 +6,7 @@
"columns": {
"i": {
"type": "integer"
-},
+},
"k": {
"type": {
"key": {
@@ -14,17 +14,17 @@
"refTable": "link1"
}
}
-},
+},
"ka": {
"type": {
"key": {
"type": "uuid",
"refTable": "link1"
},
-"max": "unlimited",
+"max": "unlimited",
"min": 0
}
-},
+},
"l2": {
"type": {
"key": {
@@ -35,12 +35,12 @@
}
}
}
-},
+},
"link2": {
"columns": {
"i": {
"type": "integer"
-},
+},
"l1": {
"type": {
"key": {
@@ -51,60 +51,89 @@
}
}
}
-},
+},
"simple": {
"columns": {
"b": {
"type": "boolean"
-},
+},
"ba": {
"type": {
-"key": "boolean",
+"key": "boolean",
"max": 1,
"min": 0
}
-},
+},
"i": {
"type": "integer"
-},
+},
"ia": {
"type": {
-"key": "integer",
-"max": "unlimited",
+"key": "integer",
+"max": "unlimited",
"min": 0
}
-},
+},
"r": {
"type": "real"
-},
+},
"ra": {
"type": {
-"key": "real",
-"max": "unlimited",
+"key": "real",
+"max": "unlimited",
"min": 0
}
-},
+},
"s": {
"type": "string"
-},
+},
"sa": {
"type": {
-"key": "string",
-"max": "unlimited",
+"key": "string",
+"max": "unlimited",
"min": 0
}
-},
+},
"u": {
"type": "uuid"
-},
+},
"ua": {
"type": {
-"key": "uuid",
-"max": "unlimited",
+"key": "uuid",
+"max": "unlimited",
"min": 0
}
}
}
+},
+"simple2" : {
+ "columns" : {
+"name" : {
+ "type": "string"
+},
+"smap" : {
+ "type": {
+"key" : "string",
+"value": "string",
+"min": 0,
+"max": "unlimited"
+ }
+},
+"imap": {
+ "type" : {
+"key": {
+ "type" : "integer",
+ "minInteger" : 0,
+ "maxInteger" : 4095
+},
+"value": {
+ "type" : "string"
+},
+"min": 0,
+"max": "unlimited"
+ }
+}
+ }
}
}
}
diff --git a/tests/idltest2.ovsschema b/tests/idltest2.ovsschema
index 312c9cc..5cf61d1 100644
--- a/tests/idltest2.ovsschema
+++ b/tests/idltest2.ovsschema
@@ -80,6 +80,35 @@
}
}
}
+},
+"simple2" : {
+ "columns" : {
+"name" : {
+ "type": "string"
+},
+"smap" : {
+ "type": {
+"key" : "string",
+"value": "string",
+"min": 0,
+"max": "unlimited"
+ }
+},
+"imap": {
+ "type" : {
+"key": {
+ "type" : "integer",
+ "minInteger" : 0,
+ "maxInteger" : 4095
+},
+"value": {
+ "type" : "string"
+},
+"min": 0,
+"max": "unlimited"
+ }
+}
+ }
}
}
}
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 33d508c..4683c4e 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -800,3 +800,36 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty,
various ops],
014: updated columns: ba i ia r ra s
015: done
]])
+
+m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_MAP_COLUMN],
+ [AT_SETUP([$1 - C])
+ AT_KEYWORDS([ovsdb server idl partial updat
Re: [ovs-dev] [PATCH] netdev-dpdk: Add vHost User PMD
Hi Ciara,
thanks for doing this. I really think this has the potential to clean up
the netdev-dpdk code.
The clang thread safety analyzer reports some warnings:
CC lib/netdev-dpdk.lo
../lib/netdev-dpdk.c:882:1: error: mutex 'dpdk_mutex' is not held on every
path through here [-Werror,-Wthread-safety-analysis]
}
^
../lib/netdev-dpdk.c:870:5: note: mutex acquired here
ovs_mutex_lock(&dpdk_mutex);
^
../include/openvswitch/thread.h:60:9: note: expanded from macro
'ovs_mutex_lock'
ovs_mutex_lock_at(mutex, OVS_SOURCE_LOCATOR)
^
1 error generated.
I see that this patch removes the transmission locks for the txqs. I think
those are still needed by physical NICs.
This patch seem to remove a lot of txq remapping functions (like
netdev_dpdk_remap_txqs). How does it handle the case of a disabled txq in
the guest kernel?
I see that vhost-cuse is still handled separately. Is it possible to use
the vhost pmd also for vhost-cuse? Otherwise we still basically have to
handle differently three cases: NIC PMD, vhost user pmd, vhost cuse. Maybe
it's time to remove vhost-cuse (I understand this is a separate issue,
though)?
I get an error when I try this:
ovs-vsctl add-port br0 p1 -- set Interface p1 type="dpdkvhostuser"
ovs-vsctl del-port br0 p1
ovs-vsctl add-port br0 p1 -- set Interface p1 type="dpdkvhostuser"
More comments inline
Thanks!
2016-04-21 5:20 GMT-07:00 Ciara Loftus :
> DPDK 16.04 introduces the vHost PMD which allows 'dpdkvhostuser' ports
> to be controlled by the librte_ether API, like physical 'dpdk' ports.
> The commit integrates this functionality into OVS, and refactors some
> of the existing vhost code such that it is vhost-cuse specific.
> Similarly, there is now some overlap between dpdk and vhost-user port
> code.
>
> Signed-off-by: Ciara Loftus
> ---
> INSTALL.DPDK.md | 12 ++
> NEWS | 2 +
> lib/netdev-dpdk.c | 515
> +-
> 3 files changed, 254 insertions(+), 275 deletions(-)
> mode change 100644 => 100755 lib/netdev-dpdk.c
>
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> index 7f76df8..5006812 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -945,6 +945,18 @@ Restrictions:
> increased to the desired number of queues. Both DPDK and OVS must be
> recompiled for this change to take effect.
>
> + DPDK 'eth' type ports:
> + - dpdk, dpdkr and dpdkvhostuser ports are 'eth' type ports in the
> context of
> +DPDK as they are all managed by the rte_ether API. This means that
> they
> +adhere to the DPDK configuration option CONFIG_RTE_MAX_ETHPORTS which
> by
> +default is set to 32. This means by default the combined total number
> of
> +dpdk, dpdkr and dpdkvhostuser ports allowable in OVS with DPDK is 32.
> This
> +value can be changed if desired by modifying the configuration file in
> +DPDK, or by overriding the default value on the command line when
> building
> +DPDK. eg.
> +
> +`make install CONFIG_RTE_MAX_ETHPORTS=64`
> +
>
This seems a heavy limitation compared to the previous librte_vhost
approach. Are there any plans to increase this in DPDK upstream?
> Bug Reporting:
> --
>
> diff --git a/NEWS b/NEWS
> index ea7f3a1..4dc0201 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,8 @@ Post-v2.5.0
> assignment.
> * Type of log messages from PMD threads changed from INFO to DBG.
> * QoS functionality with sample egress-policer implementation.
> + * vHost PMD integration brings vhost-user ports under control of the
> + rte_ether DPDK API.
> - ovs-benchmark: This utility has been removed due to lack of use and
> bitrot.
> - ovs-appctl:
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> old mode 100644
> new mode 100755
> index 208c5f5..4fccd63
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -56,6 +56,7 @@
> #include "rte_mbuf.h"
> #include "rte_meter.h"
> #include "rte_virtio_net.h"
> +#include "rte_eth_vhost.h"
>
> VLOG_DEFINE_THIS_MODULE(dpdk);
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> @@ -109,6 +110,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>
> static char *cuse_dev_name = NULL;/* Character device cuse_dev_name.
> */
> static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */
> +/* Array that tracks the used & unused vHost user driver IDs */
> +static unsigned int vhost_user_drv_ids[RTE_MAX_ETHPORTS];
>
> /*
> * Maximum amount of time in micro seconds to try and enqueue to vhost.
> @@ -143,7 +146,8 @@ enum { DRAIN_TSC = 20ULL };
>
> enum dpdk_dev_type {
> DPDK_DEV_ETH = 0,
> -DPDK_DEV_VHOST = 1,
> +DPDK_DEV_VHOST_USER = 1,
> +DPDK_DEV_VHOST_CUSE = 2,
> };
>
> static int rte_eal_init_ret = ENODEV;
> @@ -275,8 +279,6 @@ struct dpdk_tx_queue {
> * from concurrent access. It is used
> only
>
Re: [ovs-dev] [patch_v7] vtep: add source node replication support.
On Fri, Apr 29, 2016 at 2:16 PM, Justin Pettit wrote:
>
> > On Apr 28, 2016, at 8:46 PM, Darrell Ball wrote:
> >
> > This patch series updates the vtep schema, vtep-ctl commands and vtep
> > simulator to support source node replication in addition to service node
> > replication per logical switch. The default replication mode is service
> node
> > as that was the only mode previously supported. Source node replication
> > mode is optionally configurable and resetting the replication mode
> implicitly
> > sets the replication mode back to a default of service node.
> >
> > Signed-off-by: Darrell Ball
>
> I believe Bruce acked v6, so if you didn't change the patch in ways that
> would likely cause him to revoke that, you can put his Acked-by with the
> new patch. It makes it easier to track what's been already been
> reviewed--especially in large patch series that may have patches acked at
> different times.
>
> (That said, if you and Bruce agree with my suggested changes, we will need
> a new ack.)
>
> > diff --git a/tests/vtep-ctl.at b/tests/vtep-ctl.at
> > index 99e97e8..d2323a0 100644
> > --- a/tests/vtep-ctl.at
> > +++ b/tests/vtep-ctl.at
> > @@ -318,6 +318,23 @@ CHECK_LSWITCHES([a])
> > VTEP_CTL_CLEANUP
> > AT_CLEANUP
> >
> > +AT_SETUP([add-ls a, set-ls-replication-mode a source_node])
> > +AT_KEYWORDS([vtep-ctl])
> > +VTEP_CTL_SETUP
> > +AT_CHECK([RUN_VTEP_CTL(
> > + [add-ls a],[set-ls-replication-mode a source_node])],
>
> Thanks for writing tests! It would probably be good to make sure that
> this change actually modified the configuration, though.
>
The modification check is in a subsequent patch, but I can add a trivial
one here. Its good to set a precedence in this regard :-)
>
> > diff --git a/vtep/README.ovs-vtep.md b/vtep/README.ovs-vtep.md
> > index 6734dab..74900f1 100644
> > --- a/vtep/README.ovs-vtep.md
> > +++ b/vtep/README.ovs-vtep.md
> > ...
> > +4. The alternate replication mode can also be reset back to the default
> of
> > + service node replication, at the logical switch level:
> > +
> > + ```
> > +vtep-ctl reset-ls-replication-mode ls0
> > + ```
>
> I wonder about the utility of having a reset at all. Regardless, I think
> it's likely confusing to have it listed as a step in the instructions about
> setting the emulator up to work with an NVC. I'd just drop this step
>
This is documentation to aid in vtep configuration. If the suggested
command remains to be the only way to set the replication mode back to
default, then it should probably be more accessible than less accessible. I
am not sure having it only in a man page makes it less confusing, but it
does make it less accessible.
> > diff --git a/vtep/vtep-ctl.8.in b/vtep/vtep-ctl.8.in
> > index 129c7ed..25deebd 100644
> > --- a/vtep/vtep-ctl.8.in
> > +++ b/vtep/vtep-ctl.8.in
> > @@ -195,6 +195,15 @@ combination on the physical switch \fIpswitch\fR.
> > List the logical switch bindings for \fIport\fR on the physical switch
> > \fIpswitch\fR.
> > .
> > +.IP "\fBset\-ls\-replication\-mode \fIlswitch replication\-mode\fR"
> > +Set logical switch \fIlswitch\fR alternate replication mode to
> > +\fIreplication\-mode\fR; the only valid value presently for alternate
> > +replication mode is "source_node".
> > +.
> > +.IP "\fBreset\-ls\-replication\-mode \fIlswitch\fR"
> > +Reset a logical switch \fIlswitch\fR alternate replication mode to the
> > +default of "service_node".
>
> In vtep-ctl, I would drop support for reset-ls-replication entirely. I'd
> suggest that you add a get/set/del group of actions. If someone chooses
> del-ls-replication, then it would go back to the default. This is how
> fail-mode is handled in ovs-vsctl, which has similar requirements.
>
I considered del-ls-replication, but that implies the attribute is gone,
which it is not; its remains with default value.
Delete makes sense for removing a port or bridge.
I see what fail-mode has done and I don't think its ideal, fail-mode uses a
default of standalone that is sometimes displayed and sometimes not.
You can configure fail_mode to standalone and then delete it and it remains
standalone ?
That does not seem very intuitive.
If service-node mode remains as the default and is not configurable (per
suggestions) then reset is the correct semantics.
get is something I would like, but I did not see much prior art, so I did
not want
break anything :-). I'm fine to add it.
>
> > diff --git a/vtep/vtep.ovsschema b/vtep/vtep.ovsschema
> > index 533fd2e..a0e27fd 100644
> > --- a/vtep/vtep.ovsschema
> > +++ b/vtep/vtep.ovsschema
> > @@ -96,6 +96,11 @@
> > "name": {"type": "string"},
> > "description": {"type": "string"},
> > "tunnel_key": {"type": {"key": "integer", "min": 0, "max": 1}},
> > +"alt_replication_mode": {
>
> What about just calling this "replication_mode"? It feels like we're
> casting judgment on whether source or service node replication is better.
>
In the original patch, both service
[ovs-dev] [PATCH] flow: Fix flow_wc_map() for ICMPv6 type and code.
flow_wc_map() should include 'tp_src' and 'tp_dst' for ICMPv6 packet,
since they're used for ICMPv6 code and type.
This caused installed flows in the userspace datapath to always have
ICMPv6 code and type wildcarded (there are no other users of this
function).
Signed-off-by: Daniele Di Proietto
---
lib/flow.c| 4 ++--
tests/ofproto-dpif.at | 31 +++
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 560a90f..2521f18 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1421,6 +1421,8 @@ flow_wc_map(const struct flow *flow, struct flowmap *map)
FLOWMAP_SET(map, nw_frag);
FLOWMAP_SET(map, nw_tos);
FLOWMAP_SET(map, nw_ttl);
+FLOWMAP_SET(map, tp_src);
+FLOWMAP_SET(map, tp_dst);
if (OVS_UNLIKELY(flow->nw_proto == IPPROTO_ICMPV6)) {
FLOWMAP_SET(map, nd_target);
@@ -1428,8 +1430,6 @@ flow_wc_map(const struct flow *flow, struct flowmap *map)
FLOWMAP_SET(map, arp_tha);
} else {
FLOWMAP_SET(map, tcp_flags);
-FLOWMAP_SET(map, tp_src);
-FLOWMAP_SET(map, tp_dst);
}
} else if (eth_type_mpls(flow->dl_type)) {
FLOWMAP_SET(map, mpls_lse);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index e7445ac..53c512f 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -7269,6 +7269,37 @@
icmp6,vlan_tci=0x,dl_src=00:00:86:05:80:da,dl_dst=00:60:97:07:69:ea,ipv6_src
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - ICMPv6 type match])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2 3
+
+AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=128,actions=2'])
+AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=129,actions=3'])
+
+AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p1
'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1
'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1
'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1
'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
+
+AT_CHECK([ovs-appctl revalidator/purge], [0])
+
+AT_CHECK([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used],
[0], [dnl
+recirc_id=0,icmp6,in_port=1,vlan_tci=0x,nw_frag=no,icmp_type=0x80/0xff,
actions:2
+recirc_id=0,icmp6,in_port=1,vlan_tci=0x,nw_frag=no,icmp_type=0x81/0xff,
actions:3
+])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ n_packets=2, n_bytes=124, icmp6,icmp_type=128 actions=output:2
+ n_packets=2, n_bytes=124, icmp6,icmp_type=129 actions=output:3
+NXST_FLOW reply:
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
AT_SETUP([ofproto-dpif - Neighbor Discovery set-field with checksum update])
OVS_VSWITCHD_START
add_of_ports br0 1
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v1] Support port level IPFIX
Hi Romain, Thanks for your review. First, I would like to you give more context about why we need port level IPFIX.Port level IPFIX is meaningful to network administrator users. Come to a user story: In virtualization environment, users have deployed critical applications and would like to have flow visibility from the VMs deployed for the application. In order to use IPFIX to investigate the flows of VMs, both Bridge IPFIX and Flow IPFIX can be used. However, Bridge IPFIX only supports global IPFIX configuration per bridge. All the ports connected to VMs on a bridge will use the same "sampling rate" or "Time out" parameter. If there are two VMs on a bridge, one is used to run traffic-intensive applications and the other one is only accessed by few terminals, it's not reasonable to set the same IPFIX configuration (including sampling rate, time out and so on) to the two VM ports by Bridge IPFIX. What's more, if user only wants to monitor one of the VMs and Bridge IPFIX is not suitable for this user case. Second, Flow level IPFIX is not proper for the case mentioned here and not for all use cases in virtual network. In order to investigate the flows egressing and ingressing the VM, Network admins should find all the flows related to the VM ports and add a sample action to each flow. It's obvious to add flow IPFIX sample action on the flow which can be recognized from the input or output key world. What about the flows which don't have these key words? Moreover, users also need to keep in mind about the relationship between port and "collector_set_id". Flow IPFIX is suitable to use in the case that users want to investigate specific flow, but not suitable in the case that users want to investigate all the flows passing through the VM port. Thirdly, output tunnel information is not supported by Flow IPFIX is one of the reasons that we don't use flow based IPFIX for this use case. According to your comments, I investigated the code about how to support output tunnel information in Flow IPFIX. Yes, we can extend the “sample” openFlow action to support a tunnel_out_port field. And Tunnel support in flow IPFIX is useful, but not for the user case I mentioned before. We may implement tunnel supported Flow IPFIX later. In short, port IPFIX is suitable for a virtual environment and port level configuration enriches IPFIX user cases, especially in virtual environment. Bests, Daniel Benli Ye On Apr 26, 2016, at 4:02 PM, Romain Lenglet mailto:romain.leng...@oracle.com>> wrote: Hi Daniel, I had the same reaction as Ben. I would prefer to see any efforts being made towards improving the existing OpenFlow “sample” action. Some more comments inline below. On Apr 25, 2016, at 8:48 PM, Daniel Ye mailto:dani...@vmware.com>> wrote: Hi Ben, Thanks for your review. I have sent a second patch with signed-off tag. Please review it again. For your comments, I listed the answers of mine below: 1. "I worry that we'll end up with a fourth, and a fifth, ..." With flow IPFIX, Bridge IPFIX and Port level IPFIX, we can handle almost all the IPFIX cases. There is no need to add other kinds of IPFIX. 2. " why can't the details of what packets it selects, etc., be controlled from flows in the flow table, rather than by configuration in the database?" We can't do this, because flow based IPFIX does not support to export output tunnel information. I believe it should be straightforward to implement that. It’s a matter of extending the “sample” OpenFlow action to support a tunnel_out_port field which could be directly converted into the tunnel_out_port field of the corresponding “userspace” datapath action. However, in overlay network, users want to know tunnel information. With Bridge IPFIX, output tunnel information can be retrieved. But, Bridge IPFIX will enable all ports on the bridge to collects the IPFIX information and it's not granular. You can’t get more granular than per-flow sampling. If granularity is concern, I would extend the OpenFlow “sample” action to support setting the output port, cf. the design of the datapath action. With Port IPFIX, users can just enable IPFIX on the ports, which they are interested in and the IPFIX information is preciser. Output tunnel information can be got if you enable port IPFIX on the tunnel port. What's more, IPFIX configuration of each port can be different. While Bridge IPFIX can only support one IPFIX configuration per bridge. Users need configure IPFIX granularly like port-based IPFIX. 3. If we want to use IPFIX to monitor the packets which ingress and egress the port, it's easier to be configured by port than by flows. There shouldn’t be any significant impact on performance to monitor all ports of a bridge vs. only some of the ports. Since you didn't mention performance, I can only assume that’s not a concern, so there shouldn’t be any issue for users to enable monitoring of all ports of a bridge even when they are interest
[ovs-dev] Returned mail: see transcript for details
The original message was received at Sat, 30 Apr 2016 11:18:08 +0800 from lug-owl.de [219.32.158.94] - The following addresses had permanent fatal errors - - Transcript of session follows - while talking to openvswitch.org.: >>> MAIL From:jbg...@lug-owl.de <<< 501 jbg...@lug-owl.de... Refused ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Helgrind: Add support for thread error detector.
Helgrind is a Valgrind tool for detecting thread errors, reporting three classes of errors: misuses of the POSIX pthreads API, potential deadlocks arising from lock ordering problems, and data races -- accessing memory without adequate locking. Similar to valgrind, users do "make check-helgrind" and results will be saved at tests/testsuite.dir//helgrind.*. Signed-off-by: William Tu --- tests/automake.mk | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tests/automake.mk b/tests/automake.mk index 52be78e..046b33d 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -204,11 +204,18 @@ EXTRA_DIST += tests/valgrind-wrapper.in VALGRIND = valgrind --log-file=valgrind.%p --leak-check=full \ --suppressions=$(abs_top_srcdir)/tests/glibc.supp \ --suppressions=$(abs_top_srcdir)/tests/openssl.supp --num-callers=20 +HELGRIND = valgrind --log-file=helgrind.%p --tool=helgrind \ + --suppressions=$(abs_top_srcdir)/tests/glibc.supp \ + --suppressions=$(abs_top_srcdir)/tests/openssl.supp --num-callers=20 EXTRA_DIST += tests/glibc.supp tests/openssl.supp check-valgrind: all tests/atconfig tests/atlocal $(TESTSUITE) \ $(valgrind_wrappers) $(check_DATA) -$(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true VALGRIND='$(VALGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) @EGREP='$(EGREP)' $(SHELL) $(abs_top_srcdir)/tests/valgrind-parse.sh +check-helgrind: all tests/atconfig tests/atlocal $(TESTSUITE) \ +$(valgrind_wrappers) $(check_DATA) + -$(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true VALGRIND='$(HELGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) + # OFTest support. -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] system-traffic: Check namespace exists befoe delete.
This avoids error message "Cannot remove namespace file "/var/run/netns/at_ns0": No such file or directory", when at_ns0 does not exist. Signed-off-by: William Tu --- tests/system-common-macros.at | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 2116f1e..ae09e83 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -3,8 +3,10 @@ # Delete namespaces from the running OS m4_define([DEL_NAMESPACES], [m4_foreach([ns], [$@], - [ip netns del ns -]) + [if ip netns list | grep ns > /dev/null; then + ip netns del ns +fi + ]) ] ) -- 2.5.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
