Re: [ovs-dev] [PATCH 00/17] DPDK/pmd reconfiguration refactor and bugfixes
Thanks for the review and for the incremental!
I applied everything to the various commits, except one thing, see below
I accumulated enough comments to send a v2, but I wanted to have a look first
at a couple of patches that may conflict with this (mostly vhost pmd).
Daniele
On 25/11/2016 06:19, "Ilya Maximets" wrote:
>Sorry, CC to list.
>
>On 25.11.2016 17:15, Ilya Maximets wrote:
>Hi, Daniele.
>
>I've prepared some incremental changes for this path-set (only very
>simple tests performed). Mostly for the largest patch.
>I hope, you will find some of this changes useful.
>I'll continue to test and review this path-set.
>
>---
> lib/dpif-netdev.c | 65 ---
> lib/ovs-numa.c| 23 ++--
> lib/ovs-numa.h| 1 +
> 3 files changed, 36 insertions(+), 53 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 8fad4f3..31c5b5a 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -3067,11 +3067,11 @@ rxq_scheduling(struct dp_netdev *dp, bool pinned)
>OVS_REQUIRES(dp->port_mutex)
> if (!pmd) {
> VLOG_WARN("There is no PMD thread on core %d. Queue "
> "%d on port \'%s\' will not be polled.",
>- port->rxqs[qid].core_id, qid,
>- netdev_get_name(port->netdev));
>+ q->core_id, qid, netdev_get_name(port->netdev));
> } else {
> q->pmd = pmd;
> pmd->isolated = true;
>+dp_netdev_pmd_unref(pmd);
> }
> } else if (!pinned && q->core_id == RXQ_CORE_UNPINNED) {
> if (!numa) {
>@@ -3107,31 +3107,31 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
> pmd_cores = ovs_numa_dump_n_cores_per_numa(NR_PMD_THREADS);
> }
>
>-/* Check for unwanted pmd threads */
>-CMAP_FOR_EACH(pmd, node, &dp->poll_threads) {
>-if (pmd->core_id != NON_PMD_CORE_ID &&
>-!ovs_numa_dump_contains_core(pmd_cores,
>- pmd->numa_id,
>- pmd->core_id)) {
>-changed = true;
>-}
>-}
>-
>-/* Check for required new pmd threads */
>-FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>-pmd = dp_netdev_get_pmd(dp, core->core_id);
>-if (pmd) {
>-dp_netdev_pmd_unref(pmd);
>-continue;
>-}
>+/* Check for changed configuration */
>+if (ovs_numa_dump_count(pmd_cores) != cmap_count(&dp->poll_threads) - 1) {
> changed = true;
>+} else {
>+CMAP_FOR_EACH(pmd, node, &dp->poll_threads) {
>+if (pmd->core_id != NON_PMD_CORE_ID
>+&& !ovs_numa_dump_contains_core(pmd_cores,
>+pmd->numa_id,
>+pmd->core_id)) {
>+changed = true;
>+break;
>+}
>+}
Less lines of code with the same effect, looks good to me
> }
>
> /* Destroy the old and recreate the new pmd threads. We don't perform an
> * incremental update because we would have to adjust 'static_tx_qid'. */
> if (changed) {
>-struct ovs_numa_dump *all_numas;
>-struct ovs_numa_info *numa;
>+uint32_t *n_threads_on_numa;
>+int i, n_numas = ovs_numa_get_n_numas();
>+
>+ovs_assert(n_numas != OVS_NUMA_UNSPEC);
>+
>+/* This is not expensive because n_numas <= MAX_NUMA_NODES */
>+n_threads_on_numa = xzalloc(n_numas * sizeof *n_threads_on_numa);
>
> /* Do not destroy the non pmd thread. */
> dp_netdev_destroy_all_pmds(dp, false);
>@@ -3141,26 +3141,17 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
> dp_netdev_configure_pmd(pmd, dp, core->core_id, core->numa_id);
>
> pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
>+n_threads_on_numa[core->numa_id]++;
> }
>
> /* Log the number of pmd threads per numa node. */
>-all_numas = ovs_numa_dump_n_cores_per_numa(1);
>-
>-FOR_EACH_CORE_ON_DUMP(numa, all_numas) {
>-int n = 0;
>-
>-FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>-if (core->numa_id == numa->numa_id) {
>-n++;
>-}
>-}
>-
>-if (n) {
>+for (i = 0; i < n_numas; i++) {
>+if (n_threads_on_numa[i]) {
> VLOG_INFO("Created %d pmd threads on numa node %d",
>- n, numa->numa_id);
>+ n_threads_on_numa[i], i);
> }
> }
>-ovs_numa_dump_destroy(all_numas);
>+free(n_threads_on_numa);
It's probably not a big deal, but this code doesn't properly handle
non-consecutive numa nodes. I decided to add helpers in ovs-numa to
simp
Re: [ovs-dev] [ovs-dev, 16/17] dpif-netdev: Centralized threads and queues handling code.
Hi Ilya,
Thanks for the review and the useful comments
On 24/11/2016 07:20, "Ilya Maximets" wrote:
>Hi, Daniele.
>This is not a complete review. I'll continue to checking this.
>
>Few high level comments:
>
>* Did you thought about splitting 'reconfigure_datapath' apart?
> It's a very big function.
The idea behind this patch is to have all the logic that interacts with
PMD threads in a single place ,so I didn't think this was a problem, but
I think you're right, we can try and make it smaller. I don't really
want to split the steps, but I think we can introduce functions that
implement parts of single steps.
I've introduced pmd_remove_stale_ports(), which implements a part of
step 2, to remove 20 lines of code from the function.
I'll try to extract more functions from it.
>
>* IMHO, 'reload_all_pmds' is a misleading name for a function
> which reloads only selected threads.
Also a good point. I changed it to 'reload_affected_pmds'.
>
>Also, few comments inline.
>
>Best regards, Ilya Maximets.
>
>On 16.11.2016 03:46, Daniele Di Proietto wrote:
>> Currently we have three different code paths that deal with pmd threads
>> and queues, in response to different input
>>
>> 1. When a port is added
>> 2. When a port is deleted
>> 3. When the cpumask changes or a port must be reconfigured.
>>
>> 1. and 2. are carefully written to minimize disruption to the running
>> datapath, while 3. brings down all the threads reconfigure all the ports
>> and restarts everything.
>>
>> This commit removes the three separate code paths by introducing the
>> reconfigure_datapath() function, that takes care of adapting the pmd
>> threads and queues to the current datapath configuration, no matter how
>> we got there.
>>
>> This aims at simplifying mantenaince and introduces a long overdue
>> improvement: port reconfiguration (can happen quite frequently for
>> dpdkvhost ports) is now done without shutting down the whole datapath,
>> but just by temporarily removing the port that needs to be reconfigured
>> (while the rest of the datapath is running).
>>
>> We now also recompute the rxq scheduling from scratch every time a port
>> is added of deleted. This means that the queues will be more balanced,
>> especially when dealing with explicit rxq-affinity from the user
>> (without shutting down the threads and restarting them), but it also
>> means that adding or deleting a port might cause existing queues to be
>> moved between pmd threads. This negative effect can be avoided by
>> taking into account the existing distribution when computing the new
>> scheduling, but I considered code clarity and fast reconfiguration more
>> important than optimizing port addition or removal (a port is added and
>> removed only once, but can be reconfigured many times)
>>
>> Lastly, this commit moves the pmd threads state away from ovs-numa. Now
>> the pmd threads state is kept only in dpif-netdev.
>>
>> Signed-off-by: Daniele Di Proietto
>> ---
>> lib/dpif-netdev.c | 899
>> +++---
>> tests/pmd.at | 3 +-
>> 2 files changed, 453 insertions(+), 449 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 0a88df3..93de684 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -289,6 +289,7 @@ struct dp_netdev_rxq {
>>pinned. RXQ_CORE_UNPINNED if the
>>queue doesn't need to be pinned
>> to a
>>particular core. */
>> +struct dp_netdev_pmd_thread *pmd; /* pmd thread that will poll this
>> queue. */
>> };
>>
>> /* A port in a netdev-based datapath. */
>> @@ -304,6 +305,7 @@ struct dp_netdev_port {
>> struct ovs_mutex txq_used_mutex;
>> char *type; /* Port type as requested by user. */
>> char *rxq_affinity_list;/* Requested affinity of rx queues. */
>> +bool need_reconfigure; /* True if we should reconfigure netdev. */
>> };
>>
>> /* Contained by struct dp_netdev_flow's 'stats' member. */
>> @@ -506,7 +508,7 @@ struct dp_netdev_pmd_thread {
>>
>> /* Queue id used by this pmd thread to send packets on all netdevs if
>> * XPS disabled for this netdev. All static_tx_qid's are unique and less
>> - * than 'ovs_numa_get_n_cores() + 1'. */
>> + * than 'cmap_count(dp->poll_threads)'. */
>> const int static_tx_qid;
>>
>> struct ovs_mutex port_mutex;/* Mutex for 'poll_list' and
>> 'tx_ports'. */
>> @@ -536,6 +538,9 @@ struct dp_netdev_pmd_thread {
>> * reporting to the user */
>> unsigned long long stats_zero[DP_N_STATS];
>> uint64_t cycles_zero[PMD_N_CYCLES];
>> +
>> +/* Set to true if the pmd thread needs to be reloaded. */
>> +bool need_reload;
>> };
>>
>> /* Interface to netdev-based datapath. */
>> @@ -580,29 +585,26 @@ static void dp_netdev_destroy_pmd(struct
>> dp_netdev_pmd_thread
Re: [ovs-dev] [ovs-dev,01/17] dpif-netdev: Fix memory leak.
On 25/11/2016 06:22, "Ilya Maximets" wrote:
>On 16.11.2016 03:45, Daniele Di Proietto wrote:
>> We keep all the per-port classifiers around, since they can be reused,
>> but when a pmd thread is destroyed we should free them.
>>
>> Found using valgrind.
>>
>> Fixes: 3453b4d62a98("dpif-netdev: dpcls per in_port with sorted
>> subtables")
>>
>> Signed-off-by: Daniele Di Proietto
>> ---
>> lib/dpif-netdev.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index c477248..c19d3e8 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -3330,6 +3330,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd)
>> /* All flows (including their dpcls_rules) have been deleted already */
>> CMAP_FOR_EACH (cls, node, &pmd->classifiers) {
>> dpcls_destroy(cls);
>> +free(cls);
>
>free should be postponed using rcu.
>This change exists in my incremental patch:
>https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325404.html
I think that it should be safe to call free() immediately here, because nobody
else
is accessing the cmap:
* CMAP_FOR_EACH is "safe" in the sense of HMAP_FOR_EACH_SAFE. That is, it is
* safe to free the current node before going on to the next iteration. Most
* of the time, though, this doesn't matter for a cmap because node
* deallocation has to be postponed until the next grace period. This means
* that this guarantee is useful only in deallocation code already executing at
* postponed time, when it is known that the RCU grace period has already
* expired.
That said, it's probably better to postpone the free() than to remember this
special case, especially if we end up moving that code somewhere else, so I
took your advice.
Thanks,
Daniele
>
>> }
>> cmap_destroy(&pmd->classifiers);
>> cmap_destroy(&pmd->flow_table);
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto
On Tue, Nov 29, 2016 at 03:00:05PM +0100, Thomas Morin wrote: > We're hitting an issue with an assert failing in commit_set_ipv4_action(), > resulting in a core dump: > > 2016-11-29T09:22:48.829Z|5|util(revalidator77)|EMER|lib/odp-util.c:5208: > assertion flow->nw_proto == base_flow->nw_proto && flow->nw_frag == > base_flow->nw_frag failed in commit_set_ipv4_action() > > Adding a few log statements gives more info: > > 2016-11-29T09:22:48.829Z|1|odp_util(revalidator77)|WARN|commit_set_ipv4_action > assert will fail > 2016-11-29T09:22:48.829Z|2|odp_util(revalidator77)|WARN| base_flow: > ip,in_port=3,dl_vlan=1,dl_vlan_pcp=0,dl_src=fa:16:3e:84:02:4a,dl_dst=00:00:5e:00:43:64,nw_src=0.0.0.0,nw_dst=0.0.0.0,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0 > 2016-11-29T09:22:48.829Z|3|odp_util(revalidator77)|WARN| flow: > tcp,in_port=3,dl_vlan=1,dl_vlan_pcp=0,dl_src=fa:16:3e:84:02:4a,dl_dst=00:00:5e:00:43:64,nw_src=10.0.1.21,nw_dst=10.0.0.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=50962,tp_dst=8080,tcp_flags=psh|ack > 2016-11-29T09:22:48.829Z|4|odp_util(revalidator77)|WARN| flow->nw_proto > (6) != base_flow->nw_proto (0) > > As show above the assert triggers because flow->nw_proto != > base_flow->nw_proto (6 != 0). > > The assert triggers, not on a new flow but for a flow for which traffic has > been flowing for some time (tcp 50962 to 8080 in the example above). > > The issue can be reproduced on a lab platform, happening each time > approximately 5 minutes after starting a test service (a few VMs). An nw_proto change from 0 to 6 implies that something in action translation transforms a packet from IP-with-no-next-protocol to TCP. Nothing in action translation is supposed to be capable of that, hence the assertion and the crash. Do you have any idea what in your OpenFlow pipeline might do that, i.e. is there anything especially tricky in the OpenFlow flows? Are you willing to show us your OpenFlow flow table? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC] [PATCH] ovn: Support sample action in logical datapath
On Tue, Nov 29, 2016 at 07:22:32PM +0500, Valentine Sinitsyn wrote: > On 29.11.2016 05:21, Ben Pfaff wrote: > >On Fri, Oct 14, 2016 at 04:35:46PM +0500, Valentine Sinitsyn wrote: > >>This is a quick attempt to implement sample action at logical port > >>level.The goal is to export IPFIX flows for logical ports, yet it is > >>easy to extend this approach to logical switches as well. > >> > >>Nothing is done to provision OVS instances with required > >>Flow_Sample_Collector_Set and IPFIX entries at this point. > > >This is pretty cool! The integration among OVS and OVN and IPFIX is > >graceful. > > > >The part that worries me is the CMS integration. Have you actually > >built that integration already (for which CMS)? I have two concerns. > >First, I'd prefer to see at least one CMS (probably OpenStack) support > >this at or around the time that it goes into OVN. Second, I have some > >skepticism around the idea that the CMS should configure the > >Flow_Sample_Collector_Set, etc., because OVN doesn't currently require > >the CMS to have any connectivity to OVSDB on each of the hypervisors and > >this would require the CMS to add that support. > I agree that this particular bit is somewhat hacky. We plan to follow this > route for an in-house CMS we build, but I doubt OpenStack community would > pickup the idea. What alternatives do you see here? Having collector config > at south db level doesn't seem clear either. Think I want to configure > collector at 127.0.0.1:5900 - which localhost does this entry refer to? Is this a common way to configure IPFIX? I had been under the impression that generally there's one or a few collectors in a network, to which each switch forwards packets. If it's common to use a per-hypervisor collector, then that might actually makes thing easier, since that would be easy for ovn-controller to configure into OVS on each hypervisor. Otherwise, I'm inclined to at least learn what the requirements would be for common deployments of IPFIX. Even if we don't implement it them (or all of them), it's important to me to know what we're leaving out so that what we add now is built in a way that it's gracefully extensible later. For example: if a packet should be sent to a collector, should the collector be chosen based on the packet's logical network, or based on the packet's physical network (the hypervisor it's ingressing or egressing), or on some combination of those? I also find myself wondering whether logical port level is the right level at which to choose whether to sample packets. Will OVN users want finer-grained control over sampling and, if so, would it make more sense to add an ACL-like table for that purpose at the northbound level? > If the sample() integration looks good, CMS assumptions aside, is there a > chance to merge it as a stand-alone action? That's true no publicly > available CMS would use it for a while, but when they decide to, the code > would already be there. And the code is not dead, as we'll be using it as > well. It's better than no users at all. > >Do you have any thoughts about supporting other monitoring technology > >that OVS supports (e.g. sFlow) using similar techniques? > I haven't targeted any of them specifically, but it doesn't seem to be a > daunting task. One only need some way to associate sample() instance and a > sFlow receiver the same way collector_set_id does for IPFIX. > > I'd suggest to generalize Flow_Sample_Collector_Set somehow, but we agreed > configuring things through this table in OVN scenario is suboptimal. Any > thoughts? Did we have an earlier discussion? I've spent a few minutes searching my email archive and I don't see one. If there was one, can you point it out? Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch_v8] ovn: Add datapaths of interest filtering.
On Tue, Nov 29, 2016 at 4:10 AM, Liran Schour wrote: > Ben Pfaff wrote on 29/11/2016 12:51:51 AM: > > > I hope that this version is ready to go in. Liran, are you happy with > > this version? > > > > I did some short evaluation and got the following results: > Simulate 50 hosts, each host serves 20 logical ports, each logical network > is spread over 6 of the 50 hosts. Thanks for testing Liran Can a few questions be asked so the differences are understood. There is expectation that not monitoring logical ports will make a difference, but this large is not expected. 1) This topology seems to only use logical switches in a flat topology for each tenant; is that correct ? 2) What is the distribution of logical switches per HV ? 3) What is the total number of logical switches we end up with ? > > > Darrell patch: > -- > Host 1 # flows 1504 > Host 2 # flows 1626 > Host 3 # flows 1443 > ... > Logical flows = 8016 > Elapsed time 207,990ms > total of 1002 (in 208ms per port) > Ports created and bound in 5,648,773,788,428 cycles > 1% south 69,936,025,644 > 1% north 77,863,484,163 > 97% clients 5,500,974,278,621 > > Conditional monitor V16 patch: > -- > Host 1 # flows 1358 > Host 2 # flows 1482 > Host 3 # flows 1296 > ... > Logical flows = 8016 > Elapsed time 207,998ms > total of 1002 (in 208ms per port) > Ports created and bound in 1,216,848,309,516 cycles 4) This is only the CPU time to initially create and bind logical ports to each HV ? How exactly is this time measured ? 5) This total cycles is in the trillions The previous tests from V10 and V15 below seem consistent and in the 2 digit billions, which is at least 30 times less than now. Is this same test as before using the 50 hosts configuration ? Evaluation on simulated environment of 50 hosts and 1000 logical ports shows the following results (cycles #): LN spread over # hosts|master| patch| change - 1 | 24597200127 | 24339235374 | 1.0% 6 | 23788521572 | 19145229352 <(914)%20522-9352> | 19.5% 12 | 23886405758 | 17913143176 | 25.0% 18 | 25812686279 | 23675094540 | 8.2% 24 | 28414671499 | 24770202308 | 12.8% 30 | 31487218890 | 28397543436 | 9.8% 36 | 36116993930 | 34105388739 | 5.5% 42 | 37898342465 | 38647139083 | -1.9% 48 | 41637996229 | 41846616306 | -0.5% 50 | 41679995357 | 43455565977 | -4.2% and from V15 Evaluation on simulated environment of 50 hosts and 1000 logical ports shows the following results (cycles #): LN spread over # hosts|master| patch| change - 1 | 24597200127 | 24339235374 | 1.0% 6 | 23788521572 | 19145229352 <(914)%20522-9352> | 19.5% 12 | 23886405758 | 17913143176 | 25.0% 18 | 25812686279 | 23675094540 | 8.2% 24 | 28414671499 | 24770202308 | 12.8% 30 | 31487218890 | 28397543436 | 9.8% 36 | 36116993930 | 34105388739 | 5.5% 42 | 37898342465 | 38647139083 | -1.9% 48 | 41637996229 | 41846616306 | -0.5% 50 | 41679995357 | 43455565977 | -4.2% > > 2% south 30,653,838,360 > > 3% north 42,543,336,355 6) Is this ovsdb NB database server CPU cycles ?; (it is hard to see why the cycles would differ here, although the numbers are small compared to the HV) > > 93% clients 1,143,651,134,801 > > You can see that this patch significantly degrades the overhead of > computation comparing to my origin patch (rebased version). > > An obvious difference is that Darrell's patch has conditions only on the > Logical_Flow table and the origin patch did that on Port_Binding, > Logical_Flow, Multicast_Group and MAC_Binding tables. > > > I can resubmit my rebased patch again and Darell can base his changes on > top of it or just investigate the problem. > > The code is here: https://github.com/liranschour/ovsBranch: > monitor_cond_ovn_v16 > > Thanks, > - Liran > > > On Sun, Nov 27, 2016 at 01:08:59PM -0800, Darrell Ball wrote: > > > This patch adds datapaths of interest support where only datapaths of > > > local interest are monitored by the ovn-controller ovsdb client. The > > > idea is to do a flood fill in ovn-controller of datapath associations > > > calculated by northd. A new column is added to the SB database > > > datapath_binding table - related_datapaths to facilitate this so all > > > datapaths associations are known quickly in ovn-controller. This > > > allows monitoring to adapt quickly with a single new monitor setting > > > for all datapaths of interest locally. > > > > > >
[ovs-dev] [PATCH] ofproto: Support reset_counts on flow mod message.
If the reset_counts flag is set on a flow modification message, the
flow counters must be cleared, even if the flow does not already have
the reset_counts flag set. And the flow modification message is not
able to set the reset_counts flag in the flow, so the flag must be
tracked separately and not saved in the flow state.
---
include/openvswitch/ofp-util.h | 2 ++
ofproto/ofproto.c | 5 -
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 8703d2a..b7a32dc 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -277,6 +277,8 @@ enum ofputil_flow_mod_flags {
set or modified. */
OFPUTIL_FF_NO_READONLY = 1 << 7, /* Allow rules within read only tables
to be modified */
+OFPUTIL_FF_MOD_RESET_COU = 1 << 8, /* Reset counters for a flow
modification
+ or delete message */
};
/* Protocol-independent flow_mod.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 53b7226..5af1828 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5094,6 +5094,9 @@ replace_rule_start(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
new_rule->idle_timeout = old_rule->idle_timeout;
new_rule->hard_timeout = old_rule->hard_timeout;
*CONST_CAST(uint16_t *, &new_rule->importance) =
old_rule->importance;
+if (new_rule->flags & OFPUTIL_FF_RESET_COUNTS) {
+old_rule->flags |= OFPUTIL_FF_MOD_RESET_COU;
+}
new_rule->flags = old_rule->flags;
new_rule->created = old_rule->created;
}
@@ -5160,7 +5163,7 @@ replace_rule_finish(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
struct ovs_list *dead_cookies)
OVS_REQUIRES(ofproto_mutex)
{
-bool forward_counts = !(new_rule->flags & OFPUTIL_FF_RESET_COUNTS);
+bool forward_counts = !(new_rule->flags & (OFPUTIL_FF_RESET_COUNTS |
OFPUTIL_FF_MOD_RESET_COU));
struct rule *replaced_rule;
replaced_rule = (old_rule && old_rule->removed_reason != OFPRR_EVICTION)
--
2.10.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document
On Tue, Nov 29, 2016 at 02:54:38PM -0800, Yin Lin wrote: > 1. I do not like a new daemon either. But as Nithin and Ben pointed out, > there might be a lot of push back if we make ovs-switchd manipulate ovsdb > by adding and deleting ports. > 2. The cmd calls can easily be done away by creating a wrapper around > ovs-vsctl.c and compile it with my daemon. The problem was that all > functions in ovs-vsctl.c is static and I'm not too happy with copying > almost 90% of the codes over to a new file. We need to figure out the best > way to share the code. But according to feedback from Nithin, it is not > urgent for now and we can post is as a follow up patch. There is precedent for a system-specific daemon in OVS. On XenServer, OVS uses a daemon named ovs-xapi-sync to deal with some system integration issues. It's written in Python and the source code is in xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync. If you really need lots of code from ovs-vsctl in your new daemon, then the right thing to do is to move those functions into a library and then use the library from both places. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Subprocesos y actividades productivas
En línea y en Vivo / Para todo su Equipo con una sola Conexión Guía para Elaborar un Mapeo de Procesos 13 de diciembre - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00 Hrs Este seminario está diseñado, en teoría y práctica, para aprender a mapear los procesos con un enfoque analítico que identifique relaciones, dependencias y secuencias en las actividades de la organización, desde los macro procesos empresariales hasta su desdoblamiento en los subprocesos y actividades productivas. TEMARIO: 1. Estrategia, procesos, actividades y su interrelación. 2. Preparación del mapeo. 3. Realización del mapeo. 4. Resultado del mapeo y entrega de resultados. ...¡Y mucho más! ¿Requiere la información a la Brevedad? responda este email con la palabra: Info - Mapeo. centro telefónico: 018002129393 Lic. Pamela Rangel Coordinador de Evento ¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la suscripción, solicite su BAJA. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document
Hi Alin, Thanks for the feedbacks. As regards to your three concerns: 1. I do not like a new daemon either. But as Nithin and Ben pointed out, there might be a lot of push back if we make ovs-switchd manipulate ovsdb by adding and deleting ports. 2. The cmd calls can easily be done away by creating a wrapper around ovs-vsctl.c and compile it with my daemon. The problem was that all functions in ovs-vsctl.c is static and I'm not too happy with copying almost 90% of the codes over to a new file. We need to figure out the best way to share the code. But according to feedback from Nithin, it is not urgent for now and we can post is as a follow up patch. 3. There is a trade off here. Without .NET it's too much pain to work with WMI event callbacks and the code is so hard to read and prone to err that it's hardly maintainable. I don't think it's excessive to require a Windows server to have basic .NET framework installed, without which not many Windows apps are even able to launch. Best regards, Yin Lin On Mon, Nov 28, 2016 at 11:18 AM, Alin Serdean < aserd...@cloudbasesolutions.com> wrote: > Hi Yin, > > Thanks a lot for the patch and the document! > > There are a few concerns that I have with this implementation: > - the need for a new daemon, > - use of cmd calls to add/delete ports, > - addition of .NET to compile the binaries. > > As previously discussed in the meetings, we could just use 'ovs-vswitchd' > with an argument i.e. '--integration_bridge=' to add the ports. > We see the port creations in the kernel and we could issue an event to the > userspace to add ports under that bridge. We could add a new type of > 'action' or try to reuse the events that are already present in the > implementation. > Another advantage to use ovs-vswitchd is that we could talk directly to > ovsdb. Also, no additional dependency needed. > > We could add the argument '--integration_bridge=' when creating > the service via the installer with a nice text to inform the user about > what it does. Changing the service properties is normal activity for a > system administrator, so he could easily remove it afterwards. > > Thanks, > Alin. > > > -Original Message- > > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > > boun...@openvswitch.org] On Behalf Of Yin Lin > > Sent: Monday, November 21, 2016 10:07 PM > > To: d...@openvswitch.org > > Subject: [ovs-dev] OVS-Hyper-V-Discovery-Agent Design Document > > > > Hi, > > > > Below is a document that describes the design and implementation of VIF > > discovery agent for Hyper-V that I have been working on. The coding has > > already been completed and will be sent out for review in a follow up > patch. > > The document describes the effort and the choices made. Thanks Sairam > > Venugopal for the initial review and edit of the document > > > > > > > > Please have a look, and get in touch for any questions or comments. > > > > > > > > Thanks! > > > > -- Yin > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header
Can we decide if tcp and icmp is null in OvsConntrackValidateTcpPacket? It
makes the function more complete and safer by itself.
On Mon, Nov 28, 2016 at 6:11 AM, Alin Serdean <
aserd...@cloudbasesolutions.com> wrote:
> This patch checks if the TCP or ICMP header exists before trying to use
> them.
>
> The issue was found using the driver under low resources.
>
> Signed-off-by: Alin Gabriel Serdean
> ---
> datapath-windows/ovsext/Conntrack.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Conntrack.c
> b/datapath-windows/ovsext/Conntrack.c
> index e663c3b..56a7cbc 100644
> --- a/datapath-windows/ovsext/Conntrack.c
> +++ b/datapath-windows/ovsext/Conntrack.c
> @@ -194,7 +194,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
> TCPHdr tcpStorage;
> const TCPHdr *tcp;
> tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage);
> -if (!OvsConntrackValidateTcpPacket(tcp)) {
> +if (!tcp || !OvsConntrackValidateTcpPacket(tcp)) {
> goto invalid;
> }
>
> @@ -215,7 +215,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
> ICMPHdr storage;
> const ICMPHdr *icmp;
> icmp = OvsGetIcmp(curNbl, l4Offset, &storage);
> -if (!OvsConntrackValidateIcmpPacket(icmp)) {
> +if (!icmp || !OvsConntrackValidateIcmpPacket(icmp)) {
> goto invalid;
> }
>
> --
> 2.10.2.windows.1
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Fideicomisos en México
Seguridad y confidencialidad para la protección de su patrimonio Fideicomisos en México 09 de diciembre - Zona Reforma (Ciudad de México) - 9:00 a 18:00 Imparte: C.P. Juan de Dios Barba Nava Aprenda en este seminario sobre su administración, operación y la adecuada rendición de cuentas que debe realizar durante el plazo de su cumplimiento; conozca las distintas modalidades existentes, las obligaciones que implica en materia fiscal y el marco legal que lo regula. Para el financiamiento empresarial y la inversión en su pequeña, mediana o grande empresa, el fideicomiso le brindará certeza para la consecución de los fines planteados. TEMARIO: 1.Concepto jurídico del fideicomiso. 2. Régimen fiscal del fideicomiso. 3. Facultades, Obligaciones y deberes de los participantes. ...¡Y mucho más! ¿Requiere la información a la Brevedad? responda este email con la palabra: fideicomiso. centro telefónico:018002120744 Será un placer Atenderle Si desea cancelar la suscripción, solicite su BAJA y se realizará en las próximas 24 Hrs. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Payment completed
-- The payment is made. Please confirm your receipt of the payment. attached is the payment confirmation copy Jeannie Helmke H.S. Para CONFIDENTIALITY NOTICE: The information contained in this message (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally protected from disclosure. If you are not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, you are hereby notified that any retention, dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify the Sender immediately by replying to the message and deleting it from your computer.___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header
On 28 November 2016 at 06:11, Alin Serdean wrote: > This patch checks if the TCP or ICMP header exists before trying to use > them. > > The issue was found using the driver under low resources. > > Signed-off-by: Alin Gabriel Serdean (This is not a review, just a friendly reminder about patch ettiquette) Does this cause a kernel crash? Please use the imperative in git commit titles - It is easier to understand the evolution of the code in git logs when commits have clear titles like "Fix null dereference in conntrack". It is also helpful to include the potential impact of the change in the commit message (ie, kernel crash? memory leak? etc.). The "why" is more important in the commit message than the "what" - the "what" is already stated in the code. Linus wrote some good patch commit message guidelines here: https://github.com/torvalds/subsurface/blob/a48494d2fbed58c751e9b7e8fbff88582f9b2d02/README#L91 Thanks, Joe ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpdk: Fix abort on double free.
On 29/11/2016 07:57, "Aaron Conole" wrote:
>Hi Ilya,
>
>Ilya Maximets writes:
>
>> On 28.11.2016 21:55, Aaron Conole wrote:
>>> Ilya Maximets writes:
>>>
According to DPDK API (lib/librte_eal/common/include/rte_eal.h):
"After the call to rte_eal_init(), all arguments argv[x]
with x < ret may be modified and should not be accessed
by the application."
This means, that OVS must not free the arguments passed to DPDK.
In real world, 'rte_eal_init()' replaces the last argument in
'dpdk_argv' with the first one by doing this:
>>>
>>> Thanks for spotting this error, Ilya.
>>>
# eal_parse_args() from lib/librte_eal/linuxapp/eal/eal.c
char *prgname = argv[0];
...
if (optind >= 0)
argv[optind-1] = prgname;
This leads to double free inside 'deferred_argv_release()' and
possible ABORT at exit:
>>>
>>> I haven't seen this, which is both shocking and scary - the commit which
>>> does this copy is almost 4 years old; did you have to do anything
>>> specific for this behavior to occur? Did something change in DPDK
>>> recently that exposed this behavior? Just wondering how you reproduced
>>> it.
>>
>> Abort was caught up accidentally. I'm able to reproduce it on my a
>> little unusual testing system (ARMv8 + Fedora 21 + clang 3.5) without
>> any specific manipulations. The bug exists always but it's hard
>> for libc to detect double free here because there are many other
>> frees/allocations at exit time. I've used following patch to confirm
>> the issue if it wasn't detected by libc:
>
>Well, it's at least good that you can observe it consistently. Did you
>try my provided patch to see if that works as well?
>
>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>> index 49a589a..65d2d28 100644
>> --- a/lib/dpdk.c
>> +++ b/lib/dpdk.c
>> @@ -258,6 +258,8 @@ deferred_argv_release(void)
>> {
>> int result;
>> for (result = 0; result < dpdk_argc; ++result) {
>> +VLOG_INFO("DPDK ARGV release: %2d: 0x%" PRIx64 " (%s)",
>> + result, (intptr_t)dpdk_argv[result], dpdk_argv[result]);
>> free(dpdk_argv[result]);
>> }
>>
>
>It's quite glaring after studying the code. Really good catch!
I agree, thanks for spotting this
>
>>>
*** Error in `ovs-vswitchd': double free or corruption (fasttop) <...> ***
Program received signal SIGABRT, Aborted.
#0 raise () from /lib64/libc.so.6
#1 abort () from /lib64/libc.so.6
#2 __libc_message () from /lib64/libc.so.6
#3 free () from /lib64/libc.so.6
#4 deferred_argv_release () at lib/dpdk.c:261
#5 __run_exit_handlers () from /lib64/libc.so.6
#6 exit () from /lib64/libc.so.6
#7 __libc_start_main () from /lib64/libc.so.6
#8 _start ()
Fix that by not calling free for the memory passed to DPDK.
CC: Aaron Conole
Fixes: bab694097133 ("netdev-dpdk: Convert initialization from
cmdline to db")
Signed-off-by: Ilya Maximets
---
>>>
>>> We need to free the memory - I think that is not a question;
>>
>> Actually, it is. According to DPDK API (see above) 'rte_eal_init()'
>> takes the ownership of 'argv'. This means that we must not free
>> or use this memory.
>
>Apologies for the ranty-wall of text below.
>
>DPDK *cannot* take ownership of freeing this memory, unless 1) it expects a
>completely separate array from argv/argc than the one passed during
>program execution and initialization, or 2) it expects the hosted
>environment to give it the responsibility of cleaning this up. It
>explicitly claims that the argv/argc is what comes from main(), and
>therefore should obey the restrictions and privileges afforded those
>variables.
>
>In fact, I don't even see anywhere that dpdk preserves argv, *at all*.
>Looking through the history very quickly (admittedly just back to commit
>af75078fece3615088e561357c1e97603e43a5fe in dpdk) confirms that dpdk
>hasn't stored the arguments anywhere to do any processing.
>
>DPDK api guide does NOT state that it takes possession - and that matches
>with what happens in the code, BUT I will agree the statement
>
> 'all arguments argv[x] with x < ret may be modified and should not be
> accessed by the application'
>
>is a bit ambiguous. I think it's trying to say that the application should do
>its getopt()s parsing before calling the dpdk init routine, because DPDK libs
>will change the array. I don't see a reason for modifying the array in
>the code (the `argv[optind-1] = progname`), but if the dpdk library wants
>to do that, it is free to do so according to C99 5.1.2.2.1; I think
>it's best we always free what we allocate, which is why I suggested the
>side array patch which stores additional pointers to the data to be
>free'd up at exit.
>
>I am not sure which is more appropriate, since this is an exit condition,
>after all. The memory will get free()d up
[ovs-dev] prestaciones y beneficios
En línea y en Vivo / Para todo su Equipo con una sola Conexión Administración de Sueldos y Programas de Compensación 14 de diciembre - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00 Hrs Mantenga salarios competitivos sin dañar la economía de su empresa. Las presiones económicas y las regulaciones del gobierno pueden convertir su política de sueldos y salarios en una maraña de contradicciones. Analice la situación actual. "Pregunte Por Nuestra Promoción Navideña" Temario: 1. Anatomía de los Sueldos y los programas de Compensación. 2. Problemática actual de las prácticas de compensación. 3. Análisis, descripción y valuación de puestos. 4. Administración de prestaciones y beneficios. ...¡Y mucho más! ¿Requiere la información a la Brevedad? responda este email con la palabra: Info - Sueldos. centro telefónico: 018002129393 Lic. Ángel Hernández Coordinador de Evento ¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la suscripción, solicite su BAJA. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] ovn-northd: Force SNAT for multiple gateway routers.
On Tue, Nov 29, 2016 at 9:12 AM, Guru Shetty wrote:
>
>
> On 28 November 2016 at 19:33, Mickey Spiegel
> wrote:
>
>> Acked-by: Mickey Spiegel
>>
>
> Thank you for the thorough review and ideas. It looks a lot better now
> than when I started.
>
>
>>
>> A few comments and nits below.
>>
>>
>>>
>>>
>> I never noticed before, but I am unsure why the "continue" in line 3693
>> (without changes), line 3768 (with changes), should be there. In order to
>> generate ARP replies for SNAT external IPs, this seems to assume that
>> all external IPs for SNAT are router interface IPs, and that all ARP
>> requests for those IPs will arrive over the corresponding router
>> interface.
>> The latter condition is true for gateway routers, given that they connect
>> to the distributed router over a single join network.
>>
>
>> If the continue were removed, then both assumptions would go away.
>>
>> What I am pointing out is old code, it just gets emphasized more with
>> this new functionality. Explicit SNAT rules point outward and so uses
>> an IP address on a router interface pointing outward. With the new
>> force_snat functionality, due to the "continue" in line 3693, you would
>> have to use an IP address on the gateway router's interface to the join
>> network, which is exactly what you have done in the tests below.
>>
>
> Right. The general assumption so far (which I think is kind of fair) has
> been that you only need additional ARP entries for floating IP addresses
> which may not be a IP address of your gateway router. The gateway router's
> configured IP addresses already get their own ARP entries automatically.
> And a pure SNAT IP address is always one of the gateway IP addresses (when
> I say 'pure', I mean not the 'dnat_and_snat' case).
>
> If you remove the "continue", I think there will be duplicate flows
> (ovn-controller will complain about it) and that will need to be handled
> carefully with additional logic.
>
>
>>
>> I can revisit this in the distributed router patch set if you feel that
>> it is
>> not relevant to NAT on a gateway router.
>>
>
> Sure.
>
I will look at this again as part of distributed NAT. If I can assume that
there are no
duplicate IP addresses specified on a router interface, it is relatively
straightforward
to add code that avoids duplicate flows. However, I don't see any such
check, so I
probably cannot make that assumption.
>> I am unable to come up with a rationale for the different priority values
>> in the
>> S_ROUTER_IN_UNSNAT stage. Explicit SNAT rules use priority 100,
>> dnat_force_snat uses priority 110, and lb_force_snat uses priority 100.
>> It seems like there can be cases where the same IP address is specified
>> for
>> dnat_force_snat and lb_force_snat. With NAT on a distributed router, it
>> would
>> be possible for all 3 to use the same IP address. The actions are the
>> same,
>> so it will all work regardless. It just seems like it would be more
>> consistent if
>> all three cases use the same priority value, or all 3 cases use different
>> priority
>> values.
>>
>
> When they have same priority, ovn-controller will complain about duplicate
> flows. I will change it to all have different priorities to be consistent
> here and also change ovn-northd.8.xml to reflect correct behavior.
>
>
>>
>> Also note that the ovn-northd.8.xml changes describe all of these flows as
>> priority 100 flows.
>>
>
>
>> The lb_force_snat priority value of 110 is not consistent with the
>> ovn-northd.8.xml changes, which describe the use of priority 110 for
>> dnat_force_snat and 100 for lb_force_snat.
>>
>> I could not figure out why priority 110 is used for the force_snat
>> S_ROUTER_OUT_SNAT flows rather than 100, since I did not see
>> any priority 100 flows.
>>
>
> That was sloppiness from my part from changes over the iterations. I will
> change both to 100 and apply this patch with the following total
> incremental.
>
>
The incremental below looks good to me.
Mickey
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 11245c6..0e84f2f 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -1145,19 +1145,25 @@ icmp4 {
>
>
>
> - For each configuration in the OVN Northbound database, that asks
> - to change the source IP address of a packet from A to
> - B, a priority-100 flow matches ip &&
> - ip4.dst == B with an action
> - ct_snat; next;.
> + If the Gateway router has been configured to force SNAT any
> + previously DNATted packets to B, a priority-110 flow
> + matches ip && ip4.dst == B with
> + an action ct_snat; next;.
>
>
>
> - If the Gateway router has been configured to force SNAT (any
> - previously DNATted or Load-balanced packets) to B,
> - a priority-100 flow matches ip &&
> - ip4.dst == B with an action ct_snat;
> - next;.
> +
Re: [ovs-dev] [PATCH] datapath-windows: conntrack leaks
Acked-by: Sairam Venugopal
On 11/28/16, 6:26 AM, "Alin Serdean"
wrote:
>All conntrack entries should be removed before unloading/disabling the
>driver.
>
>This patch forces a flush of all the entries during the cleanup routine.
>
>The bug was found using driver verifier.
>
>Signed-off-by: Alin Gabriel Serdean
>---
> datapath-windows/ovsext/Conntrack.c | 5 +
> 1 file changed, 5 insertions(+)
>
>diff --git a/datapath-windows/ovsext/Conntrack.c
>b/datapath-windows/ovsext/Conntrack.c
>index 56a7cbc..f482783 100644
>--- a/datapath-windows/ovsext/Conntrack.c
>+++ b/datapath-windows/ovsext/Conntrack.c
>@@ -42,6 +42,8 @@ static PNDIS_RW_LOCK_EX ovsConntrackLockObj;
> extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
> static UINT64 ctTotalEntries;
>
>+static __inline NDIS_STATUS OvsCtFlush(UINT16 zone);
>+
> /*
>
>*-
>---
> * OvsInitConntrack
>@@ -117,6 +119,9 @@ OvsCleanupConntrack(VOID)
> KernelMode, FALSE, NULL);
> ObDereferenceObject(ctThreadCtx.threadObject);
>
>+/* Force flush all entries before removing */
>+OvsCtFlush(0);
>+
> if (ovsConntrackTable) {
> OvsFreeMemoryWithTag(ovsConntrackTable, OVS_CT_POOL_TAG);
> ovsConntrackTable = NULL;
>--
>2.10.2.windows.1
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_
>mailman_listinfo_ovs-2Ddev&d=DgICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5
>ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=YwYsigaUYWgveMwCzujBjqz3KqEspWrBeE5w6HP
>nHEc&s=Ow2HR_t1p-fHBAYFikzK-mUTLa_RMJhqoEi2XkUHXPk&e=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] datapath-windows: null comparison for icmp and tcp header
Acked-by: Sairam Venugopal
On 11/28/16, 6:11 AM, "Alin Serdean"
wrote:
>This patch checks if the TCP or ICMP header exists before trying to use
>them.
>
>The issue was found using the driver under low resources.
>
>Signed-off-by: Alin Gabriel Serdean
>---
> datapath-windows/ovsext/Conntrack.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Conntrack.c
>b/datapath-windows/ovsext/Conntrack.c
>index e663c3b..56a7cbc 100644
>--- a/datapath-windows/ovsext/Conntrack.c
>+++ b/datapath-windows/ovsext/Conntrack.c
>@@ -194,7 +194,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
> TCPHdr tcpStorage;
> const TCPHdr *tcp;
> tcp = OvsGetTcp(curNbl, l4Offset, &tcpStorage);
>-if (!OvsConntrackValidateTcpPacket(tcp)) {
>+if (!tcp || !OvsConntrackValidateTcpPacket(tcp)) {
> goto invalid;
> }
>
>@@ -215,7 +215,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
> ICMPHdr storage;
> const ICMPHdr *icmp;
> icmp = OvsGetIcmp(curNbl, l4Offset, &storage);
>-if (!OvsConntrackValidateIcmpPacket(icmp)) {
>+if (!icmp || !OvsConntrackValidateIcmpPacket(icmp)) {
> goto invalid;
> }
>
>--
>2.10.2.windows.1
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_
>mailman_listinfo_ovs-2Ddev&d=DgICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5
>ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=-wRL0MTKq-S9BvlASxl9rr16XAVdEV1G13MXuz-
>w4Uc&s=DpjyrkLlyPupeYfaSrHUQUlVt5PzkEdOvOzvNfViLPQ&e=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] datapath-windows: pop buffer from packet / pop_vlan bug
Looks like my previous Acked-by was missed out. Resending this.
Acked-by: Sairam Venugopal
On 10/27/16, 4:48 PM, "Alin Serdean"
wrote:
>Switch too memmove(RtlMoveMemory) instead of copy and predefined allocated
>buffer.
>
>Currently if we receive a pop_vlan action, and the vlan tag is inside the
>Ethernet frame(not in the net buffer list information) we change the frame
>without checking if the it was a vlan tagged or not.
>
>This patch checks if it a vlan tagged frame and makes the action a
>non-operation.
>
>
>Signed-off-by: Alin Gabriel Serdean
>---
>this patch is intended for 2.6 as well
>---
> datapath-windows/ovsext/Actions.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c
>b/datapath-windows/ovsext/Actions.c
>index f46309a..9a58fbd 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -1079,9 +1079,6 @@ OvsPopFieldInPacketBuf(OvsForwardingContext
>*ovsFwdCtx,
> UINT32 packetLen, mdlLen;
> PNET_BUFFER_LIST newNbl;
> NDIS_STATUS status;
>-PUINT8 tempBuffer[ETH_HEADER_LENGTH];
>-
>-ASSERT(shiftOffset > ETH_ADDR_LENGTH);
>
> newNbl = OvsPartialCopyNBL(ovsFwdCtx->switchContext,
>ovsFwdCtx->curNbl,
>0, 0, TRUE /* copy NBL info */);
>@@ -1118,8 +1115,16 @@ OvsPopFieldInPacketBuf(OvsForwardingContext
>*ovsFwdCtx,
> return NDIS_STATUS_FAILURE;
> }
> bufferStart += NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
>-RtlCopyMemory(tempBuffer, bufferStart, shiftOffset);
>-RtlCopyMemory(bufferStart + shiftLength, tempBuffer, shiftOffset);
>+/* XXX At the momemnt !bufferData means it should be treated as
>VLAN. We
>+ * should split the function and refactor. */
>+if (!bufferData) {
>+EthHdr *ethHdr = (EthHdr *)bufferStart;
>+/* If the frame is not VLAN make it a no op */
>+if (ethHdr->Type != ETH_TYPE_802_1PQ_NBO) {
>+return NDIS_STATUS_SUCCESS;
>+}
>+}
>+RtlMoveMemory(bufferStart + shiftLength, bufferStart, shiftOffset);
> NdisAdvanceNetBufferDataStart(curNb, shiftLength, FALSE, NULL);
>
> if (bufferData) {
>--
>2.9.2.windows.1
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=HblfY6XT1rD61UBQ5NSkOwYlhBq1Me
>cvuKrde4eEF2M&s=BBOF5TgehYe54YgPAl-EyK-CBXrn8o9ri_jhAhqN8kw&e=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2.5 2/2] tests: add test for the output of ovs/route/lookup
Signed-off-by: Thadeu Lima de Souza Cascardo Signed-off-by: Simon Horman --- tests/ovs-router.at | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/ovs-router.at b/tests/ovs-router.at index 96c051e..ddf6036 100644 --- a/tests/ovs-router.at +++ b/tests/ovs-router.at @@ -1,4 +1,5 @@ -AT_BANNER([appctl route/add with gateway]) +AT_BANNER([ovs-router]) + AT_SETUP([appctl - route/add with gateway]) AT_KEYWORDS([ovs_router]) AT_XFAIL_IF([test "$IS_WIN32" = "yes"]) @@ -11,3 +12,29 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.1.0/24 br0 2.2.2.10], [0], [OK ]) OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([appctl - route/lookup]) +AT_KEYWORDS([ovs_router]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy]) +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 192.0.2.1/24], [0], [OK +]) +AT_CHECK([ovs-appctl ovs/route/add 198.51.100.0/24 br0 192.0.2.254], [0], [OK +]) +AT_CHECK([ovs-appctl ovs/route/lookup 198.51.100.1], [0], [gateway 192.0.2.254 +dev br0 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([appctl - route/lookup6]) +AT_KEYWORDS([ovs_router]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy]) +AT_CHECK([ovs-appctl netdev-dummy/ip6addr br0 2001:db8:cafe::1/64], [0], [OK +]) +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:babe::/64 br0 2001:db8:cafe::2], [0], [OK +]) +AT_CHECK([ovs-appctl ovs/route/lookup 2001:db8:babe::1eaf], [0], [gateway 2001:db8:cafe::2 +dev br0 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2.5 1/2] ovs-router: Fix selection of source IP address when a gateway ip is introduced
From: Ariel Waizel Only pick up the test part of upstream commit 4e08f54bf1cee1d26d3a9cb645365dfefbe457b5. When adding a VXLAN tunnel that connects to a VTEP residing in a different IP network, the tunnel source ip needs to be selected by best fit (longest matching netmask), based on the destination VTEP ip, and the specific route's gateway ip. A bug in ovs-router.c made the source ip to be decided only based on the destination ip. Thus, if all source ips available to OVS and the destination ip are in different ip networks - no source ip is selected, and an error is returned. This error occurred when using OVS-DPDK and configuring a VXLAN tunnel, where source ip and destination ip are in different networks, and a gateway ip was in place for the specific route. The fix tries to match a source ip based on the gateway ip, if no matching source ip was found based on the destination ip. This way, the gateway becomes the first hop only if the tunnel crosses between ip networks. Signed-off-by: Ariel Waizel Signed-off-by: Thadeu Lima de Souza Cascardo Acked-by: Pravin B Shelar --- tests/automake.mk | 1 + tests/ovs-router.at | 13 + tests/testsuite.at | 1 + 3 files changed, 15 insertions(+) create mode 100644 tests/ovs-router.at diff --git a/tests/automake.mk b/tests/automake.mk index fa6b2ee..f477929 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -51,6 +51,7 @@ TESTSUITE_AT = \ tests/tunnel.at \ tests/tunnel-push-pop.at \ tests/tunnel-push-pop-ipv6.at \ + tests/ovs-router.at \ tests/lockfile.at \ tests/reconnect.at \ tests/ovs-vswitchd.at \ diff --git a/tests/ovs-router.at b/tests/ovs-router.at new file mode 100644 index 000..96c051e --- /dev/null +++ b/tests/ovs-router.at @@ -0,0 +1,13 @@ +AT_BANNER([appctl route/add with gateway]) +AT_SETUP([appctl - route/add with gateway]) +AT_KEYWORDS([ovs_router]) +AT_XFAIL_IF([test "$IS_WIN32" = "yes"]) +OVS_VSWITCHD_START([add-port br0 p2 -- set Interface p2 type=gre \ + options:local_ip=2.2.2.2 options:remote_ip=1.1.1.1 \ + -- add-port br0 p1 -- set interface p1 type=dummy]) +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 2.2.2.2/24], [0], [OK +]) +AT_CHECK([ovs-appctl ovs/route/add 1.1.1.0/24 br0 2.2.2.10], [0], [OK +]) +OVS_VSWITCHD_STOP +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index 6fd2dbd..a768f10 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -49,6 +49,7 @@ m4_include([tests/jsonrpc-py.at]) m4_include([tests/tunnel.at]) m4_include([tests/tunnel-push-pop.at]) m4_include([tests/tunnel-push-pop-ipv6.at]) +m4_include([tests/ovs-router.at]) m4_include([tests/lockfile.at]) m4_include([tests/reconnect.at]) m4_include([tests/ovs-vswitchd.at]) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] ovn-northd: Force SNAT for multiple gateway routers.
On 28 November 2016 at 19:33, Mickey Spiegel wrote:
> Acked-by: Mickey Spiegel
>
Thank you for the thorough review and ideas. It looks a lot better now
than when I started.
>
> A few comments and nits below.
>
>
>>
>>
> I never noticed before, but I am unsure why the "continue" in line 3693
> (without changes), line 3768 (with changes), should be there. In order to
> generate ARP replies for SNAT external IPs, this seems to assume that
> all external IPs for SNAT are router interface IPs, and that all ARP
> requests for those IPs will arrive over the corresponding router interface.
> The latter condition is true for gateway routers, given that they connect
> to the distributed router over a single join network.
>
> If the continue were removed, then both assumptions would go away.
>
> What I am pointing out is old code, it just gets emphasized more with
> this new functionality. Explicit SNAT rules point outward and so uses
> an IP address on a router interface pointing outward. With the new
> force_snat functionality, due to the "continue" in line 3693, you would
> have to use an IP address on the gateway router's interface to the join
> network, which is exactly what you have done in the tests below.
>
Right. The general assumption so far (which I think is kind of fair) has
been that you only need additional ARP entries for floating IP addresses
which may not be a IP address of your gateway router. The gateway router's
configured IP addresses already get their own ARP entries automatically.
And a pure SNAT IP address is always one of the gateway IP addresses (when
I say 'pure', I mean not the 'dnat_and_snat' case).
If you remove the "continue", I think there will be duplicate flows
(ovn-controller will complain about it) and that will need to be handled
carefully with additional logic.
>
> I can revisit this in the distributed router patch set if you feel that it
> is
> not relevant to NAT on a gateway router.
>
Sure.
>
>
>> @@ -3845,6 +3920,12 @@ build_lrouter_flows(struct hmap *datapaths, struct
>> hmap *ports,
>> continue;
>> }
>>
>> +ovs_be32 snat_ip;
>> +const char *dnat_force_snat_ip = get_force_snat_ip(od, "dnat",
>> + &snat_ip);
>> +const char *lb_force_snat_ip = get_force_snat_ip(od, "lb",
>> + &snat_ip);
>> +
>> /* A set to hold all ips that need defragmentation and tracking.
>> */
>> struct sset all_ips = SSET_INITIALIZER(&all_ips);
>>
>> @@ -3867,14 +3948,16 @@ build_lrouter_flows(struct hmap *datapaths,
>> struct hmap *ports,
>> sset_add(&all_ips, ip_address);
>> }
>>
>> -/* Higher priority rules are added in DNAT table to
>> match on
>> - * ct.new which in-turn have group id as an action for
>> load
>> - * balancing. */
>> +/* Higher priority rules are added for load-balancing in
>> DNAT
>> + * table. For every match (on a VIP[:port]), we add two
>> flows
>> + * via add_router_lb_flow(). One flow is for specific
>> matching
>> + * on ct.new with an action of "ct_lb($targets);". The
>> other
>> + * flow is for ct.est with an action of "ct_dnat;". */
>> ds_clear(&actions);
>> ds_put_format(&actions, "ct_lb(%s);", node->value);
>>
>> ds_clear(&match);
>> -ds_put_format(&match, "ct.new && ip && ip4.dst == %s",
>> +ds_put_format(&match, "ip && ip4.dst == %s",
>>ip_address);
>> free(ip_address);
>>
>> @@ -3886,11 +3969,11 @@ build_lrouter_flows(struct hmap *datapaths,
>> struct hmap *ports,
>> ds_put_format(&match, " && tcp && tcp.dst == %d",
>>port);
>> }
>> -ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT,
>> - 120, ds_cstr(&match),
>> ds_cstr(&actions));
>> +add_router_lb_flow(lflows, od, &match, &actions, 120,
>> + lb_force_snat_ip);
>> } else {
>> -ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT,
>> - 110, ds_cstr(&match),
>> ds_cstr(&actions));
>> +add_router_lb_flow(lflows, od, &match, &actions, 110,
>> + lb_force_snat_ip);
>> }
>> }
>> }
>> @@ -3981,7 +4064,13 @@ build_lrouter_flows(struct hmap *datapaths, struct
>> hmap *ports,
>> ds_clear(&match);
>> ds_put_format(&match, "ip && ip4.dst == %s",
>> nat->external_ip);
>> ds_clear(&actions);
>> -ds_put_format(&actions,"flags.loopback = 1;
>>
Re: [ovs-dev] [PATCH] dpdk: Fix abort on double free.
Hi Ilya,
Ilya Maximets writes:
> On 28.11.2016 21:55, Aaron Conole wrote:
>> Ilya Maximets writes:
>>
>>> According to DPDK API (lib/librte_eal/common/include/rte_eal.h):
>>>
>>> "After the call to rte_eal_init(), all arguments argv[x]
>>> with x < ret may be modified and should not be accessed
>>> by the application."
>>>
>>> This means, that OVS must not free the arguments passed to DPDK.
>>> In real world, 'rte_eal_init()' replaces the last argument in
>>> 'dpdk_argv' with the first one by doing this:
>>
>> Thanks for spotting this error, Ilya.
>>
>>> # eal_parse_args() from lib/librte_eal/linuxapp/eal/eal.c
>>>
>>> char *prgname = argv[0];
>>> ...
>>> if (optind >= 0)
>>> argv[optind-1] = prgname;
>>>
>>> This leads to double free inside 'deferred_argv_release()' and
>>> possible ABORT at exit:
>>
>> I haven't seen this, which is both shocking and scary - the commit which
>> does this copy is almost 4 years old; did you have to do anything
>> specific for this behavior to occur? Did something change in DPDK
>> recently that exposed this behavior? Just wondering how you reproduced
>> it.
>
> Abort was caught up accidentally. I'm able to reproduce it on my a
> little unusual testing system (ARMv8 + Fedora 21 + clang 3.5) without
> any specific manipulations. The bug exists always but it's hard
> for libc to detect double free here because there are many other
> frees/allocations at exit time. I've used following patch to confirm
> the issue if it wasn't detected by libc:
Well, it's at least good that you can observe it consistently. Did you
try my provided patch to see if that works as well?
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 49a589a..65d2d28 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -258,6 +258,8 @@ deferred_argv_release(void)
> {
> int result;
> for (result = 0; result < dpdk_argc; ++result) {
> +VLOG_INFO("DPDK ARGV release: %2d: 0x%" PRIx64 " (%s)",
> + result, (intptr_t)dpdk_argv[result], dpdk_argv[result]);
> free(dpdk_argv[result]);
> }
>
It's quite glaring after studying the code. Really good catch!
>>
>>> *** Error in `ovs-vswitchd': double free or corruption (fasttop) <...> ***
>>> Program received signal SIGABRT, Aborted.
>>>
>>> #0 raise () from /lib64/libc.so.6
>>> #1 abort () from /lib64/libc.so.6
>>> #2 __libc_message () from /lib64/libc.so.6
>>> #3 free () from /lib64/libc.so.6
>>> #4 deferred_argv_release () at lib/dpdk.c:261
>>> #5 __run_exit_handlers () from /lib64/libc.so.6
>>> #6 exit () from /lib64/libc.so.6
>>> #7 __libc_start_main () from /lib64/libc.so.6
>>> #8 _start ()
>>>
>>> Fix that by not calling free for the memory passed to DPDK.
>>>
>>> CC: Aaron Conole
>>> Fixes: bab694097133 ("netdev-dpdk: Convert initialization from
>>> cmdline to db")
>>> Signed-off-by: Ilya Maximets
>>> ---
>>
>> We need to free the memory - I think that is not a question;
>
> Actually, it is. According to DPDK API (see above) 'rte_eal_init()'
> takes the ownership of 'argv'. This means that we must not free
> or use this memory.
Apologies for the ranty-wall of text below.
DPDK *cannot* take ownership of freeing this memory, unless 1) it expects a
completely separate array from argv/argc than the one passed during
program execution and initialization, or 2) it expects the hosted
environment to give it the responsibility of cleaning this up. It
explicitly claims that the argv/argc is what comes from main(), and
therefore should obey the restrictions and privileges afforded those
variables.
In fact, I don't even see anywhere that dpdk preserves argv, *at all*.
Looking through the history very quickly (admittedly just back to commit
af75078fece3615088e561357c1e97603e43a5fe in dpdk) confirms that dpdk
hasn't stored the arguments anywhere to do any processing.
DPDK api guide does NOT state that it takes possession - and that matches
with what happens in the code, BUT I will agree the statement
'all arguments argv[x] with x < ret may be modified and should not be
accessed by the application'
is a bit ambiguous. I think it's trying to say that the application should do
its getopt()s parsing before calling the dpdk init routine, because DPDK libs
will change the array. I don't see a reason for modifying the array in
the code (the `argv[optind-1] = progname`), but if the dpdk library wants
to do that, it is free to do so according to C99 5.1.2.2.1; I think
it's best we always free what we allocate, which is why I suggested the
side array patch which stores additional pointers to the data to be
free'd up at exit.
I am not sure which is more appropriate, since this is an exit condition,
after all. The memory will get free()d up eventually by the
environments on which OvS runs. It doesn't _feel_ correct to leave the
memory dangling, since we can free it.
Anyone else have thoughts on this?
> Some thoughts:
> DPDK
Re: [ovs-dev] [PATCH v2 1/7] checkpatch: Announce the file where errors occur
Ben Pfaff writes: > On Fri, Oct 21, 2016 at 02:49:03PM -0400, Aaron Conole wrote: >> This makes finding the warning and error marks much easier. >> >> Signed-off-by: Aaron Conole > > Thanks, I applied this to master, it's certainly going to be a help > sometimes. > > However, I'm used to error message in a format like > filename:lineno: message > which a lot of editors can use to jump directly to the file and line > number. I don't know whether they support this kind of format; if they > tend to, it's also fine. I'll add this style as an option for the next batch of updates for checkpatch. Thanks for the suggestion. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Site web - Graphisme sur mesure - Hébergement et Maintenance
Votre site web sur mesure : votre graphisme et vos fonctionnalités... Votre site web sur mesure : votre graphisme et vos fonctionnalités... Visitez notre site web www.altaea.com [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw] lettre d'information du 11/2016 [http://www.altaea.com/www.altaea.com] ALTAEA SITE WEB - GRAPHISME - COMMUNICATION ET MARKETING ON LINE - HÉBERGEMENT MAINTENANCE [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw/1] [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw/2] VOTRE SITE WEB SUR MESURE A VOTRE IMAGE AVEC VOTRE GRAPHISME ET VOS FONCTIONNALITÉS : multilingue, espace privé de téléchargement, newsletter, e-commerce, recrutement, réservation [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw/3] CONTACTEZ-NOUS 09 72 23 61 25* *prix d’un appel local [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw/4] Nous créons votre site web à votre image [http://www.altaea.com/www.altaea.com] NOUS RÉALISONS UN SITE REPRENANT L’IDENTITÉ GRAPHIQUE DE VOTRE ENTREPRISE ET S’INTÉGRANT DANS VOTRE DÉMARCHE DE COMMUNICATION. NOUS ASSURONS L’ENSEMBLE DE LA CONCEPTION ( ERGONOMIE, ARCHITECTURE, DÉVELOPPEMENT… ) ET LE DESIGN (WEBDESIGN, IMAGES, MAQUETTES, VISUEL…) DE VOTRE SITE WEB. MAQUETTES, CRÉATION DES ÉLÉMENTS GRAPHIQUES ( CRÉATION ET RETOUCHE DES IMAGES, ICÔNES, BOUTONS DE NAVIGATION, BANIÈRES…)… Cliquez ici pour visiter notre site web www.altaea.com [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw/5] [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29t] Le logiciel le plus adapté à vos besoins de fonctionnalités [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tLw/6] NOUS METTRONS À VOTRE DISPOSITION LA PLATEFORME LOGICIELLE LA PLUS ADAPTÉE À VOS BESOINS DE FONCTIONNALITÉS : SITE MULTILINGUE, E-COMMERCE, CATALOGUE, RECRUTEMENT, MULTI RÉDACTEURS… NOS PLATEFORMES LOGICIELLES SONT ÉVOLUTIVES ET IL EST POSSIBLE D’AJOUTER AU FUR ET À MESURE DE VOS BESOINS LES « BRIQUES » NÉCESSAIRES. NOS SITES SONT CONÇUS POUR S’ADAPTER AUX DIFFÉRENTES TAILLES D’ÉCRAN ET AUX DIFFÉRENTS APPAREILS TACTILES. désincription [http://www.altaea.com/mymail/782195/c9571ff1bac6edb609fec5316ee510fa/aHR0cDovL3d3dy5hbHRhZWEuY29tL25ld3NsZXR0ZXItc2lnbnVwL3Vuc3Vic2NyaWJl] ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC] [PATCH] ovn: Support sample action in logical datapath
Hi Ben, On 29.11.2016 05:21, Ben Pfaff wrote: On Fri, Oct 14, 2016 at 04:35:46PM +0500, Valentine Sinitsyn wrote: This is a quick attempt to implement sample action at logical port level.The goal is to export IPFIX flows for logical ports, yet it is easy to extend this approach to logical switches as well. Nothing is done to provision OVS instances with required Flow_Sample_Collector_Set and IPFIX entries at this point. Does the approach I take looks sensible? If so, I can add tests and re-send this patch for in-depth review. Many thanks. Signed-off-by: Valentine Sinitsyn Sorry about the long delay in review. It's been a difficult month. No problem. This is pretty cool! The integration among OVS and OVN and IPFIX is graceful. The part that worries me is the CMS integration. Have you actually built that integration already (for which CMS)? I have two concerns. First, I'd prefer to see at least one CMS (probably OpenStack) support this at or around the time that it goes into OVN. Second, I have some skepticism around the idea that the CMS should configure the Flow_Sample_Collector_Set, etc., because OVN doesn't currently require the CMS to have any connectivity to OVSDB on each of the hypervisors and this would require the CMS to add that support. I agree that this particular bit is somewhat hacky. We plan to follow this route for an in-house CMS we build, but I doubt OpenStack community would pickup the idea. What alternatives do you see here? Having collector config at south db level doesn't seem clear either. Think I want to configure collector at 127.0.0.1:5900 - which localhost does this entry refer to? If the sample() integration looks good, CMS assumptions aside, is there a chance to merge it as a stand-alone action? That's true no publicly available CMS would use it for a while, but when they decide to, the code would already be there. And the code is not dead, as we'll be using it as well. Do you have any thoughts about supporting other monitoring technology that OVS supports (e.g. sFlow) using similar techniques? I haven't targeted any of them specifically, but it doesn't seem to be a daunting task. One only need some way to associate sample() instance and a sFlow receiver the same way collector_set_id does for IPFIX. I'd suggest to generalize Flow_Sample_Collector_Set somehow, but we agreed configuring things through this table in OVN scenario is suboptimal. Any thoughts? I hope that the long delay does not discourage you from following up. I hope to be more responsive now. Sure, I'm open for the discussion. Thanks, Valentine ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto
Hi, We're hitting an issue with an assert failing in commit_set_ipv4_action(), resulting in a core dump: 2016-11-29T09:22:48.829Z|5|util(revalidator77)|EMER|lib/odp-util.c:5208: assertion flow->nw_proto == base_flow->nw_proto && flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action() Adding a few log statements gives more info: 2016-11-29T09:22:48.829Z|1|odp_util(revalidator77)|WARN|commit_set_ipv4_action assert will fail 2016-11-29T09:22:48.829Z|2|odp_util(revalidator77)|WARN| base_flow: ip,in_port=3,dl_vlan=1,dl_vlan_pcp=0,dl_src=fa:16:3e:84:02:4a,dl_dst=00:00:5e:00:43:64,nw_src=0.0.0.0,nw_dst=0.0.0.0,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0 2016-11-29T09:22:48.829Z|3|odp_util(revalidator77)|WARN| flow: tcp,in_port=3,dl_vlan=1,dl_vlan_pcp=0,dl_src=fa:16:3e:84:02:4a,dl_dst=00:00:5e:00:43:64,nw_src=10.0.1.21,nw_dst=10.0.0.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=50962,tp_dst=8080,tcp_flags=psh|ack 2016-11-29T09:22:48.829Z|4|odp_util(revalidator77)|WARN| flow->nw_proto (6) != base_flow->nw_proto (0) As show above the assert triggers because flow->nw_proto != base_flow->nw_proto (6 != 0). The assert triggers, not on a new flow but for a flow for which traffic has been flowing for some time (tcp 50962 to 8080 in the example above). The issue can be reproduced on a lab platform, happening each time approximately 5 minutes after starting a test service (a few VMs). This is with v2.5.1, with the DKMS datapath on Ubuntu 14.04. It reproduces as well with branch-2.5 HEAD as well. Any idea ? -Thomas ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/1] Fix ovndb_servers master and VirtualIP are not on the same node
Signed-off-by: Guoshuai Li --- IntegrationGuide.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/IntegrationGuide.rst b/IntegrationGuide.rst index 8c3cca7..1a8e82c 100644 --- a/IntegrationGuide.rst +++ b/IntegrationGuide.rst @@ -259,5 +259,5 @@ with the active server. pcs constraint order VirtualIP then ovndb_servers-master -pcs constraint colocation add ovndb_servers-master with master VirtualIP \ +pcs constraint colocation add master ovndb_servers-master with VirtualIP \ score=INFINITY -- 2.10.1.windows.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-dpdk: Don't try to unregister empty vhost_id.
>
> On 29.11.2016 15:20, Ilya Maximets wrote:
> > If 'vhost-server-path' not provided for vhostuserclient port,
> > 'netdev_dpdk_vhost_destruct()' will try to unregister an empty string.
> > This leads to error message in log:
> >
> > netdev_dpdk|ERR|vhost2: Unable to unregister vhost driver for socket ''.
> >
> > CC: Ciara Loftus
> > Fixes: 2d24d165d6a5 ("netdev-dpdk: Add new 'dpdkvhostuserclient' port
> type")
> > Signed-off-by: Ilya Maximets
> > ---
> > lib/netdev-dpdk.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index b51f329..6e5cd43 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1027,6 +1027,10 @@ netdev_dpdk_vhost_destruct(struct netdev
> *netdev)
> > ovs_mutex_unlock(&dev->mutex);
> > ovs_mutex_unlock(&dpdk_mutex);
> >
> > +if (!strlen(dev->vhost_id)) {
>
> Sorry,
> s/dev->vhost_id/vhost_id/
LGTM.
Acked-by: Ciara Loftus
>
> > +goto out;
> > +}
> > +
> > if (dpdk_vhost_driver_unregister(dev, vhost_id)) {
> > VLOG_ERR("%s: Unable to unregister vhost driver for socket
> > '%s'.\n",
> > netdev->name, vhost_id);
> > @@ -1034,6 +1038,7 @@ netdev_dpdk_vhost_destruct(struct netdev
> *netdev)
> > /* OVS server mode - remove this socket from list for deletion */
> > fatal_signal_remove_file_to_unlink(vhost_id);
> > }
> > +out:
> > free(vhost_id);
> > }
> >
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Numa: Allow leading 0x on pmd-cpu-mask.
On 11/29/2016 10:24 AM, billy.o.mah...@intel.com wrote:
> From: billyom
>
> pmd-cpu-mask is interpreted as a hex bit mask. So it should be written
> with a leading 0x to indicate this. But if this is done, while the value
> is interpreted correctly and the PMDs pinned as expected, a confusing
> warning message is also issued.
>
> This patch allows but does not require a leading 0x or 0X to be used
> without a warning. Existing functionality is not affected. Relevant DPDK
> docs also updated.
works for me,
Tested-by: Kevin Traynor
>
> Signed-off-by: Billy O'Mahony
> ---
> INSTALL.DPDK-ADVANCED.rst | 13 +++--
> INSTALL.DPDK.rst |6 +++---
> lib/ovs-numa.c| 11 +--
> 3 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/INSTALL.DPDK-ADVANCED.rst b/INSTALL.DPDK-ADVANCED.rst
> index 9b2895b..1c2437d 100644
> --- a/INSTALL.DPDK-ADVANCED.rst
> +++ b/INSTALL.DPDK-ADVANCED.rst
> @@ -175,10 +175,11 @@ core shared by two logical cores, run::
> where ``N`` is the logical core number.
>
> In this example, it would show that cores ``1`` and ``21`` share the same
> -physical core., thus, the ``pmd-cpu-mask`` can be used to enable these two
> pmd
> -threads running on these two logical cores (one physical core) is::
> +physical core. As cores are counted from 0, the ``pmd-cpu-mask`` can be used
> +to enable these two pmd threads running on these two logical cores (one
> +physical core) is::
>
> -$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=12
> +$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x22
>
> Isolate Cores
> ~
> @@ -239,7 +240,7 @@ affinitized accordingly.
>By setting a bit in the mask, a pmd thread is created and pinned to the
>corresponding CPU core. e.g. to run a pmd thread on core 2::
>
> - $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4
> + $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x4
>
>.. note::
> pmd thread on a NUMA node is only created if there is at least one DPDK
> @@ -269,7 +270,7 @@ is done automatically.
> A set bit in the mask means a pmd thread is created and pinned to the
> corresponding CPU core. For example, to run pmd threads on core 1 and 2::
>
> -$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6
> +$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x6
>
> When using dpdk and dpdkvhostuser ports in a bi-directional VM loopback as
> shown below, spreading the workload over 2 or 4 pmd threads shows significant
> @@ -436,7 +437,7 @@ guide`_ to create and initialize the database, start
> ovs-vswitchd and add
> of rx queues at vhost-user interface gets automatically configured after
> virtio device connection and doesn't need manual configuration::
>
> - $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=c
> + $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xC
> $ ovs-vsctl set Interface dpdk0 options:n_rxq=2
> $ ovs-vsctl set Interface dpdk1 options:n_rxq=2
>
> diff --git a/INSTALL.DPDK.rst b/INSTALL.DPDK.rst
> index a078093..c50b7a3 100644
> --- a/INSTALL.DPDK.rst
> +++ b/INSTALL.DPDK.rst
> @@ -223,10 +223,10 @@ NUMA node 0, run::
>
> Similarly, if you wish to better scale the workloads across cores, then
> multiple pmd threads can be created and pinned to CPU cores by explicity
> -specifying ``pmd-cpu-mask``. For example, to spawn two pmd threads and pin
> -them to cores 1,2, run::
> +specifying ``pmd-cpu-mask``. Cores are numbered from 0, so to spawn two pmd
> +threads and pin them to cores 1,2, run::
>
> -$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6
> +$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x6
>
> For details on using ivshmem with DPDK, refer to `the advanced installation
> guide `__.
> diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
> index c8173e0..95a3f68 100644
> --- a/lib/ovs-numa.c
> +++ b/lib/ovs-numa.c
> @@ -535,6 +535,7 @@ ovs_numa_set_cpu_mask(const char *cmask)
> {
> int core_id = 0;
> int i;
> +int end_idx;
>
> if (!found_numa_and_core) {
> return;
> @@ -551,7 +552,13 @@ ovs_numa_set_cpu_mask(const char *cmask)
> return;
> }
>
> -for (i = strlen(cmask) - 1; i >= 0; i--) {
> +/* Ignore leading 0x. */
> +end_idx = 0;
> +if (strncmp(cmask, "0x", 2) == 0 || strncmp(cmask, "0X", 2) == 0) {
> +end_idx = 2;
> +}
> +
> +for (i = strlen(cmask) - 1; i >= end_idx; i--) {
> char hex = toupper((unsigned char)cmask[i]);
> int bin, j;
>
> @@ -575,7 +582,7 @@ ovs_numa_set_cpu_mask(const char *cmask)
> if (core_id >= hmap_count(&all_cpu_cores)) {
> return;
> }
> - }
> +}
> }
>
> /* For unspecified cores, sets 'available' to false. */
>
___
dev mailing list
d...@openvs
Re: [ovs-dev] [PATCH] netdev-dpdk: Don't try to unregister empty vhost_id.
On 29.11.2016 15:20, Ilya Maximets wrote:
> If 'vhost-server-path' not provided for vhostuserclient port,
> 'netdev_dpdk_vhost_destruct()' will try to unregister an empty string.
> This leads to error message in log:
>
> netdev_dpdk|ERR|vhost2: Unable to unregister vhost driver for socket ''.
>
> CC: Ciara Loftus
> Fixes: 2d24d165d6a5 ("netdev-dpdk: Add new 'dpdkvhostuserclient' port type")
> Signed-off-by: Ilya Maximets
> ---
> lib/netdev-dpdk.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index b51f329..6e5cd43 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1027,6 +1027,10 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
> ovs_mutex_unlock(&dev->mutex);
> ovs_mutex_unlock(&dpdk_mutex);
>
> +if (!strlen(dev->vhost_id)) {
Sorry,
s/dev->vhost_id/vhost_id/
> +goto out;
> +}
> +
> if (dpdk_vhost_driver_unregister(dev, vhost_id)) {
> VLOG_ERR("%s: Unable to unregister vhost driver for socket '%s'.\n",
> netdev->name, vhost_id);
> @@ -1034,6 +1038,7 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
> /* OVS server mode - remove this socket from list for deletion */
> fatal_signal_remove_file_to_unlink(vhost_id);
> }
> +out:
> free(vhost_id);
> }
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] netdev-dpdk: Don't try to unregister empty vhost_id.
If 'vhost-server-path' not provided for vhostuserclient port,
'netdev_dpdk_vhost_destruct()' will try to unregister an empty string.
This leads to error message in log:
netdev_dpdk|ERR|vhost2: Unable to unregister vhost driver for socket ''.
CC: Ciara Loftus
Fixes: 2d24d165d6a5 ("netdev-dpdk: Add new 'dpdkvhostuserclient' port type")
Signed-off-by: Ilya Maximets
---
lib/netdev-dpdk.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index b51f329..6e5cd43 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1027,6 +1027,10 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
ovs_mutex_unlock(&dev->mutex);
ovs_mutex_unlock(&dpdk_mutex);
+if (!strlen(dev->vhost_id)) {
+goto out;
+}
+
if (dpdk_vhost_driver_unregister(dev, vhost_id)) {
VLOG_ERR("%s: Unable to unregister vhost driver for socket '%s'.\n",
netdev->name, vhost_id);
@@ -1034,6 +1038,7 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
/* OVS server mode - remove this socket from list for deletion */
fatal_signal_remove_file_to_unlink(vhost_id);
}
+out:
free(vhost_id);
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch_v8] ovn: Add datapaths of interest filtering.
Ben Pfaff wrote on 29/11/2016 12:51:51 AM:
> I hope that this version is ready to go in. Liran, are you happy with
> this version?
>
I did some short evaluation and got the following results:
Simulate 50 hosts, each host serves 20 logical ports, each logical network
is spread over 6 of the 50 hosts.
Darrell patch:
--
Host 1 # flows 1504
Host 2 # flows 1626
Host 3 # flows 1443
...
Logical flows = 8016
Elapsed time 207,990ms
total of 1002 (in 208ms per port)
Ports created and bound in 5,648,773,788,428 cycles
1% south 69,936,025,644
1% north 77,863,484,163
97% clients 5,500,974,278,621
Conditional monitor V16 patch:
--
Host 1 # flows 1358
Host 2 # flows 1482
Host 3 # flows 1296
...
Logical flows = 8016
Elapsed time 207,998ms
total of 1002 (in 208ms per port)
Ports created and bound in 1,216,848,309,516 cycles
2% south 30,653,838,360
3% north 42,543,336,355
93% clients 1,143,651,134,801
You can see that this patch significantly degrades the overhead of
computation comparing to my origin patch (rebased version).
An obvious difference is that Darrell's patch has conditions only on the
Logical_Flow table and the origin patch did that on Port_Binding,
Logical_Flow, Multicast_Group and MAC_Binding tables.
I can resubmit my rebased patch again and Darell can base his changes on
top of it or just investigate the problem.
The code is here: https://github.com/liranschour/ovs Branch:
monitor_cond_ovn_v16
Thanks,
- Liran
> On Sun, Nov 27, 2016 at 01:08:59PM -0800, Darrell Ball wrote:
> > This patch adds datapaths of interest support where only datapaths of
> > local interest are monitored by the ovn-controller ovsdb client. The
> > idea is to do a flood fill in ovn-controller of datapath associations
> > calculated by northd. A new column is added to the SB database
> > datapath_binding table - related_datapaths to facilitate this so all
> > datapaths associations are known quickly in ovn-controller. This
> > allows monitoring to adapt quickly with a single new monitor setting
> > for all datapaths of interest locally.
> >
> > To reduce risk and minimize latency on network churn, only logical
> > flows are conditionally monitored for now. This should provide
> > the bulk of the benefit. This will be re-evaluated later to
> > possibly add additional conditional monitoring such as for
> > logical ports.
> >
> > Liran Schour contributed enhancements to the conditional
> > monitoring granularity in ovs and also submitted patches
> > for ovn usage of conditional monitoring which aided this patch
> > though sharing of concepts through code review work.
> >
> > Ben Pfaff suggested that northd could be used to pre-populate
> > related datapaths for ovn-controller to use. That idea is
> > used as part of this patch.
> >
> > CC: Ben Pfaff
> > CC: Liran Schour
> > Signed-off-by: Darrell Ball
> > ---
> >
> > v7->v8: Start wth logical flow conditional monitoring off.
> > Remove deprecated code.
> > Recover SB DB version number change.
> > Change test to be more definitive.
> >
> > v6->v7: Rebase
> >
> > v5->v6: Rebase; fix stale handling.
> >
> > v4->v5: Correct cleanup of monitors.
> > Fix warning.
> >
> > v3->v4: Refactor after incremental processing backout.
> > Limit filtering to logical flows to limit risk.
> >
> > v2->v3: Line length violation fixups :/
> >
> > v1->v2: Added logical port removal monitoring handling, factoring
> > in recent changes for incremental processing. Added some
> > comments in the code regarding initial conditions for
> > database conditional monitoring.
> >
> > ovn/controller/binding.c| 10 +++--
> > ovn/controller/lflow.c | 5 +++
> > ovn/controller/ovn-controller.c | 92
> +
> > ovn/controller/ovn-controller.h | 3 ++
> > ovn/controller/patch.c | 6 ++-
> > ovn/northd/ovn-northd.c | 76
++
> > ovn/ovn-sb.ovsschema| 11 +++--
> > ovn/ovn-sb.xml | 9
> > tests/ovn.at| 8
> > 9 files changed, 211 insertions(+), 9 deletions(-)
> >
> > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> > index d7b175e..2aff69a 100644
> > --- a/ovn/controller/binding.c
> > +++ b/ovn/controller/binding.c
> > @@ -106,7 +106,8 @@ get_local_iface_ids(const struct ovsrec_bridge
*br_int,
> >
> > static void
> > add_local_datapath(struct hmap *local_datapaths,
> > -const struct sbrec_port_binding *binding_rec)
> > +const struct sbrec_port_binding *binding_rec,
> > +const struct controller_ctx *ctx)
> > {
> > if (get_local_datapath(local_datapaths,
> > binding_rec->datapath->tunnel_key)) {
> > @@ -118,6 +119,7 @@ add_local_datapath(struct hmap *local_datapaths,
> > memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid);
> >
[ovs-dev] [PATCH] netdev-dpdk: Add vHost User PMD
The vHost PMD allows vHost User ports to be controlled by the
librte_ether API, like physical 'dpdk' ports and IVSHM 'dpdkr' ports.
This commit integrates this PMD into OVS and removes direct calls to the
librte_vhost DPDK library.
This commit requires DPDK v16.11 functionality that isn't available in
previous releases, and thus breaks compatibility with such releases.
Signed-off-by: Ciara Loftus
---
Change log:
* Rebase
* Alloc netdev->n_txq instead of max (1024) during netdev_dpdk_init
* Don't unregister unregistered callbacks.
* Move callback register to avoid need to call unregister in error case
* Detach and free pool ID if failure during client_reconfigure.
* Unregister callbacks before detach
* change vhost_pmd_id to signed int and use -1 value to indicate an
ID from the pool hasn't been alloced for it yet.
* free pool ID if rte_eth_dev_attach fails.
* check link status on destruct and print warning if the port is live.
* only free ID during destruct if it has been allocated.
* Use id-pool implementation for allocating vHost PMD IDs
* Added DPDK_DEV_VHOST_CLIENT netdev_dpdk "type"
* Reintroduced socket-id logic to correctly set client type sid
* Removed magic number & used var instead
* Remove race condition in netdev_dpdk_init by reordering code.
* Detach vHost PMD if netdev_dpdk_init fails
* Introduce "out" in client_reconfigure for when txq alloc fails
* Remove set_tx_multiq fn for vHost ports
NEWS | 2 +
lib/dpdk.c| 11 +
lib/dpdk.h| 1 +
lib/netdev-dpdk.c | 956 --
4 files changed, 373 insertions(+), 597 deletions(-)
diff --git a/NEWS b/NEWS
index 22beeb2..b979d2a 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,8 @@ Post-v2.6.0
* New option 'n_rxq_desc' and 'n_txq_desc' fields for DPDK interfaces
which set the number of rx and tx descriptors to use for the given port.
* Support for DPDK v16.11.
+ * vHost PMD integration brings vhost-user ports under control of the
+ rte_ether DPDK API.
- Fedora packaging:
* A package upgrade does not automatically restart OVS service.
- ovs-vswitchd/ovs-vsctl:
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 49a589a..dffc6f9 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -29,6 +29,7 @@
#include "dirs.h"
#include "fatal-signal.h"
+#include "id-pool.h"
#include "netdev-dpdk.h"
#include "openvswitch/dynamic-string.h"
#include "openvswitch/vlog.h"
@@ -37,6 +38,7 @@
VLOG_DEFINE_THIS_MODULE(dpdk);
static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */
+static struct id_pool *vhost_driver_ids; /* Pool of IDs for vHost PMDs */
static int
process_vhost_flags(char *flag, char *default_val, int size,
@@ -407,6 +409,9 @@ dpdk_init__(const struct smap *ovs_other_config)
}
#endif
+/* Create and initialize the pool of IDs for vHost PMD devices */
+vhost_driver_ids = id_pool_create(0, RTE_MAX_ETHPORTS - 1);
+
/* Finally, register the dpdk classes */
netdev_dpdk_register();
}
@@ -428,6 +433,12 @@ dpdk_get_vhost_sock_dir(void)
return vhost_sock_dir;
}
+struct id_pool *
+dpdk_get_vhost_id_pool(void)
+{
+return vhost_driver_ids;
+}
+
void
dpdk_set_lcore_id(unsigned cpu)
{
diff --git a/lib/dpdk.h b/lib/dpdk.h
index 673a1f1..fb85c06 100644
--- a/lib/dpdk.h
+++ b/lib/dpdk.h
@@ -35,5 +35,6 @@ struct smap;
void dpdk_init(const struct smap *ovs_other_config);
void dpdk_set_lcore_id(unsigned cpu);
const char *dpdk_get_vhost_sock_dir(void);
+struct id_pool *dpdk_get_vhost_id_pool(void);
#endif /* dpdk.h */
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index b51f329..ede257f 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -38,6 +39,7 @@
#include "dpdk.h"
#include "dpif-netdev.h"
#include "fatal-signal.h"
+#include "id-pool.h"
#include "netdev-provider.h"
#include "netdev-vport.h"
#include "odp-util.h"
@@ -122,6 +124,7 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
#define XSTAT_RX_BROADCAST_PACKETS "rx_broadcast_packets"
#define XSTAT_TX_BROADCAST_PACKETS "tx_broadcast_packets"
#define XSTAT_RX_UNDERSIZED_ERRORS "rx_undersized_errors"
+#define XSTAT_RX_UNDERSIZE_PACKETS "rx_undersize_packets"
#define XSTAT_RX_OVERSIZE_ERRORS "rx_oversize_errors"
#define XSTAT_RX_FRAGMENTED_ERRORS "rx_fragmented_errors"
#define XSTAT_RX_JABBER_ERRORS "rx_jabber_errors"
@@ -171,6 +174,7 @@ enum { DRAIN_TSC = 20ULL };
enum dpdk_dev_type {
DPDK_DEV_ETH = 0,
DPDK_DEV_VHOST = 1,
+DPDK_DEV_VHOST_CLIENT = 2,
};
/* Quality of Service */
@@ -343,15 +347,12 @@ struct netdev_dpdk {
struct rte_eth_link link;
int link_reset_cnt;
-/* virtio identifier for vhost devices */
-ovsrcu_index vid;
-
-/* True if vHost device is 'up' and has been reconfigured at least once */
-
Re: [ovs-dev] [PATCH] Numa: Allow leading 0x on pmd-cpu-mask.
> -Original Message- > From: Ben Pfaff [mailto:b...@ovn.org] > Sent: Monday, November 28, 2016 6:10 PM > To: O Mahony, Billy > Cc: ovs-dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] Numa: Allow leading 0x on pmd-cpu-mask. > > On Mon, Nov 28, 2016 at 05:06:53PM +, billy.o.mah...@intel.com > wrote: > > From: billyom > > > > pmd-cpu-mask is interpreted as a hex bit mask. So it should be written > > with a leading 0x to indicate this. But if this is done, while the > > value is interpreted correctly and the PMDs pinned as expected, a > > confusing warning message is also issued. > > > > This patch allows but does not require a leading 0x or 0X to be used > > without a warning. Existing functionality is not affected. Relevant > > DPDK docs also updated. > > > > Signed-off-by: Billy O'Mahony > > This is a whitespace-only change to the C code in OVS, so it's hard to believe > that it does what it says. [[BO'M]] Any sufficiently advanced technology is indistinguishable from magic? :) Apologies, v2 with actual code to follow. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] Numa: Allow leading 0x on pmd-cpu-mask.
From: billyom
pmd-cpu-mask is interpreted as a hex bit mask. So it should be written
with a leading 0x to indicate this. But if this is done, while the value
is interpreted correctly and the PMDs pinned as expected, a confusing
warning message is also issued.
This patch allows but does not require a leading 0x or 0X to be used
without a warning. Existing functionality is not affected. Relevant DPDK
docs also updated.
Signed-off-by: Billy O'Mahony
---
INSTALL.DPDK-ADVANCED.rst | 13 +++--
INSTALL.DPDK.rst |6 +++---
lib/ovs-numa.c| 11 +--
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/INSTALL.DPDK-ADVANCED.rst b/INSTALL.DPDK-ADVANCED.rst
index 9b2895b..1c2437d 100644
--- a/INSTALL.DPDK-ADVANCED.rst
+++ b/INSTALL.DPDK-ADVANCED.rst
@@ -175,10 +175,11 @@ core shared by two logical cores, run::
where ``N`` is the logical core number.
In this example, it would show that cores ``1`` and ``21`` share the same
-physical core., thus, the ``pmd-cpu-mask`` can be used to enable these two pmd
-threads running on these two logical cores (one physical core) is::
+physical core. As cores are counted from 0, the ``pmd-cpu-mask`` can be used
+to enable these two pmd threads running on these two logical cores (one
+physical core) is::
-$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=12
+$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x22
Isolate Cores
~
@@ -239,7 +240,7 @@ affinitized accordingly.
By setting a bit in the mask, a pmd thread is created and pinned to the
corresponding CPU core. e.g. to run a pmd thread on core 2::
- $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4
+ $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x4
.. note::
pmd thread on a NUMA node is only created if there is at least one DPDK
@@ -269,7 +270,7 @@ is done automatically.
A set bit in the mask means a pmd thread is created and pinned to the
corresponding CPU core. For example, to run pmd threads on core 1 and 2::
-$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6
+$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x6
When using dpdk and dpdkvhostuser ports in a bi-directional VM loopback as
shown below, spreading the workload over 2 or 4 pmd threads shows significant
@@ -436,7 +437,7 @@ guide`_ to create and initialize the database, start
ovs-vswitchd and add
of rx queues at vhost-user interface gets automatically configured after
virtio device connection and doesn't need manual configuration::
- $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=c
+ $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xC
$ ovs-vsctl set Interface dpdk0 options:n_rxq=2
$ ovs-vsctl set Interface dpdk1 options:n_rxq=2
diff --git a/INSTALL.DPDK.rst b/INSTALL.DPDK.rst
index a078093..c50b7a3 100644
--- a/INSTALL.DPDK.rst
+++ b/INSTALL.DPDK.rst
@@ -223,10 +223,10 @@ NUMA node 0, run::
Similarly, if you wish to better scale the workloads across cores, then
multiple pmd threads can be created and pinned to CPU cores by explicity
-specifying ``pmd-cpu-mask``. For example, to spawn two pmd threads and pin
-them to cores 1,2, run::
+specifying ``pmd-cpu-mask``. Cores are numbered from 0, so to spawn two pmd
+threads and pin them to cores 1,2, run::
-$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=6
+$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x6
For details on using ivshmem with DPDK, refer to `the advanced installation
guide `__.
diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index c8173e0..95a3f68 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -535,6 +535,7 @@ ovs_numa_set_cpu_mask(const char *cmask)
{
int core_id = 0;
int i;
+int end_idx;
if (!found_numa_and_core) {
return;
@@ -551,7 +552,13 @@ ovs_numa_set_cpu_mask(const char *cmask)
return;
}
-for (i = strlen(cmask) - 1; i >= 0; i--) {
+/* Ignore leading 0x. */
+end_idx = 0;
+if (strncmp(cmask, "0x", 2) == 0 || strncmp(cmask, "0X", 2) == 0) {
+end_idx = 2;
+}
+
+for (i = strlen(cmask) - 1; i >= end_idx; i--) {
char hex = toupper((unsigned char)cmask[i]);
int bin, j;
@@ -575,7 +582,7 @@ ovs_numa_set_cpu_mask(const char *cmask)
if (core_id >= hmap_count(&all_cpu_cores)) {
return;
}
- }
+}
}
/* For unspecified cores, sets 'available' to false. */
--
1.7.0.7
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 3/3 v6] Windows: document multiple NIC support setup
On Mon, 2016-11-28 at 16:36 +, Alin Serdean wrote: > This patch updates the documentation on how to set up OVS with > multiple > NICs. > > Also update the documentation to show users how new internal ports > are > created > > Signed-off-by: Alin Gabriel Serdean > Acked-by: Paul Boca Can't speak to the correctness of the content itself, but there are some nits that should be addressed when merged/respun. Stephen > --- > INSTALL.Windows.rst | 138 ++-- > > 1 file changed, 101 insertions(+), 37 deletions(-) > > diff --git a/INSTALL.Windows.rst b/INSTALL.Windows.rst > index 251cd10..31d5b82 100644 > --- a/INSTALL.Windows.rst > +++ b/INSTALL.Windows.rst > @@ -137,7 +137,8 @@ configure options to choose the right compiler, > linker, libraries, Open vSwitch > component installation directories, etc. For example:: > > > ./configure CC=./build-aux/cccl LD="$(which link)" \ > -LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \ > +--prefix="C:/openvswitch/usr" \ > --localstatedir="C:/openvswitch/var" \ > --sysconfdir="C:/openvswitch/etc" \ > --with-pthread="C:/pthread" > @@ -149,16 +150,18 @@ component installation directories, etc. For > example:: > To configure with SSL support, add the requisite additional > options:: > > > ./configure CC=./build-aux/cccl LD="`which link`" \ > -LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > - --localstatedir="C:/openvswitch/var" > - --sysconfdir="C:/openvswitch/etc" \ > - --with-pthread="C:/pthread" \ > - --enable-ssl --with-openssl="C:/OpenSSL-Win32" > +LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \ > +--prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" > +--sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" \ > +--enable-ssl --with-openssl="C:/OpenSSL-Win32" > > Finally, to the kernel module also:: > > > ./configure CC=./build-aux/cccl LD="`which link`" \ > -LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \ > +--prefix="C:/openvswitch/usr" \ > --localstatedir="C:/openvswitch/var" \ > --sysconfdir="C:/openvswitch/etc" \ > --with-pthread="C:/pthread" \ > @@ -253,8 +256,7 @@ to work (covered later). > The command to create a new switch named 'OVS-Extended-Switch' using > a physical > NIC named 'Ethernet 1' is:: > > -PS > New-VMSwitch "OVS-Extended-Switch" -AllowManagementOS $true > \ > --NetAdapterName "Ethernet 1" > +PS > New-VMSwitch "OVS-Extended-Switch" -NetAdapterName > "Ethernet 1" > > .. note:: > You can obtain the list of physical NICs on the host using 'Get- > NetAdapter' > @@ -378,23 +380,22 @@ the adapter named ``Ethernet0``, then in OVS we > use that name (``Ethernet0``) > as a special name to refer to that adapter. > > .. note:: > - we assume that the Hyper-V switch on which OVS extension is > enabled has a > - single physical NIC connected to it. > + we assume that the Hyper-V switch on which OVS extension is > enabled. This note doesn't really make any sense any more. Is this intentional? > -An internal port is the virtual adapter created on the Hyper-V > switch using the > -``AllowManagementOS`` setting. This has already been setup while > creating the > -switch using the instructions above. In OVS for Hyper-V, we use a > the name of > -that specific adapter as a special name to refer to that adapter. By > default it > -is created under the following rule ``vEthernet ( switch>)``. > +Internal ports are the virtual adapters created on the Hyper-V > switch using the > +ovs-vsctl add-br command. By default they are created under ``ovs-vsctl add-br `` command > the > +following rule "" and the adapters are disabled. One > needs to > +enable them and set the corresponding values to it to make them IP- > able. > > As a whole example, if we issue the following in a powershell > console:: > > -PS C:\package\binaries> Get-NetAdapter | select > Name,MacAddress,InterfaceDescription > -Name MacAddress InterfaceDescription > - -- > -Ethernet1 00-0C-29-94-05-65 Intel(R) PRO/1000 MT > Network Connection > -vEthernet (external) 00-0C-29-94-05-5B Hyper-V Virtual > Ethernet Adapter #2 > -Ethernet0 00-0C-29-94-05-5B Intel(R) PRO/1000 MT > Network Connection #2 > +PS C:\package\binaries> Get-NetAdapter | select > Name,InterfaceDescription > +Name InterfaceDescription > + > +Ethernet1 Intel(R) PRO/1000 MT Network Connection > +br-pif
Re: [ovs-dev] [PATCH v12 rebased 1/3] userspace: add support for pop_eth and push_eth actions
On Mon, 28 Nov 2016 16:56:27 -0800, Ben Pfaff wrote: > I know that this isn't intended to be applied yet, according to the > cover letter, but it doesn't build: > > ../lib/odp-util.c:5504:15: error: no member named 'base_layer' in 'struct > flow' > ../lib/odp-util.c:5504:40: error: no member named 'base_layer' in 'struct > flow' > ../lib/odp-util.c:5505:19: error: no member named 'base_layer' in 'struct > flow' > ../lib/odp-util.c:5505:33: error: use of undeclared identifier 'LAYER_2' > ../lib/odp-util.c:5512:20: error: no member named 'base_layer' in 'struct > flow' > ../lib/odp-util.c:5512:40: error: no member named 'base_layer' in 'struct > flow' > > and furthermore I can't any mention of base_layer in the OVS history so > I'm not sure what's going on here. It's in patch 2, seems that the order is mixed up. Sorry for that. Please apply all three patches together for testing. We'll be taking a different approach with this patchset, implementing EXT-382 instead. Thanks, Jiri ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
