Re: [ovs-dev] [PATCH] configure.ac: Enable 'tar-ustar' by default

2015-01-06 Thread Finucane, Stephen
> On Mon, Jan 05, 2015 at 08:57:48AM +, Finucane, Stephen wrote:
> > > On Fri, Dec 19, 2014 at 10:19:10AM +, Stephen Finucane wrote:
> > > > Automake defaults to the 'v7' legacy tar format in GNU tar, through
> > > > passing of the '-o' parameter to GNU tar. Enabling this option
> results
> > > > in errors for users with 32 bit UIDs:
> > > >
> > > > $ make dist
> > > > ...
> > > > tardir=openvswitch-2.3.90 && ${TAR-tar} chof - "$tardir" | GZIP=-
> -
> > > best gzip -c >openvswitch-2.3.90.tar.gz
> > > > tar: value 12345678 out of uid_t range 0..2097151
> > > > tar: Exiting with failure status due to previous errors
> > > > make[1]: Leaving directory `/development/ovs'
> > > > ...
> > > >
> > > > The 'tar-ustar' format is a 1988 POSIX standard that allow longer
> file
> > > > names and other niceties. It's use is an option in Automake 1.9+.
> > > > Enable this option.
> > > >
> > > > Signed-off-by: Stephen Finucane 
> > > > Reviewed-by: Mark D. Gray 
> > >
> > > Doesn't this affect every program that uses Automake?  Have you
> > > reported it to the Automake mailing list?  Is there an upstream fix?
> >
> > In theory, yes - this would affect every user with a 32bit UID who wishes
> to use the auto-generated 'dist' target or its variants. I don't think it's
> a bug per se - more of a legacy issue (the older tar, emulated by 'tar -o',
> simply didn't support 32bit UIDs).
> >
> > I had hoped there would be a 'configure' option or other command line way
> to change the default tar executable, but neither the documentation nor the
> Autotools source revealed such an option.
> >
> > I'll ping the Automake mailing list to confirm that this is the best way
> to approach this, just in case.
> 
> Thanks.

FYI - http://lists.gnu.org/archive/html/autoconf/2015-01/msg00017.html

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net] gso: do GSO for local skb with size bigger than MTU

2015-01-06 Thread Fan Du


On 2015/1/6 1:58, Jesse Gross wrote:

On Mon, Jan 5, 2015 at 1:02 AM, Fan Du  wrote:

于 2014年12月03日 10:31, Du, Fan 写道:




-Original Message-
From: Thomas Graf [mailto:t...@infradead.org] On Behalf Of Thomas Graf
Sent: Wednesday, December 3, 2014 1:42 AM
To: Michael S. Tsirkin
Cc: Du, Fan; 'Jason Wang'; net...@vger.kernel.org; da...@davemloft.net;
f...@strlen.de; dev@openvswitch.org; je...@nicira.com; pshe...@nicira.com
Subject: Re: [PATCH net] gso: do GSO for local skb with size bigger than
MTU

On 12/02/14 at 07:34pm, Michael S. Tsirkin wrote:

On Tue, Dec 02, 2014 at 05:09:27PM +, Thomas Graf wrote:

On 12/02/14 at 01:48pm, Flavio Leitner wrote:

What about containers or any other virtualization environment that
doesn't use Virtio?


The host can dictate the MTU in that case for both veth or OVS
internal which would be primary container plumbing techniques.


It typically can't do this easily for VMs with emulated devices:
real ethernet uses a fixed MTU.

IMHO it's confusing to suggest MTU as a fix for this bug, it's an
unrelated optimization.
ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED is the right fix here.


PMTU discovery only resolves the issue if an actual IP stack is running
inside the
VM. This may not be the case at all.

   ^^^

Some thoughts here:

Think otherwise, this is indeed what host stack should forge a
ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED
message with _inner_ skb network and transport header, do whatever type of
encapsulation,
and thereafter push such packet upward to Guest/Container, which make them
feel, the intermediate node
or the peer send such message. PMTU should be expected to work correct.
And such behavior should be shared by all other encapsulation tech if they
are also suffered.


Hi David, Jesse and Thomas

As discussed in here:
https://www.marc.info/?l=linux-netdev&m=141764712631150&w=4 and
quotes from Jesse:
My proposal would be something like this:
  * For L2, reduce the VM MTU to the lowest common denominator on the
segment.
  * For L3, use path MTU discovery or fragment inner packet (i.e.
normal routing behavior).
  * As a last resort (such as if using an old version of virtio in the
guest), fragment the tunnel packet.


For L2, it's a administrative action
For L3, PMTU approach looks better, because once the sender is alerted the
reduced MTU,
packet size after encapsulation will not exceed physical MTU, so no
additional fragments
efforts needed.
For "As a last resort... fragment the tunnel packet", the original patch:
https://www.marc.info/?l=linux-netdev&m=141715655024090&w=4 did the job, but
seems it's
not welcomed.

This needs to be properly integrated into IP processing if it is to
work correctly.

Do you mean the original patch in this thread? yes, it works correctly
in my cloud env. If you has any other concerns, please let me know. :)

One of the reasons for only doing path MTU discovery
for L3 is that it operates seamlessly as part of normal operation -
there is no need to forge addresses or potentially generate ICMP when
on an L2 network. However, this ignores the IP handling that is going
on (note that in OVS it is possible for L3 to be implemented as a set
of flows coming from a controller).

It also should not be VXLAN specific or duplicate VXLAN encapsulation
code. As this is happening before encapsulation, the generated ICMP
does not need to be encapsulated either if it is created in the right
location.

Yes, I agree. GRE share the same issue from the code flow.
Pushing back ICMP msg back without encapsulation without circulating down
to physical device is possible. The "right location" as far as I know
could only be in ovs_vport_send. In addition this probably requires wrapper
route looking up operation for GRE/VXLAN, after get the under layer 
device MTU

from the routing information, then calculate reduced MTU becomes feasible.



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Procedures to compile OVS code

2015-01-06 Thread tech_kals Kals
Hi Experts,

 1) could you please let me know how can i compile OVS code ? I would like
to add few debugs to understand the code flow. Can someone help me ?

2) Is there any way to enable debugs on OVS code ?


Thanks,
Tech.Kals
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v4 3/3] datapath-windows: Add a WFP system provider

2015-01-06 Thread Sorin Vinturis
This patch was enforced by the WHCK logo testing. In order to pass the
Windows Filtering Platform tests we need to add a persistent system
provider.

Signed-off-by: Sorin Vinturis 
Reported-by: Sorin Vinturis 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/65
Acked-by: Nithin Raju 
---
 datapath-windows/ovsext/Datapath.c |  16 +++
 datapath-windows/ovsext/Debug.h|   1 +
 datapath-windows/ovsext/Switch.c   |   1 -
 datapath-windows/ovsext/Switch.h   |   1 +
 datapath-windows/ovsext/TunnelFilter.c | 173 ++---
 datapath-windows/ovsext/TunnelIntf.h   |   8 ++
 6 files changed, 183 insertions(+), 17 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index a818ab9..13bc20c 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -353,19 +353,35 @@ PNDIS_SPIN_LOCK gOvsCtrlLock;
 VOID
 OvsInit()
 {
+HANDLE handle = NULL;
+
 gOvsCtrlLock = &ovsCtrlLockObj;
 NdisAllocateSpinLock(gOvsCtrlLock);
 OvsInitEventQueue();
+
+OvsTunnelEngineOpen(&handle);
+if (handle) {
+OvsTunnelAddSystemProvider(handle);
+}
+OvsTunnelEngineClose(&handle);
 }
 
 VOID
 OvsCleanup()
 {
+HANDLE handle = NULL;
+
 OvsCleanupEventQueue();
 if (gOvsCtrlLock) {
 NdisFreeSpinLock(gOvsCtrlLock);
 gOvsCtrlLock = NULL;
 }
+
+OvsTunnelEngineOpen(&handle);
+if (handle) {
+OvsTunnelRemoveSystemProvider(handle);
+}
+OvsTunnelEngineClose(&handle);
 }
 
 VOID
diff --git a/datapath-windows/ovsext/Debug.h b/datapath-windows/ovsext/Debug.h
index cc9787a..a0da5eb 100644
--- a/datapath-windows/ovsext/Debug.h
+++ b/datapath-windows/ovsext/Debug.h
@@ -39,6 +39,7 @@
 #define OVS_DBG_BUFMGMT  BIT32(19)
 #define OVS_DBG_OTHERS   BIT32(21)
 #define OVS_DBG_NETLINK  BIT32(22)
+#define OVS_DBG_TUNFLT   BIT32(23)
 
 #define OVS_DBG_RESERVED BIT32(31)
 //Please add above OVS_DBG_RESERVED.
diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index 2b68037..a228d8e 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -26,7 +26,6 @@
 #include "Event.h"
 #include "Flow.h"
 #include "IpHelper.h"
-#include "TunnelIntf.h"
 #include "Oid.h"
 
 #ifdef OVS_DBG_MOD
diff --git a/datapath-windows/ovsext/Switch.h b/datapath-windows/ovsext/Switch.h
index 61f74c4..7960072 100644
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -23,6 +23,7 @@
 
 #include "NetProto.h"
 #include "BufferMgmt.h"
+#include "TunnelIntf.h"
 #define OVS_MAX_VPORT_ARRAY_SIZE 1024
 #define OVS_MAX_PID_ARRAY_SIZE   1024
 
diff --git a/datapath-windows/ovsext/TunnelFilter.c 
b/datapath-windows/ovsext/TunnelFilter.c
index 7250c24..e0adc37 100644
--- a/datapath-windows/ovsext/TunnelFilter.c
+++ b/datapath-windows/ovsext/TunnelFilter.c
@@ -18,6 +18,11 @@
 
 #pragma warning(push)
 #pragma warning(disable:4201)   // unnamed struct/union
+#ifdef OVS_DBG_MOD
+#undef OVS_DBG_MOD
+#endif
+#define OVS_DBG_MOD OVS_DBG_TUNFLT
+#include "Debug.h"
 
 
 #include 
@@ -40,6 +45,23 @@
 #define INITGUID
 #include 
 
+/* Infinite timeout */
+#define INFINITE0x
+
+/*
+ * The provider name should always match the provider string from the install
+ * file.
+ */
+#define OVS_TUNNEL_PROVIDER_NAMEL"Open vSwitch"
+
+/*
+ * The provider description should always contain the OVS service description
+ * string from the install file.
+ */
+#define OVS_TUNNEL_PROVIDER_DESCL"Open vSwitch Extension tunnel 
provider"
+
+/* The session name isn't required but it's useful for diagnostics. */
+#define OVS_TUNNEL_SESSION_NAME L"OVS tunnel session"
 
 /* Configurable parameters (addresses and ports are in host order) */
 UINT16   configNewDestPort = VXLAN_UDP_PORT;
@@ -65,6 +87,15 @@ DEFINE_GUID(
 0x94, 0xc9, 0xf0, 0xd5, 0x25, 0xbb, 0xc1, 0x69
 );
 
+/* 6fc957d7-14e7-47c7-812b-4668be994ba1 */
+DEFINE_GUID(
+OVS_TUNNEL_PROVIDER_KEY,
+0x6fc957d7,
+0x14e7,
+0x47c7,
+0x81, 0x2b, 0x46, 0x68, 0xbe, 0x99, 0x4b, 0xa1
+);
+
 /* bfd4814c-9650-4de3-a536-1eedb9e9ba6a */
 DEFINE_GUID(
 OVS_TUNNEL_FILTER_KEY,
@@ -79,13 +110,128 @@ DEFINE_GUID(
  */
 PDEVICE_OBJECT gDeviceObject;
 
-HANDLE gEngineHandle;
+HANDLE gEngineHandle = NULL;
 UINT32 gCalloutIdV4;
 
 
 /* Callout driver implementation */
 
 NTSTATUS
+OvsTunnelEngineOpen(HANDLE *handle)
+{
+NTSTATUS status = STATUS_SUCCESS;
+FWPM_SESSION session = { 0 };
+
+/* The session name isn't required but may be useful for diagnostics. */
+session.displayData.name = OVS_TUNNEL_SESSION_NAME;
+/*
+* Set an infinite wait timeout, so we don't have to handle FWP_E_TIMEOUT
+* errors while waiting to acquire the transaction lock.
+*/
+session.txnWaitTimeoutInMSec = INFINITE;
+session.flags = FWPM_SESSION_FLAG_DYNAMIC;
+
+/* The authentication service should alwa

Re: [ovs-dev] [PATCH v4 3/3] datapath-windows: Add a WFP system provider

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 10:56:49AM +, Sorin Vinturis wrote:
> This patch was enforced by the WHCK logo testing. In order to pass the
> Windows Filtering Platform tests we need to add a persistent system
> provider.
> 
> Signed-off-by: Sorin Vinturis 
> Reported-by: Sorin Vinturis 
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/65
> Acked-by: Nithin Raju 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] Procedures to compile OVS code

2015-01-06 Thread Ben Pfaff
[dropping discuss-request, which is not a mailing list]

On Tue, Jan 06, 2015 at 04:23:58PM +0530, tech_kals Kals wrote:
>  1) could you please let me know how can i compile OVS code ? I would like
> to add few debugs to understand the code flow. Can someone help me ?

Read INSTALL.md.

> 2) Is there any way to enable debugs on OVS code ?

You might want to turn up the log level, with "-v" or "ovs-appctl
vlog/set".
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] configure.ac: Enable 'tar-ustar' by default

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 09:14:51AM +, Finucane, Stephen wrote:
> > On Mon, Jan 05, 2015 at 08:57:48AM +, Finucane, Stephen wrote:
> > > > On Fri, Dec 19, 2014 at 10:19:10AM +, Stephen Finucane wrote:
> > > > > Automake defaults to the 'v7' legacy tar format in GNU tar, through
> > > > > passing of the '-o' parameter to GNU tar. Enabling this option
> > results
> > > > > in errors for users with 32 bit UIDs:
> > > > >
> > > > > $ make dist
> > > > > ...
> > > > > tardir=openvswitch-2.3.90 && ${TAR-tar} chof - "$tardir" | GZIP=-
> > -
> > > > best gzip -c >openvswitch-2.3.90.tar.gz
> > > > > tar: value 12345678 out of uid_t range 0..2097151
> > > > > tar: Exiting with failure status due to previous errors
> > > > > make[1]: Leaving directory `/development/ovs'
> > > > > ...
> > > > >
> > > > > The 'tar-ustar' format is a 1988 POSIX standard that allow longer
> > file
> > > > > names and other niceties. It's use is an option in Automake 1.9+.
> > > > > Enable this option.
> > > > >
> > > > > Signed-off-by: Stephen Finucane 
> > > > > Reviewed-by: Mark D. Gray 
> > > >
> > > > Doesn't this affect every program that uses Automake?  Have you
> > > > reported it to the Automake mailing list?  Is there an upstream fix?
> > >
> > > In theory, yes - this would affect every user with a 32bit UID who wishes
> > to use the auto-generated 'dist' target or its variants. I don't think it's
> > a bug per se - more of a legacy issue (the older tar, emulated by 'tar -o',
> > simply didn't support 32bit UIDs).
> > >
> > > I had hoped there would be a 'configure' option or other command line way
> > to change the default tar executable, but neither the documentation nor the
> > Autotools source revealed such an option.
> > >
> > > I'll ping the Automake mailing list to confirm that this is the best way
> > to approach this, just in case.
> > 
> > Thanks.
> 
> FYI - http://lists.gnu.org/archive/html/autoconf/2015-01/msg00017.html

Thanks for the followup.  I hope that the Automake maintainers consider
the issue.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] Building OVS on Ubuntu 12.04 stuck in atomic operation unit test

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 09:01:38PM +0900, Motonori Shindo wrote:
> When building OVS (the latest master) under Ubuntu 12.04, it stuck at the 
> unit test "test atomic operations” and never finishes (or it could just be 
> unbearably slow). This problem doesn’t happen in Ubuntu 14.04, however. 
> 
> library unit tests
> 
>  21: test flow extractor ok
>  22: test TCP/IP checksummingok
>  23: test hash functions ok
>  24: test hash map   ok
>  25: test hash index ok
>  26: test cuckoo hashok
>  27: test atomic operations  
> 
> This could be because gcc in Ubuntu 12.04 is too old that it can’t
> handle atomic operations very well. Please see more details below
> about two environments I experimented. Is this a known issue?

We've heard similar reports before but it's challenging to find the
problem.  I can't reproduce the problem on my 32-bit Debian system by
just, for example, switching to GCC 4.6.

What's in config.h and config.log?

How many cores does the system running the build have?

(It would be equally useful to have these answers from anyone
experiencing this problem.)

Thanks,

Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: set the nlBuf tail properly

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 07:17:01PM +, Alin Serdean wrote:
> Move the the tail of the netlink buffer accordingly to the input data.
> Currently _MapFlowStatsToNlStats overrides the netlink header information.
> 
> 
> Signed-off-by: Alin Gabriel Serdean 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Consistently include VLAN header in flow and port stats.

2015-01-06 Thread Ben Pfaff
On Fri, Jan 02, 2015 at 06:16:33PM -0800, Pravin Shelar wrote:
> On Fri, Jan 2, 2015 at 2:54 PM, Ben Pfaff  wrote:
> > Until now, when VLAN acceleration was in use, the bytes of the VLAN header
> > were not included in port or flow byte counters.  They were however
> > included when VLAN acceleration was not used.  This commit corrects the
> > inconsistency, by always including the VLAN header in byte counters.
> >
> > Previous discussion at
> > http://openvswitch.org/pipermail/dev/2014-December/049521.html
> >
> > Already committed to upstream Linux netdev tree as
> > 24cc59d1ebaac54d933dc0b30abcd8bd86193eef.
> >
> > Reported-by: Motonori Shindo 
> > Signed-off-by: Ben Pfaff 
> > Reviewed-by: Flavio Leitner 
> > Acked-by: Pravin B Shelar 
> 
> LGTM.

Thanks, applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Consistently include VLAN header in flow and port stats.

2015-01-06 Thread Ben Pfaff
On Fri, Jan 02, 2015 at 06:16:17PM -0800, Pravin Shelar wrote:
> On Fri, Jan 2, 2015 at 3:03 PM, Ben Pfaff  wrote:
> > Pravin, you already acked this on netdev.  This is the crossport to
> > the OVS tree.  I didn't know whether the netdev ack was good enough to
> > just push to OVS too, so to play it safe I separately posted it for
> > review here.
> >
> 
> It is good idea to post the cross port on ovs-dev, specially compat
> code is involved.

OK, I'll do that then.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: set the nlBuf tail properly

2015-01-06 Thread Alin Serdean
Hi Nithin,

https://github.com/openvswitch/ovs-issues/issues/59 was solved some time ago. 
I don't remember exactly the patch.

I will close the issue.

Thanks,
Alin.

-Mesaj original-
De la: Nithin Raju [mailto:nit...@vmware.com] 
Trimis: Monday, January 5, 2015 10:36 PM
Către: Alin Serdean
Cc: dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapath-windows: set the nlBuf tail properly

On Jan 5, 2015, at 11:17 AM, Alin Serdean 
 wrote:

> Move the the tail of the netlink buffer accordingly to the input data.
> Currently _MapFlowStatsToNlStats overrides the netlink header information.
> 
> 
> Signed-off-by: Alin Gabriel Serdean 

Acked-by: Nithin Raju 

Thanks for working on this. Does this solve 
https://github.com/openvswitch/ovs-issues/issues/59?

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofproto: Don't count hidden rules in table stats.

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 01:03:59PM -0800, Jarno Rajahalme wrote:
> Looks good to me, with a small nit/comment below,
> 
> Acked-by: Jarno Rajahalme 
> 
>   Jarno
> 
> On Jan 1, 2015, at 11:06 AM, Ben Pfaff  wrote:
> 
> > The hidden rules created by in-band control and fail-open should not be
> > included in the table stats reported via OpenFlow.  I seem to recall that
> > this was done correctly in some previous version but it has broken since
> > then.  This commit fixes the problem and adds a test that should make it
> > harder to break again in the future.
> > 
> > Reported-by: Ashok Chippa 
> > Signed-off-by: Ben Pfaff 
> > ---
> > ofproto/connmgr.c   | 19 ++-
> > ofproto/connmgr.h   |  4 +++-
> > ofproto/fail-open.c | 13 -
> > ofproto/fail-open.h |  4 +++-
> > ofproto/in-band.c   | 10 +-
> > ofproto/in-band.h   |  4 +++-
> > ofproto/ofproto.c   |  6 +-
> > tests/ofproto.at| 52 
> > 
> > 8 files changed, 105 insertions(+), 7 deletions(-)
> > 
> > diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> > index 08abe1e..3d69122 100644
> > --- a/ofproto/connmgr.c
> > +++ b/ofproto/connmgr.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -1980,6 +1980,23 @@ connmgr_flushed(struct connmgr *mgr)
> > ofpbuf_uninit(&ofpacts);
> > }
> > }
> > +
> > +/* Returns the number of hidden rules created by the in-band and fail-open
> > + * implementations in table 0.  (Subtracting this count from the number of
> > + * rules in the table 0 classifier, as returned by classifier_count(), 
> > yields
> > + * the number of flows that OVS should report via OpenFlow for table 0.) */
> > +int
> > +connmgr_count_hidden_rules(const struct connmgr *mgr)
> > +{
> > +int n_hidden = 0;
> > +if (mgr->in_band) {
> > +n_hidden += in_band_count_rules(mgr->in_band);
> > +}
> > +if (mgr->fail_open) {
> > +n_hidden += fail_open_count_rules(mgr->fail_open);
> > +}
> > +return n_hidden;
> > +}
> > 
> > /* Creates a new ofservice for 'target' in 'mgr'.  Returns 0 if successful,
> >  * otherwise a positive errno value.
> > diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
> > index c0f7c35..dd1a027 100644
> > --- a/ofproto/connmgr.h
> > +++ b/ofproto/connmgr.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -193,6 +193,8 @@ bool connmgr_has_in_band(struct connmgr *);
> > /* Fail-open and in-band implementation. */
> > void connmgr_flushed(struct connmgr *);
> > 
> > +int connmgr_count_hidden_rules(const struct connmgr *);
> > +
> > /* A flow monitor managed by NXST_FLOW_MONITOR and related requests. */
> > struct ofmonitor {
> > struct ofconn *ofconn;  /* Owning 'ofconn'. */
> > diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
> > index ecdba44..d3faf10 100644
> > --- a/ofproto/fail-open.c
> > +++ b/ofproto/fail-open.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -76,6 +76,7 @@ struct fail_open {
> > int last_disconn_secs;
> > long long int next_bogus_packet_in;
> > struct rconn_packet_counter *bogus_packet_counter;
> > +bool fail_open_active;
> > };
> > 
> > static void fail_open_recover(struct fail_open *);
> > @@ -234,6 +235,15 @@ fail_open_flushed(struct fail_open *fo)
> > 
> > ofpbuf_uninit(&ofpacts);
> > }
> > +fo->fail_open_active = open;
> > +}
> > +
> > +/* Returns the number of fail-open rules currently installed in the flow
> > + * table. */
> > +int
> > +fail_open_count_rules(const struct fail_open *fo)
> > +{
> > +return fo->fail_open_active;
> 
> I’m not sure this is in line with CodingStyle, but it may be that
> CodingStyle could be updated now that we compile with C99?

It took me a minute to see what you mean here.  I could sit and
microanalyze it, but it seems easier to just add "!= 0". so that's what
I did.

Thanks!  Applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [branch-2.3] ofproto-dpif-upcall: Make handler always call poll_block.

2015-01-06 Thread Ben Pfaff
On Mon, Dec 29, 2014 at 04:06:55PM -0800, Alex Wang wrote:
> This commit makes handler threads always call poll_block() at
> the end of each handling cycle.  If there are upcalls received
> in the current cycle, the handler will register to wake up
> immediately.  Otherwise, it will wait on both the netlink
> socket and the exit latch.
> 
> Calling poll_block() at every handling cycle makes sure that
> coverage counter stats are always timely attributed, and that
> the execution of ovsrcu-postponed events is not held by any
> busy handler thread.
> 
> Signed-off-by: Alex Wang 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofproto-dpif: Do not allow recirc_id freed by non-owning ofproto.

2015-01-06 Thread Ben Pfaff
On Mon, Dec 29, 2014 at 02:18:21PM -0800, Alex Wang wrote:
> This commit changes the VLOG_ERR (for warning unmatched ofproto)
> in ofproto_dpif_free_recirc_id() to an assert statement, so that
> recirc_id is never allowed to be freed by non-owning ofproto.
> 
> Suggested-by: Ben Pfaff 
> Signed-off-by: Alex Wang 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Justin Pettit

> On Jan 2, 2015, at 1:44 PM, Ben Pfaff  wrote:
> 
> +When the embargo expires, push the (reviewed) patches to appropriate
> +branches, post the patches to the ovs-dev mailing list (noting that
> +they have already been reviewed and applied), post the security
> +advisory to appropriate mailing lists (ovs-announce, ovs-users), and

I don't think we have an ovs-users mailing list.  ovs-discuss?

It would be a good idea to link to this document from REPORTING-BUGS.md.

Thanks for pulling this together!

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 04:23:59PM +0100, Jiri Benc wrote:
> On Fri, 2 Jan 2015 17:57:14 -0800, Ben Pfaff wrote:
> > 1) Consider provisions for ensuring privacy and integrity of
> > communications around disclosure (eg, use PGP for all comms).
> 
> That never hurts. I'd argue that's not strictly required though, as the
> code speaks for itself and anybody can verify the patch does what it
> does and the reasoning is correct.

I'm in favor, obviously, of privacy and integrity of communications, but
I do not know how it works out practically.  Is there a good way to
ensure end-to-end privacy and integrity when emailing a mailing list?
And if so, what is the chance that people reporting vulnerabilities know
how to do this and have the correct software for it?

One option would be for reporters to send an encrypted email to some
individual in the security team--e.g. to me since I'm in the Debian
web-of-trust or to Jesse or Pravin since they're in the kernel
web-of-trust--but then that individual would just have the same problem
for distributing the information to the rest of the team.

> > 2) Consider provisions for handling the vuln info to prevent it from
> > being leaked / stolen from developers (this info can often be worth a
> > lot of money to certain parties with a lot of interest and motivation to
> > get hold of them). This means keeping info in some sort of secured
> > enclaves, and perhaps mixing patch code with other commits to obfuscate
> > the presence of the flaw.
> 
> I strongly disagree with that. Distributions need to be able to cherry
> pick the security fixes, any kind of obfuscation and mixing commits for
> different things makes the life of distro maintainers harder, leading
> to more mistakes, thus less security for those who use ovs via a
> distro. Such thing would not improve security of the ovs project anyway
> --the yet undiscovered bugs do not have a patch to obfuscate, and
> discovered and patched bugs are, well, patched. Anybody running on the
> latest code base has the fix applied; for backports, a clear patch to
> backport is needed.

This would almost make sense *before* release of embargo, but again I
can't imagine it working out practically.  I'm not going to spend tons
of time on writing up a bunch of red herring commits.  I don't know what
an "secured enclave" is; if it means maintaining a separate "more
secure" office or computer or whatever, the overhead is too high.

> > Parts of OVS are in kernel space, making it a
> > quite an “interesting” target, so I wouldn’t take this one lightly.
> 
> The kernel patches will need to go through the kernel security
> reporting process:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
> 
> Which is maybe a good idea to include in the documentation?

Good idea.

I added the following paragraph under "Reception":

The Linux kernel has its own vulnerability management process:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
Handling of vulnerabilities that affect both the Open vSwitch tree and
the upstream Linux kernel should be reported through both processes.
Please send your report as a single email to both the kernel and OVS
security teams to allow those teams to most easily coordinate among
themselves.

> > 3) Consider revising patch release process to ensure patched code
> > reaches deployments without disclosing the vulnerability; and release
> > the vuln info after allowing sufficient time for fixed code to percolate
> > into running environments. Currently proposed process does not appear to
> > allow for this.
> 
> It does, through the embargoed disclosure. Committing the patch without
> sending it to ovs-dev list and releasing the disclosure only later does
> not make sense. It would draw the attention to the fix anyway. And
> sending it for review to ovs-dev without telling what the patch fixes
> won't work, either.
> 
> Overall, these suggestions seem to be from somebody who's not familiar
> with how open source development works.

Could be, I don't think I know him.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 09:29:47AM -0600, Kyle Mestery wrote:
> On Mon, Jan 5, 2015 at 9:23 AM, Jiri Benc  wrote:
> 
> > On Fri, 2 Jan 2015 17:57:14 -0800, Ben Pfaff wrote:
> > > 1) Consider provisions for ensuring privacy and integrity of
> > > communications around disclosure (eg, use PGP for all comms).
> >
> > That never hurts. I'd argue that's not strictly required though, as the
> > code speaks for itself and anybody can verify the patch does what it
> > does and the reasoning is correct.
> >
> > > 2) Consider provisions for handling the vuln info to prevent it from
> > > being leaked / stolen from developers (this info can often be worth a
> > > lot of money to certain parties with a lot of interest and
> > motivation to
> > > get hold of them). This means keeping info in some sort of secured
> > > enclaves, and perhaps mixing patch code with other commits to
> > obfuscate
> > > the presence of the flaw.
> >
> > I strongly disagree with that. Distributions need to be able to cherry
> > pick the security fixes, any kind of obfuscation and mixing commits for
> > different things makes the life of distro maintainers harder, leading
> > to more mistakes, thus less security for those who use ovs via a
> > distro. Such thing would not improve security of the ovs project anyway
> > --the yet undiscovered bugs do not have a patch to obfuscate, and
> > discovered and patched bugs are, well, patched. Anybody running on the
> > latest code base has the fix applied; for backports, a clear patch to
> > backport is needed.
> >
> > > Parts of OVS are in kernel space, making it a
> > > quite an “interesting” target, so I wouldn’t take this one lightly.
> >
> > The kernel patches will need to go through the kernel security
> > reporting process:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
> >
> > Which is maybe a good idea to include in the documentation?
> >
> > ++
> 
> I think including a link to the kernel security reporting process is
> necessary here.

I added the following paragraph under "Reception".  Comments welcome.

The Linux kernel has its own vulnerability management process:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
Handling of vulnerabilities that affect both the Open vSwitch tree and
the upstream Linux kernel should be reported through both processes.
Please send your report as a single email to both the kernel and OVS
security teams to allow those teams to most easily coordinate among
themselves.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Justin Pettit
On Jan 5, 2015, at 7:04 AM, Jiri Benc  wrote:
> 
> On Fri,  2 Jan 2015 13:44:49 -0800, Ben Pfaff wrote:
> 
>> +Step 4: Embargoed Disclosure
>> +
>> +
>> +The security advisory and patches are sent to downstream stakeholders,
>> +with an embargo date and time set to 3 to 5 business days from the
>> +time sent.  Downstream stakeholders are expected not to deploy or
>> +disclose patches until the embargo is passed.
> 
> I suggest to create a closed unarchived mailing list for this, so no
> stakeholder is forgotten if/when the person sending the advisory
> changes.

The list is configured as closed, but it's archived.  In general, I like to 
keep archives, since I think it provides useful guidance about how past 
activities were handled.  Your point about downstream stakeholders is 
interesting, though.  We should have a list somewhere about who they are.  My 
initial inclination is to make it part of this document, but I can also see the 
argument for it being private.  Do we know how others do it?

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Justin Pettit

> On Jan 5, 2015, at 8:23 AM, Flavio Leitner  wrote:
> 
> My concern is with open communications.  It should be
> possible to anyone to report the issue, but further communications
> should be authenticated and closed to avoid someone else to
> fake messages/patches, etc.
> 
> Is the mailing list ready?

Yes, it is.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 06:01:14PM +0100, Thomas Graf wrote:
> On 01/05/15 at 04:23pm, Jiri Benc wrote:
> > On Fri, 2 Jan 2015 17:57:14 -0800, Ben Pfaff wrote:
> > > 1) Consider provisions for ensuring privacy and integrity of
> > > communications around disclosure (eg, use PGP for all comms).
> > 
> > That never hurts. I'd argue that's not strictly required though, as the
> > code speaks for itself and anybody can verify the patch does what it
> > does and the reasoning is correct.
> 
> I agree and I would put the emphasize on the communication around the
> disclosure. Requiring GPG for all reports is pretty much pointless
> without a previous web of trust between the team and the reporter.
> 
> We should however sign the messages when we disclose the vulnerability
> publicly.

OK, I added a paragraph to the introduction:

We encourage everyone involved in the security process to GPG-sign
their emails.  We additionally encourage GPG-encrypting one-on-one
conversations as part of the security process.

and to "Step 5: Full Disclosure":

The security advisory should be GPG-signed by a security team member
with a key that is in a public web of trust.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 02:30:10PM -0200, Flavio Leitner wrote:
> On Friday, January 02, 2015 05:57:14 PM Ben Pfaff wrote:
> > On Fri, Jan 02, 2015 at 01:44:49PM -0800, Ben Pfaff wrote:
> > > Open vSwitch needs some kind of process for handling vulnerabilities.  So
> > > far, we've been pretty lucky that way, but it can't last forever, and I
> > > think we'll be better off if we have at least the outline of an 
> > > established
> > > process whenever a significant vulnerability comes along.  Here's my draft
> > > of a process based on the documentation of the OpenStack process at
> > > https://wiki.openstack.org/wiki/Vulnerability_Management.
> > > 
> > > I don't have a lot of experience with this kind of thing myself, so I'd
> > > appreciate critical review from anyone who does.
> > > 
> > > Signed-off-by: Ben Pfaff 
> > 
> > I received the following suggestions in private email from a person who
> > said that I could pass them along to the list as long as I do not use
> > his name because he prefers "not to be associated with the security
> > field."  Fair enough!  Here they are:
> > 
> > 1) Consider provisions for ensuring privacy and integrity of
> > communications around disclosure (eg, use PGP for all comms).
> 
> Yes, exactly what I meant in the other reply.  I think it should be
> recommended but not required for the reporter when reporting
> the issue to the list.  Further communications are required to
> ensure privacy and integrity.

I added at the top:

We encourage everyone involved in the security process to GPG-sign
their emails.  We additionally encourage GPG-encrypting one-on-one
conversations as part of the security process.

and in step 5 (full disclosure):

The security advisory should be GPG-signed by a security team member
with a key that is in a public web of trust.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 09:59:28AM -0800, Justin Pettit wrote:
> On Jan 5, 2015, at 7:04 AM, Jiri Benc  wrote:
> > 
> > On Fri,  2 Jan 2015 13:44:49 -0800, Ben Pfaff wrote:
> > 
> >> +Step 4: Embargoed Disclosure
> >> +
> >> +
> >> +The security advisory and patches are sent to downstream stakeholders,
> >> +with an embargo date and time set to 3 to 5 business days from the
> >> +time sent.  Downstream stakeholders are expected not to deploy or
> >> +disclose patches until the embargo is passed.
> > 
> > I suggest to create a closed unarchived mailing list for this, so no
> > stakeholder is forgotten if/when the person sending the advisory
> > changes.
> 
> The list is configured as closed, but it's archived.  In general, I
> like to keep archives, since I think it provides useful guidance about
> how past activities were handled.  Your point about downstream
> stakeholders is interesting, though.  We should have a list somewhere
> about who they are.  My initial inclination is to make it part of this
> document, but I can also see the argument for it being private.  Do we
> know how others do it?

We have a closed, archived list for the security team, called
ovs-security.  I think that Jiri is suggesting that we create another
list for downstream stakeholders.  That's not a bad idea, for the
reasons that Jiri notes.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Kyle Mestery
On Tue, Jan 6, 2015 at 11:58 AM, Ben Pfaff  wrote:

> On Mon, Jan 05, 2015 at 09:29:47AM -0600, Kyle Mestery wrote:
> > On Mon, Jan 5, 2015 at 9:23 AM, Jiri Benc  wrote:
> >
> > > On Fri, 2 Jan 2015 17:57:14 -0800, Ben Pfaff wrote:
> > > > 1) Consider provisions for ensuring privacy and integrity of
> > > > communications around disclosure (eg, use PGP for all comms).
> > >
> > > That never hurts. I'd argue that's not strictly required though, as the
> > > code speaks for itself and anybody can verify the patch does what it
> > > does and the reasoning is correct.
> > >
> > > > 2) Consider provisions for handling the vuln info to prevent it
> from
> > > > being leaked / stolen from developers (this info can often be
> worth a
> > > > lot of money to certain parties with a lot of interest and
> > > motivation to
> > > > get hold of them). This means keeping info in some sort of
> secured
> > > > enclaves, and perhaps mixing patch code with other commits to
> > > obfuscate
> > > > the presence of the flaw.
> > >
> > > I strongly disagree with that. Distributions need to be able to cherry
> > > pick the security fixes, any kind of obfuscation and mixing commits for
> > > different things makes the life of distro maintainers harder, leading
> > > to more mistakes, thus less security for those who use ovs via a
> > > distro. Such thing would not improve security of the ovs project anyway
> > > --the yet undiscovered bugs do not have a patch to obfuscate, and
> > > discovered and patched bugs are, well, patched. Anybody running on the
> > > latest code base has the fix applied; for backports, a clear patch to
> > > backport is needed.
> > >
> > > > Parts of OVS are in kernel space, making it a
> > > > quite an “interesting” target, so I wouldn’t take this one
> lightly.
> > >
> > > The kernel patches will need to go through the kernel security
> > > reporting process:
> > >
> > >
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
> > >
> > > Which is maybe a good idea to include in the documentation?
> > >
> > > ++
> >
> > I think including a link to the kernel security reporting process is
> > necessary here.
>
> I added the following paragraph under "Reception".  Comments welcome.
>
> The Linux kernel has its own vulnerability management process:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
> Handling of vulnerabilities that affect both the Open vSwitch tree and
> the upstream Linux kernel should be reported through both processes.
> Please send your report as a single email to both the kernel and OVS
> security teams to allow those teams to most easily coordinate among
> themselves.
>

This reads quite well to me, thanks for adding this Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Justin Pettit

> On Jan 6, 2015, at 10:09 AM, Ben Pfaff  wrote:
> 
> On Tue, Jan 06, 2015 at 09:59:28AM -0800, Justin Pettit wrote:
>> On Jan 5, 2015, at 7:04 AM, Jiri Benc  wrote:
>>> 
>>> On Fri,  2 Jan 2015 13:44:49 -0800, Ben Pfaff wrote:
>>> 
 +Step 4: Embargoed Disclosure
 +
 +
 +The security advisory and patches are sent to downstream stakeholders,
 +with an embargo date and time set to 3 to 5 business days from the
 +time sent.  Downstream stakeholders are expected not to deploy or
 +disclose patches until the embargo is passed.
>>> 
>>> I suggest to create a closed unarchived mailing list for this, so no
>>> stakeholder is forgotten if/when the person sending the advisory
>>> changes.
>> 
>> The list is configured as closed, but it's archived.  In general, I
>> like to keep archives, since I think it provides useful guidance about
>> how past activities were handled.  Your point about downstream
>> stakeholders is interesting, though.  We should have a list somewhere
>> about who they are.  My initial inclination is to make it part of this
>> document, but I can also see the argument for it being private.  Do we
>> know how others do it?
> 
> We have a closed, archived list for the security team, called
> ovs-security.  I think that Jiri is suggesting that we create another
> list for downstream stakeholders.  That's not a bad idea, for the
> reasons that Jiri notes.

Oh, yes.  That makes a lot of sense.

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Mon, Jan 05, 2015 at 02:23:45PM -0200, Flavio Leitner wrote:
> On Friday, January 02, 2015 01:44:49 PM Ben Pfaff wrote:
> > Open vSwitch needs some kind of process for handling vulnerabilities.  So
> > far, we've been pretty lucky that way, but it can't last forever, and I
> > think we'll be better off if we have at least the outline of an established
> > process whenever a significant vulnerability comes along.  Here's my draft
> > of a process based on the documentation of the OpenStack process at
> > https://wiki.openstack.org/wiki/Vulnerability_Management.
> > 
> > I don't have a lot of experience with this kind of thing myself, so I'd
> > appreciate critical review from anyone who does.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> My concern is with open communications.  It should be
> possible to anyone to report the issue, but further communications
> should be authenticated and closed to avoid someone else to
> fake messages/patches, etc.

As a list open for posting, anyone can report an issue.

I don't know how to implement the authentication you mention.  Is there
a customary way to do it, that will not shut out reporters who do not
have some specific email client etc.?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 09:49:16AM -0800, Justin Pettit wrote:
> 
> > On Jan 2, 2015, at 1:44 PM, Ben Pfaff  wrote:
> > 
> > +When the embargo expires, push the (reviewed) patches to appropriate
> > +branches, post the patches to the ovs-dev mailing list (noting that
> > +they have already been reviewed and applied), post the security
> > +advisory to appropriate mailing lists (ovs-announce, ovs-users), and
> 
> I don't think we have an ovs-users mailing list.  ovs-discuss?

Fixed, thanks.

> It would be a good idea to link to this document from REPORTING-BUGS.md.

Thanks, added.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
Open vSwitch needs some kind of process for handling vulnerabilities.  So
far, we've been pretty lucky that way, but it can't last forever, and I
think we'll be better off if we have at least the outline of an established
process whenever a significant vulnerability comes along.  Here's my draft
of a process based on the documentation of the OpenStack process at
https://wiki.openstack.org/wiki/Vulnerability_Management.

I don't have a lot of experience with this kind of thing myself, so I'd
appreciate critical review from anyone who does.

Signed-off-by: Ben Pfaff 
Reviewed-by: Flavio Leitner 
---
v1->v2:
   - Suggest GPG signing and encryption.
   - Mention coordination with Linux kernel security process.
   - "ovs-users" is actually "ovs-discuss".
   - Mention SECURITY.md from REPORTING-BUGS.md.
   - Add examples.
---
 Makefile.am   |   3 +-
 REPORTING-BUGS.md |   2 +
 SECURITY.md   | 154 ++
 3 files changed, 158 insertions(+), 1 deletion(-)
 create mode 100644 SECURITY.md

diff --git a/Makefile.am b/Makefile.am
index 46e8610..26528d9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,4 +1,4 @@
-# Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+# Copyright (C) 2007-2015 Nicira, Inc.
 #
 # Copying and distribution of this file, with or without modification,
 # are permitted in any medium without royalty provided the copyright
@@ -89,6 +89,7 @@ docs = \
README-lisp.md \
README-native-tunneling.md \
REPORTING-BUGS.md \
+   SECURITY.md \
TODO.md \
WHY-OVS.md
 EXTRA_DIST = \
diff --git a/REPORTING-BUGS.md b/REPORTING-BUGS.md
index 4e08cb1..c046af9 100644
--- a/REPORTING-BUGS.md
+++ b/REPORTING-BUGS.md
@@ -7,6 +7,8 @@ bugs so as to ensure that they can be fixed as quickly as 
possible.
 
 Please report bugs by sending email to b...@openvswitch.org.  
 
+For reporting security vulnerabilities, please read SECURITY.md.
+
 The most important parts of your bug report are the following:
 
   * What you did that make the problem appear.
diff --git a/SECURITY.md b/SECURITY.md
new file mode 100644
index 000..e1db4cb
--- /dev/null
+++ b/SECURITY.md
@@ -0,0 +1,154 @@
+Security Process
+
+
+This is a proposed security vulnerability reporting and handling
+process for Open vSwitch.  It is based on the OpenStack vulnerability
+management process described at
+https://wiki.openstack.org/wiki/Vulnerability_Management.
+
+The OVS security team coordinates vulnerability management using the
+ovs-security mailing list.  Membership in the security team and
+subscription to its mailing list consists of a small number of
+trustworthy people, as determined by rough consensus of the Open
+vSwitch committers on the ovs-committers mailing list.  The Open
+vSwitch security team should include Open vSwitch committers, to
+ensure prompt and accurate vulnerability assessments and patch review.
+
+We encourage everyone involved in the security process to GPG-sign
+their emails.  We additionally encourage GPG-encrypting one-on-one
+conversations as part of the security process.
+
+
+What is a vulnerability?
+
+
+All vulnerabilities are bugs, but not every bug is a vulnerability.
+Here are some examples of vulnerabilities to which one would expect to
+apply this process:
+
+* A crafted packet that causes a kernel or userspace crash.
+
+* A flow translation bug that misforwards traffic in a way likely
+  to hop over security boundaries.
+
+* An OpenFlow protocol bug that allows a controller to read
+  arbitrary files from the file system.
+
+* Misuse of the OpenSSL library that allows bypassing certificate
+  checks.
+
+* A bug (memory corruption, overflow, ...) that allows one to
+  modify the behaviour of OVS through external configuration
+  interfaces such as OVSDB.
+
+* Privileged information is exposed to unprivileged users.
+
+If in doubt, please do use the vulnerability management process.  At
+worst, the response will be to report the bug through the usual
+channels.
+
+
+Step 1: Reception
+-
+
+To report an Open vSwitch vulnerability, send an email to the
+ovs-security mailing list (see "Contact" at the end of this document).
+A security team member should reply to the reporter acknowledging that
+the report has been received.
+
+Please consider reporting the information mentioned in
+REPORTING-BUGS.md, where relevant.
+
+The Linux kernel has its own vulnerability management process:
+https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
+Handling of vulnerabilities that affect both the Open vSwitch tree and
+the upstream Linux kernel should be reported through both processes.
+Please send your report as a single email to both the kernel and OVS
+security teams to allow those teams to most easily coordinate among
+themselves.
+
+
+Step 2: Assessment
+

Re: [ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Justin Pettit

> On Jan 6, 2015, at 10:22 AM, Ben Pfaff  wrote:
> 
> Open vSwitch needs some kind of process for handling vulnerabilities.  So
> far, we've been pretty lucky that way, but it can't last forever, and I
> think we'll be better off if we have at least the outline of an established
> process whenever a significant vulnerability comes along.  Here's my draft
> of a process based on the documentation of the OpenStack process at
> https://wiki.openstack.org/wiki/Vulnerability_Management.
> 
> I don't have a lot of experience with this kind of thing myself, so I'd
> appreciate critical review from anyone who does.
> 
> Signed-off-by: Ben Pfaff 
> Reviewed-by: Flavio Leitner 
> ---
> v1->v2:
>   - Suggest GPG signing and encryption.
>   - Mention coordination with Linux kernel security process.
>   - "ovs-users" is actually "ovs-discuss".
>   - Mention SECURITY.md from REPORTING-BUGS.md.
>   - Add examples.

Looks good to me.

Acked-by: Justin Pettit 

Thanks for driving this!

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Justin Pettit

> On Jan 6, 2015, at 10:29 AM, Justin Pettit  wrote:
> 
> 
>> On Jan 6, 2015, at 10:22 AM, Ben Pfaff  wrote:
>> 
>> Open vSwitch needs some kind of process for handling vulnerabilities.  So
>> far, we've been pretty lucky that way, but it can't last forever, and I
>> think we'll be better off if we have at least the outline of an established
>> process whenever a significant vulnerability comes along.  Here's my draft
>> of a process based on the documentation of the OpenStack process at
>> https://wiki.openstack.org/wiki/Vulnerability_Management.
>> 
>> I don't have a lot of experience with this kind of thing myself, so I'd
>> appreciate critical review from anyone who does.
>> 
>> Signed-off-by: Ben Pfaff 
>> Reviewed-by: Flavio Leitner 
>> ---
>> v1->v2:
>>  - Suggest GPG signing and encryption.
>>  - Mention coordination with Linux kernel security process.
>>  - "ovs-users" is actually "ovs-discuss".
>>  - Mention SECURITY.md from REPORTING-BUGS.md.
>>  - Add examples.
> 
> Looks good to me.
> 
> Acked-by: Justin Pettit 

Ugh, haven't been reviewing enough:

Acked-by: Justin Pettit 

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [branch-2.3] ofproto-dpif-upcall: Make handler always call poll_block.

2015-01-06 Thread Alex Wang
Thx for the review, applied to branch-2.3

Master has already done the similar thing,
and there is no need to backport to branch-2.1

On Tue, Jan 6, 2015 at 9:32 AM, Ben Pfaff  wrote:

> On Mon, Dec 29, 2014 at 04:06:55PM -0800, Alex Wang wrote:
> > This commit makes handler threads always call poll_block() at
> > the end of each handling cycle.  If there are upcalls received
> > in the current cycle, the handler will register to wake up
> > immediately.  Otherwise, it will wait on both the netlink
> > socket and the exit latch.
> >
> > Calling poll_block() at every handling cycle makes sure that
> > coverage counter stats are always timely attributed, and that
> > the execution of ovsrcu-postponed events is not held by any
> > busy handler thread.
> >
> > Signed-off-by: Alex Wang 
>
> Acked-by: Ben Pfaff 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofproto-dpif: Do not allow recirc_id freed by non-owning ofproto.

2015-01-06 Thread Alex Wang
Thx, applied to master,

On Tue, Jan 6, 2015 at 9:35 AM, Ben Pfaff  wrote:

> On Mon, Dec 29, 2014 at 02:18:21PM -0800, Alex Wang wrote:
> > This commit changes the VLOG_ERR (for warning unmatched ofproto)
> > in ofproto_dpif_free_recirc_id() to an assert statement, so that
> > recirc_id is never allowed to be freed by non-owning ofproto.
> >
> > Suggested-by: Ben Pfaff 
> > Signed-off-by: Alex Wang 
>
> Acked-by: Ben Pfaff 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] miniflow: Use 64-bit data.

2015-01-06 Thread Jarno Rajahalme

On Jan 5, 2015, at 2:39 PM, Ben Pfaff  wrote:

> On Mon, Jan 05, 2015 at 02:15:13PM -0800, Ben Pfaff wrote:
>> On Mon, Jan 05, 2015 at 02:08:41PM -0800, Jarno Rajahalme wrote:
>>> 
>>> On Dec 29, 2014, at 2:27 PM, Ben Pfaff  wrote:
>>> 
 On Wed, Dec 17, 2014 at 10:30:42AM -0800, Jarno Rajahalme wrote:
> So far the compressed flow data in struct miniflow has been in 32-bit
> words with a 63-bit map, allowing for a maximum size of struct flow of
> 252 bytes.  With the forthcoming Geneve options this is not sufficient
> any more.
> 
> This patch solves the problem by changing the miniflow data to 64-bit
> words, doubling the flow max size to 504 bytes.  Since the word size
> is doubled, there is some loss in compression efficiency.  To counter
> this some of the flow fields have been reordered to keep related
> fields together (e.g., the source and destination IP addresses share
> the same 64-bit word).
> 
> This change should speed up flow data processing on 64-bit CPUs, which
> may help counterbalance the impact of making the struct flow bigger in
> the future.
> 
> Classifier lookup stage boundaries are also changed to 64-bit
> alignment, as the current algorithm depends on each miniflow word to
> not be split between ranges.  This has resulted in new padding (part
> of the 'mpls_lse' field).
> 
> The 'dp_hash' field is also moved to packet metadata to eliminate
> otherwise needed padding there.  This allows the L4 to fit into one
> 64-bit word, and also makes matches on 'dp_hash' more efficient as
> misses can be found already on stage 1.
> 
> Signed-off-by: Jarno Rajahalme 
>>> 
 This seems mostly straightforward.  Are there particular parts you'd
 like me to look over carefully?
 
>>> 
>>> Maybe the changes to the miniflow push macros, which get a bit more
>>> complicated...
>> 
>> OK, I'll do that in a bit.
> 
> I read them through them.  I didn't spot any bugs, although they're
> somewhat tricky.

Is this an Acked-by then?

  Jarno


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net] gso: do GSO for local skb with size bigger than MTU

2015-01-06 Thread Jesse Gross
On Tue, Jan 6, 2015 at 4:34 AM, Fan Du  wrote:
>
> On 2015/1/6 1:58, Jesse Gross wrote:
>>
>> On Mon, Jan 5, 2015 at 1:02 AM, Fan Du 
>> wrote:
>>>
>>> 于 2014年12月03日 10:31, Du, Fan 写道:
>>>

> -Original Message-
> From: Thomas Graf [mailto:t...@infradead.org] On Behalf Of Thomas Graf
> Sent: Wednesday, December 3, 2014 1:42 AM
> To: Michael S. Tsirkin
> Cc: Du, Fan; 'Jason Wang'; net...@vger.kernel.org; da...@davemloft.net;
> f...@strlen.de; dev@openvswitch.org; je...@nicira.com; pshe...@nicira.com
> Subject: Re: [PATCH net] gso: do GSO for local skb with size bigger
> than
> MTU
>
> On 12/02/14 at 07:34pm, Michael S. Tsirkin wrote:
>>
>> On Tue, Dec 02, 2014 at 05:09:27PM +, Thomas Graf wrote:
>>>
>>> On 12/02/14 at 01:48pm, Flavio Leitner wrote:

 What about containers or any other virtualization environment that
 doesn't use Virtio?
>>>
>>>
>>> The host can dictate the MTU in that case for both veth or OVS
>>> internal which would be primary container plumbing techniques.
>>
>>
>> It typically can't do this easily for VMs with emulated devices:
>> real ethernet uses a fixed MTU.
>>
>> IMHO it's confusing to suggest MTU as a fix for this bug, it's an
>> unrelated optimization.
>> ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED is the right fix here.
>
>
> PMTU discovery only resolves the issue if an actual IP stack is running
> inside the
> VM. This may not be the case at all.

^^^

 Some thoughts here:

 Think otherwise, this is indeed what host stack should forge a
 ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED
 message with _inner_ skb network and transport header, do whatever type
 of
 encapsulation,
 and thereafter push such packet upward to Guest/Container, which make
 them
 feel, the intermediate node
 or the peer send such message. PMTU should be expected to work correct.
 And such behavior should be shared by all other encapsulation tech if
 they
 are also suffered.
>>>
>>>
>>> Hi David, Jesse and Thomas
>>>
>>> As discussed in here:
>>> https://www.marc.info/?l=linux-netdev&m=141764712631150&w=4 and
>>> quotes from Jesse:
>>> My proposal would be something like this:
>>>   * For L2, reduce the VM MTU to the lowest common denominator on the
>>> segment.
>>>   * For L3, use path MTU discovery or fragment inner packet (i.e.
>>> normal routing behavior).
>>>   * As a last resort (such as if using an old version of virtio in the
>>> guest), fragment the tunnel packet.
>>>
>>>
>>> For L2, it's a administrative action
>>> For L3, PMTU approach looks better, because once the sender is alerted
>>> the
>>> reduced MTU,
>>> packet size after encapsulation will not exceed physical MTU, so no
>>> additional fragments
>>> efforts needed.
>>> For "As a last resort... fragment the tunnel packet", the original patch:
>>> https://www.marc.info/?l=linux-netdev&m=141715655024090&w=4 did the job,
>>> but
>>> seems it's
>>> not welcomed.
>>
>> This needs to be properly integrated into IP processing if it is to
>> work correctly.
>
> Do you mean the original patch in this thread? yes, it works correctly
> in my cloud env. If you has any other concerns, please let me know. :)

Ok...but that doesn't actually address the points that I made.

>> One of the reasons for only doing path MTU discovery
>> for L3 is that it operates seamlessly as part of normal operation -
>> there is no need to forge addresses or potentially generate ICMP when
>> on an L2 network. However, this ignores the IP handling that is going
>> on (note that in OVS it is possible for L3 to be implemented as a set
>> of flows coming from a controller).
>>
>> It also should not be VXLAN specific or duplicate VXLAN encapsulation
>> code. As this is happening before encapsulation, the generated ICMP
>> does not need to be encapsulated either if it is created in the right
>> location.
>
> Yes, I agree. GRE share the same issue from the code flow.
> Pushing back ICMP msg back without encapsulation without circulating down
> to physical device is possible. The "right location" as far as I know
> could only be in ovs_vport_send. In addition this probably requires wrapper
> route looking up operation for GRE/VXLAN, after get the under layer device
> MTU
> from the routing information, then calculate reduced MTU becomes feasible.

As I said, it needs to be integrated into L3 processing. In OVS this
would mean adding some primitives to the kernel and then exposing the
functionality upwards into userspace/controller.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] vagrant: Provide basic Vagrantfile

2015-01-06 Thread Andy Zhou
We have been think about adding unit tests the kernel module.   Any
plans or objections to develop kmod unit tests on top of this patch?

Before seeing this patch, I played with UML (user mode linux) with
some limited success.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] PERSONAL GIFT FROM ME TO YOU

2015-01-06 Thread Hannah Murray
I'm making a free-will financial donation. Reply to partake 
{m_schaeff...@aol.com} Maria Schaeffler​​




This message and any attachment are intended solely for the addressee
and may contain confidential information. If you have received this
message in error, please send it back to me, and immediately delete it. 

Please do not use, copy or disclose the information contained in this
message or in any attachment.  Any views or opinions expressed by the
author of this email do not necessarily reflect the views of the
University of Nottingham.

This message has been checked for viruses but the contents of an
attachment may still contain software viruses which could damage your
computer system, you are advised to perform your own checks. Email
communications with the University of Nottingham may be monitored as
permitted by UK legislation.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] miniflow: Use 64-bit data.

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 11:03:15AM -0800, Jarno Rajahalme wrote:
> 
> On Jan 5, 2015, at 2:39 PM, Ben Pfaff  wrote:
> 
> > On Mon, Jan 05, 2015 at 02:15:13PM -0800, Ben Pfaff wrote:
> >> On Mon, Jan 05, 2015 at 02:08:41PM -0800, Jarno Rajahalme wrote:
> >>> 
> >>> On Dec 29, 2014, at 2:27 PM, Ben Pfaff  wrote:
> >>> 
>  On Wed, Dec 17, 2014 at 10:30:42AM -0800, Jarno Rajahalme wrote:
> > So far the compressed flow data in struct miniflow has been in 32-bit
> > words with a 63-bit map, allowing for a maximum size of struct flow of
> > 252 bytes.  With the forthcoming Geneve options this is not sufficient
> > any more.
> > 
> > This patch solves the problem by changing the miniflow data to 64-bit
> > words, doubling the flow max size to 504 bytes.  Since the word size
> > is doubled, there is some loss in compression efficiency.  To counter
> > this some of the flow fields have been reordered to keep related
> > fields together (e.g., the source and destination IP addresses share
> > the same 64-bit word).
> > 
> > This change should speed up flow data processing on 64-bit CPUs, which
> > may help counterbalance the impact of making the struct flow bigger in
> > the future.
> > 
> > Classifier lookup stage boundaries are also changed to 64-bit
> > alignment, as the current algorithm depends on each miniflow word to
> > not be split between ranges.  This has resulted in new padding (part
> > of the 'mpls_lse' field).
> > 
> > The 'dp_hash' field is also moved to packet metadata to eliminate
> > otherwise needed padding there.  This allows the L4 to fit into one
> > 64-bit word, and also makes matches on 'dp_hash' more efficient as
> > misses can be found already on stage 1.
> > 
> > Signed-off-by: Jarno Rajahalme 
> >>> 
>  This seems mostly straightforward.  Are there particular parts you'd
>  like me to look over carefully?
>  
> >>> 
> >>> Maybe the changes to the miniflow push macros, which get a bit more
> >>> complicated...
> >> 
> >> OK, I'll do that in a bit.
> > 
> > I read them through them.  I didn't spot any bugs, although they're
> > somewhat tricky.
> 
> Is this an Acked-by then?

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/8] datapath: Account for rename to vlan_insert_tag_set_proto()

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> __vlan_put_tag() was renamed to vlan_insert_tag_set_proto() with
> the argument list kept intact.
>
> Upstream: 62749e ("vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto")
> Signed-off-by: Thomas Graf 
> ---
You can rename __vlan_put_tag() from vxlan.c file so that we can
rename all __vlan_put_tag() to vlan_insert_tag_set_proto(). Then
__vlan_put_tag() can removed completely.

otherwise looks good.

Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/8] datapath: Add __vlan_insert_tag() compat helper if not available

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> Since older kernels do not have skb->vlan_proto, it is assumed that
> kernels which don't provide their own __vlan_insert_tag() will also
> not have skb->vlan_proto. The backwards compat function therefore
> only supports ETH_P_8021Q as the protocol type.
>
> Upstream: 15255a43 ("vlan: introduce __vlan_insert_tag helper which does not 
> free skb")
> Signed-off-by: Thomas Graf 

Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 3/8] datapath: move make_writable helper into common code

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> note that skb_make_writable already exists in net/netfilter/core.c
> but does something slightly different.
>
> Upstream: e219512 ("net: move make_writable helper into common code")
> Signed-off-by: Thomas Graf 

Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 4/8] datapath: move vlan pop/push functions into common code

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> So it can be used from out of openvswitch code.
> Did couple of cosmetic changes on the way, namely variable naming and
> adding support for 8021AD proto.
>
> Note on backwards compatability:
> Unlike the upstream version, the backport of skb_vlan_push() does not
> support translating a hardware accelerated 8021AD tag to software.
> This is not a problem though as it preserves existing behaviour.
>
> Upstream: 93515d53 ("net: move vlan pop/push functions into common code")
> Signed-off-by: Thomas Graf 
Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 6/8] datapath: replace remaining users of arch_fast_hash with jhash

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> This patch effectively reverts commit 500f80872645 ("net: ovs: use CRC32
> accelerated flow hash if available"), and other remaining arch_fast_hash()
> users such as from nfsd via commit 6282cd565553 ("NFSD: Don't hand out
> delegations for 30 seconds after recalling them.") where it has been used
> as a hash function for bloom filtering.
>
> While we think that these users are actually not much of concern, it has
> been requested to remove the arch_fast_hash() library bits that arose
> from [1] entirely as per recent discussion [2]. The main argument is that
> using it as a hash may introduce bias due to its linearity (see avalanche
> criterion) and thus makes it less clear (though we tried to document that)
> when this security/performance trade-off is actually acceptable for a
> general purpose library function.
>
> Lets therefore avoid any further confusion on this matter and remove it to
> prevent any future accidental misuse of it. For the time being, this is
> going to make hashing of flow keys a bit more expensive in the ovs case,
> but future work could reevaluate a different hashing discipline.
>
>   [1] https://patchwork.ozlabs.org/patch/299369/
>   [2] https://patchwork.ozlabs.org/patch/418756/
>
> Upstream: 8754589 ("net: replace remaining users of arch_fast_hash with 
> jhash")
> Signed-off-by: Thomas Graf 

LGTM.
Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 7/8] datapath: Account for new flags args of vxlan_sock_add()

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> The upstream commit 359a0ea
> ("vxlan: Add support for UDP checksums (v4 sending, v6 zero csums)")
> has introduced a new flags argument to vxlan_sock_add().
>
> OVS does not pass any flags at this point, thus specyfing 0 will be
> compatible with both the old ipv6 bool and the new u32 flags argument.
>
> Upstream: 359a0ea ("vxlan: Add support for UDP checksums (v4 sending, v6 zero 
> csums)")
> Signed-off-by: Thomas Graf 
Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 8/8] travis: Update build matrix to include latest stable kernels

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> Signed-off-by: Thomas Graf 

Acked-by: Pravin B Shelar 

Thanks for all patches.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 5/8] datapath: introduce rtnl ops stub

2015-01-06 Thread Pravin Shelar
On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> This stub now allows userspace to see IFLA_INFO_KIND for ovs master and
> IFLA_INFO_SLAVE_KIND for slave.
>
> Upstream: 5b9e7e16 ("openvswitch: introduce rtnl ops stub")
> Signed-off-by: Thomas Graf 

Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2.1 2/2] [RFC] classifier: Add support for conjunctive matches.

2015-01-06 Thread Jarno Rajahalme
I’ll review the v3 when ready,

  Jarno

On Dec 30, 2014, at 4:30 PM, Ben Pfaff  wrote:

> A "conjunctive match" allows higher-level matches in the flow table, such
> as set membership matches, without causing a cross-product explosion for
> multidimensional matches.  Please refer to the documentation that this
> commit adds to ovs-ofctl(8) for a better explanation, including an example.
> 
> Issues:
> 
> - Until now the conceptual model of a cls_rule has been that it is
>  immutable while it is in a classifier.  This commit adds a "conjunctive
>  match" (optional) to each cls_rule, and makes the new member safely
>  mutable while it is in a classifier.  This might be a conceptual failing
>  bad enough to need fixing; I am not sure.
> 
> - Needs some real tests; you can run the "test-conjunction" script inside
>  "make sandbox", for now.
> 
> - The code needs some more comments.
> ---
> v1->v2:
>  - Use 1-based dimension numbers in formatted syntax, e.g. 1/2 and 2/2,
>not 0/2 and 1/2.
>  - Add new conj_id field instead of overwriting reg0.
>  - Since priority is now an 'int', get rid of awkward +1s and comparisons
>on priorities in classifier_lookup().
>  - Fix memory leak in classifier_replace().
>  - Modify conj_id in-place in classifier_lookup() instead of copying
>entire flow.
>  - Remove prototype for nonexistent cls_rule_init_conjunction().
>  - Fix memory leak in classifier_lookup(), and eliminate memory allocation
>in the common case of few conjunctive matches.
> 
> v2->v2.1:
>  - Rebase.
> ---
> NEWS |4 +
> lib/classifier-private.h |3 +-
> lib/classifier.c |  396 +++---
> lib/classifier.h |   10 ++
> lib/flow.c   |1 +
> lib/flow.h   |3 +-
> lib/match.c  |   11 ++
> lib/match.h  |3 +-
> lib/meta-flow.c  |   17 ++
> lib/meta-flow.h  |   14 ++
> lib/nx-match.c   |4 +
> lib/ofp-actions.c|  112 +++-
> lib/ofp-actions.h|   12 ++
> lib/ofp-errors.h |8 +
> ofproto/ofproto-dpif-xlate.c |4 +
> ofproto/ofproto.c|   41 +
> tests/automake.mk|2 +
> tests/ofproto.at |3 +-
> tests/test-conjunction   |   22 +++
> utilities/ovs-ofctl.8.in |  185 
> 20 files changed, 825 insertions(+), 30 deletions(-)
> create mode 100755 tests/test-conjunction
> 
> diff --git a/NEWS b/NEWS
> index f2fceb5..0bbe6f7 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,9 @@
> Post-v2.3.0
> -
> +   - New support for a "conjunctive match" OpenFlow extension, which
> + allows constructing OpenFlow matches of the form "field1 in
> + {a,b,c...} AND field2 in {d,e,f...}" and generalizations.  For details,
> + see documentation fo the "conjunction" action in ovs-ofctl(8).
>- Add bash command-line completion support for ovs-appctl/ovs-dpctl/
>  ovs-ofctl/ovsdb-tool commands.  Please check
>  utilities/ovs-command-compgen.INSTALL.md for how to use.
> diff --git a/lib/classifier-private.h b/lib/classifier-private.h
> index 2522e91..2230286 100644
> --- a/lib/classifier-private.h
> +++ b/lib/classifier-private.h
> @@ -68,7 +68,7 @@ struct cls_partition {
> /* Internal representation of a rule in a "struct cls_subtable". */
> struct cls_match {
> /* Accessed by everybody. */
> -struct rculist list; /* Identical, lower-priority rules. */
> +struct rculist list; /* Identical, lower-priority "cls_match"es. */
> 
> /* Accessed only by writers. */
> struct cls_partition *partition;
> @@ -80,6 +80,7 @@ struct cls_match {
> /* Accessed by all readers. */
> struct cmap_node cmap_node; /* Within struct cls_subtable 'rules'. */
> const struct cls_rule *cls_rule;
> +OVSRCU_TYPE(struct cls_conjunction_set *) conj_set;
> const struct miniflow flow; /* Matching rule. Mask is in the subtable. */
> /* 'flow' must be the last field. */
> };
> diff --git a/lib/classifier.c b/lib/classifier.c
> index dd60cc7..36194d2 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -31,6 +31,33 @@ VLOG_DEFINE_THIS_MODULE(classifier);
> 
> struct trie_ctx;
> 
> +/* A collection of "struct cls_conjunction"s currently embedded into a
> + * cls_match. */
> +struct cls_conjunction_set {
> +/* Link back to the cls_match.
> + *
> + * cls_conjunction_set is mostly used during classifier lookup, and, in
> + * turn, during classifier lookup the most used member of
> + * cls_conjunction_set is the rule's priority, so we cache it here for 
> fast
> + * access. */
> +struct cls_match *match;
> +int priority;   /* Cached copy of match->priority. */
> +
> +/* Conjunction information.
> + *
> + * 'min_n_clauses' allows some optimization during classifier lookup. */
> +unsigned int n; /

[ovs-dev] [PATCH] vlog: Rename the currently used term 'facility' to 'destination'.

2015-01-06 Thread Gurucharan Shetty
In OVS, we currently use the term 'facility' to mean the place
where we log (syslog, console or file). In Linux's syslog() and
rfc5424, the term 'facility' is used to specify what type of program
is logging the message (e.g: LOG_DAEMON). This causes confusion
while reading vlog's code. This commit changes the term 'facility'
to 'destination'.

Signed-off-by: Gurucharan Shetty 
---
 include/openvswitch/vlog.h  |   40 
 lib/vlog-syn.man|4 +-
 lib/vlog-unixctl.man|6 +-
 lib/vlog.c  |  158 ---
 lib/vlog.man|8 +-
 python/ovs/vlog.py  |   56 +--
 tests/test-reconnect.c  |2 +-
 tests/test-vconn.c  |2 +-
 tests/vlog.at   |6 +-
 utilities/nlmon.c   |2 +-
 utilities/ovs-appctl.8.in   |   10 +-
 utilities/ovs-appctl.c  |2 +-
 utilities/ovs-benchmark.c   |2 +-
 utilities/ovs-command-compgen-test.bash |8 +-
 utilities/ovs-vsctl.c   |2 +-
 vtep/vtep-ctl.c |2 +-
 16 files changed, 160 insertions(+), 150 deletions(-)

diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index 3f71e4b..b1c7aa7 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -60,28 +60,29 @@ enum vlog_level {
 const char *vlog_get_level_name(enum vlog_level);
 enum vlog_level vlog_get_level_val(const char *name);
 
-/* Facilities that we can log to. */
-#define VLOG_FACILITIES \
-VLOG_FACILITY(SYSLOG, "ovs|%05N|%c%T|%p|%m")\
-VLOG_FACILITY(CONSOLE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m")\
-VLOG_FACILITY(FILE, "%D{%Y-%m-%dT%H:%M:%S.###Z}|%05N|%c%T|%p|%m")
-enum vlog_facility {
-#define VLOG_FACILITY(NAME, PATTERN) VLF_##NAME,
-VLOG_FACILITIES
-#undef VLOG_FACILITY
-VLF_N_FACILITIES,
-VLF_ANY_FACILITY = -1
+/* Destinations that we can log to. */
+#define VLOG_DESTINATIONS \
+VLOG_DESTINATION(SYSLOG, "ovs|%05N|%c%T|%p|%m")\
+VLOG_DESTINATION(CONSOLE, "%D{%Y-%m-%dT%H:%M:%SZ}|%05N|%c%T|%p|%m")\
+VLOG_DESTINATION(FILE, "%D{%Y-%m-%dT%H:%M:%S.###Z}|%05N|%c%T|%p|%m")
+enum vlog_destination {
+#define VLOG_DESTINATION(NAME, PATTERN) VLF_##NAME,
+VLOG_DESTINATIONS
+#undef VLOG_DESTINATION
+VLF_N_DESTINATIONS,
+VLF_ANY_DESTINATION = -1
 };
 
-const char *vlog_get_facility_name(enum vlog_facility);
-enum vlog_facility vlog_get_facility_val(const char *name);
+const char *vlog_get_destination_name(enum vlog_destination);
+enum vlog_destination vlog_get_destination_val(const char *name);
 
 /* A log module. */
 struct vlog_module {
 struct ovs_list list;
 const char *name; /* User-visible name. */
-int levels[VLF_N_FACILITIES]; /* Minimum log level for each facility. */
-int min_level;/* Minimum log level for any facility. */
+int levels[VLF_N_DESTINATIONS]; /* Minimum log level for each
+   destination. */
+int min_level;/* Minimum log level for any destination. */
 bool honor_rate_limits;   /* Set false to ignore rate limits. */
 };
 
@@ -125,9 +126,10 @@ struct vlog_rate_limit {
 }
 
 /* Configuring how each module logs messages. */
-enum vlog_level vlog_get_level(const struct vlog_module *, enum vlog_facility);
+enum vlog_level vlog_get_level(const struct vlog_module *,
+   enum vlog_destination);
 void vlog_set_levels(struct vlog_module *,
- enum vlog_facility, enum vlog_level);
+ enum vlog_destination, enum vlog_level);
 char *vlog_set_levels_from_string(const char *) OVS_WARN_UNUSED_RESULT;
 void vlog_set_levels_from_string_assert(const char *);
 char *vlog_get_levels(void);
@@ -136,8 +138,8 @@ bool vlog_should_drop(const struct vlog_module *, enum 
vlog_level,
   struct vlog_rate_limit *);
 void vlog_set_verbosity(const char *arg);
 
-/* Configuring log facilities. */
-void vlog_set_pattern(enum vlog_facility, const char *pattern);
+/* Configuring log destinations. */
+void vlog_set_pattern(enum vlog_destination, const char *pattern);
 int vlog_set_log_file(const char *file_name);
 int vlog_reopen_log_file(void);
 
diff --git a/lib/vlog-syn.man b/lib/vlog-syn.man
index 3df967a..09ebdf6 100644
--- a/lib/vlog-syn.man
+++ b/lib/vlog-syn.man
@@ -1,6 +1,6 @@
 .IP "Logging options:"
-[\fB\-v\fR[\fImodule\fR[\fB:\fIfacility\fR[\fB:\fIlevel\fR\&...
+[\fB\-v\fR[\fImodule\fR[\fB:\fIdestination\fR[\fB:\fIlevel\fR\&...
 .br
-[\fB\-\-verbose[=\fImodule\fR[\fB:\fIfacility\fR[\fB:\fIlevel\fR\&...
+[\fB\-\-verbose[=\fImodule\fR[\fB:\fIdestination

Re: [ovs-dev] [PATCH 1/8] datapath: Account for rename to vlan_insert_tag_set_proto()

2015-01-06 Thread Thomas Graf
On 01/06/15 at 01:46pm, Pravin Shelar wrote:
> On Fri, Jan 2, 2015 at 10:35 AM, Thomas Graf  wrote:
> > __vlan_put_tag() was renamed to vlan_insert_tag_set_proto() with
> > the argument list kept intact.
> >
> > Upstream: 62749e ("vlan: rename __vlan_put_tag to 
> > vlan_insert_tag_set_proto")
> > Signed-off-by: Thomas Graf 
> > ---
> You can rename __vlan_put_tag() from vxlan.c file so that we can
> rename all __vlan_put_tag() to vlan_insert_tag_set_proto(). Then
> __vlan_put_tag() can removed completely.
> 
> otherwise looks good.
> 
> Acked-by: Pravin B Shelar 

Thanks for the reviews. I'll rework and repost patch 1.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/2] hash: Add hash_add64().

2015-01-06 Thread Jarno Rajahalme

On Dec 29, 2014, at 2:12 PM, Ben Pfaff  wrote:

> On Wed, Dec 17, 2014 at 10:30:41AM -0800, Jarno Rajahalme wrote:
>> Add support for adding 64-bit words to hashes.  This will be used by
>> subsequent patches.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> I noticed that hash_words64() takes a 64-bit basis (which is not new).
> That seems odd because it returns a 32-bit hash value.  Usually the
> basis is used for chaining together the hashes of several values, so
> usually I expect the basis to be the same size as the hash (both
> either 32 or 64 bits, for example).
> 
> Assuming that a 64-bit basis does make sense, would you mind adding
> parentheses here in hash_words64_inline():
>> +hash = basis ^ basis >> 32;
> so that it becomes:
>   hash = basis ^ (basis >> 32);
> 
> Also, again assuming that a 64-bit basis does makes sense, if the
> provided basis is really a hash, then a plain XOR as above should be
> OK, but if it's some other value provided by the caller, without
> necessarily high or evenly distributed entropy, as we occasionally
> use, then a better hash function than XOR might be useful.
> 

I recall that I planned to use the 64-bit miniflow map as a basis value, hence 
the uint64_t. But you are right in that it is weird to have a 64-bit initial 
hash value when the range of the hash function is 32 bits, so I changed it to 
32-bits in a separate patch.

>> +for (i = 0; i < n_words; i++) {
>> +hash = hash_add64(hash, p[i]);
>> +}
>> +return hash_finish(hash, n_words * 8);
>> }
> 
> Other than that philosophical issue:
> Acked-by: Ben Pfaff 

Thanks for the review!

I also fixed the 32-bit compile error.

Merged with master,

  Jarno
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] miniflow: Use 64-bit data.

2015-01-06 Thread Jarno Rajahalme

On Jan 6, 2015, at 1:20 PM, Ben Pfaff  wrote:

> On Tue, Jan 06, 2015 at 11:03:15AM -0800, Jarno Rajahalme wrote:
>> 
>> On Jan 5, 2015, at 2:39 PM, Ben Pfaff  wrote:
>> 
>>> On Mon, Jan 05, 2015 at 02:15:13PM -0800, Ben Pfaff wrote:
 On Mon, Jan 05, 2015 at 02:08:41PM -0800, Jarno Rajahalme wrote:
> 
> On Dec 29, 2014, at 2:27 PM, Ben Pfaff  wrote:
> 
>> On Wed, Dec 17, 2014 at 10:30:42AM -0800, Jarno Rajahalme wrote:
>>> So far the compressed flow data in struct miniflow has been in 32-bit
>>> words with a 63-bit map, allowing for a maximum size of struct flow of
>>> 252 bytes.  With the forthcoming Geneve options this is not sufficient
>>> any more.
>>> 
>>> This patch solves the problem by changing the miniflow data to 64-bit
>>> words, doubling the flow max size to 504 bytes.  Since the word size
>>> is doubled, there is some loss in compression efficiency.  To counter
>>> this some of the flow fields have been reordered to keep related
>>> fields together (e.g., the source and destination IP addresses share
>>> the same 64-bit word).
>>> 
>>> This change should speed up flow data processing on 64-bit CPUs, which
>>> may help counterbalance the impact of making the struct flow bigger in
>>> the future.
>>> 
>>> Classifier lookup stage boundaries are also changed to 64-bit
>>> alignment, as the current algorithm depends on each miniflow word to
>>> not be split between ranges.  This has resulted in new padding (part
>>> of the 'mpls_lse' field).
>>> 
>>> The 'dp_hash' field is also moved to packet metadata to eliminate
>>> otherwise needed padding there.  This allows the L4 to fit into one
>>> 64-bit word, and also makes matches on 'dp_hash' more efficient as
>>> misses can be found already on stage 1.
>>> 
>>> Signed-off-by: Jarno Rajahalme 
> 
>> This seems mostly straightforward.  Are there particular parts you'd
>> like me to look over carefully?
>> 
> 
> Maybe the changes to the miniflow push macros, which get a bit more
> complicated...
 
 OK, I'll do that in a bit.
>>> 
>>> I read them through them.  I didn't spot any bugs, although they're
>>> somewhat tricky.
>> 
>> Is this an Acked-by then?
> 
> Acked-by: Ben Pfaff 

Thanks for the review, pushed to master.

  Jarno


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] tests: Make test independent of the hash function.

2015-01-06 Thread Jarno Rajahalme
Otherwise compiling with -msse4.2 (or -march=native on a SSE4.2
capable CPU) will produce a test failure due to the CRC32-based hash
function being different from mhash.

Signed-off-by: Jarno Rajahalme 
---
 tests/ofproto-dpif.at |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 22050f3..b5d484a 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -194,8 +194,8 @@ AT_CHECK([ovs-appctl revalidator/purge], [0])
 
 # Checks the flow stats in br1, should only be one flow with non-zero
 # 'n_packets' from internal table.
-AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- "n_packets" 
| grep -- "table_id" | sed -e 's/output:[[0-9]][[0-9]]*/output/'], [0], [dnl
-table_id=254, n_packets=1, n_bytes=64, 
priority=20,recirc_id=0x12d,dp_hash=0xa2/0xff,actions=output
+AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- "n_packets" 
| grep -- "table_id" | sed -e 's/dp_hash=0x[[0-9a-f]][[0-9a-f]]*/dp_hash=0x0/' 
-e 's/output:[[0-9]][[0-9]]*/output/'], [0], [dnl
+table_id=254, n_packets=1, n_bytes=64, 
priority=20,recirc_id=0x12d,dp_hash=0x0/0xff,actions=output
 ])
 
 # Checks the flow stats in br-int, should be only one match.
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] tests: Make test independent of the hash function.

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 03:11:33PM -0800, Jarno Rajahalme wrote:
> Otherwise compiling with -msse4.2 (or -march=native on a SSE4.2
> capable CPU) will produce a test failure due to the CRC32-based hash
> function being different from mhash.
> 
> Signed-off-by: Jarno Rajahalme 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] vagrant: Provide basic Vagrantfile

2015-01-06 Thread Thomas Graf
On 01/06/15 at 11:34am, Andy Zhou wrote:
> We have been think about adding unit tests the kernel module.   Any
> plans or objections to develop kmod unit tests on top of this patch?

No objections at all. This is exactly where I want to go with this.
Please go ahead. I have some other things to complete in the current
net-next merge window in parallel but I'll help out any way I can.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Thomas Graf
On 01/06/15 at 10:22am, Ben Pfaff wrote:
> Open vSwitch needs some kind of process for handling vulnerabilities.  So
> far, we've been pretty lucky that way, but it can't last forever, and I
> think we'll be better off if we have at least the outline of an established
> process whenever a significant vulnerability comes along.  Here's my draft
> of a process based on the documentation of the OpenStack process at
> https://wiki.openstack.org/wiki/Vulnerability_Management.
> 
> I don't have a lot of experience with this kind of thing myself, so I'd
> appreciate critical review from anyone who does.
> 
> Signed-off-by: Ben Pfaff 
> Reviewed-by: Flavio Leitner 

Looks perfect to me to ensure we are well prepared. We can revise as
needed when we handle the first case.

Acked-by: Thomas Graf 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Flavio Leitner
On Tuesday, January 06, 2015 10:22:10 AM Ben Pfaff wrote:
> Open vSwitch needs some kind of process for handling vulnerabilities.  So
> far, we've been pretty lucky that way, but it can't last forever, and I
> think we'll be better off if we have at least the outline of an established
> process whenever a significant vulnerability comes along.  Here's my draft
> of a process based on the documentation of the OpenStack process at
> https://wiki.openstack.org/wiki/Vulnerability_Management.
> 
> I don't have a lot of experience with this kind of thing myself, so I'd
> appreciate critical review from anyone who does.
> 
> Signed-off-by: Ben Pfaff 
> Reviewed-by: Flavio Leitner 
> ---
> v1->v2:
>- Suggest GPG signing and encryption.
>- Mention coordination with Linux kernel security process.
>- "ovs-users" is actually "ovs-discuss".
>- Mention SECURITY.md from REPORTING-BUGS.md.
>- Add examples.
> ---
>  Makefile.am   |   3 +-
>  REPORTING-BUGS.md |   2 +
>  SECURITY.md   | 154 
> ++
>  3 files changed, 158 insertions(+), 1 deletion(-)
>  create mode 100644 SECURITY.md
> 
> diff --git a/Makefile.am b/Makefile.am
> index 46e8610..26528d9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,4 +1,4 @@
> -# Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> +# Copyright (C) 2007-2015 Nicira, Inc.
>  #
>  # Copying and distribution of this file, with or without modification,
>  # are permitted in any medium without royalty provided the copyright
> @@ -89,6 +89,7 @@ docs = \
>   README-lisp.md \
>   README-native-tunneling.md \
>   REPORTING-BUGS.md \
> + SECURITY.md \
>   TODO.md \
>   WHY-OVS.md
>  EXTRA_DIST = \
> diff --git a/REPORTING-BUGS.md b/REPORTING-BUGS.md
> index 4e08cb1..c046af9 100644
> --- a/REPORTING-BUGS.md
> +++ b/REPORTING-BUGS.md
> @@ -7,6 +7,8 @@ bugs so as to ensure that they can be fixed as quickly as 
> possible.
>  
>  Please report bugs by sending email to b...@openvswitch.org.  
>  
> +For reporting security vulnerabilities, please read SECURITY.md.
> +
>  The most important parts of your bug report are the following:
>  
>* What you did that make the problem appear.
> diff --git a/SECURITY.md b/SECURITY.md
> new file mode 100644
> index 000..e1db4cb
> --- /dev/null
> +++ b/SECURITY.md
> @@ -0,0 +1,154 @@
> +Security Process
> +
> +
> +This is a proposed security vulnerability reporting and handling
> +process for Open vSwitch.  It is based on the OpenStack vulnerability
> +management process described at
> +https://wiki.openstack.org/wiki/Vulnerability_Management.
> +
> +The OVS security team coordinates vulnerability management using the
> +ovs-security mailing list.  Membership in the security team and
> +subscription to its mailing list consists of a small number of
> +trustworthy people, as determined by rough consensus of the Open
> +vSwitch committers on the ovs-committers mailing list.  The Open
> +vSwitch security team should include Open vSwitch committers, to
> +ensure prompt and accurate vulnerability assessments and patch review.
> +
> +We encourage everyone involved in the security process to GPG-sign
> +their emails.  We additionally encourage GPG-encrypting one-on-one
> +conversations as part of the security process.
> +
> +
> +What is a vulnerability?
> +
> +
> +All vulnerabilities are bugs, but not every bug is a vulnerability.
> +Here are some examples of vulnerabilities to which one would expect to
> +apply this process:
> +
> +* A crafted packet that causes a kernel or userspace crash.
> +
> +* A flow translation bug that misforwards traffic in a way likely
> +  to hop over security boundaries.
> +
> +* An OpenFlow protocol bug that allows a controller to read
> +  arbitrary files from the file system.
> +
> +* Misuse of the OpenSSL library that allows bypassing certificate
> +  checks.
> +
> +* A bug (memory corruption, overflow, ...) that allows one to
> +  modify the behaviour of OVS through external configuration
> +  interfaces such as OVSDB.
> +
> +* Privileged information is exposed to unprivileged users.
> +
> +If in doubt, please do use the vulnerability management process.  At
> +worst, the response will be to report the bug through the usual
> +channels.
> +
> +
> +Step 1: Reception
> +-
> +
> +To report an Open vSwitch vulnerability, send an email to the
> +ovs-security mailing list (see "Contact" at the end of this document).
> +A security team member should reply to the reporter acknowledging that
> +the report has been received.
> +
> +Please consider reporting the information mentioned in
> +REPORTING-BUGS.md, where relevant.
> +
> +The Linux kernel has its own vulnerability management process:
> +https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SecurityBugs
> +Handling of vulnerabiliti

Re: [ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
On Wed, Jan 07, 2015 at 12:17:45AM +0100, Thomas Graf wrote:
> On 01/06/15 at 10:22am, Ben Pfaff wrote:
> > Open vSwitch needs some kind of process for handling vulnerabilities.  So
> > far, we've been pretty lucky that way, but it can't last forever, and I
> > think we'll be better off if we have at least the outline of an established
> > process whenever a significant vulnerability comes along.  Here's my draft
> > of a process based on the documentation of the OpenStack process at
> > https://wiki.openstack.org/wiki/Vulnerability_Management.
> > 
> > I don't have a lot of experience with this kind of thing myself, so I'd
> > appreciate critical review from anyone who does.
> > 
> > Signed-off-by: Ben Pfaff 
> > Reviewed-by: Flavio Leitner 
> 
> Looks perfect to me to ensure we are well prepared. We can revise as
> needed when we handle the first case.
> 
> Acked-by: Thomas Graf 

I agree.

That's three acks or reviews, good enough for me.  I applied this to
master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] Vagrant: Add steps when pulling from git tree to INSTALL.md

2015-01-06 Thread Flavio Leitner
There are a couple missing steps needed in order to
build the Vagrantfile.  This patch adds them to the
INSTALL.md file.

Signed-off-by: Flavio Leitner 
---
 INSTALL.md | 12 
 1 file changed, 12 insertions(+)

diff --git a/INSTALL.md b/INSTALL.md
index 122c362..64a189b 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -567,6 +567,18 @@ Vagrant
 
 Requires: Vagrant and a compatible hypervisor
 
+If you pulled the sources directly from an Open vSwitch Git tree,
+follow these two steps below to build the Vagrantfile:
+
+1. Run boot.sh in the top source directory:
+
+  `% ./boot.sh`
+
+2. Configure the package by running the configure script.  You can
+usually invoke configure without any arguments.  For example:
+
+  `% ./configure`
+
 A Vagrantfile is provided allowing to compile and provision the source
 tree as found locally in a virtual machine using the following commands:
 
-- 
2.1.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] SECURITY: New document describing proposed security process for OVS.

2015-01-06 Thread Ben Pfaff
Oops, I committed this at the same time you sent your message.  Thanks
for agreeing (in IRC) to send it as a patch tomorrow.

On Tue, Jan 6, 2015 at 3:21 PM, Flavio Leitner  wrote:
> On Tuesday, January 06, 2015 10:22:10 AM Ben Pfaff wrote:
>> Open vSwitch needs some kind of process for handling vulnerabilities.  So
>> far, we've been pretty lucky that way, but it can't last forever, and I
>> think we'll be better off if we have at least the outline of an established
>> process whenever a significant vulnerability comes along.  Here's my draft
>> of a process based on the documentation of the OpenStack process at
>> https://wiki.openstack.org/wiki/Vulnerability_Management.
>>
>> I don't have a lot of experience with this kind of thing myself, so I'd
>> appreciate critical review from anyone who does.
>>
>> Signed-off-by: Ben Pfaff 
>> Reviewed-by: Flavio Leitner 
>> ---
>> v1->v2:
>>- Suggest GPG signing and encryption.
>>- Mention coordination with Linux kernel security process.
>>- "ovs-users" is actually "ovs-discuss".
>>- Mention SECURITY.md from REPORTING-BUGS.md.
>>- Add examples.
>> ---
>>  Makefile.am   |   3 +-
>>  REPORTING-BUGS.md |   2 +
>>  SECURITY.md   | 154 
>> ++
>>  3 files changed, 158 insertions(+), 1 deletion(-)
>>  create mode 100644 SECURITY.md
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 46e8610..26528d9 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -1,4 +1,4 @@
>> -# Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>> +# Copyright (C) 2007-2015 Nicira, Inc.
>>  #
>>  # Copying and distribution of this file, with or without modification,
>>  # are permitted in any medium without royalty provided the copyright
>> @@ -89,6 +89,7 @@ docs = \
>>   README-lisp.md \
>>   README-native-tunneling.md \
>>   REPORTING-BUGS.md \
>> + SECURITY.md \
>>   TODO.md \
>>   WHY-OVS.md
>>  EXTRA_DIST = \
>> diff --git a/REPORTING-BUGS.md b/REPORTING-BUGS.md
>> index 4e08cb1..c046af9 100644
>> --- a/REPORTING-BUGS.md
>> +++ b/REPORTING-BUGS.md
>> @@ -7,6 +7,8 @@ bugs so as to ensure that they can be fixed as quickly as 
>> possible.
>>
>>  Please report bugs by sending email to b...@openvswitch.org.
>>
>> +For reporting security vulnerabilities, please read SECURITY.md.
>> +
>>  The most important parts of your bug report are the following:
>>
>>* What you did that make the problem appear.
>> diff --git a/SECURITY.md b/SECURITY.md
>> new file mode 100644
>> index 000..e1db4cb
>> --- /dev/null
>> +++ b/SECURITY.md
>> @@ -0,0 +1,154 @@
>> +Security Process
>> +
>> +
>> +This is a proposed security vulnerability reporting and handling
>> +process for Open vSwitch.  It is based on the OpenStack vulnerability
>> +management process described at
>> +https://wiki.openstack.org/wiki/Vulnerability_Management.
>> +
>> +The OVS security team coordinates vulnerability management using the
>> +ovs-security mailing list.  Membership in the security team and
>> +subscription to its mailing list consists of a small number of
>> +trustworthy people, as determined by rough consensus of the Open
>> +vSwitch committers on the ovs-committers mailing list.  The Open
>> +vSwitch security team should include Open vSwitch committers, to
>> +ensure prompt and accurate vulnerability assessments and patch review.
>> +
>> +We encourage everyone involved in the security process to GPG-sign
>> +their emails.  We additionally encourage GPG-encrypting one-on-one
>> +conversations as part of the security process.
>> +
>> +
>> +What is a vulnerability?
>> +
>> +
>> +All vulnerabilities are bugs, but not every bug is a vulnerability.
>> +Here are some examples of vulnerabilities to which one would expect to
>> +apply this process:
>> +
>> +* A crafted packet that causes a kernel or userspace crash.
>> +
>> +* A flow translation bug that misforwards traffic in a way likely
>> +  to hop over security boundaries.
>> +
>> +* An OpenFlow protocol bug that allows a controller to read
>> +  arbitrary files from the file system.
>> +
>> +* Misuse of the OpenSSL library that allows bypassing certificate
>> +  checks.
>> +
>> +* A bug (memory corruption, overflow, ...) that allows one to
>> +  modify the behaviour of OVS through external configuration
>> +  interfaces such as OVSDB.
>> +
>> +* Privileged information is exposed to unprivileged users.
>> +
>> +If in doubt, please do use the vulnerability management process.  At
>> +worst, the response will be to report the bug through the usual
>> +channels.
>> +
>> +
>> +Step 1: Reception
>> +-
>> +
>> +To report an Open vSwitch vulnerability, send an email to the
>> +ovs-security mailing list (see "Contact" at the end of this document).
>> +A security team member should reply to the reporter acknowledging that
>> +the report has been received.
>> 

Re: [ovs-dev] [PATCH] Vagrant: Add steps when pulling from git tree to INSTALL.md

2015-01-06 Thread Ben Pfaff
On Tue, Jan 06, 2015 at 09:29:04PM -0200, Flavio Leitner wrote:
> There are a couple missing steps needed in order to
> build the Vagrantfile.  This patch adds them to the
> INSTALL.md file.
> 
> Signed-off-by: Flavio Leitner 

Thomas, I assume you'll review this?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Vagrant: Add steps when pulling from git tree to INSTALL.md

2015-01-06 Thread Thomas Graf
On 01/06/15 at 09:29pm, Flavio Leitner wrote:
> There are a couple missing steps needed in order to
> build the Vagrantfile.  This patch adds them to the
> INSTALL.md file.
> 
> Signed-off-by: Flavio Leitner 

Agreed that it's a good idea to mention this requirement.

Should we refer to the "Building and Installing Open vSwitch for
Linux, FreeBSD or NetBSD" section instead which already covers these
steps.

I left this out because other similar sections such as testsuite, ryu,
oftest, ... also assume a configured (and built) local source tree.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Vagrant: Add steps when pulling from git tree to INSTALL.md

2015-01-06 Thread Flavio Leitner
On Wednesday, January 07, 2015 12:35:45 AM Thomas Graf wrote:
> On 01/06/15 at 09:29pm, Flavio Leitner wrote:
> > There are a couple missing steps needed in order to
> > build the Vagrantfile.  This patch adds them to the
> > INSTALL.md file.
> > 
> > Signed-off-by: Flavio Leitner 
> 
> Agreed that it's a good idea to mention this requirement.
> 
> Should we refer to the "Building and Installing Open vSwitch for
> Linux, FreeBSD or NetBSD" section instead which already covers these
> steps.

I thought about that but that section includes the whole process
which is not needed.

> I left this out because other similar sections such as testsuite, ryu,
> oftest, ... also assume a configured (and built) local source tree.

What about create a small section to bootstrap the git tree and
put references where it is needed?

fbl
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Vagrant: Add steps when pulling from git tree to INSTALL.md

2015-01-06 Thread Thomas Graf
On 01/06/15 at 09:45pm, Flavio Leitner wrote:
> On Wednesday, January 07, 2015 12:35:45 AM Thomas Graf wrote:
> > I left this out because other similar sections such as testsuite, ryu,
> > oftest, ... also assume a configured (and built) local source tree.
> 
> What about create a small section to bootstrap the git tree and
> put references where it is needed?

That sounds perfect.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 0/6 net-next] VXLAN Group Policy Extension

2015-01-06 Thread Thomas Graf
Implements supports for the Group Policy VXLAN extension [0] to provide
a lightweight and simple security label mechanism across network peers
based on VXLAN. The security context and associated metadata is mapped
to/from skb->mark. This allows further mapping to a SELinux context
using SECMARK, to implement ACLs directly with nftables, iptables, OVS,
tc, etc.

The extension is disabled by default and should be run on a distinct
port in mixed Linux VXLAN VTEP environments. Liberal VXLAN VTEPs
which ignore unknown reserved bits will be able to receive VXLAN-GBP
frames.

Simple usage example:

10.1.1.1:
   # ip link add vxlan0 type vxlan id 10 remote 10.1.1.2 gbp
   # iptables -I OUTPUT -m owner --uid-owner 101 -j MARK --set-mark 0x200

10.1.1.2:
   # ip link add vxlan0 type vxlan id 10 remote 10.1.1.1 gbp
   # iptables -I INPUT -m mark --mark 0x200 -j DROP

iproute2 [1] and OVS [2] support will be provided in separate patches.

[0] https://tools.ietf.org/html/draft-smith-vxlan-group-policy
[1] https://github.com/tgraf/iproute2/tree/vxlan-gbp
[2] https://github.com/tgraf/ovs/tree/vxlan-gbp

Thomas Graf (6):
  vxlan: Allow for VXLAN extensions to be implemented
  vxlan: Group Policy extension
  vxlan: Only bind to sockets with correct extensions enabled
  vxlan: Fail build if VXLAN header is misdefined
  openvswitch: Rename GENEVE_TUN_OPTS() to TUN_METADATA_OPTS()
  openvswitch: Support VXLAN Group Policy extension

 drivers/net/vxlan.c  | 221 +++
 include/net/vxlan.h  | 104 --
 include/uapi/linux/if_link.h |   8 ++
 include/uapi/linux/openvswitch.h |  19 
 net/openvswitch/flow.c   |   2 +-
 net/openvswitch/flow.h   |  14 +--
 net/openvswitch/flow_netlink.c   | 111 
 net/openvswitch/vport-vxlan.c|  89 +++-
 8 files changed, 435 insertions(+), 133 deletions(-)

-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/6] vxlan: Allow for VXLAN extensions to be implemented

2015-01-06 Thread Thomas Graf
The VXLAN receive code is currently conservative in what it accepts and
will reject any frame that uses any of the reserved VXLAN protocol fields.
The VXLAN draft specifies that "reserved fields MUST be set to zero on
transmit and ignored on receive.".

Retain the current conservative parsing behaviour by default but allows
these fields to be used by VXLAN extensions which are explicitly enabled
on the VXLAN socket respectively VXLAN net_device.

Signed-off-by: Thomas Graf 
---
 drivers/net/vxlan.c | 29 +++--
 include/net/vxlan.h | 32 +---
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 2ab0922..4d52aa9 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -65,7 +65,7 @@
 #define VXLAN_VID_MASK (VXLAN_N_VID - 1)
 #define VXLAN_HLEN (sizeof(struct udphdr) + sizeof(struct vxlanhdr))
 
-#define VXLAN_FLAGS 0x0800 /* struct vxlanhdr.vx_flags required value. */
+#define VXLAN_FLAGS 0x0800 /* struct vxlanhdr.vx_flags default value. */
 
 /* UDP port for VXLAN traffic.
  * The IANA assigned port is 4789, but the Linux default is 8472
@@ -1100,22 +1100,28 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct 
sk_buff *skb)
if (!pskb_may_pull(skb, VXLAN_HLEN))
goto error;
 
+   vs = rcu_dereference_sk_user_data(sk);
+   if (!vs)
+   goto drop;
+
/* Return packets with reserved bits set */
vxh = (struct vxlanhdr *)(udp_hdr(skb) + 1);
-   if (vxh->vx_flags != htonl(VXLAN_FLAGS) ||
-   (vxh->vx_vni & htonl(0xff))) {
-   netdev_dbg(skb->dev, "invalid vxlan flags=%#x vni=%#x\n",
-  ntohl(vxh->vx_flags), ntohl(vxh->vx_vni));
-   goto error;
+
+   /* For backwards compatibility, only allow reserved fields to be
+* used by VXLAN extensions if explicitly requested.
+*/
+   if (vs->exts) {
+   if (!vxh->vni_present)
+   goto error_invalid_header;
+   } else {
+   if (vxh->vx_flags != htonl(VXLAN_FLAGS) ||
+   (vxh->vx_vni & htonl(0xff)))
+   goto error_invalid_header;
}
 
if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB)))
goto drop;
 
-   vs = rcu_dereference_sk_user_data(sk);
-   if (!vs)
-   goto drop;
-
vs->rcv(vs, skb, vxh->vx_vni);
return 0;
 
@@ -1124,6 +1130,9 @@ drop:
kfree_skb(skb);
return 0;
 
+error_invalid_header:
+   netdev_dbg(skb->dev, "invalid vxlan flags=%#x vni=%#x\n",
+  ntohl(vxh->vx_flags), ntohl(vxh->vx_vni));
 error:
/* Return non vxlan pkt */
return 1;
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 903461a..3e98d31 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -11,10 +11,35 @@
 #define VNI_HASH_BITS  10
 #define VNI_HASH_SIZE  (1<"
+#endif
+   __u8vx_reserved1;
+   __be16  vx_reserved2;
+   };
+   __be32 vx_flags;
+   };
+   __be32  vx_vni;
 };
 
 struct vxlan_sock;
@@ -25,6 +50,7 @@ struct vxlan_sock {
struct hlist_node hlist;
vxlan_rcv_t  *rcv;
void *data;
+   u32   exts;
struct work_struct del_work;
struct socket*sock;
struct rcu_head   rcu;
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 3/6] vxlan: Only bind to sockets with correct extensions enabled

2015-01-06 Thread Thomas Graf
A VXLAN net_device looking for an appropriate socket may only
consider a socket which has the exact set of extensions enabled.
If none can be found, a new socket must be created.

The OVS VXLAN port is kept unaware of extensions at this point.

Signed-off-by: Thomas Graf 
---
 drivers/net/vxlan.c   | 35 +--
 include/net/vxlan.h   |  2 +-
 net/openvswitch/vport-vxlan.c |  2 +-
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 30b7b59..2b75c62 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -271,14 +271,15 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct 
vxlan_fdb *fdb)
 }
 
 /* Find VXLAN socket based on network namespace, address family and UDP port */
-static struct vxlan_sock *vxlan_find_sock(struct net *net,
- sa_family_t family, __be16 port)
+static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
+ __be16 port, u32 exts)
 {
struct vxlan_sock *vs;
 
hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
if (inet_sk(vs->sock->sk)->inet_sport == port &&
-   inet_sk(vs->sock->sk)->sk.sk_family == family)
+   inet_sk(vs->sock->sk)->sk.sk_family == family &&
+   vs->exts == exts)
return vs;
}
return NULL;
@@ -298,11 +299,12 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct 
vxlan_sock *vs, u32 id)
 
 /* Look up VNI in a per net namespace table */
 static struct vxlan_dev *vxlan_find_vni(struct net *net, u32 id,
-   sa_family_t family, __be16 port)
+   sa_family_t family, __be16 port,
+   u32 exts)
 {
struct vxlan_sock *vs;
 
-   vs = vxlan_find_sock(net, family, port);
+   vs = vxlan_find_sock(net, family, port, exts);
if (!vs)
return NULL;
 
@@ -1770,7 +1772,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
 
ip_rt_put(rt);
dst_vxlan = vxlan_find_vni(vxlan->net, vni,
-  dst->sa.sa_family, dst_port);
+  dst->sa.sa_family, dst_port,
+  vxlan->exts);
if (!dst_vxlan)
goto tx_error;
vxlan_encap_bypass(skb, vxlan, dst_vxlan);
@@ -1829,7 +1832,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct 
net_device *dev,
 
dst_release(ndst);
dst_vxlan = vxlan_find_vni(vxlan->net, vni,
-  dst->sa.sa_family, dst_port);
+  dst->sa.sa_family, dst_port,
+  vxlan->exts);
if (!dst_vxlan)
goto tx_error;
vxlan_encap_bypass(skb, vxlan, dst_vxlan);
@@ -1999,7 +2003,7 @@ static int vxlan_init(struct net_device *dev)
 
spin_lock(&vn->sock_lock);
vs = vxlan_find_sock(vxlan->net, ipv6 ? AF_INET6 : AF_INET,
-vxlan->dst_port);
+vxlan->dst_port, vxlan->exts);
if (vs && atomic_add_unless(&vs->refcnt, 1, 0)) {
/* If we have a socket with same port already, reuse it */
vxlan_vs_add_dev(vs, vxlan);
@@ -2353,7 +2357,7 @@ static struct socket *vxlan_create_sock(struct net *net, 
bool ipv6,
 /* Create new listen socket if needed */
 static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
  vxlan_rcv_t *rcv, void *data,
- u32 flags)
+ u32 flags, u32 exts)
 {
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
struct vxlan_sock *vs;
@@ -2381,6 +2385,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net 
*net, __be16 port,
atomic_set(&vs->refcnt, 1);
vs->rcv = rcv;
vs->data = data;
+   vs->exts = exts;
 
/* Initialize the vxlan udp offloads structure */
vs->udp_offloads.port = port;
@@ -2405,13 +2410,14 @@ static struct vxlan_sock *vxlan_socket_create(struct 
net *net, __be16 port,
 
 struct vxlan_sock *vxlan_sock_add(struct net *net, __be16 port,
  vxlan_rcv_t *rcv, void *data,
- bool no_share, u32 flags)
+ bool no_share, u32 flags,
+ u32 exts)
 {
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
st

[ovs-dev] [PATCH 4/6] vxlan: Fail build if VXLAN header is misdefined

2015-01-06 Thread Thomas Graf
Due to the complexity of struct vxlanhdr, protect against unwanted
and undesired changes by failing the build if the size of the struct
changes.

Signed-off-by: Thomas Graf 
---
 drivers/net/vxlan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 2b75c62..293d524 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2842,6 +2842,8 @@ static int __init vxlan_init_module(void)
 {
int rc;
 
+   BUILD_BUG_ON(sizeof(struct vxlanhdr) != 8);
+
vxlan_wq = alloc_workqueue("vxlan", 0, 0);
if (!vxlan_wq)
return -ENOMEM;
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 2/6] vxlan: Group Policy extension

2015-01-06 Thread Thomas Graf
Implements supports for the Group Policy VXLAN extension [0] to provide
a lightweight and simple security label mechanism across network peers
based on VXLAN. The security context and associated metadata is mapped
to/from skb->mark. This allows further mapping to a SELinux context
using SECMARK, to implement ACLs directly with nftables, iptables, OVS,
tc, etc.

The group membership is defined by the lower 16 bits of skb->mark, the
upper 16 bits are used for flags.

SELinux allows to manage label to secure local resources. However,
distributed applications require ACLs to implemented across hosts. This
is typically achieved by matching on L2-L4 fields to identify the
original sending host and process on the receiver. On top of that,
netlabel and specifically CIPSO [1] allow to map security contexts to
universal labels.  However, netlabel and CIPSO are relatively complex.
This patch provides a lightweight alternative for overlay network
environments with a trusted underlay. No additional control protocol
is required.

   Host 1:   Host 2:

  Group AGroup BGroup B Group A
  +-+   +-++---+   +-+
  | lxc |   | SELinux CTX || httpd |   | VM  |
  +--+--+   +--+--++---+---+   +--+--+
  \---+---/ \+---/
  |  |
  +---+---+  +---+---+
  | vxlan |  | vxlan |
  +---+---+  +---+---+
  +--+

Backwards compatibility:
A VXLAN-GBP socket can receive standard VXLAN frames and will assign
the default group 0x to such frames. A Linux VXLAN socket will
drop VXLAN-GBP  frames. The extension is therefore disabled by default
and needs to be specifically enabled:

   ip link add [...] type vxlan [...] gbp

In a mixed environment with VXLAN and VXLAN-GBP sockets, the GBP socket
must run on a separate port number.

Examples:
  iptables:
  $ iptables -I OUTPUT -p icmp -j MARK --set-mark 0x200
  $ iptables -I INPUT -i br0 -m mark --mark 0x200 -j ACCEPT

  OVS (patches provided separately):
  in_port=1, actions=load:0x200->NXM_NX_TUN_GBP_ID[],NORMAL

[0] https://tools.ietf.org/html/draft-smith-vxlan-group-policy
[1] http://lwn.net/Articles/204905/

Signed-off-by: Thomas Graf 
---
 drivers/net/vxlan.c   | 155 ++
 include/net/vxlan.h   |  80 --
 include/uapi/linux/if_link.h  |   8 +++
 net/openvswitch/vport-vxlan.c |   9 ++-
 4 files changed, 197 insertions(+), 55 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 4d52aa9..30b7b59 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -132,6 +132,7 @@ struct vxlan_dev {
__u8  tos;  /* TOS override */
__u8  ttl;
u32   flags;/* VXLAN_F_* in vxlan.h */
+   u32   exts; /* Enabled extensions */
 
struct work_struct sock_work;
struct work_struct igmp_join;
@@ -568,7 +569,8 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff 
**head, struct sk_buff
continue;
 
vh2 = (struct vxlanhdr *)(p->data + off_vx);
-   if (vh->vx_vni != vh2->vx_vni) {
+   if (vh->vx_flags != vh2->vx_flags ||
+   vh->vx_vni != vh2->vx_vni) {
NAPI_GRO_CB(p)->same_flow = 0;
continue;
}
@@ -1095,6 +1097,7 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct 
sk_buff *skb)
 {
struct vxlan_sock *vs;
struct vxlanhdr *vxh;
+   struct vxlan_metadata md = {0};
 
/* Need Vxlan and inner Ethernet header to be present */
if (!pskb_may_pull(skb, VXLAN_HLEN))
@@ -1113,6 +1116,19 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct 
sk_buff *skb)
if (vs->exts) {
if (!vxh->vni_present)
goto error_invalid_header;
+
+   if (vxh->gbp_present) {
+   if (!(vs->exts & VXLAN_EXT_GBP))
+   goto error_invalid_header;
+
+   md.gbp = ntohs(vxh->gbp.policy_id);
+
+   if (vxh->gbp.dont_learn)
+   md.gbp |= VXLAN_GBP_DONT_LEARN;
+
+   if (vxh->gbp.policy_applied)
+   md.gbp |= VXLAN_GBP_POLICY_APPLIED;
+   }
} else {
if (vxh->vx_flags != htonl(VXLAN_FLAGS) ||
(vxh->vx_vni & htonl(0xff)))
@@ -1122,7 +1138,8 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct 
sk_buff *skb)
if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB)))
goto drop;
 
-   vs->rcv(vs, skb, vxh->vx_vni);
+   md.vni = vxh->vx_vni;
+   vs->rcv(vs,

[ovs-dev] [PATCH 6/6] openvswitch: Support VXLAN Group Policy extension

2015-01-06 Thread Thomas Graf
Introduces support for the group policy extension to the VXLAN virtual
port. The extension is disabled by default and only enabled if the user
has provided the respective configuration.

  ovs-vsctl add-port br0 vxlan0 -- \
 set Interface vxlan0 type=vxlan options:exts=gbp

The configuration interface to enable the extension is based on a new
attribute OVS_VXLAN_EXT_GBP nested inside OVS_TUNNEL_ATTR_EXTENSION
which can carry additional extensions as needed in the future.

The group policy metadata is handled in the same way as Geneve options
and transported as binary blob in a new Netlink attribute
OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS which is mutually exclusive to the
existing OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS.

Signed-off-by: Thomas Graf 
---
 include/uapi/linux/openvswitch.h | 19 ++
 net/openvswitch/flow_netlink.c   | 78 +--
 net/openvswitch/vport-vxlan.c| 80 +++-
 3 files changed, 148 insertions(+), 29 deletions(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 3a6dcaa..676a89e 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -248,11 +248,29 @@ enum ovs_vport_attr {
 
 #define OVS_VPORT_ATTR_MAX (__OVS_VPORT_ATTR_MAX - 1)
 
+/**
+ * struct ovs_vxlan_opts - VXLAN tunnel options
+ * @gbp: Group policy bits
+ */
+struct ovs_vxlan_opts {
+   __u32 gbp;
+};
+
+enum {
+   OVS_VXLAN_EXT_UNSPEC,
+   OVS_VXLAN_EXT_GBP,
+   __OVS_VXLAN_EXT_MAX,
+};
+
+#define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
+
+
 /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
  */
 enum {
OVS_TUNNEL_ATTR_UNSPEC,
OVS_TUNNEL_ATTR_DST_PORT, /* 16-bit UDP port, used by L4 tunnels. */
+   OVS_TUNNEL_ATTR_EXTENSION,
__OVS_TUNNEL_ATTR_MAX
 };
 
@@ -324,6 +342,7 @@ enum ovs_tunnel_key_attr {
OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS,/* Array of Geneve options. */
OVS_TUNNEL_KEY_ATTR_TP_SRC, /* be16 src Transport Port. */
OVS_TUNNEL_KEY_ATTR_TP_DST, /* be16 dst Transport Port. */
+   OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS, /* struct ovs_vxlan_opts. */
__OVS_TUNNEL_KEY_ATTR_MAX
 };
 
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index c60ae3f..1528709 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -446,6 +446,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
int rem;
bool ttl = false;
__be16 tun_flags = 0;
+   int opts_type = 0;
 
nla_for_each_nested(a, attr, rem) {
int type = nla_type(a);
@@ -463,6 +464,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
[OVS_TUNNEL_KEY_ATTR_TP_DST] = sizeof(u16),
[OVS_TUNNEL_KEY_ATTR_OAM] = 0,
[OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = -1,
+   [OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS] = -1,
};
 
if (type > OVS_TUNNEL_KEY_ATTR_MAX) {
@@ -519,11 +521,18 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
tun_flags |= TUNNEL_OAM;
break;
case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS:
+   case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS:
+   if (opts_type) {
+   OVS_NLERR(log, "Multiple metadata blocks 
provided");
+   return -EINVAL;
+   }
+
err = tun_md_opt_from_nlattr(a, match, is_mask, log);
if (err)
return err;
 
tun_flags |= TUNNEL_OPTIONS_PRESENT;
+   opts_type = type;
break;
default:
OVS_NLERR(log, "Unknown IPv4 tunnel attribute %d",
@@ -552,7 +561,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
}
}
 
-   return 0;
+   return opts_type;
 }
 
 static int __ipv4_tun_to_nlattr(struct sk_buff *skb,
@@ -1537,6 +1546,34 @@ void ovs_match_init(struct sw_flow_match *match,
}
 }
 
+static int validate_and_copy_geneve_opts(struct sw_flow_key *key)
+{
+   struct geneve_opt *option;
+   int opts_len = key->tun_opts_len;
+   bool crit_opt = false;
+
+   option = (struct geneve_opt *) TUN_METADATA_OPTS(key, 
key->tun_opts_len);
+   while (opts_len > 0) {
+   int len;
+
+   if (opts_len < sizeof(*option))
+   return -EINVAL;
+
+   len = sizeof(*option) + option->length * 4;
+   if (len > opts_len)
+   return -EINVAL;
+
+   crit_opt |= !!(option->type & GENEVE_CRIT_OPT_TYPE);
+
+   option = (struct geneve_opt *)((u8 *)option + len);
+   opts_len -= len;
+   };
+
+   k

[ovs-dev] [PATCH 5/6] openvswitch: Rename GENEVE_TUN_OPTS() to TUN_METADATA_OPTS()

2015-01-06 Thread Thomas Graf
A subsequent patch will introduce VXLAN options. Rename the existing
GENEVE_TUN_OPTS() to reflect its extended purpose of carrying generic
tunnel metadata options.

Signed-off-by: Thomas Graf 
---
 net/openvswitch/flow.c |  2 +-
 net/openvswitch/flow.h | 14 +++---
 net/openvswitch/flow_netlink.c | 37 +
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 70bef2a..bfc74ac 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -690,7 +690,7 @@ int ovs_flow_key_extract(const struct ovs_tunnel_info 
*tun_info,
BUILD_BUG_ON((1 << (sizeof(tun_info->options_len) *
   8)) - 1
> sizeof(key->tun_opts));
-   memcpy(GENEVE_OPTS(key, tun_info->options_len),
+   memcpy(TUN_METADATA_OPTS(key, tun_info->options_len),
   tun_info->options, tun_info->options_len);
key->tun_opts_len = tun_info->options_len;
} else {
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index a8b30f3..d3d0a40 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -53,7 +53,7 @@ struct ovs_key_ipv4_tunnel {
 
 struct ovs_tunnel_info {
struct ovs_key_ipv4_tunnel tunnel;
-   const struct geneve_opt *options;
+   const void *options;
u8 options_len;
 };
 
@@ -61,10 +61,10 @@ struct ovs_tunnel_info {
  * maximum size. This allows us to get the benefits of variable length
  * matching for small options.
  */
-#define GENEVE_OPTS(flow_key, opt_len) \
-   ((struct geneve_opt *)((flow_key)->tun_opts + \
-  FIELD_SIZEOF(struct sw_flow_key, tun_opts) - \
-  opt_len))
+#define TUN_METADATA_OFFSET(opt_len) \
+   (FIELD_SIZEOF(struct sw_flow_key, tun_opts) - opt_len)
+#define TUN_METADATA_OPTS(flow_key, opt_len) \
+   ((void *)((flow_key)->tun_opts + TUN_METADATA_OFFSET(opt_len)))
 
 static inline void __ovs_flow_tun_info_init(struct ovs_tunnel_info *tun_info,
__be32 saddr, __be32 daddr,
@@ -73,7 +73,7 @@ static inline void __ovs_flow_tun_info_init(struct 
ovs_tunnel_info *tun_info,
__be16 tp_dst,
__be64 tun_id,
__be16 tun_flags,
-   const struct geneve_opt *opts,
+   const void *opts,
u8 opts_len)
 {
tun_info->tunnel.tun_id = tun_id;
@@ -105,7 +105,7 @@ static inline void ovs_flow_tun_info_init(struct 
ovs_tunnel_info *tun_info,
  __be16 tp_dst,
  __be64 tun_id,
  __be16 tun_flags,
- const struct geneve_opt *opts,
+ const void *opts,
  u8 opts_len)
 {
__ovs_flow_tun_info_init(tun_info, iph->saddr, iph->daddr,
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index d1eecf7..c60ae3f 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -387,20 +387,20 @@ static int parse_flow_nlattrs(const struct nlattr *attr,
return __parse_flow_nlattrs(attr, a, attrsp, log, false);
 }
 
-static int genev_tun_opt_from_nlattr(const struct nlattr *a,
-struct sw_flow_match *match, bool is_mask,
-bool log)
+static int tun_md_opt_from_nlattr(const struct nlattr *a,
+ struct sw_flow_match *match, bool is_mask,
+ bool log)
 {
unsigned long opt_key_offset;
 
if (nla_len(a) > sizeof(match->key->tun_opts)) {
-   OVS_NLERR(log, "Geneve option length err (len %d, max %zu).",
+   OVS_NLERR(log, "Tunnel metadata option length err (len %d, max 
%zu).",
  nla_len(a), sizeof(match->key->tun_opts));
return -EINVAL;
}
 
if (nla_len(a) % 4 != 0) {
-   OVS_NLERR(log, "Geneve opt len %d is not a multiple of 4.",
+   OVS_NLERR(log, "Tunnel metadata opt len %d is not a multiple of 
4.",
  nla_len(a));
return -EINVAL;
}
@@ -424,7 +424,7 @@ static int genev_tun_opt_from_nlattr(const struct nlattr *a,
 * information later.
 */
if (match->key->tun_opts_len != nla_len(a)) {
-   OVS_NLERR(log, "Geneve option len %d != mask len %d",
+   OVS_NLERR(

[ovs-dev] [PATCH 0/8 v3] Datapath backports to support 3.18.x, net, net-next

2015-01-06 Thread Thomas Graf
This series includes several backports which affect the datapath
and brings it closer to upstream. It also allows to build the
datapath against current net and net-next kernels.

v3:
 - As requested by Pravin:
   - Convert compat code to use vlan_insert_tag_set_proto()
   - Remove any reference to old __vlan_insert_tag()
v2:
 - Improved commit message of patch 2
 - New patch to address arg changes to vxlan_sock_add()

Thomas Graf (8):
  datapath: Account for rename to vlan_insert_tag_set_proto()
  datapath: Add __vlan_insert_tag() compat helper if not available
  datapath: move make_writable helper into common code
  datapath: move vlan pop/push functions into common code
  datapath: introduce rtnl ops stub
  datapath: replace remaining users of arch_fast_hash with jhash
  datapath: Account for new flags args of vxlan_sock_add()
  travis: Update build matrix to include latest stable kernels

 .travis.yml   |  17 ++--
 acinclude.m4  |   7 +-
 datapath/actions.c| 120 +-
 datapath/datapath.c   |  11 ++-
 datapath/flow_table.c |   4 +-
 datapath/linux/Modules.mk |   4 -
 datapath/linux/compat/gso.c   |   4 +-
 datapath/linux/compat/hash-x86.c  |  95 
 datapath/linux/compat/hash.c  |  51 ---
 datapath/linux/compat/include/asm/hash.h  |  18 
 datapath/linux/compat/include/linux/hash.h|  44 --
 datapath/linux/compat/include/linux/if_vlan.h |  46 --
 datapath/linux/compat/include/linux/skbuff.h  |  15 
 datapath/linux/compat/include/net/vxlan.h |   2 +-
 datapath/linux/compat/skbuff-openvswitch.c| 110 +++
 datapath/linux/compat/vxlan.c |   8 +-
 datapath/vport-geneve.c   |   6 +-
 datapath/vport-gre.c  |   6 +-
 datapath/vport-internal_dev.c |  21 -
 datapath/vport-internal_dev.h |   2 +
 datapath/vport-vxlan.c|   2 +-
 21 files changed, 248 insertions(+), 345 deletions(-)
 delete mode 100644 datapath/linux/compat/hash-x86.c
 delete mode 100644 datapath/linux/compat/hash.c
 delete mode 100644 datapath/linux/compat/include/asm/hash.h
 delete mode 100644 datapath/linux/compat/include/linux/hash.h

-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/8] datapath: Account for rename to vlan_insert_tag_set_proto()

2015-01-06 Thread Thomas Graf
__vlan_put_tag() was renamed to vlan_insert_tag_set_proto() with
the argument list kept intact.

Upstream: 62749e ("vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto")
Signed-off-by: Thomas Graf 
---
 acinclude.m4  |  1 +
 datapath/actions.c|  2 +-
 datapath/datapath.c   |  2 +-
 datapath/linux/compat/gso.c   |  4 ++--
 datapath/linux/compat/include/linux/if_vlan.h | 17 +++--
 datapath/linux/compat/vxlan.c |  6 +++---
 datapath/vport-geneve.c   |  6 +++---
 datapath/vport-gre.c  |  6 +++---
 datapath/vport-internal_dev.c |  6 +++---
 9 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 3121b09..05dc112 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -376,6 +376,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
 
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [ADD_ALL_VLANS_CMD],
   [OVS_DEFINE([HAVE_VLAN_BUG_WORKAROUND])])
+  OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_insert_tag_set_proto])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/u64_stats_sync.h], 
[u64_stats_fetch_begin_irq])
 
diff --git a/datapath/actions.c b/datapath/actions.c
index 5a1dbe2..9a49cd5 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -292,7 +292,7 @@ static int push_vlan(struct sk_buff *skb, struct 
sw_flow_key *key,
/* push down current VLAN tag */
current_tag = vlan_tx_tag_get(skb);
 
-   if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
+   if (!vlan_insert_tag_set_proto(skb, skb->vlan_proto, 
current_tag))
return -ENOMEM;
/* Update mac_len for subsequent MPLS actions */
skb->mac_len += VLAN_HLEN;
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3607170..ebab68c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -428,7 +428,7 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
if (!nskb)
return -ENOMEM;
 
-   nskb = __vlan_put_tag(nskb, nskb->vlan_proto, 
vlan_tx_tag_get(nskb));
+   nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto, 
vlan_tx_tag_get(nskb));
if (!nskb)
return -ENOMEM;
 
diff --git a/datapath/linux/compat/gso.c b/datapath/linux/compat/gso.c
index 5079f79..56f9493 100644
--- a/datapath/linux/compat/gso.c
+++ b/datapath/linux/compat/gso.c
@@ -110,8 +110,8 @@ int rpl_dev_queue_xmit(struct sk_buff *skb)
features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
  NETIF_F_UFO | NETIF_F_FSO);
 
-   skb = __vlan_put_tag(skb, skb->vlan_proto,
-vlan_tx_tag_get(skb));
+   skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+   vlan_tx_tag_get(skb));
if (unlikely(!skb))
return err;
vlan_set_tci(skb, 0);
diff --git a/datapath/linux/compat/include/linux/if_vlan.h 
b/datapath/linux/compat/include/linux/if_vlan.h
index 730175b..616b3bf 100644
--- a/datapath/linux/compat/include/linux/if_vlan.h
+++ b/datapath/linux/compat/include/linux/if_vlan.h
@@ -5,9 +5,10 @@
 #include 
 #include_next 
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,10,0)
+#ifndef HAVE_VLAN_INSERT_TAG_SET_PROTO
 /*
- * The behavior of __vlan_put_tag() has changed over time:
+ * The behavior of __vlan_put_tag()/vlan_insert_tag_set_proto() has changed
+ * over time:
  *
  *  - In 2.6.26 and earlier, it adjusted both MAC and network header
  *pointers.  (The latter didn't make any sense.)
@@ -16,13 +17,16 @@
  *
  *  - In 2.6.29 and later, it adjusts the MAC header pointer only.
  *
+ *  - In 3.19 and later, it was renamed to vlan_insert_tag_set_proto()
+ *
  * This is the version from 2.6.33.  We unconditionally substitute this version
  * to avoid the need to guess whether the version in the kernel tree is
  * acceptable.
  */
-#define __vlan_put_tag(skb, proto, tag)  rpl__vlan_put_tag(skb, tag)
-
-static inline struct sk_buff *rpl__vlan_put_tag(struct sk_buff *skb, u16 
vlan_tci)
+#define vlan_insert_tag_set_proto(skb, proto, vlan_tci) \
+   rpl_vlan_insert_tag_set_proto(skb, vlan_tci)
+static inline struct sk_buff *rpl_vlan_insert_tag_set_proto(struct sk_buff 
*skb,
+   u16 vlan_tci)
 {
struct vlan_ethhdr *veth;
 
@@ -46,7 +50,9 @@ static inline struct sk_buff *rpl__vlan_put_tag(struct 
sk_buff *skb, u16 vlan_tc
 
return skb;
 }
+#endif
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,10,0)
 static inline struct sk_buff *rpl___vlan_hwaccel_put_tag(struct sk_

[ovs-dev] [PATCH 2/8] datapath: Add __vlan_insert_tag() compat helper if not available

2015-01-06 Thread Thomas Graf
Since older kernels do not have skb->vlan_proto, it is assumed that
kernels which don't provide their own __vlan_insert_tag() will also
not have skb->vlan_proto. The backwards compat function therefore
only supports ETH_P_8021Q as the protocol type.

Upstream: 15255a43 ("vlan: introduce __vlan_insert_tag helper which does not 
free skb")
Signed-off-by: Thomas Graf 
---
 acinclude.m4  |  2 ++
 datapath/linux/compat/include/linux/if_vlan.h | 29 +++
 2 files changed, 31 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index 05dc112..444141d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -377,6 +377,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [ADD_ALL_VLANS_CMD],
   [OVS_DEFINE([HAVE_VLAN_BUG_WORKAROUND])])
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_insert_tag_set_proto])
+  OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [__vlan_insert_tag])
+
 
   OVS_GREP_IFELSE([$KSRC/include/linux/u64_stats_sync.h], 
[u64_stats_fetch_begin_irq])
 
diff --git a/datapath/linux/compat/include/linux/if_vlan.h 
b/datapath/linux/compat/include/linux/if_vlan.h
index 616b3bf..28c4dae 100644
--- a/datapath/linux/compat/include/linux/if_vlan.h
+++ b/datapath/linux/compat/include/linux/if_vlan.h
@@ -106,4 +106,33 @@ static inline void vlan_set_encap_proto(struct sk_buff 
*skb, struct vlan_hdr *vh
skb->protocol = htons(ETH_P_802_2);
 }
 #endif
+
+#ifndef HAVE___VLAN_INSERT_TAG
+/* Kernels which don't have __vlan_insert_tag() also don't have skb->vlan_proto
+ * so ignore the proto paramter.
+ */
+#define __vlan_insert_tag(skb, proto, tci) rpl_vlan_insert_tag(skb, tci)
+static inline int rpl_vlan_insert_tag(struct sk_buff *skb, u16 vlan_tci)
+{
+   struct vlan_ethhdr *veth;
+
+   if (skb_cow_head(skb, VLAN_HLEN) < 0)
+   return -ENOMEM;
+
+   veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN);
+
+   /* Move the mac addresses to the beginning of the new header. */
+   memmove(skb->data, skb->data + VLAN_HLEN, 2 * ETH_ALEN);
+   skb->mac_header -= VLAN_HLEN;
+
+   /* first, the ethernet type */
+   veth->h_vlan_proto = htons(ETH_P_8021Q);
+
+   /* now, the TCI */
+   veth->h_vlan_TCI = htons(ETH_P_8021Q);
+
+   return 0;
+}
+#endif
+
 #endif /* linux/if_vlan.h wrapper */
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 3/8] datapath: move make_writable helper into common code

2015-01-06 Thread Thomas Graf
note that skb_make_writable already exists in net/netfilter/core.c
but does something slightly different.

Upstream: e219512 ("net: move make_writable helper into common code")
Signed-off-by: Thomas Graf 
---
 acinclude.m4 |  1 +
 datapath/actions.c   | 39 ++--
 datapath/linux/compat/include/linux/skbuff.h |  5 
 datapath/linux/compat/skbuff-openvswitch.c   | 13 ++
 4 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 444141d..9766bed 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -345,6 +345,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [int.skb_zerocopy(],
   [OVS_DEFINE([HAVE_SKB_ZEROCOPY])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [l4_rxhash])
+  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_ensure_writable])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/types.h], [bool],
   [OVS_DEFINE([HAVE_BOOL_TYPE])])
diff --git a/datapath/actions.c b/datapath/actions.c
index 9a49cd5..7f61915 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -122,17 +122,6 @@ static bool is_flow_key_valid(const struct sw_flow_key 
*key)
return !!key->eth.type;
 }
 
-static int make_writable(struct sk_buff *skb, int write_len)
-{
-   if (!pskb_may_pull(skb, write_len))
-   return -ENOMEM;
-
-   if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
-   return 0;
-
-   return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
-}
-
 static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 const struct ovs_action_push_mpls *mpls)
 {
@@ -174,7 +163,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key 
*key,
struct ethhdr *hdr;
int err;
 
-   err = make_writable(skb, skb->mac_len + MPLS_HLEN);
+   err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
if (unlikely(err))
return err;
 
@@ -207,7 +196,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key 
*key,
__be32 *stack;
int err;
 
-   err = make_writable(skb, skb->mac_len + MPLS_HLEN);
+   err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
if (unlikely(err))
return err;
 
@@ -229,7 +218,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 
*current_tci)
struct vlan_hdr *vhdr;
int err;
 
-   err = make_writable(skb, VLAN_ETH_HLEN);
+   err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
if (unlikely(err))
return err;
 
@@ -313,7 +302,7 @@ static int set_eth_addr(struct sk_buff *skb, struct 
sw_flow_key *key,
const struct ovs_key_ethernet *eth_key)
 {
int err;
-   err = make_writable(skb, ETH_HLEN);
+   err = skb_ensure_writable(skb, ETH_HLEN);
if (unlikely(err))
return err;
 
@@ -419,8 +408,8 @@ static int set_ipv4(struct sk_buff *skb, struct sw_flow_key 
*key,
struct iphdr *nh;
int err;
 
-   err = make_writable(skb, skb_network_offset(skb) +
-sizeof(struct iphdr));
+   err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ sizeof(struct iphdr));
if (unlikely(err))
return err;
 
@@ -457,8 +446,8 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key 
*key,
__be32 *saddr;
__be32 *daddr;
 
-   err = make_writable(skb, skb_network_offset(skb) +
-   sizeof(struct ipv6hdr));
+   err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ sizeof(struct ipv6hdr));
if (unlikely(err))
return err;
 
@@ -500,7 +489,7 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key 
*key,
return 0;
 }
 
-/* Must follow make_writable() since that can move the skb data. */
+/* Must follow skb_ensure_writable() since that can move the skb data. */
 static void set_tp_port(struct sk_buff *skb, __be16 *port,
 __be16 new_port, __sum16 *check)
 {
@@ -530,8 +519,8 @@ static int set_udp(struct sk_buff *skb, struct sw_flow_key 
*key,
struct udphdr *uh;
int err;
 
-   err = make_writable(skb, skb_transport_offset(skb) +
-sizeof(struct udphdr));
+   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+ sizeof(struct udphdr));
if (unlikely(err))
return err;
 
@@ -555,8 +544,8 @@ static int set_tcp(struct sk_buff *skb, struct sw_flow_key 
*key,
struct tcphdr *th;
int err;
 
-   err = make_writable(skb, skb_transport_offset(skb) +
-sizeof(struct tcphdr));
+   err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+ 

[ovs-dev] [PATCH 4/8] datapath: move vlan pop/push functions into common code

2015-01-06 Thread Thomas Graf
So it can be used from out of openvswitch code.
Did couple of cosmetic changes on the way, namely variable naming and
adding support for 8021AD proto.

Note on backwards compatability:
Unlike the upstream version, the backport of skb_vlan_push() does not
support translating a hardware accelerated 8021AD tag to software.
This is not a problem though as it preserves existing behaviour.

Upstream: 93515d53 ("net: move vlan pop/push functions into common code")
Signed-off-by: Thomas Graf 
---
 acinclude.m4 |  2 +
 datapath/actions.c   | 83 +++-
 datapath/linux/compat/include/linux/skbuff.h | 10 +++
 datapath/linux/compat/skbuff-openvswitch.c   | 97 
 4 files changed, 119 insertions(+), 73 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 9766bed..2579754 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -346,6 +346,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   [OVS_DEFINE([HAVE_SKB_ZEROCOPY])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [l4_rxhash])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_ensure_writable])
+  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_pop])
+  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_push])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/types.h], [bool],
   [OVS_DEFINE([HAVE_BOOL_TYPE])])
diff --git a/datapath/actions.c b/datapath/actions.c
index 7f61915..0ac6684 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -212,90 +212,29 @@ static int set_mpls(struct sk_buff *skb, struct 
sw_flow_key *key,
return 0;
 }
 
-/* remove VLAN header from packet and update csum accordingly. */
-static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
-{
-   struct vlan_hdr *vhdr;
-   int err;
-
-   err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
-   if (unlikely(err))
-   return err;
-
-   if (skb->ip_summed == CHECKSUM_COMPLETE)
-   skb->csum = csum_sub(skb->csum, csum_partial(skb->data
-   + (2 * ETH_ALEN), VLAN_HLEN, 0));
-
-   vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
-   *current_tci = vhdr->h_vlan_TCI;
-
-   memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
-   __skb_pull(skb, VLAN_HLEN);
-
-   vlan_set_encap_proto(skb, vhdr);
-   skb->mac_header += VLAN_HLEN;
-   /* Update mac_len for subsequent MPLS actions */
-   skb->mac_len -= VLAN_HLEN;
-
-   return 0;
-}
-
 static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 {
-   __be16 tci;
int err;
 
-   if (likely(vlan_tx_tag_present(skb))) {
-   vlan_set_tci(skb, 0);
-   } else {
-   if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
-skb->len < VLAN_ETH_HLEN))
-   return 0;
-
-   err = __pop_vlan_tci(skb, &tci);
-   if (err)
-   return err;
-   }
-   /* move next vlan tag to hw accel tag */
-   if (likely(skb->protocol != htons(ETH_P_8021Q) ||
-  skb->len < VLAN_ETH_HLEN)) {
+   err = skb_vlan_pop(skb);
+   if (vlan_tx_tag_present(skb))
+   invalidate_flow_key(key);
+   else
key->eth.tci = 0;
-   return 0;
-   }
-
-   invalidate_flow_key(key);
-   err = __pop_vlan_tci(skb, &tci);
-   if (unlikely(err))
-   return err;
 
-   __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
-   return 0;
+   return err;
 }
 
 static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
 const struct ovs_action_push_vlan *vlan)
 {
-   if (unlikely(vlan_tx_tag_present(skb))) {
-   u16 current_tag;
-
-   /* push down current VLAN tag */
-   current_tag = vlan_tx_tag_get(skb);
-
-   if (!vlan_insert_tag_set_proto(skb, skb->vlan_proto, 
current_tag))
-   return -ENOMEM;
-   /* Update mac_len for subsequent MPLS actions */
-   skb->mac_len += VLAN_HLEN;
-
-   if (skb->ip_summed == CHECKSUM_COMPLETE)
-   skb->csum = csum_add(skb->csum, csum_partial(skb->data
-   + (2 * ETH_ALEN), VLAN_HLEN, 0));
-
+   if (vlan_tx_tag_present(skb))
invalidate_flow_key(key);
-   } else {
+   else
key->eth.tci = vlan->vlan_tci;
-   }
-   __vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & 
~VLAN_TAG_PRESENT);
-   return 0;
+
+   return skb_vlan_push(skb, vlan->vlan_tpid,
+ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
 }
 
 static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
@@ -865,8 +804,6 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
 
 

[ovs-dev] [PATCH 5/8] datapath: introduce rtnl ops stub

2015-01-06 Thread Thomas Graf
This stub now allows userspace to see IFLA_INFO_KIND for ovs master and
IFLA_INFO_SLAVE_KIND for slave.

Upstream: 5b9e7e16 ("openvswitch: introduce rtnl ops stub")
Signed-off-by: Thomas Graf 
---
 datapath/datapath.c   |  9 -
 datapath/vport-internal_dev.c | 15 +++
 datapath/vport-internal_dev.h |  2 ++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index ebab68c..de912f6 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -2149,10 +2149,14 @@ static int __init dp_init(void)
if (err)
goto error;
 
-   err = ovs_flow_init();
+   err = ovs_internal_dev_rtnl_link_register();
if (err)
goto error_action_fifos_exit;
 
+   err = ovs_flow_init();
+   if (err)
+   goto error_unreg_rtnl_link;
+
err = ovs_vport_init();
if (err)
goto error_flow_exit;
@@ -2179,6 +2183,8 @@ error_vport_exit:
ovs_vport_exit();
 error_flow_exit:
ovs_flow_exit();
+error_unreg_rtnl_link:
+   ovs_internal_dev_rtnl_link_unregister();
 error_action_fifos_exit:
action_fifos_exit();
 error:
@@ -2193,6 +2199,7 @@ static void dp_cleanup(void)
rcu_barrier();
ovs_vport_exit();
ovs_flow_exit();
+   ovs_internal_dev_rtnl_link_unregister();
action_fifos_exit();
 }
 
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 997bb3f..b698f6f 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -143,6 +143,10 @@ static const struct net_device_ops internal_dev_netdev_ops 
= {
 #endif
 };
 
+static struct rtnl_link_ops internal_dev_link_ops __read_mostly = {
+   .kind = "openvswitch",
+};
+
 static void do_setup(struct net_device *netdev)
 {
ether_setup(netdev);
@@ -153,6 +157,7 @@ static void do_setup(struct net_device *netdev)
netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
netdev->destructor = internal_dev_destructor;
netdev->ethtool_ops = &internal_dev_ethtool_ops;
+   netdev->rtnl_link_ops = &internal_dev_link_ops;
netdev->tx_queue_len = 0;
 
netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
@@ -300,3 +305,13 @@ struct vport *ovs_internal_dev_get_vport(struct net_device 
*netdev)
 
return internal_dev_priv(netdev)->vport;
 }
+
+int ovs_internal_dev_rtnl_link_register(void)
+{
+   return rtnl_link_register(&internal_dev_link_ops);
+}
+
+void ovs_internal_dev_rtnl_link_unregister(void)
+{
+   rtnl_link_unregister(&internal_dev_link_ops);
+}
diff --git a/datapath/vport-internal_dev.h b/datapath/vport-internal_dev.h
index 9a7d30e..1b179a1 100644
--- a/datapath/vport-internal_dev.h
+++ b/datapath/vport-internal_dev.h
@@ -24,5 +24,7 @@
 
 int ovs_is_internal_dev(const struct net_device *);
 struct vport *ovs_internal_dev_get_vport(struct net_device *);
+int ovs_internal_dev_rtnl_link_register(void);
+void ovs_internal_dev_rtnl_link_unregister(void);
 
 #endif /* vport-internal_dev.h */
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 8/8] travis: Update build matrix to include latest stable kernels

2015-01-06 Thread Thomas Graf
Signed-off-by: Thomas Graf 
---
 .travis.yml | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 7056f54..1ffd15a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -7,16 +7,17 @@ before_install: ./.travis/prepare.sh
 
 env:
   - OPTS="--disable-ssl"
-  - TESTSUITE=1 KERNEL=3.17.4
-  - KERNEL=3.17.4 DPDK=1
+  - TESTSUITE=1 KERNEL=3.18.1
   - TESTSUITE=1 OPTS="--enable-shared"
-  - KERNEL=3.17.4 DPDK=1 OPTS="--enable-shared"
+  - KERNEL=3.17.7 DPDK=1
+  - KERNEL=3.17.7 DPDK=1 OPTS="--enable-shared"
+  - KERNEL=3.17.7
   - KERNEL=3.16.7
-  - KERNEL=3.14.25
-  - KERNEL=3.12.33
-  - KERNEL=3.10.61
-  - KERNEL=3.4.104
-  - KERNEL=2.6.32.64
+  - KERNEL=3.14.27
+  - KERNEL=3.12.35
+  - KERNEL=3.10.63
+  - KERNEL=3.4.105
+  - KERNEL=2.6.32.65
 
 script: ./.travis/build.sh $OPTS
 
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 7/8] datapath: Account for new flags args of vxlan_sock_add()

2015-01-06 Thread Thomas Graf
The upstream commit 359a0ea
("vxlan: Add support for UDP checksums (v4 sending, v6 zero csums)")
has introduced a new flags argument to vxlan_sock_add().

OVS does not pass any flags at this point, thus specyfing 0 will be
compatible with both the old ipv6 bool and the new u32 flags argument.

Upstream: 359a0ea ("vxlan: Add support for UDP checksums (v4 sending, v6 zero 
csums)")
Signed-off-by: Thomas Graf 
---
 datapath/linux/compat/include/net/vxlan.h | 2 +-
 datapath/linux/compat/vxlan.c | 2 +-
 datapath/vport-vxlan.c| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/datapath/linux/compat/include/net/vxlan.h 
b/datapath/linux/compat/include/net/vxlan.h
index 5fc4dea..83e9210 100644
--- a/datapath/linux/compat/include/net/vxlan.h
+++ b/datapath/linux/compat/include/net/vxlan.h
@@ -53,7 +53,7 @@ struct vxlan_sock {
 
 struct vxlan_sock *vxlan_sock_add(struct net *net, __be16 port,
  vxlan_rcv_t *rcv, void *data,
- bool no_share, bool ipv6);
+ bool no_share, u32 flags);
 
 void vxlan_sock_release(struct vxlan_sock *vs);
 
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 49d3864..c509ee8 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -299,7 +299,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net 
*net, __be16 port,
 
 struct vxlan_sock *vxlan_sock_add(struct net *net, __be16 port,
  vxlan_rcv_t *rcv, void *data,
- bool no_share, bool ipv6)
+ bool no_share, u32 flags)
 {
return vxlan_socket_create(net, port, rcv, data);
 }
diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
index 81347f2..c9e356e 100644
--- a/datapath/vport-vxlan.c
+++ b/datapath/vport-vxlan.c
@@ -126,7 +126,7 @@ static struct vport *vxlan_tnl_create(const struct 
vport_parms *parms)
vxlan_port = vxlan_vport(vport);
strncpy(vxlan_port->name, parms->name, IFNAMSIZ);
 
-   vs = vxlan_sock_add(net, htons(dst_port), vxlan_rcv, vport, true, 
false);
+   vs = vxlan_sock_add(net, htons(dst_port), vxlan_rcv, vport, true, 0);
if (IS_ERR(vs)) {
ovs_vport_free(vport);
return (void *)vs;
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 6/8] datapath: replace remaining users of arch_fast_hash with jhash

2015-01-06 Thread Thomas Graf
This patch effectively reverts commit 500f80872645 ("net: ovs: use CRC32
accelerated flow hash if available"), and other remaining arch_fast_hash()
users such as from nfsd via commit 6282cd565553 ("NFSD: Don't hand out
delegations for 30 seconds after recalling them.") where it has been used
as a hash function for bloom filtering.

While we think that these users are actually not much of concern, it has
been requested to remove the arch_fast_hash() library bits that arose
from [1] entirely as per recent discussion [2]. The main argument is that
using it as a hash may introduce bias due to its linearity (see avalanche
criterion) and thus makes it less clear (though we tried to document that)
when this security/performance trade-off is actually acceptable for a
general purpose library function.

Lets therefore avoid any further confusion on this matter and remove it to
prevent any future accidental misuse of it. For the time being, this is
going to make hashing of flow keys a bit more expensive in the ovs case,
but future work could reevaluate a different hashing discipline.

  [1] https://patchwork.ozlabs.org/patch/299369/
  [2] https://patchwork.ozlabs.org/patch/418756/

Upstream: 8754589 ("net: replace remaining users of arch_fast_hash with jhash")
Signed-off-by: Thomas Graf 
---
 acinclude.m4   |  1 -
 datapath/flow_table.c  |  4 +-
 datapath/linux/Modules.mk  |  4 --
 datapath/linux/compat/hash-x86.c   | 95 --
 datapath/linux/compat/hash.c   | 51 
 datapath/linux/compat/include/asm/hash.h   | 18 --
 datapath/linux/compat/include/linux/hash.h | 44 --
 7 files changed, 2 insertions(+), 215 deletions(-)
 delete mode 100644 datapath/linux/compat/hash-x86.c
 delete mode 100644 datapath/linux/compat/hash.c
 delete mode 100644 datapath/linux/compat/include/asm/hash.h
 delete mode 100644 datapath/linux/compat/include/linux/hash.h

diff --git a/acinclude.m4 b/acinclude.m4
index 2579754..10ede83 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -284,7 +284,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
 
   OVS_GREP_IFELSE([$KSRC/include/linux/err.h], [ERR_CAST])
   OVS_GREP_IFELSE([$KSRC/include/linux/err.h], [IS_ERR_OR_NULL])
-  OVS_GREP_IFELSE([$KSRC/include/linux/hash.h], [fast_hash_ops])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [eth_hw_addr_random])
   OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [ether_addr_copy])
diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index ad410fd..2f8f3fb 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -25,7 +25,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -448,7 +448,7 @@ static u32 flow_hash(const struct sw_flow_key *key, int 
key_start,
/* Make sure number of hash bytes are multiple of u32. */
BUILD_BUG_ON(sizeof(long) % sizeof(u32));
 
-   return arch_fast_hash2(hash_key, hash_u32s, 0);
+   return jhash2(hash_key, hash_u32s, 0);
 }
 
 static int flow_key_start(const struct sw_flow_key *key)
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 4b38fd5..a463331 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -6,8 +6,6 @@ openvswitch_sources += \
linux/compat/gre.c \
linux/compat/gso.c \
linux/compat/genetlink-openvswitch.c \
-   linux/compat/hash.c \
-   linux/compat/hash-x86.c \
linux/compat/ip_tunnels_core.c \
linux/compat/netdevice.c \
linux/compat/net_namespace.c \
@@ -17,7 +15,6 @@ openvswitch_sources += \
linux/compat/utils.c
 openvswitch_headers += \
linux/compat/gso.h \
-   linux/compat/include/asm/hash.h \
linux/compat/include/linux/percpu.h \
linux/compat/include/linux/bug.h \
linux/compat/include/linux/compiler.h \
@@ -26,7 +23,6 @@ openvswitch_headers += \
linux/compat/include/linux/err.h \
linux/compat/include/linux/etherdevice.h \
linux/compat/include/linux/flex_array.h \
-   linux/compat/include/linux/hash.h \
linux/compat/include/linux/icmp.h \
linux/compat/include/linux/icmpv6.h \
linux/compat/include/linux/if.h \
diff --git a/datapath/linux/compat/hash-x86.c b/datapath/linux/compat/hash-x86.c
deleted file mode 100644
index ca259b9..000
--- a/datapath/linux/compat/hash-x86.c
+++ /dev/null
@@ -1,95 +0,0 @@
-/*
- * Some portions derived from code covered by the following notice:
- *
- * Copyright (c) 2010-2013 Intel Corporation. All rights reserved.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- *   * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *   * Redistributions in bina

Re: [ovs-dev] [PATCH 2/6] vxlan: Group Policy extension

2015-01-06 Thread Alexei Starovoitov
On Tue, Jan 6, 2015 at 6:05 PM, Thomas Graf  wrote:
> +struct vxlan_gbp {
> +#ifdef __LITTLE_ENDIAN_BITFIELD
> +   __u8reserved_flags1:3,
...
> +   __be16 policy_id;
> +} __packed;

are you sure that compiler will be smart enough
to do single short load when you pack
u8 + struct vxlan_gbp inside struct vxlanhdr ?
I suspect compiler will use two byte loads
with shifts and ors every time to access policy_id.
Even it works ok, I think this struct layout is ugly.
imo would be much easier to read if you replace
the whole vxlanhdr with vxlanhdr_gbp
or split vxlanhdr into two 32-bit structs.
then __packed hacks won't be needed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/6] vxlan: Allow for VXLAN extensions to be implemented

2015-01-06 Thread Tom Herbert
On Tue, Jan 6, 2015 at 6:05 PM, Thomas Graf  wrote:
> The VXLAN receive code is currently conservative in what it accepts and
> will reject any frame that uses any of the reserved VXLAN protocol fields.
> The VXLAN draft specifies that "reserved fields MUST be set to zero on
> transmit and ignored on receive.".
>
IMO it is an unfortunate decision in VXLAN to ignore set reserved
fields on receive. Had they not done this, then adding a protocol
field to the header would have been feasible and we wouldn't need yet
another encapsulation protocol (i.e. VXLAN-GPE). Rejecting frames with
reserved bits set is the better behavior, but I think the comment
about this needs to be clear about why this diverges from RFC7348.

> Retain the current conservative parsing behaviour by default but allows
> these fields to be used by VXLAN extensions which are explicitly enabled
> on the VXLAN socket respectively VXLAN net_device.
>
Conceptually, VXLAN has both mandatory flags and optional flags for
extensions. You may want to look at the VXLAN RCO patches that added
an extension and infrastructure for them.

Tom

> Signed-off-by: Thomas Graf 
> ---
>  drivers/net/vxlan.c | 29 +++--
>  include/net/vxlan.h | 32 +---
>  2 files changed, 48 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 2ab0922..4d52aa9 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -65,7 +65,7 @@
>  #define VXLAN_VID_MASK (VXLAN_N_VID - 1)
>  #define VXLAN_HLEN (sizeof(struct udphdr) + sizeof(struct vxlanhdr))
>
> -#define VXLAN_FLAGS 0x0800 /* struct vxlanhdr.vx_flags required value. */
> +#define VXLAN_FLAGS 0x0800 /* struct vxlanhdr.vx_flags default value. */
>
>  /* UDP port for VXLAN traffic.
>   * The IANA assigned port is 4789, but the Linux default is 8472
> @@ -1100,22 +1100,28 @@ static int vxlan_udp_encap_recv(struct sock *sk, 
> struct sk_buff *skb)
> if (!pskb_may_pull(skb, VXLAN_HLEN))
> goto error;
>
> +   vs = rcu_dereference_sk_user_data(sk);
> +   if (!vs)
> +   goto drop;
> +
> /* Return packets with reserved bits set */
> vxh = (struct vxlanhdr *)(udp_hdr(skb) + 1);
> -   if (vxh->vx_flags != htonl(VXLAN_FLAGS) ||
> -   (vxh->vx_vni & htonl(0xff))) {
> -   netdev_dbg(skb->dev, "invalid vxlan flags=%#x vni=%#x\n",
> -  ntohl(vxh->vx_flags), ntohl(vxh->vx_vni));
> -   goto error;
> +
> +   /* For backwards compatibility, only allow reserved fields to be
> +* used by VXLAN extensions if explicitly requested.
> +*/
> +   if (vs->exts) {
> +   if (!vxh->vni_present)
> +   goto error_invalid_header;
> +   } else {
> +   if (vxh->vx_flags != htonl(VXLAN_FLAGS) ||
> +   (vxh->vx_vni & htonl(0xff)))
> +   goto error_invalid_header;
> }
>
> if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB)))
> goto drop;
>
> -   vs = rcu_dereference_sk_user_data(sk);
> -   if (!vs)
> -   goto drop;
> -
> vs->rcv(vs, skb, vxh->vx_vni);
> return 0;
>
> @@ -1124,6 +1130,9 @@ drop:
> kfree_skb(skb);
> return 0;
>
> +error_invalid_header:
> +   netdev_dbg(skb->dev, "invalid vxlan flags=%#x vni=%#x\n",
> +  ntohl(vxh->vx_flags), ntohl(vxh->vx_vni));
>  error:
> /* Return non vxlan pkt */
> return 1;
> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index 903461a..3e98d31 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -11,10 +11,35 @@
>  #define VNI_HASH_BITS  10
>  #define VNI_HASH_SIZE  (1<
> -/* VXLAN protocol header */
> +/* VXLAN protocol header:
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |R|R|R|R|I|R|R|R|   Reserved|
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |VXLAN Network Identifier (VNI) |   Reserved|
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + *
> + * I = 1   VXLAN Network Identifier (VNI) present
> + */
>  struct vxlanhdr {
> -   __be32 vx_flags;
> -   __be32 vx_vni;
> +   union {
> +   struct {
> +#ifdef __LITTLE_ENDIAN_BITFIELD
> +   __u8reserved_flags1:3,
> +   vni_present:1,
> +   reserved_flags2:4;
> +#elif defined(__BIG_ENDIAN_BITFIELD)
> +   __u8reserved_flags2:4,
> +   vni_present:1,
> +   reserved_flags1:3;
> +#else
> +#error "Please fix "
> +#endif
> +   __u8vx_reserved1;
> +   __be16  vx_reserved2;
> +   };
> +   __be32 vx_flags;
> +  

Re: [ovs-dev] [PATCH 1/8] datapath: Account for rename to vlan_insert_tag_set_proto()

2015-01-06 Thread Pravin Shelar
On Tue, Jan 6, 2015 at 6:10 PM, Thomas Graf  wrote:
> __vlan_put_tag() was renamed to vlan_insert_tag_set_proto() with
> the argument list kept intact.
>
> Upstream: 62749e ("vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto")
> Signed-off-by: Thomas Graf 
Looks good to me.

Acked-by: Pravin B Shelar 

Thanks,
Pravin.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] bug in flow stats for VLAN acceleration?

2015-01-06 Thread Motonori Shindo
Ben,

Thanks for addressing this issue. I found that it was just merged into
master. I will verify the new behavior first and then start working on
fixing NetFlow part. Will keep you posted.

Regards,

---
Motonori Shindo

2015-01-01 1:48 GMT+09:00 Ben Pfaff :

> I sent out a patch that should fix this:
> http://openvswitch.org/pipermail/dev/2014-December/049972.html
>
> It will take some time, probably a few days, for it to get into the
> kernel and the OVS tree's kernel module.  After that, I think you'll be
> ready to go.  (However, the buggy behavior will still be present if a
> user upgrades userspace without upgrading their kernel module.  I doubt
> that that is really a big deal though.)
>
> On Thu, Dec 11, 2014 at 12:36:51AM +0900, Motonori Shindo wrote:
> > Ben and Jesse,
> >
> > Thanks for picking this issue up.
> >
> > Although current behavior is not consistent in VLAN and no-VLAN cases,
> as far as NetFlow concerns it doesn’t matter in fact because NetFlow only
> counts L3 packet size anyway.
> >
> > Here’s the story. First, I wanted to fix an issue that OVS currently
> reports L2 packet size, not L3 packet size in NetFlow flow records (NetFlow
> should report L3 packet size). I thought the easiest way to fix this
> problem was to subtract the ethernet header length when reporting a NetFlow
> flow record. Then, I realized that I should also take VLAN header into
> account if it is present. I checked how NetFlow behaves in VLAN case and
> no-VLAN case and found out that OVS reports the same packet size in NetFlow
> regardless to the presence of VLAN header.
> >
> > Once this packet size issue related to VLAN header gets solved by
> someone else (as this is probably beyond of my understanding of the
> relevant code), I will work on fixing the L2 vs L3 packet size issue
> specifically in NetFlow.
> >
> > Regards,
> >
> > On 2014/12/09 3:22, "Jesse Gross"  je...@nicira.com>> wrote:
> >
> > On Mon, Dec 8, 2014 at 9:58 AM, Ben Pfaff  b...@nicira.com>> wrote:
> > Motonori Shindo (CCed) reported to me recently that NetFlow does not
> > include VLAN headers in the byte counts that it exports.  Taking a
> > look at the kernel datapath code, I see that ovs_flow_stats_update()
> > accounts raw skb->len to byte counters.  I suspect that it should add
> > 4 if a VLAN header was present but removed by VLAN acceleration.
> >
> > Presumably we should also do this for port stats as well to be
> consistent.
> >
> >
> >
> > ---
> > Motonori Shindo
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net] gso: do GSO for local skb with size bigger than MTU

2015-01-06 Thread Fan Du

于 2015年01月07日 03:11, Jesse Gross 写道:

One of the reasons for only doing path MTU discovery
>>for L3 is that it operates seamlessly as part of normal operation -
>>there is no need to forge addresses or potentially generate ICMP when
>>on an L2 network. However, this ignores the IP handling that is going
>>on (note that in OVS it is possible for L3 to be implemented as a set
>>of flows coming from a controller).
>>
>>It also should not be VXLAN specific or duplicate VXLAN encapsulation
>>code. As this is happening before encapsulation, the generated ICMP
>>does not need to be encapsulated either if it is created in the right
>>location.

>
>Yes, I agree. GRE share the same issue from the code flow.
>Pushing back ICMP msg back without encapsulation without circulating down
>to physical device is possible. The "right location" as far as I know
>could only be in ovs_vport_send. In addition this probably requires wrapper
>route looking up operation for GRE/VXLAN, after get the under layer device
>MTU
>from the routing information, then calculate reduced MTU becomes feasible.

As I said, it needs to be integrated into L3 processing. In OVS this
would mean adding some primitives to the kernel and then exposing the
functionality upwards into userspace/controller.


I'm bit of confused with "L3 processing" you mentioned here... SORRY
Apparently I'm not seeing the whole picture as you pointed out. Could you please
elaborate "L3 processing" a bit more? docs/codes/or other useful links. 
Appreciated.

My understanding is:
controller sets the forwarding rules into kernel datapath, any flow not matching
with the rules are threw to controller by upcall. Once the rule decision is made
by controller, then, this flow packet is pushed down to datapath to be forwarded
again according to the new rule.

So I'm not sure whether pushing the over-MTU-sized packet or pushing the forged 
ICMP
without encapsulation to controller is required by current ovs implementation. 
By doing
so, such over-MTU-sized packet is treated as a event for the controller to be 
take
care of.



--
No zuo no die but I have to try.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev