Re: [ovs-dev] [cleanups 06/12] dpif: Don't log warning for ENOENT with dpif_port_exists().
On Nov 16, 2012, at 9:46 AM, Ben Pfaff wrote: > On Fri, Nov 16, 2012 at 12:02:59AM -0800, Justin Pettit wrote: >> Signed-off-by: Justin Pettit > > It seems fine, but the commit message should explain why we're doing > this. Done. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenVswitch dev document
On Mon, Nov 19, 2012 at 03:51:45PM +0800, wrote: > I'm a student and now going on some project related to OpenVswtich. But > the source code seems so complicated to me. Do you have development > document about openvswitch? If you have, could you please mail me a copy? The PORTING file at the top of the tree is a good place to start. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] don't use select.POLL* constants
On Mon, Nov 19, 2012 at 03:50:52AM +0900, FUJITA Tomonori wrote: > Python doesn't have select.POLL* constants on some architectures > (e.g. MacOSX). This code needs to define the constants for itself. It > uses select.POLL* constants only internally (doesn't pass them > outside). So there is no harm even if the definition would conflict > with Python's those. > > Signed-off-by: FUJITA Tomonori I applied this to master, thank you. Why doesn't Mac OS X have the POLL* constants? Mac OS X is a Unix variant and does have the poll() system call. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] vswitchd: Make Interface's ofport a persistent column.
Currently, the 'ofport' column in Interface table is
ephemeral and is populated by vswitchd everytime it is
started or when a new interface is created with vswitchd
running.
Making it persistent lets vswitchd try and assign the
same ofport number to a particular interface across
restarts. This is just a fallback option when
'ofport_request' column is empty.
Signed-off-by: Gurucharan Shetty
---
vswitchd/bridge.c |7 +--
vswitchd/vswitch.ovsschema |7 +++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 316ecc7..6185c4f 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -290,12 +290,14 @@ bridge_init_ofproto(const struct ovsrec_open_vswitch *cfg)
for (k = 0; k < port_cfg->n_interfaces; k++) {
struct ovsrec_interface *if_cfg = port_cfg->interfaces[k];
struct iface_hint *iface_hint;
+int64_t ofport = if_cfg->n_ofport ?
+*if_cfg->ofport : OFPP_NONE;
iface_hint = xmalloc(sizeof *iface_hint);
iface_hint->br_name = br_cfg->name;
iface_hint->br_type = br_cfg->datapath_type;
iface_hint->ofp_port = if_cfg->n_ofport_request ?
-*if_cfg->ofport_request : OFPP_NONE;
+*if_cfg->ofport_request : ofport;
shash_add(&iface_hints, if_cfg->name, iface_hint);
}
@@ -2529,10 +2531,11 @@ bridge_queue_if_cfg(struct bridge *br,
const struct ovsrec_port *parent)
{
struct if_cfg *if_cfg = xmalloc(sizeof *if_cfg);
+int64_t ofport = cfg->n_ofport ? *cfg->ofport : OFPP_NONE;
if_cfg->cfg = cfg;
if_cfg->parent = parent;
-if_cfg->ofport = cfg->n_ofport_request ? *cfg->ofport_request : OFPP_NONE;
+if_cfg->ofport = cfg->n_ofport_request ? *cfg->ofport_request : ofport;
hmap_insert(&br->if_cfg_todo, &if_cfg->hmap_node,
hash_string(if_cfg->cfg->name, 0));
}
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 1234488..35cc564 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
{"name": "Open_vSwitch",
- "version": "6.11.0",
- "cksum": "3699219253 17163",
+ "version": "6.12.0",
+ "cksum": "1488803988 17135",
"tables": {
"Open_vSwitch": {
"columns": {
@@ -176,8 +176,7 @@
"external_ids": {
"type": {"key": "string", "value": "string", "min": 0, "max":
"unlimited"}},
"ofport": {
- "type": {"key": "integer", "min": 0, "max": 1},
- "ephemeral": true},
+ "type": {"key": "integer", "min": 0, "max": 1}},
"ofport_request": {
"type": {
"key": {"type": "integer",
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vswitchd: Make Interface's ofport a persistent column.
On Mon, Nov 19, 2012 at 09:09:34AM -0800, Gurucharan Shetty wrote: > Currently, the 'ofport' column in Interface table is > ephemeral and is populated by vswitchd everytime it is > started or when a new interface is created with vswitchd > running. > > Making it persistent lets vswitchd try and assign the > same ofport number to a particular interface across > restarts. This is just a fallback option when > 'ofport_request' column is empty. > > Signed-off-by: Gurucharan Shetty Let's add a helper function to figure out the desired port, since the same calculation is happening in two places. Will you update vswitch.xml to explain the new behavior? Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 0/8] Run-Time Open Flow Version Configuration
On Mon, Nov 19, 2012 at 02:59:27PM +0900, Simon Horman wrote: > This series adds run-time configuration of allowed OpenFlow versions to > ovs-vswtichd, ovs-ofctl and ovs-controller; and adds tests for Open Flow > 1.2 messages that utilise the run-time configuration of allowed OpenFlow > versions. > > Changes since v5: > > * Rebased, including dropping patches from v6 that have been merged > > * Reorder, moving "ofp-util: Flow Dump Protocol for OpenFlow 12" > from near the end to near the begining of the series. This > seems to be a more appropriate place for it as it is a change > to core code. > > * Revise "vswitchd: Configuration of allowed OpenFlow versions" so > that an unknown version string is not treated as a fatal error. > > Changes are noted in the changelog of each patch. > > Git and diffstat information is provided below to aid review. > > Ben, please feel free to modify these patches or send me feedback, > whichever is more convenient for you. Thanks for the new spin! I'll start looking these over. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/8] connmgr: Use version of underlying rconn
On Mon, Nov 19, 2012 at 02:59:28PM +0900, Simon Horman wrote: > Signed-off-by: Simon Horman Applied to master. Thank you! I think it's possible for ofconn_get_protocol() to return OFPUTIL_P_NONE in some cases for sending asynchronous messages, so I updated ofconn_receives_async_msg() to return 'false' if that happens, which I think takes care of all those cases. I also expanded the comment on ofconn_get_protocol() to describe the situation a bit better. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/8] ofp-util: Flow Dump Protocol for OpenFlow 12
On Mon, Nov 19, 2012 at 02:59:29PM +0900, Simon Horman wrote:
> Allow only OpenFlow 12 as a flow dump protocol for OpenFlow12.
> Allow OpenFlow10 and NXM as flow dump protocols for other OpenFlow versions.
> This only changes the behaviour prior to this patch in the case of OpenFlow12
> which seemed broken as an attempt would be made to set NXM as the
> flow dump format using an OpenFlow 1.0 message even though an OpenFlow 1.2
> connection had been negotiated..
>
> Signed-off-by: Simon Horman
I think that this is no longer needed with the changes that I made a few
patches back. set_protocol_for_flow_dump() will try to set the protocol
with try_set_protocol(), which in turn will call
ofputil_encode_set_protocol(), which in turn will see that the versions
are incompatible, with:
cur_version = ofputil_protocol_to_ofp_version(current);
want_version = ofputil_protocol_to_ofp_version(want);
if (cur_version != want_version) {
*next = current;
return NULL;
}
which makes try_set_protocol() return that it failed to set that
protocol, which makes set_protocol_for_flow_dump() go on to the next
one.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] VXLAN source port hashing performance problems
On Fri, Nov 16, 2012 at 2:30 PM, Kyle Mestery (kmestery) wrote: > On Nov 16, 2012, at 3:31 PM, Kyle Mestery (kmestery) > wrote: >> On Nov 16, 2012, at 3:18 PM, Jesse Gross wrote: >>> On Fri, Nov 16, 2012 at 1:00 PM, Kyle Mestery (kmestery) >>> wrote: On Nov 15, 2012, at 3:32 PM, Kyle Mestery (kmestery) wrote: > On Nov 15, 2012, at 3:13 PM, Kyle Mestery (kmestery) > wrote: >> On Nov 15, 2012, at 1:03 PM, Kyle Mestery (kmestery) >> wrote: >>> Jesse: >>> >>> I modified the source port hashing for the VXLAN patch I submitted a >>> few days ago, >>> but I've noticed when using the upstream source port hashing routine, >>> performance >>> drops off by 3.5 times when using iperf between two VMs. From what I >>> can tell, it >>> has to be that all skbuffs coming into the VXLAN tunnel have not >>> already had their >>> rxhash set, and this function is what's killing performance. Let me >>> share the details: >>> >> I think I figured this out. The upstream source port selection algorithm >> is exploding flows >> in the fast path. Here are iperf runs with both and subsequent >> "ovs-dpctl dump-flows" >> commands for comparison. The first one is with the upstream version, the >> second is >> with the one in my patch. Note that I just piped "ovs-dpctl dump-flows" >> into wc to >> summarize the flow count. >> >> Upstream verison: >> [root@linux-br ~]# iperf -c 10.1.2.14 && ovs-dpctl dump-flows | wc > > > Figured this out, fixing it now, will repost the patch with only this > change soon. > So after looking at this, the upstream source port selection function will cause an explosion of fast path flows due to an ever changing skbuff->rxhash. By using the flow->hash, we don't see this problem. Jesse, any comments on this particular issue? I think using the upstream function will allow for greater spreading across links depending on the hashing algorithm used on upstream switches, but will cause this flow explosion on the host itself. >>> >>> Generally speaking, the OVS flow extraction pulls out more fields than >>> are usually used by skb_get_rxhash() so I'm surprised that it would >>> have this effect (of course, neither is supposed to be so fine grained >>> that it breaks down a single 'real' flow). It sounds to me like there >>> is a bug that is pulling in random data that's not supposed to be part >>> of the flow if it changes on a per packet basis. >>> >> Right, I agree. I'll keep digging to see what I can find. >> > I figured this one out. What was throwing the hash off was the UDP source > port for the VXLAN > packets was apparently random garbage. Clearing it to zero before calling the > routine to acquire > one (which calls skb_get_rxhash() itself) fixed the problem. I'll submit the > latest patch (which has > this as the only change) soon. Good catch. The problem is that this means that we're hashing the outer tunnel headers (which in retrospect makes sense given that we've already started building those headers). As a result, the VXLAN source port will always be constant for a given tunnel endpoint and therefore not add any additional flow entropy. I do think that using the same set of fields as the upstream code is generally a good idea for consistency but at this point fixing it is probably more trouble than it is worth. It will get easier once we can drop all of the compatibility code from OVS (soon but not quite yet since userspace hasn't switched over yet) so it probably makes sense to use the OVS flow hash for the time being. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] VXLAN source port hashing performance problems
On Nov 19, 2012, at 12:16 PM, Jesse Gross wrote: > On Fri, Nov 16, 2012 at 2:30 PM, Kyle Mestery (kmestery) > wrote: >> On Nov 16, 2012, at 3:31 PM, Kyle Mestery (kmestery) >> wrote: >>> On Nov 16, 2012, at 3:18 PM, Jesse Gross wrote: On Fri, Nov 16, 2012 at 1:00 PM, Kyle Mestery (kmestery) wrote: > On Nov 15, 2012, at 3:32 PM, Kyle Mestery (kmestery) > wrote: >> On Nov 15, 2012, at 3:13 PM, Kyle Mestery (kmestery) >> wrote: >>> On Nov 15, 2012, at 1:03 PM, Kyle Mestery (kmestery) >>> wrote: Jesse: I modified the source port hashing for the VXLAN patch I submitted a few days ago, but I've noticed when using the upstream source port hashing routine, performance drops off by 3.5 times when using iperf between two VMs. From what I can tell, it has to be that all skbuffs coming into the VXLAN tunnel have not already had their rxhash set, and this function is what's killing performance. Let me share the details: >>> I think I figured this out. The upstream source port selection >>> algorithm is exploding flows >>> in the fast path. Here are iperf runs with both and subsequent >>> "ovs-dpctl dump-flows" >>> commands for comparison. The first one is with the upstream version, >>> the second is >>> with the one in my patch. Note that I just piped "ovs-dpctl dump-flows" >>> into wc to >>> summarize the flow count. >>> >>> Upstream verison: >>> [root@linux-br ~]# iperf -c 10.1.2.14 && ovs-dpctl dump-flows | wc >> >> >> Figured this out, fixing it now, will repost the patch with only this >> change soon. >> > > So after looking at this, the upstream source port selection function > will cause an explosion > of fast path flows due to an ever changing skbuff->rxhash. By using the > flow->hash, we > don't see this problem. Jesse, any comments on this particular issue? I > think using the > upstream function will allow for greater spreading across links depending > on the hashing > algorithm used on upstream switches, but will cause this flow explosion > on the host itself. Generally speaking, the OVS flow extraction pulls out more fields than are usually used by skb_get_rxhash() so I'm surprised that it would have this effect (of course, neither is supposed to be so fine grained that it breaks down a single 'real' flow). It sounds to me like there is a bug that is pulling in random data that's not supposed to be part of the flow if it changes on a per packet basis. >>> Right, I agree. I'll keep digging to see what I can find. >>> >> I figured this one out. What was throwing the hash off was the UDP source >> port for the VXLAN >> packets was apparently random garbage. Clearing it to zero before calling >> the routine to acquire >> one (which calls skb_get_rxhash() itself) fixed the problem. I'll submit the >> latest patch (which has >> this as the only change) soon. > > Good catch. The problem is that this means that we're hashing the > outer tunnel headers (which in retrospect makes sense given that we've > already started building those headers). As a result, the VXLAN > source port will always be constant for a given tunnel endpoint and > therefore not add any additional flow entropy. > > I do think that using the same set of fields as the upstream code is > generally a good idea for consistency but at this point fixing it is > probably more trouble than it is worth. It will get easier once we > can drop all of the compatibility code from OVS (soon but not quite > yet since userspace hasn't switched over yet) so it probably makes > sense to use the OVS flow hash for the time being. OK, that makes sense. I'll resubmit the patch with the change to use the OVS flow has for the time being then. Thanks, Kyle ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v3] datapath: Add support for VXLAN tunnels to Open vSwitch
Add support for VXLAN tunnels to Open vSwitch. Add support
for setting the destination UDP port on a per-port basis.
This is done by adding a "dst_port" parameter to the port
configuration. This is only applicable currently to VXLAN
tunnels.
The only difference in v3 of this patch series is the source port
selection algorithm uses OVS_CB(skb)->flow->hash now instead of
skb->rxhash.
This patch set is based on one posted by Ben Pfaff on Oct. 12, 2011
to the ovs-dev mailing list:
http://openvswitch.org/pipermail/dev/2011-October/012051.html
The patch has been maintained, updated, and freshened by me and is
availalbe at the following github repository:
https://github.com/mestery/ovs-vxlan/tree/vxlan
I've tested this patch with multiple VXLAN tunnels between hosts
using different UDP port numbers.
See the following IETF draft for additional information about VXLAN:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02
Signed-off-by: Kyle Mestery
---
NEWS | 3 +
README | 2 +-
datapath/Modules.mk | 3 +-
datapath/linux/.gitignore| 2 +
datapath/tunnel.c| 7 +
datapath/tunnel.h| 2 +
datapath/vport-vxlan.c | 473 +++
datapath/vport.c | 1 +
datapath/vport.h | 1 +
debian/control | 2 +-
debian/openvswitch-ipsec.init| 5 +-
debian/ovs-monitor-ipsec | 110 +
include/linux/openvswitch.h | 1 +
include/openflow/nicira-ext.h| 8 +-
include/openvswitch/tunnel.h | 1 +
lib/netdev-vport.c | 27 +++
rhel/etc_init.d_openvswitch | 2 +
tests/ovs-monitor-ipsec.at | 67 ++
vswitchd/vswitch.xml | 59 -
xenserver/etc_init.d_openvswitch | 2 +
20 files changed, 718 insertions(+), 60 deletions(-)
create mode 100644 datapath/vport-vxlan.c
diff --git a/NEWS b/NEWS
index 0372965..a98c194 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ post-v1.9.0
v1.9.0 - xx xxx
+- New support for the experimental VXLAN tunnel protocol (see
+ http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02)
+ and VXLAN over IPSEC.
- 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/README b/README
index 7c680d4..d7691a0 100644
--- a/README
+++ b/README
@@ -24,7 +24,7 @@ vSwitch supports the following features:
* NIC bonding with or without LACP on upstream switch
* NetFlow, sFlow(R), and mirroring for increased visibility
* QoS (Quality of Service) configuration, plus policing
-* GRE, GRE over IPSEC, and CAPWAP tunneling
+* GRE, GRE over IPSEC, CAPWAP, VXLAN, and VXLAN over IPSEC tunneling
* 802.1ag connectivity fault management
* OpenFlow 1.0 plus numerous extensions
* Transactional configuration database with C and Python bindings
diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 24c1075..24e6559 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -26,7 +26,8 @@ openvswitch_sources = \
vport-gre.c \
vport-internal_dev.c \
vport-netdev.c \
- vport-patch.c
+ vport-patch.c \
+ vport-vxlan.c
openvswitch_headers = \
checksum.h \
diff --git a/datapath/linux/.gitignore b/datapath/linux/.gitignore
index d6de397..968948c 100644
--- a/datapath/linux/.gitignore
+++ b/datapath/linux/.gitignore
@@ -23,6 +23,7 @@
/kmemdup.c
/loop_counter.c
/modules.order
+/net_namespace.c
/netdevice.c
/net_namespace.c
/random32.c
@@ -39,5 +40,6 @@
/vport-internal_dev.c
/vport-netdev.c
/vport-patch.c
+/vport-vxlan.c
/vport.c
/workqueue.c
diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index fb4854a..05a73df 100644
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -1042,6 +1042,7 @@ static const struct nla_policy
tnl_policy[OVS_TUNNEL_ATTR_MAX + 1] = {
[OVS_TUNNEL_ATTR_IN_KEY] = { .type = NLA_U64 },
[OVS_TUNNEL_ATTR_TOS] = { .type = NLA_U8 },
[OVS_TUNNEL_ATTR_TTL] = { .type = NLA_U8 },
+ [OVS_TUNNEL_ATTR_DST_PORT] = { .type = NLA_U16 },
};
/* Sets OVS_TUNNEL_ATTR_* fields in 'mutable', which must initially be
@@ -1087,6 +1088,9 @@ static int tnl_set_config(struct net *net, struct nlattr
*options,
if (a[OVS_TUNNEL_ATTR_TTL])
mutable->ttl = nla_get_u8(a[OVS_TUNNEL_ATTR_TTL]);
+ if (a[OVS_TUNNEL_ATTR_DST_PORT])
+ mutable->dst_port = nla_get_u16(a[OVS_TUNNEL_ATTR_DST_PORT]);
+
if (!a[OVS_TUNNEL_ATTR_IN_KEY]) {
mutable->key.tunnel_type |= TNL_T_KEY_MATCH;
mutable->flags |= TNL_F_IN_KEY_MATCH;
@@ -1242,6 +1246,9 @@ int ovs_tnl_get_options(const struct vport *vport, struct
sk_buff *skb)
Re: [ovs-dev] [PATCHv3] datapath: add skb mark matching and set action
On Sat, Nov 17, 2012 at 11:28 AM, Ansis Atteka wrote: > diff --git a/lib/flow.c b/lib/flow.c > index 7084079..816d561 100644 > --- a/lib/flow.c > +++ b/lib/flow.c > @@ -352,6 +352,7 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority, > } > flow->in_port = ofp_in_port; > flow->skb_priority = skb_priority; > +flow->skb_mark = 0; I think we need to actually pass in this value so that we can initialize it based on what the kernel gave us. Otherwise, there is nothing to translate between the conversion from Netlink and actual userspace flow. We're starting to accumulate several fields in this category so we might want to group them together in a struct if it makes sense. > diff --git a/lib/odp-util.h b/lib/odp-util.h > index 5cdb204..0214a8b 100644 > --- a/lib/odp-util.h > +++ b/lib/odp-util.h > @@ -56,6 +56,7 @@ int odp_actions_from_string(const char *, const struct > simap *port_names, > * OVS_KEY_ATTR_TUN_ID8-- 4 12 > * OVS_KEY_ATTR_IPV4_TUNNEL 24-- 4 28 > * OVS_KEY_ATTR_IN_PORT 4-- 4 8 > + * OVS_KEY_ATTR_SKB_MARK 4-- 4 8 > * OVS_KEY_ATTR_ETHERNET 12-- 4 16 > * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 (outer VLAN ethertype) > * OVS_KEY_ATTR_8021Q 4-- 4 8 > @@ -65,12 +66,12 @@ int odp_actions_from_string(const char *, const struct > simap *port_names, > * OVS_KEY_ATTR_ICMPV62 2 4 8 > * OVS_KEY_ATTR_ND 28-- 4 32 > * - > - * total 184 > + * total 192 > * > * We include some slack space in case the calculation isn't quite right or > we > * add another field and forget to adjust this value. > */ > -#define ODPUTIL_FLOW_KEY_BYTES 200 > +#define ODPUTIL_FLOW_KEY_BYTES 208 At this point, let's just bump this up a little more since we're starting to get close - maybe 256. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/8] vswitchd: Configuration of allowed OpenFlow versions
On Mon, Nov 19, 2012 at 02:59:30PM +0900, Simon Horman wrote: > Versions may be configured using the protocols column of > the bridge table. The protocols column is a set which accepts zero > or more of the values: 'OpenFlow10' and 'OpenFlow12'. > > If the protocols column is empty, then OpenFlow10 is used. > This default is consistent with the behaviour of ovs-vswtichd > prior to this patch. > > Signed-off-by: Simon Horman The comment on bridge_get_allowed_versions() was a cut-and-paste error, so I updated it. The vswitch.xml entry specified the allowed values, but that's unnecessary because it's automatically included in the documentation (extracted from the schema constraints). I removed the extra specification. With those changes, I applied this to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] automake: Always include route-table.h in sources.
The Linux, ESX, and BSD route table implementations all require route-table.h. However, it was only being included as a source in the Linux build. Signed-off-by: Ethan Jackson --- lib/automake.mk |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/automake.mk b/lib/automake.mk index f6b2aa1..d1c5d35 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -138,6 +138,7 @@ lib_libopenvswitch_a_SOURCES = \ lib/rconn.h \ lib/reconnect.c \ lib/reconnect.h \ + lib/route-table.h \ lib/sat-math.h \ lib/sha1.c \ lib/sha1.h \ @@ -242,8 +243,7 @@ lib_libopenvswitch_a_SOURCES += \ lib/netlink-socket.h \ lib/rtnetlink-link.c \ lib/rtnetlink-link.h \ - lib/route-table.c \ - lib/route-table.h + lib/route-table.c endif if ESX -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] automake: Always include route-table.h in sources.
On Mon, Nov 19, 2012 at 01:32:01PM -0800, Ethan Jackson wrote: > The Linux, ESX, and BSD route table implementations all require > route-table.h. However, it was only being included as a source in > the Linux build. > > Signed-off-by: Ethan Jackson This shouldn't have any effect. It basically doesn't matter where you list .h files as long as you list them somewhere. Did this fix any actual bug that you encountered? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenVswitch dev document
I've uploaded some slides that I presented last month that included some material on developing for OVS: http://openvswitch.org/slides/OVS-Development.pdf Hope it helps. --Justin On Nov 19, 2012, at 2:51 AM, 李强 wrote: > Hi,all. > I'm a student and now going on some project related to OpenVswtich. But > the source code seems so complicated to me. Do you have development document > about openvswitch? If you have, could you please mail me a copy? >Thanks you so much! > > -- > Best regards, > > Jayce > > ___ > 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] don't use select.POLL* constants
Hi, On Mon, 19 Nov 2012 09:12:07 -0800 Ben Pfaff wrote: > Why doesn't Mac OS X have the POLL* constants? Mac OS X is a Unix > variant and does have the poll() system call. https://github.com/mxcl/homebrew/issues/9531 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] don't use select.POLL* constants
On Tue, Nov 20, 2012 at 07:27:31AM +0900, FUJITA Tomonori wrote: > On Mon, 19 Nov 2012 09:12:07 -0800 > Ben Pfaff wrote: > > > Why doesn't Mac OS X have the POLL* constants? Mac OS X is a Unix > > variant and does have the poll() system call. > > https://github.com/mxcl/homebrew/issues/9531 Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] dpif-netdev: Make odp_flow_key_to_flow() check more explicit.
ODP_FIT_PERFECT has value 0 but this 'if' statement doesn't make much sense
unless you happen to know that.
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c9e3210..6835b2b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -657,7 +657,7 @@ static int
dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len,
struct flow *flow)
{
-if (odp_flow_key_to_flow(key, key_len, flow)) {
+if (odp_flow_key_to_flow(key, key_len, flow) != ODP_FIT_PERFECT) {
/* This should not happen: it indicates that odp_flow_key_from_flow()
* and odp_flow_key_to_flow() disagree on the acceptable form of a
* flow. Log the problem as an error, with enough details to enable
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] automake: Always include route-table.h in sources.
Im pretty sure it did at some point in time. This patch has been sitting in my repo for a long time and I can't seem to figure out what problem I was trying to solve. I'll shelve it for now. Ethan On Mon, Nov 19, 2012 at 1:44 PM, Ben Pfaff wrote: > On Mon, Nov 19, 2012 at 01:32:01PM -0800, Ethan Jackson wrote: > > The Linux, ESX, and BSD route table implementations all require > > route-table.h. However, it was only being included as a source in > > the Linux build. > > > > Signed-off-by: Ethan Jackson > > This shouldn't have any effect. It basically doesn't matter where you > list .h files as long as you list them somewhere. > > Did this fix any actual bug that you encountered? > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] automake: Always include route-table.h in sources.
As an experiment, I moved lib/route-table.h into an "if ESX" block, and it still got distributed on a non-ESX build. Also, I notice that the BSD-only .c and .h files still get distributed even on non-BSD. So I think something else must have been at work. On Mon, Nov 19, 2012 at 03:18:05PM -0800, Ethan Jackson wrote: > Im pretty sure it did at some point in time. This patch has been sitting in > my repo for a long time and I can't seem to figure out what problem I was > trying to solve. I'll shelve it for now. > > Ethan > > > On Mon, Nov 19, 2012 at 1:44 PM, Ben Pfaff wrote: > > > On Mon, Nov 19, 2012 at 01:32:01PM -0800, Ethan Jackson wrote: > > > The Linux, ESX, and BSD route table implementations all require > > > route-table.h. However, it was only being included as a source in > > > the Linux build. > > > > > > Signed-off-by: Ethan Jackson > > > > This shouldn't have any effect. It basically doesn't matter where you > > list .h files as long as you list them somewhere. > > > > Did this fix any actual bug that you encountered? > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [so_error 1/3] python/ovs/socket_util: Fix error path in set_nonblocking.
'e' is an exception, not a socket, so get_exception_errno() is the
appropriate function to obtain an error code from it.
Signed-off-by: Ben Pfaff
---
python/ovs/socket_util.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 1fc80fd..f54b904 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -182,7 +182,7 @@ def set_nonblocking(sock):
sock.setblocking(0)
except socket.error, e:
vlog.err("could not set nonblocking mode on socket: %s"
- % os.strerror(get_socket_error(e)))
+ % os.strerror(get_exception_errno(e)))
def set_dscp(sock, dscp):
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [so_error 2/3] socket-util: Avoid using SO_ERROR.
ESX doesn't implement it, and there's another approach that should work
everywhere, so drop back to that.
Signed-off-by: Ben Pfaff
---
lib/socket-util.c | 13 +++--
python/ovs/socket_util.py | 18 --
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 4edf956..4843cc5 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -221,6 +221,7 @@ get_socket_error(int fd)
int
check_connection_completion(int fd)
{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 10);
struct pollfd pfd;
int retval;
@@ -230,9 +231,17 @@ check_connection_completion(int fd)
retval = poll(&pfd, 1, 0);
} while (retval < 0 && errno == EINTR);
if (retval == 1) {
-return get_socket_error(fd);
+if (pfd.revents & POLLERR) {
+ssize_t n = send(fd, "", 1, MSG_DONTWAIT);
+if (n < 0) {
+return errno;
+} else {
+VLOG_ERR_RL(&rl, "poll return POLLERR but send succeeded");
+return EPROTO;
+}
+}
+return 0;
} else if (retval < 0) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 10);
VLOG_ERR_RL(&rl, "poll: %s", strerror(errno));
return errno;
} else {
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index f54b904..e6b6fce 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -78,8 +78,22 @@ def make_unix_socket(style, nonblock, bind_path,
connect_path):
def check_connection_completion(sock):
p = ovs.poller.SelectPoll()
p.register(sock, ovs.poller.POLLOUT)
-if len(p.poll(0)) == 1:
-return get_socket_error(sock)
+pfds = p.poll(0)
+if len(pfds) == 1:
+revents = pfds[0][1]
+if revents & ovs.poller.POLLERR:
+try:
+# The following should raise an exception.
+socket.send("\0", socket.MSG_DONTWAIT)
+
+# (Here's where we end up if it didn't.)
+# XXX rate-limit
+vlog.err("poll return POLLERR but send succeeded")
+return errno.EPROTO
+except socket.error, e:
+return get_exception_errno(e)
+else:
+return 0
else:
return errno.EAGAIN
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [so_error 3/3] socket-util: Remove get_socket_error().
It has no remaining users.
Signed-off-by: Ben Pfaff
---
lib/socket-util.c | 13 -
lib/socket-util.h |1 -
python/ovs/socket_util.py |6 --
3 files changed, 0 insertions(+), 20 deletions(-)
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 4843cc5..48f42f1 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -205,19 +205,6 @@ lookup_hostname(const char *host_name, struct in_addr
*addr)
: EINVAL);
}
-/* Returns the error condition associated with socket 'fd' and resets the
- * socket's error status. */
-int
-get_socket_error(int fd)
-{
-int error;
-
-if (getsockopt_int(fd, SOL_SOCKET, SO_ERROR, "SO_ERROR", &error)) {
-error = errno;
-}
-return error;
-}
-
int
check_connection_completion(int fd)
{
diff --git a/lib/socket-util.h b/lib/socket-util.h
index a00b32e..5bf8529 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -36,7 +36,6 @@ int lookup_ipv6(const char *host_name, struct in6_addr
*address);
int lookup_hostname(const char *host_name, struct in_addr *);
-int get_socket_error(int sock);
int get_socket_rcvbuf(int sock);
int check_connection_completion(int fd);
int drain_rcvbuf(int fd);
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index e6b6fce..8fecbc7 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -133,12 +133,6 @@ def inet_open_active(style, target, default_port, dscp):
return get_exception_errno(e), None
-def get_socket_error(sock):
-"""Returns the errno value associated with 'socket' (0 if no error) and
-resets the socket's error status."""
-return sock.getsockopt(socket.SOL_SOCKET, socket.SO_ERROR)
-
-
def get_exception_errno(e):
"""A lot of methods on Python socket objects raise socket.error, but that
exception is documented as having two completely different forms of
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/8] ofp-util: Flow Dump Protocol for OpenFlow 12
On Mon, Nov 19, 2012 at 10:13:22AM -0800, Ben Pfaff wrote:
> On Mon, Nov 19, 2012 at 02:59:29PM +0900, Simon Horman wrote:
> > Allow only OpenFlow 12 as a flow dump protocol for OpenFlow12.
> > Allow OpenFlow10 and NXM as flow dump protocols for other OpenFlow versions.
> > This only changes the behaviour prior to this patch in the case of
> > OpenFlow12
> > which seemed broken as an attempt would be made to set NXM as the
> > flow dump format using an OpenFlow 1.0 message even though an OpenFlow 1.2
> > connection had been negotiated..
> >
> > Signed-off-by: Simon Horman
>
> I think that this is no longer needed with the changes that I made a few
> patches back. set_protocol_for_flow_dump() will try to set the protocol
> with try_set_protocol(), which in turn will call
> ofputil_encode_set_protocol(), which in turn will see that the versions
> are incompatible, with:
>
> cur_version = ofputil_protocol_to_ofp_version(current);
> want_version = ofputil_protocol_to_ofp_version(want);
> if (cur_version != want_version) {
> *next = current;
> return NULL;
> }
>
> which makes try_set_protocol() return that it failed to set that
> protocol, which makes set_protocol_for_flow_dump() go on to the next
> one.
Hi Ben,
I'm somewhat unsure how you envisage this working.
Without this patch the code you refer to does indeed see that
all versions are incompatible. But in the end it runs out of options
and produces a fatal error.
$ ovs-ofctl --allowed-ofp-versions OpenFlow12 dump-flows br0
ovs-ofctl: switch does not support any of the usable flow formats (any)
ovs-vswitchd is configured to accept OpenFlow12 and OpenFlow10.
The aim of this patch is to allow the above combination to succeed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/8] ofp-util: Flow Dump Protocol for OpenFlow 12
On Tue, Nov 20, 2012 at 09:39:56AM +0900, Simon Horman wrote:
> On Mon, Nov 19, 2012 at 10:13:22AM -0800, Ben Pfaff wrote:
> > On Mon, Nov 19, 2012 at 02:59:29PM +0900, Simon Horman wrote:
> > > Allow only OpenFlow 12 as a flow dump protocol for OpenFlow12.
> > > Allow OpenFlow10 and NXM as flow dump protocols for other OpenFlow
> > > versions.
> > > This only changes the behaviour prior to this patch in the case of
> > > OpenFlow12
> > > which seemed broken as an attempt would be made to set NXM as the
> > > flow dump format using an OpenFlow 1.0 message even though an OpenFlow 1.2
> > > connection had been negotiated..
> > >
> > > Signed-off-by: Simon Horman
> >
> > I think that this is no longer needed with the changes that I made a few
> > patches back. set_protocol_for_flow_dump() will try to set the protocol
> > with try_set_protocol(), which in turn will call
> > ofputil_encode_set_protocol(), which in turn will see that the versions
> > are incompatible, with:
> >
> > cur_version = ofputil_protocol_to_ofp_version(current);
> > want_version = ofputil_protocol_to_ofp_version(want);
> > if (cur_version != want_version) {
> > *next = current;
> > return NULL;
> > }
> >
> > which makes try_set_protocol() return that it failed to set that
> > protocol, which makes set_protocol_for_flow_dump() go on to the next
> > one.
>
> I'm somewhat unsure how you envisage this working.
>
> Without this patch the code you refer to does indeed see that
> all versions are incompatible. But in the end it runs out of options
> and produces a fatal error.
Do we just need to add OFPUTIL_P_OF12_OXM to
ofputil_flow_dump_protocols[], at the beginning?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovs-ofctl: Don't rely on stat() to check unix sockets.
ESX supports unix sockets, but they don't manifest themselves in
file system like they do on Linux. Instead of using stat to check
if a unix socket exist, this patch simply tries to open it instead.
Signed-off-by: Ethan Jackson
---
lib/stream-unix.c |1 -
utilities/ovs-ofctl.c | 40 +---
2 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/lib/stream-unix.c b/lib/stream-unix.c
index 4feefdf..d5534b2 100644
--- a/lib/stream-unix.c
+++ b/lib/stream-unix.c
@@ -48,7 +48,6 @@ unix_open(const char *name, char *suffix, struct stream
**streamp,
fd = make_unix_socket(SOCK_STREAM, true, NULL, connect_path);
if (fd < 0) {
-VLOG_WARN("%s: connection failed (%s)", connect_path, strerror(-fd));
return -fd;
}
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 08c3aa9..363c0a3 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -52,6 +52,7 @@
#include "poll-loop.h"
#include "random.h"
#include "stream-ssl.h"
+#include "socket-util.h"
#include "timeval.h"
#include "unixctl.h"
#include "util.h"
@@ -332,14 +333,20 @@ run(int retval, const char *message, ...)
/* Generic commands. */
-static void
+static int
open_vconn_socket(const char *name, struct vconn **vconnp)
{
char *vconn_name = xasprintf("unix:%s", name);
-VLOG_DBG("connecting to %s", vconn_name);
-run(vconn_open_block(vconn_name, 0, vconnp),
-"connecting to %s", vconn_name);
+int error;
+
+error = vconn_open(vconn_name, 0, vconnp, DSCP_DEFAULT);
+if (error && error != ENOENT) {
+ovs_fatal(0, "%s: failed to open socket (%s)", name,
+ strerror(error));
+}
free(vconn_name);
+
+return error;
}
static enum ofputil_protocol
@@ -350,7 +357,7 @@ open_vconn__(const char *name, const char *default_suffix,
enum ofputil_protocol protocol;
char *bridge_path;
int ofp_version;
-struct stat s;
+int error;
bridge_path = xasprintf("%s/%s.%s", ovs_rundir(), name, default_suffix);
@@ -362,16 +369,12 @@ open_vconn__(const char *name, const char *default_suffix,
if (strchr(name, ':')) {
run(vconn_open_block(name, 0, vconnp), "connecting to %s", name);
-} else if (!stat(name, &s) && S_ISSOCK(s.st_mode)) {
-open_vconn_socket(name, vconnp);
-} else if (!stat(bridge_path, &s) && S_ISSOCK(s.st_mode)) {
-open_vconn_socket(bridge_path, vconnp);
-} else if (!stat(socket_name, &s)) {
-if (!S_ISSOCK(s.st_mode)) {
-ovs_fatal(0, "cannot connect to %s: %s is not a socket",
- name, socket_name);
-}
-open_vconn_socket(socket_name, vconnp);
+} else if (!open_vconn_socket(name, vconnp)) {
+/* Fall Through. */
+} else if (!open_vconn_socket(bridge_path, vconnp)) {
+/* Fall Through. */
+} else if (!open_vconn_socket(socket_name, vconnp)) {
+/* Fall Through. */
} else {
ovs_fatal(0, "%s is not a bridge or a socket", name);
}
@@ -379,6 +382,13 @@ open_vconn__(const char *name, const char *default_suffix,
free(bridge_path);
free(socket_name);
+VLOG_DBG("connecting to %s", vconn_get_name(*vconnp));
+error = vconn_connect_block(*vconnp);
+if (error) {
+ovs_fatal(0, "%s: failed to connect to socket (%s)", name,
+ strerror(error));
+}
+
ofp_version = vconn_get_version(*vconnp);
protocol = ofputil_protocol_from_ofp_version(ofp_version);
if (!protocol) {
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/8] ofp-util: Flow Dump Protocol for OpenFlow 12
On Mon, Nov 19, 2012 at 05:11:21PM -0800, Ben Pfaff wrote:
> On Tue, Nov 20, 2012 at 09:39:56AM +0900, Simon Horman wrote:
> > On Mon, Nov 19, 2012 at 10:13:22AM -0800, Ben Pfaff wrote:
> > > On Mon, Nov 19, 2012 at 02:59:29PM +0900, Simon Horman wrote:
> > > > Allow only OpenFlow 12 as a flow dump protocol for OpenFlow12.
> > > > Allow OpenFlow10 and NXM as flow dump protocols for other OpenFlow
> > > > versions.
> > > > This only changes the behaviour prior to this patch in the case of
> > > > OpenFlow12
> > > > which seemed broken as an attempt would be made to set NXM as the
> > > > flow dump format using an OpenFlow 1.0 message even though an OpenFlow
> > > > 1.2
> > > > connection had been negotiated..
> > > >
> > > > Signed-off-by: Simon Horman
> > >
> > > I think that this is no longer needed with the changes that I made a few
> > > patches back. set_protocol_for_flow_dump() will try to set the protocol
> > > with try_set_protocol(), which in turn will call
> > > ofputil_encode_set_protocol(), which in turn will see that the versions
> > > are incompatible, with:
> > >
> > > cur_version = ofputil_protocol_to_ofp_version(current);
> > > want_version = ofputil_protocol_to_ofp_version(want);
> > > if (cur_version != want_version) {
> > > *next = current;
> > > return NULL;
> > > }
> > >
> > > which makes try_set_protocol() return that it failed to set that
> > > protocol, which makes set_protocol_for_flow_dump() go on to the next
> > > one.
> >
> > I'm somewhat unsure how you envisage this working.
> >
> > Without this patch the code you refer to does indeed see that
> > all versions are incompatible. But in the end it runs out of options
> > and produces a fatal error.
>
> Do we just need to add OFPUTIL_P_OF12_OXM to
> ofputil_flow_dump_protocols[], at the beginning?
Sorry for missing that. Yes that seems to be sufficient.
An updated patch is below, which should apply to master.
I don't think there are any other changes but
would you like me to re-spin the series anyway?
From: Simon Horman
ofp-util: Flow Dump Protocol for OpenFlow 12
Allow only OpenFlow 12 as a flow dump protocol.
The implementation of set_protocol_for_flow_dump ensures that
this will only be selected if an OpenFlow12 connection has
been negotiated.
Signed-off-by: Simon Horman
---
v7
* As suggested by Ben Pfaff
- Vastly simplify the implementation, it is now
sufficient to simply add OFPUTIL_P_OF12_OXM to the beginning
of ofputil_flow_dump_protocols[].
v6
* No change
v5
* Rebase
v4
* Rebase
---
lib/ofp-util.c |1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1a9d611..e335ff8 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -588,6 +588,7 @@ static const struct proto_abbrev proto_abbrevs[] = {
#define N_PROTO_ABBREVS ARRAY_SIZE(proto_abbrevs)
enum ofputil_protocol ofputil_flow_dump_protocols[] = {
+OFPUTIL_P_OF12_OXM,
OFPUTIL_P_OF10_NXM,
OFPUTIL_P_OF10_STD,
};
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenVswitch dev document
On Mon, Nov 19, 2012 at 3:51 PM, 李强 wrote: > Hi,all. > I'm a student and now going on some project related to OpenVswtich. But > the source code seems so complicated to me. Do you have development document > about openvswitch? If you have, could you please mail me a copy? I have written an article: http://wangcong.org/blog/archives/2131 But it mostly focuses on the kernel part. Hope it helps. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/8] ofp-util: Flow Dump Protocol for OpenFlow 12
On Tue, Nov 20, 2012 at 10:50:39AM +0900, Simon Horman wrote:
> On Mon, Nov 19, 2012 at 05:11:21PM -0800, Ben Pfaff wrote:
> > On Tue, Nov 20, 2012 at 09:39:56AM +0900, Simon Horman wrote:
> > > On Mon, Nov 19, 2012 at 10:13:22AM -0800, Ben Pfaff wrote:
> > > > On Mon, Nov 19, 2012 at 02:59:29PM +0900, Simon Horman wrote:
> > > > > Allow only OpenFlow 12 as a flow dump protocol for OpenFlow12.
> > > > > Allow OpenFlow10 and NXM as flow dump protocols for other OpenFlow
> > > > > versions.
> > > > > This only changes the behaviour prior to this patch in the case of
> > > > > OpenFlow12
> > > > > which seemed broken as an attempt would be made to set NXM as the
> > > > > flow dump format using an OpenFlow 1.0 message even though an
> > > > > OpenFlow 1.2
> > > > > connection had been negotiated..
> > > > >
> > > > > Signed-off-by: Simon Horman
> > > >
> > > > I think that this is no longer needed with the changes that I made a few
> > > > patches back. set_protocol_for_flow_dump() will try to set the protocol
> > > > with try_set_protocol(), which in turn will call
> > > > ofputil_encode_set_protocol(), which in turn will see that the versions
> > > > are incompatible, with:
> > > >
> > > > cur_version = ofputil_protocol_to_ofp_version(current);
> > > > want_version = ofputil_protocol_to_ofp_version(want);
> > > > if (cur_version != want_version) {
> > > > *next = current;
> > > > return NULL;
> > > > }
> > > >
> > > > which makes try_set_protocol() return that it failed to set that
> > > > protocol, which makes set_protocol_for_flow_dump() go on to the next
> > > > one.
> > >
> > > I'm somewhat unsure how you envisage this working.
> > >
> > > Without this patch the code you refer to does indeed see that
> > > all versions are incompatible. But in the end it runs out of options
> > > and produces a fatal error.
> >
> > Do we just need to add OFPUTIL_P_OF12_OXM to
> > ofputil_flow_dump_protocols[], at the beginning?
>
> Sorry for missing that. Yes that seems to be sufficient.
>
> An updated patch is below, which should apply to master.
Applied, thank you!
> I don't think there are any other changes but
> would you like me to re-spin the series anyway?
It is not necessary. I'm going to continue through the series.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
