Re: [ovs-dev] [PATCH 2/2] rhel: Remove the firewall hole that we create for gre.
On 04/13/2013 12:53 AM, Ben Pfaff wrote: > On Fri, Apr 12, 2013 at 01:50:43PM -0700, Gurucharan Shetty wrote: >> Till now, by default, we add firewall holes for >> gre traffic. There may be users that do not use gre tunnels >> and they may be surprised with this behavior. > > It would be nice to add a sentence or a paragraph mentioning why we > leave the hole for XenServer. > > These two patches seem OK to me--I think this is a better approach > overall--but I think it would be nice to complete our conversation > with Lorand in the thread for the patch he posted, and try to reach > consensus, before we apply them. I also lean towards keeping the ports closed by default, but I'm pretty sure there will be several users bitten by this. Perhaps we can add a paragraph to INSTALL.RHEL and INSTALL.XenServer (and the FAQ?) about some tunnel ports needing holes in the firewall, and how to "properly" configure OVS so the necessary ports are opened automatically on system and OVS restart (and closed on OVS stop). -Lori ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL: Admit that only GNU make works.
On 15 March 2013 17:30, Ben Pfaff wrote: > I promised some time ago to take a look at fixing the behavior of the > Makefiles with non-GNU make, but it doesn't realistically seem that I will. > Thanks. Even if we addressed the current issues I wouldn't be surprised to find new failures appearing in the future, so agree that just being explicit that GNU make is required is best. -Ed ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/1] [branch-1.4] [ofproto-dpif] Memory leak at specific PACKET_INs
When the packet is sent to the controller due to an userspace rule (and not
a kernel-space flow), execute_controller_action is invoked with clone=true,
so handle_flow_miss retains ownership of the packet buffer. But if it returns
true (which means the packet had only a PACKET_IN action), nothing frees up
the buffer.
The problem has seen only in 1.4 branch.
Tested with XenServer 6.1:
- set up the controller, DVSC is fine
- add a rule with high priority to force everything to the controller:
ovs-ofctl add-flow [mgmt int] priority=65535,actions=controller
- send a bunch of UDP packets to random ports to trigger misses in kernel:
mz -B [mgmt IP] -d 100m eth0 -t udp sp=3000,dp=1024-65535,iplen=1400
- track memory consumption over time
Signed-off-by: Zoltan Kiss
---
ofproto/ofproto-dpif.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2949085..0a4d0f4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2570,6 +2570,8 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct
flow_miss *miss,
: xmemdup(subfacet->actions, subfacet->actions_len));
execute->actions_len = subfacet->actions_len;
execute->packet = packet;
+} else {
+ofpbuf_delete(packet);
}
}
--
1.7.0.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/1] [branch-1.4] [ofproto-dpif] Memory leak at specific PACKET_INs
On Mon, Apr 15, 2013 at 03:59:52PM +0100, Zoltan Kiss wrote: > When the packet is sent to the controller due to an userspace rule (and not > a kernel-space flow), execute_controller_action is invoked with clone=true, > so handle_flow_miss retains ownership of the packet buffer. But if it returns > true (which means the packet had only a PACKET_IN action), nothing frees up > the buffer. > The problem has seen only in 1.4 branch. > Tested with XenServer 6.1: > - set up the controller, DVSC is fine > - add a rule with high priority to force everything to the controller: > ovs-ofctl add-flow [mgmt int] priority=65535,actions=controller > - send a bunch of UDP packets to random ports to trigger misses in kernel: > mz -B [mgmt IP] -d 100m eth0 -t udp sp=3000,dp=1024-65535,iplen=1400 > - track memory consumption over time > > Signed-off-by: Zoltan Kiss I think you're right. But in that case, wouldn't it solve the problem in a better way (doing less memory allocation and copying) by passing clone=false, instead of passing clone=true and then freeing the packet in the caller? Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] rhel: Remove the firewall hole that we create for gre.
On Mon, Apr 15, 2013 at 3:42 AM, Lori Jakab wrote: > On 04/13/2013 12:53 AM, Ben Pfaff wrote: > > On Fri, Apr 12, 2013 at 01:50:43PM -0700, Gurucharan Shetty wrote: > >> Till now, by default, we add firewall holes for > >> gre traffic. There may be users that do not use gre tunnels > >> and they may be surprised with this behavior. > > > > It would be nice to add a sentence or a paragraph mentioning why we > > leave the hole for XenServer. > > > > These two patches seem OK to me--I think this is a better approach > > overall--but I think it would be nice to complete our conversation > > with Lorand in the thread for the patch he posted, and try to reach > > consensus, before we apply them. > > I also lean towards keeping the ports closed by default, but I'm pretty > sure there will be several users bitten by this. Perhaps we can add a > paragraph to INSTALL.RHEL and INSTALL.XenServer (and the FAQ?) about > some tunnel ports needing holes in the firewall, and how to "properly" > configure OVS so the necessary ports are opened automatically on system > and OVS restart (and closed on OVS stop). > Thanks, I think we are all on the same page then. I will send in a patch for the documentation update. > > -Lori > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 1/1] debian: build with debugging symbol
On Mon, Apr 15, 2013 at 02:54:10AM -0400, Zang MingJie wrote: > > Signed-off-by: Zang MingJie Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] rhel: Remove the firewall hole that we create for gre.
On Mon, Apr 15, 2013 at 10:16:53AM -0700, Gurucharan Shetty wrote: > On Mon, Apr 15, 2013 at 3:42 AM, Lori Jakab wrote: > > > On 04/13/2013 12:53 AM, Ben Pfaff wrote: > > > On Fri, Apr 12, 2013 at 01:50:43PM -0700, Gurucharan Shetty wrote: > > >> Till now, by default, we add firewall holes for > > >> gre traffic. There may be users that do not use gre tunnels > > >> and they may be surprised with this behavior. > > > > > > It would be nice to add a sentence or a paragraph mentioning why we > > > leave the hole for XenServer. > > > > > > These two patches seem OK to me--I think this is a better approach > > > overall--but I think it would be nice to complete our conversation > > > with Lorand in the thread for the patch he posted, and try to reach > > > consensus, before we apply them. > > > > I also lean towards keeping the ports closed by default, but I'm pretty > > sure there will be several users bitten by this. Perhaps we can add a > > paragraph to INSTALL.RHEL and INSTALL.XenServer (and the FAQ?) about > > some tunnel ports needing holes in the firewall, and how to "properly" > > configure OVS so the necessary ports are opened automatically on system > > and OVS restart (and closed on OVS stop). > > > > Thanks, I think we are all on the same page then. I will send in a patch > for the > documentation update. Let's add an item to NEWS also. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] INSTALL.XenServer, INSTALL.RHEL: Add a note for tunnel firewall rules.
Signed-off-by: Gurucharan Shetty --- INSTALL.RHEL |6 ++ INSTALL.XenServer | 13 - 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/INSTALL.RHEL b/INSTALL.RHEL index eaa2e7c..a698fae 100644 --- a/INSTALL.RHEL +++ b/INSTALL.RHEL @@ -101,6 +101,12 @@ RHEL. On RHEL 5, the default RPM source directory is in this example: "kmod-openvswitch", "kmod-openvswitch-debug", and "kmod-openvswitch-kdump". +A RHEL host has default firewall rules that prevent any Open vSwitch tunnel +traffic from passing through. If a user configures Open vSwitch tunnels like +GRE, VXLAN, LISP etc., they will either have to manually add iptables firewall +rules to allow the tunnel traffic or add it through a startup script (Please +refer to the "enable-protocol" command in the ovs-ctl(8) manpage). + Red Hat Network Scripts Integration --- diff --git a/INSTALL.XenServer b/INSTALL.XenServer index 7a4dd76..e31788a 100644 --- a/INSTALL.XenServer +++ b/INSTALL.XenServer @@ -158,7 +158,10 @@ command. The plugin script does roughly the following: * If XAPI is configured for a manager, configures the OVS manager to match with "ovs-vsctl set-manager". -The Open vSwitch boot sequence only configures an OVS configuration +Notes +- + +* The Open vSwitch boot sequence only configures an OVS configuration database manager. There is no way to directly configure an OpenFlow controller on XenServer and, as a consequence of the step above that deletes all of the bridges at boot time, controller configuration only @@ -166,6 +169,14 @@ persists until XenServer reboot. The configuration database manager can, however, configure controllers for bridges. See the BUGS section of ovs-controller(8) for more information on this topic. +* The Open vSwitch startup script automatically adds a firewall rule +to allow GRE traffic. This rule is needed for the XenServer feature +called "Cross-Host Internal Networks" (CHIN) that uses GRE. If a user +configures tunnels other than GRE (ex: VXLAN, LISP), they will have +to either manually add a iptables firewall rule to allow the tunnel traffic +or add it through a startup script (Please refer to the "enable-protocol" +command in the ovs-ctl(8) manpage). + Reporting Bugs -- -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] NEWS: Add a note about the removal of GRE firewall rule in RHEL.
Signed-off-by: Gurucharan Shetty --- NEWS |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 39e6e5d..f23b366 100644 --- a/NEWS +++ b/NEWS @@ -13,7 +13,10 @@ post-v1.10.0 - ovs-dpctl: * New debugging commands "add-flow", "mod-flow", "del-flow". - New syslog format, prefixed with "ovs|", to be easier to filter. - +- RHEL: Removes the default firewall rule that allowed GRE traffic to + pass through. Any users that relied on this automatic firewall hole + will have to manually configure it. The ovs-ctl(8) manpage documents + the "enable-protocol" command that can be used as an alternative. v1.10.0 - xx xxx - -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] INSTALL.XenServer, INSTALL.RHEL: Add a note for tunnel firewall rules.
On Mon, Apr 15, 2013 at 10:18:03AM -0700, Gurucharan Shetty wrote: > Signed-off-by: Gurucharan Shetty It looks to me like these documentation updates are necessary as a consequence of changes that we make in other commits. Usually it is better to do a code change and the corresponding documentation update in a single commit if we can, so that in every commit the code and the documentation match up. Would it therefore make sense to squash these two changes together with the corresponding code changes? Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] INSTALL.XenServer, INSTALL.RHEL: Add a note for tunnel firewall rules.
On Mon, Apr 15, 2013 at 11:07 AM, Ben Pfaff wrote: > On Mon, Apr 15, 2013 at 10:18:03AM -0700, Gurucharan Shetty wrote: > > Signed-off-by: Gurucharan Shetty > > It looks to me like these documentation updates are necessary as a > consequence of changes that we make in other commits. Usually it is > better to do a code change and the corresponding documentation update > in a single commit if we can, so that in every commit the code and the > documentation match up. Would it therefore make sense to squash these > two changes together with the corresponding code changes? > > Thanks, > > Ben. > Okay. I will squash the commits and send them. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] INSTALL.XenServer, INSTALL.RHEL: Add a note for tunnel firewall rules.
The wording looks good to me, thanks! On 04/15/2013 08:18 PM, Gurucharan Shetty wrote: > Signed-off-by: Gurucharan Shetty > --- > INSTALL.RHEL |6 ++ > INSTALL.XenServer | 13 - > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/INSTALL.RHEL b/INSTALL.RHEL > index eaa2e7c..a698fae 100644 > --- a/INSTALL.RHEL > +++ b/INSTALL.RHEL > @@ -101,6 +101,12 @@ RHEL. On RHEL 5, the default RPM source directory is > in this example: "kmod-openvswitch", "kmod-openvswitch-debug", and > "kmod-openvswitch-kdump". > > +A RHEL host has default firewall rules that prevent any Open vSwitch tunnel > +traffic from passing through. If a user configures Open vSwitch tunnels like > +GRE, VXLAN, LISP etc., they will either have to manually add iptables > firewall > +rules to allow the tunnel traffic or add it through a startup script (Please > +refer to the "enable-protocol" command in the ovs-ctl(8) manpage). > + > Red Hat Network Scripts Integration > --- > > diff --git a/INSTALL.XenServer b/INSTALL.XenServer > index 7a4dd76..e31788a 100644 > --- a/INSTALL.XenServer > +++ b/INSTALL.XenServer > @@ -158,7 +158,10 @@ command. The plugin script does roughly the following: > * If XAPI is configured for a manager, configures the OVS >manager to match with "ovs-vsctl set-manager". > > -The Open vSwitch boot sequence only configures an OVS configuration > +Notes > +- > + > +* The Open vSwitch boot sequence only configures an OVS configuration > database manager. There is no way to directly configure an OpenFlow > controller on XenServer and, as a consequence of the step above that > deletes all of the bridges at boot time, controller configuration only > @@ -166,6 +169,14 @@ persists until XenServer reboot. The configuration > database manager > can, however, configure controllers for bridges. See the BUGS section > of ovs-controller(8) for more information on this topic. > > +* The Open vSwitch startup script automatically adds a firewall rule > +to allow GRE traffic. This rule is needed for the XenServer feature > +called "Cross-Host Internal Networks" (CHIN) that uses GRE. If a user > +configures tunnels other than GRE (ex: VXLAN, LISP), they will have > +to either manually add a iptables firewall rule to allow the tunnel traffic > +or add it through a startup script (Please refer to the "enable-protocol" > +command in the ovs-ctl(8) manpage). > + > Reporting Bugs > -- > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] datapath: Fix stale comment in flow.c.
Signed-off-by: Pravin B Shelar --- datapath/flow.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/datapath/flow.c b/datapath/flow.c index 841e8be..dce237e 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -1265,9 +1265,9 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp, /** * ovs_flow_metadata_from_nlattrs - parses Netlink attributes into a flow key. - * @in_port: receives the extracted input port. - * @tun_id: receives the extracted tunnel ID. - * @key: Netlink attribute holding nested %OVS_KEY_ATTR_* Netlink attribute + * @flow: Receives extracted in_port, priority, tun_key and skb_mark. + * @key_len: Length of key in @flow. Used for calculating flow hash. + * @attr: Netlink attribute holding nested %OVS_KEY_ATTR_* Netlink attribute * sequence. * * This parses a series of Netlink attributes that form a flow key, which must -- 1.7.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 2/2] INSTALL.XenServer: Add a note for tunnel firewall rules.
Signed-off-by: Gurucharan Shetty --- INSTALL.XenServer | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/INSTALL.XenServer b/INSTALL.XenServer index 7a4dd76..e31788a 100644 --- a/INSTALL.XenServer +++ b/INSTALL.XenServer @@ -158,7 +158,10 @@ command. The plugin script does roughly the following: * If XAPI is configured for a manager, configures the OVS manager to match with "ovs-vsctl set-manager". -The Open vSwitch boot sequence only configures an OVS configuration +Notes +- + +* The Open vSwitch boot sequence only configures an OVS configuration database manager. There is no way to directly configure an OpenFlow controller on XenServer and, as a consequence of the step above that deletes all of the bridges at boot time, controller configuration only @@ -166,6 +169,14 @@ persists until XenServer reboot. The configuration database manager can, however, configure controllers for bridges. See the BUGS section of ovs-controller(8) for more information on this topic. +* The Open vSwitch startup script automatically adds a firewall rule +to allow GRE traffic. This rule is needed for the XenServer feature +called "Cross-Host Internal Networks" (CHIN) that uses GRE. If a user +configures tunnels other than GRE (ex: VXLAN, LISP), they will have +to either manually add a iptables firewall rule to allow the tunnel traffic +or add it through a startup script (Please refer to the "enable-protocol" +command in the ovs-ctl(8) manpage). + Reporting Bugs -- -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 1/2] rhel: Remove the firewall hole that we create for GRE.
Till now, by default, we add firewall holes for
gre traffic. There may be users that do not use GRE tunnels
and they may be surprised with this behavior. So, don't add
the firewall rules by default and update the documentation
to mention the same.
This patch does not remove the default GRE firewall rule for
xenserver because xenserver has a feature called "Cross-Host
Internal Networks" (CHIN) that uses GRE.
Signed-off-by: Gurucharan Shetty
---
INSTALL.RHEL|6 ++
NEWS|5 -
rhel/etc_init.d_openvswitch |2 --
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/INSTALL.RHEL b/INSTALL.RHEL
index eaa2e7c..a698fae 100644
--- a/INSTALL.RHEL
+++ b/INSTALL.RHEL
@@ -101,6 +101,12 @@ RHEL. On RHEL 5, the default RPM source directory is
in this example: "kmod-openvswitch", "kmod-openvswitch-debug", and
"kmod-openvswitch-kdump".
+A RHEL host has default firewall rules that prevent any Open vSwitch tunnel
+traffic from passing through. If a user configures Open vSwitch tunnels like
+GRE, VXLAN, LISP etc., they will either have to manually add iptables firewall
+rules to allow the tunnel traffic or add it through a startup script (Please
+refer to the "enable-protocol" command in the ovs-ctl(8) manpage).
+
Red Hat Network Scripts Integration
---
diff --git a/NEWS b/NEWS
index 39e6e5d..f23b366 100644
--- a/NEWS
+++ b/NEWS
@@ -13,7 +13,10 @@ post-v1.10.0
- ovs-dpctl:
* New debugging commands "add-flow", "mod-flow", "del-flow".
- New syslog format, prefixed with "ovs|", to be easier to filter.
-
+- RHEL: Removes the default firewall rule that allowed GRE traffic to
+ pass through. Any users that relied on this automatic firewall hole
+ will have to manually configure it. The ovs-ctl(8) manpage documents
+ the "enable-protocol" command that can be used as an alternative.
v1.10.0 - xx xxx
-
diff --git a/rhel/etc_init.d_openvswitch b/rhel/etc_init.d_openvswitch
index 55a13a7..7e64132 100755
--- a/rhel/etc_init.d_openvswitch
+++ b/rhel/etc_init.d_openvswitch
@@ -48,8 +48,6 @@ start () {
set "$@" $OVS_CTL_OPTS
"$@"
-ovs_ctl --protocol=gre enable-protocol
-
touch /var/lock/subsys/openvswitch
}
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] sparse: Support recent distributions
On Fri, Apr 05, 2013 at 04:56:52PM -0700, Andy Zhou wrote: > sparse support seems to be broken on some recent Linux distributions. > For example, ubuntu 12.04 with Linux 3.5 kernel, and Debian latest test > distribution, running Linux 3.2 kernel. > > On both systems that sparse was broken, It was not able find the header files > in the default system include directories. GCC finds them by default. > > This patch adds the required GCC default search path when running sparse. > > Tested on: > > Ubuntu 12.04 - w/ linux 3.5 kernel > Debian-6 March test distribution - w/ linux 3.2 kernel > > Signed-off-by: Andy Zhou This approach doesn't make me happy, but after some poking around I can't figure out a better way. One issue to consider: as-is, this adds an extra gcc and sed invocation for every compile (inside ``). That might extend the overall "make" by a second or more. Would you mind measuring the cost? If it is high, then it would be worth doing the gcc and sed invocation just once, at configure time, and then using the results for every "make'. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 1/2] rhel: Remove the firewall hole that we create for GRE.
On Mon, Apr 15, 2013 at 11:12:53AM -0700, Gurucharan Shetty wrote: > Till now, by default, we add firewall holes for > gre traffic. There may be users that do not use GRE tunnels > and they may be surprised with this behavior. So, don't add > the firewall rules by default and update the documentation > to mention the same. > > This patch does not remove the default GRE firewall rule for > xenserver because xenserver has a feature called "Cross-Host > Internal Networks" (CHIN) that uses GRE. > > Signed-off-by: Gurucharan Shetty Both of these patches look good to me. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next] openvswitch: Simplify datapath locking.
Currently OVS uses combination of genl and rtnl lock to protect
datapath state. This was done due to networking stack locking.
But this has complicated locking and there are few lock ordering
issues with new tunneling protocols.
Following patch simplifies locking by introducing new ovs mutex
and now this lock is used to protect entire ovs state.
Signed-off-by: Pravin B Shelar
---
net/openvswitch/datapath.c | 274 +-
net/openvswitch/datapath.h | 69 ++---
net/openvswitch/dp_notify.c | 82 +++---
net/openvswitch/vport-internal_dev.c |6 +
net/openvswitch/vport-netdev.c |8 +-
net/openvswitch/vport.c | 22 +--
net/openvswitch/vport.h |4 +-
7 files changed, 298 insertions(+), 167 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index d406503..b7d0b7c 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -56,21 +57,13 @@
#include "flow.h"
#include "vport-internal_dev.h"
-/**
- * struct ovs_net - Per net-namespace data for ovs.
- * @dps: List of datapaths to enable dumping them all out.
- * Protected by genl_mutex.
- */
-struct ovs_net {
- struct list_head dps;
-};
-
-static int ovs_net_id __read_mostly;
#define REHASH_FLOW_INTERVAL (10 * 60 * HZ)
static void rehash_flow_table(struct work_struct *work);
static DECLARE_DELAYED_WORK(rehash_flow_wq, rehash_flow_table);
+int ovs_net_id __read_mostly;
+
static void ovs_notify(struct sk_buff *skb, struct genl_info *info,
struct genl_multicast_group *grp)
{
@@ -81,20 +74,42 @@ static void ovs_notify(struct sk_buff *skb, struct
genl_info *info,
/**
* DOC: Locking:
*
- * Writes to device state (add/remove datapath, port, set operations on vports,
- * etc.) are protected by RTNL.
- *
- * Writes to other state (flow table modifications, set miscellaneous datapath
- * parameters, etc.) are protected by genl_mutex. The RTNL lock nests inside
- * genl_mutex.
+ * All writes e.g. Writes to device state (add/remove datapath, port, set
+ * operations on vports, etc.), Writes to other state (flow table
+ * modifications, set miscellaneous datapath parameters, etc.) are protected
+ * by ovs_lock.
*
* Reads are protected by RCU.
*
* There are a few special cases (mostly stats) that have their own
* synchronization but they nest under all of above and don't interact with
* each other.
+ *
+ * The RTNL lock nests inside ovs_mutex.
*/
+static DEFINE_MUTEX(ovs_mutex);
+
+void ovs_lock(void)
+{
+ mutex_lock(&ovs_mutex);
+}
+
+void ovs_unlock(void)
+{
+ mutex_unlock(&ovs_mutex);
+}
+
+#ifdef CONFIG_LOCKDEP
+int lockdep_ovsl_is_held(void)
+{
+ if (debug_locks)
+ return lockdep_is_held(&ovs_mutex);
+ else
+ return 1;
+}
+#endif
+
static struct vport *new_vport(const struct vport_parms *);
static int queue_gso_packets(struct net *, int dp_ifindex, struct sk_buff *,
const struct dp_upcall_info *);
@@ -102,7 +117,7 @@ static int queue_userspace_packet(struct net *, int
dp_ifindex,
struct sk_buff *,
const struct dp_upcall_info *);
-/* Must be called with rcu_read_lock, genl_mutex, or RTNL lock. */
+/* Must be called with rcu_read_lock or ovs_mutex. */
static struct datapath *get_dp(struct net *net, int dp_ifindex)
{
struct datapath *dp = NULL;
@@ -120,10 +135,10 @@ static struct datapath *get_dp(struct net *net, int
dp_ifindex)
return dp;
}
-/* Must be called with rcu_read_lock or RTNL lock. */
+/* Must be called with rcu_read_lock or ovs_mutex. */
const char *ovs_dp_name(const struct datapath *dp)
{
- struct vport *vport = ovs_vport_rtnl_rcu(dp, OVSP_LOCAL);
+ struct vport *vport = ovs_vport_ovsl_rcu(dp, OVSP_LOCAL);
return vport->ops->get_name(vport);
}
@@ -175,7 +190,7 @@ struct vport *ovs_lookup_vport(const struct datapath *dp,
u16 port_no)
return NULL;
}
-/* Called with RTNL lock and genl_lock. */
+/* Called with ovs_mutex. */
static struct vport *new_vport(const struct vport_parms *parms)
{
struct vport *vport;
@@ -187,14 +202,12 @@ static struct vport *new_vport(const struct vport_parms
*parms)
hlist_add_head_rcu(&vport->dp_hash_node, head);
}
-
return vport;
}
-/* Called with RTNL lock. */
void ovs_dp_detach_port(struct vport *p)
{
- ASSERT_RTNL();
+ ASSERT_OVSL();
/* First drop references to device. */
hlist_del_rcu(&p->dp_hash_node);
@@ -432,13 +445,13 @@ out:
return err;
}
-/* Called with genl_mutex. */
+/* Called with ovs_mutex. */
static int flush_flows(struct datapath *dp)
{
struct flow_table *old_table;
struct flow_table *new_tab
[ovs-dev] [PATCH net-next] openvswitch: Use generic struct pcpu_tstats.
Rather than defining ovs specific stats struct (vport_percpu_stats),
we can use existing pcpu_tstats to achieve exactly same functionality.
Signed-off-by: Pravin B Shelar
---
net/openvswitch/vport.c | 22 +++---
net/openvswitch/vport.h | 11 ++-
2 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index c90d856..7206231 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -128,7 +128,7 @@ struct vport *ovs_vport_alloc(int priv_size, const struct
vport_ops *ops,
vport->ops = ops;
INIT_HLIST_NODE(&vport->dp_hash_node);
- vport->percpu_stats = alloc_percpu(struct vport_percpu_stats);
+ vport->percpu_stats = alloc_percpu(struct pcpu_tstats);
if (!vport->percpu_stats) {
kfree(vport);
return ERR_PTR(-ENOMEM);
@@ -260,16 +260,16 @@ void ovs_vport_get_stats(struct vport *vport, struct
ovs_vport_stats *stats)
spin_unlock_bh(&vport->stats_lock);
for_each_possible_cpu(i) {
- const struct vport_percpu_stats *percpu_stats;
- struct vport_percpu_stats local_stats;
+ const struct pcpu_tstats *percpu_stats;
+ struct pcpu_tstats local_stats;
unsigned int start;
percpu_stats = per_cpu_ptr(vport->percpu_stats, i);
do {
- start = u64_stats_fetch_begin_bh(&percpu_stats->sync);
+ start = u64_stats_fetch_begin_bh(&percpu_stats->syncp);
local_stats = *percpu_stats;
- } while (u64_stats_fetch_retry_bh(&percpu_stats->sync, start));
+ } while (u64_stats_fetch_retry_bh(&percpu_stats->syncp, start));
stats->rx_bytes += local_stats.rx_bytes;
stats->rx_packets += local_stats.rx_packets;
@@ -327,13 +327,13 @@ int ovs_vport_get_options(const struct vport *vport,
struct sk_buff *skb)
*/
void ovs_vport_receive(struct vport *vport, struct sk_buff *skb)
{
- struct vport_percpu_stats *stats;
+ struct pcpu_tstats *stats;
stats = this_cpu_ptr(vport->percpu_stats);
- u64_stats_update_begin(&stats->sync);
+ u64_stats_update_begin(&stats->syncp);
stats->rx_packets++;
stats->rx_bytes += skb->len;
- u64_stats_update_end(&stats->sync);
+ u64_stats_update_end(&stats->syncp);
ovs_dp_process_received_packet(vport, skb);
}
@@ -352,14 +352,14 @@ int ovs_vport_send(struct vport *vport, struct sk_buff
*skb)
int sent = vport->ops->send(vport, skb);
if (likely(sent)) {
- struct vport_percpu_stats *stats;
+ struct pcpu_tstats *stats;
stats = this_cpu_ptr(vport->percpu_stats);
- u64_stats_update_begin(&stats->sync);
+ u64_stats_update_begin(&stats->syncp);
stats->tx_packets++;
stats->tx_bytes += sent;
- u64_stats_update_end(&stats->sync);
+ u64_stats_update_end(&stats->syncp);
}
return sent;
}
diff --git a/net/openvswitch/vport.h b/net/openvswitch/vport.h
index 7282b84..7ba08c3 100644
--- a/net/openvswitch/vport.h
+++ b/net/openvswitch/vport.h
@@ -19,6 +19,7 @@
#ifndef VPORT_H
#define VPORT_H 1
+#include
#include
#include
#include
@@ -50,14 +51,6 @@ int ovs_vport_send(struct vport *, struct sk_buff *);
/* The following definitions are for implementers of vport devices: */
-struct vport_percpu_stats {
- u64 rx_bytes;
- u64 rx_packets;
- u64 tx_bytes;
- u64 tx_packets;
- struct u64_stats_sync sync;
-};
-
struct vport_err_stats {
u64 rx_dropped;
u64 rx_errors;
@@ -89,7 +82,7 @@ struct vport {
struct hlist_node dp_hash_node;
const struct vport_ops *ops;
- struct vport_percpu_stats __percpu *percpu_stats;
+ struct pcpu_tstats __percpu *percpu_stats;
spinlock_t stats_lock;
struct vport_err_stats err_stats;
--
1.7.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Correctly implement the OpenFlow 1.2+ OXM_OF_IP_DSCP field.
NXM puts the DSCP value in bits 2-7 of NXM_OF_IP_TOS.
OXM puts the DSCP value in bits 0-6 of OXM_OF_IP_DSCP.
Before this commit, Open vSwitch incorrectly implemented OXM_OF_IP_DSCP
with the same format as NXM_OF_IP_TOS. This commit fixes the problem and
adds a test (previously missing but I don't know why).
Reported-by: Hiroshi Miyata
Signed-off-by: Ben Pfaff
---
AUTHORS|1 +
lib/meta-flow.c| 31 +++
lib/meta-flow.h| 12 +++-
lib/nx-match.c |9 ++---
tests/ovs-ofctl.at | 12
5 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index f9343de..b30fd69 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -139,6 +139,7 @@ Hassan Khan hassan.k...@seecs.edu.pk
Hector Oron hector.o...@gmail.com
Henrik Amrenhen...@nicira.com
Hiroshi Tanaka htan...@nicira.com
+Hiroshi Miyata miyahiro.d...@gmail.com
Igor Ganichev iganic...@nicira.com
Jacob Cherkas jcher...@nicira.com
Jad Naous jna...@gmail.com
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 6f7a3aa..9296faa 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -359,6 +359,15 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
MFP_IP_ANY,
true,
NXM_OF_IP_TOS, "NXM_OF_IP_TOS",
+NXM_OF_IP_TOS, "NXM_OF_IP_TOS",
+}, {
+MFF_IP_DSCP_SHIFTED, "nw_tos_shifted", NULL,
+MF_FIELD_SIZES(u8),
+MFM_NONE,
+MFS_DECIMAL,
+MFP_IP_ANY,
+true,
+OXM_OF_IP_DSCP, "OXM_OF_IP_DSCP",
OXM_OF_IP_DSCP, "OXM_OF_IP_DSCP",
}, {
MFF_IP_ECN, "nw_ecn", NULL,
@@ -733,6 +742,7 @@ mf_is_all_wild(const struct mf_field *mf, const struct
flow_wildcards *wc)
case MFF_IP_PROTO:
return !wc->masks.nw_proto;
case MFF_IP_DSCP:
+case MFF_IP_DSCP_SHIFTED:
return !(wc->masks.nw_tos & IP_DSCP_MASK);
case MFF_IP_ECN:
return !(wc->masks.nw_tos & IP_ECN_MASK);
@@ -916,6 +926,8 @@ mf_is_value_valid(const struct mf_field *mf, const union
mf_value *value)
case MFF_IP_DSCP:
return !(value->u8 & ~IP_DSCP_MASK);
+case MFF_IP_DSCP_SHIFTED:
+return !(value->u8 & (~IP_DSCP_MASK >> 2));
case MFF_IP_ECN:
return !(value->u8 & ~IP_ECN_MASK);
case MFF_IP_FRAG:
@@ -1065,6 +1077,10 @@ mf_get_value(const struct mf_field *mf, const struct
flow *flow,
value->u8 = flow->nw_tos & IP_DSCP_MASK;
break;
+case MFF_IP_DSCP_SHIFTED:
+value->u8 = flow->nw_tos >> 2;
+break;
+
case MFF_IP_ECN:
value->u8 = flow->nw_tos & IP_ECN_MASK;
break;
@@ -1244,6 +1260,10 @@ mf_set_value(const struct mf_field *mf,
match_set_nw_dscp(match, value->u8);
break;
+case MFF_IP_DSCP_SHIFTED:
+match_set_nw_dscp(match, value->u8 << 2);
+break;
+
case MFF_IP_ECN:
match_set_nw_ecn(match, value->u8);
break;
@@ -1424,6 +1444,11 @@ mf_set_flow_value(const struct mf_field *mf,
flow->nw_tos |= value->u8 & IP_DSCP_MASK;
break;
+case MFF_IP_DSCP_SHIFTED:
+flow->nw_tos &= ~IP_DSCP_MASK;
+flow->nw_tos |= value->u8 << 2;
+break;
+
case MFF_IP_ECN:
flow->nw_tos &= ~IP_ECN_MASK;
flow->nw_tos |= value->u8 & IP_ECN_MASK;
@@ -1624,6 +1649,7 @@ mf_set_wild(const struct mf_field *mf, struct match
*match)
break;
case MFF_IP_DSCP:
+case MFF_IP_DSCP_SHIFTED:
match->wc.masks.nw_tos &= ~IP_DSCP_MASK;
match->flow.nw_tos &= ~IP_DSCP_MASK;
break;
@@ -1726,6 +1752,7 @@ mf_set(const struct mf_field *mf,
case MFF_IP_PROTO:
case MFF_IP_TTL:
case MFF_IP_DSCP:
+case MFF_IP_DSCP_SHIFTED:
case MFF_IP_ECN:
case MFF_ARP_OP:
case MFF_ICMPV4_TYPE:
@@ -1956,6 +1983,10 @@ mf_random_value(const struct mf_field *mf, union
mf_value *value)
value->u8 &= IP_DSCP_MASK;
break;
+case MFF_IP_DSCP_SHIFTED:
+value->u8 &= IP_DSCP_MASK >> 2;
+break;
+
case MFF_IP_ECN:
value->u8 &= IP_ECN_MASK;
break;
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 57f6df5..9577a10 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -91,8 +91,18 @@ enum mf_field_id {
MFF_IPV6_DST, /* ipv6 */
MFF_IPV6_LABEL, /* be32 */
+/* The IPv4/IPv6 DSCP field has two different views:
+ *
+ * - MFF_IP_DSCP has the DSCP in bits 2-7, their bit positions in the
+ * IPv4 and IPv6 "traffic class" field, as used in OpenFlow 1.0 and 1.1
+ * flow format and in NXM's NXM_OF_IP_TOS
+ *
Re: [ovs-dev] [PATCH v2] datapath: Fix stale comment in flow.c.
On Mon, Apr 15, 2013 at 11:32 AM, Pravin B Shelar wrote: > Signed-off-by: Pravin B Shelar Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] list: Style fixes.
Signed-off-by: Ben Pfaff
---
lib/list.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/list.c b/lib/list.c
index fd06b81..227546e 100644
--- a/lib/list.c
+++ b/lib/list.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -46,8 +46,9 @@ list_insert(struct list *before, struct list *elem)
void
list_splice(struct list *before, struct list *first, struct list *last)
{
-if (first == last)
+if (first == last) {
return;
+}
last = last->prev;
/* Cleanly remove 'first'...'last' from its current list. */
@@ -160,8 +161,9 @@ list_size(const struct list *list)
const struct list *e;
size_t cnt = 0;
-for (e = list->next; e != list; e = e->next)
+for (e = list->next; e != list; e = e->next) {
cnt++;
+}
return cnt;
}
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] list: Style fixes.
Looks good.
On Mon, Apr 15, 2013 at 2:48 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
> ---
> lib/list.c |8 +---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/list.c b/lib/list.c
> index fd06b81..227546e 100644
> --- a/lib/list.c
> +++ b/lib/list.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -46,8 +46,9 @@ list_insert(struct list *before, struct list *elem)
> void
> list_splice(struct list *before, struct list *first, struct list *last)
> {
> -if (first == last)
> +if (first == last) {
> return;
> +}
> last = last->prev;
>
> /* Cleanly remove 'first'...'last' from its current list. */
> @@ -160,8 +161,9 @@ list_size(const struct list *list)
> const struct list *e;
> size_t cnt = 0;
>
> -for (e = list->next; e != list; e = e->next)
> +for (e = list->next; e != list; e = e->next) {
> cnt++;
> +}
> return cnt;
> }
>
> --
> 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 net-next] openvswitch: Simplify datapath locking.
On Mon, Apr 15, 2013 at 1:23 PM, Pravin B Shelar wrote: > Currently OVS uses combination of genl and rtnl lock to protect > datapath state. This was done due to networking stack locking. > But this has complicated locking and there are few lock ordering > issues with new tunneling protocols. > Following patch simplifies locking by introducing new ovs mutex > and now this lock is used to protect entire ovs state. > > Signed-off-by: Pravin B Shelar Applied, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next] openvswitch: Use generic struct pcpu_tstats.
On Mon, Apr 15, 2013 at 1:30 PM, Pravin B Shelar wrote: > Rather than defining ovs specific stats struct (vport_percpu_stats), > we can use existing pcpu_tstats to achieve exactly same functionality. > > Signed-off-by: Pravin B Shelar Also applied. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] sparse: Support recent distributions
This patch does the way you recommended already. On Mon, Apr 15, 2013 at 1:00 PM, Ben Pfaff wrote: > On Fri, Apr 05, 2013 at 04:56:52PM -0700, Andy Zhou wrote: > > sparse support seems to be broken on some recent Linux distributions. > > For example, ubuntu 12.04 with Linux 3.5 kernel, and Debian latest test > > distribution, running Linux 3.2 kernel. > > > > On both systems that sparse was broken, It was not able find the header > files > > in the default system include directories. GCC finds them by default. > > > > This patch adds the required GCC default search path when running sparse. > > > > Tested on: > > > > Ubuntu 12.04 - w/ linux 3.5 kernel > > Debian-6 March test distribution - w/ linux 3.2 kernel > > > > Signed-off-by: Andy Zhou > > This approach doesn't make me happy, but after some poking around I > can't figure out a better way. > > One issue to consider: as-is, this adds an extra gcc and sed > invocation for every compile (inside ``). That might extend the > overall "make" by a second or more. Would you mind measuring the > cost? If it is high, then it would be worth doing the gcc and sed > invocation just once, at configure time, and then using the results > for every "make'. > > Thanks, > > Ben. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] sparse: Support recent distributions
I misunderstood. Thanks, I'm happy with it as is then. On Apr 15, 2013 5:17 PM, "Andy Zhou" wrote: > This patch does the way you recommended already. > > > On Mon, Apr 15, 2013 at 1:00 PM, Ben Pfaff wrote: > >> On Fri, Apr 05, 2013 at 04:56:52PM -0700, Andy Zhou wrote: >> > sparse support seems to be broken on some recent Linux distributions. >> > For example, ubuntu 12.04 with Linux 3.5 kernel, and Debian latest test >> > distribution, running Linux 3.2 kernel. >> > >> > On both systems that sparse was broken, It was not able find the header >> files >> > in the default system include directories. GCC finds them by default. >> > >> > This patch adds the required GCC default search path when running >> sparse. >> > >> > Tested on: >> > >> > Ubuntu 12.04 - w/ linux 3.5 kernel >> > Debian-6 March test distribution - w/ linux 3.2 kernel >> > >> > Signed-off-by: Andy Zhou >> >> This approach doesn't make me happy, but after some poking around I >> can't figure out a better way. >> >> One issue to consider: as-is, this adds an extra gcc and sed >> invocation for every compile (inside ``). That might extend the >> overall "make" by a second or more. Would you mind measuring the >> cost? If it is high, then it would be worth doing the gcc and sed >> invocation just once, at configure time, and then using the results >> for every "make'. >> >> Thanks, >> >> Ben. >> > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] python/ovs/poller.py: workaround an eventlet bug
Signed-off-by: YAMAMOTO Takashi
---
python/ovs/poller.py | 13 +
1 file changed, 13 insertions(+)
diff --git a/python/ovs/poller.py b/python/ovs/poller.py
index 7d15f3e..ffd6a39 100644
--- a/python/ovs/poller.py
+++ b/python/ovs/poller.py
@@ -18,6 +18,15 @@ import ovs.vlog
import select
import socket
+try:
+import eventlet.patcher
+
+def _using_eventlet_green_select():
+return eventlet.patcher.is_monkey_patched(select)
+except:
+def _using_eventlet_green_select():
+return False
+
vlog = ovs.vlog.Vlog("poller")
POLLIN = 0x001
@@ -59,6 +68,10 @@ class _SelectSelect(object):
timeout = None
else:
timeout = float(timeout) / 1000
+# XXX workaround a bug in eventlet
+# see https://github.com/eventlet/eventlet/pull/25
+if timeout == 0 and _using_eventlet_green_select():
+timeout = 0.1
rlist, wlist, xlist = select.select(self.rlist, self.wlist, self.xlist,
timeout)
--
1.8.0.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
