[ovs-dev] ovs-ofctl interoperability
Hi, I'd like to know, can I use old ovs-ofctl(wdp codebase) with new OVS(ovs 2.3.0)? And do I need to advertise the OF variant(1.0 or 1.3) to OVS? If yes then how? Regards, Abhijit ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Returned mail: see transcript for details
The original message was received at Fri, 5 Jun 2015 16:32:09 +0700 from whos.amung.us [218.30.188.166] - The following addresses had permanent fatal errors - ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v6] This commit adds the windows installer to the OVS tree.
Sorry but unfortunately I could not get the patch on the ML. Created the following pull request instead: https://github.com/openvswitch/ovs/pull/49 . Thank you, Alin. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-dpdk: Do not flush tx queue which is shared among CPUs since it is always flushed
>
> When tx queue is shared among CPUS,the pkts always be flush in
> 'netdev_dpdk_eth_send'
> So it is unnecessarily for flushing in netdev_dpdk_rxq_recv Otherwise tx will
> be accessed without locking
Are you seeing a specific bug or is this just to account for a device with
less queues than pmds?
>
> Signed-off-by: Wei li
> ---
> lib/netdev-dpdk.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 63243d8..25e3a73
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -892,8 +892,11 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,
> struct dp_packet **packets,
> int nb_rx;
>
> /* There is only one tx queue for this core. Do not flush other
> - * queueus. */
> -if (rxq_->queue_id == rte_lcore_id()) {
> + * queueus.
s/queueus/queues
> + * Do not flush tx queue which is shared among CPUs
> + * since it is always flushed */
> +if (rxq_->queue_id == rte_lcore_id() &&
> + OVS_LIKELY(!dev->txq_needs_locking)) {
> dpdk_queue_flush(dev, rxq_->queue_id);
Do you see any drop in performance in a simple phy-phy case before
and after this patch?
> }
>
> --
> 1.9.5.msysgit.1
>
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6] This commit adds the windows installer to the OVS tree.
Alin, I do not see Alessandro's signed-off. Can you please add. On Fri, Jun 5, 2015 at 5:16 AM, Alin Serdean wrote: > Sorry but unfortunately I could not get the patch on the ML. > Created the following pull request instead: > https://github.com/openvswitch/ovs/pull/49 . > > Thank you, > Alin. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] ofproto-dpif: Avoid creating OpenFlow ports for duplicate tunnels.
Until now, when two tunnels had an identical configuration, both of them
were assigned OpenFlow ports, but only one of those OpenFlow ports was
functional. With this commit, only one of the two (or more) identically
configured tunnels will be assigned an OpenFlow port number.
Reported-by: Keith Holleman
Signed-off-by: Ben Pfaff
Co-authored-by: Andy Zhou
---
v1->v2: Fix reference counting and memory leak (from Andy Zhou).
AUTHORS| 1 +
ofproto/ofproto-dpif.c | 10 --
ofproto/tunnel.c | 14 ++
ofproto/tunnel.h | 6 +++---
4 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index f155105..28899a8 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -277,6 +277,7 @@ Joan Cirer j...@ev0.net
John Darrington j...@darrington.wattle.id.au
John Galgay j...@galgay.net
John Hurley john.hur...@netronome.com
+Keith Holleman hollemani...@gmail.com
K 華k940...@hotmail.com
Kevin Mancuso kevin.manc...@rackspace.com
Kiran Shanbhog ki...@vmware.com
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c1daa1d..bd45305 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1671,8 +1671,14 @@ port_construct(struct ofport *port_)
if (netdev_get_tunnel_config(netdev)) {
atomic_count_inc(&ofproto->backer->tnl_count);
-tnl_port_add(port, port->up.netdev, port->odp_port,
- ovs_native_tunneling_is_on(ofproto), namebuf);
+error = tnl_port_add(port, port->up.netdev, port->odp_port,
+ ovs_native_tunneling_is_on(ofproto), namebuf);
+if (error) {
+atomic_count_dec(&ofproto->backer->tnl_count);
+dpif_port_destroy(&dpif_port);
+return error;
+}
+
port->is_tunnel = true;
if (ofproto->ipfix) {
dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_, port->odp_port);
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 3ea0eb4..d2ac7c6 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2013, 2014 Nicira, Inc.
+/* Copyright (c) 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.
@@ -203,14 +203,20 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
struct netdev *netdev,
/* Adds 'ofport' to the module with datapath port number 'odp_port'. 'ofport's
* must be added before they can be used by the module. 'ofport' must be a
- * tunnel. */
-void
+ * tunnel.
+ *
+ * Returns 0 if successful, otherwise a positive errno value. */
+int
tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev,
odp_port_t odp_port, bool native_tnl, const char name[])
OVS_EXCLUDED(rwlock)
{
+bool ok;
+
fat_rwlock_wrlock(&rwlock);
-tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
+ok = tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
fat_rwlock_unlock(&rwlock);
+
+return ok ? 0 : EEXIST;
}
/* Checks if the tunnel represented by 'ofport' reconfiguration due to changes
diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
index 6181762..b8415ab 100644
--- a/ofproto/tunnel.h
+++ b/ofproto/tunnel.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2013 Nicira, Inc.
+/* Copyright (c) 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.
@@ -33,8 +33,8 @@ void ofproto_tunnel_init(void);
bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev *,
odp_port_t, bool native_tnl, const char name[]);
-void tnl_port_add(const struct ofport_dpif *, const struct netdev *,
- odp_port_t odp_port, bool native_tnl, const char name[]);
+int tnl_port_add(const struct ofport_dpif *, const struct netdev *,
+ odp_port_t odp_port, bool native_tnl, const char name[]);
void tnl_port_del(const struct ofport_dpif *);
const struct ofport_dpif *tnl_port_receive(const struct flow *);
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Avoid creating OpenFlow ports for duplicate tunnels.
Wow, thanks for figuring that out!
I think that these changes are OK.
I'll fold them into my patch and repost.
On Thu, Jun 04, 2015 at 02:13:51AM -0700, Andy Zhou wrote:
> By bumb luck, I noticed that 'ofproto->backer->tnl_count' will affect
> how ovs_native_tunneling_is_on() gives its answer. The following
> incremental diff seems to help 'tunnel_push_pop" test to pass. BTW, I
> am not suggesting this is the right fix, since it looks ugly.
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 431f739..44b5749 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1670,13 +1670,14 @@ port_construct(struct ofport *port_)
> port->odp_port = dpif_port.port_no;
>
> if (netdev_get_tunnel_config(netdev)) {
> +atomic_count_inc(&ofproto->backer->tnl_count);
> error = tnl_port_add(port, port->up.netdev, port->odp_port,
> ovs_native_tunneling_is_on(ofproto), namebuf);
> if (error) {
> +atomic_count_dec(&ofproto->backer->tnl_count);
> return error;
> }
>
> -atomic_count_inc(&ofproto->backer->tnl_count);
> port->is_tunnel = true;
> if (ofproto->ipfix) {
> dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_, port->odp_port);
>
>
> In addition, there may be a memory leak in case we bail out early.
>
> proto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 44b5749..bd45305 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1675,6 +1675,7 @@ port_construct(struct ofport *port_)
> ovs_native_tunneling_is_on(ofproto), namebuf);
> if (error) {
> atomic_count_dec(&ofproto->backer->tnl_count);
> +dpif_port_destroy(&dpif_port);
> return error;
> }
>
> The unit tests are still happy with both patches applied.
>
>
>
> On Wed, May 27, 2015 at 9:36 AM, Ben Pfaff wrote:
> > Oh, unfortunately it breaks test 629 "tunnel_push_pop - action". I
> > haven't had a chance to fully investigate yet.
> >
> > On Wed, May 27, 2015 at 09:33:07AM -0700, Ben Pfaff wrote:
> >> Wow, thanks!
> >>
> >> I added a Tested-by for you also, and I'll apply this to master in a
> >> minute.
> >>
> >> On Wed, May 27, 2015 at 09:17:23AM -0700, Alex Wang wrote:
> >> > Tested,
> >> >
> >> >
> >> > Acked-by: Alex Wang
> >> >
> >> > On Tue, May 26, 2015 at 6:48 PM, Ben Pfaff wrote:
> >> >
> >> > > Until now, when two tunnels had an identical configuration, both of
> >> > > them
> >> > > were assigned OpenFlow ports, but only one of those OpenFlow ports was
> >> > > functional. With this commit, only one of the two (or more)
> >> > > identically
> >> > > configured tunnels will be assigned an OpenFlow port number.
> >> > >
> >> > > Reported-by: Keith Holleman
> >> > > Signed-off-by: Ben Pfaff
> >> > > ---
> >> > > AUTHORS| 1 +
> >> > > ofproto/ofproto-dpif.c | 8 ++--
> >> > > ofproto/tunnel.c | 14 ++
> >> > > ofproto/tunnel.h | 6 +++---
> >> > > 4 files changed, 20 insertions(+), 9 deletions(-)
> >> > >
> >> > > diff --git a/AUTHORS b/AUTHORS
> >> > > index 5178b43..60b8cfa 100644
> >> > > --- a/AUTHORS
> >> > > +++ b/AUTHORS
> >> > > @@ -276,6 +276,7 @@ Joan Cirer j...@ev0.net
> >> > > John Darrington j...@darrington.wattle.id.au
> >> > > John Galgay j...@galgay.net
> >> > > John Hurley john.hur...@netronome.com
> >> > > +Keith Holleman hollemani...@gmail.com
> >> > > K 華k940...@hotmail.com
> >> > > Kevin Mancuso kevin.manc...@rackspace.com
> >> > > Kiran Shanbhog ki...@vmware.com
> >> > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >> > > index d151bb7..532fc4a 100644
> >> > > --- a/ofproto/ofproto-dpif.c
> >> > > +++ b/ofproto/ofproto-dpif.c
> >> > > @@ -1686,9 +1686,13 @@ port_construct(struct ofport *port_)
> >> > > port->odp_port = dpif_port.port_no;
> >> > >
> >> > > if (netdev_get_tunnel_config(netdev)) {
> >> > > +error = tnl_port_add(port, port->up.netdev, port->odp_port,
> >> > > + ovs_native_tunneling_is_on(ofproto),
> >> > > namebuf);
> >> > > +if (error) {
> >> > > +return error;
> >> > > +}
> >> > > +
> >> > > atomic_count_inc(&ofproto->backer->tnl_count);
> >> > > -tnl_port_add(port, port->up.netdev, port->odp_port,
> >> > > - ovs_native_tunneling_is_on(ofproto), namebuf);
> >> > > port->is_tunnel = true;
> >> > > if (ofproto->ipfix) {
> >> > > dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_,
> >> > > port->odp_port);
> >> > > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> >> > > index 3ea0eb4..d2ac7c6 100644
> >> > > --- a/ofproto/tunnel.c
> >> > > +++ b/ofproto/tunnel.c
> >> > > @@ -1,4 +1,4 @@
> >> > > -/* Copyright (c) 2013, 2014 Nicira,
[ovs-dev] sign-off (was: [PATCH v2] ofproto-dpif: Avoid creating OpenFlow ports for duplicate tunnels.)
Andy, I added you as co-author on this, can you give me a Signed-off-by
for your contribution?
On Fri, Jun 05, 2015 at 08:13:28AM -0700, Ben Pfaff wrote:
> Until now, when two tunnels had an identical configuration, both of them
> were assigned OpenFlow ports, but only one of those OpenFlow ports was
> functional. With this commit, only one of the two (or more) identically
> configured tunnels will be assigned an OpenFlow port number.
>
> Reported-by: Keith Holleman
> Signed-off-by: Ben Pfaff
> Co-authored-by: Andy Zhou
> ---
> v1->v2: Fix reference counting and memory leak (from Andy Zhou).
>
> AUTHORS| 1 +
> ofproto/ofproto-dpif.c | 10 --
> ofproto/tunnel.c | 14 ++
> ofproto/tunnel.h | 6 +++---
> 4 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index f155105..28899a8 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -277,6 +277,7 @@ Joan Cirer j...@ev0.net
> John Darrington j...@darrington.wattle.id.au
> John Galgay j...@galgay.net
> John Hurley john.hur...@netronome.com
> +Keith Holleman hollemani...@gmail.com
> K 華k940...@hotmail.com
> Kevin Mancuso kevin.manc...@rackspace.com
> Kiran Shanbhog ki...@vmware.com
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index c1daa1d..bd45305 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1671,8 +1671,14 @@ port_construct(struct ofport *port_)
>
> if (netdev_get_tunnel_config(netdev)) {
> atomic_count_inc(&ofproto->backer->tnl_count);
> -tnl_port_add(port, port->up.netdev, port->odp_port,
> - ovs_native_tunneling_is_on(ofproto), namebuf);
> +error = tnl_port_add(port, port->up.netdev, port->odp_port,
> + ovs_native_tunneling_is_on(ofproto), namebuf);
> +if (error) {
> +atomic_count_dec(&ofproto->backer->tnl_count);
> +dpif_port_destroy(&dpif_port);
> +return error;
> +}
> +
> port->is_tunnel = true;
> if (ofproto->ipfix) {
> dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_, port->odp_port);
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 3ea0eb4..d2ac7c6 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2013, 2014 Nicira, Inc.
> +/* Copyright (c) 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.
> @@ -203,14 +203,20 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
> struct netdev *netdev,
>
> /* Adds 'ofport' to the module with datapath port number 'odp_port'.
> 'ofport's
> * must be added before they can be used by the module. 'ofport' must be a
> - * tunnel. */
> -void
> + * tunnel.
> + *
> + * Returns 0 if successful, otherwise a positive errno value. */
> +int
> tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev,
> odp_port_t odp_port, bool native_tnl, const char name[])
> OVS_EXCLUDED(rwlock)
> {
> +bool ok;
> +
> fat_rwlock_wrlock(&rwlock);
> -tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
> +ok = tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
> fat_rwlock_unlock(&rwlock);
> +
> +return ok ? 0 : EEXIST;
> }
>
> /* Checks if the tunnel represented by 'ofport' reconfiguration due to
> changes
> diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
> index 6181762..b8415ab 100644
> --- a/ofproto/tunnel.h
> +++ b/ofproto/tunnel.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2013 Nicira, Inc.
> +/* Copyright (c) 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.
> @@ -33,8 +33,8 @@ void ofproto_tunnel_init(void);
> bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev *,
>odp_port_t, bool native_tnl, const char name[]);
>
> -void tnl_port_add(const struct ofport_dpif *, const struct netdev *,
> - odp_port_t odp_port, bool native_tnl, const char name[]);
> +int tnl_port_add(const struct ofport_dpif *, const struct netdev *,
> + odp_port_t odp_port, bool native_tnl, const char name[]);
> void tnl_port_del(const struct ofport_dpif *);
>
> const struct ofport_dpif *tnl_port_receive(const struct flow *);
> --
> 2.1.3
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
On Wed, Jun 03, 2015 at 11:21:50PM -0700, Alex Wang wrote: > OVS datapath has check which prevents the installation of flow > that matches VLAN TCI but does not have exact match for VLAN_CFI > bit. However, the ovs userspace does not enforce it, so OpenFlow > flow like "vlan_tci=0x000a/0x0fff,action=output:local" can be added > to ovs. Subsequently, the generated megaflow will have match > field for vlan like "vlan(vid=5/0xfff,pcp=0/0x0,cfi=1/0)". > > With the OVS datapath check, the installation of such megaflow > will be rejected with: > "|WARN|system@ovs-system: failed to put[create][modify] (Invalid argument)" > > This commit adds a check in userspace that mark the vlan mask > invalid if it does not exact match for VLAN_CFI. So users will > be asked to provide correct mask. This is not the right fix, because it is legitimate and useful not to match on the "CFI" (actually "vlan present") bit in OpenFlow. See the comment in meta-flow.h: /* "vlan_tci". * * 802.1Q TCI. * * For a packet with an 802.1Q header, this is the Tag Control Information * (TCI) field, with the CFI bit forced to 1. For a packet with no 802.1Q * header, this has value 0. * * This field can be used in various ways: * * - If it is not constrained at all, the nx_match matches packets * without an 802.1Q header or with an 802.1Q header that has any TCI * value. * * - Testing for an exact match with 0 matches only packets without an * 802.1Q header. * * - Testing for an exact match with a TCI value with CFI=1 matches * packets that have an 802.1Q header with a specified VID and PCP. * * - Testing for an exact match with a nonzero TCI value with CFI=0 does * not make sense. The switch may reject this combination. * * - Testing with a specific VID and CFI=1, with nxm_mask=0x1fff, matches * packets that have an 802.1Q header with that VID (and any PCP). * * - Testing with a specific PCP and CFI=1, with nxm_mask=0xf000, matches * packets that have an 802.1Q header with that PCP (and any VID). * * - Testing with nxm_value=0, nxm_mask=0x0fff matches packets with no * 802.1Q header or with an 802.1Q header with a VID of 0. * * - Testing with nxm_value=0, nxm_mask=0xe000 matches packets with no * 802.1Q header or with an 802.1Q header with a PCP of 0. * * - Testing with nxm_value=0, nxm_mask=0xefff matches packets with no * 802.1Q header or with an 802.1Q header with both VID and PCP of 0. * * Type: be16. * Maskable: bitwise. * Formatting: hexadecimal. * Prerequisites: none. * Access: read/write. * NXM: NXM_OF_VLAN_TCI(4) since v1.1. * OXM: none. * OF1.0: exact match. * OF1.1: exact match. */ MFF_VLAN_TCI, I think that we should fix this in flow translation, by "unwildcarding" the CFI bit if any other bits in vlan_tci are unwildcarded. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
Thx for the reference, exactly what i want, Will change the solution~ Thanks, Alex Wang, On Fri, Jun 5, 2015 at 8:24 AM, Ben Pfaff wrote: > On Wed, Jun 03, 2015 at 11:21:50PM -0700, Alex Wang wrote: > > OVS datapath has check which prevents the installation of flow > > that matches VLAN TCI but does not have exact match for VLAN_CFI > > bit. However, the ovs userspace does not enforce it, so OpenFlow > > flow like "vlan_tci=0x000a/0x0fff,action=output:local" can be added > > to ovs. Subsequently, the generated megaflow will have match > > field for vlan like "vlan(vid=5/0xfff,pcp=0/0x0,cfi=1/0)". > > > > With the OVS datapath check, the installation of such megaflow > > will be rejected with: > > "|WARN|system@ovs-system: failed to put[create][modify] (Invalid > argument)" > > > > This commit adds a check in userspace that mark the vlan mask > > invalid if it does not exact match for VLAN_CFI. So users will > > be asked to provide correct mask. > > This is not the right fix, because it is legitimate and useful not to > match on the "CFI" (actually "vlan present") bit in OpenFlow. See the > comment in meta-flow.h: > > /* "vlan_tci". > * > * 802.1Q TCI. > * > * For a packet with an 802.1Q header, this is the Tag Control > Information > * (TCI) field, with the CFI bit forced to 1. For a packet with no > 802.1Q > * header, this has value 0. > * > * This field can be used in various ways: > * > * - If it is not constrained at all, the nx_match matches packets > * without an 802.1Q header or with an 802.1Q header that has any > TCI > * value. > * > * - Testing for an exact match with 0 matches only packets without > an > * 802.1Q header. > * > * - Testing for an exact match with a TCI value with CFI=1 matches > * packets that have an 802.1Q header with a specified VID and PCP. > * > * - Testing for an exact match with a nonzero TCI value with CFI=0 > does > * not make sense. The switch may reject this combination. > * > * - Testing with a specific VID and CFI=1, with nxm_mask=0x1fff, > matches > * packets that have an 802.1Q header with that VID (and any PCP). > * > * - Testing with a specific PCP and CFI=1, with nxm_mask=0xf000, > matches > * packets that have an 802.1Q header with that PCP (and any VID). > * > * - Testing with nxm_value=0, nxm_mask=0x0fff matches packets with > no > * 802.1Q header or with an 802.1Q header with a VID of 0. > * > * - Testing with nxm_value=0, nxm_mask=0xe000 matches packets with > no > * 802.1Q header or with an 802.1Q header with a PCP of 0. > * > * - Testing with nxm_value=0, nxm_mask=0xefff matches packets with > no > * 802.1Q header or with an 802.1Q header with both VID and PCP of > 0. > * > * Type: be16. > * Maskable: bitwise. > * Formatting: hexadecimal. > * Prerequisites: none. > * Access: read/write. > * NXM: NXM_OF_VLAN_TCI(4) since v1.1. > * OXM: none. > * OF1.0: exact match. > * OF1.1: exact match. > */ > MFF_VLAN_TCI, > > I think that we should fix this in flow translation, by "unwildcarding" > the CFI bit if any other bits in vlan_tci are unwildcarded. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v9] ovs-ofctl:Implementation of eviction on the basis of Importance
On Fri, May 29, 2015 at 04:00:27PM +0530, saloni.jai...@gmail.com wrote: > From: Saloni Jain > > This commit enables the eviction mechanism on the basis of importance as > per the openflow specification 1.4. > > "ovs-ofctl -O OpenFlow14 mod-table evict" > Enable eviction on of . Eviction adds a mechanism > enabling the switch to automatically eliminate entries of lower > importance to make space for newer entries.If want to enable eviction > on all tables, user can set the as 'ALL'. > > "ovs-ofctl -O OpenFlow14 mod-table noevict" > Disable eviction on of . > > "ovs-ofctl -O OpenFlow14 dump-tables-desc " > This command provides a way to list the current configuration > (eviction for importance) of the tables on a , which is set > using the mod-table command. > > Signed-off-by: Saloni Jain This adds abstracted OFPUTIL_TABLE_CONFIG_* but it doesn't do any translation between them and OFPTC14_*. This adds a weird CLEAR_EVICTION_BIT definition. Use an OFPUTIL_TABLE_CONFIG_* constant. This leaves in the following comment that is now obsolete: /* We do not understand any properties yet, so we do not bother * parsing them. */ It appears to me that all of the OFPUTIL_TABLE_CONFIG modes are mutually exclusive. Why are they individual bits? Alternatively, there is a bit for "NO_EVICTION" and one for "EVICTION"; why both? Why does this expression contain parentheses? return UINT32_MAX - (rule->importance); ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ovs-vswitchd: Update documentation
On Wed, May 27, 2015 at 10:34:31AM +0200, Mijo Safradin wrote: > Commit 7a6cf343a410d77e05ebd7bf5b5ade52803879ae raised the MAXFD > limit from 7500 to 65535. > > Signed-off-by: Mijo Safradin Applied, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables
On Thu, May 14, 2015 at 08:04:02PM -0700, Shashank Shanbhag wrote: > > It's unusual to dynamically allocate an ovs_list head. I would > > ordinarily just declare 'tables' as a plain ovs_list instead of a > > pointer. > > > > Do you think there is much value in maintaining the order of table > > update using the linked list? It seems likely to me that ordinarily > > the update order is not important, and in cases where it is important > > it seems that the user could call ovs-ofctl more than once. It would > > simplify the implementation a little, and I guess that it would be > > just as easy to explain, if the tables were simply updated in order of > > increasing table number. > > I think update order matters. I would expect table 0 to be updated last > after all the flows in later tables are updated to, say, reflect a change > in policy. > > Also, calling ovs-ofctl 255 times in the worst case would be really > expensive, considering the fact that the FILE can be large, and the > connection may be over SSL. OK, that's fine then. > > I think that we might as well just use an array for the table of > > classifiers. It's only an array of 255 elements, after all. I don't > > see a benefit to using a hash table. > > Trying to avoid the array of 255 elements, especially for cases where there > are less number of tables being replaced. An array of 255 bytes is not a big deal, and it would be easier to understand. > > In read_flows_from_switch(), it seems odd to me to use > > parse_ofp_flow_stats_request_str() to initialize the flow stats > > request. I would tend to continue initializing it "by hand". > > I'd like to avoid doing it this way. Any future updates will need to be > done at a single place. Also, parse_ofp_flow_stats_request_str() is not > being used anywhere in the code, FYI. I must have been looking at something else, the code here look fine to me in v2. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables
On Sat, May 16, 2015 at 07:46:59PM -0700, shashank.shanb...@gmail.com wrote: > From: Shashank Shanbhag > > Fix replace-flows and diff-flows to modify/diff flows in multiple tables. > Add a --tables option that allows the user to specify a comma-separated > list of table indexes to replace/diff. > > Signed-off-by: Shashank Shanbhag > Acked-by: Romain Lenglet Besides the comments in the thread we already have going, I think that, in the default case, these commands should probably start by querying the switch to find out what tables it has, so that it only tries to read and update the tables that the switch actually announces. This is particularly important for OVS, because OVS has "hidden tables" that are not announced in the messages that describe tables, but controllers are allowed to read (but, currently, not write). We might add more of these tables later. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables
Hi Ben, Thanks. I thought there are only hidden flows, and not hidden tables. We are reading all tables from 0 to OFPTT_MAX-1. But, I see what you're saying. I can probably use OFPMP_TABLE_FEATURES (openflow1.4 spec Section 7.3.5.6) to get all the configured tables. Will this ensure the hidden tables aren't returned? Please correct me if I'm wrong. Shashank - Shashank Shanbhag, - On Fri, Jun 5, 2015 at 10:12 AM, Ben Pfaff wrote: > On Sat, May 16, 2015 at 07:46:59PM -0700, shashank.shanb...@gmail.com > wrote: > > From: Shashank Shanbhag > > > > Fix replace-flows and diff-flows to modify/diff flows in multiple tables. > > Add a --tables option that allows the user to specify a comma-separated > > list of table indexes to replace/diff. > > > > Signed-off-by: Shashank Shanbhag > > Acked-by: Romain Lenglet > > Besides the comments in the thread we already have going, I think that, > in the default case, these commands should probably start by querying > the switch to find out what tables it has, so that it only tries to read > and update the tables that the switch actually announces. This is > particularly important for OVS, because OVS has "hidden tables" that are > not announced in the messages that describe tables, but controllers are > allowed to read (but, currently, not write). We might add more of these > tables later. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto-dpif: Use xzalloc instead of xmalloc
Use xzalloc instead of xmalloc for some key structure allocations in
ofproto-dpif (viz. ofproto_dpif, ofport_dpif and rule_dpif) so as to
prevent uninitialized values in these structures. Also add seat belts
around these structure allocations.
Signed-off-by: Sabyasachi Sengupta
---
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c1daa1d..ae38d4e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -812,7 +812,10 @@ static int add_internal_flows(struct ofproto_dpif *);
static struct ofproto *
alloc(void)
{
-struct ofproto_dpif *ofproto = xmalloc(sizeof *ofproto);
+struct ofproto_dpif *ofproto = xzalloc(sizeof *ofproto);
+if (!ofproto) {
+return NULL;
+}
return &ofproto->up;
}
@@ -1608,7 +1611,10 @@ query_tables(struct ofproto *ofproto,
static struct ofport *
port_alloc(void)
{
-struct ofport_dpif *port = xmalloc(sizeof *port);
+struct ofport_dpif *port = xzalloc(sizeof *port);
+if (!port) {
+return NULL;
+}
return &port->up;
}
@@ -3876,7 +3882,10 @@ static struct rule_dpif *rule_dpif_cast(const struct
rule *rule)
static struct rule *
rule_alloc(void)
{
-struct rule_dpif *rule = xmalloc(sizeof *rule);
+struct rule_dpif *rule = xzalloc(sizeof *rule);
+if (!rule) {
+return NULL;
+}
return &rule->up;
}
--
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows
On Tue, May 19, 2015 at 10:02 AM, Alin Serdean wrote: > This patch implements two functionalities needed for an active manager: > 1. ARP lookup > 2. Next hop > > The first functionality relies on the internal Windows API: > https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365956-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=AjthuTJGRGkQqO4iaxxvp5uVrWnSngSiynyZFrmI4fM&e= > > The second one: > https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365915-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=cYblpE7EGRgk2wDKdIc9MFjjG9U-91iBWzp2rstlaH4&e= > > Both API's are found in the Iphlpapi library. We need to add this library > when compiling. > > Documentation and appveyor config has been updated to match the use of the > new library. > > Tested using opendaylight. > > Signed-off-by: Alin Gabriel Serdean > Reported-by: Alin Gabriel Serdean Alin, This commit has broken the appveyor build tests. Can you please update appveyor.yml in the repo to include the necessary configure changes? > Reported-at: https://github.com/openvswitch/ovs-issues/issues/63 > --- > v2: call GetIpNetTable and GetAdaptersAddresses with a zero length buffer. > add pretty printing > --- > --- > INSTALL.Windows.md | 25 +-- > lib/netdev-windows.c | 114 > +++ > 2 files changed, 127 insertions(+), 12 deletions(-) > > diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md > index 78af0a1..0ec0af0 100644 > --- a/INSTALL.Windows.md > +++ b/INSTALL.Windows.md > @@ -62,9 +62,10 @@ or from a distribution tar ball. >the right compiler, linker, libraries, Open vSwitch component installation >directories, etc. For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > - --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > - --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > + LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > + --localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" > \ > + --with-pthread="C:/pthread" > > By default, the above enables compiler optimization for fast code. > For default compiler optimization, pass the "--with-debug" configure > @@ -114,10 +115,10 @@ Note down the directory where OpenSSL is installed > (e.g.: C:/OpenSSL-Win32). > * While configuring the package, specify the OpenSSL directory path. > For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > ---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > ---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \ > ---enable-ssl --with-openssl="C:/OpenSSL-Win32" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > +LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" --enable-ssl > --with-openssl="C:/OpenSSL-Win32" > > * Run make for the ported executables. > > @@ -131,11 +132,11 @@ level 'make' will invoke building the kernel datapath, > if the > '--with-vstudioddk' argument is specified while configuring the package. > For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > ---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > ---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \ > ---enable-ssl --with-openssl="C:/OpenSSL-Win32" \ > ---with-vstudioddk="" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > +LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" --enable-ssl \ > +--with-openssl="C:/OpenSSL-Win32" --with-vstudioddk="" > > Possible values for "" are: > "Win8.1 Debug", "Win8.1 Release", "Win8 Debug" and "Win8 Release". > diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c > index 1fc1da7..1eb8727 100644 > --- a/lib/netdev-windows.c > +++ b/lib/netdev-windows.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include > > @@ -373,6 +374,117 @@ netdev_windows_update_flags(struct netdev *netdev_, > return 0; > } > > +/* Looks up in the ARP table entry for a given 'ip'. If it is found, the > + * corresponding MAC address will be copied in 'mac' and return 0. If no > + * matching entry is found or an error occurs it will log it and return > ENXIO. > + */ > +stat
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
How about this?
diff --git a/lib/match.c b/lib/match.c
index 7d0b409..e34572d 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -360,6 +360,10 @@ match_set_dl_tci(struct match *match, ovs_be16 tci)
void
match_set_dl_tci_masked(struct match *match, ovs_be16 tci, ovs_be16 mask)
{
+if (mask && mask != htons(0x)) {
+tci |= htons(VLAN_CFI);
+mask |= htons(VLAN_CFI);
+}
match->flow.vlan_tci = tci & mask;
match->wc.masks.vlan_tci = mask;
}
On Fri, Jun 5, 2015 at 9:24 AM, Alex Wang wrote:
> Thx for the reference, exactly what i want,
>
> Will change the solution~
>
> Thanks,
> Alex Wang,
>
> On Fri, Jun 5, 2015 at 8:24 AM, Ben Pfaff wrote:
>
>> On Wed, Jun 03, 2015 at 11:21:50PM -0700, Alex Wang wrote:
>> > OVS datapath has check which prevents the installation of flow
>> > that matches VLAN TCI but does not have exact match for VLAN_CFI
>> > bit. However, the ovs userspace does not enforce it, so OpenFlow
>> > flow like "vlan_tci=0x000a/0x0fff,action=output:local" can be added
>> > to ovs. Subsequently, the generated megaflow will have match
>> > field for vlan like "vlan(vid=5/0xfff,pcp=0/0x0,cfi=1/0)".
>> >
>> > With the OVS datapath check, the installation of such megaflow
>> > will be rejected with:
>> > "|WARN|system@ovs-system: failed to put[create][modify] (Invalid
>> argument)"
>> >
>> > This commit adds a check in userspace that mark the vlan mask
>> > invalid if it does not exact match for VLAN_CFI. So users will
>> > be asked to provide correct mask.
>>
>> This is not the right fix, because it is legitimate and useful not to
>> match on the "CFI" (actually "vlan present") bit in OpenFlow. See the
>> comment in meta-flow.h:
>>
>> /* "vlan_tci".
>> *
>> * 802.1Q TCI.
>> *
>> * For a packet with an 802.1Q header, this is the Tag Control
>> Information
>> * (TCI) field, with the CFI bit forced to 1. For a packet with no
>> 802.1Q
>> * header, this has value 0.
>> *
>> * This field can be used in various ways:
>> *
>> * - If it is not constrained at all, the nx_match matches packets
>> * without an 802.1Q header or with an 802.1Q header that has any
>> TCI
>> * value.
>> *
>> * - Testing for an exact match with 0 matches only packets without
>> an
>> * 802.1Q header.
>> *
>> * - Testing for an exact match with a TCI value with CFI=1 matches
>> * packets that have an 802.1Q header with a specified VID and
>> PCP.
>> *
>> * - Testing for an exact match with a nonzero TCI value with CFI=0
>> does
>> * not make sense. The switch may reject this combination.
>> *
>> * - Testing with a specific VID and CFI=1, with nxm_mask=0x1fff,
>> matches
>> * packets that have an 802.1Q header with that VID (and any PCP).
>> *
>> * - Testing with a specific PCP and CFI=1, with nxm_mask=0xf000,
>> matches
>> * packets that have an 802.1Q header with that PCP (and any VID).
>> *
>> * - Testing with nxm_value=0, nxm_mask=0x0fff matches packets with
>> no
>> * 802.1Q header or with an 802.1Q header with a VID of 0.
>> *
>> * - Testing with nxm_value=0, nxm_mask=0xe000 matches packets with
>> no
>> * 802.1Q header or with an 802.1Q header with a PCP of 0.
>> *
>> * - Testing with nxm_value=0, nxm_mask=0xefff matches packets with
>> no
>> * 802.1Q header or with an 802.1Q header with both VID and PCP
>> of 0.
>> *
>> * Type: be16.
>> * Maskable: bitwise.
>> * Formatting: hexadecimal.
>> * Prerequisites: none.
>> * Access: read/write.
>> * NXM: NXM_OF_VLAN_TCI(4) since v1.1.
>> * OXM: none.
>> * OF1.0: exact match.
>> * OF1.1: exact match.
>> */
>> MFF_VLAN_TCI,
>>
>> I think that we should fix this in flow translation, by "unwildcarding"
>> the CFI bit if any other bits in vlan_tci are unwildcarded.
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows
Sure. Alin. -Mesaj original- De la: Gurucharan Shetty [mailto:shet...@nicira.com] Trimis: Friday, June 5, 2015 9:00 PM Către: Alin Serdean Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows On Tue, May 19, 2015 at 10:02 AM, Alin Serdean wrote: > This patch implements two functionalities needed for an active manager: > 1. ARP lookup > 2. Next hop > > The first functionality relies on the internal Windows API: > https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.co > m_en-2Dus_library_windows_desktop_aa365956-2528v-3Dvs.85-2529.aspx&d=A > wIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY79 > 8tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3Qrymbo > wLc&s=AjthuTJGRGkQqO4iaxxvp5uVrWnSngSiynyZFrmI4fM&e= > > The second one: > https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.co > m_en-2Dus_library_windows_desktop_aa365915-2528v-3Dvs.85-2529.aspx&d=A > wIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY79 > 8tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3Qrymbo > wLc&s=cYblpE7EGRgk2wDKdIc9MFjjG9U-91iBWzp2rstlaH4&e= > > Both API's are found in the Iphlpapi library. We need to add this library > when compiling. > > Documentation and appveyor config has been updated to match the use of the > new library. > > Tested using opendaylight. > > Signed-off-by: Alin Gabriel Serdean > Reported-by: Alin Gabriel Serdean Alin, This commit has broken the appveyor build tests. Can you please update appveyor.yml in the repo to include the necessary configure changes? > Reported-at: https://github.com/openvswitch/ovs-issues/issues/63 > --- > v2: call GetIpNetTable and GetAdaptersAddresses with a zero length buffer. > add pretty printing > --- > --- > INSTALL.Windows.md | 25 +-- > lib/netdev-windows.c | 114 > +++ > 2 files changed, 127 insertions(+), 12 deletions(-) > > diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md index > 78af0a1..0ec0af0 100644 > --- a/INSTALL.Windows.md > +++ b/INSTALL.Windows.md > @@ -62,9 +62,10 @@ or from a distribution tar ball. >the right compiler, linker, libraries, Open vSwitch component installation >directories, etc. For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > - --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > - --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > + LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > + --localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" > \ > + --with-pthread="C:/pthread" > > By default, the above enables compiler optimization for fast code. > For default compiler optimization, pass the "--with-debug" > configure @@ -114,10 +115,10 @@ Note down the directory where OpenSSL is > installed (e.g.: C:/OpenSSL-Win32). > * While configuring the package, specify the OpenSSL directory path. > For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > ---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > ---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \ > ---enable-ssl --with-openssl="C:/OpenSSL-Win32" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > +LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" --enable-ssl > --with-openssl="C:/OpenSSL-Win32" > > * Run make for the ported executables. > > @@ -131,11 +132,11 @@ level 'make' will invoke building the kernel > datapath, if the '--with-vstudioddk' argument is specified while configuring > the package. > For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > ---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > ---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \ > ---enable-ssl --with-openssl="C:/OpenSSL-Win32" \ > ---with-vstudioddk="" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > +LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" --enable-ssl \ > +--with-openssl="C:/OpenSSL-Win32" --with-vstudioddk="" > > Possible values for "" are: > "Win8.1 Debug", "Win8.1 Release", "Win8 Debug" and "Win8 Release". > diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c index > 1fc1da7..1eb8727 100644 > --- a/lib/netdev-windows.c > +++ b/lib/netdev-windows.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > > #include > > @@ -373,6 +374,117 @@ netdev_windows_update_flags(struct netdev
Re: [ovs-dev] [PATCH v6] This commit adds the windows installer to the OVS tree.
Sure. Alin. -Mesaj original- De la: Gurucharan Shetty [mailto:shet...@nicira.com] Trimis: Friday, June 5, 2015 5:57 PM Către: Alin Serdean Cc: dev@openvswitch.org; Nithin Raju; Eitan Eliahu Subiect: Re: [PATCH v6] This commit adds the windows installer to the OVS tree. Alin, I do not see Alessandro's signed-off. Can you please add. On Fri, Jun 5, 2015 at 5:16 AM, Alin Serdean wrote: > Sorry but unfortunately I could not get the patch on the ML. > Created the following pull request instead: > https://github.com/openvswitch/ovs/pull/49 . > > Thank you, > Alin. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] sign-off (was: [PATCH v2] ofproto-dpif: Avoid creating OpenFlow ports for duplicate tunnels.)
Thank you for your kindness.
Signed-off-by: Andy Zhou
On Fri, Jun 5, 2015 at 8:14 AM, Ben Pfaff wrote:
> Andy, I added you as co-author on this, can you give me a Signed-off-by
> for your contribution?
>
> On Fri, Jun 05, 2015 at 08:13:28AM -0700, Ben Pfaff wrote:
>> Until now, when two tunnels had an identical configuration, both of them
>> were assigned OpenFlow ports, but only one of those OpenFlow ports was
>> functional. With this commit, only one of the two (or more) identically
>> configured tunnels will be assigned an OpenFlow port number.
>>
>> Reported-by: Keith Holleman
>> Signed-off-by: Ben Pfaff
>> Co-authored-by: Andy Zhou
>> ---
>> v1->v2: Fix reference counting and memory leak (from Andy Zhou).
>>
>> AUTHORS| 1 +
>> ofproto/ofproto-dpif.c | 10 --
>> ofproto/tunnel.c | 14 ++
>> ofproto/tunnel.h | 6 +++---
>> 4 files changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/AUTHORS b/AUTHORS
>> index f155105..28899a8 100644
>> --- a/AUTHORS
>> +++ b/AUTHORS
>> @@ -277,6 +277,7 @@ Joan Cirer j...@ev0.net
>> John Darrington j...@darrington.wattle.id.au
>> John Galgay j...@galgay.net
>> John Hurley john.hur...@netronome.com
>> +Keith Holleman hollemani...@gmail.com
>> K 華k940...@hotmail.com
>> Kevin Mancuso kevin.manc...@rackspace.com
>> Kiran Shanbhog ki...@vmware.com
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index c1daa1d..bd45305 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -1671,8 +1671,14 @@ port_construct(struct ofport *port_)
>>
>> if (netdev_get_tunnel_config(netdev)) {
>> atomic_count_inc(&ofproto->backer->tnl_count);
>> -tnl_port_add(port, port->up.netdev, port->odp_port,
>> - ovs_native_tunneling_is_on(ofproto), namebuf);
>> +error = tnl_port_add(port, port->up.netdev, port->odp_port,
>> + ovs_native_tunneling_is_on(ofproto), namebuf);
>> +if (error) {
>> +atomic_count_dec(&ofproto->backer->tnl_count);
>> +dpif_port_destroy(&dpif_port);
>> +return error;
>> +}
>> +
>> port->is_tunnel = true;
>> if (ofproto->ipfix) {
>> dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_,
>> port->odp_port);
>> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
>> index 3ea0eb4..d2ac7c6 100644
>> --- a/ofproto/tunnel.c
>> +++ b/ofproto/tunnel.c
>> @@ -1,4 +1,4 @@
>> -/* Copyright (c) 2013, 2014 Nicira, Inc.
>> +/* Copyright (c) 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.
>> @@ -203,14 +203,20 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
>> struct netdev *netdev,
>>
>> /* Adds 'ofport' to the module with datapath port number 'odp_port'.
>> 'ofport's
>> * must be added before they can be used by the module. 'ofport' must be a
>> - * tunnel. */
>> -void
>> + * tunnel.
>> + *
>> + * Returns 0 if successful, otherwise a positive errno value. */
>> +int
>> tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev,
>> odp_port_t odp_port, bool native_tnl, const char name[])
>> OVS_EXCLUDED(rwlock)
>> {
>> +bool ok;
>> +
>> fat_rwlock_wrlock(&rwlock);
>> -tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
>> +ok = tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
>> fat_rwlock_unlock(&rwlock);
>> +
>> +return ok ? 0 : EEXIST;
>> }
>>
>> /* Checks if the tunnel represented by 'ofport' reconfiguration due to
>> changes
>> diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
>> index 6181762..b8415ab 100644
>> --- a/ofproto/tunnel.h
>> +++ b/ofproto/tunnel.h
>> @@ -1,4 +1,4 @@
>> -/* Copyright (c) 2013 Nicira, Inc.
>> +/* Copyright (c) 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.
>> @@ -33,8 +33,8 @@ void ofproto_tunnel_init(void);
>> bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev *,
>>odp_port_t, bool native_tnl, const char name[]);
>>
>> -void tnl_port_add(const struct ofport_dpif *, const struct netdev *,
>> - odp_port_t odp_port, bool native_tnl, const char name[]);
>> +int tnl_port_add(const struct ofport_dpif *, const struct netdev *,
>> + odp_port_t odp_port, bool native_tnl, const char name[]);
>> void tnl_port_del(const struct ofport_dpif *);
>>
>> const struct ofport_dpif *tnl_port_receive(const struct flow *);
>> --
>> 2.1.3
>>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Link library updates for appveyor
Add the library iphlpapi to the appveyor.yml build script. Signed-off-by: Alin Gabriel Serdean --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index a14f0fc..863b561 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -39,5 +39,5 @@ build_script: - C:\MinGW\msys\1.0\bin\bash -lc "cp /c/pthreads-win32/Pre-built.2/dll/x86/*.dll /c/openvswitch/." - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe" - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh" -- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=-lws2_32 --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudioddk=\"Win8.1 Debug\"" +- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudioddk=\"Win8.1 Debug\"" - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make" -- 1.9.5.msysgit.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows
I just sent the fix: http://openvswitch.org/pipermail/dev/2015-June/056007.html . Sorry for the failing build. I will repost the pull request after this one is merged. Thanks, Alin. -Mesaj original- De la: dev [mailto:dev-boun...@openvswitch.org] În numele Alin Serdean Trimis: Friday, June 5, 2015 9:20 PM Către: Gurucharan Shetty Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows Sure. Alin. -Mesaj original- De la: Gurucharan Shetty [mailto:shet...@nicira.com] Trimis: Friday, June 5, 2015 9:00 PM Către: Alin Serdean Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows On Tue, May 19, 2015 at 10:02 AM, Alin Serdean wrote: > This patch implements two functionalities needed for an active manager: > 1. ARP lookup > 2. Next hop > > The first functionality relies on the internal Windows API: > https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.co > m_en-2Dus_library_windows_desktop_aa365956-2528v-3Dvs.85-2529.aspx&d=A > wIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY79 > 8tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3Qrymbo > wLc&s=AjthuTJGRGkQqO4iaxxvp5uVrWnSngSiynyZFrmI4fM&e= > > The second one: > https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.co > m_en-2Dus_library_windows_desktop_aa365915-2528v-3Dvs.85-2529.aspx&d=A > wIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY79 > 8tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3Qrymbo > wLc&s=cYblpE7EGRgk2wDKdIc9MFjjG9U-91iBWzp2rstlaH4&e= > > Both API's are found in the Iphlpapi library. We need to add this library > when compiling. > > Documentation and appveyor config has been updated to match the use of the > new library. > > Tested using opendaylight. > > Signed-off-by: Alin Gabriel Serdean > Reported-by: Alin Gabriel Serdean Alin, This commit has broken the appveyor build tests. Can you please update appveyor.yml in the repo to include the necessary configure changes? > Reported-at: https://github.com/openvswitch/ovs-issues/issues/63 > --- > v2: call GetIpNetTable and GetAdaptersAddresses with a zero length buffer. > add pretty printing > --- > --- > INSTALL.Windows.md | 25 +-- > lib/netdev-windows.c | 114 > +++ > 2 files changed, 127 insertions(+), 12 deletions(-) > > diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md index > 78af0a1..0ec0af0 100644 > --- a/INSTALL.Windows.md > +++ b/INSTALL.Windows.md > @@ -62,9 +62,10 @@ or from a distribution tar ball. >the right compiler, linker, libraries, Open vSwitch component installation >directories, etc. For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > - --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > - --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > + LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > + --localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" > \ > + --with-pthread="C:/pthread" > > By default, the above enables compiler optimization for fast code. > For default compiler optimization, pass the "--with-debug" > configure @@ -114,10 +115,10 @@ Note down the directory where OpenSSL is > installed (e.g.: C:/OpenSSL-Win32). > * While configuring the package, specify the OpenSSL directory path. > For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > ---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > ---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \ > ---enable-ssl --with-openssl="C:/OpenSSL-Win32" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > +LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" --enable-ssl > --with-openssl="C:/OpenSSL-Win32" > > * Run make for the ported executables. > > @@ -131,11 +132,11 @@ level 'make' will invoke building the kernel > datapath, if the '--with-vstudioddk' argument is specified while configuring > the package. > For example, > > -% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \ > ---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \ > ---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \ > ---enable-ssl --with-openssl="C:/OpenSSL-Win32" \ > ---with-vstudioddk="" > +% ./configure CC=./build-aux/cccl LD="`which link`" \ > +LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \ > +--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \ > +--with-pthread="C:/pthread" --enable-ssl \ > +--with-openssl="C:/OpenS
Re: [ovs-dev] [PATCH v2 16/19] ovs-ofctl: Add 'bundle' command and unit testing.
Thanks for the suggestons, all applied in the v3 series I’ll post soon.
Jarno
> On May 20, 2015, at 11:26 AM, Romain Lenglet
> wrote:
>
> Jarno,
> Thanks a lot for this updated patch, this will be very useful.
> Sorry, I don’t feel comfortable reviewing the other patches.
>
> Acked-by: Romain Lenglet
> --
> Romain Lenglet
>
> On May 18, 2015 at 4:26:12 PM, Jarno Rajahalme
> (jrajaha...@nicira.com(mailto:jrajaha...@nicira.com)) wrote:
>
>> The new ovs-ofctl 'bundle' command accepts files similar to
>> 'add-flows', but each line can optionally start with 'add', 'modify',
>> 'delete', 'modify_strict', or 'delete_strict' keyword, so that
>> arbitrary flow table modifications may be specified in a single file.
>> The modifications in the file are executed as a single transaction
>> using a OpenFlow 1.4 bundle.
>>
>> Similarly, all existing ovs-ofctl flow mod commands now take an
>> optional '--bundle' argument, which executes the flow mods as a single
>> transaction.
>>
>> OpenFlow 1.4 requires bundles to support at least flow and port mods.
>> This implementation does not yet support port mods in bundles.
>>
>> Another restriction is that the atomic transactions are not yet
>> supported.
>>
>> Signed-off-by: Jarno Rajahalme
>> ---
>> NEWS | 14 ++-
>> include/openvswitch/vconn.h | 3 +
>> lib/ofp-parse.c | 40 ++-
>> lib/ofp-parse.h | 6 +-
>> lib/ofp-util.c | 30 ++
>> lib/ofp-util.h | 2 +
>> lib/ofp-version-opt.c | 7 ++
>> lib/ofp-version-opt.h | 1 +
>> lib/vconn.c | 236 +
>> tests/ofproto-macros.at | 10 ++
>> tests/ofproto.at | 244 +++
>> tests/ovs-ofctl.at | 107 +++
>> utilities/ovs-ofctl.8.in | 67 +---
>> utilities/ovs-ofctl.c | 110 ++-
>> 14 files changed, 834 insertions(+), 43 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index a480607..ac60451 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -1,6 +1,6 @@
>> Post-v2.3.0
>> -
>> - - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
>> + - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
>> - Add bash command-line completion support for ovs-vsctl Please check
>> utilities/ovs-command-compgen.INSTALL.md for how to use.
>> - The MAC learning feature now includes per-port fairness to mitigate
>> @@ -27,6 +27,11 @@ Post-v2.3.0
>> commands are now redundant and will be removed in a future
>> release. See ovs-vswitchd(8) for details.
>> - OpenFlow:
>> + * OpenFlow 1.4 bundles are now supported, but for flow mod
>> + messages only. 'atomic' bundles are not yet supported, and
>> + 'ordered' bundles are trivially supported, as all bundled
>> + messages are executed in the order they were added to the
>> + bundle regardless of the presence of the 'ordered' flag.
>> * IPv6 flow label and neighbor discovery fields are now modifiable.
>> * OpenFlow 1.5 extended registers are now supported.
>> * The OpenFlow 1.5 actset_output field is now supported.
>> @@ -41,6 +46,13 @@ Post-v2.3.0
>> * A new Netronome extension to OpenFlow 1.5+ allows control over the
>> fields hashed for OpenFlow select groups. See "selection_method" and
>> related options in ovs-ofctl(8) for details.
>> + - ovs-ofctl has a new 'bundle' command that accepts a file of
>> + arbitrary flow mods as an input that are executed as a single
>> + transaction using the new OpenFlow 1.4 bundles support.
>> + - ovs-ofctl has a new '--bundle' option that makes the flow mod
>> + commands (add-flow, add-flows, mod-flows, del-flows, and
>> + replace-flows) use an OpenFlow 1.4 bundle to operate the
>> + modifications as a single transaction.
>> - ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
>> MD5 is no longer secure and some operating systems have started to disable
>> it in OpenSSL.
>> diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
>> index 3b157e1..f8b6655 100644
>> --- a/include/openvswitch/vconn.h
>> +++ b/include/openvswitch/vconn.h
>> @@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct
>> ofpbuf **);
>> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf
>> **);
>> int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list
>> *requests,
>> struct ofpbuf **replyp);
>> +int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
>> + uint16_t bundle_flags,
>> + void (*error_reporter)(const struct ofp_header *));
>>
>> void vconn_run(struct vconn *);
>> void vconn_run_wait(struct vconn *);
>> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
>> index 126980c..3ed9937 100644
>> --- a/lib/ofp-parse.c
>> +++ b/lib/ofp-parse.c
>> @@ -257,6 +257,29 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int
>> command, char *string,
>>
>> *usable_protocols = OFPUTIL_P_ANY;
>>
>> + if (command == -2) {
>> + size_t len;
>> +
>> + string += strspn(string, " \t\r\n"); /* Skip white space. */
>> + len = strcspn(string, ", \t\r\n"); /* Get length of the first
Re: [ovs-dev] [PATCH v2 16/19] ovs-ofctl: Add 'bundle' command and unit testing.
> On May 29, 2015, at 8:58 PM, Ben Pfaff wrote: > > On Mon, May 18, 2015 at 04:10:25PM -0700, Jarno Rajahalme wrote: >> The new ovs-ofctl 'bundle' command accepts files similar to >> 'add-flows', but each line can optionally start with 'add', 'modify', >> 'delete', 'modify_strict', or 'delete_strict' keyword, so that >> arbitrary flow table modifications may be specified in a single file. >> The modifications in the file are executed as a single transaction >> using a OpenFlow 1.4 bundle. >> >> Similarly, all existing ovs-ofctl flow mod commands now take an >> optional '--bundle' argument, which executes the flow mods as a single >> transaction. >> >> OpenFlow 1.4 requires bundles to support at least flow and port mods. >> This implementation does not yet support port mods in bundles. >> >> Another restriction is that the atomic transactions are not yet >> supported. >> >> Signed-off-by: Jarno Rajahalme > > I like the new command for doing a mix of operations from one file, but > I suspect that this could be almost as useful without bundling. Users > could want to add and delete a mix of flows with any version of > OpenFlow, and the bundle is really just a bonus. > > So: what if you gave the command a more generic name, and then gave it a > --bundle option like the rest of the commands that can be bundled? (In > fact, you could do this without adding a new command at all, instead > allowing "add-flows" to support any command instead of just "add". I > think this would be fully backward compatible.) > Great suggestion, done! > In the manpage, usually we use bullets, with \(bu, instead of hyphens, > for bulleted lists. OK. > > I think that ofctl_flow_mod__() could call bundle_flow_mod__() if > 'bundle' is true, instead of having the caller deal with that. > OK. > When vconn_bundle_transact() finds an error and sends an > OFPBCT_DISCARD_REQUEST, I suspect that the return value from > vconn_bundle_control_transact() should not overwrite 'error', because > then the previous error will be lost. > Done. The discard error, if any should anyway be either reported or logged, so we now keep the original error code for the return value. > I'm a little surprised to see the new "error_reporter" callback > function. Can you explain a little? > Within a bundle we are now streaming the flow mods without waiting for a response after each flow mod, so error responses to earlier mods may arrive any time before the closing bundle control transaction. As there is no uniform way the users may wish to handle errors, this new callback allows the caller to do what they want. ovs-ofctl prints the error messages to stderr, some other callers may want to log them instead. > In parse_ofp_str__(), I suspect that the "strict" versions won't ever be > matched, because the shorter non-strict versions will be matched > earlier. > Actually, with strncmp() with the length of the token in the string, it is the reverse! The shorter non-strict version will not match a longer keyword, but if the order of the comparisons was changed, the longer strict version would match the shorter non-strict version in the string. (I tried this both ways.) > Thanks for adding tests! > > This will be really nice, thanks for implementing it. > > (Now, to find out why there are three more commits...) Will post a v3 with the rest of the series soon. Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Link library updates for appveyor
On Fri, Jun 5, 2015 at 11:39 AM, Alin Serdean wrote: > Add the library iphlpapi to the appveyor.yml build script. > > Signed-off-by: Alin Gabriel Serdean Thanks, applied! > --- > appveyor.yml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/appveyor.yml b/appveyor.yml > index a14f0fc..863b561 100644 > --- a/appveyor.yml > +++ b/appveyor.yml > @@ -39,5 +39,5 @@ build_script: > - C:\MinGW\msys\1.0\bin\bash -lc "cp > /c/pthreads-win32/Pre-built.2/dll/x86/*.dll /c/openvswitch/." > - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe" > - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh" > -- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure > CC=build-aux/cccl LD=\"`which link`\" LIBS=-lws2_32 > --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 > --with-vstudioddk=\"Win8.1 Debug\"" > +- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure > CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi\" > --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 > --with-vstudioddk=\"Win8.1 Debug\"" > - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make" > -- > 1.9.5.msysgit.0 > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] appveyor: Add a newer ssl link.
The older version is no longer available for download. Signed-off-by: Gurucharan Shetty --- appveyor.yml |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 863b561..ebd937b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -15,9 +15,9 @@ init: Invoke-WebRequest $source -OutFile $destination -$source = "http://slproweb.com/download/Win32OpenSSL-1_0_1L.exe"; +$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2a.exe"; -$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_1L.exe" +$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2a.exe" Invoke-WebRequest $source -OutFile $destination @@ -27,7 +27,7 @@ init: cd C:\ovs-build-downloads -.\Win32OpenSSL-1_0_1L.exe /silent /verysilent /sp- /suppressmsgboxes +.\Win32OpenSSL-1_0_2a.exe /silent /verysilent /sp- /suppressmsgboxes Start-Sleep -s 30 -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] appveyor: Add a newer ssl link.
Acked-by: Alin Serdean . -Mesaj original- De la: dev [mailto:dev-boun...@openvswitch.org] În numele Gurucharan Shetty Trimis: Friday, June 5, 2015 10:27 PM Către: dev@openvswitch.org Cc: Gurucharan Shetty Subiect: [ovs-dev] [PATCH] appveyor: Add a newer ssl link. The older version is no longer available for download. Signed-off-by: Gurucharan Shetty --- appveyor.yml |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 863b561..ebd937b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -15,9 +15,9 @@ init: Invoke-WebRequest $source -OutFile $destination -$source = "http://slproweb.com/download/Win32OpenSSL-1_0_1L.exe"; +$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2a.exe"; -$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_1L.exe" +$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2a.exe" Invoke-WebRequest $source -OutFile $destination @@ -27,7 +27,7 @@ init: cd C:\ovs-build-downloads -.\Win32OpenSSL-1_0_1L.exe /silent /verysilent /sp- /suppressmsgboxes +.\Win32OpenSSL-1_0_2a.exe /silent /verysilent /sp- + /suppressmsgboxes Start-Sleep -s 30 -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] appveyor: Add a newer ssl link.
On Fri, Jun 5, 2015 at 2:21 PM, Alin Serdean wrote: > Acked-by: Alin Serdean . Thank you, applied! > > > -Mesaj original- > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Gurucharan Shetty > Trimis: Friday, June 5, 2015 10:27 PM > Către: dev@openvswitch.org > Cc: Gurucharan Shetty > Subiect: [ovs-dev] [PATCH] appveyor: Add a newer ssl link. > > The older version is no longer available for download. > > Signed-off-by: Gurucharan Shetty > --- > appveyor.yml |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/appveyor.yml b/appveyor.yml index 863b561..ebd937b 100644 > --- a/appveyor.yml > +++ b/appveyor.yml > @@ -15,9 +15,9 @@ init: > > Invoke-WebRequest $source -OutFile $destination > > -$source = "http://slproweb.com/download/Win32OpenSSL-1_0_1L.exe"; > +$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2a.exe"; > > -$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_1L.exe" > +$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2a.exe" > > Invoke-WebRequest $source -OutFile $destination > > @@ -27,7 +27,7 @@ init: > > cd C:\ovs-build-downloads > > -.\Win32OpenSSL-1_0_1L.exe /silent /verysilent /sp- /suppressmsgboxes > +.\Win32OpenSSL-1_0_2a.exe /silent /verysilent /sp- > + /suppressmsgboxes > > Start-Sleep -s 30 > > -- > 1.7.9.5 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v3 01/12] ofp-parse: Use F_OUT_PORT when parsing.
We set this field flag for the cases when an out_port should be
parsed, but failed to make use of it.
Two test cases needed to be updated due to use of out_port in flow
add, while out_port is legal for flow deletes only.
Suggested-by: Ben Pfaff
Signed-off-by: Jarno Rajahalme
---
lib/ofp-parse.c|2 +-
tests/ovs-ofctl.at |8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 856044d..6125f27 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -353,7 +353,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command,
char *string,
if (fm->table_id != 0xff) {
*usable_protocols &= OFPUTIL_P_TID;
}
-} else if (!strcmp(name, "out_port")) {
+} else if (fields & F_OUT_PORT && !strcmp(name, "out_port")) {
if (!ofputil_port_from_string(value, &fm->out_port)) {
error = xasprintf("%s is not a valid OpenFlow port",
value);
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 42be8f0..1e12827 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -140,7 +140,7 @@ AT_CLEANUP
AT_SETUP([ovs-ofctl parse-flows (OpenFlow 1.0)])
AT_DATA([flows.txt], [[
# comment
-tcp,tp_src=123,out_port=5,actions=flood
+tcp,tp_src=123,actions=flood
in_port=LOCAL dl_vlan=9 dl_src=00:0A:E4:25:6B:B0 actions=drop
udp dl_vlan_pcp=7 idle_timeout=5 actions=strip_vlan output:0
tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output:1
@@ -159,7 +159,7 @@ AT_CHECK([ovs-ofctl parse-flows flows.txt
AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0],
[[usable protocols: any
chosen protocol: OpenFlow10-table_id
-OFPT_FLOW_MOD: ADD tcp,tp_src=123 out_port:5 actions=FLOOD
+OFPT_FLOW_MOD: ADD tcp,tp_src=123 actions=FLOOD
OFPT_FLOW_MOD: ADD in_port=LOCAL,dl_vlan=9,dl_src=00:0a:e4:25:6b:b0
actions=drop
OFPT_FLOW_MOD: ADD udp,dl_vlan_pcp=7 idle:5 actions=strip_vlan,output:0
OFPT_FLOW_MOD: ADD tcp,nw_src=192.168.0.3,tp_dst=80
actions=set_queue:37,output:1
@@ -177,7 +177,7 @@ AT_CLEANUP
AT_SETUP([ovs-ofctl parse-flows (OpenFlow 1.1)])
AT_DATA([flows.txt], [[
# comment
-tcp,tp_src=123,out_port=5,actions=flood
+tcp,tp_src=123,actions=flood
in_port=LOCAL dl_vlan=9 dl_src=00:0A:E4:25:6B:B0 actions=drop
udp dl_vlan_pcp=7 idle_timeout=5 actions=strip_vlan output:0
tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output:1
@@ -196,7 +196,7 @@ AT_CHECK([ovs-ofctl --protocols OpenFlow11 parse-flows
flows.txt
AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0],
[[usable protocols: any
chosen protocol: OpenFlow11
-OFPT_FLOW_MOD (OF1.1): ADD tcp,tp_src=123 out_port:5 actions=FLOOD
+OFPT_FLOW_MOD (OF1.1): ADD tcp,tp_src=123 actions=FLOOD
OFPT_FLOW_MOD (OF1.1): ADD in_port=LOCAL,dl_vlan=9,dl_src=00:0a:e4:25:6b:b0
actions=drop
OFPT_FLOW_MOD (OF1.1): ADD udp,dl_vlan_pcp=7 idle:5 actions=pop_vlan,output:0
OFPT_FLOW_MOD (OF1.1): ADD tcp,nw_src=192.168.0.3,tp_dst=80
actions=set_queue:37,output:1
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v3 00/12] Atomic bundles with classifier versioning.
This series adds support for atomic flow mod bundles with classifier versioning. Changes since v2: - F_OUT_PORT parsing is fixed as per Ben's suggestion (patch 01). - ovs-ofctl 'bundle' command added in v2 is now folded in to the existing 'add-flow' and 'add-flows' commands. Flow mods are executed serially, separated by barriers, so this new behavior works with all versions of OpenFlow. With the new '--bundle' option the mods are executed as an ordered OpenFlow 1.4 bundle (patch 03). Later patches in the series make these bundles also atomic. - Classifier 'to_be_removed' and 'visible' flags are now folded in into a single 'visibility' attribute, which also has cleaner definition than in v2 (patch 04). - Classifier traversing of identical rules list is now more robust. It has been somewhat fragile since the support for duplicates with identical priorities was recently added (patch 05). - Flow counts in each oftable are now explicitly maintained so that versioning is taken into account. Earlier (since patch 07) the count of rules in a classifier was used as the table's flow count, but now that the classifier may temporarily hold duplicate rules visible in different versions, and the removals are RCU-postponed, the classifier rule count no longer represents the control plane notion of table's rule count (patch 08). - Handling evictions was broken in v2 (and in patch 07), as eviction took place early in the commit, and actually inappropriately bumped the version number too early. Now eviction is treated much like a flow modification, where a new rule replaces the old one, but just without any 'inheritance' from the evicted rule to the new rule. This makes evictions to be executed only when commit is successful, as evictions are reverted like any other changes when the commit fails (patch 09). - Sending flow removed messages is now postponed to the actual destruction of the rule. Previously, the rule could have accrued stats even after the flow removed messages was sent (patch 10). - Bundled messages now take less momory by using a struct minimatch instead of a struct match for flow mods (patch 11). - Bundles now support also port mods, but only for non-atomic bundles. The reason for this is that we have easy way of making port status changes atomic w.r.t. classifier version number changes (patch 12). For correctness, patches 07, 08, and 09 need to be squashed before committing, but these are separated in this series to make reviewing easier (and as a test for Ben for either reading this text or noticing the problems while reading patch 07 :-). Jarno Rajahalme (12): ofp-parse: Use F_OUT_PORT when parsing. ofproto: Rename *_begin functions as *_start. ovs-ofctl: Add bundle support and unit testing. classifier: Support table versioning classifier: Make traversing identical rules robust. ofproto: Infra for table versioning. Use classifier versioning. ofproto: Accurate flow counts. ofproto: Revertible eviction. ofproto: Postpone sending flow removed messages. ofproto: Use minimatch for making bundles smaller. ofproto: Support port mods in bundles. NEWS | 25 +- OPENFLOW-1.1+.md |6 + include/openflow/openflow-common.h |2 + include/openvswitch/vconn.h|3 + lib/classifier-private.h | 158 - lib/classifier.c | 297 + lib/classifier.h | 112 ++-- lib/match.c|9 + lib/match.h|1 + lib/ofp-parse.c| 42 +- lib/ofp-parse.h|6 +- lib/ofp-print.c|1 + lib/ofp-util.c | 44 ++ lib/ofp-util.h | 102 +-- lib/ofp-version-opt.c |7 + lib/ofp-version-opt.h |1 + lib/ovs-router.c |2 +- lib/tnl-ports.c|9 +- lib/vconn.c| 238 ++- ofproto/bundles.h | 20 +- ofproto/connmgr.c |2 +- ofproto/ofproto-dpif-xlate.c | 17 +- ofproto/ofproto-dpif.c | 128 ++-- ofproto/ofproto-dpif.h |5 +- ofproto/ofproto-provider.h | 77 +-- ofproto/ofproto.c | 1289 +--- tests/ofproto-macros.at| 10 + tests/ofproto.at | 244 +++ tests/ovs-ofctl.at | 115 +++- tests/test-classifier.c| 15 +- utilities/ovs-ofctl.8.in | 59 +- utilities/ovs-ofctl.c | 89 ++- 32 files changed, 2234 insertions(+), 901 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v3 02/12] ofproto: Rename *_begin functions as *_start.
Weirdest things can bother you at night when you try to sleep ;-)
Now we have function triples such as add_flow_begin(),
add_flow_finish(), and add_flow_revert(), where a modification is
started in *_begin(), which can fail, and when successful can be
either made permanent with *_finish(), or cancelled with *_revert().
Linguistically it should be either "begin/end" or "start/finish", not
"begin/finish". "begin/end" has some C++ STL baggage, so let's go
with "start/finish". IMO "revert" rhymes with it, too.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0a1d032..9f43034 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -278,7 +278,7 @@ static bool ofproto_group_exists(const struct ofproto
*ofproto,
OVS_EXCLUDED(ofproto->groups_rwlock);
static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *);
static void handle_openflow(struct ofconn *, const struct ofpbuf *);
-static enum ofperr do_bundle_flow_mod_begin(struct ofproto *,
+static enum ofperr do_bundle_flow_mod_start(struct ofproto *,
struct ofputil_flow_mod *,
struct ofp_bundle_entry *)
OVS_REQUIRES(ofproto_mutex);
@@ -4356,7 +4356,7 @@ set_conjunctions(struct rule *rule, const struct
cls_conjunction *conjs,
*
* The caller retains ownership of 'fm->ofpacts'. */
static enum ofperr
-add_flow_begin(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+add_flow_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
struct rule **rulep, bool *modify)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4468,7 +4468,7 @@ add_flow_begin(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
return 0;
}
-/* Revert the effects of add_flow_begin().
+/* Revert the effects of add_flow_start().
* 'new_rule' must be passed in as NULL, if no new rule was allocated and
* inserted to the classifier.
* Note: evictions cannot be reverted. */
@@ -4705,7 +4705,7 @@ modify_flows__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
}
static enum ofperr
-modify_flows_begin__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+modify_flows_start__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4717,7 +4717,7 @@ modify_flows_begin__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
|| fm->new_cookie == OVS_BE64_MAX)) {
bool modify;
-error = add_flow_begin(ofproto, fm, &rules->rules[0], &modify);
+error = add_flow_start(ofproto, fm, &rules->rules[0], &modify);
if (!error) {
ovs_assert(!modify);
}
@@ -4734,7 +4734,7 @@ modify_flows_begin__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
* 'ofconn' is used to retrieve the packet buffer specified in fm->buffer_id,
* if any. */
static enum ofperr
-modify_flows_begin_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+modify_flows_start_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4749,7 +4749,7 @@ modify_flows_begin_loose(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
rule_criteria_destroy(&criteria);
if (!error) {
-error = modify_flows_begin__(ofproto, fm, rules);
+error = modify_flows_start__(ofproto, fm, rules);
}
if (error) {
@@ -4787,7 +4787,7 @@ modify_flows_finish(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
/* Implements OFPFC_MODIFY_STRICT. Returns 0 on success or an OpenFlow error
* code on failure. */
static enum ofperr
-modify_flow_begin_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+modify_flow_start_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4803,7 +4803,7 @@ modify_flow_begin_strict(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
if (!error) {
/* collect_rules_strict() can return max 1 rule. */
-error = modify_flows_begin__(ofproto, fm, rules);
+error = modify_flows_start__(ofproto, fm, rules);
}
if (error) {
@@ -4862,7 +4862,7 @@ delete_flows__(const struct rule_collection *rules,
/* Implements OFPFC_DELETE. */
static enum ofperr
-delete_flows_begin_loose(struct ofproto *ofproto,
+delete_flows_start_loose(struct ofproto *ofproto,
const struct ofputil_flow_mod *fm,
struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
@@ -4913,7 +4913,7 @@ delete_flows_finish(const struct ofputil_flow_mod *fm,
/* Implements OFPFC_DELETE_STRICT. */
static enum ofperr
-delete_flow_begin_strict(s
[ovs-dev] [PATCH v3 03/12] ovs-ofctl: Add bundle support and unit testing.
All existing ovs-ofctl flow mod commands now take an optional
'--bundle' argument, which executes the flow mods as a single
transaction. OpenFlow 1.4+ is implicitly assumed when '--bundle' is
specified.
ovs-ofctl 'add-flow' and 'add-flows' commands now accept flow
specifications that start with an optional 'add', 'modify', 'delete',
'modify_strict', or 'delete_strict' keyword, so that arbitrary flow
table modifications may be specified. For backwards compatibility, a
missing keyword is treated as an 'add'. With the new '--bundle'
option all the modifications are executed as a single transaction
using an OpenFlow 1.4 bundle.
OpenFlow 1.4 requires bundles to support at least flow and port mods.
This implementation does not yet support port mods in bundles.
Another restriction is that the atomic transactions are not yet
supported.
Signed-off-by: Jarno Rajahalme
---
NEWS| 19 +++-
include/openvswitch/vconn.h |3 +
lib/ofp-parse.c | 40 ++-
lib/ofp-parse.h |6 +-
lib/ofp-util.c | 30 ++
lib/ofp-util.h |2 +
lib/ofp-version-opt.c |7 ++
lib/ofp-version-opt.h |1 +
lib/vconn.c | 238 +
tests/ofproto-macros.at | 10 ++
tests/ofproto.at| 244 +++
tests/ovs-ofctl.at | 107 +++
utilities/ovs-ofctl.8.in| 60 ---
utilities/ovs-ofctl.c | 88 +++-
14 files changed, 813 insertions(+), 42 deletions(-)
diff --git a/NEWS b/NEWS
index a480607..5bea237 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,6 @@
Post-v2.3.0
-
- - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
+ - Added support for SFQ, FQ_CoDel and CoDel qdiscs.
- Add bash command-line completion support for ovs-vsctl Please check
utilities/ovs-command-compgen.INSTALL.md for how to use.
- The MAC learning feature now includes per-port fairness to mitigate
@@ -27,6 +27,11 @@ Post-v2.3.0
commands are now redundant and will be removed in a future
release. See ovs-vswitchd(8) for details.
- OpenFlow:
+ * OpenFlow 1.4 bundles are now supported, but for flow mod
+ messages only. 'atomic' bundles are not yet supported, and
+ 'ordered' bundles are trivially supported, as all bundled
+ messages are executed in the order they were added to the
+ bundle regardless of the presence of the 'ordered' flag.
* IPv6 flow label and neighbor discovery fields are now modifiable.
* OpenFlow 1.5 extended registers are now supported.
* The OpenFlow 1.5 actset_output field is now supported.
@@ -41,6 +46,18 @@ Post-v2.3.0
* A new Netronome extension to OpenFlow 1.5+ allows control over the
fields hashed for OpenFlow select groups. See "selection_method" and
related options in ovs-ofctl(8) for details.
+ - ovs-ofctl has a new '--bundle' option that makes the flow mod commands
+ ('add-flow', 'add-flows', 'mod-flows', 'del-flows', and 'replace-flows')
+ use an OpenFlow 1.4 bundle to operate the modifications as a single
+ transaction. If any of the flow mods in a transaction fail, none of
+ them are executed.
+ - ovs-ofctl 'add-flow' and 'add-flows' commands now accept arbitrary flow
+ mods as an input by allowing the flow specification to start with an
+ explicit 'add', 'modify', 'modify_strict', 'delete', or 'delete_strict'
+ keyword. A missing keyword is treated as 'add', so this is fully
+ backwards compatible. With the new '--bundle' option all the flow mods
+ are executed as a single transaction using the new OpenFlow 1.4 bundles
+ support.
- ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
MD5 is no longer secure and some operating systems have started to disable
it in OpenSSL.
diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
index 3b157e1..f8b6655 100644
--- a/include/openvswitch/vconn.h
+++ b/include/openvswitch/vconn.h
@@ -55,6 +55,9 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct
ofpbuf **);
int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf **);
int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list *requests,
struct ofpbuf **replyp);
+int vconn_bundle_transact(struct vconn *, struct ovs_list *requests,
+ uint16_t bundle_flags,
+ void (*error_reporter)(const struct ofp_header *));
void vconn_run(struct vconn *);
void vconn_run_wait(struct vconn *);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 6125f27..210feed 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -258,6 +258,29 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command,
char *string,
*usable_protocols = OFPUTIL_P_ANY;
+if (command ==
[ovs-dev] [PATCH v3 04/12] classifier: Support table versioning
This patch allows classifier rules to become visible and invisible in
specific versions. A 'version' is defined as a positive monotonically
increasing integer, which never wraps around.
The new 'visibility' attribute replaces the prior 'to_be_removed' and
'visible' attributes.
When versioning is not used, the 'version' parameter should be passed
as 'CLS_NO_VERSION'.
This feature enables the support for atomic OpenFlow bundles without
significant performance penalty on 64-bit systems. There is a
performance decrease in 32-bit systems due to 64-bit atomics used.
Signed-off-by: Jarno Rajahalme
---
lib/classifier-private.h | 52 +-
lib/classifier.c | 179 ++
lib/classifier.h | 112 ++---
lib/ovs-router.c |2 +-
lib/tnl-ports.c |9 +--
ofproto/ofproto-dpif.c |2 +-
ofproto/ofproto.c| 46 +---
tests/test-classifier.c | 13 ++--
utilities/ovs-ofctl.c|1 -
9 files changed, 272 insertions(+), 144 deletions(-)
diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index a7edbe9..51cb8cc 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -79,13 +79,63 @@ struct cls_match {
* 'indices'. */
/* Accessed by all readers. */
struct cmap_node cmap_node; /* Within struct cls_subtable 'rules'. */
-bool visible;
+
+/* Controls rule's visibility to lookups.
+ *
+ * When 'visibility' is:
+ *
+ * > 0 - rule is visible starting from version 'visibility'
+ * <= 0 - rule is visible upto version '-visibility'
+ *
+ * The minimum version number used in lookups is 1 (== CLS_NO_VERSION),
+ * which implies that when 'visibility' is:
+ *
+ * 1- rule is visible in all lookup versions
+ * 0- rule is invisible in all lookup versions. */
+atomic_llong visibility;
+
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. */
};
+static inline void
+cls_match_set_visibility(struct cls_match *rule, long long version)
+{
+atomic_store_relaxed(&rule->visibility, version);
+}
+
+static inline bool
+cls_match_visible_in_version(const struct cls_match *rule, long long version)
+{
+long long visibility;
+
+/* clang does not want to read from a const atomic. */
+atomic_read_relaxed(&CONST_CAST(struct cls_match *, rule)->visibility,
+&visibility);
+
+if (OVS_LIKELY(visibility > 0)) {
+/* Rule is visible starting from version 'visibility'. */
+return version >= visibility;
+} else {
+/* Rule is visible upto version '-visibility'. */
+return version <= -visibility;
+}
+}
+
+static inline bool
+cls_match_is_to_be_removed(const struct cls_match *rule)
+{
+long long visibility;
+
+/* clang does not want to read from a const atomic. */
+atomic_read_relaxed(&CONST_CAST(struct cls_match *, rule)->visibility,
+&visibility);
+
+return visibility <= 0;
+}
+
/* A longest-prefix match tree. */
struct trie_node {
uint32_t prefix; /* Prefix bits for this node, MSB first. */
diff --git a/lib/classifier.c b/lib/classifier.c
index 6075cf7..6e657a6 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -99,7 +99,7 @@ cls_match_alloc(const struct cls_rule *rule,
rculist_init(&cls_match->list);
*CONST_CAST(const struct cls_rule **, &cls_match->cls_rule) = rule;
*CONST_CAST(int *, &cls_match->priority) = rule->priority;
-cls_match->visible = false;
+atomic_init(&cls_match->visibility, 0); /* By default INvisibile. */
miniflow_clone_inline(CONST_CAST(struct miniflow *, &cls_match->flow),
&rule->match.flow, count);
ovsrcu_set_hidden(&cls_match->conj_set,
@@ -115,6 +115,7 @@ static struct cls_subtable *insert_subtable(struct
classifier *cls,
static void destroy_subtable(struct classifier *cls, struct cls_subtable *);
static const struct cls_match *find_match_wc(const struct cls_subtable *,
+ long long version,
const struct flow *,
struct trie_ctx *,
unsigned int n_tries,
@@ -139,12 +140,12 @@ next_rule_in_list(const struct cls_match *rule, const
struct cls_match *head)
/* Return the next lower-priority rule in the list that is visible. Multiple
* identical rules with the same priority may exist transitionally. In that
- * case the first rule of a given priority has been marked as 'to_be_removed',
- * and the later rules are marked as '!visible'. This gets a bit complex if
- * there are two rules of the same pr
[ovs-dev] [PATCH v3 05/12] classifier: Make traversing identical rules robust.
The traversal of the list of identical rules from the lookup threads
is fragile in the list head is removed during the list traversal.
This patch simplifies the implementation of that list by making the
list NULL terminated, singly linked RCU-protected list. By having the
NULL at the end there is no longer a possiblity of missing the point
when the list wraps around. This is significant when there can be
multiple elements with the same priority in the list.
This change also decreases the size of the struct cls_match back
pre-'visibility' attribute size.
Signed-off-by: Jarno Rajahalme
---
lib/classifier-private.h | 106 +-
lib/classifier.c | 128 ++
tests/test-classifier.c |2 +-
3 files changed, 154 insertions(+), 82 deletions(-)
diff --git a/lib/classifier-private.h b/lib/classifier-private.h
index 51cb8cc..3b974f0 100644
--- a/lib/classifier-private.h
+++ b/lib/classifier-private.h
@@ -68,7 +68,12 @@ 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 "cls_match"es. */
+OVSRCU_TYPE(struct cls_match *) next; /* Identical "cls_match"es in the
+ * order of decreasing priority. At
+ * most one rule of a given priority
+ * may be visible to any given lookup
+ * version. */
+OVSRCU_TYPE(struct cls_conjunction_set *) conj_set;
/* Accessed only by writers. */
struct cls_partition *partition;
@@ -95,7 +100,6 @@ struct cls_match {
atomic_llong visibility;
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. */
};
@@ -136,6 +140,104 @@ cls_match_is_to_be_removed(const struct cls_match *rule)
return visibility <= 0;
}
+
+/* cls_match 'next' */
+
+static inline const struct cls_match *
+cls_match_next(const struct cls_match *rule)
+{
+return ovsrcu_get(struct cls_match *, &rule->next);
+}
+
+static inline struct cls_match *
+cls_match_next_protected(const struct cls_match *rule)
+{
+return ovsrcu_get_protected(struct cls_match *, &rule->next);
+}
+
+/* Puts 'rule' in the position between 'prev' and 'next'. If 'prev' == NULL,
+ * then the 'rule' is the new list head, and if 'next' == NULL, the rule is the
+ * new list tail. */
+static inline void
+cls_match_insert(struct cls_match *prev, struct cls_match *next,
+ struct cls_match *rule)
+{
+if (!prev) {
+/* 'rule' is the new head. */
+ovsrcu_set_hidden(&rule->next, next);
+} else {
+/* 'rule' is new node after 'prev' */
+ovsrcu_set_hidden(&rule->next, cls_match_next_protected(prev));
+ovsrcu_set(&prev->next, rule);
+}
+}
+
+void cls_match_poison(struct cls_match *);
+
+/* Puts 'new_rule' in the position of 'old_rule', which is the next node after
+ * 'prev'. If 'prev' == NULL, then the 'rule' is the new list head.
+ *
+ * Returns the replaced cls_match, which is not linked to any more, but still
+ * links to the later rules, and may still be referenced by other threads until
+ * all other threads quiesce. The returned rule may not be re-inserted,
+ * re-initialized, or deleted until after all other threads have quiesced (use
+ * ovsrcu_postpone). */
+static inline void
+cls_match_replace(struct cls_match *prev,
+ struct cls_match *old_rule, struct cls_match *new_rule)
+{
+if (prev == NULL) {
+/* 'new_rule' replaces the old head. */
+ovsrcu_set_hidden(&new_rule->next, cls_match_next_protected(old_rule));
+} else {
+/* 'new_rule' replaces an existing non-head node. */
+struct cls_match *next = cls_match_next_protected(old_rule);
+
+ovsrcu_set_hidden(&new_rule->next, next);
+ovsrcu_set(&prev->next, new_rule);
+}
+
+#ifndef NDEBUG
+ovsrcu_postpone(cls_match_poison, old_rule);
+#endif
+}
+
+/* Removes 'rule' following 'prev' from the list. If 'prev' is NULL, then the
+ * 'rule' is a list head, and the 'rule' is only poisoned after all threads
+ * have quiesced.
+ *
+ * Afterward, the removed rule is not linked to any more, but still links to
+ * the following rules, and may still be referenced by other threads until all
+ * other threads quiesce. The removed rule may not be re-inserted,
+ * re-initialized, or deleted until after all other threads have quiesced (use
+ * ovsrcu_postpone).
+ */
+static inline void
+cls_match_remove(struct cls_match *prev, struct cls_match *rule)
+{
+if (prev) {
+struct cls_match *next = cls_match_next_protected(rule);
+
+ovsrcu_set(&prev->next, next);
+}
+#
[ovs-dev] [PATCH v3 06/12] ofproto: Infra for table versioning.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto-dpif-xlate.c | 17 -
ofproto/ofproto-dpif.c | 43 +-
ofproto/ofproto-dpif.h |5 -
ofproto/ofproto-provider.h |6 ++
ofproto/ofproto.c|5 +
5 files changed, 65 insertions(+), 11 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 71b8bef..9161030 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -159,6 +159,9 @@ struct xlate_ctx {
const struct xbridge *xbridge;
+/* Flow tables version at the beginning of the translation. */
+long long tables_version;
+
/* Flow at the last commit. */
struct flow base_flow;
@@ -2774,6 +2777,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t
ofp_port,
const struct xport *peer = xport->peer;
struct flow old_flow = ctx->xin->flow;
bool old_was_mpls = ctx->was_mpls;
+long long old_version = ctx->tables_version;
enum slow_path_reason special;
struct ofpbuf old_stack = ctx->stack;
union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
@@ -2789,6 +2793,10 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
memset(flow->regs, 0, sizeof flow->regs);
flow->actset_output = OFPP_UNSET;
+/* Set tables version after the bridge has been fixed. */
+ctx->tables_version
+= ofproto_dpif_get_tables_version(ctx->xbridge->ofproto);
+
special = process_special(ctx, &ctx->xin->flow, peer,
ctx->xin->packet);
if (special) {
@@ -2835,6 +2843,9 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t
ofp_port,
ofpbuf_uninit(&ctx->stack);
ctx->stack = old_stack;
+/* Restore calling bridge's lookup version. */
+ctx->tables_version = old_version;
+
/* The peer bridge popping MPLS should have no effect on the original
* bridge. */
ctx->was_mpls = old_was_mpls;
@@ -3056,6 +3067,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t
in_port, uint8_t table_id,
wc = (ctx->xin->skip_wildcards) ? NULL : &ctx->xout->wc;
rule = rule_dpif_lookup_from_table(ctx->xbridge->ofproto,
+ ctx->tables_version,
&ctx->xin->flow, wc,
ctx->xin->xcache != NULL,
ctx->xin->resubmit_stats,
@@ -4826,9 +4838,12 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
flow->recirc_id);
return;
}
+/* Set tables version after the bridge has been fixed. */
+ctx.tables_version = ofproto_dpif_get_tables_version(ctx.xbridge->ofproto);
if (!xin->ofpacts && !ctx.rule) {
-rule = rule_dpif_lookup_from_table(ctx.xbridge->ofproto, flow, wc,
+rule = rule_dpif_lookup_from_table(ctx.xbridge->ofproto,
+ ctx.tables_version, flow, wc,
ctx.xin->xcache != NULL,
ctx.xin->resubmit_stats,
&ctx.table_id,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 83ba003..632dcc3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -287,6 +287,8 @@ struct ofproto_dpif {
struct ofproto up;
struct dpif_backer *backer;
+atomic_llong tables_version; /* Version # to use in classifier lookups. */
+
uint64_t dump_seq; /* Last read of udpif_dump_seq(). */
/* Special OpenFlow rules. */
@@ -1227,6 +1229,7 @@ construct(struct ofproto *ofproto_)
return error;
}
+atomic_init(&ofproto->tables_version, CLS_NO_VERSION);
ofproto->netflow = NULL;
ofproto->sflow = NULL;
ofproto->ipfix = NULL;
@@ -1605,6 +1608,15 @@ query_tables(struct ofproto *ofproto,
}
}
+static void
+set_tables_version(struct ofproto *ofproto_, long long version)
+{
+struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
+
+atomic_store_relaxed(&ofproto->tables_version, version);
+}
+
+
static struct ofport *
port_alloc(void)
{
@@ -3701,6 +3713,15 @@ rule_set_recirc_id(struct rule *rule_, uint32_t id)
ovs_mutex_unlock(&rule->up.mutex);
}
+long long
+ofproto_dpif_get_tables_version(struct ofproto_dpif *ofproto)
+{
+long long version;
+
+atomic_read_relaxed(&ofproto->tables_version, &version);
+return version;
+}
+
/* The returned rule (if any) is valid at least until the next RCU quiescent
* period. If the rule needs to stay around longer, a non-zero 'take_ref'
* must be passed in to cause a reference to be taken on it.
@@ -3708,16 +3729,16 @@ rule_set_recirc_id(struct rule *rule_, uint32_t id)
* 'flow' is non-
[ovs-dev] [PATCH v3 09/12] ofproto: Revertible eviction.
Handling evictions was broken in the previous patches. Eviction took
place early in the commit, and actually inappropriately bumped the
version number too early. Now eviction is treated much like a flow
modification, where a new rule replaces the old one, but just without
any 'inheritance' from the evicted rule to the new rule. This makes
evictions to be executed only when commit is successful, as evictions
are reverted like any other changes when the commit fails.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto.c | 122 +
1 file changed, 77 insertions(+), 45 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 2bcdf73..7416699 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -81,8 +81,7 @@ static void oftable_destroy(struct oftable *);
static void oftable_set_name(struct oftable *, const char *name);
-static enum ofperr evict_rules_from_table(struct oftable *,
- unsigned int extra_space)
+static enum ofperr evict_rules_from_table(struct oftable *)
OVS_REQUIRES(ofproto_mutex);
static void oftable_disable_eviction(struct oftable *);
static void oftable_enable_eviction(struct oftable *,
@@ -264,8 +263,8 @@ static void replace_rule_start(struct ofproto *,
struct cls_conjunction *, size_t n_conjs)
OVS_REQUIRES(ofproto_mutex);
-static void replace_rule_revert(struct ofproto *,
-struct rule *old_rule, struct rule *new_rule)
+static void replace_rule_revert(struct ofproto *, struct rule *old_rule,
+struct rule *new_rule)
OVS_REQUIRES(ofproto_mutex);
static void replace_rule_finish(struct ofproto *, struct ofputil_flow_mod *,
@@ -1432,7 +1431,7 @@ ofproto_configure_table(struct ofproto *ofproto, int
table_id,
}
ovs_mutex_lock(&ofproto_mutex);
-evict_rules_from_table(table, 0);
+evict_rules_from_table(table);
ovs_mutex_unlock(&ofproto_mutex);
}
@@ -4350,12 +4349,12 @@ handle_queue_stats_request(struct ofconn *ofconn,
}
static enum ofperr
-evict_rules_from_table(struct oftable *table, unsigned int extra_space)
+evict_rules_from_table(struct oftable *table)
OVS_REQUIRES(ofproto_mutex)
{
enum ofperr error = 0;
struct rule_collection rules;
-unsigned int count = table->n_flows + extra_space;
+unsigned int count = table->n_flows;
unsigned int max_flows = table->max_flows;
rule_collection_init(&rules);
@@ -4490,10 +4489,17 @@ add_flow_start(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
}
/* If necessary, evict an existing rule to clear out space. */
-error = evict_rules_from_table(table, 1);
-if (error) {
-cls_rule_destroy(&cr);
-return error;
+if (table->n_flows >= table->max_flows) {
+if (!choose_rule_to_evict(table, &rule)) {
+error = OFPERR_OFPFMFC_TABLE_FULL;
+cls_rule_destroy(&cr);
+return error;
+}
+eviction_group_remove_rule(rule);
+/* Marks '*old_rule' as an evicted rule rather than replaced rule.
+ */
+fm->delete_reason = OFPRR_EVICTION;
+*old_rule = rule;
}
} else {
fm->modify_cookie = true;
@@ -4513,13 +4519,17 @@ add_flow_start(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
return 0;
}
-/* Revert the effects of add_flow_start().
- * XXX: evictions cannot be reverted. */
+/* Revert the effects of add_flow_start(). */
static void
-add_flow_revert(struct ofproto *ofproto, struct rule *old_rule,
-struct rule *new_rule)
+add_flow_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+struct rule *old_rule, struct rule *new_rule)
OVS_REQUIRES(ofproto_mutex)
{
+if (old_rule && fm->delete_reason == OFPRR_EVICTION) {
+/* Revert the eviction. */
+eviction_group_add_rule(old_rule);
+}
+
replace_rule_revert(ofproto, old_rule, new_rule);
}
@@ -4606,7 +4616,7 @@ replace_rule_create(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
rule->version = ofproto->tables_version + 1; /* Originally visible in the
* next version */
/* Copy values from old rule for modify semantics. */
-if (old_rule) {
+if (old_rule && fm->delete_reason != OFPRR_EVICTION) {
/* 'fm' says that */
bool change_cookie = (fm->modify_cookie
&& fm->new_cookie != OVS_BE64_MAX
@@ -4645,6 +4655,7 @@ replace_rule_start(struct ofproto *ofproto,
{
struct oftable *table = &ofproto->tables[new_rule->table_id];
+/* 'old_rule' may be either an evicted rule or replaced rule. */
if (old_rule) {
/* Mark the old rule for removal in the next version. */
cls_rule_make_removable_aft
[ovs-dev] [PATCH v3 07/12] Use classifier versioning.
Each rule is now added or deleted in a specific tables version. Flow
tables are versioned with a monotonically increasing 64-bit integer,
where positive values are valid version numbers.
Rule modifications are implemented as an insertion of a new rule and a
deletion of the old rule, both taking place in the same tables
version. Since concurrent lookups may use different versions, both
the old and new rule must be available for lookups at the same time.
The ofproto provider interface is changed to accomodate the above. As
rule's actions need not be modified any more, we no longer need
'rule_premodify_actions', nor 'rule_modify_actions'. 'rule_insert'
now takes a pointer to the old rule and adds a flag that tells whether
the old stats should be forwarded to the new rule or not (this
replaces the 'reset_counters' flag of the now removed
'rule_modify_actions').
Versioning all flow table changes has the side effect of making
learned flows visible for future lookups only. I.e., the upcall that
executes the learn action, will not see the newly learned action in
it's classifier lookups. Only upcalls that start executing after the
new flow was added will match on it.
Classifier versioning only affects the classifier lookups. Classifier
iterators and other control functions operating on the classifier will
always see the latest version of the classifier.
Signed-off-by: Jarno Rajahalme
---
NEWS | 21 +-
ofproto/bundles.h |5 +-
ofproto/ofproto-dpif.c | 87 +++--
ofproto/ofproto-provider.h | 59 +--
ofproto/ofproto.c | 907
tests/ofproto.at | 42 +-
tests/ovs-ofctl.at | 40 +-
utilities/ovs-ofctl.8.in | 19 +-
utilities/ovs-ofctl.c |4 +-
9 files changed, 636 insertions(+), 548 deletions(-)
diff --git a/NEWS b/NEWS
index 5bea237..f171cfc 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,10 @@
Post-v2.3.0
-
+ - Flow table modifications are now atomic, meaning that each packet
+ now sees a coherent version of the OpenFlow pipeline. For
+ example, if a controller removes all flows, no packet sees an
+ intermediate version of the OpenFlow pipeline where only some of
+ the flows have been deleted.
- Added support for SFQ, FQ_CoDel and CoDel qdiscs.
- Add bash command-line completion support for ovs-vsctl Please check
utilities/ovs-command-compgen.INSTALL.md for how to use.
@@ -28,10 +33,10 @@ Post-v2.3.0
release. See ovs-vswitchd(8) for details.
- OpenFlow:
* OpenFlow 1.4 bundles are now supported, but for flow mod
- messages only. 'atomic' bundles are not yet supported, and
- 'ordered' bundles are trivially supported, as all bundled
- messages are executed in the order they were added to the
- bundle regardless of the presence of the 'ordered' flag.
+ messages only. Both 'atomic' and 'ordered' bundle flags are
+ trivially supported, as all bundled messages are executed in
+ the order they were added and all flow table modifications are
+ now atomic to the datapath.
* IPv6 flow label and neighbor discovery fields are now modifiable.
* OpenFlow 1.5 extended registers are now supported.
* The OpenFlow 1.5 actset_output field is now supported.
@@ -49,15 +54,15 @@ Post-v2.3.0
- ovs-ofctl has a new '--bundle' option that makes the flow mod commands
('add-flow', 'add-flows', 'mod-flows', 'del-flows', and 'replace-flows')
use an OpenFlow 1.4 bundle to operate the modifications as a single
- transaction. If any of the flow mods in a transaction fail, none of
- them are executed.
+ atomic transaction. If any of the flow mods in a transaction fail, none
+ of them are executed. All flow mods in a bundle appear to datapath
+ lookups simultaneously.
- ovs-ofctl 'add-flow' and 'add-flows' commands now accept arbitrary flow
mods as an input by allowing the flow specification to start with an
explicit 'add', 'modify', 'modify_strict', 'delete', or 'delete_strict'
keyword. A missing keyword is treated as 'add', so this is fully
backwards compatible. With the new '--bundle' option all the flow mods
- are executed as a single transaction using the new OpenFlow 1.4 bundles
- support.
+ are executed as a single atomic transaction using an OpenFlow 1.4 bundle.
- ovs-pki: Changed message digest algorithm from MD5 to SHA-1 because
MD5 is no longer secure and some operating systems have started to disable
it in OpenSSL.
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 778cba2..0c7daf2 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -40,9 +40,8 @@ struct ofp_bundle_entry {
};
/* Used during commit. */
-struct rule_collection rules; /* Affected rules. */
-struct rule *rule;
-bool modify;
+struct rule_collection old_rules; /* Affect
[ovs-dev] [PATCH v3 10/12] ofproto: Postpone sending flow removed messages.
The final flow stats are available only after there are no references
to the rule. Postpone sending the flow removed message until the
final stats are available.
Signed-off-by: Jarno Rajahalme
---
include/openflow/openflow-common.h |2 ++
lib/ofp-print.c|1 +
ofproto/ofproto-provider.h |5 +
ofproto/ofproto.c | 37 +---
4 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/include/openflow/openflow-common.h
b/include/openflow/openflow-common.h
index e4fecce..d32213f 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -301,6 +301,8 @@ enum ofp_flow_removed_reason {
OFPRR_GROUP_DELETE, /* Group was removed. */
OFPRR_METER_DELETE, /* Meter was removed. */
OFPRR_EVICTION, /* Switch eviction to free resources. */
+
+OVS_OFPRR_NONE /* OVS internal_use only, keep last!. */
};
/* What changed about the physical port */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index d773dca..929594c 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -900,6 +900,7 @@ ofp_flow_removed_reason_to_string(enum
ofp_flow_removed_reason reason,
return "eviction";
case OFPRR_METER_DELETE:
return "meter_delete";
+case OVS_OFPRR_NONE:
default:
snprintf(reasonbuf, bufsize, "%d", (int) reason);
return reasonbuf;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7fbad00..d37b991 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -354,6 +354,11 @@ struct rule {
/* Eviction precedence. */
uint16_t importance OVS_GUARDED;
+/* Removal reason for sending flow removed message.
+ * Used only if 'flags' has OFPUTIL_FF_SEND_FLOW_REM set and if the
+ * value is not OVS_OFPRR_NONE. */
+uint8_t removed_reason;
+
/* Eviction groups (see comment on struct eviction_group for explanation) .
*
* 'eviction_group' is this rule's eviction group, or NULL if it is not in
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7416699..8ae472b 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -231,7 +231,8 @@ struct ofport_usage {
};
/* rule. */
-static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
+static void ofproto_rule_send_removed(struct rule *)
+OVS_EXCLUDED(ofproto_mutex);
static bool rule_is_readonly(const struct rule *);
static void ofproto_rule_insert__(struct ofproto *, struct rule *)
OVS_REQUIRES(ofproto_mutex);
@@ -2752,7 +2753,14 @@ ofproto_rule_destroy__(struct rule *rule)
static void
rule_destroy_cb(struct rule *rule)
+OVS_NO_THREAD_SAFETY_ANALYSIS
{
+/* Send rule removed if needed. */
+if (rule->flags & OFPUTIL_FF_SEND_FLOW_REM
+&& rule->removed_reason != OVS_OFPRR_NONE
+&& !rule_is_hidden(rule)) {
+ofproto_rule_send_removed(rule);
+}
rule->ofproto->ofproto_class->rule_destruct(rule);
ofproto_rule_destroy__(rule);
}
@@ -4602,6 +4610,7 @@ replace_rule_create(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
rule->idle_timeout = fm->idle_timeout;
rule->hard_timeout = fm->hard_timeout;
rule->importance = fm->importance;
+rule->removed_reason = OVS_OFPRR_NONE;
*CONST_CAST(uint8_t *, &rule->table_id) = table_id;
rule->flags = fm->flags & OFPUTIL_FF_STATE;
@@ -4752,9 +4761,7 @@ replace_rule_finish(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
} else {
/* XXX: This is slight duplication with delete_flows_finish__() */
-/* XXX: This call should done when rule's refcount reaches
- * zero to get accurate stats in the flow removed message. */
-ofproto_rule_send_removed(old_rule, OFPRR_EVICTION);
+old_rule->removed_reason = OFPRR_EVICTION;
ofmonitor_report(ofproto->connmgr, old_rule, NXFME_DELETED,
OFPRR_EVICTION,
@@ -4957,7 +4964,10 @@ delete_flows_finish__(struct ofproto *ofproto,
for (size_t i = 0; i < rules->n; i++) {
struct rule *rule = rules->rules[i];
-ofproto_rule_send_removed(rule, reason);
+/* This value will be used to send the flow removed message right
+ * before the rule is actually destroyed. */
+rule->removed_reason = reason;
+
ofmonitor_report(ofproto->connmgr, rule, NXFME_DELETED, reason,
req ? req->ofconn : NULL,
req ? req->request->xid : 0, NULL);
@@ -5071,23 +5081,20 @@ delete_flow_start_strict(struct ofproto *ofproto,
return error;
}
-/* XXX: This should be sent right when the rule refcount gets to zero! */
+/* This may only be called by rule_destroy_cb()! */
static void
-ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
-OVS_REQUIRES(ofproto_mutex)
+ofpr
[ovs-dev] [PATCH v3 08/12] ofproto: Accurate flow counts.
Classifier's rule count now contains temporary duplicates and rules
whose deletion has been deferred. Maintain a new 'n_flows' count in
struct oftable to as the count of rules in the latest version.
Signed-off-by: Jarno Rajahalme
---
ofproto/connmgr.c |2 +-
ofproto/ofproto-provider.h |5 -
ofproto/ofproto.c | 38 ++
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 495364f..519f2b0 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2027,7 +2027,7 @@ connmgr_flushed(struct connmgr *mgr)
/* 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
+ * rules in the table 0 classifier, as maintained in struct oftable, yields
* the number of flows that OVS should report via OpenFlow for table 0.) */
int
connmgr_count_hidden_rules(const struct connmgr *mgr)
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 6123adc..7fbad00 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -220,6 +220,9 @@ struct oftable {
/* Maximum number of flows or UINT_MAX if there is no limit besides any
* limit imposed by resource limitations. */
unsigned int max_flows;
+/* Current number of flows, not counting temporary duplicates nor deferred
+ * deletions. */
+unsigned int n_flows;
/* These members determine the handling of an attempt to add a flow that
* would cause the table to have more than 'max_flows' flows.
@@ -818,7 +821,7 @@ struct ofproto_class {
*
* - 'table_id' to the array index.
*
- * - 'active_count' to the classifier_count() for the table.
+ * - 'active_count' to the 'n_flows' of struct ofproto for the table.
*
* - 'lookup_count' and 'matched_count' to 0.
*
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c805357..2bcdf73 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1693,12 +1693,11 @@ ofproto_run(struct ofproto *p)
continue;
}
-if (classifier_count(&table->cls) > 10) {
+if (table->n_flows > 10) {
static struct vlog_rate_limit count_rl =
VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&count_rl, "Table %"PRIuSIZE" has an excessive"
- " number of rules: %d", i,
- classifier_count(&table->cls));
+ " number of rules: %d", i, table->n_flows);
}
ovs_mutex_lock(&ofproto_mutex);
@@ -1792,7 +1791,7 @@ ofproto_get_memory_usage(const struct ofproto *ofproto,
struct simap *usage)
n_rules = 0;
OFPROTO_FOR_EACH_TABLE (table, ofproto) {
-n_rules += classifier_count(&table->cls);
+n_rules += table->n_flows;
}
simap_increase(usage, "rules", n_rules);
@@ -3152,10 +3151,9 @@ query_tables(struct ofproto *ofproto,
stats = *statsp = xcalloc(ofproto->n_tables, sizeof *stats);
for (i = 0; i < ofproto->n_tables; i++) {
struct ofputil_table_stats *s = &stats[i];
-struct classifier *cls = &ofproto->tables[i].cls;
s->table_id = i;
-s->active_count = classifier_count(cls);
+s->active_count = ofproto->tables[i].n_flows;
if (i == 0) {
s->active_count -= connmgr_count_hidden_rules(
ofproto->connmgr);
@@ -4357,7 +4355,7 @@ evict_rules_from_table(struct oftable *table, unsigned
int extra_space)
{
enum ofperr error = 0;
struct rule_collection rules;
-unsigned int count = classifier_count(&table->cls) + extra_space;
+unsigned int count = table->n_flows + extra_space;
unsigned int max_flows = table->max_flows;
rule_collection_init(&rules);
@@ -4651,6 +4649,8 @@ replace_rule_start(struct ofproto *ofproto,
/* Mark the old rule for removal in the next version. */
cls_rule_make_removable_after_version(&old_rule->cr,
ofproto->tables_version);
+} else {
+table->n_flows++;
}
/* Insert flow to the classifier, so that later flow_mods may relate
* to it. This is reversible, in case later errors require this to
@@ -4667,10 +4667,15 @@ static void replace_rule_revert(struct ofproto *ofproto,
{
struct oftable *table = &ofproto->tables[new_rule->table_id];
-/* Restore the original visibility of the old rule if it
- * was inserted before this transaction. */
-if (old_rule && old_rule->version <= ofproto->tables_version) {
-cls_rule_restore_version(&old_rule->cr, old_rule->version);
+if (old_rule) {
+/* Restore the original visibility of the old r
[ovs-dev] [PATCH v3 11/12] ofproto: Use minimatch for making bundles smaller.
struct match in ofputil_flow_mod uses a lot of space, when used for a
stored bundle message. This patch adds a new struct
ofputil_miniflow_mod, that uses a minimatch instead of match in hopes
of using less memory when handling large bundles.
Signed-off-by: Jarno Rajahalme
---
lib/match.c|9 +++
lib/match.h|1 +
lib/ofp-util.c | 14
lib/ofp-util.h | 100 +--
ofproto/bundles.h |7 +-
ofproto/ofproto-provider.h |2 +-
ofproto/ofproto.c | 192 +++-
7 files changed, 209 insertions(+), 116 deletions(-)
diff --git a/lib/match.c b/lib/match.c
index 7d0b409..a56a306 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1193,6 +1193,15 @@ minimatch_init(struct minimatch *dst, const struct match
*src)
miniflow_init_with_minimask(&dst->flow, &src->flow, &dst->mask);
}
+/* Initializes 'match' as a "catch-all" match that matches every packet. */
+void
+minimatch_init_catchall(struct minimatch *match)
+{
+match->mask.masks.map = match->flow.map = 0;
+match->mask.masks.values_inline = true;
+match->flow.values_inline = true;
+}
+
/* Initializes 'dst' as a copy of 'src'. The caller must eventually free 'dst'
* with minimatch_destroy(). */
void
diff --git a/lib/match.h b/lib/match.h
index 6633304..822ab3f 100644
--- a/lib/match.h
+++ b/lib/match.h
@@ -170,6 +170,7 @@ struct minimatch {
};
void minimatch_init(struct minimatch *, const struct match *);
+void minimatch_init_catchall(struct minimatch *);
void minimatch_clone(struct minimatch *, const struct minimatch *);
void minimatch_move(struct minimatch *dst, struct minimatch *src);
void minimatch_destroy(struct minimatch *);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 9c0bb2d..56c70cf 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1839,6 +1839,20 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
fm->table_id, max_table, protocol);
}
+void
+ofputil_miniflow_mod_init(struct ofputil_miniflow_mod *mfm,
+ const struct ofputil_flow_mod *fm)
+{
+memcpy(mfm, fm, offsetof(struct ofputil_miniflow_mod, match));
+minimatch_init(&mfm->match, &fm->match);
+}
+
+void
+ofputil_miniflow_mod_uninit(struct ofputil_miniflow_mod *mfm)
+{
+minimatch_destroy(&mfm->match);
+}
+
static enum ofperr
ofputil_pull_bands(struct ofpbuf *msg, size_t len, uint16_t *n_bands,
struct ofpbuf *bands)
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 644a679..ff7a1d4 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -270,51 +270,59 @@ enum ofputil_flow_mod_flags {
*
* The handling of cookies across multiple versions of OpenFlow is a bit
* confusing. See DESIGN for the details. */
-struct ofputil_flow_mod {
-struct ovs_list list_node; /* For queuing flow_mods. */
-struct match match;
+/* We need this same layout in two places. Ideally this would be a struct, but
+ * then all access to these would become uglier. This is one of the rare
+ * instances where I hope we allowed some elemeentary C++ in the code base.
+ * This define is somewhat ugly, but avoids repeating these in multiple places
+ * and the related maintenance problems. */
+#define OFPUTIL_FLOW_MOD_DATA \
+/* Cookie matching. The flow_mod affects only flows that have \
+ * cookies that bitwise match 'cookie' bits in positions where \
+ * 'cookie_mask' has 1-bits.\
+ * 'cookie_mask' should be zero for OFPFC_ADD flow_mods. */ \
+ovs_be64 cookie; /* Cookie bits to match. */\
+ovs_be64 cookie_mask;/* 1-bit in each 'cookie' bit to match. */ \
+\
+/* Cookie changes. \
+ * \
+ * OFPFC_ADD uses 'new_cookie' as the new flow's cookie.\
+ * 'new_cookie' should not be UINT64_MAX. \
+ * \
+ * OFPFC_MODIFY and OFPFC_MODIFY_STRICT have two cases: \
+ * \
+ * - If one or more matching flows exist and 'modify_cookie' is \
+ * true, then the flow_mod changes the existing flows' cookies \
+ * to 'new_cookie'. 'new_cookie' should not be UINT64_MAX. \
+ * \
+ * - If no matching flow exists, 'new_cookie' is not UINT64_MAX, \
+ * and 'cookie_mask' is 0, then the flow_mod adds a new flow\
+ * with 'new_cookie' as its cookie. \
+ */
[ovs-dev] [PATCH v3 12/12] ofproto: Support port mods in bundles.
Add support for port mods in an OpenFlow 1.4 bundle, as required for
the minimum support level by the OpenFlow 1.4 specification. If the
bundle includes port mods, it may not specify the OFPBF_ATOMIC flag.
Port mods and flow mods in a bundle are always applied in order and
the consecutive flow mods between port mods are made available to
lookups atomically.
Note that ovs-ofctl does not support creating bundles with port mods.
Signed-off-by: Jarno Rajahalme
---
NEWS | 11 +++---
OPENFLOW-1.1+.md |6 +++
ofproto/bundles.h | 12 --
ofproto/ofproto.c | 106 +++--
4 files changed, 99 insertions(+), 36 deletions(-)
diff --git a/NEWS b/NEWS
index f171cfc..c3524b6 100644
--- a/NEWS
+++ b/NEWS
@@ -32,11 +32,12 @@ Post-v2.3.0
commands are now redundant and will be removed in a future
release. See ovs-vswitchd(8) for details.
- OpenFlow:
- * OpenFlow 1.4 bundles are now supported, but for flow mod
- messages only. Both 'atomic' and 'ordered' bundle flags are
- trivially supported, as all bundled messages are executed in
- the order they were added and all flow table modifications are
- now atomic to the datapath.
+ * OpenFlow 1.4 bundles are now supported for flow mods and port
+ mods. For flow mods, both 'atomic' and 'ordered' bundle flags
+ are trivially supported, as all bundled messages are executed
+ in the order they were added and all flow table modifications
+ are now atomic to the datapath. Port mods may not appear in
+ atomic bundles, as port status modifications are not atomic.
* IPv6 flow label and neighbor discovery fields are now modifiable.
* OpenFlow 1.5 extended registers are now supported.
* The OpenFlow 1.5 actset_output field is now supported.
diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
index 7911406..0b2f0f9 100644
--- a/OPENFLOW-1.1+.md
+++ b/OPENFLOW-1.1+.md
@@ -148,6 +148,12 @@ parallel in OVS.
Transactional modification. OpenFlow 1.4 requires to support
flow_mods and port_mods in a bundle if bundle is supported.
(Not related to OVS's 'ofbundle' stuff.)
+Implemented as an OpenFlow 1.4 feature. Only flow_mods and
+port_mods are supported in a bundle. If the bundle includes port
+mods, it may not specify the OFPBF_ATOMIC flag. Nevertheless,
+port mods and flow mods in a bundle are always applied in order
+and consecutive flow mods between port mods are made available to
+lookups atomically.
[EXT-230]
[optional for OF1.4+]
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 98cb2ba..3723a5d 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -34,14 +34,17 @@ extern "C" {
struct ofp_bundle_entry {
struct ovs_list node;
enum ofptype type; /* OFPTYPE_FLOW_MOD or OFPTYPE_PORT_MOD. */
+long long version; /* Version in which the changes take
+ * effect. */
union {
struct ofputil_miniflow_mod fm; /* 'fm.ofpacts' must be malloced. */
struct ofputil_port_mod pm;
};
/* Used during commit. */
+struct ofport *port;/* Affected port. */
struct rule_collection old_rules; /* Affected rules. */
-struct rule_collection new_rules; /* Affected rules. */
+struct rule_collection new_rules; /* Replacement rules. */
/* OpenFlow header and some of the message contents for error reporting. */
struct ofp_header ofp_msg[DIV_ROUND_UP(64, sizeof(struct ofp_header))];
@@ -63,7 +66,7 @@ struct ofp_bundle {
};
static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc(
-enum ofptype type, const struct ofp_header *oh);
+enum ofptype type, long long version, const struct ofp_header *oh);
static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *);
enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags);
@@ -75,7 +78,8 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t
id,
void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *, bool success);
static inline struct ofp_bundle_entry *
-ofp_bundle_entry_alloc(enum ofptype type, const struct ofp_header *oh)
+ofp_bundle_entry_alloc(enum ofptype type, long long version,
+ const struct ofp_header *oh)
{
struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
@@ -83,6 +87,8 @@ ofp_bundle_entry_alloc(enum ofptype type, const struct
ofp_header *oh)
if (type == OFPTYPE_FLOW_MOD) {
minimatch_init_catchall(&entry->fm.match);
}
+entry->version = version;
+
/* Max 64 bytes for error reporting. */
memcpy(entry->ofp_msg, oh, MIN(ntohs(oh->length), sizeof entry->ofp_msg));
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a07b0fe..7df7bef 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3436,9 +3436,34 @@ update_port_config(struct ofcon
Re: [ovs-dev] [PATCH v2 16/19] ovs-ofctl: Add 'bundle' command and unit testing.
> On Jun 5, 2015, at 12:55 PM, Jarno Rajahalme wrote: > >> This will be really nice, thanks for implementing it. >> >> (Now, to find out why there are three more commits...) > > > Will post a v3 with the rest of the series soon. v3 posted! Have a nice weekend, Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6] This commit adds the windows installer to the OVS tree.
Rebased and added the sign off: https://github.com/openvswitch/ovs/pull/50 Alin. -Mesaj original- De la: dev [mailto:dev-boun...@openvswitch.org] În numele Alin Serdean Trimis: Friday, June 5, 2015 9:20 PM Către: Gurucharan Shetty Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH v6] This commit adds the windows installer to the OVS tree. Sure. Alin. -Mesaj original- De la: Gurucharan Shetty [mailto:shet...@nicira.com] Trimis: Friday, June 5, 2015 5:57 PM Către: Alin Serdean Cc: dev@openvswitch.org; Nithin Raju; Eitan Eliahu Subiect: Re: [PATCH v6] This commit adds the windows installer to the OVS tree. Alin, I do not see Alessandro's signed-off. Can you please add. On Fri, Jun 5, 2015 at 5:16 AM, Alin Serdean wrote: > Sorry but unfortunately I could not get the patch on the ML. > Created the following pull request instead: > https://github.com/openvswitch/ovs/pull/49 . > > Thank you, > Alin. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] 答复： [PATCH] netdev-dpdk: Do not flush tx queue which is shared among CPUs since it is always flushed
My environment is in VMware
OS: CentOS 7ovs: compiled by git masterDPDK: 2.0.0qemu:2.3.0
qemu conecting ovs by vhost-userOne NIC bind to ovs
When I send pkts from qemu to VMware`s NIC by using iperf3, Segmentation falt
occurs
Do you need to see the backtrace? I forget to save it, if need, I will repeat
it
*RTFSC*--发件人：Gray,
Mark D 发送时间：2015年6月5日(星期五) 20:27收件人：钢锁0310
，d...@openvswitch.com 主 题：RE: [ovs-dev]
[PATCH] netdev-dpdk: Do not flush tx queue which isshared among CPUs
since it is always flushed> > When tx queue is shared among CPUS,the pkts
always be flush in> 'netdev_dpdk_eth_send'> So it is unnecessarily for flushing
in netdev_dpdk_rxq_recv Otherwise tx will> be accessed without lockingAre you
seeing a specific bug or is this just to account for a device withless queues
than pmds?> > Signed-off-by: Wei li > ---> lib/netdev-dpdk.c
| 7 +--> 1 file changed, 5 insertions(+), 2 deletions(-)> > diff --git
a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 63243d8..25e3a73> 100644> ---
a/lib/netdev-dpdk.c> +++ b/lib/netdev-dpdk.c> @@ -892,8 +892,11 @@
netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,> struct dp_packet **packets,>
int nb_rx;> > /* There is only one tx queue for this core. Do not flush
other> - * queueus. */> -if (rxq_->queue_id == rte_lcore_id()) {> +
* queueus.s/queueus/queues> + * Do not flush tx queue which is shared among
CPUs> + * since it is always flushed */> +if (rxq_->queue_id ==
rte_lcore_id() &&> + OVS_LIKELY(!dev->txq_needs_locking)) {>
dpdk_queue_flush(dev, rxq_->queue_id);Do you see any drop in performance in a
simple phy-phy case beforeand after this patch?> }> > --> 1.9.5.msysgit.1>
> > ___> dev mailing list>
dev@openvswitch.org> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] How to fix the layout of email? Anyone help?
Layout of my Email is
wrong*RTFSC*--From:钢锁0310
Time:2015 Jun 6 (Sat) 08:30To:Gray, Mark D
, d...@openvswitch.com
Subject:[ovs-dev] 答复： [PATCH] netdev-dpdk: Do not flush
tx queue which is shared among CPUs since it is always flushedMy
environment is in VMwareOS: CentOS 7ovs: compiled by git masterDPDK:
2.0.0qemu:2.3.0qemu conecting ovs by vhost-userOne NIC bind to ovs When I send
pkts from qemu to VMware`s NIC by using iperf3, Segmentation falt occursDo you
need to see the backtrace? I forget to save it, if need, I will repeat
it*RTFSC*--发件人：Gray,
Mark D 发送时间：2015年6月5日(星期五) 20:27收件人：钢锁0310
，d...@openvswitch.com 主 题：RE: [ovs-dev]
[PATCH] netdev-dpdk: Do not flush tx queue which is shared among CPUs since it
is always flushed> > When tx queue is shared among CPUS,the pkts always be
flush in> 'netdev_dpdk_eth_send'> So it is unnecessarily for flushing in
netdev_dpdk_rxq_recv Otherwise tx will> be accessed without lockingAre you
seeing a specific bug or is this just to account for a device withless queues
than pmds?> > Signed-off-by: Wei li > ---> lib/netdev-dpdk.c
| 7 +--> 1 file changed, 5 insertions(+), 2 deletions(-)> > diff --git
a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 63243d8..25e3a73> 100644> ---
a/lib/netdev-dpdk.c> +++ b/lib/netdev-dpdk.c> @@ -892,8 +892,11 @@
netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,> struct dp_packet **packets,>
int nb_rx;> > /* There is only one tx queue for this core. Do not flush
other> - * queueus. */> -if (rxq_->queue_id == rte_lcore_id()) {> +
* queueus.s/queueus/queues> + * Do not flush tx queue which is shared among
CPUs> + * since it is always flushed */> +if (rxq_->queue_id ==
rte_lcore_id() &&> + OVS_LIKELY(!dev->txq_needs_locking)) {>
dpdk_queue_flush(dev, rxq_->queue_id);Do you see any drop in performance in a
simple phy-phy case beforeand after this patch?> }> > --> 1.9.5.msysgit.1>
> > ___> dev mailing list>
dev@openvswitch.org>
http://openvswitch.org/mailman/listinfo/dev___dev
mailing listdev@openvswitch.orghttp://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] error
Message could not be delivered ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 00/12] Atomic bundles with classifier versioning.
> On Jun 5, 2015, at 15:08, Jarno Rajahalme wrote: > (Snip) > > - Bundles now support also port mods, but only for non-atomic bundles. > The reason for this is that we have Add "no" here. > easy way of making port status > changes atomic w.r.t. classifier version number changes (patch 12). > > For correctness, patches 07, 08, and 09 need to be squashed before > committing, but these are separated in this series to make reviewing > easier (and as a test for Ben for either reading this text or noticing > the problems while reading patch 07 :-). > > Jarno Rajahalme (12): > ofp-parse: Use F_OUT_PORT when parsing. > ofproto: Rename *_begin functions as *_start. > ovs-ofctl: Add bundle support and unit testing. > classifier: Support table versioning > classifier: Make traversing identical rules robust. > ofproto: Infra for table versioning. > Use classifier versioning. > ofproto: Accurate flow counts. > ofproto: Revertible eviction. > ofproto: Postpone sending flow removed messages. > ofproto: Use minimatch for making bundles smaller. > ofproto: Support port mods in bundles. > > NEWS | 25 +- > OPENFLOW-1.1+.md |6 + > include/openflow/openflow-common.h |2 + > include/openvswitch/vconn.h|3 + > lib/classifier-private.h | 158 - > lib/classifier.c | 297 + > lib/classifier.h | 112 ++-- > lib/match.c|9 + > lib/match.h|1 + > lib/ofp-parse.c| 42 +- > lib/ofp-parse.h|6 +- > lib/ofp-print.c|1 + > lib/ofp-util.c | 44 ++ > lib/ofp-util.h | 102 +-- > lib/ofp-version-opt.c |7 + > lib/ofp-version-opt.h |1 + > lib/ovs-router.c |2 +- > lib/tnl-ports.c|9 +- > lib/vconn.c| 238 ++- > ofproto/bundles.h | 20 +- > ofproto/connmgr.c |2 +- > ofproto/ofproto-dpif-xlate.c | 17 +- > ofproto/ofproto-dpif.c | 128 ++-- > ofproto/ofproto-dpif.h |5 +- > ofproto/ofproto-provider.h | 77 +-- > ofproto/ofproto.c | 1289 +--- > tests/ofproto-macros.at| 10 + > tests/ofproto.at | 244 +++ > tests/ovs-ofctl.at | 115 +++- > tests/test-classifier.c| 15 +- > utilities/ovs-ofctl.8.in | 59 +- > utilities/ovs-ofctl.c | 89 ++- > 32 files changed, 2234 insertions(+), 901 deletions(-) > > -- > 1.7.10.4 > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] sign-off (was: [PATCH v2] ofproto-dpif: Avoid creating OpenFlow ports for duplicate tunnels.)
Don't underestimate the value of a bug fix.
Thanks, I applied this patch, with the sign-off, to master.
On Fri, Jun 05, 2015 at 11:23:23AM -0700, Andy Zhou wrote:
> Thank you for your kindness.
>
> Signed-off-by: Andy Zhou
>
> On Fri, Jun 5, 2015 at 8:14 AM, Ben Pfaff wrote:
> > Andy, I added you as co-author on this, can you give me a Signed-off-by
> > for your contribution?
> >
> > On Fri, Jun 05, 2015 at 08:13:28AM -0700, Ben Pfaff wrote:
> >> Until now, when two tunnels had an identical configuration, both of them
> >> were assigned OpenFlow ports, but only one of those OpenFlow ports was
> >> functional. With this commit, only one of the two (or more) identically
> >> configured tunnels will be assigned an OpenFlow port number.
> >>
> >> Reported-by: Keith Holleman
> >> Signed-off-by: Ben Pfaff
> >> Co-authored-by: Andy Zhou
> >> ---
> >> v1->v2: Fix reference counting and memory leak (from Andy Zhou).
> >>
> >> AUTHORS| 1 +
> >> ofproto/ofproto-dpif.c | 10 --
> >> ofproto/tunnel.c | 14 ++
> >> ofproto/tunnel.h | 6 +++---
> >> 4 files changed, 22 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/AUTHORS b/AUTHORS
> >> index f155105..28899a8 100644
> >> --- a/AUTHORS
> >> +++ b/AUTHORS
> >> @@ -277,6 +277,7 @@ Joan Cirer j...@ev0.net
> >> John Darrington j...@darrington.wattle.id.au
> >> John Galgay j...@galgay.net
> >> John Hurley john.hur...@netronome.com
> >> +Keith Holleman hollemani...@gmail.com
> >> K 華k940...@hotmail.com
> >> Kevin Mancuso kevin.manc...@rackspace.com
> >> Kiran Shanbhog ki...@vmware.com
> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >> index c1daa1d..bd45305 100644
> >> --- a/ofproto/ofproto-dpif.c
> >> +++ b/ofproto/ofproto-dpif.c
> >> @@ -1671,8 +1671,14 @@ port_construct(struct ofport *port_)
> >>
> >> if (netdev_get_tunnel_config(netdev)) {
> >> atomic_count_inc(&ofproto->backer->tnl_count);
> >> -tnl_port_add(port, port->up.netdev, port->odp_port,
> >> - ovs_native_tunneling_is_on(ofproto), namebuf);
> >> +error = tnl_port_add(port, port->up.netdev, port->odp_port,
> >> + ovs_native_tunneling_is_on(ofproto),
> >> namebuf);
> >> +if (error) {
> >> +atomic_count_dec(&ofproto->backer->tnl_count);
> >> +dpif_port_destroy(&dpif_port);
> >> +return error;
> >> +}
> >> +
> >> port->is_tunnel = true;
> >> if (ofproto->ipfix) {
> >> dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_,
> >> port->odp_port);
> >> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> >> index 3ea0eb4..d2ac7c6 100644
> >> --- a/ofproto/tunnel.c
> >> +++ b/ofproto/tunnel.c
> >> @@ -1,4 +1,4 @@
> >> -/* Copyright (c) 2013, 2014 Nicira, Inc.
> >> +/* Copyright (c) 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.
> >> @@ -203,14 +203,20 @@ tnl_port_add__(const struct ofport_dpif *ofport,
> >> const struct netdev *netdev,
> >>
> >> /* Adds 'ofport' to the module with datapath port number 'odp_port'.
> >> 'ofport's
> >> * must be added before they can be used by the module. 'ofport' must be a
> >> - * tunnel. */
> >> -void
> >> + * tunnel.
> >> + *
> >> + * Returns 0 if successful, otherwise a positive errno value. */
> >> +int
> >> tnl_port_add(const struct ofport_dpif *ofport, const struct netdev
> >> *netdev,
> >> odp_port_t odp_port, bool native_tnl, const char name[])
> >> OVS_EXCLUDED(rwlock)
> >> {
> >> +bool ok;
> >> +
> >> fat_rwlock_wrlock(&rwlock);
> >> -tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
> >> +ok = tnl_port_add__(ofport, netdev, odp_port, true, native_tnl, name);
> >> fat_rwlock_unlock(&rwlock);
> >> +
> >> +return ok ? 0 : EEXIST;
> >> }
> >>
> >> /* Checks if the tunnel represented by 'ofport' reconfiguration due to
> >> changes
> >> diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
> >> index 6181762..b8415ab 100644
> >> --- a/ofproto/tunnel.h
> >> +++ b/ofproto/tunnel.h
> >> @@ -1,4 +1,4 @@
> >> -/* Copyright (c) 2013 Nicira, Inc.
> >> +/* Copyright (c) 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.
> >> @@ -33,8 +33,8 @@ void ofproto_tunnel_init(void);
> >> bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev
> >> *,
> >>odp_port_t, bool native_tnl, const char name[]);
> >>
> >> -void tnl_port_add(const struct ofport_dpif *, const struct netdev *,
> >> - odp_port_t odp_port, bool native_tnl, const char
> >> name[]);
> >> +int tnl_port_add(con
Re: [ovs-dev] [PATCH] ovs-ofctl: replace-flows and diff-flows support for multiple tables
On Fri, Jun 05, 2015 at 10:41:48AM -0700, Shashank Shanbhag wrote: > I thought there are only hidden flows, and not hidden tables. > We are reading all tables from 0 to OFPTT_MAX-1. > > But, I see what you're saying. I can probably use OFPMP_TABLE_FEATURES > (openflow1.4 spec Section 7.3.5.6) to get all the configured tables. Will > this ensure the hidden tables aren't returned? Please correct me if I'm > wrong. OFPMP_TABLE_FEATURES would work, but it was introduced into OpenFlow rather late, meaning that you'd need to use a different message on older versions of OpenFlow, and has more information than we really need. I think that OFPST_TABLE (which got renamed OFPMP_TABLE_STATS later but has the same form) will give you table numbers in every version of OpenFlow. The replies that OVS sends to OFPST_TABLE ought to omit hidden tables, in all versions of OpenFlow. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Use xzalloc instead of xmalloc
On Fri, Jun 05, 2015 at 10:48:30AM -0700, Sabyasachi Sengupta wrote: > > Use xzalloc instead of xmalloc for some key structure allocations in > ofproto-dpif (viz. ofproto_dpif, ofport_dpif and rule_dpif) so as to > prevent uninitialized values in these structures. Also add seat belts > around these structure allocations. > > Signed-off-by: Sabyasachi Sengupta It makes no sense to check the xmalloc() or xzalloc() return value for NULL. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 16/19] ovs-ofctl: Add 'bundle' command and unit testing.
On Fri, Jun 05, 2015 at 12:55:41PM -0700, Jarno Rajahalme wrote: > > > On May 29, 2015, at 8:58 PM, Ben Pfaff wrote: > > I'm a little surprised to see the new "error_reporter" callback > > function. Can you explain a little? > > Within a bundle we are now streaming the flow mods without waiting for > a response after each flow mod, so error responses to earlier mods may > arrive any time before the closing bundle control transaction. As > there is no uniform way the users may wish to handle errors, this new > callback allows the caller to do what they want. ovs-ofctl prints the > error messages to stderr, some other callers may want to log them > instead. OK. I'll give this a re-read and re-think for v3. > > In parse_ofp_str__(), I suspect that the "strict" versions won't ever be > > matched, because the shorter non-strict versions will be matched > > earlier. > > > > Actually, with strncmp() with the length of the token in the string, > it is the reverse! The shorter non-strict version will not match a > longer keyword, but if the order of the comparisons was changed, the > longer strict version would match the shorter non-strict version in > the string. (I tried this both ways.) I missed that the length argument to strncmp() was the length of the string that varies, not of the one that is a literal argument. Interesting--thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Use xzalloc instead of xmalloc
Ah, I see the out_of_memory code.. I'd resend without the NULL checks (with just the conversion to xzalloc). --- Nuage Networks Business Unit, Alcatel-Lucent, 755 Ravendale Drive, Mountain View, CA, 94043 Mailstop: 1127, Ph: 1-650-623-3461 On Fri, 5 Jun 2015, Ben Pfaff wrote: On Fri, Jun 05, 2015 at 10:48:30AM -0700, Sabyasachi Sengupta wrote: Use xzalloc instead of xmalloc for some key structure allocations in ofproto-dpif (viz. ofproto_dpif, ofport_dpif and rule_dpif) so as to prevent uninitialized values in these structures. Also add seat belts around these structure allocations. Signed-off-by: Sabyasachi Sengupta It makes no sense to check the xmalloc() or xzalloc() return value for NULL. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] ofproto-dpif: Use xzalloc instead of xmalloc
Use xzalloc instead of xmalloc for some key structure allocations in
ofproto-dpif (viz. ofproto_dpif, ofport_dpif and rule_dpif) so as to
prevent uninitialized values in these structures.
Signed-off-by: Sabyasachi Sengupta
---
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c1daa1d..c114027 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -812,7 +812,7 @@ static int add_internal_flows(struct ofproto_dpif *);
static struct ofproto *
alloc(void)
{
-struct ofproto_dpif *ofproto = xmalloc(sizeof *ofproto);
+struct ofproto_dpif *ofproto = xzalloc(sizeof *ofproto);
return &ofproto->up;
}
@@ -1608,7 +1608,7 @@ query_tables(struct ofproto *ofproto,
static struct ofport *
port_alloc(void)
{
-struct ofport_dpif *port = xmalloc(sizeof *port);
+struct ofport_dpif *port = xzalloc(sizeof *port);
return &port->up;
}
@@ -3876,7 +3876,7 @@ static struct rule_dpif *rule_dpif_cast(const struct
rule *rule)
static struct rule *
rule_alloc(void)
{
-struct rule_dpif *rule = xmalloc(sizeof *rule);
+struct rule_dpif *rule = xzalloc(sizeof *rule);
return &rule->up;
}
--
---
Nuage Networks Business Unit, Alcatel-Lucent,
755 Ravendale Drive, Mountain View, CA, 94043
Mailstop: 1127, Ph: 1-650-623-3461
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
This will still change the user's intent (and change the flows that the
user sets up), breaking some of the forms described in meta-flow.h.
This is closer to what I had in mind:
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 71b8bef..41a03ef 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5033,6 +5033,12 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
wc->masks.tp_src &= htons(UINT8_MAX);
wc->masks.tp_dst &= htons(UINT8_MAX);
}
+
+/* When there is a VLAN, the VLAN match must always include match on
+ * "present" bit. */
+if (flow->vlan_tci && wc->masks.vlan_tci) {
+wc->masks.vlan_tci |= htons(VLAN_CFI);
+}
}
}
However, this still doesn't fix every case, because of the cases like
the vlan_tci=0x000a/0xfff you mention, where "unwildcarding" the CFI bit
still results in an exact-match on a 0-bit, which the kernel will
reject. The most obvious place to fix that up is in the translation to
the kernel flow key. I think that we could do that in
odp_flow_key_from_flow__(). Do you think that would work out?
On Fri, Jun 05, 2015 at 11:03:53AM -0700, Alex Wang wrote:
> How about this?
>
> diff --git a/lib/match.c b/lib/match.c
> index 7d0b409..e34572d 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -360,6 +360,10 @@ match_set_dl_tci(struct match *match, ovs_be16 tci)
> void
> match_set_dl_tci_masked(struct match *match, ovs_be16 tci, ovs_be16 mask)
> {
> +if (mask && mask != htons(0x)) {
> +tci |= htons(VLAN_CFI);
> +mask |= htons(VLAN_CFI);
> +}
> match->flow.vlan_tci = tci & mask;
> match->wc.masks.vlan_tci = mask;
> }
>
>
> On Fri, Jun 5, 2015 at 9:24 AM, Alex Wang wrote:
>
> > Thx for the reference, exactly what i want,
> >
> > Will change the solution~
> >
> > Thanks,
> > Alex Wang,
> >
> > On Fri, Jun 5, 2015 at 8:24 AM, Ben Pfaff wrote:
> >
> >> On Wed, Jun 03, 2015 at 11:21:50PM -0700, Alex Wang wrote:
> >> > OVS datapath has check which prevents the installation of flow
> >> > that matches VLAN TCI but does not have exact match for VLAN_CFI
> >> > bit. However, the ovs userspace does not enforce it, so OpenFlow
> >> > flow like "vlan_tci=0x000a/0x0fff,action=output:local" can be added
> >> > to ovs. Subsequently, the generated megaflow will have match
> >> > field for vlan like "vlan(vid=5/0xfff,pcp=0/0x0,cfi=1/0)".
> >> >
> >> > With the OVS datapath check, the installation of such megaflow
> >> > will be rejected with:
> >> > "|WARN|system@ovs-system: failed to put[create][modify] (Invalid
> >> argument)"
> >> >
> >> > This commit adds a check in userspace that mark the vlan mask
> >> > invalid if it does not exact match for VLAN_CFI. So users will
> >> > be asked to provide correct mask.
> >>
> >> This is not the right fix, because it is legitimate and useful not to
> >> match on the "CFI" (actually "vlan present") bit in OpenFlow. See the
> >> comment in meta-flow.h:
> >>
> >> /* "vlan_tci".
> >> *
> >> * 802.1Q TCI.
> >> *
> >> * For a packet with an 802.1Q header, this is the Tag Control
> >> Information
> >> * (TCI) field, with the CFI bit forced to 1. For a packet with no
> >> 802.1Q
> >> * header, this has value 0.
> >> *
> >> * This field can be used in various ways:
> >> *
> >> * - If it is not constrained at all, the nx_match matches packets
> >> * without an 802.1Q header or with an 802.1Q header that has any
> >> TCI
> >> * value.
> >> *
> >> * - Testing for an exact match with 0 matches only packets without
> >> an
> >> * 802.1Q header.
> >> *
> >> * - Testing for an exact match with a TCI value with CFI=1 matches
> >> * packets that have an 802.1Q header with a specified VID and
> >> PCP.
> >> *
> >> * - Testing for an exact match with a nonzero TCI value with CFI=0
> >> does
> >> * not make sense. The switch may reject this combination.
> >> *
> >> * - Testing with a specific VID and CFI=1, with nxm_mask=0x1fff,
> >> matches
> >> * packets that have an 802.1Q header with that VID (and any PCP).
> >> *
> >> * - Testing with a specific PCP and CFI=1, with nxm_mask=0xf000,
> >> matches
> >> * packets that have an 802.1Q header with that PCP (and any VID).
> >> *
> >> * - Testing with nxm_value=0, nxm_mask=0x0fff matches packets with
> >> no
> >> * 802.1Q header or with an 802.1Q header with a VID of 0.
> >> *
> >> * - Testing with nxm_value=0, nxm_mask=0xe000 matches packets with
> >> no
> >> * 802.1Q header or with an 802.1Q header with a PCP of 0.
> >> *
> >> * - Testing with nxm_value=0, nxm_mask=0xefff matches packets with
> >> no
> >> * 802.1Q header or with an 802.1Q header with both VID and PCP
> >> of 0.
> >> *
> >>
Re: [ovs-dev] [PATCH] tunneling: Fix a tunnel name display bug
On Thu, Jun 04, 2015 at 01:52:16PM -0700, Andy Zhou wrote: > Currently, 'ovs-appctl tnl/ports/show' command won't display gre port > name correctly. Since netdev_vport_get_dpif_port() will not always > set the 'namebuf' it receives. Should use the name by its return > value instead. Found by inspection. > > Also extend the test case to cover this command. > > Signed-off-by: Andy Zhou Good catch! I see a possible refinement; I'll send out a separate patch to add that later. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] 答复： [PATCH] netdev-dpdk: Do not flush tx queue which is shared among CPUs since it is always flushed
We got a crash of ovs-vswitchd.
I start ovs following the instructions of INSTALL.DPDK.md, with the ovs master
code.
In my enironment, there are four cpu cores, the "real_n_txq" of dpdk port is 1
and "txq_needs_locking" is "true""...ovs-vsctl add-br br0 -- set bridge br0
datapath_type=netdevovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0
type=dpdkovs-vsctl add-port br0 dpdkvhost0 -- set Interface dpdkvhost0
type=dpdkvhost"It works, then i bring up a guest and do some tests, a
segmentfault happenned to ovs-vswitchd when I run "iperf3" clients on guest and
host at the same time:guest: iperf client sends tcp packets -> dpdkvhost0 ->
OVS -> dpdk0 -> outsidehost: iperf client sends tcp packets -> br0 -> OVS ->
dpdk0 -> outside The segmentfault is like this:Program received signal SIGSEGV,
Segmentation fault.eth_em_xmit_pkts (tx_queue=0x7f623d0f9880,
tx_pkts=0x7f623d0ebee8, nb_pkts=65535)at
dpdk-2.0.0/lib/librte_pmd_e1000/em_rxtx.c:436436 ol_flags =
tx_pkt->ol_flags;(gdb) bt#0 eth_em_xmit_pkts (tx_queue=0x7f623d0f9880,
tx_pkts=0x7f623d0ebee8, nb_pkts=65535)at
dpdk-2.0.0/lib/librte_pmd_e1000/em_rxtx.c:436#1 0x00625d3d in
rte_eth_tx_burst at
dpdk-2.0.0/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:2572#2
dpdk_queue_flush__ (dev=dev@entry=0x7f623d0f9940, qid=qid@entry=0) at
lib/netdev-dpdk.c:808#3 0x00627324 in dpdk_queue_pkts (cnt=1,
pkts=0x7fff5da5a8f0, qid=0, dev=0x7f623d0f9940) at lib/netdev-dpdk.c:1003#4
dpdk_do_tx_copy (netdev=netdev@entry=0x7f623d0f9940, qid=qid@entry=0,
pkts=pkts@entry=0x7fff5da5ae80, cnt=cnt@entry=1) at lib/netdev-dpdk.c:1073#5
0x00627e96 in netdev_dpdk_send__ (may_steal=, cnt=1,
pkts=0x7fff5da5ae80, qid=0, dev=0x7f623d0f9940) at lib/netdev-dpdk.c:1116#6
netdev_dpdk_eth_send (netdev=0x7f623d0f9940, qid=,
pkts=0x7fff5da5ae80, cnt=1, may_steal=) at
lib/netdev-dpdk.c:1169The traffics of guest and br0 are processing by main
thread and pmd thread seperately. The pkts of br0 are sent by
"netdev_dpdk_send__" with lock "rte_spinlock_lock" via dpdk port, and the pmd
main thread processes the sending direction pkts of dpdk port by
dpdk_queue_flush without lock. The carsh seems to be caused by this.
We fixed it with this patch and the segmentfault did not happen again.The
modification does not affect the performance phy-phy when the txq num is equal
to cpu num, we tesed it on server with Intel Xecn E5-2630 and 82599 xge nic.I
guess it also does not affect the performance of the opposite condition(not
equal) for the "flush_tx" of the txq is "true" and it will be flushed every
time in "dpdk_queue_pkts".
We dont clearly kown if there is better modificaion, some help will be greately
appreciated.
--发件人：Gray,
Mark D 发送时间：2015年6月5日(星期五) 22:53收件人：钢锁0310
，d...@openvswitch.com 主 题：Re: [ovs-dev]
[PATCH] netdev-dpdk: Do not flush tx queue which is shared among CPUs
since it is always flushed> > When tx queue is shared among CPUS,the pkts
always be flush in> 'netdev_dpdk_eth_send'> So it is unnecessarily for flushing
in netdev_dpdk_rxq_recv Otherwise tx will> be accessed without lockingAre you
seeing a specific bug or is this just to account for a device withless queues
than pmds?> > Signed-off-by: Wei li > ---> lib/netdev-dpdk.c
| 7 +--> 1 file changed, 5 insertions(+), 2 deletions(-)> > diff --git
a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 63243d8..25e3a73> 100644> ---
a/lib/netdev-dpdk.c> +++ b/lib/netdev-dpdk.c> @@ -892,8 +892,11 @@
netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,> struct dp_packet **packets,>
int nb_rx;> > /* There is only one tx queue for this core. Do not flush
other> - * queueus. */> -if (rxq_->queue_id == rte_lcore_id()) {> +
* queueus.s/queueus/queues> + * Do not flush tx queue which is shared among
CPUs> + * since it is always flushed */> +if (rxq_->queue_id ==
rte_lcore_id() &&> + OVS_LIKELY(!dev->txq_needs_locking)) {>
dpdk_queue_flush(dev, rxq_->queue_id);Do you see any drop in performance in a
simple phy-phy case beforeand after this patch?> }> > --> 1.9.5.msysgit.1>
> > ___> dev mailing list>
dev@openvswitch.org>
http://openvswitch.org/mailman/listinfo/dev___dev
mailing listdev@openvswitch.orghttp://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-vport: Mark netdev_vport_get_dpif_port() as OVS_WARN_UNUSED_RESULT.
Ignoring the result of this function means that the caller is quite likely
blindly using the character array passed in, instead of the return value,
which leads to latent bugs.
This would have prevented one of the bugs fixed by commit "tunneling: Fix a
tunnel name display bug".
CC: Andy Zhou
Signed-off-by: Ben Pfaff
---
This needs the patch from
http://openvswitch.org/pipermail/dev/2015-June/055981.html
to be applied first, otherwise it adds a warning to the build.
lib/netdev-vport.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index b20c407..be02cb5 100644
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2011, 2013 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 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.
@@ -19,6 +19,7 @@
#include
#include
+#include "compiler.h"
struct dpif_netlink_vport;
struct dpif_flow_stats;
@@ -48,7 +49,8 @@ enum { NETDEV_VPORT_NAME_BUFSIZE = 16 };
enum { NETDEV_VPORT_NAME_BUFSIZE = 256 };
#endif
const char *netdev_vport_get_dpif_port(const struct netdev *,
- char namebuf[], size_t bufsize);
+ char namebuf[], size_t bufsize)
+OVS_WARN_UNUSED_RESULT;
char *netdev_vport_get_dpif_port_strdup(const struct netdev *);
#endif /* netdev-vport.h */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ofproto-dpif: Use xzalloc instead of xmalloc
On Fri, Jun 05, 2015 at 09:49:47PM -0700, Sabyasachi Sengupta wrote: > > > Use xzalloc instead of xmalloc for some key structure allocations in > ofproto-dpif (viz. ofproto_dpif, ofport_dpif and rule_dpif) so as to > prevent uninitialized values in these structures. > > Signed-off-by: Sabyasachi Sengupta "git am" told me the patch was corrupted but I managed to apply the 3 one-character changes by hand ;-) Applied, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
On Fri, Jun 5, 2015 at 10:00 PM, Ben Pfaff wrote:
> This will still change the user's intent (and change the flows that the
> user sets up), breaking some of the forms described in meta-flow.h.
>
> This is closer to what I had in mind:
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 71b8bef..41a03ef 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -5033,6 +5033,12 @@ xlate_actions(struct xlate_in *xin, struct
> xlate_out *xout)
> wc->masks.tp_src &= htons(UINT8_MAX);
> wc->masks.tp_dst &= htons(UINT8_MAX);
> }
> +
> +/* When there is a VLAN, the VLAN match must always include match
> on
> + * "present" bit. */
> +if (flow->vlan_tci && wc->masks.vlan_tci) {
> +wc->masks.vlan_tci |= htons(VLAN_CFI);
> +}
> }
> }
>
I see what you mean,~
>
> However, this still doesn't fix every case, because of the cases like
> the vlan_tci=0x000a/0xfff you mention, where "unwildcarding" the CFI bit
> still results in an exact-match on a 0-bit, which the kernel will
> reject. The most obvious place to fix that up is in the translation to
> the kernel flow key. I think that we could do that in
> odp_flow_key_from_flow__(). Do you think that would work out?
>
>
I think kernel will always set the CFI in 'odp flow key' when the received
pkt contains vlan header. So my understanding is that the "unwildcarding
the CFI bit still results in an exact-match on a 0-bit" issue should not
happen.
datapath call sequence:
ovs_vport_receive()->ovs_flow_key_extract()->parse_vlan()->
key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
I think odp_flow_key_from_flow__() is the right place to fix it, so how
about
this?
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 3204d16..e17712c 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3486,10 +3486,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const
struct flow *flow,
if (flow->vlan_tci != htons(0) || flow->dl_type ==
htons(ETH_TYPE_VLAN)) {
if (export_mask) {
nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
+nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN,
+data->vlan_tci | htons(VLAN_CFI));
} else {
nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
htons(ETH_TYPE_VLAN));
+nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
}
-nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
if (flow->vlan_tci == htons(0)) {
goto unencap;
> On Fri, Jun 05, 2015 at 11:03:53AM -0700, Alex Wang wrote:
> > How about this?
> >
> > diff --git a/lib/match.c b/lib/match.c
> > index 7d0b409..e34572d 100644
> > --- a/lib/match.c
> > +++ b/lib/match.c
> > @@ -360,6 +360,10 @@ match_set_dl_tci(struct match *match, ovs_be16 tci)
> > void
> > match_set_dl_tci_masked(struct match *match, ovs_be16 tci, ovs_be16
> mask)
> > {
> > +if (mask && mask != htons(0x)) {
> > +tci |= htons(VLAN_CFI);
> > +mask |= htons(VLAN_CFI);
> > +}
> > match->flow.vlan_tci = tci & mask;
> > match->wc.masks.vlan_tci = mask;
> > }
> >
> >
> > On Fri, Jun 5, 2015 at 9:24 AM, Alex Wang wrote:
> >
> > > Thx for the reference, exactly what i want,
> > >
> > > Will change the solution~
> > >
> > > Thanks,
> > > Alex Wang,
> > >
> > > On Fri, Jun 5, 2015 at 8:24 AM, Ben Pfaff wrote:
> > >
> > >> On Wed, Jun 03, 2015 at 11:21:50PM -0700, Alex Wang wrote:
> > >> > OVS datapath has check which prevents the installation of flow
> > >> > that matches VLAN TCI but does not have exact match for VLAN_CFI
> > >> > bit. However, the ovs userspace does not enforce it, so OpenFlow
> > >> > flow like "vlan_tci=0x000a/0x0fff,action=output:local" can be added
> > >> > to ovs. Subsequently, the generated megaflow will have match
> > >> > field for vlan like "vlan(vid=5/0xfff,pcp=0/0x0,cfi=1/0)".
> > >> >
> > >> > With the OVS datapath check, the installation of such megaflow
> > >> > will be rejected with:
> > >> > "|WARN|system@ovs-system: failed to put[create][modify] (Invalid
> > >> argument)"
> > >> >
> > >> > This commit adds a check in userspace that mark the vlan mask
> > >> > invalid if it does not exact match for VLAN_CFI. So users will
> > >> > be asked to provide correct mask.
> > >>
> > >> This is not the right fix, because it is legitimate and useful not to
> > >> match on the "CFI" (actually "vlan present") bit in OpenFlow. See the
> > >> comment in meta-flow.h:
> > >>
> > >> /* "vlan_tci".
> > >> *
> > >> * 802.1Q TCI.
> > >> *
> > >> * For a packet with an 802.1Q header, this is the Tag Control
> > >> Information
> > >> * (TCI) field, with the CFI bit forced to 1. For a packet with
> no
> > >> 802.1Q
> > >> * header, this has value 0.
> > >> *
> > >> * This field can be us
[ovs-dev] [PATCH branch-2.3] ofproto: Don't report that group chaining is supported.
Group chaining hasn't been supported, so we shouldn't report that it is. (This is a good demonstration of why I don't like feature bits like this. It's too easy for even well-intentioned implementers to get them wrong.) Signed-off-by: Ben Pfaff --- This is intended for branch-2.3, even though the same bug is on master, because I'll shortly post a patch for master that adds support for group chaining there. ofproto/ofproto.c | 3 +-- tests/ofproto.at | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 6c6dc2d..c3e734c 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -537,8 +537,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type, hmap_init(&ofproto->groups); ovs_mutex_unlock(&ofproto_mutex); ofproto->ogf.types = 0xf; -ofproto->ogf.capabilities = OFPGFC_CHAINING | OFPGFC_SELECT_LIVENESS | -OFPGFC_SELECT_WEIGHT; +ofproto->ogf.capabilities = OFPGFC_SELECT_LIVENESS | OFPGFC_SELECT_WEIGHT; ofproto->ogf.max_groups[OFPGT11_ALL] = OFPG_MAX; ofproto->ogf.max_groups[OFPGT11_SELECT] = OFPG_MAX; ofproto->ogf.max_groups[OFPGT11_INDIRECT] = OFPG_MAX; diff --git a/tests/ofproto.at b/tests/ofproto.at index 5162938..3e786ee 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -398,7 +398,7 @@ AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPST_GROUP_FEATURES reply (OF1.2): Group table: Types: 0xf -Capabilities: 0x7 +Capabilities: 0x3 All group : max_groups = 0xff00 actions=0x03ff9801 Select group : -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
On Fri, Jun 05, 2015 at 10:36:55PM -0700, Alex Wang wrote:
> On Fri, Jun 5, 2015 at 10:00 PM, Ben Pfaff wrote:
>
> > This will still change the user's intent (and change the flows that the
> > user sets up), breaking some of the forms described in meta-flow.h.
> >
> > This is closer to what I had in mind:
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 71b8bef..41a03ef 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -5033,6 +5033,12 @@ xlate_actions(struct xlate_in *xin, struct
> > xlate_out *xout)
> > wc->masks.tp_src &= htons(UINT8_MAX);
> > wc->masks.tp_dst &= htons(UINT8_MAX);
> > }
> > +
> > +/* When there is a VLAN, the VLAN match must always include match
> > on
> > + * "present" bit. */
> > +if (flow->vlan_tci && wc->masks.vlan_tci) {
> > +wc->masks.vlan_tci |= htons(VLAN_CFI);
> > +}
> > }
> > }
> >
>
>
> I see what you mean,~
>
>
>
> >
> > However, this still doesn't fix every case, because of the cases like
> > the vlan_tci=0x000a/0xfff you mention, where "unwildcarding" the CFI bit
> > still results in an exact-match on a 0-bit, which the kernel will
> > reject. The most obvious place to fix that up is in the translation to
> > the kernel flow key. I think that we could do that in
> > odp_flow_key_from_flow__(). Do you think that would work out?
> >
> >
>
> I think kernel will always set the CFI in 'odp flow key' when the received
> pkt contains vlan header. So my understanding is that the "unwildcarding
> the CFI bit still results in an exact-match on a 0-bit" issue should not
> happen.
>
> datapath call sequence:
> ovs_vport_receive()->ovs_flow_key_extract()->parse_vlan()->
> key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
>
>
> I think odp_flow_key_from_flow__() is the right place to fix it, so how
> about
> this?
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 3204d16..e17712c 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -3486,10 +3486,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const
> struct flow *flow,
> if (flow->vlan_tci != htons(0) || flow->dl_type ==
> htons(ETH_TYPE_VLAN)) {
> if (export_mask) {
> nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
> +nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN,
> +data->vlan_tci | htons(VLAN_CFI));
> } else {
> nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
> htons(ETH_TYPE_VLAN));
> +nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
> }
> -nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
> encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
> if (flow->vlan_tci == htons(0)) {
> goto unencap;
I think you're right, want to post a full patch?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto-dpif-xlate: Add support for OpenFlow group chaining.
Requested-by: Vinllen Chen
Signed-off-by: Ben Pfaff
---
AUTHORS | 1 +
ofproto/ofproto-dpif-xlate.c | 29 +++--
ofproto/ofproto-dpif.c | 13 -
tests/ofproto-dpif.at| 12 +++-
4 files changed, 11 insertions(+), 44 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 28899a8..d600a90 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -352,6 +352,7 @@ Torbjorn Tornkvist kruska...@gmail.com
Valentin Budvalen...@hackaserver.com
Vasiliy Tolstov v.tols...@selfip.ru
Vasu Dasari vdas...@gmail.com
+Vinllen Chencvinl...@gmail.com
Vishal Swarankarvishal.swarn...@gmail.com
Vjekoslav Brajkovic bal...@cs.washington.edu
Voravit T. vora...@kth.se
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 71b8bef..933191e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3280,6 +3280,7 @@ xlate_select_group(struct xlate_ctx *ctx, struct
group_dpif *group)
static void
xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
{
+bool was_in_group = ctx->in_group;
ctx->in_group = true;
switch (group_dpif_get_type(group)) {
@@ -3298,37 +3299,13 @@ xlate_group_action__(struct xlate_ctx *ctx, struct
group_dpif *group)
}
group_dpif_unref(group);
-ctx->in_group = false;
-}
-
-static bool
-xlate_group_resource_check(struct xlate_ctx *ctx)
-{
-if (!xlate_resubmit_resource_check(ctx)) {
-return false;
-} else if (ctx->in_group) {
-/* Prevent nested translation of OpenFlow groups.
- *
- * OpenFlow allows this restriction. We enforce this restriction only
- * because, with the current architecture, we would otherwise have to
- * take a possibly recursive read lock on the ofgroup rwlock, which is
- * unsafe given that POSIX allows taking a read lock to block if there
- * is a thread blocked on taking the write lock. Other solutions
- * without this restriction are also possible, but seem unwarranted
- * given the current limited use of groups. */
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-
-VLOG_ERR_RL(&rl, "cannot recursively translate OpenFlow group");
-return false;
-} else {
-return true;
-}
+ctx->in_group = was_in_group;
}
static bool
xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
{
-if (xlate_group_resource_check(ctx)) {
+if (xlate_resubmit_resource_check(ctx)) {
struct group_dpif *group;
bool got_group;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 22e5d5f..d913f33 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4045,19 +4045,6 @@ static enum ofperr
group_construct(struct ofgroup *group_)
{
struct group_dpif *group = group_dpif_cast(group_);
-const struct ofputil_bucket *bucket;
-
-/* Prevent group chaining because our locking structure makes it hard to
- * implement deadlock-free. (See xlate_group_resource_check().) */
-LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
-const struct ofpact *a;
-
-OFPACT_FOR_EACH (a, bucket->ofpacts, bucket->ofpacts_len) {
-if (a->type == OFPACT_GROUP) {
-return OFPERR_OFPGMFC_CHAINING_UNSUPPORTED;
-}
-}
-}
ovs_mutex_init_adaptive(&group->stats_mutex);
ovs_mutex_lock(&group->stats_mutex);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index b5a9ad9..4784a38 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -294,13 +294,15 @@ Datapath actions: 10,set(tcp(src=91)),11
OVS_VSWITCHD_STOP
AT_CLEANUP
-AT_SETUP([ofproto-dpif - group chaining not supported])
+AT_SETUP([ofproto-dpif - group chaining])
OVS_VSWITCHD_START
ADD_OF_PORTS([br0], [1], [10], [11])
-AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0
'group_id=1234,type=all,bucket=output:10,set_field:192.168.3.90->ip_src,group:123,bucket=output:11'],
- [1], [], [stderr])
-AT_CHECK([STRIP_XIDS stderr | sed 1q], [0],
- [OFPT_ERROR (OF1.2): OFPGMFC_CHAINING_UNSUPPORTED
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0
'group_id=1234,type=all,bucket=set_field:192.168.3.90->ip_src,group:123,bucket=output:11'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0
'group_id=123,type=all,bucket=output:10'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip actions=group:1234'])
+AT_CHECK([ovs-appctl ofproto/trace br0
'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'],
[0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+ [Datapath actions:
set(ipv4(src=192.168.3.90)),10,set(ipv4(src=192.168.0.1)),11
])
OVS_VSWITCHD_STOP
AT_CLEANUP
--
2.1.3
___
dev mailing list
dev@ope
Re: [ovs-dev] Can we trivially enable group chaining?
On Wed, Jun 03, 2015 at 04:02:43PM +0900, Takashi Yamamoto wrote:
> hi,
>
> On Wed, Jun 3, 2015 at 3:36 PM, Ben Pfaff wrote:
> > We have the following comment in ofproto-dpif-xlate.c:
> >
> > /* Prevent nested translation of OpenFlow groups.
> > *
> > * OpenFlow allows this restriction. We enforce this restriction
> > only
> > * because, with the current architecture, we would otherwise have
> > to
> > * take a possibly recursive read lock on the ofgroup rwlock, which
> > is
> > * unsafe given that POSIX allows taking a read lock to block if
> > there
> > * is a thread blocked on taking the write lock. Other solutions
> > * without this restriction are also possible, but seem unwarranted
> > * given the current limited use of groups. */
> >
> > But I think that the reasoning no longer holds. At the time the comment
> > was written, each ofgroup had its own internal rwlock, which was held
> > for reading whenever the ofgroup was being translated. Since then,
> > ofgroups have transitioned to using RCU for protection, and no rwlock is
> > held during translation.
> >
> > I think that, therefore, we can trivially enable group chaining with a
> > commit like this (untested):
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 71b8bef..ce90b2a 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -3302,33 +3302,9 @@ xlate_group_action__(struct xlate_ctx *ctx, struct
> > group_dpif *group)
> > }
> >
> > static bool
> > -xlate_group_resource_check(struct xlate_ctx *ctx)
> > -{
> > -if (!xlate_resubmit_resource_check(ctx)) {
> > -return false;
> > -} else if (ctx->in_group) {
> > -/* Prevent nested translation of OpenFlow groups.
> > - *
> > - * OpenFlow allows this restriction. We enforce this restriction
> > only
> > - * because, with the current architecture, we would otherwise have
> > to
> > - * take a possibly recursive read lock on the ofgroup rwlock,
> > which is
> > - * unsafe given that POSIX allows taking a read lock to block if
> > there
> > - * is a thread blocked on taking the write lock. Other solutions
> > - * without this restriction are also possible, but seem unwarranted
> > - * given the current limited use of groups. */
> > -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > -
> > -VLOG_ERR_RL(&rl, "cannot recursively translate OpenFlow group");
> > -return false;
> > -} else {
> > -return true;
> > -}
> > -}
> > -
> > -static bool
> > xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
> > {
> > -if (xlate_group_resource_check(ctx)) {
> > +if (xlate_resubmit_resource_check(ctx)) {
> > struct group_dpif *group;
> > bool got_group;
> >
> > Anyone know a reason this might be a bad idea?
>
> i think it's a good idea.
> in_group needs to be a counter if we allow recursion.
Thanks.
I posted a patch:
http://openvswitch.org/pipermail/dev/2015-June/056048.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.
Yeah, later, with a test, ~
On Fri, Jun 5, 2015 at 10:43 PM, Ben Pfaff wrote:
> On Fri, Jun 05, 2015 at 10:36:55PM -0700, Alex Wang wrote:
> > On Fri, Jun 5, 2015 at 10:00 PM, Ben Pfaff wrote:
> >
> > > This will still change the user's intent (and change the flows that the
> > > user sets up), breaking some of the forms described in meta-flow.h.
> > >
> > > This is closer to what I had in mind:
> > >
> > > diff --git a/ofproto/ofproto-dpif-xlate.c
> b/ofproto/ofproto-dpif-xlate.c
> > > index 71b8bef..41a03ef 100644
> > > --- a/ofproto/ofproto-dpif-xlate.c
> > > +++ b/ofproto/ofproto-dpif-xlate.c
> > > @@ -5033,6 +5033,12 @@ xlate_actions(struct xlate_in *xin, struct
> > > xlate_out *xout)
> > > wc->masks.tp_src &= htons(UINT8_MAX);
> > > wc->masks.tp_dst &= htons(UINT8_MAX);
> > > }
> > > +
> > > +/* When there is a VLAN, the VLAN match must always include
> match
> > > on
> > > + * "present" bit. */
> > > +if (flow->vlan_tci && wc->masks.vlan_tci) {
> > > +wc->masks.vlan_tci |= htons(VLAN_CFI);
> > > +}
> > > }
> > > }
> > >
> >
> >
> > I see what you mean,~
> >
> >
> >
> > >
> > > However, this still doesn't fix every case, because of the cases like
> > > the vlan_tci=0x000a/0xfff you mention, where "unwildcarding" the CFI
> bit
> > > still results in an exact-match on a 0-bit, which the kernel will
> > > reject. The most obvious place to fix that up is in the translation to
> > > the kernel flow key. I think that we could do that in
> > > odp_flow_key_from_flow__(). Do you think that would work out?
> > >
> > >
> >
> > I think kernel will always set the CFI in 'odp flow key' when the
> received
> > pkt contains vlan header. So my understanding is that the "unwildcarding
> > the CFI bit still results in an exact-match on a 0-bit" issue should not
> > happen.
> >
> > datapath call sequence:
> > ovs_vport_receive()->ovs_flow_key_extract()->parse_vlan()->
> > key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
> >
> >
> > I think odp_flow_key_from_flow__() is the right place to fix it, so how
> > about
> > this?
> >
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index 3204d16..e17712c 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -3486,10 +3486,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf,
> const
> > struct flow *flow,
> > if (flow->vlan_tci != htons(0) || flow->dl_type ==
> > htons(ETH_TYPE_VLAN)) {
> > if (export_mask) {
> > nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
> > +nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN,
> > +data->vlan_tci | htons(VLAN_CFI));
> > } else {
> > nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
> > htons(ETH_TYPE_VLAN));
> > +nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
> > }
> > -nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
> > encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
> > if (flow->vlan_tci == htons(0)) {
> > goto unencap;
>
> I think you're right, want to post a full patch?
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] IGMPv3 support
On Wed, Jun 03, 2015 at 04:49:14PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Support IGMPv3 messages with multiple records. Make sure all IGMPv3
> messages go through slow path, since they may carry multiple multicast
> addresses, unlike IGMPv2.
>
> Tests done:
>
> * multiple addresses in IGMPv3 report are inserted in mdb;
> * address is removed from IGMPv3 if record is INCLUDE_MODE;
> * reports sent on a burst with same flow all go to userspace;
> * IGMPv3 reports go to mrouters, i.e., ports that have issued a query.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo
Thanks for working on this!
I get a ton of errors like this trying to compile with this applied:
In file included from ../lib/hmap.h:22:0,
from ../lib/cfm.h:21,
from ../ofproto/ofproto-provider.h:36,
from ../ofproto/bond.h:22,
from ../ofproto/bond.c:19:
../lib/packets.h:555:37: error: expected expression before '==' token
BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
^
../lib/util.h:48:61: note: in definition of macro 'BUILD_ASSERT__'
sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1; })
^
../lib/packets.h:555:1: note: in expansion of macro 'BUILD_ASSERT_DECL'
BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
^
../lib/packets.h:555:37: error: bit-field 'build_assert_failed' width not an
integer constant
BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
^
../lib/util.h:48:61: note: in definition of macro 'BUILD_ASSERT__'
sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 : -1; })
^
../lib/packets.h:555:1: note: in expansion of macro 'BUILD_ASSERT_DECL'
BUILD_ASSERT_DECL(IGMPV3_RECORD_LEN == sizeof(struct igmpv3_record));
^
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-dpdk: Do not flush tx queue which is shared among CPUs since it is always flushed
Sorry the last e-mail had wrong output format.
We got a crash of ovs-vswitchd.I start ovs following the instructions of
INSTALL.DPDK.md, with the ovs master code.In my enironment, there are four cpu
cores, the "real_n_txq" of dpdk port is 1 and "txq_needs_locking" is "true"
...ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdevovs-vsctl
add-port br0 dpdk0 -- set Interface dpdk0 type=dpdkovs-vsctl add-port br0
dpdkvhost0 -- set Interface dpdkvhost0 type=dpdkvhost
It works, then i bring up a guest and do some tests, a segmentfault happenned
to ovs-vswitchd when I run "iperf3" clients on guest and host at the same
time.guest: iperf client sends tcp packets -> dpdkvhost0 -> OVS -> dpdk0 ->
outsidehost: iperf client sends tcp packets -> br0 -> OVS -> dpdk0 -> outside
The segmentfault like this:Program received signal SIGSEGV, Segmentation
fault.eth_em_xmit_pkts (tx_queue=0x7f623d0f9880, tx_pkts=0x7f623d0ebee8,
nb_pkts=65535)at dpdk-2.0.0/lib/librte_pmd_e1000/em_rxtx.c:436436
ol_flags = tx_pkt->ol_flags;(gdb) bt#0 eth_em_xmit_pkts
(tx_queue=0x7f623d0f9880, tx_pkts=0x7f623d0ebee8, nb_pkts=65535)at
dpdk-2.0.0/lib/librte_pmd_e1000/em_rxtx.c:436#1 0x00625d3d in
rte_eth_tx_burst at
dpdk-2.0.0/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:2572#2
dpdk_queue_flush__ (dev=dev@entry=0x7f623d0f9940, qid=qid@entry=0) at
lib/netdev-dpdk.c:808#3 0x00627324 in dpdk_queue_pkts (cnt=1,
pkts=0x7fff5da5a8f0, qid=0, dev=0x7f623d0f9940) at lib/netdev-dpdk.c:1003#4
dpdk_do_tx_copy (netdev=netdev@entry=0x7f623d0f9940, qid=qid@entry=0,
pkts=pkts@entry=0x7fff5da5ae80, cnt=cnt@entry=1) at lib/netdev-dpdk.c:1073#5
0x00627e96 in netdev_dpdk_send__ (may_steal=, cnt=1,
pkts=0x7fff5da5ae80, qid=0, dev=0x7f623d0f9940) at lib/netdev-dpdk.c:1116#6
netdev_dpdk_eth_send (netdev=0x7f623d0f9940, qid=,
pkts=0x7fff5da5ae80, cnt=1, may_steal=) at lib/netdev-dpdk.c:1169
The traffics of guest and br0 are processing by main thread and pmd thread
seperately. The pkts of br0 are sent by "netdev_dpdk_send__" with lock
"rte_spinlock_lock" via dpdk port, and the pmd main thread processes the
sending direction pkts of dpdk port by dpdk_queue_flush without lock. The carsh
seems to be caused by this.We fixed it with this patch and the segmentfault did
not happen again.The modification does not affect the performance phy to phy
when the txq num is equal to cpu num, we tesed it on server with Intel Xecn
E52630 and 82599 xge nic.I guess it also does not affect the performance of
the opposite condition(not equal) for the "flush_tx" of the txq is "true" and
it will be flushed every time in "dpdk_queue_pkts".We dont clearly kown if
there is better modificaion, some help will be greately appreciated.
--From:Gray,
Mark D Time:2015 Jun 5 (Fri) 22:53To:钢锁0310
, d...@openvswitch.com Subject:Re:
[ovs-dev] [PATCH] netdev-dpdk: Do not flush tx queue which is shared
among CPUs since it is always flushed> > When tx queue is shared among CPUS,the
pkts always be flush in> 'netdev_dpdk_eth_send'> So it is unnecessarily for
flushing in netdev_dpdk_rxq_recv Otherwise tx will> be accessed without
lockingAre you seeing a specific bug or is this just to account for a device
withless queues than pmds?> > Signed-off-by: Wei li > --->
lib/netdev-dpdk.c | 7 +--> 1 file changed, 5 insertions(+), 2
deletions(-)> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
63243d8..25e3a73> 100644> --- a/lib/netdev-dpdk.c> +++ b/lib/netdev-dpdk.c> @@
-892,8 +892,11 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_,> struct
dp_packet **packets,> int nb_rx;> > /* There is only one tx queue for
this core. Do not flush other> - * queueus. */> -if (rxq_->queue_id ==
rte_lcore_id()) {> + * queueus.s/queueus/queues> + * Do not flush tx
queue which is shared among CPUs> + * since it is always flushed */> +
if (rxq_->queue_id == rte_lcore_id() &&> +
OVS_LIKELY(!dev->txq_needs_locking)) {> dpdk_queue_flush(dev,
rxq_->queue_id);Do you see any drop in performance in a simple phy-phy case
beforeand after this patch?> }> > --> 1.9.5.msysgit.1> > >
___> dev mailing list>
dev@openvswitch.org>
http://openvswitch.org/mailman/listinfo/dev___dev
mailing listdev@openvswitch.orghttp://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
