[ovs-dev] [PATCH] ovs-vsctl: check if the device name is valid
Before waiting for the kernel to reject an invalid name, we
can actually check it before going into the kernel. The code
is stolen from linux kernel function dev_valid_name(),
but it should apply to non-Linux arch as well, because
IFNAMSIZ is POSIX and other errors are obvious.
After this patch I got:
# ovs-vsctl add-port ovsbr0 12345678901234567890
ovs-vsctl: cannot create a port named 12345678901234567890 because the name is
not valid
# ovs-vsctl add-br 12345678901234567890
ovs-vsctl: cannot create a bridge named 12345678901234567890 because the name
is not valid
Cc: Ben Pfaff
Signed-off-by: Cong Wang
---
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index fda3a89..3015f8c 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "command-line.h"
#include "compiler.h"
@@ -992,6 +993,28 @@ vsctl_context_populate_cache(struct vsctl_context *ctx)
sset_destroy(&bridges);
}
+/* Stolen from Linux kernel net/core/dev.c::dev_valid_name()
+ * Catch the most obvious errors and enforce the POSIX limit IFNAMSIZ
+ */
+static int
+is_valid_name(const char *name)
+{
+if (*name == '\0')
+return 0;
+if (strlen(name) >= IFNAMSIZ)
+return 0;
+if (!strcmp(name, ".") || !strcmp(name, ".."))
+return 0;
+
+while (*name) {
+if (*name == '/' || isspace(*name))
+return 0;
+name++;
+}
+
+return 1;
+}
+
static void
check_conflicts(struct vsctl_context *ctx, const char *name,
char *msg)
@@ -1001,6 +1024,10 @@ check_conflicts(struct vsctl_context *ctx, const char
*name,
verify_ports(ctx);
+if (!is_valid_name(name)) {
+vsctl_fatal("%s because the name is not valid", msg);
+}
+
if (shash_find(&ctx->bridges, name)) {
vsctl_fatal("%s because a bridge named %s already exists",
msg, name);
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Fix zero key tunnels.
On Oct 23, 2012, at 8:42 PM, Pravin B Shelar wrote: > Datapath tunneling check for flag OVS_FLOW_TNL_F_KEY is failing, > causing it to drop packet. This only happens on tunnels with > zero key as vswitchd does not generate set-tunnel action. Set > tunnel action sets this flags for given action. To fix this issue > the check is dropped. > > Bug #13666 > > Signed-off-by: Pravin B Shelar Looks good to me. Acked-by: Kyle Mestery ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-vsctl: check if the device name is valid
On Wed, Oct 24, 2012 at 05:45:36PM +0800, Cong Wang wrote: > Before waiting for the kernel to reject an invalid name, we > can actually check it before going into the kernel. The code > is stolen from linux kernel function dev_valid_name(), > but it should apply to non-Linux arch as well, because > IFNAMSIZ is POSIX and other errors are obvious. > > After this patch I got: > > # ovs-vsctl add-port ovsbr0 12345678901234567890 > ovs-vsctl: cannot create a port named 12345678901234567890 because the name > is not valid > # ovs-vsctl add-br 12345678901234567890 > ovs-vsctl: cannot create a bridge named 12345678901234567890 because the name > is not valid I understand why this is an attractive patch, but it restricts what ovs-vsctl can do to what Linux can handle. ovs-vsctl, and Open vSwitch, are meant to be more portable than that. Different operating systems have different limits on the maximum length and the allowed format of port names. I don't have the ESX source code right here, for example, but if I recall correctly the maximum length of a port name is much longer in ESX, and less restricted, than in Linux. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Linux Kernel 3.6.x and OVS
Is anyone working on getting this kernel working with OVS? I've started looking at this, as Fedora has moved to a 3.6.2 kernel, and it looks like the routing code has changed such that the tunneling code in OVS needs an update. I can continue to proceed down this path unless someone else is already aware of this and working on it. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/3] rconn: Factor code out from copy_to_monitor().
This prepares for the introduction of a second user in the following
commit.
Signed-off-by: Ben Pfaff
---
lib/rconn.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/rconn.c b/lib/rconn.c
index ddf578c..5d7595f 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -144,6 +144,7 @@ static void reconnect(struct rconn *);
static void report_error(struct rconn *, int error);
static void disconnect(struct rconn *, int error);
static void flush_queue(struct rconn *);
+static void close_monitor(struct rconn *, size_t idx, int retval);
static void copy_to_monitor(struct rconn *, const struct ofpbuf *);
static bool is_connected_state(enum state);
static bool is_admitted_msg(const struct ofpbuf *);
@@ -1042,6 +1043,15 @@ state_transition(struct rconn *rc, enum state state)
}
static void
+close_monitor(struct rconn *rc, size_t idx, int retval)
+{
+VLOG_DBG("%s: closing monitor connection to %s: %s",
+ rconn_get_name(rc), vconn_get_name(rc->monitors[idx]),
+ ovs_retval_to_string(retval));
+rc->monitors[idx] = rc->monitors[--rc->n_monitors];
+}
+
+static void
copy_to_monitor(struct rconn *rc, const struct ofpbuf *b)
{
struct ofpbuf *clone = NULL;
@@ -1058,10 +1068,7 @@ copy_to_monitor(struct rconn *rc, const struct ofpbuf *b)
if (!retval) {
clone = NULL;
} else if (retval != EAGAIN) {
-VLOG_DBG("%s: closing monitor connection to %s: %s",
- rconn_get_name(rc), vconn_get_name(vconn),
- strerror(retval));
-rc->monitors[i] = rc->monitors[--rc->n_monitors];
+close_monitor(rc, i, retval);
continue;
}
i++;
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/3] ovs-ofctl: Make "ovs-ofctl monitor" respond to echo requests.
Otherwise the command will time out after a while when there's no traffic,
which probably isn't what we want.
Reported-by: Henry Mai
Signed-off-by: Ben Pfaff
---
utilities/ovs-ofctl.c | 34 +++---
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index a67a554..4f7dbbf 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1271,8 +1271,12 @@ ofctl_unblock(struct unixctl_conn *conn, int argc
OVS_UNUSED,
}
}
+/* Prints to stdout all of the messages received on 'vconn'.
+ *
+ * Iff 'reply_to_echo_requests' is true, sends a reply to any echo request
+ * received on 'vconn'. */
static void
-monitor_vconn(struct vconn *vconn)
+monitor_vconn(struct vconn *vconn, bool reply_to_echo_requests)
{
struct barrier_aux barrier_aux = { vconn, NULL };
struct unixctl_server *server;
@@ -1325,12 +1329,28 @@ monitor_vconn(struct vconn *vconn)
ofptype_decode(&type, b->data);
ofp_print(stderr, b->data, b->size, verbosity + 2);
-ofpbuf_delete(b);
-if (barrier_aux.conn && type == OFPTYPE_BARRIER_REPLY) {
-unixctl_command_reply(barrier_aux.conn, NULL);
-barrier_aux.conn = NULL;
+switch ((int) type) {
+case OFPTYPE_BARRIER_REPLY:
+if (barrier_aux.conn) {
+unixctl_command_reply(barrier_aux.conn, NULL);
+barrier_aux.conn = NULL;
+}
+break;
+
+case OFPTYPE_ECHO_REQUEST:
+if (reply_to_echo_requests) {
+struct ofpbuf *reply;
+
+reply = make_echo_reply(b->data);
+retval = vconn_send(vconn, reply);
+if (retval && retval != EAGAIN) {
+ovs_fatal(retval, "failed to send echo reply");
+}
+}
+break;
}
+ofpbuf_delete(b);
}
if (exiting) {
@@ -1400,7 +1420,7 @@ ofctl_monitor(int argc, char *argv[])
}
}
-monitor_vconn(vconn);
+monitor_vconn(vconn, true);
}
static void
@@ -1409,7 +1429,7 @@ ofctl_snoop(int argc OVS_UNUSED, char *argv[])
struct vconn *vconn;
open_vconn__(argv[1], "snoop", &vconn);
-monitor_vconn(vconn);
+monitor_vconn(vconn, false);
}
static void
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/3] rconn: Discard messages received on monitor connections.
Otherwise, if a monitor connection happens to be talking to a (misguided?)
peer that sends it messages, such as replies to what the peer perceives as
echo requests meant for it, then the peer will eventually hang trying to
send data because the monitor connection never sinks it.
Signed-off-by: Ben Pfaff
---
lib/rconn.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/rconn.c b/lib/rconn.c
index 5d7595f..8962fe2 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -497,8 +497,21 @@ rconn_run(struct rconn *rc)
if (rc->vconn) {
vconn_run(rc->vconn);
}
-for (i = 0; i < rc->n_monitors; i++) {
+for (i = 0; i < rc->n_monitors; ) {
+struct ofpbuf *msg;
+int retval;
+
vconn_run(rc->monitors[i]);
+
+/* Drain any stray message that came in on the monitor connection. */
+retval = vconn_recv(rc->monitors[i], &msg);
+if (!retval) {
+ofpbuf_delete(msg);
+} else if (retval != EAGAIN) {
+close_monitor(rc, i, retval);
+continue;
+}
+i++;
}
do {
@@ -529,6 +542,7 @@ rconn_run_wait(struct rconn *rc)
}
for (i = 0; i < rc->n_monitors; i++) {
vconn_run_wait(rc->monitors[i]);
+vconn_recv_wait(rc->monitors[i]);
}
timeo = timeout(rc);
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 04/18] ofp-util: Add version bitmap to hello messages
On Thu, Oct 18, 2012 at 02:58:04PM +0900, Simon Horman wrote: > Allow encoding and decoding of version bitmap in hello messages > as specified in Open Flow 1.3.1. > > Signed-off-by: Simon Horman Thanks for doing this. It looks like ofputil_decode_hello() only processes a single hello element, only if that one is an OFPHET_VERSIONBITMAP element. OF1.3.1 says: Implementations must ignore (skip) all elements of a Hello message that they do not support. so I'd be inclined to say that, instead, we should iterate over each element that is present, looking for an OFPHET_VERSIONBITMAP element and silently ignoring any others that we find. It looks like the declarations of ofputil_hello, ofputil_decode_hello(), and ofputil_encode_hello() should be moved from patch 2 to this patch, because I don't see any reference to them before this patch. I'm not certain that ofputil_hello actually needs the 'version' member. To decode a hello message without a bitmap, one can initialize the bitmap as every version up to the one specified; to decode a hello message with a bitmap, one initializes the bitmap from the included bitmap. To encode a hello message given only a bitmap is equally straightforward. (And if I'm right about all that, then maybe we don't need a struct at all--perhaps a single uint32_t bitmap is sufficient?) Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Linux Kernel 3.6.x and OVS
Hi Kyle, I have patches for 3.6 support. I will send it out after 1.8 release. Thanks, Pravin. On Wed, Oct 24, 2012 at 9:19 AM, Kyle Mestery (kmestery) wrote: > Is anyone working on getting this kernel working with OVS? I've started > looking at this, as Fedora has moved to a 3.6.2 kernel, and it looks like the > routing code has changed such that the tunneling code in OVS needs an update. > I can continue to proceed down this path unless someone else is already aware > of this and working on it. > > Thanks, > Kyle > ___ > 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] Linux Kernel 3.6.x and OVS
On Oct 24, 2012, at 12:59 PM, Pravin Shelar wrote: > Hi Kyle, > I have patches for 3.6 support. I will send it out after 1.8 release. > Any chance you can send out the diff now? I'd like to run with it if it's ok with you. Thanks! Kyle > Thanks, > Pravin. > > On Wed, Oct 24, 2012 at 9:19 AM, Kyle Mestery (kmestery) > wrote: >> Is anyone working on getting this kernel working with OVS? I've started >> looking at this, as Fedora has moved to a 3.6.2 kernel, and it looks like >> the routing code has changed such that the tunneling code in OVS needs an >> update. I can continue to proceed down this path unless someone else is >> already aware of this and working on it. >> >> Thanks, >> Kyle >> ___ >> 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] Linux Kernel 3.6.x and OVS
Actually, is there any reason these can't go in before 1.8 release? If not, 1.8 won't work with the latest Fedora, for example, without your patch. Thanks, Kyle On Oct 24, 2012, at 12:59 PM, Pravin Shelar wrote: > Hi Kyle, > I have patches for 3.6 support. I will send it out after 1.8 release. > > Thanks, > Pravin. > > On Wed, Oct 24, 2012 at 9:19 AM, Kyle Mestery (kmestery) > wrote: >> Is anyone working on getting this kernel working with OVS? I've started >> looking at this, as Fedora has moved to a 3.6.2 kernel, and it looks like >> the routing code has changed such that the tunneling code in OVS needs an >> update. I can continue to proceed down this path unless someone else is >> already aware of this and working on it. >> >> Thanks, >> Kyle >> ___ >> 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] Linux Kernel 3.6.x and OVS
On Wed, Oct 24, 2012 at 11:02 AM, Kyle Mestery (kmestery) wrote: > On Oct 24, 2012, at 12:59 PM, Pravin Shelar wrote: >> Hi Kyle, >> I have patches for 3.6 support. I will send it out after 1.8 release. >> > Any chance you can send out the diff now? I'd like to run with it if it's ok > with you. > OK, I need to finish few tests with the patches, then I can send it out. may be by end of day today I will send it out. Thanks, Pravin. > Thanks! > Kyle > >> Thanks, >> Pravin. >> >> On Wed, Oct 24, 2012 at 9:19 AM, Kyle Mestery (kmestery) >> wrote: >>> Is anyone working on getting this kernel working with OVS? I've started >>> looking at this, as Fedora has moved to a 3.6.2 kernel, and it looks like >>> the routing code has changed such that the tunneling code in OVS needs an >>> update. I can continue to proceed down this path unless someone else is >>> already aware of this and working on it. >>> >>> Thanks, >>> Kyle >>> ___ >>> 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] [1.4 backports 0/6] backports of datapaths fixes to 1.4
Yesterday, while going through datapath changes on master, I noticed that some bugfixes seem to be relevant and missing on master. Here is a series of backports for review. Thanks, Ben. Ansis Atteka (1): datapath: Release rtnl_lock if ovs_vport_cmd_build_info() failed Ben Pfaff (1): datapath: Honor dp_ifindex, when specified, for vport lookup by name. Jesse Gross (2): datapath: Fix checksum update for actions on UDP packets. flow: Add length check when retrieving TCP flags. Pravin B Shelar (2): datapath: Move CSUM_MANGLED_0 definition to net checksum header. datapath: Fix Tunnel options TOS datapath/actions.c | 45 +-- datapath/datapath.c|8 +++-- datapath/flow.c|3 +- datapath/linux/Modules.mk |1 + datapath/linux/compat/include/linux/checksum.h | 10 + datapath/linux/compat/include/net/checksum.h |4 ++ datapath/tunnel.c | 20 ++ lib/dpif-netdev.c |3 +- vswitchd/vswitch.xml |3 +- 9 files changed, 71 insertions(+), 26 deletions(-) create mode 100644 datapath/linux/compat/include/linux/checksum.h -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [1.4 backports 1/6] datapath: Honor dp_ifindex, when specified, for vport lookup by name.
When OVS_VPORT_ATTR_NAME is specified and dp_ifindex is nonzero, the
logical behavior would be for the vport name lookup scope to be limited
to the specified datapath, but in fact the dp_ifindex value was ignored.
This commit causes the search scope to be honored.
This is a crossport of commit 24ce832d5e076e5686b15d2aadd39e8c0818e932
from master.
Bug #9889.
Signed-off-by: Ben Pfaff
Acked-by: Pravin B Shelar
Acked-by: Jesse Gross
---
datapath/datapath.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 4ee8f86..11a74da 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1693,6 +1693,9 @@ static struct vport *lookup_vport(struct ovs_header
*ovs_header,
vport = ovs_vport_locate(nla_data(a[OVS_VPORT_ATTR_NAME]));
if (!vport)
return ERR_PTR(-ENODEV);
+ if (ovs_header->dp_ifindex &&
+ ovs_header->dp_ifindex != get_dpifindex(vport->dp))
+ return ERR_PTR(-ENODEV);
return vport;
} else if (a[OVS_VPORT_ATTR_PORT_NO]) {
u32 port_no = nla_get_u32(a[OVS_VPORT_ATTR_PORT_NO]);
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [1.4 backports 2/6] datapath: Move CSUM_MANGLED_0 definition to net checksum header.
From: Pravin B Shelar Following patch fixes compilation error on older kernel. This is a crossport of commit 08d19ca9fef29b23826f1fb52e2368a9077783ca from master. Signed-off-by: Pravin B Shelar Acked-by: Jesse Gross --- datapath/linux/compat/include/net/checksum.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/datapath/linux/compat/include/net/checksum.h b/datapath/linux/compat/include/net/checksum.h index ee64f24..502d02d 100644 --- a/datapath/linux/compat/include/net/checksum.h +++ b/datapath/linux/compat/include/net/checksum.h @@ -38,4 +38,8 @@ static inline void csum_replace2(__sum16 *sum, __be16 from, __be16 to) (__force __be32)(to), pseudohdr) #endif +#ifndef CSUM_MANGLED_0 +#define CSUM_MANGLED_0 ((__force __sum16)0x) +#endif + #endif /* checksum.h */ -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [1.4 backports 3/6] datapath: Fix checksum update for actions on UDP packets.
From: Jesse Gross
When modifying IP addresses or ports on a UDP packet we don't
correctly follow the rules for unchecksummed packets. This meant
that packets without a checksum can be given a incorrect new checksum
and packets with a checksum can become marked as being unchecksummed.
This fixes it to handle those requirements.
This is a crossport of commit 55ce87bcd542cc26def11000c9dee7690b7c3155
from master.
Bug #8937.
Signed-off-by: Jesse Gross
Acked-by: Ben Pfaff
---
datapath/actions.c | 45 +--
datapath/linux/Modules.mk |1 +
datapath/linux/compat/include/linux/checksum.h | 10 +
3 files changed, 44 insertions(+), 12 deletions(-)
create mode 100644 datapath/linux/compat/include/linux/checksum.h
diff --git a/datapath/actions.c b/datapath/actions.c
index 824791d..57e6bcb 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2007-2011 Nicira Networks.
+ * Copyright (c) 2007-2012 Nicira Networks.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General Public
@@ -147,9 +147,17 @@ static void set_ip_addr(struct sk_buff *skb, struct iphdr
*nh,
inet_proto_csum_replace4(&tcp_hdr(skb)->check, skb,
*addr, new_addr, 1);
} else if (nh->protocol == IPPROTO_UDP) {
- if (likely(transport_len >= sizeof(struct udphdr)))
- inet_proto_csum_replace4(&udp_hdr(skb)->check, skb,
-*addr, new_addr, 1);
+ if (likely(transport_len >= sizeof(struct udphdr))) {
+ struct udphdr *uh = udp_hdr(skb);
+
+ if (uh->check ||
+ get_ip_summed(skb) == OVS_CSUM_PARTIAL) {
+ inet_proto_csum_replace4(&uh->check, skb,
+*addr, new_addr, 1);
+ if (!uh->check)
+ uh->check = CSUM_MANGLED_0;
+ }
+ }
}
csum_replace4(&nh->check, *addr, new_addr);
@@ -199,8 +207,22 @@ static void set_tp_port(struct sk_buff *skb, __be16 *port,
skb_clear_rxhash(skb);
}
-static int set_udp_port(struct sk_buff *skb,
- const struct ovs_key_udp *udp_port_key)
+static void set_udp_port(struct sk_buff *skb, __be16 *port, __be16 new_port)
+{
+ struct udphdr *uh = udp_hdr(skb);
+
+ if (uh->check && get_ip_summed(skb) != OVS_CSUM_PARTIAL) {
+ set_tp_port(skb, port, new_port, &uh->check);
+
+ if (!uh->check)
+ uh->check = CSUM_MANGLED_0;
+ } else {
+ *port = new_port;
+ skb_clear_rxhash(skb);
+ }
+}
+
+static int set_udp(struct sk_buff *skb, const struct ovs_key_udp *udp_port_key)
{
struct udphdr *uh;
int err;
@@ -212,16 +234,15 @@ static int set_udp_port(struct sk_buff *skb,
uh = udp_hdr(skb);
if (udp_port_key->udp_src != uh->source)
- set_tp_port(skb, &uh->source, udp_port_key->udp_src,
&uh->check);
+ set_udp_port(skb, &uh->source, udp_port_key->udp_src);
if (udp_port_key->udp_dst != uh->dest)
- set_tp_port(skb, &uh->dest, udp_port_key->udp_dst, &uh->check);
+ set_udp_port(skb, &uh->dest, udp_port_key->udp_dst);
return 0;
}
-static int set_tcp_port(struct sk_buff *skb,
- const struct ovs_key_tcp *tcp_port_key)
+static int set_tcp(struct sk_buff *skb, const struct ovs_key_tcp *tcp_port_key)
{
struct tcphdr *th;
int err;
@@ -334,11 +355,11 @@ static int execute_set_action(struct sk_buff *skb,
break;
case OVS_KEY_ATTR_TCP:
- err = set_tcp_port(skb, nla_data(nested_attr));
+ err = set_tcp(skb, nla_data(nested_attr));
break;
case OVS_KEY_ATTR_UDP:
- err = set_udp_port(skb, nla_data(nested_attr));
+ err = set_udp(skb, nla_data(nested_attr));
break;
}
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 1f9973b..baaebb0 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -10,6 +10,7 @@ openvswitch_sources += \
linux/compat/skbuff-openvswitch.c \
linux/compat/time.c
openvswitch_headers += \
+ linux/compat/include/linux/checksum.h \
linux/compat/include/linux/compiler.h \
linux/compat/include/linux/compiler-gcc.h \
linux/compat/include/linux/cpumask.h \
diff --git a/datapath/linux/compat/include/linux/checksum.h
b/datapath/linux/compat/include/linux/checksum.h
new file mode 100644
index 000..1d4fefc
--- /dev/null
+++ b/datapath/l
[ovs-dev] [1.4 backports 4/6] flow: Add length check when retrieving TCP flags.
From: Jesse Gross
When collecting TCP flags we check that the IP header indicates that
a TCP header is present but not that the packet is actually long
enough to contain the header. This adds a check to prevent reading
off the end of the packet.
In practice, this is only likely to result in reading of bad data and
not a crash due to the presence of struct skb_shared_info at the end
of the packet.
This is a crossport of commit 9c47b45a3bb56009bf2553c493d097eeadd7e5c2
from master.
Signed-off-by: Jesse Gross
Acked-by: Pravin B Shelar
---
datapath/flow.c |3 ++-
lib/dpif-netdev.c |3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/datapath/flow.c b/datapath/flow.c
index c6f591a..06df0f6 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -239,7 +239,8 @@ void ovs_flow_used(struct sw_flow *flow, struct sk_buff
*skb)
u8 tcp_flags = 0;
if (flow->key.eth.type == htons(ETH_P_IP) &&
- flow->key.ip.proto == IPPROTO_TCP) {
+ flow->key.ip.proto == IPPROTO_TCP &&
+ likely(skb->len >= skb_transport_offset(skb) + sizeof(struct
tcphdr))) {
u8 *tcp = (u8 *)tcp_hdr(skb);
tcp_flags = *(tcp + TCP_FLAGS_OFFSET) & TCP_FLAG_MASK;
}
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 67b5189..0f93f96 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -987,7 +987,8 @@ dp_netdev_flow_used(struct dp_netdev_flow *flow, struct
flow *key,
flow->used = time_msec();
flow->packet_count++;
flow->byte_count += packet->size;
-if (key->dl_type == htons(ETH_TYPE_IP) && key->nw_proto == IPPROTO_TCP) {
+if (key->dl_type == htons(ETH_TYPE_IP) &&
+key->nw_proto == IPPROTO_TCP && packet->l7) {
struct tcp_header *th = packet->l4;
flow->tcp_ctl |= th->tcp_ctl;
}
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [1.4 backports 5/6] datapath: Release rtnl_lock if ovs_vport_cmd_build_info() failed
From: Ansis Atteka
This patch fixes a possible lock-up bug where rtnl_lock might not
get released.
This is a crossport of commit 7a6c067d1ad65ae4abdb723b25a4ab591d1d2bc3
from master.
Acked-by: Jesse Gross
Signed-off-by: Ansis Atteka
---
datapath/datapath.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 11a74da..c030cd2 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1847,10 +1847,9 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct
genl_info *info)
reply = ovs_vport_cmd_build_info(vport, info->snd_pid, info->snd_seq,
OVS_VPORT_CMD_NEW);
if (IS_ERR(reply)) {
- err = PTR_ERR(reply);
netlink_set_err(INIT_NET_GENL_SOCK, 0,
- ovs_dp_vport_multicast_group.id, err);
- return 0;
+ ovs_dp_vport_multicast_group.id,
PTR_ERR(reply));
+ goto exit_unlock;
}
genl_notify(reply, genl_info_net(info), info->snd_pid,
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [1.4 backports 6/6] datapath: Fix Tunnel options TOS
From: Pravin B Shelar
Use DSCP bits from ToS set on tunnel.
This is a crossport of commit 749ae9504293dbb695dd67402acbd47acbcbeb83
from master.
Bug #8822.
Signed-off-by: Pravin B Shelar
Acked-by: Jesse Gross
---
datapath/tunnel.c| 20
vswitchd/vswitch.xml |3 ++-
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index 4ce830f..1ebbc77 100644
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -991,12 +991,15 @@ unlock:
static struct rtable *__find_route(const struct tnl_mutable_config *mutable,
u8 ipproto, u8 tos)
{
+ /* Tunnel configuration keeps DSCP part of TOS bits, But Linux
+* router expect RT_TOS bits only. */
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,39)
struct flowi fl = { .nl_u = { .ip4_u = {
.daddr = mutable->key.daddr,
.saddr = mutable->key.saddr,
- .tos = tos } },
- .proto = ipproto };
+ .tos = RT_TOS(tos) } },
+ .proto = ipproto };
struct rtable *rt;
if (unlikely(ip_route_output_key(&init_net, &rt, &fl)))
@@ -1006,7 +1009,7 @@ static struct rtable *__find_route(const struct
tnl_mutable_config *mutable,
#else
struct flowi4 fl = { .daddr = mutable->key.daddr,
.saddr = mutable->key.saddr,
-.flowi4_tos = tos,
+.flowi4_tos = RT_TOS(tos),
.flowi4_proto = ipproto };
return ip_route_output_key(&init_net, &fl);
@@ -1023,7 +1026,7 @@ static struct rtable *find_route(struct vport *vport,
*cache = NULL;
tos = RT_TOS(tos);
- if (likely(tos == mutable->tos &&
+ if (likely(tos == RT_TOS(mutable->tos) &&
check_cache_valid(cur_cache, mutable))) {
*cache = cur_cache;
return cur_cache->rt;
@@ -1034,7 +1037,7 @@ static struct rtable *find_route(struct vport *vport,
if (IS_ERR(rt))
return NULL;
- if (likely(tos == mutable->tos))
+ if (likely(tos == RT_TOS(mutable->tos)))
*cache = build_cache(vport, mutable, rt);
return rt;
@@ -1208,8 +1211,6 @@ int ovs_tnl_send(struct vport *vport, struct sk_buff *skb)
else
tos = mutable->tos;
- tos = INET_ECN_encapsulate(tos, inner_tos);
-
/* Route lookup */
rt = find_route(vport, mutable, tos, &cache);
if (unlikely(!rt))
@@ -1217,6 +1218,8 @@ int ovs_tnl_send(struct vport *vport, struct sk_buff *skb)
if (unlikely(!cache))
unattached_dst = &rt_dst(rt);
+ tos = INET_ECN_encapsulate(tos, inner_tos);
+
/* Reset SKB */
nf_reset(skb);
secpath_reset(skb);
@@ -1389,7 +1392,8 @@ static int tnl_set_config(struct nlattr *options, const
struct tnl_ops *tnl_ops,
if (a[OVS_TUNNEL_ATTR_TOS]) {
mutable->tos = nla_get_u8(a[OVS_TUNNEL_ATTR_TOS]);
- if (mutable->tos != RT_TOS(mutable->tos))
+ /* Reject ToS config with ECN bits set. */
+ if (mutable->tos & INET_ECN_MASK)
return -EINVAL;
}
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index ebbcbe7..303a98d 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1259,7 +1259,8 @@
Optional. The value of the ToS bits to be set on the encapsulating
-packet. It may also be the word inherit, in which case
+packet. ToS is interpreted as DSCP and ECN bits, ECN part must be
+zero. It may also be the word inherit, in which case
the ToS will be copied from the inner packet if it is IPv4 or IPv6
(otherwise it will be 0). The ECN fields are always inherited.
Default is 0.
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] userspace datapath performance (was: Re: Threaded userspace datapath)
On Thu, Oct 11, 2012 at 01:13:44AM +0200, Luigi Rizzo wrote: > Anyways, let's see if we can find some agreement on how to > proceed with a face-to-face meeting Will you attend the ONF workday next week Tuesday in Santa Clara? It is free to all ONF members (Google is one, so that's probably good enough). I will be there and I'm happy to talk this over. Details: http://onfmemberworkday.eventbrite.com ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovs-ctl.in: Don't save flows if the daemons are not running.
When a 'ovs-ctl restart' is executed and the userspace daemons
like ovsdb-server and ovs-vswitchd are not running, attempt to
save flows can wait forever. This also results in the daemons
from not getting started.
Signed-off-by: Gurucharan Shetty
---
utilities/ovs-ctl.in |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 9ce4973..e8b72ba 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -405,10 +405,12 @@ force_reload_kmod () {
## --- ##
restart () {
-script_flows=`mktemp`
-trap 'rm -f "${script_flows}"' 0 1 2 13 15
+if daemon_is_running ovsdb-server && daemon_is_running ovs-vswitchd; then
+script_flows=`mktemp`
+trap 'rm -f "${script_flows}"' 0 1 2 13 15
-action "Saving flows" save_flows
+action "Saving flows" save_flows
+fi
# Restart the database first, since a large database may take a
# while to load, and we want to minimize forwarding disruption.
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-ctl.in: Don't save flows if the daemons are not running.
On Wed, Oct 24, 2012 at 01:19:24PM -0700, Gurucharan Shetty wrote: > When a 'ovs-ctl restart' is executed and the userspace daemons > like ovsdb-server and ovs-vswitchd are not running, attempt to > save flows can wait forever. This also results in the daemons > from not getting started. > > Signed-off-by: Gurucharan Shetty Looks good, thanks. Perhaps we should have timeouts (-T or --timeout) on the program invocations that could hang. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto: Report 0 Mbps when speed not available instead of 100 Mbps.
When a link is down, or when a link has no speed because it is not a
physical interface, Open vSwitch previously reported that its rate is 100
Mbps as a default. This is counterintuitive, however, so this commit
changes Open vSwitch behavior to report 0 Mbps when a link is down or its
speed is otherwise unavailable.
Bug #13388.
Reported-by: Hiroshi Tanaka
Signed-off-by: Ben Pfaff
---
lib/netdev-linux.c |4 ++--
lib/netdev.c |7 ---
lib/netdev.h |3 ++-
lib/ofp-util.c |4 ++--
ofproto/ofproto-dpif-sflow.c |2 +-
ofproto/ofproto.c|4 ++--
tests/ofp-print.at |2 +-
tests/ofproto.at | 10 +-
vswitchd/bridge.c| 18 ++
9 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 412a92d..0460c06 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2668,7 +2668,7 @@ htb_parse_qdisc_details__(struct netdev *netdev,
enum netdev_features current;
netdev_get_features(netdev, ¤t, NULL, NULL, NULL);
-hc->max_rate = netdev_features_to_bps(current) / 8;
+hc->max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
}
hc->min_rate = hc->max_rate;
hc->burst = 0;
@@ -3147,7 +3147,7 @@ hfsc_parse_qdisc_details__(struct netdev *netdev, const
struct smap *details,
enum netdev_features current;
netdev_get_features(netdev, ¤t, NULL, NULL, NULL);
-max_rate = netdev_features_to_bps(current) / 8;
+max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
}
class->min_rate = max_rate;
diff --git a/lib/netdev.c b/lib/netdev.c
index c135c6f..1921ac0 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -620,9 +620,10 @@ netdev_get_features(const struct netdev *netdev,
/* Returns the maximum speed of a network connection that has the NETDEV_F_*
* bits in 'features', in bits per second. If no bits that indicate a speed
- * are set in 'features', assumes 100Mbps. */
+ * are set in 'features', returns 'default_bps'. */
uint64_t
-netdev_features_to_bps(enum netdev_features features)
+netdev_features_to_bps(enum netdev_features features,
+ uint64_t default_bps)
{
enum {
F_100MB = NETDEV_F_1TB_FD,
@@ -641,7 +642,7 @@ netdev_features_to_bps(enum netdev_features features)
: features & F_1000MB? UINT64_C(10)
: features & F_100MB ? UINT64_C(1)
: features & F_10MB ? UINT64_C(1000)
- : UINT64_C(1));
+ : default_bps);
}
/* Returns true if any of the NETDEV_F_* bits that indicate a full-duplex link
diff --git a/lib/netdev.h b/lib/netdev.h
index d2cc8b5..67122ee 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -146,7 +146,8 @@ int netdev_get_features(const struct netdev *,
enum netdev_features *advertised,
enum netdev_features *supported,
enum netdev_features *peer);
-uint64_t netdev_features_to_bps(enum netdev_features features);
+uint64_t netdev_features_to_bps(enum netdev_features features,
+uint64_t default_bps);
bool netdev_features_is_full_duplex(enum netdev_features features);
int netdev_set_advertisements(struct netdev *, enum netdev_features advertise);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 34255da..ad59a34 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2420,8 +2420,8 @@ ofputil_decode_ofp10_phy_port(struct ofputil_phy_port *pp,
pp->supported = netdev_port_features_from_ofp10(opp->supported);
pp->peer = netdev_port_features_from_ofp10(opp->peer);
-pp->curr_speed = netdev_features_to_bps(pp->curr) / 1000;
-pp->max_speed = netdev_features_to_bps(pp->supported) / 1000;
+pp->curr_speed = netdev_features_to_bps(pp->curr, 0) / 1000;
+pp->max_speed = netdev_features_to_bps(pp->supported, 0) / 1000;
return 0;
}
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 23f5498..37b662c 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -179,7 +179,7 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller,
if (!netdev_get_features(dsp->ofport->netdev, ¤t, NULL, NULL, NULL))
{
/* The values of ifDirection come from MAU MIB (RFC 2668): 0 = unknown,
1 = full-duplex, 2 = half-duplex, 3 = in, 4=out */
-counters->ifSpeed = netdev_features_to_bps(current);
+counters->ifSpeed = netdev_features_to_bps(current, 0);
counters->ifDirection = (netdev_features_is_full_duplex(current)
? 1 : 2);
} else {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 2fb2fc8..0979f70 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1
[ovs-dev] [PATCH 1/3] timeval: Take a backtrace on each SIGALRM.
With this patch, timeval will take a backtrace with each SIGALRM
allowing it to retrieve a profiling snapshot instantly. This will
be useful in future patches when backtraces are logged.
Signed-off-by: Ethan Jackson
---
lib/timeval.c | 68 ++---
1 file changed, 26 insertions(+), 42 deletions(-)
diff --git a/lib/timeval.c b/lib/timeval.c
index 9029faa..e507140 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -75,9 +75,8 @@ struct trace {
};
#define MAX_TRACES 50
-static struct unixctl_conn *backtrace_conn = NULL;
-static struct trace *traces = NULL;
-static size_t n_traces = 0;
+static struct trace traces[MAX_TRACES];
+static size_t trace_head = 0;
static void set_up_timer(void);
static void set_up_signal(int flags);
@@ -91,7 +90,6 @@ static struct rusage *get_recent_rusage(void);
static void refresh_rusage(void);
static void timespec_add(struct timespec *sum,
const struct timespec *a, const struct timespec *b);
-static void trace_run(void);
static unixctl_cb_func backtrace_cb;
/* Initializes the timetracking module, if not already initialized. */
@@ -100,16 +98,13 @@ time_init(void)
{
static bool inited;
-/* The best place to do this is probably a timeval_run() function.
- * However, none exists and this function is usually so fast that doing it
- * here seems fine for now. */
-trace_run();
-
if (inited) {
return;
}
inited = true;
+memset(traces, 0, sizeof traces);
+
if (HAVE_EXECINFO_H && CACHE_TIME) {
unixctl_command_register("backtrace", "", 0, 0, backtrace_cb, NULL);
}
@@ -377,7 +372,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long
int timeout_when,
break;
}
-if (!blocked && CACHE_TIME && !backtrace_conn) {
+if (!blocked && CACHE_TIME) {
block_sigalrm(&oldsigs);
blocked = true;
}
@@ -398,10 +393,12 @@ sigalrm_handler(int sig_nr OVS_UNUSED)
monotonic_tick = true;
#if HAVE_EXECINFO_H
-if (backtrace_conn && n_traces < MAX_TRACES) {
-struct trace *trace = &traces[n_traces++];
+if (CACHE_TIME) {
+struct trace *trace = &traces[trace_head];
+
trace->n_frames = backtrace(trace->backtrace,
ARRAY_SIZE(trace->backtrace));
+trace_head = (trace_head + 1) % MAX_TRACES;
}
#endif
}
@@ -581,42 +578,38 @@ get_cpu_usage(void)
}
static void
-trace_run(void)
+format_backtraces(struct ds *ds)
{
+time_init();
+
#if HAVE_EXECINFO_H
-if (backtrace_conn && n_traces >= MAX_TRACES) {
-struct unixctl_conn *reply_conn = backtrace_conn;
-struct ds ds = DS_EMPTY_INITIALIZER;
+if (CACHE_TIME) {
sigset_t oldsigs;
size_t i;
block_sigalrm(&oldsigs);
-for (i = 0; i < n_traces; i++) {
+for (i = 0; i < MAX_TRACES; i++) {
struct trace *trace = &traces[i];
char **frame_strs;
size_t j;
+if (!trace->n_frames) {
+continue;
+}
+
frame_strs = backtrace_symbols(trace->backtrace, trace->n_frames);
-ds_put_format(&ds, "Backtrace %zu\n", i + 1);
+ds_put_format(ds, "Backtrace %zu\n", i + 1);
for (j = 0; j < trace->n_frames; j++) {
-ds_put_format(&ds, "%s\n", frame_strs[j]);
+ds_put_format(ds, "%s\n", frame_strs[j]);
}
-ds_put_cstr(&ds, "\n");
+ds_put_cstr(ds, "\n");
free(frame_strs);
}
-
-free(traces);
-traces = NULL;
-n_traces = 0;
-backtrace_conn = NULL;
-
+ds_chomp(ds, '\n');
unblock_sigalrm(&oldsigs);
-
-unixctl_command_reply(reply_conn, ds_cstr(&ds));
-ds_destroy(&ds);
}
#endif
}
@@ -664,21 +657,12 @@ backtrace_cb(struct unixctl_conn *conn,
int argc OVS_UNUSED, const char *argv[] OVS_UNUSED,
void *aux OVS_UNUSED)
{
-sigset_t oldsigs;
+struct ds ds = DS_EMPTY_INITIALIZER;
assert(HAVE_EXECINFO_H && CACHE_TIME);
-
-if (backtrace_conn) {
-unixctl_command_reply_error(conn, "In Use");
-return;
-}
-assert(!traces);
-
-block_sigalrm(&oldsigs);
-backtrace_conn = conn;
-traces = xmalloc(MAX_TRACES * sizeof *traces);
-n_traces = 0;
-unblock_sigalrm(&oldsigs);
+format_backtraces(&ds);
+unixctl_command_reply(conn, ds_cstr(&ds));
+ds_destroy(&ds);
}
void
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/3] timeval: Coalesce backtraces with counts.
With this patch, `ovs-appctl backtrace` will return a unique list
of backtraces and a count of how many times it has been recorded.
This work had previously been done by ovs-parse-backtrace. However,
in future patches poll-loop will begin logging backtraces as a
matter of course. At this point, coalescing the backtraces will
help keep these log messages brief.
Signed-off-by: Ethan Jackson
---
lib/timeval.c| 53 +++---
utilities/ovs-parse-backtrace.in | 26 ---
2 files changed, 61 insertions(+), 18 deletions(-)
diff --git a/lib/timeval.c b/lib/timeval.c
index e507140..bd2a84d 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -32,6 +32,8 @@
#include "dummy.h"
#include "dynamic-string.h"
#include "fatal-signal.h"
+#include "hash.h"
+#include "hmap.h"
#include "signals.h"
#include "unixctl.h"
#include "util.h"
@@ -72,6 +74,10 @@ static long long int deadline = LLONG_MAX;
struct trace {
void *backtrace[32]; /* Populated by backtrace(). */
size_t n_frames; /* Number of frames in 'backtrace'. */
+
+/* format_backtraces() helper data. */
+struct hmap_node node;
+size_t count;
};
#define MAX_TRACES 50
@@ -577,6 +583,29 @@ get_cpu_usage(void)
return cpu_usage;
}
+static uint32_t
+hash_trace(struct trace *trace)
+{
+return hash_bytes(trace->backtrace,
+ trace->n_frames * sizeof *trace->backtrace, 0);
+}
+
+static struct trace *
+trace_map_lookup(struct hmap *trace_map, struct trace *key)
+{
+struct trace *value;
+
+HMAP_FOR_EACH_WITH_HASH (value, node, hash_trace(key), trace_map) {
+if (key->n_frames == value->n_frames
+&& !memcmp(key->backtrace, value->backtrace,
+ key->n_frames * sizeof *key->backtrace)) {
+return value;
+}
+}
+return NULL;
+}
+
+
static void
format_backtraces(struct ds *ds)
{
@@ -584,6 +613,8 @@ format_backtraces(struct ds *ds)
#if HAVE_EXECINFO_H
if (CACHE_TIME) {
+struct hmap trace_map = HMAP_INITIALIZER(&trace_map);
+struct trace *trace, *next;
sigset_t oldsigs;
size_t i;
@@ -591,16 +622,30 @@ format_backtraces(struct ds *ds)
for (i = 0; i < MAX_TRACES; i++) {
struct trace *trace = &traces[i];
-char **frame_strs;
-size_t j;
+struct trace *map_trace;
if (!trace->n_frames) {
continue;
}
+map_trace = trace_map_lookup(&trace_map, trace);
+if (map_trace) {
+map_trace->count++;
+} else {
+hmap_insert(&trace_map, &trace->node, hash_trace(trace));
+trace->count = 1;
+}
+}
+
+HMAP_FOR_EACH_SAFE (trace, next, node, &trace_map) {
+char **frame_strs;
+size_t j;
+
+hmap_remove(&trace_map, &trace->node);
+
frame_strs = backtrace_symbols(trace->backtrace, trace->n_frames);
-ds_put_format(ds, "Backtrace %zu\n", i + 1);
+ds_put_format(ds, "Count %zu\n", trace->count);
for (j = 0; j < trace->n_frames; j++) {
ds_put_format(ds, "%s\n", frame_strs[j]);
}
@@ -608,6 +653,8 @@ format_backtraces(struct ds *ds)
free(frame_strs);
}
+hmap_destroy(&trace_map);
+
ds_chomp(ds, '\n');
unblock_sigalrm(&oldsigs);
}
diff --git a/utilities/ovs-parse-backtrace.in b/utilities/ovs-parse-backtrace.in
index 4f793be..350cbd9 100755
--- a/utilities/ovs-parse-backtrace.in
+++ b/utilities/ovs-parse-backtrace.in
@@ -73,23 +73,19 @@ result. Expected usage is for ovs-appctl backtrace to be
piped in.""")
print "Binary: %s\n" % binary
stdin = sys.stdin.read()
-trace_list = stdin.strip().split("\n\n")
-try:
-#Remove the first line from each trace.
-trace_list = [trace[(trace.index("\n") + 1):] for trace in trace_list]
-except ValueError:
-sys.stdout.write(stdin)
-sys.exit(1)
-
-trace_map = {}
-for trace in trace_list:
-trace_map[trace] = trace_map.get(trace, 0) + 1
-
-sorted_traces = sorted(trace_map.items(), key=(lambda x: x[1]),
- reverse=True)
-for trace, count in sorted_traces:
+traces = []
+for trace in stdin.strip().split("\n\n"):
lines = trace.splitlines()
+match = re.search(r'Count (\d+)', lines[0])
+if match:
+count = int(match.group(1))
+else:
+count = 0
+traces.append((lines[1:], count))
+traces = sorted(traces, key=(lambda x: x[1]), reverse=True)
+
+for lines, count in traces:
longest = max(len(l) for l in lines)
print "Backtrace Count: %d" % count
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman
[ovs-dev] [PATCH 3/3] poll-loop: Log backtraces when CPU usage is high.
Often when debugging Open vSwitch, one will see in the logs that
CPU usage has been high for some period of time, but it's totally
unclear why. In an attempt to remedy the situation, this patch
logs backtraces taken at regular intervals as a poor man's
profiling alternative.
Signed-off-by: Ethan Jackson
---
lib/poll-loop.c |6 ++
lib/timeval.c | 15 +++
lib/timeval.h |2 ++
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/lib/poll-loop.c b/lib/poll-loop.c
index 516cf13..23f0c22 100644
--- a/lib/poll-loop.c
+++ b/lib/poll-loop.c
@@ -157,6 +157,7 @@ poll_immediate_wake(const char *where)
static void
log_wakeup(const char *where, const struct pollfd *pollfd, int timeout)
{
+static struct vlog_rate_limit trace_rl = VLOG_RATE_LIMIT_INIT(1, 1);
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
enum vlog_level level;
int cpu_usage;
@@ -200,6 +201,11 @@ log_wakeup(const char *where, const struct pollfd *pollfd,
int timeout)
}
if (cpu_usage >= 0) {
ds_put_format(&s, " (%d%% CPU usage)", cpu_usage);
+
+if (!vlog_should_drop(THIS_MODULE, level, &trace_rl)) {
+ds_put_cstr(&s, "\nBacktraces:\n");
+format_backtraces(&s, 2);
+}
}
VLOG(level, "%s", ds_cstr(&s));
ds_destroy(&s);
diff --git a/lib/timeval.c b/lib/timeval.c
index bd2a84d..b3245ca 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -605,9 +605,12 @@ trace_map_lookup(struct hmap *trace_map, struct trace *key)
return NULL;
}
-
-static void
-format_backtraces(struct ds *ds)
+/* Appends a string to 'ds' representing backtraces recorded at regular
+ * intervals in the recent past. This information can be used to get a sense
+ * of what the process has been spending the majority of time doing. Will
+ * ommit any backtraces which have not occurred at least 'min_count' times. */
+void
+format_backtraces(struct ds *ds, size_t min_count)
{
time_init();
@@ -643,6 +646,10 @@ format_backtraces(struct ds *ds)
hmap_remove(&trace_map, &trace->node);
+if (trace->count < min_count) {
+continue;
+}
+
frame_strs = backtrace_symbols(trace->backtrace, trace->n_frames);
ds_put_format(ds, "Count %zu\n", trace->count);
@@ -707,7 +714,7 @@ backtrace_cb(struct unixctl_conn *conn,
struct ds ds = DS_EMPTY_INITIALIZER;
assert(HAVE_EXECINFO_H && CACHE_TIME);
-format_backtraces(&ds);
+format_backtraces(&ds, 0);
unixctl_command_reply(conn, ds_cstr(&ds));
ds_destroy(&ds);
}
diff --git a/lib/timeval.h b/lib/timeval.h
index 0f7d15c..5a7b6e2 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -25,6 +25,7 @@
extern "C" {
#endif
+struct ds;
struct pollfd;
struct timespec;
struct timeval;
@@ -83,6 +84,7 @@ long long int timeval_to_msec(const struct timeval *);
void xgettimeofday(struct timeval *);
int get_cpu_usage(void);
+void format_backtraces(struct ds *, size_t min_count);
long long int time_boot_msec(void);
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] xenserver, rhel, debian: Use ovs-ctl restart.
ovs-ctl has a new command called "restart" which
saves and restores the openflow flows on bridges.
Use that command from the init scripts when doing
a "restart --save-flows=yes".
Feature #13555.
Signed-off-by: Gurucharan Shetty
---
debian/openvswitch-switch.init | 13 +++--
rhel/etc_init.d_openvswitch | 13 +++--
xenserver/etc_init.d_openvswitch | 14 --
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/debian/openvswitch-switch.init b/debian/openvswitch-switch.init
index f650f87..c24dc0a 100755
--- a/debian/openvswitch-switch.init
+++ b/debian/openvswitch-switch.init
@@ -78,6 +78,15 @@ stop () {
ovs_ctl stop
}
+restart () {
+if [ "$1" = "--save-flows=yes" ]; then
+start restart
+else
+stop
+start
+fi
+}
+
case $1 in
start)
start
@@ -89,8 +98,8 @@ case $1 in
# The OVS daemons keep up-to-date.
;;
restart)
-stop
-start
+shift
+restart "$@"
;;
status)
ovs_ctl status
diff --git a/rhel/etc_init.d_openvswitch b/rhel/etc_init.d_openvswitch
index 85cf55d..3b2343f 100755
--- a/rhel/etc_init.d_openvswitch
+++ b/rhel/etc_init.d_openvswitch
@@ -62,6 +62,15 @@ stop () {
rm -f /var/lock/subsys/openvswitch
}
+restart () {
+if [ "$1" = "--save-flows=yes" ]; then
+start restart
+else
+stop
+start
+fi
+}
+
ovs_ctl=/usr/share/openvswitch/scripts/ovs-ctl
case $1 in
start)
@@ -71,8 +80,8 @@ case $1 in
stop
;;
restart)
-stop
-start
+shift
+restart "$@"
;;
reload|force-reload)
# Nothing to do.
diff --git a/xenserver/etc_init.d_openvswitch b/xenserver/etc_init.d_openvswitch
index 48fc174..d4a056d 100755
--- a/xenserver/etc_init.d_openvswitch
+++ b/xenserver/etc_init.d_openvswitch
@@ -109,6 +109,16 @@ stop () {
rm -f /var/lock/subsys/openvswitch
}
+restart () {
+if [ "$1" = "--save-flows=yes" ]; then
+stop_daemon ovs-xapi-sync
+start restart
+else
+stop
+start
+fi
+}
+
ovs_ctl=/usr/share/openvswitch/scripts/ovs-ctl
case $1 in
start)
@@ -118,8 +128,8 @@ case $1 in
stop
;;
restart)
-stop
-start
+shift
+restart "$@"
;;
reload|force-reload)
# The main OVS daemons keep up-to-date, but ovs-xapi-sync needs help.
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] debian: Use restart --save-flows=yes during postinst.
When debian package for openvswitch-switch is upgraded, restart the daemons using the "--save-flows=yes" flag. This will save the openflow flows in vswitchd and re-apply it after the upgrade. Feature #13555. Signed-off-by: Gurucharan Shetty --- debian/openvswitch-switch.postinst | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/debian/openvswitch-switch.postinst b/debian/openvswitch-switch.postinst index 7b9d7bc..548ce1e 100755 --- a/debian/openvswitch-switch.postinst +++ b/debian/openvswitch-switch.postinst @@ -49,6 +49,18 @@ esac OVS_MISSING_KMOD_OK=yes export OVS_MISSING_KMOD_OK -#DEBHELPER# +if [ -x "/etc/init.d/openvswitch-switch" ]; then +update-rc.d openvswitch-switch defaults >/dev/null + if [ -n "$2" ]; then + _dh_action="restart --save-flows=yes" + else + _dh_action=start + fi + if [ -x "`which invoke-rc.d 2>/dev/null`" ]; then + invoke-rc.d openvswitch-switch $_dh_action || exit $? + else + /etc/init.d/openvswitch-switch $_dh_action || exit $? + fi +fi exit 0 -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Allow processing of RARP packets.
On Tue, Oct 23, 2012 at 7:17 PM, Mehak Mahajan wrote: > With this commit, the datapath will process the ARP header for > RARP packets. It also fixes a bug whereby if the ARP opcode is > something other than ARP request or reply, the key_len is not > adjusted to include ARP info. > > Signed-off-by: Mehak Mahajan Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Allow processing of RARP packets.
Thanks for the review Jesse. ~ Mehak On Wed, Oct 24, 2012 at 3:19 PM, Jesse Gross wrote: > On Tue, Oct 23, 2012 at 7:17 PM, Mehak Mahajan > wrote: > > With this commit, the datapath will process the ARP header for > > RARP packets. It also fixes a bug whereby if the ARP opcode is > > something other than ARP request or reply, the key_len is not > > adjusted to include ARP info. > > > > Signed-off-by: Mehak Mahajan > > Acked-by: Jesse Gross > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] xenserver, rhel, debian: Use ovs-ctl restart.
On Wed, Oct 24, 2012 at 02:21:39PM -0700, Gurucharan Shetty wrote: > ovs-ctl has a new command called "restart" which > saves and restores the openflow flows on bridges. > Use that command from the init scripts when doing > a "restart --save-flows=yes". > > Feature #13555. > Signed-off-by: Gurucharan Shetty Won't --save-flows=yes generally be in $2? All of these look at $1, but I think that's "restart" (unless there's a "shift" I missed somewhere). ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] xenserver, rhel, debian: Use ovs-ctl restart.
On Wed, Oct 24, 2012 at 3:33 PM, Ben Pfaff wrote: > On Wed, Oct 24, 2012 at 02:21:39PM -0700, Gurucharan Shetty wrote: > > ovs-ctl has a new command called "restart" which > > saves and restores the openflow flows on bridges. > > Use that command from the init scripts when doing > > a "restart --save-flows=yes". > > > > Feature #13555. > > Signed-off-by: Gurucharan Shetty > > Won't --save-flows=yes generally be in $2? All of these look at $1, > but I think that's "restart" (unless there's a "shift" I missed > somewhere). > There is a "shift" right before the function call of "restart". Or I am understanding you incorrectly. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Fix zero key tunnels.
On Tue, Oct 23, 2012 at 6:42 PM, Pravin B Shelar wrote: > Datapath tunneling check for flag OVS_FLOW_TNL_F_KEY is failing, > causing it to drop packet. This only happens on tunnels with > zero key as vswitchd does not generate set-tunnel action. Set > tunnel action sets this flags for given action. To fix this issue > the check is dropped. > > Bug #13666 > > Signed-off-by: Pravin B Shelar Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] debian: Use restart --save-flows=yes during postinst.
On Wed, Oct 24, 2012 at 02:21:40PM -0700, Gurucharan Shetty wrote: > When debian package for openvswitch-switch is upgraded, > restart the daemons using the "--save-flows=yes" flag. > This will save the openflow flows in vswitchd and > re-apply it after the upgrade. > > Feature #13555. > Signed-off-by: Gurucharan Shetty I think that _dh_* is supposed to be reserved namespace for use by debhelper, so I would remove the _dh_ prefix. I don't think it's a good idea to remove #DEBEHELPER#, because debhelper does more than just restart the daemon. Maybe that's all it does in this postinst script now (did you check?), but it could do more in the future, in which case we'd end up with surprising problems. Actually we already ran into related problems with init scripts, see commit 8a5b3cfd91841c97fbc8a003857cacbd602646ed: debian: Use a different way to avoid failing install without kernel module. The dh_installinit --error-handler option makes a lot of sense, but after playing with it for a while I could not figure out a nice way to use it only for openvswitch-switch without either duplicating the dh_installinit fragments in postinst and prerm (the actual bug that was reported) or omitting them for some package. Also, we forgot to write the error handler function for the prerm. This commit switches to a different way to avoid failing the install when the kernel module is not available, without using --error-handler. CC: 663...@bugs.debian.org Reported-by: Thomas Goirand Reviewed-by: Simon Horman Signed-off-by: Ben Pfaff Looking at the solution we used there, it might be easiest to, instead of using a command line option, to use an environment variable and then put above #DEBEHELPER# the commands to set and export that variable, like we do with OVS_MISSING_KMOD_OK. Sorry about all the trouble. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] xenserver, rhel, debian: Use ovs-ctl restart.
On Wed, Oct 24, 2012 at 03:35:51PM -0700, Gurucharan Shetty wrote: > On Wed, Oct 24, 2012 at 3:33 PM, Ben Pfaff wrote: > > > On Wed, Oct 24, 2012 at 02:21:39PM -0700, Gurucharan Shetty wrote: > > > ovs-ctl has a new command called "restart" which > > > saves and restores the openflow flows on bridges. > > > Use that command from the init scripts when doing > > > a "restart --save-flows=yes". > > > > > > Feature #13555. > > > Signed-off-by: Gurucharan Shetty > > > > Won't --save-flows=yes generally be in $2? All of these look at $1, > > but I think that's "restart" (unless there's a "shift" I missed > > somewhere). > > > There is a "shift" right before the function call of "restart". Or I am > understanding you incorrectly. How did I miss that? Thanks, this looks good (but please read my thoughts on 2/2 before you push it, since another strategy might be better for other reasons). ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] Prepare for 1.9.0.
Signed-off-by: Ben Pfaff --- This will be committed to master and be the branch point for a new branch-1.9 branch also. NEWS |4 ++-- configure.ac |2 +- debian/changelog | 43 --- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index f5d7f9e..d04db25 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,5 @@ -post-v1.8.0 - +v1.9.0 - xx xxx + - The tunneling code no longer assumes input and output keys are symmetric. If they are not, PMTUD needs to be disabled for tunneling to work. Note this only applies to flow-based keys. diff --git a/configure.ac b/configure.ac index 2b20f40..060b53f 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 1.8.90, ovs-b...@openvswitch.org) +AC_INIT(openvswitch, 1.9.0, 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 7b8bedb..bf45202 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,9 +1,46 @@ -openvswitch (1.8.90-1) unstable; urgency=low +openvswitch (1.9.0-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- Nothing yet! Try NEWS... - - -- Open vSwitch team Mon, 07 May 2012 14:14:52 +0900 +- The tunneling code no longer assumes input and output keys are symmetric. + If they are not, PMTUD needs to be disabled for tunneling to work. Note + this only applies to flow-based keys. +- FreeBSD is now a supported platform, thanks to code contributions from + Gaetano Catalli, Ed Maste, and Giuseppe Lettieri. +- ovs-bugtool: New --ovs option to report only OVS related information. +- New %t and %T log escapes to identify the subprogram within a + cooperating group of processes or threads that emitted a log message. + The default log patterns now include this information. +- OpenFlow: + - Allow bitwise masking for SHA and THA fields in ARP, SLL and TLL +fields in IPv6 neighbor discovery messages, and IPv6 flow label. + - Adds support for writing to the metadata field for a flow. +- ovs-ofctl: + - Commands and actions that accept port numbers now also accept keywords +that represent those ports (such as LOCAL, NONE, and ALL). This is +also the recommended way to specify these ports, for compatibility +with OpenFlow 1.1 and later (which use the OpenFlow 1.0 numbers +for these ports for different purposes). +- ovs-dpctl: + - Support requesting the port number with the "port_no" option in +the "add-if" command. +- ovs-pki: The "online PKI" features have been removed, along with + the ovs-pki-cgi program that facilitated it, because of some + alarmist insecurity claims. We do not believe that these claims + are true, but because we do not know of any users for this + feature it seems better on balance to remove it. (The ovs-pki-cgi + program was not included in distribution packaging.) +- ovsdb-server now enforces the immutability of immutable columns. This + was not enforced in earlier versions due to an oversight. +- New support for a nonstandard form of GRE that supports a 64-bit key. +- The following features are now deprecated. They will be removed no + earlier than February 2013. Please email dev@openvswitch.org with + concerns. +- Stable bond mode. +- The autopath action. +- Interface type "null". +- Numeric values for reserved ports (see "ovs-ofctl" note above). + + -- Open vSwitch team Wed, 24 Oct 2012 16:10:39 -0700 openvswitch (1.8.0-1) unstable; urgency=low [ Open vSwitch team ] -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] Prepare for post-1.9.0 (1.9.90).
Signed-off-by: Ben Pfaff --- This will go to master only, not to branch-1.9. NEWS |4 configure.ac |2 +- debian/changelog |7 +++ 3 files changed, 12 insertions(+), 1 deletions(-) diff --git a/NEWS b/NEWS index d04db25..3cb1bd3 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +post-v1.9.0 + + + v1.9.0 - xx xxx - The tunneling code no longer assumes input and output keys are symmetric. diff --git a/configure.ac b/configure.ac index 060b53f..32940a5 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.64) -AC_INIT(openvswitch, 1.9.0, ovs-b...@openvswitch.org) +AC_INIT(openvswitch, 1.9.90, 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 bf45202..9ac3d5d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +openvswitch (1.9.90-1) unstable; urgency=low + [ Open vSwitch team ] + * New upstream version + - Nothing yet! Try NEWS... + + -- Open vSwitch team Wed, 24 Oct 2012 16:12:57 -0700 + openvswitch (1.9.0-1) unstable; urgency=low [ Open vSwitch team ] * New upstream version -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] Prepare for post-1.9.0 (1.9.90).
The series looks good to me. --Justin On Oct 25, 2012, at 12:18 AM, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff > --- > This will go to master only, not to branch-1.9. > > NEWS |4 > configure.ac |2 +- > debian/changelog |7 +++ > 3 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/NEWS b/NEWS > index d04db25..3cb1bd3 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,7 @@ > +post-v1.9.0 > + > + > + > v1.9.0 - xx xxx > > - The tunneling code no longer assumes input and output keys are > symmetric. > diff --git a/configure.ac b/configure.ac > index 060b53f..32940a5 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -13,7 +13,7 @@ > # limitations under the License. > > AC_PREREQ(2.64) > -AC_INIT(openvswitch, 1.9.0, ovs-b...@openvswitch.org) > +AC_INIT(openvswitch, 1.9.90, 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 bf45202..9ac3d5d 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -1,3 +1,10 @@ > +openvswitch (1.9.90-1) unstable; urgency=low > + [ Open vSwitch team ] > + * New upstream version > + - Nothing yet! Try NEWS... > + > + -- Open vSwitch team Wed, 24 Oct 2012 16:12:57 -0700 > + > openvswitch (1.9.0-1) unstable; urgency=low >[ Open vSwitch team ] >* New upstream version > -- > 1.7.2.5 > > ___ > 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: Fix zero key tunnels.
On Wed, Oct 24, 2012 at 3:44 PM, Jesse Gross wrote: > On Tue, Oct 23, 2012 at 6:42 PM, Pravin B Shelar wrote: >> Datapath tunneling check for flag OVS_FLOW_TNL_F_KEY is failing, >> causing it to drop packet. This only happens on tunnels with >> zero key as vswitchd does not generate set-tunnel action. Set >> tunnel action sets this flags for given action. To fix this issue >> the check is dropped. >> >> Bug #13666 >> >> Signed-off-by: Pravin B Shelar > > Acked-by: Jesse Gross Thanks guys, I pushed patch to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] xenserver, rhel, debian: Use ovs-ctl restart.
ovs-ctl has a new command called "restart" which
saves and restores the openflow flows on bridges.
Use that command from the init scripts when doing
a "restart --save-flows=yes".
Also, the debian package postinst script can
set the variable OVS_RESTART_SAVE_FLOWS to "yes"
to ask for save and restore of flows.
Feature #13555.
Signed-off-by: Gurucharan Shetty
---
debian/openvswitch-switch.init | 15 +--
rhel/etc_init.d_openvswitch | 13 +++--
xenserver/etc_init.d_openvswitch | 14 --
3 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/debian/openvswitch-switch.init b/debian/openvswitch-switch.init
index f650f87..301bc73 100755
--- a/debian/openvswitch-switch.init
+++ b/debian/openvswitch-switch.init
@@ -78,6 +78,17 @@ stop () {
ovs_ctl stop
}
+restart () {
+# OVS_RESTART_SAVE_FLOWS can be set by package postinst script.
+if [ "$OVS_RESTART_SAVE_FLOWS" = "yes" ] || \
+ [ "$1" = "--save-flows=yes" ]; then
+start restart
+else
+stop
+start
+fi
+}
+
case $1 in
start)
start
@@ -89,8 +100,8 @@ case $1 in
# The OVS daemons keep up-to-date.
;;
restart)
-stop
-start
+shift
+restart "$@"
;;
status)
ovs_ctl status
diff --git a/rhel/etc_init.d_openvswitch b/rhel/etc_init.d_openvswitch
index 85cf55d..3b2343f 100755
--- a/rhel/etc_init.d_openvswitch
+++ b/rhel/etc_init.d_openvswitch
@@ -62,6 +62,15 @@ stop () {
rm -f /var/lock/subsys/openvswitch
}
+restart () {
+if [ "$1" = "--save-flows=yes" ]; then
+start restart
+else
+stop
+start
+fi
+}
+
ovs_ctl=/usr/share/openvswitch/scripts/ovs-ctl
case $1 in
start)
@@ -71,8 +80,8 @@ case $1 in
stop
;;
restart)
-stop
-start
+shift
+restart "$@"
;;
reload|force-reload)
# Nothing to do.
diff --git a/xenserver/etc_init.d_openvswitch b/xenserver/etc_init.d_openvswitch
index 48fc174..d4a056d 100755
--- a/xenserver/etc_init.d_openvswitch
+++ b/xenserver/etc_init.d_openvswitch
@@ -109,6 +109,16 @@ stop () {
rm -f /var/lock/subsys/openvswitch
}
+restart () {
+if [ "$1" = "--save-flows=yes" ]; then
+stop_daemon ovs-xapi-sync
+start restart
+else
+stop
+start
+fi
+}
+
ovs_ctl=/usr/share/openvswitch/scripts/ovs-ctl
case $1 in
start)
@@ -118,8 +128,8 @@ case $1 in
stop
;;
restart)
-stop
-start
+shift
+restart "$@"
;;
reload|force-reload)
# The main OVS daemons keep up-to-date, but ovs-xapi-sync needs help.
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] debian: Save openflow flows during package upgrade.
When debian package for openvswitch-switch is upgraded, export a variable, OVS_RESTART_SAVE_FLOWS=yes. This will save the openflow flows in vswitchd and re-apply it after the upgrade. Feature #13555. Signed-off-by: Gurucharan Shetty --- debian/openvswitch-switch.postinst |4 1 file changed, 4 insertions(+) diff --git a/debian/openvswitch-switch.postinst b/debian/openvswitch-switch.postinst index 7b9d7bc..ac6ed65 100755 --- a/debian/openvswitch-switch.postinst +++ b/debian/openvswitch-switch.postinst @@ -49,6 +49,10 @@ esac OVS_MISSING_KMOD_OK=yes export OVS_MISSING_KMOD_OK +# Save and restore openflow flows during a package upgrade. +OVS_RESTART_SAVE_FLOWS=yes +export OVS_RESTART_SAVE_FLOWS + #DEBHELPER# exit 0 -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] Prepare for post-1.9.0 (1.9.90).
Thanks, I pushed this to master and branch-1.9. On Thu, Oct 25, 2012 at 12:21:52AM +0100, Justin Pettit wrote: > The series looks good to me. > > --Justin > > > On Oct 25, 2012, at 12:18 AM, Ben Pfaff wrote: > > > Signed-off-by: Ben Pfaff > > --- > > This will go to master only, not to branch-1.9. > > > > NEWS |4 > > configure.ac |2 +- > > debian/changelog |7 +++ > > 3 files changed, 12 insertions(+), 1 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index d04db25..3cb1bd3 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -1,3 +1,7 @@ > > +post-v1.9.0 > > + > > + > > + > > v1.9.0 - xx xxx > > > > - The tunneling code no longer assumes input and output keys are > > symmetric. > > diff --git a/configure.ac b/configure.ac > > index 060b53f..32940a5 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -13,7 +13,7 @@ > > # limitations under the License. > > > > AC_PREREQ(2.64) > > -AC_INIT(openvswitch, 1.9.0, ovs-b...@openvswitch.org) > > +AC_INIT(openvswitch, 1.9.90, 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 bf45202..9ac3d5d 100644 > > --- a/debian/changelog > > +++ b/debian/changelog > > @@ -1,3 +1,10 @@ > > +openvswitch (1.9.90-1) unstable; urgency=low > > + [ Open vSwitch team ] > > + * New upstream version > > + - Nothing yet! Try NEWS... > > + > > + -- Open vSwitch team Wed, 24 Oct 2012 16:12:57 > > -0700 > > + > > openvswitch (1.9.0-1) unstable; urgency=low > >[ Open vSwitch team ] > >* New upstream version > > -- > > 1.7.2.5 > > > > ___ > > 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] debian: Use restart --save-flows=yes during postinst.
On Wed, Oct 24, 2012 at 3:45 PM, Ben Pfaff wrote: > On Wed, Oct 24, 2012 at 02:21:40PM -0700, Gurucharan Shetty wrote: > > When debian package for openvswitch-switch is upgraded, > > restart the daemons using the "--save-flows=yes" flag. > > This will save the openflow flows in vswitchd and > > re-apply it after the upgrade. > > > > Feature #13555. > > Signed-off-by: Gurucharan Shetty > > I think that _dh_* is supposed to be reserved namespace for use by > debhelper, so I would remove the _dh_ prefix. > > I don't think it's a good idea to remove #DEBEHELPER#, because > debhelper does more than just restart the daemon. Maybe that's all it > does in this postinst script now (did you check?), but it could do > more in the future, in which case we'd end up with surprising > problems. > > Actually we already ran into related problems with init scripts, see > commit 8a5b3cfd91841c97fbc8a003857cacbd602646ed: > > debian: Use a different way to avoid failing install without kernel > module. > > The dh_installinit --error-handler option makes a lot of sense, but > after > playing with it for a while I could not figure out a nice way to use it > only for openvswitch-switch without either duplicating the > dh_installinit > fragments in postinst and prerm (the actual bug that was reported) or > omitting them for some package. > > Also, we forgot to write the error handler function for the prerm. > > This commit switches to a different way to avoid failing the install > when > the kernel module is not available, without using --error-handler. > > CC: 663...@bugs.debian.org > Reported-by: Thomas Goirand > Reviewed-by: Simon Horman > Signed-off-by: Ben Pfaff > > Looking at the solution we used there, it might be easiest to, instead > of using a command line option, to use an environment variable and > then put above #DEBEHELPER# the commands to set and export that > variable, like we do with OVS_MISSING_KMOD_OK. > > Sorry about all the trouble. > Thanks. This makes it more easier. I have sent a new patch. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] xenserver, rhel, debian: Use ovs-ctl restart.
On Wed, Oct 24, 2012 at 04:20:39PM -0700, Gurucharan Shetty wrote: > ovs-ctl has a new command called "restart" which > saves and restores the openflow flows on bridges. > Use that command from the init scripts when doing > a "restart --save-flows=yes". > > Also, the debian package postinst script can > set the variable OVS_RESTART_SAVE_FLOWS to "yes" > to ask for save and restore of flows. > > Feature #13555. > Signed-off-by: Gurucharan Shetty Looks good, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] debian: Save openflow flows during package upgrade.
On Wed, Oct 24, 2012 at 04:20:40PM -0700, Gurucharan Shetty wrote: > When debian package for openvswitch-switch is upgraded, > export a variable, OVS_RESTART_SAVE_FLOWS=yes. > This will save the openflow flows in vswitchd and > re-apply it after the upgrade. > > Feature #13555. > Signed-off-by: Gurucharan Shetty Looks good, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] timeval: Coalesce backtraces with counts.
On Wed, Oct 24, 2012 at 01:49:08PM -0700, Ethan Jackson wrote: > With this patch, `ovs-appctl backtrace` will return a unique list > of backtraces and a count of how many times it has been recorded. > This work had previously been done by ovs-parse-backtrace. However, > in future patches poll-loop will begin logging backtraces as a > matter of course. At this point, coalescing the backtraces will > help keep these log messages brief. > > Signed-off-by: Ethan Jackson I think that trace_map_lookup() might need to be marked OVS_UNUSED because it looks like the only call to it is from an #if...#endif block. (Or, we could eliminate the #if...#endif blocks by writing no-op stubs for backtrace() and backtrace_symbols().) Otherwise this looks good. I didn't test it. I didn't look over ovs-parse-backtrace.in very well. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/3] timeval: Take a backtrace on each SIGALRM.
On Wed, Oct 24, 2012 at 01:49:07PM -0700, Ethan Jackson wrote: > With this patch, timeval will take a backtrace with each SIGALRM > allowing it to retrieve a profiling snapshot instantly. This will > be useful in future patches when backtraces are logged. > > Signed-off-by: Ethan Jackson Looks good, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-ctl.in: Don't save flows if the daemons are not running.
On Wed, Oct 24, 2012 at 1:42 PM, Ben Pfaff wrote: > On Wed, Oct 24, 2012 at 01:19:24PM -0700, Gurucharan Shetty wrote: > > When a 'ovs-ctl restart' is executed and the userspace daemons > > like ovsdb-server and ovs-vswitchd are not running, attempt to > > save flows can wait forever. This also results in the daemons > > from not getting started. > > > > Signed-off-by: Gurucharan Shetty > > Looks good, thanks. > Thanks. Pushed this to master. > > Perhaps we should have timeouts (-T or --timeout) on the program > invocations that could hang. > I will send another patch for that. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] debian: Save openflow flows during package upgrade.
On Wed, Oct 24, 2012 at 4:29 PM, Ben Pfaff wrote: > On Wed, Oct 24, 2012 at 04:20:40PM -0700, Gurucharan Shetty wrote: > > When debian package for openvswitch-switch is upgraded, > > export a variable, OVS_RESTART_SAVE_FLOWS=yes. > > This will save the openflow flows in vswitchd and > > re-apply it after the upgrade. > > > > Feature #13555. > > Signed-off-by: Gurucharan Shetty > > Looks good, thanks. > Thanks. I pushed this series to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] poll-loop: Log backtraces when CPU usage is high.
On Wed, Oct 24, 2012 at 01:49:09PM -0700, Ethan Jackson wrote:
> Often when debugging Open vSwitch, one will see in the logs that
> CPU usage has been high for some period of time, but it's totally
> unclear why. In an attempt to remedy the situation, this patch
> logs backtraces taken at regular intervals as a poor man's
> profiling alternative.
>
> Signed-off-by: Ethan Jackson
I only have a minor cosmetic suggestion. Here:
> if (cpu_usage >= 0) {
> ds_put_format(&s, " (%d%% CPU usage)", cpu_usage);
> +
> +if (!vlog_should_drop(THIS_MODULE, level, &trace_rl)) {
> +ds_put_cstr(&s, "\nBacktraces:\n");
> +format_backtraces(&s, 2);
> +}
it might make sense to drop "Backtraces:\n" (you probably want to keep
the newline) because otherwise if there aren't any repeated backtraces
or if we're on a system without backtrace() then you'll get a
funny-looking two-line log message without any additional detail.
Otherwise I think (hope, anyway) that this will be very useful.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] vswitchd: Make the maximum size of MAC learning tables user-configurable.
We've had a couple of requests for this over the years. It's easy to do,
so let's implement it.
Signed-off-by: Ben Pfaff
---
NEWS |1 +
lib/mac-learning.c | 24 ++---
lib/mac-learning.h |5 +++-
ofproto/ofproto-dpif.c |6 +++-
ofproto/ofproto-provider.h | 11 +--
ofproto/ofproto.c | 11 +---
ofproto/ofproto.h |3 +-
tests/ofproto-dpif.at | 62
vswitchd/bridge.c | 19 ++---
vswitchd/vswitch.xml |9 ++
10 files changed, 131 insertions(+), 20 deletions(-)
diff --git a/NEWS b/NEWS
index 3cb1bd3..72c5b87 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
post-v1.9.0
+- The maximum size of the MAC learning table is now configurable.
v1.9.0 - xx xxx
diff --git a/lib/mac-learning.c b/lib/mac-learning.c
index 3c541af..f609d48 100644
--- a/lib/mac-learning.c
+++ b/lib/mac-learning.c
@@ -110,7 +110,8 @@ normalize_idle_time(unsigned int idle_time)
}
/* Creates and returns a new MAC learning table with an initial MAC aging
- * timeout of 'idle_time' seconds. */
+ * timeout of 'idle_time' seconds and an initial maximum of MAC_DEFAULT_MAX
+ * entries. */
struct mac_learning *
mac_learning_create(unsigned int idle_time)
{
@@ -122,6 +123,7 @@ mac_learning_create(unsigned int idle_time)
ml->secret = random_uint32();
ml->flood_vlans = NULL;
ml->idle_time = normalize_idle_time(idle_time);
+ml->max_entries = MAC_DEFAULT_MAX;
return ml;
}
@@ -176,6 +178,16 @@ mac_learning_set_idle_time(struct mac_learning *ml,
unsigned int idle_time)
}
}
+/* Sets the maximum number of entries in 'ml' to 'max_entries', adjusting it
+ * to be within a reasonable range. */
+void
+mac_learning_set_max_entries(struct mac_learning *ml, size_t max_entries)
+{
+ml->max_entries = (max_entries < 10 ? 10
+ : max_entries > 1000 * 1000 ? 1000 * 1000
+ : max_entries);
+}
+
static bool
is_learning_vlan(const struct mac_learning *ml, uint16_t vlan)
{
@@ -212,7 +224,7 @@ mac_learning_insert(struct mac_learning *ml,
if (!e) {
uint32_t hash = mac_table_hash(ml, src_mac, vlan);
-if (hmap_count(&ml->table) >= MAC_MAX) {
+if (hmap_count(&ml->table) >= ml->max_entries) {
get_lru(ml, &e);
mac_learning_expire(ml, e);
}
@@ -311,7 +323,9 @@ void
mac_learning_run(struct mac_learning *ml, struct tag_set *set)
{
struct mac_entry *e;
-while (get_lru(ml, &e) && time_now() >= e->expires) {
+while (get_lru(ml, &e)
+ && (hmap_count(&ml->table) > ml->max_entries
+ || time_now() >= e->expires)) {
COVERAGE_INC(mac_learning_expired);
if (set) {
tag_set_add(set, e->tag);
@@ -323,7 +337,9 @@ mac_learning_run(struct mac_learning *ml, struct tag_set
*set)
void
mac_learning_wait(struct mac_learning *ml)
{
-if (!list_is_empty(&ml->lrus)) {
+if (hmap_count(&ml->table) > ml->max_entries) {
+poll_immediate_wake();
+} else if (!list_is_empty(&ml->lrus)) {
struct mac_entry *e = mac_entry_from_lru_node(ml->lrus.next);
poll_timer_wait_until(e->expires * 1000LL);
}
diff --git a/lib/mac-learning.h b/lib/mac-learning.h
index 8f8fd45..284e7f6 100644
--- a/lib/mac-learning.h
+++ b/lib/mac-learning.h
@@ -26,7 +26,8 @@
struct mac_learning;
-#define MAC_MAX 2048
+/* Default maximum size of a MAC learning table, in entries. */
+#define MAC_DEFAULT_MAX 2048
/* Time, in seconds, before expiring a mac_entry due to inactivity. */
#define MAC_ENTRY_DEFAULT_IDLE_TIME 300
@@ -83,6 +84,7 @@ struct mac_learning {
uint32_t secret;/* Secret for randomizing hash table. */
unsigned long *flood_vlans; /* Bitmap of learning disabled VLANs. */
unsigned int idle_time; /* Max age before deleting an entry. */
+size_t max_entries; /* Max number of learned MACs. */
};
/* Basics. */
@@ -96,6 +98,7 @@ void mac_learning_wait(struct mac_learning *);
bool mac_learning_set_flood_vlans(struct mac_learning *,
const unsigned long *bitmap);
void mac_learning_set_idle_time(struct mac_learning *, unsigned int idle_time);
+void mac_learning_set_max_entries(struct mac_learning *, size_t max_entries);
/* Learning. */
bool mac_learning_may_learn(const struct mac_learning *,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c800386..ffb500a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2422,10 +2422,12 @@ forward_bpdu_changed(struct ofproto *ofproto_)
}
static void
-set_mac_idle_time(struct ofproto *ofproto_, unsigned int idle_time)
+set_mac_table_config(struct ofproto *ofproto_, unsigned int idle_time,
+ size_t max_entries)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
Re: [ovs-dev] [PATCH] netdev-vport: Disable Path MTU Discovery by default
On Mon, Oct 22, 2012 at 12:40 PM, Ben Pfaff wrote: > On Mon, Oct 22, 2012 at 10:10:10AM +0300, Ansis Atteka wrote: >> This patch changes the default path MTU discovery value to >> disabled. >> >> Signed-off-by: Ansis Atteka > > Jesse, Ansis says that this is at your request. Would you mind > supplying a few words for the commit message explaining why we're > changing this? Then we'll have some idea not just now but years from > now. We're going to have to drop our path MTU discovery feature when we switch to flow-based tunneling. Disabling it by default is more of a transitional mechanism to give users a one release heads-up before it goes away for real. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 01/18] vconn: Paramatise pvconn_open() over minimum OpenFlow version
On Tue, Oct 23, 2012 at 11:04:09PM -0700, Ben Pfaff wrote: > On Wed, Oct 24, 2012 at 08:59:57AM +0900, Simon Horman wrote: > > On Tue, Oct 23, 2012 at 09:00:42AM -0700, Ben Pfaff wrote: > > > On Thu, Oct 18, 2012 at 02:58:01PM +0900, Simon Horman wrote: > > > > The motivation for this is to avoid avoids needing to provide a minimu > > > > version parameter to pvconn_accept(). This will in turn avoid the need > > > > to > > > > pass version information to connmgr_run() when the range of accetable > > > > OpenFlow versions is made configurable. > > > > > > > > Signed-off-by: Simon Horman > > > > > > What's the reason to do this if soon after the acceptable versions will > > > become configurable via bitmap? > > > > To avoid version information being passed to connmgr_run(), > > regardless of if it is a minimum version or a bitmap. > > OK, that's reasonable, and I didn't see that at first glance. > > If you think it's better to separate those steps, then feel free to > leave #1 and #3 as separate patches. Or if you agree that it makes > sense to squash them, go ahead and squash them. I'll see how things look, but I think that I am happy to squash them. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 04/18] ofp-util: Add version bitmap to hello messages
On Wed, Oct 24, 2012 at 10:14:30AM -0700, Ben Pfaff wrote: > On Thu, Oct 18, 2012 at 02:58:04PM +0900, Simon Horman wrote: > > Allow encoding and decoding of version bitmap in hello messages > > as specified in Open Flow 1.3.1. > > > > Signed-off-by: Simon Horman > > Thanks for doing this. > > It looks like ofputil_decode_hello() only processes a single hello > element, only if that one is an OFPHET_VERSIONBITMAP element. OF1.3.1 > says: > > Implementations must ignore (skip) all elements of a Hello message > that they do not support. > > so I'd be inclined to say that, instead, we should iterate over each > element that is present, looking for an OFPHET_VERSIONBITMAP element and > silently ignoring any others that we find. Good point. I will add that logic. > It looks like the declarations of ofputil_hello, ofputil_decode_hello(), > and ofputil_encode_hello() should be moved from patch 2 to this patch, > because I don't see any reference to them before this patch. Sorry, I think that is just an artifact of my patch shuffling. I'll clean that up. > I'm not certain that ofputil_hello actually needs the 'version' member. > To decode a hello message without a bitmap, one can initialize the > bitmap as every version up to the one specified; to decode a hello > message with a bitmap, one initializes the bitmap from the included > bitmap. To encode a hello message given only a bitmap is equally > straightforward. (And if I'm right about all that, then maybe we don't > need a struct at all--perhaps a single uint32_t bitmap is sufficient?) I had considered that and I am happy with that approach. The reason I decided on the implementation with a separate version number is to allow ofp_print_hello() to omit printing the bitmap if it wasn't in the hello message on the wire. However, this is arguably cosmetic and always using a bitmap would simplify several code paths. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [1.4 backports 2/6] datapath: Move CSUM_MANGLED_0 definition to net checksum header.
On Wed, Oct 24, 2012 at 12:47 PM, Ben Pfaff wrote: > From: Pravin B Shelar > > Following patch fixes compilation error on older kernel. > > This is a crossport of commit 08d19ca9fef29b23826f1fb52e2368a9077783ca > from master. > > Signed-off-by: Pravin B Shelar > Acked-by: Jesse Gross I think there's an ordering problem here. This patch is supposed to come after patch #3 and remove the CSUM_MANGLED definition from linux/checksum.h as well. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [1.4 backports 0/6] backports of datapaths fixes to 1.4
On Wed, Oct 24, 2012 at 12:47 PM, Ben Pfaff wrote: > Yesterday, while going through datapath changes on master, I > noticed that some bugfixes seem to be relevant and missing on > master. Here is a series of backports for review. Thanks, other than the one patch that I commented on this series looks good to me. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-vport: Disable Path MTU Discovery by default
On Wed, Oct 24, 2012 at 05:43:29PM -0700, Jesse Gross wrote: > On Mon, Oct 22, 2012 at 12:40 PM, Ben Pfaff wrote: > > On Mon, Oct 22, 2012 at 10:10:10AM +0300, Ansis Atteka wrote: > >> This patch changes the default path MTU discovery value to > >> disabled. > >> > >> Signed-off-by: Ansis Atteka > > > > Jesse, Ansis says that this is at your request. Would you mind > > supplying a few words for the commit message explaining why we're > > changing this? Then we'll have some idea not just now but years from > > now. > > We're going to have to drop our path MTU discovery feature when we > switch to flow-based tunneling. Disabling it by default is more of a > transitional mechanism to give users a one release heads-up before it > goes away for real. OK, that's what I wanted to know. Ansis, will you add that information to the commit message before you push it? Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] OF11: push_vlan support
This implementes push_vlan with 802.1Q.
NOTE: 802.1AD (QinQ) is not supported. It requires another effort.
Signed-off-by: Isaku Yamahata
---
lib/ofp-actions.c | 25 +
lib/ofp-actions.h |9 +
lib/ofp-parse.c| 13 +
lib/ofp-util.def |2 +-
lib/packets.h |7 ++-
ofproto/ofproto-dpif.c |5 +
tests/ofp-actions.at |3 +++
7 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index c6ba131..ed22db9 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -714,6 +714,11 @@ ofpact_from_openflow11(const union ofp_action *a, struct
ofpbuf *out)
ofpact_put_SET_VLAN_PCP(out)->vlan_pcp = a->vlan_pcp.vlan_pcp;
break;
+case OFPUTIL_OFPAT11_PUSH_VLAN:
+ofpact_put_PUSH_VLAN(out)->ethertype =
+((const struct ofp11_action_push *)a)->ethertype;
+break;
+
case OFPUTIL_OFPAT11_POP_VLAN:
ofpact_put_STRIP_VLAN(out);
break;
@@ -1062,6 +1067,13 @@ ofpact_check__(const struct ofpact *a, const struct flow
*flow, int max_ports)
case OFPACT_BUNDLE:
return bundle_check(ofpact_get_BUNDLE(a), max_ports, flow);
+case OFPACT_PUSH_VLAN:
+if (ofpact_get_PUSH_VLAN(a)->ethertype != htons(ETH_TYPE_VLAN_8021Q)) {
+/* TODO:XXX 802.1AD(QinQ) isn't supported at the moment */
+return OFPERR_OFPET_BAD_ACTION;
+}
+return 0;
+
case OFPACT_SET_VLAN_VID:
case OFPACT_SET_VLAN_PCP:
case OFPACT_STRIP_VLAN:
@@ -1358,6 +1370,7 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf
*out)
case OFPACT_SET_VLAN_VID:
case OFPACT_SET_VLAN_PCP:
case OFPACT_STRIP_VLAN:
+case OFPACT_PUSH_VLAN:
case OFPACT_SET_ETH_SRC:
case OFPACT_SET_ETH_DST:
case OFPACT_SET_IPV4_SRC:
@@ -1456,6 +1469,7 @@ ofpact_to_openflow10(const struct ofpact *a, struct
ofpbuf *out)
= htons(ofpact_get_SET_L4_DST_PORT(a)->port);
break;
+case OFPACT_PUSH_VLAN:
case OFPACT_CLEAR_ACTIONS:
case OFPACT_GOTO_TABLE:
/* TODO:XXX */
@@ -1548,6 +1562,11 @@ ofpact_to_openflow11(const struct ofpact *a, struct
ofpbuf *out)
ofputil_put_OFPAT11_POP_VLAN(out);
break;
+case OFPACT_PUSH_VLAN:
+ofputil_put_OFPAT11_PUSH_VLAN(out)->ethertype =
+ofpact_get_PUSH_VLAN(a)->ethertype;
+break;
+
case OFPACT_SET_ETH_SRC:
memcpy(ofputil_put_OFPAT11_SET_DL_SRC(out)->dl_addr,
ofpact_get_SET_ETH_SRC(a)->mac, ETH_ADDR_LEN);
@@ -1711,6 +1730,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact,
uint16_t port)
case OFPACT_SET_VLAN_VID:
case OFPACT_SET_VLAN_PCP:
case OFPACT_STRIP_VLAN:
+case OFPACT_PUSH_VLAN:
case OFPACT_SET_ETH_SRC:
case OFPACT_SET_ETH_DST:
case OFPACT_SET_IPV4_SRC:
@@ -1894,6 +1914,11 @@ ofpact_format(const struct ofpact *a, struct ds *s)
ds_put_cstr(s, "strip_vlan");
break;
+case OFPACT_PUSH_VLAN:
+ds_put_format(s, "push_vlan:%#"PRIx16,
+ ntohs(ofpact_get_PUSH_VLAN(a)->ethertype));
+break;
+
case OFPACT_SET_ETH_SRC:
ds_put_format(s, "mod_dl_src:"ETH_ADDR_FMT,
ETH_ADDR_ARGS(ofpact_get_SET_ETH_SRC(a)->mac));
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 410103e..26ce791 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -60,6 +60,7 @@
DEFINE_OFPACT(SET_VLAN_VID,ofpact_vlan_vid, ofpact)\
DEFINE_OFPACT(SET_VLAN_PCP,ofpact_vlan_pcp, ofpact)\
DEFINE_OFPACT(STRIP_VLAN, ofpact_null, ofpact)\
+DEFINE_OFPACT(PUSH_VLAN, ofpact_push, ofpact)\
DEFINE_OFPACT(SET_ETH_SRC, ofpact_mac, ofpact)\
DEFINE_OFPACT(SET_ETH_DST, ofpact_mac, ofpact)\
DEFINE_OFPACT(SET_IPV4_SRC,ofpact_ipv4, ofpact)\
@@ -309,6 +310,14 @@ struct ofpact_reg_load {
union mf_subvalue subvalue; /* Least-significant bits are used. */
};
+/* OFPACT_PUSH_VLAN/MPLS/PBB
+ *
+ * used for NXAST_PUSH_MPLS, OFPAT13_PUSH_VLAN/MPLS/PBB */
+struct ofpact_push {
+struct ofpact ofpact;
+ovs_be16 ethertype;
+};
+
/* OFPACT_SET_TUNNEL.
*
* Used for NXAST_SET_TUNNEL, NXAST_SET_TUNNEL64. */
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 33065aa..fce3c51 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -384,6 +384,7 @@ parse_named_action(enum ofputil_action_code code, const
struct flow *flow,
{
struct ofpact_tunnel *tunnel;
uint16_t vid;
+uint16_t ethertype;
ovs_be32 ip;
uint8_t pcp;
uint8_t tos;
@@ -424,6 +425,18 @@ parse_named_action(enum ofputil_action_code code, const
struct flow *flow,
ofpact_put_STRIP_VLAN(ofpacts);
break;
+case OFPUTIL_OFPAT11_PUSH_VLAN:
+ethertype = str_to_u16(arg, "ethertype");
+
