Re: [ovs-dev] [PATCH] netdev-vport: Add Stateless TCP Tunneling protocol

2015-01-21 Thread Thomas Graf
On 01/20/15 at 02:55pm, Jesse Gross wrote:
> On Tue, Jan 20, 2015 at 12:26 PM, Pravin B Shelar  wrote:
> > From: Jesse Gross 
> >
> > Add STT netev-vport so that vswitchd can create STT vport in kernel
> > datapath.
> >
> > Signed-off-by: Jesse Gross 
> > Signed-off-by: Pravin B Shelar 
> 
> The references to STTv0 look a little funny to me - while it is in
> fact v0 of the protocol, there are no later versions.
> 
> I think supporting ipsec_stt requires some additional updates to
> ovs-monitor-ipsec.
> 
> It might be nice to beef up the documentation a some with a bit of a
> description on why the protocol should be used or a link the draft.

I think a FAQ entry to describe the issues of STT with stateful
firewalls would be helpful as well.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC v6 1/1] netdev-dpdk: add dpdk vhost ports

2015-01-21 Thread Michael S. Tsirkin
On Thu, Jan 08, 2015 at 11:05:02PM +, Kevin Traynor wrote:
> This patch adds support for a new port type to userspace datapath
> called dpdkvhost. This allows KVM (QEMU) to offload the servicing
> of virtio-net devices to its associated dpdkvhost port. Instructions
> for use are in INSTALL.DPDK.
> 
> This has been tested on Intel multi-core platforms and with clients
> that have virtio-net interfaces.
> 
>  ver 6:
>- rebased with master
>- modified to use DPDK v1.8.0 vhost library
>- reworked for review comments
>  ver 5:
>- rebased against latest master
>  ver 4:
>- added eventfd_link.h and eventfd_link.c to EXTRA_DIST in
>  utilities/automake.mk
>- rebased with master to work with DPDK 1.7 ver 3:
>- rebased with master
>  ver 2:
>- rebased with master
> 
> Signed-off-by: Ciara Loftus 
> Signed-off-by: Kevin Traynor 
> Signed-off-by: Maryam Tahhan 
> ---
>  INSTALL.DPDK.md |  236 +
>  Makefile.am |4 +
>  lib/automake.mk |1 +
>  lib/netdev-dpdk.c   |  649 
> +++
>  lib/netdev.c|3 +-
>  utilities/automake.mk   |3 +-
>  utilities/qemu-wrap.py  |  389 
>  vswitchd/ovs-vswitchd.c |4 +-
>  8 files changed, 1177 insertions(+), 112 deletions(-)
>  mode change 100644 => 100755 lib/netdev-dpdk.c
>  create mode 100755 utilities/qemu-wrap.py
> 
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> index 2cc7636..da8116d 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -17,6 +17,7 @@ Building and Installing:
>  
>  
>  Required DPDK 1.7
> +Optional `fuse`, `fuse-devel`
>  
>  1. Configure build & install DPDK:
>1. Set `$DPDK_DIR`
> @@ -264,6 +265,241 @@ A general rule of thumb for better performance is that 
> the client
>  application should not be assigned the same dpdk core mask "-c" as
>  the vswitchd.
>  
> +DPDK vHost:
> +---
> +
> +Prerequisites:
> +1.  DPDK 1.8 with vHost support enabled and recompile OVS as above.
> +
> + Update `config/common_linuxapp` so that DPDK is built with vHost
> + libraries:
> +
> + `CONFIG_RTE_LIBRTE_VHOST=y`
> +
> +2.  Insert the Fuse module:
> +
> +  `modprobe fuse`
> +
> +3.  Build and insert the `eventfd_link` module:
> +
> + `cd $DPDK_DIR/lib/librte_vhost/eventfd_link/`
> + `make`
> + `insmod $DPDK_DIR/lib/librte_vhost/eventfd_link.ko`
> +
> +4.  Remove /dev/vhost-net character device:
> +
> +  `rm -rf /dev/vhost-net`

I think it's not a good idea to tell people to do this,
best to drop this section and put "with standard vhost"
here instead.

> +
> +Following the steps above to create a bridge, you can now add DPDK vHost
> +as a port to the vswitch.
> +
> +`ovs-vsctl add-port br0 dpdkvhost0 -- set Interface dpdkvhost0 
> type=dpdkvhost`
> +
> +Unlike DPDK ring ports, DPDK vHost ports can have arbitrary names:
> +
> +`ovs-vsctl add-port br0 port123ABC -- set Interface port123ABC 
> type=dpdkvhost`
> +
> +However, please note that when attaching userspace devices to QEMU, the
> +name provided during the add-port operation must match the ifname parameter
> +on the QEMU command line.
> +
> +DPDK vHost VM configuration:
> +
> +
> +1. Configure virtio-net adaptors:
> +   The guest must be configured with virtio-net adapters and offloads
> +   MUST BE DISABLED.

Any plans to address this?

> +This means the following parameters should be passed
> +   to the QEMU binary:
> +
> + ```
> + -netdev tap,id=,script=no,downscript=no,ifname=,vhost=on
> + -device virtio-net-pci,netdev=net1,mac=,csum=off,gso=off,
> + guest_tso4=off,guest_tso6=off,guest_ecn=off
> + ```
> +
> + Repeat the above parameters for multiple devices.
> +
> +2. Configure huge pages:
> +   QEMU must allocate the VM's memory on hugetlbfs. Vhost ports access a
> +   virtio-net device's virtual rings and packet buffers mapping the VM's
> +   physical memory on hugetlbfs. To enable vhost-ports to map the VM's
> +   memory into their process address space, pass the following paramters
> +   to QEMU:
> +
> + `-mem-path /dev/hugepages -mem-prealloc`

I guess you also need to request MAP_SHARED mappings - otherwise
I think you won't be able to poke at them.

> +
> +DPDK vHost with standard vHost:
> +---
> +
> +DPDK vHost ports use a Linux* character device to communicate with QEMU.
> +By default it is set to `/dev/vhost-net`. This conflicts with the kernel
> +vHost device, hence the need to remove `/dev/vhost-net` above. However,
> +if you wish to use kernel vhost in parallel, you can specify an
> +alternative basename on the vswitchd command line like so:
> +
> + `./vswitchd/ovs-vswitchd --dpdk --basename my-vhost-net -c 0x1 ...`
> +
> +Note that the basename arguement and associated string must be the first
> +arguements after `--dpdk` and come before the EAL arguements.
> +
> +

[ovs-dev] [PATCH 1/2] ofp-errors: Explain the whole "expected duplications" thing.

2015-01-21 Thread Ben Pfaff
That way I won't have to figure it out again the next time.

Signed-off-by: Ben Pfaff 
---
 lib/ofp-errors.h | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index 69c92f9..342877d 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -45,7 +45,8 @@ struct ofpbuf;
 
 #define OFPERR_OFS (1 << 30)
 
-/* OpenFlow error codes.
+/* OpenFlow error codes
+ * 
  *
  * The comments below are parsed by the extract-ofp-errors program at build
  * time and used to determine the mapping between "enum ofperr" constants and
@@ -71,13 +72,46 @@ struct ofpbuf;
  *   - Additional text is a human-readable description of the meaning of each
  * error, used to explain the error to the user.  Any text enclosed in
  * square brackets is omitted; this can be used to explain rationale for
- * choice of error codes in the case where this is desirable. */
+ * choice of error codes in the case where this is desirable.
+ *
+ *
+ * Expected duplications
+ * -
+ *
+ * Occasionally, in one version of OpenFlow a single named error can indicate
+ * two or more distinct errors, then a later version of OpenFlow splits those
+ * meanings into different error codes.  When that happens, both errors are
+ * assigned the same value in the earlier version.  That is ordinarily a
+ * mistake, so the build system reports an error.  When that happens, add the
+ * error message to the list of "Expected duplications" below to suppress the
+ * error.  In such a case, the named error defined earlier is how OVS
+ * interprets the earlier, merged form of the error.
+ *
+ * For example, OpenFlow 1.1 defined (3,5) as OFPBIC_UNSUP_EXP_INST, then
+ * OpenFlow 1.2 broke this error into OFPBIC_BAD_EXPERIMENTER as (3,5) and
+ * OFPBIC_BAD_EXT_TYPE as (3,6).  To allow the OVS code to report just a single
+ * error code, instead of protocol version dependent errors, this list of
+ * errors only lists the latter two errors, giving both of them the same code
+ * (3,5) for OpenFlow 1.1.  Then, when OVS serializes either error into
+ * OpenFlow 1.1, it uses the same code (3,5).  In the other direction, when OVS
+ * deserializes (3,5) from OpenFlow 1.1, it translates it into
+ * OFPBIC_BAD_EXPERIMENTER (because its definition precedes that of
+ * OFPBIC_BAD_EXT_TYPE below).  See the "encoding OFPBIC_* experimenter errors"
+ * and "decoding OFPBIC_* experimenter errors" tests in tests/ofp-errors.at for
+ * full details.
+ */
 enum ofperr {
 /* Expected duplications. */
 
 /* Expected: 0x0,3,5 in OF1.1 means both OFPBIC_BAD_EXPERIMENTER and
  * OFPBIC_BAD_EXP_TYPE. */
 
+/* Expected: 0x0,1,5 in OF1.0 means both OFPBRC_EPERM and
+ * OFPBRC_IS_SLAVE. */
+
+/* Expected: 0x0,1,5 in OF1.1 means both OFPBRC_EPERM and
+ * OFPBRC_IS_SLAVE. */
+
 /* ## -- ## */
 /* ## OFPET_HELLO_FAILED ## */
 /* ## -- ## */
-- 
2.1.3

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


[ovs-dev] [PATCH 2/2] ofp-errors: Use OFPERR_OFPBRC_IS_SLAVE to reject slave controllers.

2015-01-21 Thread Ben Pfaff
This error code was just overlooked before.

Reported-by: Anup Khadka 
Signed-off-by: Ben Pfaff 
---
 lib/ofp-errors.h  | 2 +-
 ofproto/ofproto.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index 342877d..aff2075 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -160,7 +160,7 @@ enum ofperr {
  *   code defined the specification. ] */
 OFPERR_OFPBRC_BAD_TABLE_ID,
 
-/* OF1.2+(1,10).  Denied because controller is slave. */
+/* OF1.0-1.1(1,5), OF1.2+(1,10).  Denied because controller is slave. */
 OFPERR_OFPBRC_IS_SLAVE,
 
 /* NX1.0-1.1(1,514), OF1.2+(1,11).  Invalid port.  [ A non-standard error
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ba4263e..b3909ad 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3110,7 +3110,7 @@ reject_slave_controller(struct ofconn *ofconn)
 {
 if (ofconn_get_type(ofconn) == OFCONN_PRIMARY
 && ofconn_get_role(ofconn) == OFPCR12_ROLE_SLAVE) {
-return OFPERR_OFPBRC_EPERM;
+return OFPERR_OFPBRC_IS_SLAVE;
 } else {
 return 0;
 }
-- 
2.1.3

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


Re: [ovs-dev] [PATCH net-next v13 5/5] openvswitch: Add support for unique flow IDs.

2015-01-21 Thread Pravin Shelar
On Tue, Jan 20, 2015 at 10:32 AM, Joe Stringer  wrote:
> Previously, flows were manipulated by userspace specifying a full,
> unmasked flow key. This adds significant burden onto flow
> serialization/deserialization, particularly when dumping flows.
>
> This patch adds an alternative way to refer to flows using a
> variable-length "unique flow identifier" (UFID). At flow setup time,
> userspace may specify a UFID for a flow, which is stored with the flow
> and inserted into a separate table for lookup, in addition to the
> standard flow table. Flows created using a UFID must be fetched or
> deleted using the UFID.
>
> All flow dump operations may now be made more terse with OVS_UFID_F_*
> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
> omit the flow key from a datapath operation if the flow has a
> corresponding UFID. This significantly reduces the time spent assembling
> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
> enabled, the datapath only returns the UFID and statistics for each flow
> during flow dump, increasing ovs-vswitchd revalidator performance by 40%
> or more.
>
> Signed-off-by: Joe Stringer 

Its almost ready. But I saw minor issues,
few checkpatch.pl failures.
in ovs_flow_cmd_new() we should use unmasked key to lookup in flow
table for legacy case.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net-next v13 5/5] openvswitch: Add support for unique flow IDs.

2015-01-21 Thread Joe Stringer
On 21 January 2015 at 10:31, Pravin Shelar  wrote:
> On Tue, Jan 20, 2015 at 10:32 AM, Joe Stringer  wrote:
>> Previously, flows were manipulated by userspace specifying a full,
>> unmasked flow key. This adds significant burden onto flow
>> serialization/deserialization, particularly when dumping flows.
>>
>> This patch adds an alternative way to refer to flows using a
>> variable-length "unique flow identifier" (UFID). At flow setup time,
>> userspace may specify a UFID for a flow, which is stored with the flow
>> and inserted into a separate table for lookup, in addition to the
>> standard flow table. Flows created using a UFID must be fetched or
>> deleted using the UFID.
>>
>> All flow dump operations may now be made more terse with OVS_UFID_F_*
>> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
>> omit the flow key from a datapath operation if the flow has a
>> corresponding UFID. This significantly reduces the time spent assembling
>> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
>> enabled, the datapath only returns the UFID and statistics for each flow
>> during flow dump, increasing ovs-vswitchd revalidator performance by 40%
>> or more.
>>
>> Signed-off-by: Joe Stringer 
>
> Its almost ready. But I saw minor issues,
> few checkpatch.pl failures.
> in ovs_flow_cmd_new() we should use unmasked key to lookup in flow
> table for legacy case.

Thanks for review, I can send out a fresh version soon. Should I
resend the whole series or is just a new version of this patch
sufficient?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Bug#768095: openvswitch-datapath-dkms fails to build on Debian 7.7 3.2.0-4-amd64 (3.2.63-2+deb7u1)

2015-01-21 Thread Thomas Goirand
On 01/17/2015 08:10 PM, Chris wrote:
> Hi,
> 
> Thanks to Jonathan who did approve the patch by the release team but I'm a 
> little bit worried : Debian 7.8 is out and still no news ...
> Openvswitch kernel module is still not working in Debian stable :(
> 
> I suspect that you are quite busy and it's of course a voluntary work.
> 
> Thank you in advance.
> 
> Regards,

Hi Chris,

openvswitch has been uploaded (I sponsored the upload), and it's now in
stable-proposed-update. It will stay there, until the next point
release. It's a shame btw that packages.debian.org doesn't show what's
in proposed-updates, but I can tell you that
1.4.2+git20120612-9.1~deb7u1.1 is in (I received the ACK email).

So if you want to have it *now*, just add the wheezy-proposed-update
repository to your sources.list (IMO, not enough people use it...).

Cheers,

Thomas Goirand (zigo)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] datapath-windows: Support for multiple VXLAN tunnels

2015-01-21 Thread Sorin Vinturis
At the moment the OVS extension supports only one VXLAN tunnel that
is cached in the extension switch context. Replaced the latter
cached pointer with an array list that contains all VXLAN tunnel
vports.

Signed-off-by: Sorin Vinturis 
Reported-by: Alin Gabriel Serdean 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/64
---
 datapath-windows/ovsext/Actions.c |  8 +++--
 datapath-windows/ovsext/Switch.c  | 16 ++
 datapath-windows/ovsext/Switch.h  |  2 +-
 datapath-windows/ovsext/Tunnel.c  |  3 +-
 datapath-windows/ovsext/Vport.c   | 63 ++-
 datapath-windows/ovsext/Vport.h   | 14 +
 datapath-windows/ovsext/Vxlan.c   |  2 +-
 datapath-windows/ovsext/Vxlan.h   |  4 +--
 8 files changed, 71 insertions(+), 41 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index a93fe03..20d2f1e 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -203,9 +203,10 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
  * packets only if they are at least VXLAN header size.
  */
 if (!flowKey->ipKey.nwFrag &&
-flowKey->ipKey.nwProto == IPPROTO_UDP &&
-flowKey->ipKey.l4.tpDst == VXLAN_UDP_PORT_NBO) {
-tunnelVport = ovsFwdCtx->switchContext->vxlanVport;
+flowKey->ipKey.nwProto == IPPROTO_UDP) {
+UINT16 dstPort = htons(flowKey->ipKey.l4.tpDst);
+tunnelVport = OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
+  dstPort);
 ovsActionStats.rxVxlan++;
 }
 
@@ -1318,6 +1319,7 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
status = OvsTunnelAttrToIPv4TunnelKey((PNL_ATTR)a, &tunKey);
 ASSERT(status == NDIS_STATUS_SUCCESS);
 tunKey.flow_hash = (uint16)(hash ? *hash : OvsHashFlow(key));
+tunKey.dst_port = key->ipKey.l4.tpDst;
 RtlCopyMemory(&ovsFwdCtx->tunKey, &tunKey, sizeof ovsFwdCtx->tunKey);
 
 break;
diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index cf5e3c4..665aff4 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -366,6 +366,8 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 OvsAllocateMemory(sizeof (LIST_ENTRY) * OVS_MAX_VPORT_ARRAY_SIZE);
 switchContext->pidHashArray = (PLIST_ENTRY)
 OvsAllocateMemory(sizeof(LIST_ENTRY) * OVS_MAX_PID_ARRAY_SIZE);
+switchContext->tunnelVportsArray = (PLIST_ENTRY)
+OvsAllocateMemory(sizeof(LIST_ENTRY) * OVS_MAX_VPORT_ARRAY_SIZE);
 status = OvsAllocateFlowTable(&switchContext->datapath, switchContext);
 
 if (status == NDIS_STATUS_SUCCESS) {
@@ -376,7 +378,8 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 switchContext->portNoHashArray == NULL ||
 switchContext->ovsPortNameHashArray == NULL ||
 switchContext->portIdHashArray== NULL ||
-switchContext->pidHashArray == NULL) {
+switchContext->pidHashArray == NULL ||
+switchContext->tunnelVportsArray == NULL) {
 if (switchContext->dispatchLock) {
 NdisFreeRWLock(switchContext->dispatchLock);
 }
@@ -394,6 +397,10 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 OvsFreeMemory(switchContext->pidHashArray);
 }
 
+if (switchContext->tunnelVportsArray) {
+OvsFreeMemory(switchContext->tunnelVportsArray);
+}
+
 OvsDeleteFlowTable(&switchContext->datapath);
 OvsCleanupBufferPool(switchContext);
 
@@ -403,12 +410,9 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 
 for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
 InitializeListHead(&switchContext->ovsPortNameHashArray[i]);
-}
-for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
 InitializeListHead(&switchContext->portIdHashArray[i]);
-}
-for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
 InitializeListHead(&switchContext->portNoHashArray[i]);
+InitializeListHead(&switchContext->tunnelVportsArray[i]);
 }
 
 for (i = 0; i < OVS_MAX_PID_ARRAY_SIZE; i++) {
@@ -445,6 +449,8 @@ OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
 switchContext->portNoHashArray = NULL;
 OvsFreeMemory(switchContext->pidHashArray);
 switchContext->pidHashArray = NULL;
+OvsFreeMemory(switchContext->tunnelVportsArray);
+switchContext->tunnelVportsArray = NULL;
 OvsDeleteFlowTable(&switchContext->datapath);
 OvsCleanupBufferPool(switchContext);
 OVS_LOG_TRACE("Exit: Delete switchContext: %p", switchContext);
diff --git a/datapath-windows/ovsext/Switch.h b/datapath-windows/ovsext/Switch.h
index 7960072..f2558e5 100644
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -132,7 +132,7 @@ typedef struct _OVS_SWITCH_CONTEXT
 POVS_VPORT_ENTRYvirtualExternalVport;   // the virtual adapte

Re: [ovs-dev] [PATCH 1/2] ofp-errors: Explain the whole "expected duplications" thing.

2015-01-21 Thread Thomas Graf
On 01/21/15 at 09:46am, Ben Pfaff wrote:
>  enum ofperr {
>  /* Expected duplications. */
>  
>  /* Expected: 0x0,3,5 in OF1.1 means both OFPBIC_BAD_EXPERIMENTER and
>   * OFPBIC_BAD_EXP_TYPE. */
>  
> +/* Expected: 0x0,1,5 in OF1.0 means both OFPBRC_EPERM and
> + * OFPBRC_IS_SLAVE. */
> +
> +/* Expected: 0x0,1,5 in OF1.1 means both OFPBRC_EPERM and
> + * OFPBRC_IS_SLAVE. */
> +
>  /* ## -- ## */
>  /* ## OFPET_HELLO_FAILED ## */
>  /* ## -- ## */

Did you intend to have this last change in patch 2?


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


[ovs-dev] Possible memory leak in bundle_destroy?

2015-01-21 Thread Sabyasachi Sengupta


Hi,

I saw that bundle_destroy calls mbridge_unregister_bundle with bundle->aux. 
However the second parameter of mbridge_unregister_bundle is ofbundle. When 
creating the bundle in mbridge_bundle_register, we setup mbundle using 
ofbundle. Shouldn't the call to unregister be just ofbundle pointer instead? 
I'm wondering how would otherwise mbridge_unregister_bundle find the mirror 
bundle and clean it up? Is there a memory leak?


--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2343,7 +2343,11 @@ bundle_destroy(struct ofbundle *bundle)
 }

 ofproto = bundle->ofproto;
-mbridge_unregister_bundle(ofproto->mbridge, bundle->aux);
+mbridge_unregister_bundle(ofproto->mbridge, bundle);

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


Re: [ovs-dev] [PATCH net-next v13 5/5] openvswitch: Add support for unique flow IDs.

2015-01-21 Thread Pravin Shelar
On Wed, Jan 21, 2015 at 11:29 AM, Joe Stringer  wrote:
> On 21 January 2015 at 10:31, Pravin Shelar  wrote:
>> On Tue, Jan 20, 2015 at 10:32 AM, Joe Stringer  
>> wrote:
>>> Previously, flows were manipulated by userspace specifying a full,
>>> unmasked flow key. This adds significant burden onto flow
>>> serialization/deserialization, particularly when dumping flows.
>>>
>>> This patch adds an alternative way to refer to flows using a
>>> variable-length "unique flow identifier" (UFID). At flow setup time,
>>> userspace may specify a UFID for a flow, which is stored with the flow
>>> and inserted into a separate table for lookup, in addition to the
>>> standard flow table. Flows created using a UFID must be fetched or
>>> deleted using the UFID.
>>>
>>> All flow dump operations may now be made more terse with OVS_UFID_F_*
>>> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
>>> omit the flow key from a datapath operation if the flow has a
>>> corresponding UFID. This significantly reduces the time spent assembling
>>> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
>>> enabled, the datapath only returns the UFID and statistics for each flow
>>> during flow dump, increasing ovs-vswitchd revalidator performance by 40%
>>> or more.
>>>
>>> Signed-off-by: Joe Stringer 
>>
>> Its almost ready. But I saw minor issues,
>> few checkpatch.pl failures.
>> in ovs_flow_cmd_new() we should use unmasked key to lookup in flow
>> table for legacy case.
>
> Thanks for review, I can send out a fresh version soon. Should I
> resend the whole series or is just a new version of this patch
> sufficient?

I think you have to send entire series. Dave will not apply series
patches from different versions.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH net-next v14 0/5] openvswitch: Introduce 128-bit unique flow identifiers.

2015-01-21 Thread Joe Stringer
This series extends the openvswitch datapath interface for flow commands to use
128-bit unique identifiers as an alternative to the netlink-formatted flow key.
This significantly reduces the cost of assembling messages between the kernel
and userspace, in particular improving Open vSwitch revalidation performance by
40% or more.

v14:
- Perform lookup using unmasked key in legacy case.
- Fix minor checkpatch.pl style violations.

v13:
- Embed sw_flow_id in sw_flow to save memory allocation in UFID case.
- Malloc unmasked key for id in non-UFID case.
- Fix bug where non-UFID case could double-serialize keys.

v12:
- Userspace patches fully merged into Open vSwitch master
- New minor refactor patches (2,3,4)
- Merge unmasked_key, ufid representation of flow identifier in sw_flow
- Improve memory allocation sizes when serializing ufid
- Handle corner case where a flow_new is requested with a flow that has an
  identical ufid as an existing flow, but a different flow key
- Limit UFID to between 1-16 octets inclusive.
- Add various helper functions to improve readibility

v11:
- Pushed most of the prerequisite patches for this series to OVS master.
- Split out openvswitch.h interface changes from datapath implementation
- Datapath implementation to be reviewed on net-next, separately

v10:
- New patch allowing datapath to serialize masked keys
- Simplify datapath interface by accepting UFID or flow_key, but not both
- Flows set up with UFID must be queried/deleted using UFID
- Reduce sw_flow memory usage for UFID
- Don't periodically rehash UFID table in linux datapath
- Remove kernel_only UFID in linux datapath

v9:
- No kernel changes

v8:
- Rename UID -> UFID
- Fix null dereference in datapath when paired with older userspace
- All patches are reviewed/acked except datapath changes.

v7:
- Remove OVS_DP_F_INDEX_BY_UID
- Rework datapath UID serialization for variable length UIDs

v6:
- Reduce netlink conversions for all datapaths
- Various bugfixes

v5:
- Various bugfixes
- Improve logging

v4:
- Datapath memory leak fixes
- Enable UID-based terse dumping and deleting by default
- Various fixes

RFCv3:
- Add datapath implementation

Joe Stringer (5):
  openvswitch: Refactor ovs_nla_fill_match().
  openvswitch: Refactor ovs_flow_tbl_insert().
  openvswitch: Use sw_flow_key_range for key ranges.
  genetlink: Add genlmsg_parse() helper function.
  openvswitch: Add support for unique flow IDs.

 Documentation/networking/openvswitch.txt |   13 ++
 include/net/genetlink.h  |   17 +++
 include/uapi/linux/openvswitch.h |   20 +++
 net/openvswitch/datapath.c   |  226 --
 net/openvswitch/flow.h   |   28 +++-
 net/openvswitch/flow_netlink.c   |  102 +-
 net/openvswitch/flow_netlink.h   |   13 +-
 net/openvswitch/flow_table.c |  226 +++---
 net/openvswitch/flow_table.h |8 +-
 9 files changed, 519 insertions(+), 134 deletions(-)

-- 
1.7.10.4

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


[ovs-dev] [PATCH net-next v14 1/5] openvswitch: Refactor ovs_nla_fill_match().

2015-01-21 Thread Joe Stringer
Refactor the ovs_nla_fill_match() function into separate netlink
serialization functions ovs_nla_put_{unmasked_key,mask}(). Modify
ovs_nla_put_flow() to handle attribute nesting and expose the 'is_mask'
parameter - all callers need to nest the flow, and callers have better
knowledge about whether it is serializing a mask or not.

Signed-off-by: Joe Stringer 
---
 net/openvswitch/datapath.c |   41 ++--
 net/openvswitch/flow_netlink.c |   38 ++---
 net/openvswitch/flow_netlink.h |7 +--
 3 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index f45f1bf..257b975 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -461,10 +461,8 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
 0, upcall_info->cmd);
upcall->dp_ifindex = dp_ifindex;
 
-   nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_KEY);
-   err = ovs_nla_put_flow(key, key, user_skb);
+   err = ovs_nla_put_key(key, key, OVS_PACKET_ATTR_KEY, false, user_skb);
BUG_ON(err);
-   nla_nest_end(user_skb, nla);
 
if (upcall_info->userdata)
__nla_put(user_skb, OVS_PACKET_ATTR_USERDATA,
@@ -676,37 +674,6 @@ static size_t ovs_flow_cmd_msg_size(const struct 
sw_flow_actions *acts)
 }
 
 /* Called with ovs_mutex or RCU read lock. */
-static int ovs_flow_cmd_fill_match(const struct sw_flow *flow,
-  struct sk_buff *skb)
-{
-   struct nlattr *nla;
-   int err;
-
-   /* Fill flow key. */
-   nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
-   if (!nla)
-   return -EMSGSIZE;
-
-   err = ovs_nla_put_flow(&flow->unmasked_key, &flow->unmasked_key, skb);
-   if (err)
-   return err;
-
-   nla_nest_end(skb, nla);
-
-   /* Fill flow mask. */
-   nla = nla_nest_start(skb, OVS_FLOW_ATTR_MASK);
-   if (!nla)
-   return -EMSGSIZE;
-
-   err = ovs_nla_put_flow(&flow->key, &flow->mask->key, skb);
-   if (err)
-   return err;
-
-   nla_nest_end(skb, nla);
-   return 0;
-}
-
-/* Called with ovs_mutex or RCU read lock. */
 static int ovs_flow_cmd_fill_stats(const struct sw_flow *flow,
   struct sk_buff *skb)
 {
@@ -787,7 +754,11 @@ static int ovs_flow_cmd_fill_info(const struct sw_flow 
*flow, int dp_ifindex,
 
ovs_header->dp_ifindex = dp_ifindex;
 
-   err = ovs_flow_cmd_fill_match(flow, skb);
+   err = ovs_nla_put_unmasked_key(flow, skb);
+   if (err)
+   goto error;
+
+   err = ovs_nla_put_mask(flow, skb);
if (err)
goto error;
 
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index d210d1b..33751f8 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -1216,12 +1216,12 @@ int ovs_nla_get_flow_metadata(const struct nlattr *attr,
return metadata_from_nlattrs(&match, &attrs, a, false, log);
 }
 
-int ovs_nla_put_flow(const struct sw_flow_key *swkey,
-const struct sw_flow_key *output, struct sk_buff *skb)
+static int __ovs_nla_put_key(const struct sw_flow_key *swkey,
+const struct sw_flow_key *output, bool is_mask,
+struct sk_buff *skb)
 {
struct ovs_key_ethernet *eth_key;
struct nlattr *nla, *encap;
-   bool is_mask = (swkey != output);
 
if (nla_put_u32(skb, OVS_KEY_ATTR_RECIRC_ID, output->recirc_id))
goto nla_put_failure;
@@ -1431,6 +1431,38 @@ nla_put_failure:
return -EMSGSIZE;
 }
 
+int ovs_nla_put_key(const struct sw_flow_key *swkey,
+   const struct sw_flow_key *output, int attr, bool is_mask,
+   struct sk_buff *skb)
+{
+   int err;
+   struct nlattr *nla;
+
+   nla = nla_nest_start(skb, attr);
+   if (!nla)
+   return -EMSGSIZE;
+   err = __ovs_nla_put_key(swkey, output, is_mask, skb);
+   if (err)
+   return err;
+   nla_nest_end(skb, nla);
+
+   return 0;
+}
+
+/* Called with ovs_mutex or RCU read lock. */
+int ovs_nla_put_unmasked_key(const struct sw_flow *flow, struct sk_buff *skb)
+{
+   return ovs_nla_put_key(&flow->unmasked_key, &flow->unmasked_key,
+   OVS_FLOW_ATTR_KEY, false, skb);
+}
+
+/* Called with ovs_mutex or RCU read lock. */
+int ovs_nla_put_mask(const struct sw_flow *flow, struct sk_buff *skb)
+{
+   return ovs_nla_put_key(&flow->key, &flow->mask->key,
+   OVS_FLOW_ATTR_MASK, true, skb);
+}
+
 #define MAX_ACTIONS_BUFSIZE(32 * 1024)
 
 static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
diff --git a/net/openvswitch/flow_netlink.h b/net/openvswitch/flow_netlink.h
index 577f12b..9ed09e6 100644

[ovs-dev] [PATCH net-next v14 2/5] openvswitch: Refactor ovs_flow_tbl_insert().

2015-01-21 Thread Joe Stringer
Rework so that ovs_flow_tbl_insert() calls flow_{key,mask}_insert().
This tidies up a future patch.

Signed-off-by: Joe Stringer 
---
 net/openvswitch/flow_table.c |   21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 5899bf1..81b977d 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -585,16 +585,10 @@ static int flow_mask_insert(struct flow_table *tbl, 
struct sw_flow *flow,
 }
 
 /* Must be called with OVS mutex held. */
-int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
-   const struct sw_flow_mask *mask)
+static void flow_key_insert(struct flow_table *table, struct sw_flow *flow)
 {
struct table_instance *new_ti = NULL;
struct table_instance *ti;
-   int err;
-
-   err = flow_mask_insert(table, flow, mask);
-   if (err)
-   return err;
 
flow->hash = flow_hash(&flow->key, flow->mask->range.start,
flow->mask->range.end);
@@ -613,6 +607,19 @@ int ovs_flow_tbl_insert(struct flow_table *table, struct 
sw_flow *flow,
table_instance_destroy(ti, true);
table->last_rehash = jiffies;
}
+}
+
+/* Must be called with OVS mutex held. */
+int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
+   const struct sw_flow_mask *mask)
+{
+   int err;
+
+   err = flow_mask_insert(table, flow, mask);
+   if (err)
+   return err;
+   flow_key_insert(table, flow);
+
return 0;
 }
 
-- 
1.7.10.4

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


[ovs-dev] [PATCH net-next v14 4/5] genetlink: Add genlmsg_parse() helper function.

2015-01-21 Thread Joe Stringer
The first user will be the next patch.

Signed-off-by: Joe Stringer 
---
 include/net/genetlink.h |   17 +
 1 file changed, 17 insertions(+)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index f24aa83..d5a9a8b 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -206,6 +206,23 @@ static inline struct nlmsghdr *genlmsg_nlhdr(void 
*user_hdr,
 }
 
 /**
+ * genlmsg_parse - parse attributes of a genetlink message
+ * @nlh: netlink message header
+ * @family: genetlink message family
+ * @tb: destination array with maxtype+1 elements
+ * @maxtype: maximum attribute type to be expected
+ * @policy: validation policy
+ * */
+static inline int genlmsg_parse(const struct nlmsghdr *nlh,
+   const struct genl_family *family,
+   struct nlattr *tb[], int maxtype,
+   const struct nla_policy *policy)
+{
+   return nlmsg_parse(nlh, family->hdrsize + GENL_HDRLEN, tb, maxtype,
+  policy);
+}
+
+/**
  * genl_dump_check_consistent - check if sequence is consistent and advertise 
if not
  * @cb: netlink callback structure that stores the sequence number
  * @user_hdr: user header as returned from genlmsg_put()
-- 
1.7.10.4

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


[ovs-dev] [PATCH net-next v14 5/5] openvswitch: Add support for unique flow IDs.

2015-01-21 Thread Joe Stringer
Previously, flows were manipulated by userspace specifying a full,
unmasked flow key. This adds significant burden onto flow
serialization/deserialization, particularly when dumping flows.

This patch adds an alternative way to refer to flows using a
variable-length "unique flow identifier" (UFID). At flow setup time,
userspace may specify a UFID for a flow, which is stored with the flow
and inserted into a separate table for lookup, in addition to the
standard flow table. Flows created using a UFID must be fetched or
deleted using the UFID.

All flow dump operations may now be made more terse with OVS_UFID_F_*
flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
omit the flow key from a datapath operation if the flow has a
corresponding UFID. This significantly reduces the time spent assembling
and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
enabled, the datapath only returns the UFID and statistics for each flow
during flow dump, increasing ovs-vswitchd revalidator performance by 40%
or more.

Signed-off-by: Joe Stringer 
---
v14: Perform lookup using unmasked key in legacy case.
 Fix minor checkpatch.pl style violations.
v13: Embed sw_flow_id in sw_flow.
 Malloc unmasked_key for legacy case.
 Fix bug where key is double-serialized in legacy case.
v12: Merge unmasked_key, ufid into 'struct sw_flow_id'.
 Add identifier_is_{ufid,key}() helpers.
 Add should_fill_{key,mask,actions}() helpers.
 Revert rework of ovs_flow_cmd_set() -- requires key,acts.
 Streamline sw_flow_id copy/alloc.
 Use genlmsg_parse() to find ufid flags in dump.
 Handle new_flow case with same ufid, different flow key.
 Calculate exact message length in ovs_flow_cmd_msg_size().
 Limit UFID to between 1-16 octets.
 Check minimum length for UFID in flow_policy.
 Use jhash() for ufid_hash (arch_fast_hash() went away).
 Rebase.
v11: Separate UFID and unmasked key from sw_flow.
 Modify interface to remove nested UFID attributes.
 Only allow UFIDs between 1-256 octets.
 Move UFID nla fetch helpers to flow_netlink.h.
 Perform complete nlmsg_parsing in ovs_flow_cmd_dump().
 Check UFID table for flows with duplicate UFID at flow setup.
 Tidy up mask/key/ufid insertion into flow_table.
 Rebase.
v10: Ignore flow_key in requests if UFID is specified.
 Only allow UFID flows to be indexed by UFID.
 Only allow non-UFID flows to be indexed by unmasked flow key.
 Unite the unmasked_key and ufid+ufid_hash in 'struct sw_flow'.
 Don't periodically rehash the UFID table.
 Resize the UFID table independently from the flow table.
 Modify table_destroy() to iterate once and delete from both tables.
 Fix UFID memory leak in flow_free().
 Remove kernel-only UFIDs for non-UFID cases.
 Rename "OVS_UFID_F_SKIP_*" -> "OVS_UFID_F_OMIT_*"
 Update documentation.
 Rebase.
v9: No change.
v8: Rename UID -> UFID "unique flow identifier".
Fix null dereference when adding flow without uid or mask.
If UFID and not match are specified, and lookup fails, return ENOENT.
Rebase.
v7: Remove OVS_DP_F_INDEX_BY_UID.
Rework UID serialisation for variable-length UID.
Log error if uid not specified and OVS_UID_F_SKIP_KEY is set.
Rebase against "probe" logging changes.
v6: Fix documentation for supporting UIDs between 32-128 bits.
Minor style fixes.
Rebase.
v5: No change.
v4: Fix memory leaks.
Log when triggering the older userspace issue above.
v3: Initial post.
---
 Documentation/networking/openvswitch.txt |   13 ++
 include/uapi/linux/openvswitch.h |   20 +++
 net/openvswitch/datapath.c   |  207 ++
 net/openvswitch/flow.h   |   28 +++-
 net/openvswitch/flow_netlink.c   |   68 +-
 net/openvswitch/flow_netlink.h   |8 +-
 net/openvswitch/flow_table.c |  187 ++-
 net/openvswitch/flow_table.h |8 +-
 8 files changed, 448 insertions(+), 91 deletions(-)

diff --git a/Documentation/networking/openvswitch.txt 
b/Documentation/networking/openvswitch.txt
index 37c20ee..b3b9ac6 100644
--- a/Documentation/networking/openvswitch.txt
+++ b/Documentation/networking/openvswitch.txt
@@ -131,6 +131,19 @@ performs best-effort detection of overlapping wildcarded 
flows and may reject
 some but not all of them. However, this behavior may change in future versions.
 
 
+Unique flow identifiers
+---
+
+An alternative to using the original match portion of a key as the handle for
+flow identification is a unique flow identifier, or "UFID". UFIDs are optional
+for both the kernel and user space program.
+
+User space programs that support UFID are expected to provide it during flow
+setup in addition to the flow, then refer to the flow using the UFID for all
+future operations. The kernel is not required to index flows by the original
+flow key if a UFID i

[ovs-dev] [PATCH net-next v14 3/5] openvswitch: Use sw_flow_key_range for key ranges.

2015-01-21 Thread Joe Stringer
These minor tidyups make a future patch a little tidier.

Signed-off-by: Joe Stringer 
---
 net/openvswitch/flow_table.c |   20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 81b977d..9a3f41f 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -357,9 +357,11 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table)
return 0;
 }
 
-static u32 flow_hash(const struct sw_flow_key *key, int key_start,
-int key_end)
+static u32 flow_hash(const struct sw_flow_key *key,
+const struct sw_flow_key_range *range)
 {
+   int key_start = range->start;
+   int key_end = range->end;
const u32 *hash_key = (const u32 *)((const u8 *)key + key_start);
int hash_u32s = (key_end - key_start) >> 2;
 
@@ -395,9 +397,9 @@ static bool cmp_key(const struct sw_flow_key *key1,
 
 static bool flow_cmp_masked_key(const struct sw_flow *flow,
const struct sw_flow_key *key,
-   int key_start, int key_end)
+   const struct sw_flow_key_range *range)
 {
-   return cmp_key(&flow->key, key, key_start, key_end);
+   return cmp_key(&flow->key, key, range->start, range->end);
 }
 
 bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
@@ -416,18 +418,15 @@ static struct sw_flow *masked_flow_lookup(struct 
table_instance *ti,
 {
struct sw_flow *flow;
struct hlist_head *head;
-   int key_start = mask->range.start;
-   int key_end = mask->range.end;
u32 hash;
struct sw_flow_key masked_key;
 
ovs_flow_mask_key(&masked_key, unmasked, mask);
-   hash = flow_hash(&masked_key, key_start, key_end);
+   hash = flow_hash(&masked_key, &mask->range);
head = find_bucket(ti, hash);
hlist_for_each_entry_rcu(flow, head, hash_node[ti->node_ver]) {
if (flow->mask == mask && flow->hash == hash &&
-   flow_cmp_masked_key(flow, &masked_key,
- key_start, key_end))
+   flow_cmp_masked_key(flow, &masked_key, &mask->range))
return flow;
}
return NULL;
@@ -590,8 +589,7 @@ static void flow_key_insert(struct flow_table *table, 
struct sw_flow *flow)
struct table_instance *new_ti = NULL;
struct table_instance *ti;
 
-   flow->hash = flow_hash(&flow->key, flow->mask->range.start,
-   flow->mask->range.end);
+   flow->hash = flow_hash(&flow->key, &flow->mask->range);
ti = ovsl_dereference(table->ti);
table_instance_insert(ti, flow);
table->count++;
-- 
1.7.10.4

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


Re: [ovs-dev] [PATCH net-next v13 5/5] openvswitch: Add support for unique flow IDs.

2015-01-21 Thread Joe Stringer
On 21 January 2015 at 15:41, Pravin Shelar  wrote:
> On Wed, Jan 21, 2015 at 11:29 AM, Joe Stringer  wrote:
>> On 21 January 2015 at 10:31, Pravin Shelar  wrote:
>>> On Tue, Jan 20, 2015 at 10:32 AM, Joe Stringer  
>>> wrote:
 Previously, flows were manipulated by userspace specifying a full,
 unmasked flow key. This adds significant burden onto flow
 serialization/deserialization, particularly when dumping flows.

 This patch adds an alternative way to refer to flows using a
 variable-length "unique flow identifier" (UFID). At flow setup time,
 userspace may specify a UFID for a flow, which is stored with the flow
 and inserted into a separate table for lookup, in addition to the
 standard flow table. Flows created using a UFID must be fetched or
 deleted using the UFID.

 All flow dump operations may now be made more terse with OVS_UFID_F_*
 flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
 omit the flow key from a datapath operation if the flow has a
 corresponding UFID. This significantly reduces the time spent assembling
 and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
 enabled, the datapath only returns the UFID and statistics for each flow
 during flow dump, increasing ovs-vswitchd revalidator performance by 40%
 or more.

 Signed-off-by: Joe Stringer 
>>>
>>> Its almost ready. But I saw minor issues,
>>> few checkpatch.pl failures.
>>> in ovs_flow_cmd_new() we should use unmasked key to lookup in flow
>>> table for legacy case.
>>
>> Thanks for review, I can send out a fresh version soon. Should I
>> resend the whole series or is just a new version of this patch
>> sufficient?
>
> I think you have to send entire series. Dave will not apply series
> patches from different versions.

Thanks. I also realized there were checkpatch issues in the first
patch, so I resent the whole series anyway.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net-next v14 2/5] openvswitch: Refactor ovs_flow_tbl_insert().

2015-01-21 Thread Pravin Shelar
On Wed, Jan 21, 2015 at 4:42 PM, Joe Stringer  wrote:
> Rework so that ovs_flow_tbl_insert() calls flow_{key,mask}_insert().
> This tidies up a future patch.
>
> Signed-off-by: Joe Stringer 
Acked-by: Pravin B Shelar 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH net-next v14 3/5] openvswitch: Use sw_flow_key_range for key ranges.

2015-01-21 Thread Pravin Shelar
On Wed, Jan 21, 2015 at 4:42 PM, Joe Stringer  wrote:
> These minor tidyups make a future patch a little tidier.
>
> Signed-off-by: Joe Stringer 

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


Re: [ovs-dev] [PATCH net-next v14 4/5] genetlink: Add genlmsg_parse() helper function.

2015-01-21 Thread Pravin Shelar
On Wed, Jan 21, 2015 at 4:42 PM, Joe Stringer  wrote:
> The first user will be the next patch.
>
> Signed-off-by: Joe Stringer 

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


Re: [ovs-dev] [PATCH net-next v14 5/5] openvswitch: Add support for unique flow IDs.

2015-01-21 Thread Pravin Shelar
On Wed, Jan 21, 2015 at 4:42 PM, Joe Stringer  wrote:
> Previously, flows were manipulated by userspace specifying a full,
> unmasked flow key. This adds significant burden onto flow
> serialization/deserialization, particularly when dumping flows.
>
> This patch adds an alternative way to refer to flows using a
> variable-length "unique flow identifier" (UFID). At flow setup time,
> userspace may specify a UFID for a flow, which is stored with the flow
> and inserted into a separate table for lookup, in addition to the
> standard flow table. Flows created using a UFID must be fetched or
> deleted using the UFID.
>
> All flow dump operations may now be made more terse with OVS_UFID_F_*
> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
> omit the flow key from a datapath operation if the flow has a
> corresponding UFID. This significantly reduces the time spent assembling
> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
> enabled, the datapath only returns the UFID and statistics for each flow
> during flow dump, increasing ovs-vswitchd revalidator performance by 40%
> or more.
>
> Signed-off-by: Joe Stringer 

Looks good.
Acked-by: Pravin B Shelar 

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


Re: [ovs-dev] [PATCH net-next v14 1/5] openvswitch: Refactor ovs_nla_fill_match().

2015-01-21 Thread Pravin Shelar
On Wed, Jan 21, 2015 at 4:42 PM, Joe Stringer  wrote:
> Refactor the ovs_nla_fill_match() function into separate netlink
> serialization functions ovs_nla_put_{unmasked_key,mask}(). Modify
> ovs_nla_put_flow() to handle attribute nesting and expose the 'is_mask'
> parameter - all callers need to nest the flow, and callers have better
> knowledge about whether it is serializing a mask or not.
>
> Signed-off-by: Joe Stringer 

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


Re: [ovs-dev] [PATCH 1/2] ofp-errors: Explain the whole "expected duplications" thing.

2015-01-21 Thread Ben Pfaff
On Thu, Jan 22, 2015 at 12:21:20AM +0100, Thomas Graf wrote:
> On 01/21/15 at 09:46am, Ben Pfaff wrote:
> >  enum ofperr {
> >  /* Expected duplications. */
> >  
> >  /* Expected: 0x0,3,5 in OF1.1 means both OFPBIC_BAD_EXPERIMENTER and
> >   * OFPBIC_BAD_EXP_TYPE. */
> >  
> > +/* Expected: 0x0,1,5 in OF1.0 means both OFPBRC_EPERM and
> > + * OFPBRC_IS_SLAVE. */
> > +
> > +/* Expected: 0x0,1,5 in OF1.1 means both OFPBRC_EPERM and
> > + * OFPBRC_IS_SLAVE. */
> > +
> >  /* ## -- ## */
> >  /* ## OFPET_HELLO_FAILED ## */
> >  /* ## -- ## */
> 
> Did you intend to have this last change in patch 2?

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


Re: [ovs-dev] Possible memory leak in bundle_destroy?

2015-01-21 Thread Ben Pfaff
On Wed, Jan 21, 2015 at 03:38:48PM -0800, Sabyasachi Sengupta wrote:
> I saw that bundle_destroy calls mbridge_unregister_bundle with bundle->aux.
> However the second parameter of mbridge_unregister_bundle is ofbundle. When
> creating the bundle in mbridge_bundle_register, we setup mbundle using
> ofbundle. Shouldn't the call to unregister be just ofbundle pointer instead?
> I'm wondering how would otherwise mbridge_unregister_bundle find the mirror
> bundle and clean it up? Is there a memory leak?
> 
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2343,7 +2343,11 @@ bundle_destroy(struct ofbundle *bundle)
>  }
> 
>  ofproto = bundle->ofproto;
> -mbridge_unregister_bundle(ofproto->mbridge, bundle->aux);
> +mbridge_unregister_bundle(ofproto->mbridge, bundle);

I agree.  I pushed a fix.  Thanks a lot!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Regarding permission for usage of OpenVSwitch reference.

2015-01-21 Thread Shashwat Srivastava
Hi Team,

I am writing a whitepaper and wanted your permission to use the reference 
of Open VSwitch in our whitepaper.
Is this the right place to seek permission or please direct me where to 
send the mail for the same ?


Regards
Shashwat Srivastava
=-=-=
Notice: The information contained in this e-mail
message and/or attachments to it may contain 
confidential or privileged information. If you are 
not the intended recipient, any dissemination, use, 
review, distribution, printing or copying of the 
information contained in this e-mail message 
and/or attachments to it are strictly prohibited. If 
you have received this communication in error, 
please notify us by reply e-mail or telephone and 
immediately and permanently delete the message 
and any attachments. Thank you


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