Re: [ovs-dev] [PATCH v2.56] datapath: Add basic MPLS support to kernel
On Fri, Apr 25, 2014 at 12:57:20PM -0700, Jesse Gross wrote: > On Fri, Apr 25, 2014 at 1:06 AM, YAMAMOTO Takashi > wrote: > >> On Thu, Apr 24, 2014 at 05:57:29PM +0900, YAMAMOTO Takashi wrote: > >>> hi, > >>> > >>> > + * Due to the sample action there may be multiple possible eth types. > >>> > + * In order to correctly validate actions all possible types are > >>> > tracked > >>> > + * and verified. This is done using struct eth_types. > >>> > >>> is there any real-world use cases of these actions inside a sample? > >>> otherwise, how about just rejecting such combinations? > >>> it doesn't seem to worth the code complexity to me. > >>> (sorry if it has been already discussed. it's the first time for me > >>> to seriously read this long-lived patch.) > >> > >> Good point, the code is rather complex. > >> > >> My understanding is that it comes into effect in the case > >> of sflow or ipfix being configured on the bridge. I tend > >> to think these are real-world use-cases, though that thinking > >> is by no means fixed. > >> > >> My reading of the code is that in the case of sflow and ipfix a single > >> sample rule appears at the beginning of the flow. And that it may be > >> possible to replace the code that you are referring to with something > >> simpler to handle these cases. > > > > it seems that they put only a userland action inside a sample. > > it's what i expected from its name "sample". > > Yes, that's the only current use case. In theory, this could be used > more generally although nothing has come up yet. > > In retrospect, I regret the design of the sample action - not the part > that allows it to handle different actions but the fact that the > results can affect things outside of the sample action list. I think > that if we had made it more like a subroutine then that would have > retained all of the functionality but none of the complexity here. > Perhaps if we can find a clean way to restructure it without breaking > compatibility then that would simplify the validation here. I have not thought deeply about this but it seems to me that it should be easy enough to provide compatibility for a new user-space to work with both new and old datapaths. But it is not clear to me how to achieve the reverse: allowing a new datapath to work with both new and old user-spaces. I assume that we care about such compatibility. > >> My understanding is that the code you are referring to also comes into > >> effect when a sample action (a Nicira extension) is used directly in a > >> rule. I am less sure that this is a real-world case but the complex logic > >> you are referring to should to handle this use-case. > > > > probably nicira folks can clarify? > > It's the same set of use cases, just extending it to OpenFlow to > enable building sampling into different situations. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] netlink statistics
Hi All, I want to print the netlink statistics of vswitchd process. My requirement is to get all the netlink count for flow additions from vswitchd process. Is there any way to get the netlink statistics? Please help me to get these statistics. -- Thanks & Regards Neelakantam Gaddam ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] Rapid Spanning Protocol Implementation (IEEE 802.1D) + functional tests
Jarno Rajahalme wrote:
move_rstp() does not check ‘changes’ before clearing it. Consider enforcing the
invariant,
or maybe do not bother setting ‘changes’ right before calling ‘move_rstp()’ ?
Not clear to me. Function move_rstp does check 'changes':
move_rstp(struct rstp *rstp )
{
int port_no, num_iterations;
num_iterations = 0;
while (rstp->changes == true && num_iterations < MAX_RSTP_ITERATIONS) {
[...]
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Ich bitte um Ihre Unterstuetzung!!
Sehr geehrter Damen und Herren, Vorname/ Martins Name/ U.Ezoh Nationalitaet/ Sierra Leonean Ich habe die Summe von USD 7.2millionen von meinen verstorbenen Eltern geebrt. Meine Eltern wurden Diamanten Haendler. Das Geld wurde von meinen Eltern in der Sicherheitsfirma fuer sicheres Behalten deponiert. Momentan lebe ich in Bouake in Elfenbeinkueste und zwar gibt es Buergerkrieg bei uns, deswegen moechte ich das Geld in Ihrem Land per Ihre Hilfe investieren. Ich moechte in den gunstigen Branchen investieren wenn Sie mich unterstuetzen koennen. Koennen Sie mich unterstuetzen? Ich warte auf Ihre baldige Antwort! Mit freundlichen Gruessen, Frue Ezoh U. Martins___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] Rapid Spanning Protocol Implementation (IEEE 802.1D) + functional tests
On Apr 28, 2014, at 2:09 AM, Martino Fornasa wrote:
> Jarno Rajahalme wrote:
>
>> move_rstp() does not check ‘changes’ before clearing it. Consider enforcing
>> the invariant,
>> or maybe do not bother setting ‘changes’ right before calling ‘move_rstp()’ ?
>>
> Not clear to me. Function move_rstp does check 'changes':
>
I missed that, sorry.
Jarno
>> move_rstp(struct rstp *rstp )
>> {
>> int port_no, num_iterations;
>> num_iterations = 0;
>>
>> while (rstp->changes == true && num_iterations < MAX_RSTP_ITERATIONS) {
>> [...]
>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [urcu-fixes 2/4] ovs-thread: Quiesce in xpthread_barrier_wait().
Otherwise the udpif revalidator threads can postpone RCU callbacks
essentially forever, especially if there are many revalidator threads and
little network traffic.
Reported-by: Alex Wang
Signed-off-by: Ben Pfaff
---
lib/ovs-thread.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 3ca686f..f33f5f1 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -240,7 +240,10 @@ xpthread_barrier_wait(pthread_barrier_t *barrier)
{
int error;
+ovsrcu_quiesce_start();
error = pthread_barrier_wait(barrier);
+ovsrcu_quiesce_end();
+
if (error && OVS_UNLIKELY(error != PTHREAD_BARRIER_SERIAL_THREAD)) {
ovs_abort(error, "pthread_barrier_wait failed");
}
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [urcu-fixes 0/4] Fix busy wait in RCU postpone thread
Alex Wang pointed out that the RCU postpone thread can end up busy-waiting. The first two patches in this series fix the problem. The remaining two patches make it easier to track down threads that never quiesce. Ben Pfaff (4): timeval: Preserve quiescence across time_poll(). ovs-thread: Quiesce in xpthread_barrier_wait(). ovs-thread: Make caller provide thread name when creating a thread. ovs-rcu: Log a helpful warning when ovsrcu_synchronize() stalls. lib/dpif-netdev.c |6 +- lib/netdev-dpdk.c |4 +--- lib/ovs-rcu.c | 34 +++--- lib/ovs-rcu.h |1 + lib/ovs-thread.c | 20 +++- lib/ovs-thread.h |2 +- lib/timeval.c | 14 +- ofproto/ofproto-dpif-monitor.c |3 +-- ofproto/ofproto-dpif-upcall.c | 26 -- vswitchd/system-stats.c|3 ++- 10 files changed, 70 insertions(+), 43 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [urcu-fixes 1/4] timeval: Preserve quiescence across time_poll().
Otherwise ovsrcu_synchronize() busy-waits in its loop because its
poll_block() un-quiesces, causing the global_seqno to increase, which is
what it waits for.
Reported-by: Alex Wang
Signed-off-by: Ben Pfaff
---
lib/ovs-rcu.c |7 +++
lib/ovs-rcu.h |1 +
lib/timeval.c | 14 +-
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index 269f51b..b3c434d 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -134,6 +134,13 @@ ovsrcu_quiesce(void)
ovsrcu_quiesced();
}
+bool
+ovsrcu_is_quiescent(void)
+{
+ovsrcu_init();
+return pthread_getspecific(perthread_key) == NULL;
+}
+
static void
ovsrcu_synchronize(void)
{
diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index 710870a..4b451b2 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -178,5 +178,6 @@ void ovsrcu_postpone__(void (*function)(void *aux), void
*aux);
void ovsrcu_quiesce_start(void);
void ovsrcu_quiesce_end(void);
void ovsrcu_quiesce(void);
+bool ovsrcu_is_quiescent(void);
#endif /* ovs-rcu.h */
diff --git a/lib/timeval.c b/lib/timeval.c
index ebbdb98..d2a4380 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -263,6 +263,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE
*handles OVS_UNUSED,
{
long long int *last_wakeup = last_wakeup_get();
long long int start;
+bool quiescent;
int retval = 0;
time_init();
@@ -274,6 +275,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE
*handles OVS_UNUSED,
start = time_msec();
timeout_when = MIN(timeout_when, deadline);
+quiescent = ovsrcu_is_quiescent();
for (;;) {
long long int now = time_msec();
@@ -287,10 +289,12 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE
*handles OVS_UNUSED,
time_left = timeout_when - now;
}
-if (!time_left) {
-ovsrcu_quiesce();
-} else {
-ovsrcu_quiesce_start();
+if (!quiescent) {
+if (!time_left) {
+ovsrcu_quiesce();
+} else {
+ovsrcu_quiesce_start();
+}
}
#ifndef _WIN32
@@ -313,7 +317,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE
*handles OVS_UNUSED,
}
#endif
-if (time_left) {
+if (!quiescent && time_left) {
ovsrcu_quiesce_end();
}
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [urcu-fixes 3/4] ovs-thread: Make caller provide thread name when creating a thread.
Thread names are occasionally very useful for debugging, but from time to
time we've forgotten to set one. This commit adds the new thread's name
as a parameter to the function to start a thread, to make that mistake
impossible. This also simplifies code, since two function calls become
only one.
This makes a few other changes to the thread creation function:
* Since it is no longer a direct wrapper around a pthread function,
rename it to avoid giving that impression.
* Remove 'pthread_attr_t *' param that every caller supplied as NULL.
* Change 'pthread *' parameter into a return value, for convenience.
The system-stats code hadn't set a thread name, so this fixes that issue.
This patch is a prerequisite for making RCU report the name of a thread
that is blocking RCU synchronization, because the easiest way to do that
gets the name of the thread in ovsrcu_quiesce_end(), which is called before
the thread function is called (so it won't get a name set within the thread
function itself).
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c |6 +-
lib/netdev-dpdk.c |4 +---
lib/ovs-rcu.c |3 +--
lib/ovs-thread.c | 15 ++-
lib/ovs-thread.h |2 +-
ofproto/ofproto-dpif-monitor.c |3 +--
ofproto/ofproto-dpif-upcall.c | 26 --
vswitchd/system-stats.c|3 ++-
8 files changed, 25 insertions(+), 37 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 07f181d..96c5feb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -323,7 +323,6 @@ struct pmd_thread {
pthread_t thread;
int id;
atomic_uint change_seq;
-char *name;
};
/* Interface to netdev-based datapath. */
@@ -1877,8 +1876,6 @@ pmd_thread_main(void *f_)
int poll_cnt;
int i;
-f->name = xasprintf("pmd_%u", ovsthread_id_self());
-set_subprogram_name("%s", f->name);
poll_cnt = 0;
poll_list = NULL;
@@ -1918,7 +1915,6 @@ reload:
}
free(poll_list);
-free(f->name);
return NULL;
}
@@ -1955,7 +1951,7 @@ dp_netdev_set_pmd_threads(struct dp_netdev *dp, int n)
/* Each thread will distribute all devices rx-queues among
* themselves. */
-xpthread_create(&f->thread, NULL, pmd_thread_main, f);
+f->thread = ovs_thread_create("pmd", pmd_thread_main, f);
}
}
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d9676ed..fd991ab 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -133,8 +133,6 @@ static struct list dpdk_list OVS_GUARDED_BY(dpdk_mutex)
static struct list dpdk_mp_list OVS_GUARDED_BY(dpdk_mutex)
= LIST_INITIALIZER(&dpdk_mp_list);
-static pthread_t watchdog_thread;
-
struct dpdk_mp {
struct rte_mempool *mp;
int mtu;
@@ -1105,7 +1103,7 @@ dpdk_class_init(void)
"[netdev] up|down", 1, 2,
netdev_dpdk_set_admin_state, NULL);
-xpthread_create(&watchdog_thread, NULL, dpdk_watchdog, NULL);
+ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
return 0;
}
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index b3c434d..c1ac61a 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -99,7 +99,7 @@ ovsrcu_quiesced(void)
} else {
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
if (ovsthread_once_start(&once)) {
-xpthread_create(NULL, NULL, ovsrcu_postpone_thread, NULL);
+ovs_thread_create("urcu", ovsrcu_postpone_thread, NULL);
ovsthread_once_done(&once);
}
}
@@ -234,7 +234,6 @@ ovsrcu_call_postponed(void)
static void *
ovsrcu_postpone_thread(void *arg OVS_UNUSED)
{
-set_subprogram_name("urcu");
pthread_detach(pthread_self());
for (;;) {
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index f33f5f1..d835b39 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -256,6 +256,7 @@ DEFINE_EXTERN_PER_THREAD_DATA(ovsthread_id, 0);
struct ovsthread_aux {
void *(*start)(void *);
void *arg;
+char name[16];
};
static void *
@@ -273,13 +274,16 @@ ovsthread_wrapper(void *aux_)
aux = *auxp;
free(auxp);
+set_subprogram_name("%s%u", aux.name, id);
+
ovsrcu_quiesce_end();
return aux.start(aux.arg);
}
-void
-xpthread_create(pthread_t *threadp, pthread_attr_t *attr,
-void *(*start)(void *), void *arg)
+/* Starts a thread that calls 'start(arg)'. Sets the thread's name to 'name'
+ * (suffixed by its ovsthread_id()). Returns the new thread's pthread_t. */
+pthread_t
+ovs_thread_create(const char *name, void *(*start)(void *), void *arg)
{
struct ovsthread_aux *aux;
pthread_t thread;
@@ -292,12 +296,13 @@ xpthread_create(pthread_t *threadp, pthread_attr_t *attr,
aux = xmalloc(sizeof *aux);
aux->start = start;
aux->arg = arg;
+ovs_strlcpy(aux->name, name, sizeof aux->name);
-error = pthrea
[ovs-dev] [urcu-fixes 4/4] ovs-rcu: Log a helpful warning when ovsrcu_synchronize() stalls.
This made it easier for me to find a thread that was causing stalls.
Signed-off-by: Ben Pfaff
---
lib/ovs-rcu.c| 24 +++-
lib/ovs-thread.c |4 +++-
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index c1ac61a..8a12564 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -21,6 +21,10 @@
#include "ovs-thread.h"
#include "poll-loop.h"
#include "seq.h"
+#include "timeval.h"
+#include "vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(ovs_rcu);
struct ovsrcu_cb {
void (*function)(void *aux);
@@ -34,11 +38,12 @@ struct ovsrcu_cbset {
};
struct ovsrcu_perthread {
-struct list list_node; /* In global list. */
+struct list list_node; /* In global list. */
struct ovs_mutex mutex;
uint64_t seqno;
struct ovsrcu_cbset *cbset;
+char name[16]; /* This thread's name. */
};
static struct seq *global_seqno;
@@ -70,6 +75,8 @@ ovsrcu_perthread_get(void)
ovs_mutex_init(&perthread->mutex);
perthread->seqno = seq_read(global_seqno);
perthread->cbset = NULL;
+ovs_strlcpy(perthread->name, get_subprogram_name(),
+sizeof perthread->name);
ovs_mutex_lock(&ovsrcu_threads_mutex);
list_push_back(&ovsrcu_threads, &perthread->list_node);
@@ -144,7 +151,9 @@ ovsrcu_is_quiescent(void)
static void
ovsrcu_synchronize(void)
{
+unsigned int warning_threshold = 1000;
uint64_t target_seqno;
+long long int start;
if (single_threaded()) {
return;
@@ -152,15 +161,20 @@ ovsrcu_synchronize(void)
target_seqno = seq_read(global_seqno);
ovsrcu_quiesce_start();
+start = time_msec();
for (;;) {
uint64_t cur_seqno = seq_read(global_seqno);
struct ovsrcu_perthread *perthread;
+char stalled_thread[16];
+unsigned int elapsed;
bool done = true;
ovs_mutex_lock(&ovsrcu_threads_mutex);
LIST_FOR_EACH (perthread, list_node, &ovsrcu_threads) {
if (perthread->seqno <= target_seqno) {
+ovs_strlcpy(stalled_thread, perthread->name,
+sizeof stalled_thread);
done = false;
break;
}
@@ -171,6 +185,14 @@ ovsrcu_synchronize(void)
break;
}
+elapsed = time_msec() - start;
+if (elapsed >= warning_threshold) {
+VLOG_WARN("blocked %u ms waiting for %s to quiesce",
+ elapsed, stalled_thread);
+warning_threshold *= 2;
+}
+poll_timer_wait_until(start + warning_threshold);
+
seq_wait(global_seqno, cur_seqno);
poll_block();
}
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index d835b39..19edf8f 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -274,9 +274,11 @@ ovsthread_wrapper(void *aux_)
aux = *auxp;
free(auxp);
+/* The order of the following calls is important, because
+ * ovsrcu_quiesce_end() saves a copy of the thread name. */
set_subprogram_name("%s%u", aux.name, id);
-
ovsrcu_quiesce_end();
+
return aux.start(aux.arg);
}
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 1/1] Python Logging Formatting Improvements
On Sun, Mar 30, 2014 at 12:26:55PM +0100, Dave Tucker wrote: > The Open vSwitch daemons written in C support user-configured logging > patterns as described in ovs-appctl(8). This commit adds this capability > to the daemons written in Python. > > - Add a '__log_patterns' attribute to the Vlog class > - Populate this using the default patterns in ovs-appctl(8) > - Add a '__start_time' attribute to the Vlog class to support '%r' > - Update the '_log' method to build the log message according to the > pattern > - Add a 'set_pattern' method to allow the default patterns to be changed > - Update 'set_levels_from_string' to support setting the pattern from a > string > > Signed-off-by: Dave Tucker Applied, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [urcu-fixes 1/4] timeval: Preserve quiescence across time_poll().
This is a hard to notice one~,
Acked-by: Alex Wang
On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff wrote:
> Otherwise ovsrcu_synchronize() busy-waits in its loop because its
> poll_block() un-quiesces, causing the global_seqno to increase, which is
> what it waits for.
>
> Reported-by: Alex Wang
> Signed-off-by: Ben Pfaff
> ---
> lib/ovs-rcu.c |7 +++
> lib/ovs-rcu.h |1 +
> lib/timeval.c | 14 +-
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> index 269f51b..b3c434d 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -134,6 +134,13 @@ ovsrcu_quiesce(void)
> ovsrcu_quiesced();
> }
>
> +bool
> +ovsrcu_is_quiescent(void)
> +{
> +ovsrcu_init();
> +return pthread_getspecific(perthread_key) == NULL;
> +}
> +
> static void
> ovsrcu_synchronize(void)
> {
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index 710870a..4b451b2 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -178,5 +178,6 @@ void ovsrcu_postpone__(void (*function)(void *aux),
> void *aux);
> void ovsrcu_quiesce_start(void);
> void ovsrcu_quiesce_end(void);
> void ovsrcu_quiesce(void);
> +bool ovsrcu_is_quiescent(void);
>
> #endif /* ovs-rcu.h */
> diff --git a/lib/timeval.c b/lib/timeval.c
> index ebbdb98..d2a4380 100644
> --- a/lib/timeval.c
> +++ b/lib/timeval.c
> @@ -263,6 +263,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds,
> HANDLE *handles OVS_UNUSED,
> {
> long long int *last_wakeup = last_wakeup_get();
> long long int start;
> +bool quiescent;
> int retval = 0;
>
> time_init();
> @@ -274,6 +275,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds,
> HANDLE *handles OVS_UNUSED,
> start = time_msec();
>
> timeout_when = MIN(timeout_when, deadline);
> +quiescent = ovsrcu_is_quiescent();
>
> for (;;) {
> long long int now = time_msec();
> @@ -287,10 +289,12 @@ time_poll(struct pollfd *pollfds, int n_pollfds,
> HANDLE *handles OVS_UNUSED,
> time_left = timeout_when - now;
> }
>
> -if (!time_left) {
> -ovsrcu_quiesce();
> -} else {
> -ovsrcu_quiesce_start();
> +if (!quiescent) {
> +if (!time_left) {
> +ovsrcu_quiesce();
> +} else {
> +ovsrcu_quiesce_start();
> +}
> }
>
> #ifndef _WIN32
> @@ -313,7 +317,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds,
> HANDLE *handles OVS_UNUSED,
> }
> #endif
>
> -if (time_left) {
> +if (!quiescent && time_left) {
> ovsrcu_quiesce_end();
> }
>
> --
> 1.7.10.4
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [urcu-fixes 2/4] ovs-thread: Quiesce in xpthread_barrier_wait().
thx for fixing this,
Acked-by: Alex Wang
On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff wrote:
> Otherwise the udpif revalidator threads can postpone RCU callbacks
> essentially forever, especially if there are many revalidator threads and
> little network traffic.
>
> Reported-by: Alex Wang
> Signed-off-by: Ben Pfaff
> ---
> lib/ovs-thread.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index 3ca686f..f33f5f1 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -240,7 +240,10 @@ xpthread_barrier_wait(pthread_barrier_t *barrier)
> {
> int error;
>
> +ovsrcu_quiesce_start();
> error = pthread_barrier_wait(barrier);
> +ovsrcu_quiesce_end();
> +
> if (error && OVS_UNLIKELY(error != PTHREAD_BARRIER_SERIAL_THREAD)) {
> ovs_abort(error, "pthread_barrier_wait failed");
> }
> --
> 1.7.10.4
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [urcu-fixes 3/4] ovs-thread: Make caller provide thread name when creating a thread.
This is helpful, just two comments,
On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff wrote:
> This patch is a prerequisite for making RCU report the name of a thread
> that is blocking RCU synchronization,
> *because the easiest way to do that gets the name of the thread in
> ovsrcu_quiesce_end(), *which is called before
> the thread function is called (so it won't get a name set within the thread
> function itself).
>
Just curious, should we change the sentence in bold to "to do that is to
get ..."
do we still need the ovsrcu_quiesce_end() here?
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -293,7 +293,6 @@ ovs_thread_create(const char *name, void *(*start)(void
*), void *arg)
forbid_forking("multiple threads exist");
multithreaded = true;
-ovsrcu_quiesce_end();
aux = xmalloc(sizeof *aux);
aux->start = start;
Acked-by: Alex Wang
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [urcu-fixes 4/4] ovs-rcu: Log a helpful warning when ovsrcu_synchronize() stalls.
Acked-by: Alex Wang
On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff wrote:
> This made it easier for me to find a thread that was causing stalls.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovs-rcu.c| 24 +++-
> lib/ovs-thread.c |4 +++-
> 2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> index c1ac61a..8a12564 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -21,6 +21,10 @@
> #include "ovs-thread.h"
> #include "poll-loop.h"
> #include "seq.h"
> +#include "timeval.h"
> +#include "vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(ovs_rcu);
>
> struct ovsrcu_cb {
> void (*function)(void *aux);
> @@ -34,11 +38,12 @@ struct ovsrcu_cbset {
> };
>
> struct ovsrcu_perthread {
> -struct list list_node; /* In global list. */
> +struct list list_node; /* In global list. */
>
> struct ovs_mutex mutex;
> uint64_t seqno;
> struct ovsrcu_cbset *cbset;
> +char name[16]; /* This thread's name. */
> };
>
> static struct seq *global_seqno;
> @@ -70,6 +75,8 @@ ovsrcu_perthread_get(void)
> ovs_mutex_init(&perthread->mutex);
> perthread->seqno = seq_read(global_seqno);
> perthread->cbset = NULL;
> +ovs_strlcpy(perthread->name, get_subprogram_name(),
> +sizeof perthread->name);
>
> ovs_mutex_lock(&ovsrcu_threads_mutex);
> list_push_back(&ovsrcu_threads, &perthread->list_node);
> @@ -144,7 +151,9 @@ ovsrcu_is_quiescent(void)
> static void
> ovsrcu_synchronize(void)
> {
> +unsigned int warning_threshold = 1000;
> uint64_t target_seqno;
> +long long int start;
>
> if (single_threaded()) {
> return;
> @@ -152,15 +161,20 @@ ovsrcu_synchronize(void)
>
> target_seqno = seq_read(global_seqno);
> ovsrcu_quiesce_start();
> +start = time_msec();
>
> for (;;) {
> uint64_t cur_seqno = seq_read(global_seqno);
> struct ovsrcu_perthread *perthread;
> +char stalled_thread[16];
> +unsigned int elapsed;
> bool done = true;
>
> ovs_mutex_lock(&ovsrcu_threads_mutex);
> LIST_FOR_EACH (perthread, list_node, &ovsrcu_threads) {
> if (perthread->seqno <= target_seqno) {
> +ovs_strlcpy(stalled_thread, perthread->name,
> +sizeof stalled_thread);
> done = false;
> break;
> }
> @@ -171,6 +185,14 @@ ovsrcu_synchronize(void)
> break;
> }
>
> +elapsed = time_msec() - start;
> +if (elapsed >= warning_threshold) {
> +VLOG_WARN("blocked %u ms waiting for %s to quiesce",
> + elapsed, stalled_thread);
> +warning_threshold *= 2;
> +}
> +poll_timer_wait_until(start + warning_threshold);
> +
> seq_wait(global_seqno, cur_seqno);
> poll_block();
> }
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index d835b39..19edf8f 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -274,9 +274,11 @@ ovsthread_wrapper(void *aux_)
> aux = *auxp;
> free(auxp);
>
> +/* The order of the following calls is important, because
> + * ovsrcu_quiesce_end() saves a copy of the thread name. */
> set_subprogram_name("%s%u", aux.name, id);
> -
> ovsrcu_quiesce_end();
> +
> return aux.start(aux.arg);
> }
>
> --
> 1.7.10.4
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Check for backported skb_orphan_frags().
On Sun, Apr 27, 2014 at 7:07 PM, Joe Stringer wrote: > This was causing build failures on debian wheezy. Check for the feature > rather than the version. > > Signed-off-by: Joe Stringer Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2.56] datapath: Add basic MPLS support to kernel
On Mon, Apr 28, 2014 at 12:00 AM, Simon Horman wrote: > On Fri, Apr 25, 2014 at 12:57:20PM -0700, Jesse Gross wrote: >> On Fri, Apr 25, 2014 at 1:06 AM, YAMAMOTO Takashi >> wrote: >> >> On Thu, Apr 24, 2014 at 05:57:29PM +0900, YAMAMOTO Takashi wrote: >> >>> hi, >> >>> >> >>> > + * Due to the sample action there may be multiple possible eth types. >> >>> > + * In order to correctly validate actions all possible types are >> >>> > tracked >> >>> > + * and verified. This is done using struct eth_types. >> >>> >> >>> is there any real-world use cases of these actions inside a sample? >> >>> otherwise, how about just rejecting such combinations? >> >>> it doesn't seem to worth the code complexity to me. >> >>> (sorry if it has been already discussed. it's the first time for me >> >>> to seriously read this long-lived patch.) >> >> >> >> Good point, the code is rather complex. >> >> >> >> My understanding is that it comes into effect in the case >> >> of sflow or ipfix being configured on the bridge. I tend >> >> to think these are real-world use-cases, though that thinking >> >> is by no means fixed. >> >> >> >> My reading of the code is that in the case of sflow and ipfix a single >> >> sample rule appears at the beginning of the flow. And that it may be >> >> possible to replace the code that you are referring to with something >> >> simpler to handle these cases. >> > >> > it seems that they put only a userland action inside a sample. >> > it's what i expected from its name "sample". >> >> Yes, that's the only current use case. In theory, this could be used >> more generally although nothing has come up yet. >> >> In retrospect, I regret the design of the sample action - not the part >> that allows it to handle different actions but the fact that the >> results can affect things outside of the sample action list. I think >> that if we had made it more like a subroutine then that would have >> retained all of the functionality but none of the complexity here. >> Perhaps if we can find a clean way to restructure it without breaking >> compatibility then that would simplify the validation here. > > I have not thought deeply about this but it seems to me that it should be > easy enough to provide compatibility for a new user-space to work with both > new and old datapaths. But it is not clear to me how to achieve the > reverse: allowing a new datapath to work with both new and old user-spaces. > I assume that we care about such compatibility. Generally, I would say yes although there is potentially some room for debate here. No version of OVS userspace has ever put an action other than userspace in the sample field. I know that other people have talked about writing different userspaces that run on the OVS kernel module but I highly doubt that they use this action or would do so differently. I can't prove that but it might be OK to bite the bullet. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] Prepare for 2.2.0.
Signed-off-by: Justin Pettit --- NEWS |4 ++-- configure.ac |2 +- debian/changelog | 24 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 02ecf5b..98db34e 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -Post-v2.1.0 +v2.2.0 - xx xxx xxx - - Internal ports are no longer brought up by default, because it should be an administrator task to bring up devices as they are @@ -19,7 +19,7 @@ Post-v2.1.0 - Added support for custom vlog patterns in Python -v2.1.0 - xx xxx +v2.1.0 - 19 Mar 2014 - - Address prefix tracking support for flow tables. New columns "prefixes" in OVS-DB table "Flow_Table" controls which packet diff --git a/configure.ac b/configure.ac index 3065528..4eebaee 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 2.1.90, b...@openvswitch.org) +AC_INIT(openvswitch, 2.2.0, b...@openvswitch.org) AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/debian/changelog b/debian/changelog index 3a1056f..601302e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,9 +1,25 @@ -openvswitch (2.1.90-1) unstable; urgency=low +openvswitch (2.2.0-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- Nothing yet! Try NEWS... - - -- Open vSwitch team Mon, 23 Dec 2013 18:07:18 -0700 + - Internal ports are no longer brought up by default, because it + should be an administrator task to bring up devices as they are + configured properly. + - ovs-vsctl now reports when ovs-vswitchd fails to create a new port or + bridge. + - The "ovsdbmonitor" graphical tool has been removed, because it was + poorly maintained and not widely used. + - New "check-ryu" Makefile target for running Ryu tests for OpenFlow + controllers against Open vSwitch. See INSTALL for details. + - Added IPFIX support for SCTP flows and templates for ICMPv4/v6 flows. + - Upon the receipt of a SIGHUP signal, ovs-vswitchd no longer reopens its + log file (it will terminate instead). Please use 'ovs-appctl vlog/reopen' + instead. + - Support for Linux kernels up to 3.13. From Kernel 3.12 onwards OVS uses + tunnel API for GRE and VXLAN. + - Added DPDK support. + - Added support for custom vlog patterns in Python + + -- Open vSwitch team Wed, 19 Mar 2014 16:08:38 -0700 openvswitch (2.1.0-1) unstable; urgency=low [ Open vSwitch team ] @@ -71,7 +87,7 @@ openvswitch (2.1.0-1) unstable; urgency=low users assumed incorrectly that ovs-controller was a necessary or desirable part of an Open vSwitch deployment. - -- Open vSwitch team Mon, 23 Dec 2013 18:07:18 -0700 + -- Open vSwitch team Wed, 19 Mar 2014 16:08:38 -0700 openvswitch (2.0.0-1) unstable; urgency=low [ Open vSwitch team ] -- 1.7.5.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] Prepare for post-2.2.0 (2.2.90).
Signed-off-by: Justin Pettit --- NEWS |4 configure.ac |2 +- debian/changelog |7 +++ 3 files changed, 12 insertions(+), 1 deletions(-) diff --git a/NEWS b/NEWS index 98db34e..74b57ff 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +Post-v2.2.0 +- + + v2.2.0 - xx xxx xxx - - Internal ports are no longer brought up by default, because it diff --git a/configure.ac b/configure.ac index 4eebaee..9672cb0 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 2.2.0, b...@openvswitch.org) +AC_INIT(openvswitch, 2.2.90, b...@openvswitch.org) AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/debian/changelog b/debian/changelog index 601302e..d6ed63d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +openvswitch (2.2.90-1) unstable; urgency=low + [ Open vSwitch team ] + * New upstream version +- Nothing yet! Try NEWS... + + -- Open vSwitch team Wed, 19 Mar 2014 16:08:38 -0700 + openvswitch (2.2.0-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- 1.7.5.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] Prepare 2.1.1 release.
Signed-off-by: Justin Pettit --- NEWS |3 ++- configure.ac |2 +- debian/changelog |9 + 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index eeba849..32dc352 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,8 @@ -Post-v2.1.0 +v2.1.1 - 28 Apr 2014 - - The "ovsdbmonitor" graphical tool has been removed, because it was poorly maintained and not widely used. + - Bug fixes. v2.1.0 - 19 Mar 2014 - diff --git a/configure.ac b/configure.ac index bec9844..3b9087b 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 2.1.0, ovs-b...@openvswitch.org) +AC_INIT(openvswitch, 2.1.1, ovs-b...@openvswitch.org) AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/debian/changelog b/debian/changelog index 7c35dbe..deea337 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +openvswitch (2.1.1-1) unstable; urgency=low + [ Open vSwitch team ] + * New upstream version + - The "ovsdbmonitor" graphical tool has been removed, because it was + poorly maintained and not widely used. + - Bug fixes. + + -- Open vSwitch team Mon, 28 Apr 2014 14:51:37 -0700 + openvswitch (2.1.0-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- 1.7.5.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] Prepare for 2.1.2.
We now increment the version immediately after a release. Signed-off-by: Justin Pettit --- NEWS |4 configure.ac |2 +- debian/changelog |6 ++ 3 files changed, 11 insertions(+), 1 deletions(-) diff --git a/NEWS b/NEWS index 32dc352..a31740e 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +v2.1.2 - xx xxx +- + + v2.1.1 - 28 Apr 2014 - - The "ovsdbmonitor" graphical tool has been removed, because it was diff --git a/configure.ac b/configure.ac index 3b9087b..af86fd5 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 2.1.1, ovs-b...@openvswitch.org) +AC_INIT(openvswitch, 2.1.2, ovs-b...@openvswitch.org) AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/debian/changelog b/debian/changelog index deea337..4db805c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +openvswitch (2.1.2-1) unstable; urgency=low + [ Open vSwitch team ] + - Nothing yet! + + -- Open vSwitch team Mon, 28 Apr 2014 14:51:37 -0700 + openvswitch (2.1.1-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- 1.7.5.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Check for backported skb_orphan_frags().
Thanks, applied. On 29 April 2014 09:27, Jesse Gross wrote: > On Sun, Apr 27, 2014 at 7:07 PM, Joe Stringer > wrote: > > This was causing build failures on debian wheezy. Check for the feature > > rather than the version. > > > > Signed-off-by: Joe Stringer > > Acked-by: Jesse Gross > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [urcu-fixes 3/4] ovs-thread: Make caller provide thread name when creating a thread.
On Mon, Apr 28, 2014 at 11:48:34AM -0700, Alex Wang wrote:
> This is helpful,??? just two comments,
>
> On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff wrote:
>
> > This patch is a prerequisite for making RCU report the name of a thread
> > that is blocking RCU synchronization,
> > *because the easiest way to do that gets the name of the thread in
> > ovsrcu_quiesce_end(), *which is called before
> > the thread function is called (so it won't get a name set within the thread
> > function itself).
> >
>
> Just curious, should we change the sentence in bold to "to do that is to
> get ..."
This paragraph was not well written. I think that this might be a
little clearer:
This patch is a prerequisite for making RCU report the name of a thread
that is blocking RCU synchronization, because the easiest way to do that is
for ovsrcu_quiesce_end() to record the current thread's name.
ovsrcu_quiesce_end() is called before the thread function is called, so it
won't get a name set within the thread function itself. Setting the thread
name earlier, as in this patch, avoids the problem.
> do we still need the ovsrcu_quiesce_end() here?
Yes, this patch doesn't change any properties of RCU yet.
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -293,7 +293,6 @@ ovs_thread_create(const char *name, void *(*start)(void
> *), void *arg)
>
> forbid_forking("multiple threads exist");
> multithreaded = true;
> -ovsrcu_quiesce_end();
>
> aux = xmalloc(sizeof *aux);
> aux->start = start;
>
> Acked-by: Alex Wang
Thanks for the review!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [urcu-fixes 4/4] ovs-rcu: Log a helpful warning when ovsrcu_synchronize() stalls.
Thank you for the reviews. I'll push these patches in a minute.
On Mon, Apr 28, 2014 at 02:10:27PM -0700, Alex Wang wrote:
> Acked-by: Alex Wang
>
>
> On Mon, Apr 28, 2014 at 9:06 AM, Ben Pfaff wrote:
>
> > This made it easier for me to find a thread that was causing stalls.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/ovs-rcu.c| 24 +++-
> > lib/ovs-thread.c |4 +++-
> > 2 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> > index c1ac61a..8a12564 100644
> > --- a/lib/ovs-rcu.c
> > +++ b/lib/ovs-rcu.c
> > @@ -21,6 +21,10 @@
> > #include "ovs-thread.h"
> > #include "poll-loop.h"
> > #include "seq.h"
> > +#include "timeval.h"
> > +#include "vlog.h"
> > +
> > +VLOG_DEFINE_THIS_MODULE(ovs_rcu);
> >
> > struct ovsrcu_cb {
> > void (*function)(void *aux);
> > @@ -34,11 +38,12 @@ struct ovsrcu_cbset {
> > };
> >
> > struct ovsrcu_perthread {
> > -struct list list_node; /* In global list. */
> > +struct list list_node; /* In global list. */
> >
> > struct ovs_mutex mutex;
> > uint64_t seqno;
> > struct ovsrcu_cbset *cbset;
> > +char name[16]; /* This thread's name. */
> > };
> >
> > static struct seq *global_seqno;
> > @@ -70,6 +75,8 @@ ovsrcu_perthread_get(void)
> > ovs_mutex_init(&perthread->mutex);
> > perthread->seqno = seq_read(global_seqno);
> > perthread->cbset = NULL;
> > +ovs_strlcpy(perthread->name, get_subprogram_name(),
> > +sizeof perthread->name);
> >
> > ovs_mutex_lock(&ovsrcu_threads_mutex);
> > list_push_back(&ovsrcu_threads, &perthread->list_node);
> > @@ -144,7 +151,9 @@ ovsrcu_is_quiescent(void)
> > static void
> > ovsrcu_synchronize(void)
> > {
> > +unsigned int warning_threshold = 1000;
> > uint64_t target_seqno;
> > +long long int start;
> >
> > if (single_threaded()) {
> > return;
> > @@ -152,15 +161,20 @@ ovsrcu_synchronize(void)
> >
> > target_seqno = seq_read(global_seqno);
> > ovsrcu_quiesce_start();
> > +start = time_msec();
> >
> > for (;;) {
> > uint64_t cur_seqno = seq_read(global_seqno);
> > struct ovsrcu_perthread *perthread;
> > +char stalled_thread[16];
> > +unsigned int elapsed;
> > bool done = true;
> >
> > ovs_mutex_lock(&ovsrcu_threads_mutex);
> > LIST_FOR_EACH (perthread, list_node, &ovsrcu_threads) {
> > if (perthread->seqno <= target_seqno) {
> > +ovs_strlcpy(stalled_thread, perthread->name,
> > +sizeof stalled_thread);
> > done = false;
> > break;
> > }
> > @@ -171,6 +185,14 @@ ovsrcu_synchronize(void)
> > break;
> > }
> >
> > +elapsed = time_msec() - start;
> > +if (elapsed >= warning_threshold) {
> > +VLOG_WARN("blocked %u ms waiting for %s to quiesce",
> > + elapsed, stalled_thread);
> > +warning_threshold *= 2;
> > +}
> > +poll_timer_wait_until(start + warning_threshold);
> > +
> > seq_wait(global_seqno, cur_seqno);
> > poll_block();
> > }
> > diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> > index d835b39..19edf8f 100644
> > --- a/lib/ovs-thread.c
> > +++ b/lib/ovs-thread.c
> > @@ -274,9 +274,11 @@ ovsthread_wrapper(void *aux_)
> > aux = *auxp;
> > free(auxp);
> >
> > +/* The order of the following calls is important, because
> > + * ovsrcu_quiesce_end() saves a copy of the thread name. */
> > set_subprogram_name("%s%u", aux.name, id);
> > -
> > ovsrcu_quiesce_end();
> > +
> > return aux.start(aux.arg);
> > }
> >
> > --
> > 1.7.10.4
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] Prepare 2.1.1 release.
On Mon, Apr 28, 2014 at 02:57:17PM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit For both: Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] Prepare for 2.2.0.
On Mon, Apr 28, 2014 at 02:48:56PM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit For both patches: Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] tests: Fix up "ofproto-dpif - ofproto-dpif-monitor 1".
Commit 1335a8d578b03e (tests: Fix race condition waiting for monitor thread.) fixed a race condition in a test. Commit 8ba0a5227f6 (ovs-thread: Make caller provide thread name when creating a thread.) slightly changed the output that the test checked, breaking the test. However, I was used to the test occasionally failing due to the race (not realizing that the race had been fixed) so I applied the commit anyway. This commit fixes the broken test. Signed-off-by: Ben Pfaff --- tests/ofproto-dpif.at |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index a1442f9..a32bc41 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -4516,12 +4516,12 @@ OVS_WAIT_UNTIL([grep "monitor thread created" ovs-vswitchd.log]) # disable bfd on p0. AT_CHECK([ovs-vsctl set interface p0 bfd:enable=false]) # check log, there should not be the log of thread terminated. -AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor)|INFO|\(.* terminated\)$/\1/p" ovs-vswitchd.log], [0], [dnl +AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor[[0-9]]*)|INFO|\(.* terminated\)$/\1/p" ovs-vswitchd.log], [0], [dnl ]) # reenable bfd on p0. AT_CHECK([ovs-vsctl set interface p0 bfd:enable=true]) # check log, should still be on log of thread created. -AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor)|INFO|\(.* created\)$/\1/p" ovs-vswitchd.log], [0], [dnl +AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor[[0-9]]*)|INFO|\(.* created\)$/\1/p" ovs-vswitchd.log], [0], [dnl monitor thread created ]) # disable bfd and cfm together. -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] lacp: Ensure that mutex is initialized when used in lacp_status().
Thanks, I applied these.
On Fri, Apr 25, 2014 at 11:50:43AM -0700, Andy Zhou wrote:
> Looks great. Thanks!
>
> On Fri, Apr 25, 2014 at 9:56 AM, Ben Pfaff wrote:
> > If 'lacp' is NULL, then lacp_create() might not have been called to
> > indirectly initialize the mutex via lacp_init(), so call lacp_init()
> > from lacp_status().
> >
> > Signed-off-by: Ben Pfaff
> > Acked-by: Andy Zhou
> > ---
> > lib/lacp.c | 18 +++---
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/lacp.c b/lib/lacp.c
> > index 4aee64f..dfb7159 100644
> > --- a/lib/lacp.c
> > +++ b/lib/lacp.c
> > @@ -199,21 +199,23 @@ parse_lacp_packet(const struct ofpbuf *b)
> > void
> > lacp_init(void)
> > {
> > -unixctl_command_register("lacp/show", "[port]", 0, 1,
> > - lacp_unixctl_show, NULL);
> > +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > +
> > +if (ovsthread_once_start(&once)) {
> > +ovs_mutex_init_recursive(&mutex);
> > +unixctl_command_register("lacp/show", "[port]", 0, 1,
> > + lacp_unixctl_show, NULL);
> > +ovsthread_once_done(&once);
> > +}
> > }
> >
> > /* Creates a LACP object. */
> > struct lacp *
> > lacp_create(void) OVS_EXCLUDED(mutex)
> > {
> > -static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > struct lacp *lacp;
> >
> > -if (ovsthread_once_start(&once)) {
> > -ovs_mutex_init_recursive(&mutex);
> > -ovsthread_once_done(&once);
> > -}
> > +lacp_init();
> >
> > lacp = xzalloc(sizeof *lacp);
> > hmap_init(&lacp->slaves);
> > @@ -347,6 +349,8 @@ lacp_status(const struct lacp *lacp) OVS_EXCLUDED(mutex)
> > {
> > enum lacp_status ret;
> >
> > +lacp_init();
> > +
> > ovs_mutex_lock(&mutex);
> > if (!lacp) {
> > ret = LACP_DISABLED;
> > --
> > 1.7.10.4
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 08/10] lib/classifier: Separate cls_rule internals from the API.
Acked-by: Ethan Jackson
I don't really like the API we've ended up with here. But I think
it's fine for now. As discussed offline, I intend to change it later.
Ethan
On Thu, Apr 24, 2014 at 6:25 PM, Ethan Jackson wrote:
> I haven't read this patch yet, but a high level question. Why not
> just hide all of struct cls_rule and make callers embed a pointer to
> it? We'd replace the cls_rule_init() function with a
> cls_rule_create() function which mallocs the rule and returns it (for
> example).
>
> Ethan
>
> On Fri, Apr 18, 2014 at 12:41 PM, Jarno Rajahalme
> wrote:
>> Keep an internal representation of a rule separate from the one
>> embedded into user's structs. This allows for further memory
>> optimization in the classifier.
>>
>> Signed-off-by: Jarno Rajahalme
>> ---
>> lib/classifier.c| 211
>> +--
>> lib/classifier.h| 22 ++---
>> tests/test-classifier.c | 18 ++--
>> 3 files changed, 149 insertions(+), 102 deletions(-)
>>
>> diff --git a/lib/classifier.c b/lib/classifier.c
>> index 0e67b7f..0517aa7 100644
>> --- a/lib/classifier.c
>> +++ b/lib/classifier.c
>> @@ -51,6 +51,10 @@ struct cls_subtable_cache {
>> size_t size; /* One past last valid array element. */
>> };
>>
>> +enum {
>> +CLS_MAX_INDICES = 3 /* Maximum number of lookup indices per subtable.
>> */
>> +};
>> +
>> struct cls_classifier {
>> int n_rules;/* Total number of rules. */
>> uint8_t n_flow_segments;
>> @@ -90,7 +94,30 @@ struct cls_partition {
>> struct tag_tracker tracker; /* Tracks the bits in 'tags'. */
>> };
>>
>> +/* Internal representation of a rule in a "struct cls_subtable". */
>> +struct cls_match {
>> +struct cls_rule *cls_rule;
>> +struct hindex_node index_nodes[CLS_MAX_INDICES]; /* Within subtable's
>> + * 'indices'. */
>> +struct hmap_node hmap_node; /* Within struct cls_subtable 'rules'. */
>> +unsigned int priority; /* Larger numbers are higher priorities. */
>> +struct cls_partition *partition;
>> +struct list list; /* List of identical, lower-priority rules.
>> */
>> +struct minimatch match; /* Matching rule. */
>> +};
>>
>> +static struct cls_match *
>> +cls_match_alloc(struct cls_rule *rule)
>> +{
>> +struct cls_match *cls_match = xmalloc(sizeof *cls_match);
>> +
>> +cls_match->cls_rule = rule;
>> +minimatch_clone(&cls_match->match, &rule->match);
>> +cls_match->priority = rule->priority;
>> +rule->cls_match = cls_match;
>> +
>> +return cls_match;
>> +}
>>
>> struct trie_ctx;
>> static struct cls_subtable *find_subtable(const struct cls_classifier *,
>> @@ -107,14 +134,14 @@ static void update_subtables_after_removal(struct
>> cls_classifier *,
>> struct cls_subtable *,
>> unsigned int del_priority);
>>
>> -static struct cls_rule *find_match_wc(const struct cls_subtable *,
>> - const struct flow *, struct trie_ctx
>> *,
>> - unsigned int n_tries,
>> - struct flow_wildcards *);
>> -static struct cls_rule *find_equal(struct cls_subtable *,
>> - const struct miniflow *, uint32_t hash);
>> -static struct cls_rule *insert_rule(struct cls_classifier *,
>> -struct cls_subtable *, struct cls_rule
>> *);
>> +static struct cls_match *find_match_wc(const struct cls_subtable *,
>> + const struct flow *, struct trie_ctx
>> *,
>> + unsigned int n_tries,
>> + struct flow_wildcards *);
>> +static struct cls_match *find_equal(struct cls_subtable *,
>> +const struct miniflow *, uint32_t hash);
>> +static struct cls_match *insert_rule(struct cls_classifier *,
>> + struct cls_subtable *, struct cls_rule
>> *);
>>
>> /* Iterates RULE over HEAD and all of the cls_rules on HEAD->list. */
>> #define FOR_EACH_RULE_IN_LIST(RULE, HEAD) \
>> @@ -124,8 +151,8 @@ static struct cls_rule *insert_rule(struct
>> cls_classifier *,
>> (RULE) != NULL && ((NEXT) = next_rule_in_list(RULE), true);\
>> (RULE) = (NEXT))
>>
>> -static struct cls_rule *next_rule_in_list__(struct cls_rule *);
>> -static struct cls_rule *next_rule_in_list(struct cls_rule *);
>> +static struct cls_match *next_rule_in_list__(struct cls_match *);
>> +static struct cls_match *next_rule_in_list(struct cls_match *);
>>
>> static unsigned int minimask_get_prefix_len(const struct minimask *,
>> const struct mf_field *);
>> @@ -415,6 +442,7 @@ cls_rule_init(struct cls_rule
Re: [ovs-dev] [PATCH] [RFC] flow: Do not clear L3+ fields of flow in flow_push_mpls()
On Mar 20, 2014, at 10:05 AM, Ben Pfaff wrote: > On Fri, Mar 14, 2014 at 04:19:52PM +0900, Simon Horman wrote: >> When creating a flow in the datapath as the result of an upcall >> the match itself is the match supplied in the upcall while >> the mask of the match, if supplied, is generated based on the >> flow and mask composed during action translation. >> >> In the case of, for example a UDP packet, the match will include >> of L2, L3 and L4 fields. However, if the flow is cleared in >> flow_push_mpls() then the mask that is synthesised from it will >> not include L3 and L4 fields. This seems incorrect and the kernel >> datapath complains about this mismatch. >> >> Signed-off-by: Simon Horman > > The goal of clearing the fields is to ensure that later flow tables > can't match on fields that aren't visible anymore. That's important for > accurate OpenFlow implementation, so I'd rather not change it. On the > other hand, I see the point you're making, but I don't immediately > understand why it happens that way. After all, I can change fields with > OpenFlow actions and the datapath flows work out OK, why doesn't this > work out OK too? Do you understand the reason? As the flow’s dl_type is changed to an MPLS type, later non-MPLS rules will not match on the modified flow. AFAIK, you can match on L3/L4 fields only by also matching on the corresponding dl_type as a prerequisite, no? If this holds, I’d rather not clear the fields so we can properly do a set IPv4 action followed by an MPLS push action. Currently the the MPLS action clears the flow values at the translation time set in the preceding action, so that at the commit time the values intended for set IPv4 action are lost. Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] ofproto-bond: do not allow recirculation when we failed to allocate recirc_id
On Thu, Apr 24, 2014 at 10:34:14PM -0700, Andy Zhou wrote: > When recirc pool is exhausted, a new bond won't be allocate a new > recirc_id. The bond->recirc_id will remain zero. This condition > should prevent the bond from use recirculation. This check was missing > before this patch. > > Signed-off-by: Andy Zhou Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] tests: Fix up "ofproto-dpif - ofproto-dpif-monitor 1".
Acked-by: Justin Pettit On April 28, 2014 at 3:45:26 PM, Ben Pfaff (b...@nicira.com) wrote: > Commit 1335a8d578b03e (tests: Fix race condition waiting for monitor > thread.) fixed a race condition in a test. Commit 8ba0a5227f6 (ovs-thread: > Make caller provide thread name when creating a thread.) slightly changed > the output that the test checked, breaking the test. However, I was used > to the test occasionally failing due to the race (not realizing that the > race had been fixed) so I applied the commit anyway. > > This commit fixes the broken test. > > Signed-off-by: Ben Pfaff > --- > tests/ofproto-dpif.at | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index a1442f9..a32bc41 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -4516,12 +4516,12 @@ OVS_WAIT_UNTIL([grep "monitor thread created" > ovs-vswitchd.log]) > # disable bfd on p0. > AT_CHECK([ovs-vsctl set interface p0 bfd:enable=false]) > # check log, there should not be the log of thread terminated. > -AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor)|INFO|\(.* > terminated\)$/\1/p" > ovs-vswitchd.log], [0], [dnl > +AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor[[0-9]]*)|INFO|\(.* > terminated\)$/\1/p" > ovs-vswitchd.log], [0], [dnl > ]) > # reenable bfd on p0. > AT_CHECK([ovs-vsctl set interface p0 bfd:enable=true]) > # check log, should still be on log of thread created. > -AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor)|INFO|\(.* > created\)$/\1/p" > ovs-vswitchd.log], [0], [dnl > +AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor[[0-9]]*)|INFO|\(.* > created\)$/\1/p" > ovs-vswitchd.log], [0], [dnl > monitor thread created > ]) > # disable bfd and cfm together. > -- > 1.7.10.4 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] ofproto-dpif: restore bond rebalance for non-recirc bond
On Thu, Apr 24, 2014 at 10:34:15PM -0700, Andy Zhou wrote: > Bond rebalancing was disabled for bonds not using recirculation. The > patch fixes this bug. > > While fixing the bug, the bond_rebalance() was also restructured > slightly to move bond related logic back into ofproto/bond.c > > Signed-off-by: Andy Zhou Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-linux: favor netlink stats for physical ports
Currently physical ports stats are collected from kernel datapath.
However, those counter do not reflect actual wire packet counters
when LSO or TSO are enabled by the NIC. In the meantime, the stats
collected form routing stack does. While both stats are valid, Reporting
netlink stats for packet counts and byte counts make it easier
to correlate those numbers with external measurements.
Signed-off-by: Andy Zhou
---
lib/netdev-linux.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 5c7065f..c7b3fec 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1566,6 +1566,13 @@ netdev_linux_get_stats(const struct netdev *netdev_,
/* stats not available from OVS then use ioctl stats. */
*stats = dev_stats;
} else {
+/* Use Netlink's packet and byte counts since vport's counters
+ * do not reflect packet counts on the wire when LSO/TSO are enabled.
*/
+stats->rx_packets = dev_stats.rx_packets;
+stats->rx_bytes = dev_stats.rx_bytes;
+stats->tx_packets = dev_stats.tx_packets;
+stats->tx_bytes = dev_stats.tx_bytes;
+
stats->rx_errors += dev_stats.rx_errors;
stats->tx_errors += dev_stats.tx_errors;
stats->rx_dropped += dev_stats.rx_dropped;
@@ -1629,6 +1636,13 @@ netdev_tap_get_stats(const struct netdev *netdev_,
struct netdev_stats *stats)
stats->tx_heartbeat_errors = 0;
stats->tx_window_errors = 0;
} else {
+/* Use Netlink's packet and byte counts since vport counters
+ * do not reflect packet counts on the wire when LSO/TSO are enabled.
*/
+stats->rx_packets = dev_stats.tx_packets;
+stats->rx_bytes = dev_stats.tx_bytes;
+stats->tx_packets = dev_stats.rx_packets;
+stats->tx_bytes = dev_stats.rx_bytes;
+
stats->rx_dropped += dev_stats.tx_dropped;
stats->tx_dropped += dev_stats.rx_dropped;
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] tests: Fix up "ofproto-dpif - ofproto-dpif-monitor 1".
Thanks, applied. On Mon, Apr 28, 2014 at 03:57:24PM -0700, Justin Pettit wrote: > Acked-by: Justin Pettit > > > > On April 28, 2014 at 3:45:26 PM, Ben Pfaff (b...@nicira.com) wrote: > > Commit 1335a8d578b03e (tests: Fix race condition waiting for monitor > > thread.) fixed a race condition in a test. Commit 8ba0a5227f6 (ovs-thread: > > Make caller provide thread name when creating a thread.) slightly changed > > the output that the test checked, breaking the test. However, I was used > > to the test occasionally failing due to the race (not realizing that the > > race had been fixed) so I applied the commit anyway. > > > > This commit fixes the broken test. > > > > Signed-off-by: Ben Pfaff > > --- > > tests/ofproto-dpif.at | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > > index a1442f9..a32bc41 100644 > > --- a/tests/ofproto-dpif.at > > +++ b/tests/ofproto-dpif.at > > @@ -4516,12 +4516,12 @@ OVS_WAIT_UNTIL([grep "monitor thread created" > > ovs-vswitchd.log]) > > # disable bfd on p0. > > AT_CHECK([ovs-vsctl set interface p0 bfd:enable=false]) > > # check log, there should not be the log of thread terminated. > > -AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor)|INFO|\(.* > > terminated\)$/\1/p" > > ovs-vswitchd.log], [0], [dnl > > +AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor[[0-9]]*)|INFO|\(.* > > terminated\)$/\1/p" > > ovs-vswitchd.log], [0], [dnl > > ]) > > # reenable bfd on p0. > > AT_CHECK([ovs-vsctl set interface p0 bfd:enable=true]) > > # check log, should still be on log of thread created. > > -AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor)|INFO|\(.* > > created\)$/\1/p" > > ovs-vswitchd.log], [0], [dnl > > +AT_CHECK([sed -n "s/^.*|ofproto_dpif_monitor(monitor[[0-9]]*)|INFO|\(.* > > created\)$/\1/p" > > ovs-vswitchd.log], [0], [dnl > > monitor thread created > > ]) > > # disable bfd and cfm together. > > -- > > 1.7.10.4 > > > > ___ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2.56] datapath: Add basic MPLS support to kernel
On Mon, Apr 28, 2014 at 02:37:47PM -0700, Jesse Gross wrote: > On Mon, Apr 28, 2014 at 12:00 AM, Simon Horman wrote: > > On Fri, Apr 25, 2014 at 12:57:20PM -0700, Jesse Gross wrote: > >> On Fri, Apr 25, 2014 at 1:06 AM, YAMAMOTO Takashi > >> wrote: > >> >> On Thu, Apr 24, 2014 at 05:57:29PM +0900, YAMAMOTO Takashi wrote: > >> >>> hi, > >> >>> > >> >>> > + * Due to the sample action there may be multiple possible eth > >> >>> > types. > >> >>> > + * In order to correctly validate actions all possible types are > >> >>> > tracked > >> >>> > + * and verified. This is done using struct eth_types. > >> >>> > >> >>> is there any real-world use cases of these actions inside a sample? > >> >>> otherwise, how about just rejecting such combinations? > >> >>> it doesn't seem to worth the code complexity to me. > >> >>> (sorry if it has been already discussed. it's the first time for me > >> >>> to seriously read this long-lived patch.) > >> >> > >> >> Good point, the code is rather complex. > >> >> > >> >> My understanding is that it comes into effect in the case > >> >> of sflow or ipfix being configured on the bridge. I tend > >> >> to think these are real-world use-cases, though that thinking > >> >> is by no means fixed. > >> >> > >> >> My reading of the code is that in the case of sflow and ipfix a single > >> >> sample rule appears at the beginning of the flow. And that it may be > >> >> possible to replace the code that you are referring to with something > >> >> simpler to handle these cases. > >> > > >> > it seems that they put only a userland action inside a sample. > >> > it's what i expected from its name "sample". > >> > >> Yes, that's the only current use case. In theory, this could be used > >> more generally although nothing has come up yet. > >> > >> In retrospect, I regret the design of the sample action - not the part > >> that allows it to handle different actions but the fact that the > >> results can affect things outside of the sample action list. I think > >> that if we had made it more like a subroutine then that would have > >> retained all of the functionality but none of the complexity here. > >> Perhaps if we can find a clean way to restructure it without breaking > >> compatibility then that would simplify the validation here. > > > > I have not thought deeply about this but it seems to me that it should be > > easy enough to provide compatibility for a new user-space to work with both > > new and old datapaths. But it is not clear to me how to achieve the > > reverse: allowing a new datapath to work with both new and old user-spaces. > > I assume that we care about such compatibility. > > Generally, I would say yes although there is potentially some room for > debate here. No version of OVS userspace has ever put an action other > than userspace in the sample field. I know that other people have > talked about writing different userspaces that run on the OVS kernel > module but I highly doubt that they use this action or would do so > differently. I can't prove that but it might be OK to bite the bullet. I am also concerned about the sample() action which is exposed via OpenFlow (as a Nicira extension) and in turn ovs-ofctl. Thus it seems to me that there could be users adding flows with sample actions whose behaviour would either no longer be supported or would be changed. But I believe that we should reason about this case the same way that you reason about alternate user-spaces above. Perhaps a way forward would be (for me) to come up with a prototype to alter the sample action. And we can see how clean it is (or could be) and what it buys us. It seems that the motivation for this change is is twofold: To contain the sample action in the hope of making it easier to deal with in the future and; to avoid some rather complex verification code introduced in the MPLS datapath patch. And I think it would be good to keep that in mind when assessing any prototype code. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] lib/ofp-actions: Update comment.
Pushed to master, Jarno On Apr 14, 2014, at 1:23 PM, Jarno Rajahalme wrote: > >> On Apr 14, 2014, at 1:00 PM, Justin Pettit wrote: >> >> It looks like there are a few other places that need to get updated, too. >> Did you want me to take a pass at them? >> > > Why not, seems you found them already, so please go ahead :-) > > Jarno > >> For this, though: >> >> Acked-by: Justin Pettit >> >> --Justin >> >> >>> On April 14, 2014 at 10:51:08 AM, Jarno Rajahalme (jrajaha...@nicira.com) >>> wrote: >>> We recently renamed ofpbuf's 'l2' member as 'frame'. >>> >>> Signed-off-by: Jarno Rajahalme >>> --- >>> lib/ofp-actions.h | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h >>> index 89bf867..1db3bde 100644 >>> --- a/lib/ofp-actions.h >>> +++ b/lib/ofp-actions.h >>> @@ -641,8 +641,8 @@ void *ofpact_put(struct ofpbuf *, enum ofpact_type, >>> size_t len); >>> * After using this function to add a variable-length action, add the >>> * elements of the flexible array (e.g. with ofpbuf_put()), then use >>> * ofpact_update_len() to update the length embedded into the action. >>> - * (Keep in mind the need to refresh the structure from 'ofpacts->l2' after >>> - * adding data to 'ofpacts'.) >>> + * (Keep in mind the need to refresh the structure from 'ofpacts->frame' >>> + * after adding data to 'ofpacts'.) >>> * >>> * struct *ofpact_get_(const struct ofpact *ofpact); >>> * >>> -- >>> 1.7.10.4 >>> >>> ___ >>> dev mailing list >>> dev@openvswitch.org >>> http://openvswitch.org/mailman/listinfo/dev >> ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 1/8] openvswitch.h: Note that 64 bit ints are 4-aligned.
Pushed to master, Jarno On Apr 25, 2014, at 1:47 PM, Jesse Gross wrote: > On Fri, Apr 11, 2014 at 4:29 PM, Jarno Rajahalme > wrote: >> In general, all Netlink 64-bit data may be 4-byte aligned, due to >> netlink header and attributes being 4-aligned. >> >> To avoid unaligned access the data should be copied out of the netlink >> attribute before access. >> >> Signed-off-by: Jarno Rajahalme > > Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 2/8] lib/odp-util: Remove extra parenthesis from sctp key output.
Thanks for the review, pushed to master,
Jarno
On Apr 14, 2014, at 7:25 PM, YAMAMOTO Takashi wrote:
>> Signed-off-by: Jarno Rajahalme
>
> Reviewed-by: YAMAMOTO Takashi
>
>> ---
>> lib/odp-util.c |2 +-
>> tests/odp.at |1 +
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/odp-util.c b/lib/odp-util.c
>> index b58f1c0..10bf925 100644
>> --- a/lib/odp-util.c
>> +++ b/lib/odp-util.c
>> @@ -1308,7 +1308,7 @@ format_odp_key_attr(const struct nlattr *a, const
>> struct nlattr *ma,
>> } else {
>> const struct ovs_key_sctp *sctp_key = nl_attr_get(a);
>>
>> -ds_put_format(ds, "(src=%"PRIu16",dst=%"PRIu16")",
>> +ds_put_format(ds, "src=%"PRIu16",dst=%"PRIu16,
>> ntohs(sctp_key->sctp_src),
>> ntohs(sctp_key->sctp_dst));
>> }
>> break;
>> diff --git a/tests/odp.at b/tests/odp.at
>> index 34bc187..26f561f 100644
>> --- a/tests/odp.at
>> +++ b/tests/odp.at
>> @@ -242,6 +242,7 @@ set(eth_type(0x1234))
>> set(ipv4(src=35.8.2.41,dst=172.16.0.20,proto=5,tos=0x80,ttl=128,frag=no))
>> set(tcp(src=80,dst=8080))
>> set(udp(src=81,dst=6632))
>> +set(sctp(src=82,dst=6633))
>> set(icmp(type=1,code=2))
>> set(ipv6(src=::1,dst=::2,label=0,proto=10,tclass=0x70,hlimit=128,frag=no))
>> set(icmpv6(type=1,code=2))
>> --
>> 1.7.10.4
>>
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 7/8] openvswitch.h: Clarify use of key attributes.
Thanks for the review, pushed to master, Jarno On Apr 25, 2014, at 1:48 PM, Jesse Gross wrote: > On Fri, Apr 11, 2014 at 4:29 PM, Jarno Rajahalme > wrote: >> Key attributes relating to actual packet headers are ignored for >> OVS_PACKET_CMD_EXECUTE as the header key attributes are retrieved >> from the packet itself. >> >> Signed-off-by: Jarno Rajahalme > > Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] Prepare 2.1.1 release.
Will you push a tag for this too? On 29 April 2014 10:31, Ben Pfaff wrote: > On Mon, Apr 28, 2014 at 02:57:17PM -0700, Justin Pettit wrote: > > Signed-off-by: Justin Pettit > > For both: > Acked-by: Ben Pfaff > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 09/10] lib/flow: Maintain miniflow offline values explicitly.
Acked-by: Ethan Jackson
I'm not sure I totally follow Kmindg's comment, perhaps he could
explain further? At any rate, once addressed I"m happy with this.
Ethan
On Sat, Apr 19, 2014 at 10:09 PM, Kmindg G wrote:
> On Sat, Apr 19, 2014 at 3:42 AM, Jarno Rajahalme
> wrote:
>> This allows use of miniflows that have all of their values inline.
>>
>> Signed-off-by: Jarno Rajahalme
>> ---
>> lib/classifier.c | 36 +++--
>> lib/dpif-netdev.c | 32 ++-
>> lib/flow.c| 91
>> ++---
>> lib/flow.h| 70 +
>> lib/match.c |4 +--
>> 5 files changed, 127 insertions(+), 106 deletions(-)
>>
>> diff --git a/lib/classifier.c b/lib/classifier.c
>> index 0517aa7..75befad 100644
>> --- a/lib/classifier.c
>> +++ b/lib/classifier.c
>> @@ -40,7 +40,7 @@ struct cls_trie {
>>
>> struct cls_subtable_entry {
>> struct cls_subtable *subtable;
>> -uint32_t *mask_values;
>> +const uint32_t *mask_values;
>> tag_type tag;
>> unsigned int max_priority;
>> };
>> @@ -279,8 +279,9 @@ static inline uint32_t
>> flow_hash_in_minimask(const struct flow *flow, const struct minimask *mask,
>>uint32_t basis)
>> {
>> +const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks);
>> const uint32_t *flow_u32 = (const uint32_t *)flow;
>> -const uint32_t *p = mask->masks.values;
>> +const uint32_t *p = mask_values;
>> uint32_t hash;
>> uint64_t map;
>>
>> @@ -289,7 +290,7 @@ flow_hash_in_minimask(const struct flow *flow, const
>> struct minimask *mask,
>> hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p++);
>> }
>>
>> -return mhash_finish(hash, (p - mask->masks.values) * 4);
>> +return mhash_finish(hash, (p - mask_values) * 4);
>> }
>>
>> /* Returns a hash value for the bits of 'flow' where there are 1-bits in
>> @@ -301,7 +302,8 @@ static inline uint32_t
>> miniflow_hash_in_minimask(const struct miniflow *flow,
>>const struct minimask *mask, uint32_t basis)
>> {
>> -const uint32_t *p = mask->masks.values;
>> +const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks);
>> +const uint32_t *p = mask_values;
>> uint32_t hash = basis;
>> uint32_t flow_u32;
>>
>> @@ -309,7 +311,7 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
>> hash = mhash_add(hash, flow_u32 & *p++);
>> }
>>
>> -return mhash_finish(hash, (p - mask->masks.values) * 4);
>> +return mhash_finish(hash, (p - mask_values) * 4);
>> }
>>
>> /* Returns a hash value for the bits of range [start, end) in 'flow',
>> @@ -322,11 +324,12 @@ flow_hash_in_minimask_range(const struct flow *flow,
>> const struct minimask *mask,
>> uint8_t start, uint8_t end, uint32_t *basis)
>> {
>> +const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks);
>> const uint32_t *flow_u32 = (const uint32_t *)flow;
>> unsigned int offset;
>> uint64_t map = miniflow_get_map_in_range(&mask->masks, start, end,
>> &offset);
>> -const uint32_t *p = mask->masks.values + offset;
>> +const uint32_t *p = mask_values + offset;
>> uint32_t hash = *basis;
>>
>> for (; map; map = zero_rightmost_1bit(map)) {
>> @@ -334,7 +337,7 @@ flow_hash_in_minimask_range(const struct flow *flow,
>> }
>>
>> *basis = hash; /* Allow continuation from the unfinished value. */
>> -return mhash_finish(hash, (p - mask->masks.values) * 4);
>> +return mhash_finish(hash, (p - mask_values) * 4);
>> }
>>
>> /* Fold minimask 'mask''s wildcard mask into 'wc's wildcard mask. */
>> @@ -356,7 +359,7 @@ flow_wildcards_fold_minimask_range(struct flow_wildcards
>> *wc,
>> unsigned int offset;
>> uint64_t map = miniflow_get_map_in_range(&mask->masks, start, end,
>> &offset);
>> -const uint32_t *p = mask->masks.values + offset;
>> +const uint32_t *p = miniflow_get_u32_values(&mask->masks) + offset;
>>
>> for (; map; map = zero_rightmost_1bit(map)) {
>> dst_u32[raw_ctz(map)] |= *p++;
>> @@ -367,7 +370,8 @@ flow_wildcards_fold_minimask_range(struct flow_wildcards
>> *wc,
>> static inline uint32_t
>> miniflow_hash(const struct miniflow *flow, uint32_t basis)
>> {
>> -const uint32_t *p = flow->values;
>> +const uint32_t *values = miniflow_get_u32_values(flow);
>> +const uint32_t *p = values;
>> uint32_t hash = basis;
>> uint64_t hash_map = 0;
>> uint64_t map;
>> @@ -382,7 +386,7 @@ miniflow_hash(const struct miniflow *flow, uint32_t
>> basis)
>> hash = mhash_add(hash, hash_map);
>> hash = mhash_add(hash, hash_map >> 32);
>>
>> -return mhash_finish(hash, p - flow->values);
>> +return mhash_finish(hash, p - values);
>> }
>>
Re: [ovs-dev] [PATCH 10/10] lib/classifier: Support variable sized miniflows.
The comment of miniflow_and_mask_matches_flow() need to be reworded.
Do we really need that function? Is it really that much faster?
Having two functions which do the exact same thing is a bit confusing.
So I suppose the only reason for a non zero inline_values array is for
those callers who can't allocate something that's variable sized, but
still want to avoid the extra memory allocation. However, I don't
think there are any of those callers which are performance critical.
I'm wondering if we could simplify this significantly by always having
offline values passed in by the caller. For those callers which can
make it variable sized, they would, everyone else would make it
FLOW_U32s sized?
What do you think?
Ethan
On Sat, Apr 19, 2014 at 9:51 PM, Kmindg G wrote:
> On Sat, Apr 19, 2014 at 3:42 AM, Jarno Rajahalme
> wrote:
>> Change the classifier to allocate variable sized miniflows and
>> minimasks in cls_match and cls_subtable, respectively. Do not
>> duplicate the mask in cls_rule any more.
>>
>> miniflow_clone and miniflow_move can now take variably sized miniflows
>> as source. The destination is assumed to be regularly sized miniflow.
>>
>> Inlining miniflow and mask values reduces memory indirection and helps
>> reduce cache misses.
>>
>> Signed-off-by: Jarno Rajahalme
>> ---
>> lib/classifier.c | 82
>> +++---
>> lib/flow.c | 55 +++-
>> lib/flow.h | 30 ++--
>> 3 files changed, 134 insertions(+), 33 deletions(-)
>>
>> diff --git a/lib/classifier.c b/lib/classifier.c
>> index 75befad..1198a76 100644
>> --- a/lib/classifier.c
>> +++ b/lib/classifier.c
>> @@ -40,7 +40,6 @@ struct cls_trie {
>>
>> struct cls_subtable_entry {
>> struct cls_subtable *subtable;
>> -const uint32_t *mask_values;
>> tag_type tag;
>> unsigned int max_priority;
>> };
>> @@ -72,7 +71,6 @@ struct cls_subtable {
>> struct hmap_node hmap_node; /* Within struct cls_classifier 'subtables'
>> * hmap. */
>> struct hmap rules; /* Contains "struct cls_rule"s. */
>> -struct minimask mask; /* Wildcards for fields. */
>> int n_rules;/* Number of rules, including duplicates. */
>> unsigned int max_priority; /* Max priority of any rule in the
>> subtable. */
>> unsigned int max_count; /* Count of max_priority rules. */
>> @@ -81,6 +79,8 @@ struct cls_subtable {
>> uint8_t index_ofs[CLS_MAX_INDICES]; /* u32 flow segment boundaries. */
>> struct hindex indices[CLS_MAX_INDICES]; /* Staged lookup indices. */
>> unsigned int trie_plen[CLS_MAX_TRIES]; /* Trie prefix length in
>> 'mask'. */
>> +struct minimask mask; /* Wildcards for fields. */
>> +/* 'mask' must be the last field. */
>> };
>>
>> /* Associates a metadata value (that is, a value of the OpenFlow 1.1+
>> metadata
>> @@ -103,16 +103,21 @@ struct cls_match {
>> unsigned int priority; /* Larger numbers are higher priorities. */
>> struct cls_partition *partition;
>> struct list list; /* List of identical, lower-priority rules.
>> */
>> -struct minimatch match; /* Matching rule. */
>> +struct miniflow flow; /* Matching rule. Mask is in the subtable.
>> */
>> +/* 'flow' must be the last field. */
>> };
>>
>> static struct cls_match *
>> cls_match_alloc(struct cls_rule *rule)
>> {
>> -struct cls_match *cls_match = xmalloc(sizeof *cls_match);
>> +int count = count_1bits(rule->match.flow.map);
>> +
>> +struct cls_match *cls_match
>> += xmalloc(sizeof *cls_match - sizeof cls_match->flow.inline_values
>> + + MINIFLOW_VALUES_SIZE(count));
>
> Would it lead to a potential array access violation problem when
> 'sizeof cls_match->flow.inline_values' is bigger than
> 'MINIFLOW_VALUES_SIZE(count)'?
>
>>
>> cls_match->cls_rule = rule;
>> -minimatch_clone(&cls_match->match, &rule->match);
>> +miniflow_clone_inline(&cls_match->flow, &rule->match.flow, count);
>> cls_match->priority = rule->priority;
>> rule->cls_match = cls_match;
>>
>> @@ -875,7 +880,6 @@ static inline void
>> lookahead_subtable(const struct cls_subtable_entry *subtables)
>> {
>> ovs_prefetch_range(subtables->subtable, sizeof *subtables->subtable);
>> -ovs_prefetch_range(subtables->mask_values, 1);
>> }
>>
>> /* Finds and returns the highest-priority rule in 'cls' that matches 'flow'.
>> @@ -969,16 +973,19 @@ classifier_lookup(const struct classifier *cls_, const
>> struct flow *flow,
>> }
>>
>> /* Returns true if 'target' satisifies 'match', that is, if each bit for
>> which
>> - * 'match' specifies a particular value has the correct value in 'target'.
>> */
>> + * 'match' specifies a particular value has the correct value in 'target'.
>> + *
>> + * 'flow' and 'mask' have the same mask! */
>> static bool
>> -minimatch_ma
Re: [ovs-dev] [PATCH 09/10] lib/flow: Maintain miniflow offline values explicitly.
On Tue, Apr 29, 2014 at 9:43 AM, Ethan Jackson wrote:
> Acked-by: Ethan Jackson
>
> I'm not sure I totally follow Kmindg's comment, perhaps he could
> explain further? At any rate, once addressed I"m happy with this.
>
After a second look into this, I found that I misunderstood these code before.
Sorry for the noise.
> Ethan
>
> On Sat, Apr 19, 2014 at 10:09 PM, Kmindg G wrote:
>> On Sat, Apr 19, 2014 at 3:42 AM, Jarno Rajahalme
>> wrote:
>>> This allows use of miniflows that have all of their values inline.
>>>
>>> Signed-off-by: Jarno Rajahalme
>>> ---
>>> lib/classifier.c | 36 +++--
>>> lib/dpif-netdev.c | 32 ++-
>>> lib/flow.c| 91
>>> ++---
>>> lib/flow.h| 70 +
>>> lib/match.c |4 +--
>>> 5 files changed, 127 insertions(+), 106 deletions(-)
>>>
>>> diff --git a/lib/classifier.c b/lib/classifier.c
>>> index 0517aa7..75befad 100644
>>> --- a/lib/classifier.c
>>> +++ b/lib/classifier.c
>>> @@ -40,7 +40,7 @@ struct cls_trie {
>>>
>>> struct cls_subtable_entry {
>>> struct cls_subtable *subtable;
>>> -uint32_t *mask_values;
>>> +const uint32_t *mask_values;
>>> tag_type tag;
>>> unsigned int max_priority;
>>> };
>>> @@ -279,8 +279,9 @@ static inline uint32_t
>>> flow_hash_in_minimask(const struct flow *flow, const struct minimask *mask,
>>>uint32_t basis)
>>> {
>>> +const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks);
>>> const uint32_t *flow_u32 = (const uint32_t *)flow;
>>> -const uint32_t *p = mask->masks.values;
>>> +const uint32_t *p = mask_values;
>>> uint32_t hash;
>>> uint64_t map;
>>>
>>> @@ -289,7 +290,7 @@ flow_hash_in_minimask(const struct flow *flow, const
>>> struct minimask *mask,
>>> hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p++);
>>> }
>>>
>>> -return mhash_finish(hash, (p - mask->masks.values) * 4);
>>> +return mhash_finish(hash, (p - mask_values) * 4);
>>> }
>>>
>>> /* Returns a hash value for the bits of 'flow' where there are 1-bits in
>>> @@ -301,7 +302,8 @@ static inline uint32_t
>>> miniflow_hash_in_minimask(const struct miniflow *flow,
>>>const struct minimask *mask, uint32_t basis)
>>> {
>>> -const uint32_t *p = mask->masks.values;
>>> +const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks);
>>> +const uint32_t *p = mask_values;
>>> uint32_t hash = basis;
>>> uint32_t flow_u32;
>>>
>>> @@ -309,7 +311,7 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
>>> hash = mhash_add(hash, flow_u32 & *p++);
>>> }
>>>
>>> -return mhash_finish(hash, (p - mask->masks.values) * 4);
>>> +return mhash_finish(hash, (p - mask_values) * 4);
>>> }
>>>
>>> /* Returns a hash value for the bits of range [start, end) in 'flow',
>>> @@ -322,11 +324,12 @@ flow_hash_in_minimask_range(const struct flow *flow,
>>> const struct minimask *mask,
>>> uint8_t start, uint8_t end, uint32_t *basis)
>>> {
>>> +const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks);
>>> const uint32_t *flow_u32 = (const uint32_t *)flow;
>>> unsigned int offset;
>>> uint64_t map = miniflow_get_map_in_range(&mask->masks, start, end,
>>> &offset);
>>> -const uint32_t *p = mask->masks.values + offset;
>>> +const uint32_t *p = mask_values + offset;
>>> uint32_t hash = *basis;
>>>
>>> for (; map; map = zero_rightmost_1bit(map)) {
>>> @@ -334,7 +337,7 @@ flow_hash_in_minimask_range(const struct flow *flow,
>>> }
>>>
>>> *basis = hash; /* Allow continuation from the unfinished value. */
>>> -return mhash_finish(hash, (p - mask->masks.values) * 4);
>>> +return mhash_finish(hash, (p - mask_values) * 4);
>>> }
>>>
>>> /* Fold minimask 'mask''s wildcard mask into 'wc's wildcard mask. */
>>> @@ -356,7 +359,7 @@ flow_wildcards_fold_minimask_range(struct
>>> flow_wildcards *wc,
>>> unsigned int offset;
>>> uint64_t map = miniflow_get_map_in_range(&mask->masks, start, end,
>>> &offset);
>>> -const uint32_t *p = mask->masks.values + offset;
>>> +const uint32_t *p = miniflow_get_u32_values(&mask->masks) + offset;
>>>
>>> for (; map; map = zero_rightmost_1bit(map)) {
>>> dst_u32[raw_ctz(map)] |= *p++;
>>> @@ -367,7 +370,8 @@ flow_wildcards_fold_minimask_range(struct
>>> flow_wildcards *wc,
>>> static inline uint32_t
>>> miniflow_hash(const struct miniflow *flow, uint32_t basis)
>>> {
>>> -const uint32_t *p = flow->values;
>>> +const uint32_t *values = miniflow_get_u32_values(flow);
>>> +const uint32_t *p = values;
>>> uint32_t hash = basis;
>>> uint64_t hash_map = 0;
>>> uint64_t map;
>>> @@
Re: [ovs-dev] [PATCH] datapath: Check for backported skb_orphan_frags().
Applied to branch-2.1 as well. On 29 April 2014 10:21, Joe Stringer wrote: > Thanks, applied. > > > On 29 April 2014 09:27, Jesse Gross wrote: > >> On Sun, Apr 27, 2014 at 7:07 PM, Joe Stringer >> wrote: >> > This was causing build failures on debian wheezy. Check for the feature >> > rather than the version. >> > >> > Signed-off-by: Joe Stringer >> >> Acked-by: Jesse Gross >> > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] Prepare 2.1.1 release.
I normally wait for the announcement and the tarball, just in case some last minute bug fix gets slipped in. On Tue, Apr 29, 2014 at 01:40:46PM +1200, Joe Stringer wrote: > Will you push a tag for this too? > > > On 29 April 2014 10:31, Ben Pfaff wrote: > > > On Mon, Apr 28, 2014 at 02:57:17PM -0700, Justin Pettit wrote: > > > Signed-off-by: Justin Pettit > > > > For both: > > Acked-by: Ben Pfaff > > ___ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Revert "Prepare for 2.1.2."
This reverts commit 82e413df because bug fix bf52ed4 (datapath: Check for backported skb_orphan_frags().) was required. Signed-off-by: Justin Pettit --- NEWS |4 configure.ac |2 +- debian/changelog |6 -- 3 files changed, 1 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index a31740e..32dc352 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,3 @@ -v2.1.2 - xx xxx -- - - v2.1.1 - 28 Apr 2014 - - The "ovsdbmonitor" graphical tool has been removed, because it was diff --git a/configure.ac b/configure.ac index af86fd5..3b9087b 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 2.1.2, ovs-b...@openvswitch.org) +AC_INIT(openvswitch, 2.1.1, ovs-b...@openvswitch.org) AC_CONFIG_SRCDIR([datapath/datapath.c]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/debian/changelog b/debian/changelog index 4db805c..deea337 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,9 +1,3 @@ -openvswitch (2.1.2-1) unstable; urgency=low - [ Open vSwitch team ] - - Nothing yet! - - -- Open vSwitch team Mon, 28 Apr 2014 14:51:37 -0700 - openvswitch (2.1.1-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- 1.7.5.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] netdev: Fix an use of uninitialized mutex.
Commit 05bf6d3c62e1d (ovs-thread: Add checking for mutex and
rwlock initialization.) helps find an use of uninitialized
mutex (netdev_class_mutex) during upgrade. The assertion
check aborts the ovs.
This commit fixes the issue by adding the proper initialization.
Bug #1239914.
Bug #1240598.
Bug #1240626.
Signed-off-by: Alex Wang
---
lib/netdev.c |1 +
1 file changed, 1 insertion(+)
diff --git a/lib/netdev.c b/lib/netdev.c
index 45a4165..2fc1834 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -151,6 +151,7 @@ netdev_run(void)
{
struct netdev_registered_class *rc;
+netdev_initialize();
ovs_mutex_lock(&netdev_class_mutex);
HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
if (rc->class->run) {
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] dpif-linux: Fix a bug in creating port.
Based on the policy of 'OVS_VPORT_ATTR_UPCALL_PID', if upcall should not be sent to userspace (i.e. the number of handler threads is zero), the netlink message for creating vport should be an one-element array of value 0. However, dpif_linux_port_add__() fails to obey it and generates zero-payload netlink message which causes the netlink transaction failing with ERANGE error. This is particularly bad when the 'flow-restore-wait' is set during upgrade, since number of handler threads is not set in dpif-linux module and ovs is not able to add port in datapath until the 'flow-restore-wait' is disabled. Connection may lose due to this bug. This bug was introduced by commit 1579cf677fc (dpif-linux: Implement the API functions to allow multiple handler threads read upcall.). This commit fixes the bug by fixing the dpif_linux_port_add__() to generate the correct netlink message when the number of handler threads is not set. Bug #1239914. Bug #1240598. Bug #1240626. Reported-by: Gurucharan Shetty Signed-off-by: Alex Wang --- lib/dpif-linux.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index a575b78..abb4b51 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -676,7 +676,7 @@ dpif_linux_port_add__(struct dpif_linux *dpif, struct netdev *netdev, request.port_no = *port_nop; upcall_pids = vport_socksp_to_pids(socksp, dpif->n_handlers); -request.n_upcall_pids = dpif->n_handlers; +request.n_upcall_pids = socksp ? dpif->n_handlers : 1; request.upcall_pids = upcall_pids; error = dpif_linux_vport_transact(&request, &reply, &buf); -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
