Re: [ovs-dev] [PATCH 2/3] service controller: Allow change to punix socket file group ownership.

[Alex Wang]

On Wed, Aug 26, 2015 at 2:13 PM, Ben Pfaff  wrote:

> On Fri, Aug 21, 2015 at 11:10:32PM -0700, Alex Wang wrote:
> > This commit adds a new key-value pair, 'punix_file_group=',
> > to the 'other_config' column in the 'Controller' table.  This new config
> > allows user to change the punix socket file's group ownership, so that
> > non-root process can also connect to ovs bridge.
> >
> > Signed-off-by: Alex Wang 
>
> POSIX says sysconf(_SC_GETGR_R_SIZE_MAX) can return -1.  It's probably
> best to pick some reasonable default in that case.
>
> I don't think POSIX requires getgrnam_r() to set errno; it's pretty
> unclear on that account.  It definitely requires the return value to be
> a nonzero errno value to indicate an error, so I'd recommend using the
> return value instead of errno.
>
> I am not sure that all systems have a group named "root".  I imagine
> that using a GID of 0 instead of a group "root" is more portable.
>
>

All make sense, will adopt the suggestions~~~


The text in the log messages use " : " as separators but the common
> style in OVS log messages is ": ", that is, no space before the colon.
>


Could you point me to the place?  only used ':' in the subject.



> The chmod is to 0770 but the log message says 0700.
>


Sure will fix that,



> I would consider adding support for setting the owner and the mode also.
>


Could you explain more?  I think since ovs-vswitchd (running as root)
creates the socket file, we should keep the ownership~

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

[ovs-dev] [PATCH V2] bridge: Relax the whitelist format for punix path.

[Alex Wang]

This commit relaxes the whitelist format for punix path for
service controller.  Instead of only allowing
punix:/.controller, the new format
allows any suffix, like punix:/.*.
(except '/').

Signed-off-by: Alex Wang 
---
PATCH->V2:
- prevent the punix path from specifying directory other than the
  ovs_rundir.
---
 tests/ovs-vswitchd.at | 14 ++
 vswitchd/bridge.c | 12 +++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
index 3b7c516..912354f 100644
--- a/tests/ovs-vswitchd.at
+++ b/tests/ovs-vswitchd.at
@@ -153,3 +153,17 @@ AT_CHECK([sed -n "
 ])
 
 AT_CLEANUP
+
+dnl --
+AT_SETUP([ovs-vswitchd -- set service controller])
+AT_SKIP_IF([test "$IS_WIN32" = "yes"])
+OVS_VSWITCHD_START
+
+AT_CHECK([ovs-vsctl set-controller br0 punix:$(pwd)/br0.void])
+OVS_WAIT_UNTIL([test -e br0.void])
+
+AT_CHECK([ovs-vsctl set-controller br0 
punix:$(pwd)/br0.void/../overwrite.file])
+OVS_WAIT_UNTIL([test -n "`grep ERR ovs-vswitchd.log | grep overwrite.file`"])
+
+OVS_VSWITCHD_STOP(["/Not adding Unix domain socket controller/d"])
+AT_CLEANUP
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f021360..a551590 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3559,18 +3559,20 @@ bridge_configure_remotes(struct bridge *br,
 continue;
 }
 } else {
-   whitelist = xasprintf("punix:%s/%s.controller",
+   whitelist = xasprintf("punix:%s/%s.",
  ovs_rundir(), br->name);
-   if (!equal_pathnames(c->target, whitelist, SIZE_MAX)) {
+   if (!equal_pathnames(c->target, whitelist, strlen(whitelist))
+   || strchr(c->target + strlen(whitelist), '/')) {
/* Prevent remote ovsdb-server users from accessing
 * arbitrary Unix domain sockets and overwriting arbitrary
 * local files. */
VLOG_ERR_RL(&rl, "bridge %s: Not adding Unix domain socket "
   "controller \"%s\" due to possibility of "
   "overwriting local files. Instead, specify "
-  "whitelisted \"%s\" or connect to "
-  "\"unix:%s/%s.mgmt\" (which is always "
-  "available without special configuration).",
+  "path in whitelisted format \"%s*\" or "
+  "connect to \"unix:%s/%s.mgmt\" (which is "
+  "always available without special "
+  "configuration).",
   br->name, c->target, whitelist,
   ovs_rundir(), br->name);
free(whitelist);
-- 
1.9.1

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

[ovs-dev] [PATCH] openswitch: fix typo CONFIG_NF_CONNTRACK_LABEL

[Valentin Rothberg]

Fix typo in conntrack.c
s/CONFIG_NF_CONNTRACK_LABEL/CONFIG_NF_CONNTRACK_LABELS/

Signed-off-by: Valentin Rothberg 
---
The typo was added by commmit c2ac66735870 ("openvswitch: Allow matching
on conntrack label").
I detected the issue scripts/checkkconfigsymbols.py


 net/openvswitch/conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 890d3eedb447..886bd2758502 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -169,7 +169,7 @@ int ovs_ct_put_key(const struct sw_flow_key *key, struct 
sk_buff *skb)
nla_put_u32(skb, OVS_KEY_ATTR_CT_MARK, key->ct.mark))
return -EMSGSIZE;
 
-   if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABEL) &&
+   if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS) &&
nla_put(skb, OVS_KEY_ATTR_CT_LABEL, sizeof(key->ct.label),
&key->ct.label))
return -EMSGSIZE;
-- 
1.9.1

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

[ovs-dev] Returned mail: see transcript for details

[Automatic Email Delivery Software]

The original message was received at Fri, 28 Aug 2015 14:56:52 +0530
from 192.25.171.63

- The following addresses had permanent fatal errors -




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

Re: [ovs-dev] [PATCH] dpdk: add support for v2.1.0

[Puha, TimoX]

Hi,

> From: gowrishankar [mailto:gowrishanka...@linux.vnet.ibm.com] 
> To have dpdk built with CONFIG_RTE_LIBRTE_VHOST_NUMA, we would need
> -lnuma in above extralib. It would be helpful if we document this
> as well (INSTALL.DPDK).

Thanks for your comments. 

We decided to leave this new vhost numa feature support to a separate patch as 
it is not really required for taking DPDK 2.1 into use. We are planning to 
submit a separate patch for this after testing it for performance effects 
though.

-Timo

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

Re: [ovs-dev] [lacp 2/2] lacp: Allow configurable aggregation keys.

[Zayats, Michael]

Hi Ethan,

Regarding some old patch in which you added other_config:lacp-aggregation-key 
to the Interface table.

I must be missing a reason of adding it to the Interface and not to the Port, 
where it would affect all slaves at once: 
when should users configure the key differently on slaves of the same bond and 
what impact would it have?

Thanks,
Michael

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

Re: [ovs-dev] [PATCH] dpdk: add support for v2.1.0

[Puha, TimoX]

Hi,

Thanks for your comments. Some details inline.

> From: Daniele Di Proietto [mailto:diproiet...@vmware.com]
> >diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> >index 35dd9a0..39e1b5d 100644
> >--- a/INSTALL.DPDK.md
> >+++ b/INSTALL.DPDK.md
> >@@ -16,7 +16,7 @@ OVS needs a system with 1GB hugepages support.
> > Building and Installing:
> > 
> >
> >-Required: DPDK 2.0
> >+Required: DPDK 2.1
> > Optional (if building with vhost-cuse): `fuse`, `fuse-devel`
> >(`libfuse-dev`
> > on Debian/Ubuntu)
> >
> >@@ -24,7 +24,7 @@ on Debian/Ubuntu)
> >   1. Set `$DPDK_DIR`
> >
> >  ```
> >- export DPDK_DIR=/usr/src/dpdk-2.0
> >+ export DPDK_DIR=/usr/src/dpdk-2.1
> >  cd $DPDK_DIR
> >  ```
> 
> Should we also remove the requirement to change
> CONFIG_RTE_LIBRTE_VHOST?
> It is set by default in DPDK 2.1
Ok. I had missed this part getting out of date.

> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 3444bb1..5798a93 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -68,7 +68,8 @@ static struct vlog_rate_limit rl =
> >VLOG_RATE_LIMIT_INIT(5, 20);
> >  */
> >
> > #define MTU_TO_MAX_LEN(mtu)  ((mtu) + ETHER_HDR_LEN +
> ETHER_CRC_LEN)
> >-#define MBUF_SIZE(mtu)   (MTU_TO_MAX_LEN(mtu) + (512) + \
> >+#define MBUF_PADDING 16
> >+#define MBUF_SIZE(mtu)   (MTU_TO_MAX_LEN(mtu) + (512) +
> >(MBUF_PADDING) + \
> >  sizeof(struct rte_mbuf) +
> >RTE_PKTMBUF_HEADROOM)
> 
> This change was not immediately clear to me. Then I tried to remove it and
> I noticed a performance regression.
> 
> If the goal is just to make sure that the data room is at least
> 2048 + RTE_PKTMBUF_HEADROOM bytes, I would rewrite the macro like this:
> 
> #define MBUF_SIZE_MTU(mtu)   (MTU_TO_MAX_LEN(mtu) \
>   + sizeof(struct dp_packet) \
>   + RTE_PKTMBUF_HEADROOM)
> #define MBUF_SIZE_DRIVER (2048 \
>   + sizeof (struct rte_mbuf) \
>   + RTE_PKTMBUF_HEADROOM)
> #define MBUF_SIZE(mtu)   MAX(MBUF_SIZE_MTU(mtu), MBUF_SIZE_DRIVER)
> 
> 
> and add a comment to explain the logic in the code.
> 
> What do you think?
As you suggest, the purpose is to ensure the minimum size is big enough to 
avoid scattering with normal Ethernet MTU.
We actually had a patch version similar to your suggestion at one time, but 
dropped it for the even simpler alternative because the jumbo frame support is 
not really there anyway. Anyway, I agree that this MTU sensitive code makes the 
intention clearer.

I will send a V2 soon.

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

[ovs-dev] Returned mail: Data format error

[Automatic Email Delivery Software]

The original message was received at Fri, 28 Aug 2015 17:00:06 +0530
from openvswitch.org [203.44.122.133]

- The following addresses had permanent fatal errors -
dev@openvswitch.org



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

[ovs-dev] [PATCH 0/5] Fixes to build datapath on centos6/rhel6

[Flavio Leitner]

This patchset enables the datapath to compile on newer centos
or Red Hat Enterprise Linux 6.6.

I did a quick test on:
6.4 2.6.32-358.el6.x86_64
6.6 2.6.32-504.el6.x86_64 
6 devel 2.6.32-578.el6.x86_64
7.1 3.10.0-229.el7.x86_64

Flavio Leitner (5):
  datapath: improve l4_rxhash regex
  datapath: check for backported proto_ports_offset
  datapath: check for backported ip_is_fragment
  datapath: check for el6 kernels for per_cpu
  datapath: check for rx_handler register

 acinclude.m4| 10 +-
 datapath/linux/compat/dev-openvswitch.c |  2 +-
 datapath/linux/compat/include/linux/in.h|  2 +-
 datapath/linux/compat/include/linux/netdevice.h |  2 +-
 datapath/linux/compat/include/linux/percpu.h|  2 +-
 datapath/linux/compat/include/net/ip.h  |  2 +-
 datapath/vport-netdev.c | 12 +---
 7 files changed, 23 insertions(+), 9 deletions(-)

-- 
2.1.0

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

[ovs-dev] [PATCH 1/5] datapath: improve l4_rxhash regex

[Flavio Leitner]

Red Hat Enterprise Linux 6 has a comment saying
that it doesn't support l4_rxhash which matches
the current grep regex.

Signed-off-by: Flavio Leitner 
---
 acinclude.m4 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 45cfaf6..db0de0d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -381,7 +381,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_clear_hash])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [int.skb_zerocopy(],
   [OVS_DEFINE([HAVE_SKB_ZEROCOPY])])
-  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [l4_rxhash])
+  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [u8.*l4_rxhash],
+  [OVS_DEFINE([HAVE_L4_RXHASH])])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_ensure_writable])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_pop])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_push])
-- 
2.1.0

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

[ovs-dev] [PATCH 3/5] datapath: check for backported ip_is_fragment

[Flavio Leitner]

Red Hat Enterprise Linux 6 has backported it from upstream,
so check for ip_is_fragment instead of kernel version.

Signed-off-by: Flavio Leitner 
---
 acinclude.m4   | 1 +
 datapath/linux/compat/include/net/ip.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 5e34d92..6e28788 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -329,6 +329,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   [OVS_DEFINE([HAVE_IP_SELECT_IDENT_USING_DST_ENTRY])])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [inet_get_local_port_range.*net],
   [OVS_DEFINE([HAVE_INET_GET_LOCAL_PORT_RANGE_USING_NET])])
+  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_is_fragment])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_disable_lro])
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_get_stats])
diff --git a/datapath/linux/compat/include/net/ip.h 
b/datapath/linux/compat/include/net/ip.h
index b606177..a9401a7 100644
--- a/datapath/linux/compat/include/net/ip.h
+++ b/datapath/linux/compat/include/net/ip.h
@@ -5,7 +5,7 @@
 
 #include 
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,1,0)
+#ifndef HAVE_IP_IS_FRAGMENT
 static inline bool ip_is_fragment(const struct iphdr *iph)
 {
return (iph->frag_off & htons(IP_MF | IP_OFFSET)) != 0;
-- 
2.1.0

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

[ovs-dev] [PATCH 2/5] datapath: check for backported proto_ports_offset

[Flavio Leitner]

Red Hat Enterprise Linux 6 has backported it from upstream,
so check for proto_ports_offset instead of kernel version.

Signed-off-by: Flavio Leitner 
---
 acinclude.m4 | 1 +
 datapath/linux/compat/include/linux/in.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index db0de0d..5e34d92 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -324,6 +324,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_set_encap_proto])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/in.h], [ipv4_is_multicast])
+  OVS_GREP_IFELSE([$KSRC/include/linux/in.h], [proto_ports_offset])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [__ip_select_ident.*dst_entry],
   [OVS_DEFINE([HAVE_IP_SELECT_IDENT_USING_DST_ENTRY])])
   OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [inet_get_local_port_range.*net],
diff --git a/datapath/linux/compat/include/linux/in.h 
b/datapath/linux/compat/include/linux/in.h
index fa2e026..78f8d77 100644
--- a/datapath/linux/compat/include/linux/in.h
+++ b/datapath/linux/compat/include/linux/in.h
@@ -4,7 +4,7 @@
 #include_next 
 
 #include 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,37)
+#ifndef HAVE_PROTO_PORTS_OFFSET
 static inline int proto_ports_offset(int proto)
 {
switch (proto) {
-- 
2.1.0

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

[ovs-dev] [PATCH 5/5] datapath: check for rx_handler register

[Flavio Leitner]

Red Hat Enterprise Linux 6 has backported the netdev RX
handler facility so use the netdev_rx_handler_register as
an indicator if the kernel is earlier than 2.3.36 or has
the facitilty backported.

Signed-off-by: Flavio Leitner 
---
 acinclude.m4|  2 ++
 datapath/linux/compat/dev-openvswitch.c |  2 +-
 datapath/linux/compat/include/linux/netdevice.h |  2 +-
 datapath/vport-netdev.c | 12 +---
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 73bbe8c..f77d4fc 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -338,6 +338,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [can_checksum_protocol])
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [netdev_features_t])
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [pcpu_sw_netstats])
+  OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], 
[netdev_rx_handler_register])
+  OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [net_device_extended])
   OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hookfn.*nf_hook_ops],
   [OVS_DEFINE([HAVE_NF_HOOKFN_ARG_OPS])])
 
diff --git a/datapath/linux/compat/dev-openvswitch.c 
b/datapath/linux/compat/dev-openvswitch.c
index 256d581..38ec8fe 100644
--- a/datapath/linux/compat/dev-openvswitch.c
+++ b/datapath/linux/compat/dev-openvswitch.c
@@ -33,7 +33,7 @@ void dev_disable_lro(struct net_device *dev) { }
 
 #endif /* HAVE_DEV_DISABLE_LRO */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) || \
+#if !defined HAVE_NETDEV_RX_HANDLER_REGISTER || \
 defined HAVE_RHEL_OVS_HOOK
 
 static int nr_bridges;
diff --git a/datapath/linux/compat/include/linux/netdevice.h 
b/datapath/linux/compat/include/linux/netdevice.h
index 3deb93d..0fb2144 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -43,7 +43,7 @@ extern void unregister_netdevice_many(struct list_head *head);
 extern void dev_disable_lro(struct net_device *dev);
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) || \
+#if !defined HAVE_NETDEV_RX_HANDLER_REGISTER || \
 defined HAVE_RHEL_OVS_HOOK
 
 #ifdef HAVE_RHEL_OVS_HOOK
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index de85087..5d1e74f 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -38,7 +38,8 @@
 static struct vport_ops ovs_netdev_vport_ops;
 static void netdev_port_receive(struct vport *vport, struct sk_buff *skb);
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39) || \
+defined HAVE_NETDEV_RX_HANDLER_REGISTER
 /* Called with rcu_read_lock and bottom-halves disabled. */
 static rx_handler_result_t netdev_frame_hook(struct sk_buff **pskb)
 {
@@ -257,7 +258,7 @@ drop:
 /* Returns null if this device is not attached to a datapath. */
 struct vport *ovs_netdev_get_vport(struct net_device *dev)
 {
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,36) || \
+#if defined HAVE_NETDEV_RX_HANDLER_REGISTER || \
 defined HAVE_RHEL_OVS_HOOK
 #ifdef HAVE_OVS_DATAPATH
if (likely(dev->priv_flags & IFF_OVS_DATAPATH))
@@ -267,8 +268,13 @@ struct vport *ovs_netdev_get_vport(struct net_device *dev)
 #ifdef HAVE_RHEL_OVS_HOOK
return (struct vport *)rcu_dereference_rtnl(dev->ax25_ptr);
 #else
+#ifdef HAVE_NET_DEVICE_EXTENDED
+   return (struct vport *)
+   
rcu_dereference_rtnl(netdev_extended(dev)->rx_handler_data);
+#else
return (struct vport 
*)rcu_dereference_rtnl(dev->rx_handler_data);
 #endif
+#endif
else
return NULL;
 #else
@@ -294,7 +300,7 @@ void ovs_netdev_exit(void)
ovs_vport_ops_unregister(&ovs_netdev_vport_ops);
 }
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) && \
+#if !defined HAVE_NETDEV_RX_HANDLER_REGISTER && \
 !defined HAVE_RHEL_OVS_HOOK
 /*
  * Enforces, mutual exclusion with the Linux bridge module, by declaring and
-- 
2.1.0

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

[ovs-dev] [PATCH 4/5] datapath: check for el6 kernels for per_cpu

[Flavio Leitner]

The OVS hook has been backported so it doesn't work to
decide per_cpu work arounds.

Signed-off-by: Flavio Leitner 
---
 acinclude.m4 | 3 +++
 datapath/linux/compat/include/linux/percpu.h | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 6e28788..73bbe8c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -449,6 +449,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/uapi/linux/netdevice.h], [NET_NAME_UNKNOWN],
   [OVS_DEFINE([HAVE_NET_NAME_UNKNOWN])])
 
+  OVS_GREP_IFELSE([$KSRC/include/linux/utsrelease.h], [el6],
+  [OVS_DEFINE([HAVE_RHEL6_PER_CPU])])
+
   if cmp -s datapath/linux/kcompat.h.new \
 datapath/linux/kcompat.h >/dev/null 2>&1; then
 rm datapath/linux/kcompat.h.new
diff --git a/datapath/linux/compat/include/linux/percpu.h 
b/datapath/linux/compat/include/linux/percpu.h
index 4ec64cf..2459f55 100644
--- a/datapath/linux/compat/include/linux/percpu.h
+++ b/datapath/linux/compat/include/linux/percpu.h
@@ -7,7 +7,7 @@
 #define this_cpu_ptr(ptr) per_cpu_ptr(ptr, smp_processor_id())
 #endif
 
-#ifdef HAVE_RHEL_OVS_HOOK
+#ifdef HAVE_RHEL6_PER_CPU
 #undef this_cpu_read
 #undef this_cpu_inc
 #undef this_cpu_dec
-- 
2.1.0

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

[ovs-dev] openvswitch 2.4.0 Fedora packages

[Flavio Leitner]

Hi folks,

I've updated the Fedora packages to 2.4.0:

 Distro   -  status
Rawhide   -  available
Fedora 23 -  testing [1]
Fedora 22 -  testing [2]
Fedora 21 -  testing [3]

The status ``testing´´ means people can enable the testing repo,
get the package, test and report feedback on bodhi url listed below.
After 3 good feedbacks, the package is moved to stable repo.

[1] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14366
[2] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14365
[3] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14364

Thanks,
fbl


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

[ovs-dev] Use next_cfg/cur_cfg in the presence of more than one client

[Ashwin Paranjpe]

 I have a few questions about the following commit:
http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commit;h=b54e22e91eee43eb04ad53e2fa919be44f34e731

- What would happen if there are two clients and both of them increment the
next_cfg to the same value?
I'd imagine the operation won’t be atomic, so this would be possible. One
of the clients would incorrectly conclude that vswitchd has applied its
configuration changes; Is my understanding correct?

- Is there any way we can guarantee correctness in the presence of more
than 1 client?

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

Re: [ovs-dev] Use next_cfg/cur_cfg in the presence of more than one client

[Ben Pfaff]

On Fri, Aug 28, 2015 at 08:33:23AM -0700, Ashwin Paranjpe wrote:
>  I have a few questions about the following commit:
> http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commit;h=b54e22e91eee43eb04ad53e2fa919be44f34e731
> 
> - What would happen if there are two clients and both of them increment the
> next_cfg to the same value?
> I'd imagine the operation won’t be atomic, so this would be possible. One
> of the clients would incorrectly conclude that vswitchd has applied its
> configuration changes; Is my understanding correct?
> 
> - Is there any way we can guarantee correctness in the presence of more
> than 1 client?

The increment is atomic.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] AUTHORS: Add Sairam Venugopal

[Russell Bryant]

On 08/27/2015 08:45 PM, Nithin Raju wrote:
> Signed-off-by: Nithin Raju 
> ---
>  AUTHORS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/AUTHORS b/AUTHORS
> index 254095e..6c683f8 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -157,6 +157,7 @@ Rob Hoesrob.h...@citrix.com
>  Romain Lenglet  romain.leng...@berabera.info
>  Russell Bryant  rbry...@redhat.com
>  Ryan Wilson wr...@nicira.com
> +Sairam Venugopalvsai...@vmware.com
>  Sajjad Lateef   slat...@nicira.com
>  Samuel Ghinet   sghi...@cloudbasesolutions.com
>  Sanjay Sane ss...@nicira.com
> 

lgtm, author of b8b00f0ce4044fd6cb535f3514f5f6b205bf50a2

Acked-by: Russell Bryant 

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

Re: [ovs-dev] [PATCH 2/3] service controller: Allow change to punix socket file group ownership.

[Ben Pfaff]

On Fri, Aug 28, 2015 at 12:25:26AM -0700, Alex Wang wrote:
> On Wed, Aug 26, 2015 at 2:13 PM, Ben Pfaff  wrote:
> 
> > The text in the log messages use " : " as separators but the common
> > style in OVS log messages is ": ", that is, no space before the colon.
> 
> Could you point me to the place?  only used ':' in the subject.

Pretty much every VLOG_*() in your patch.

> > I would consider adding support for setting the owner and the mode also.
> 
> Could you explain more?  I think since ovs-vswitchd (running as root)
> creates the socket file, we should keep the ownership~

Setting the ownership would be a way to allow a process with a
particular uid to access the socket.

This could wait until later.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V2 1/2] ofproto-dpif-upcall: Do not attribute stats when flow_del returns error.

[Daniele Di Proietto]

With the older version of this series I was able to reproduce the problem
and this patch appears to fix it.

Acked-by: Daniele Di Proietto 

Thanks!

On 28/08/2015 06:25, "Alex Wang"  wrote:

>In the push_ukey_ops__(), when flow_del operation returns error, the
>'struct
>stats' passed to the operation function will be set to all zero.  And we
>should not use it to calculate the delta (i.e. minus the zero stats by the
>cached stats causes overflow).
>
>Even though this should rarely happen, it is still good to make
>push_ukey_ops__() just ignore the operation when it fails.
>
>Signed-off-by: Alex Wang 
>
>---
>V2:
>- new patch.
>---
> ofproto/ofproto-dpif-upcall.c | 5 +
> 1 file changed, 5 insertions(+)
>
>diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>index a0994a2..419fd1a 100644
>--- a/ofproto/ofproto-dpif-upcall.c
>+++ b/ofproto/ofproto-dpif-upcall.c
>@@ -1906,6 +1906,11 @@ push_ukey_ops__(struct udpif *udpif, struct
>ukey_op *ops, size_t n_ops)
> continue;
> }
> 
>+if (op->dop.error) {
>+/* flow_del error, 'stats' is unusable. */
>+continue;
>+}
>+
> if (op->ukey) {
> ovs_mutex_lock(&op->ukey->mutex);
> push->used = MAX(stats->used, op->ukey->stats.used);
>-- 
>1.9.1
>

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

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[Jarno Rajahalme]

> On Aug 27, 2015, at 10:25 PM, Alex Wang  wrote:
> 
> When dpdk configuration changes, all pmd threads are recreated
> and rx queues of each port are reloaded.  After this process,
> rx queue could be mapped to a different pmd thread other than
> the one before reconfiguration.  However, this is totally
> transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
> module still holds ukeys generated before pmd thread recreation,
> this old ukey will collide with the ukey for the new upcalls
> from same traffic flow, causing flow installation failure.
> 
> To fix the bug, this commit adds a new call-back function
> in dpif layer for notifying upper layer the purging of datapath
> (e.g. pmd thread deletion in dpif-netdev).  So, the
> ofproto-dpif-upcall module can react properly with deleting
> the ukeys and with collecting flows' last stats.
> 
> Reported-by: Ilya Maximets 
> Signed-off-by: Alex Wang 
> 
> ---
> PATCH->V2:
> - PATCH does not fix the problem,
> - call the purge_cb between pmd thread stopping and deletion.
>  (which should be the right place)
> ---
> lib/dpif-netdev.c | 23 ---
> lib/dpif-netlink.c|  1 +
> lib/dpif-provider.h   | 11 ++-
> lib/dpif.c|  8 
> lib/dpif.h| 13 +
> ofproto/ofproto-dpif-upcall.c | 36 
> 6 files changed, 88 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index f841876..2d187da 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -204,6 +204,11 @@ struct dp_netdev {
> upcall_callback *upcall_cb;  /* Callback function for executing upcalls. 
> */
> void *upcall_aux;
> 
> +/* Callback function for notifying the purging of dp flows (during
> + * reseting pmd deletion). */
> +dp_purge_callback *dp_purge_cb;
> +void *dp_purge_aux;
> +
> /* Stores all 'struct dp_netdev_pmd_thread's. */
> struct cmap poll_threads;
> 
> @@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd)
> /* Stops the pmd thread, removes it from the 'dp->poll_threads',
>  * and unrefs the struct. */
> static void
> -dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> +dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread *pmd)
> {
> /* Uninit the 'flow_cache' since there is
>  * no actual thread uninit it for NON_PMD_CORE_ID. */
> @@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> ovs_numa_unpin_core(pmd->core_id);
> xpthread_join(pmd->thread, NULL);
> }
> +/* Purges the 'pmd''s flows after stoping the thread. */

“stoping” -> “stopping”

I would also like the comment elaborated like this: “, but before destroying 
the flows, so that the flow stats can be collected."

> +dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
> cmap_remove(&pmd->dp->poll_threads, &pmd->node, hash_int(pmd->core_id, 
> 0));
> dp_netdev_pmd_unref(pmd);
> }


  Jarno

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

Re: [ovs-dev] [PATCH] AUTHORS: Add Sairam Venugopal

[Justin Pettit]

> On Aug 28, 2015, at 8:53 AM, Russell Bryant  wrote:
> 
> On 08/27/2015 08:45 PM, Nithin Raju wrote:
>> Signed-off-by: Nithin Raju 
>> ---
>> AUTHORS | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/AUTHORS b/AUTHORS
>> index 254095e..6c683f8 100644
>> --- a/AUTHORS
>> +++ b/AUTHORS
>> @@ -157,6 +157,7 @@ Rob Hoesrob.h...@citrix.com
>> Romain Lenglet  romain.leng...@berabera.info
>> Russell Bryant  rbry...@redhat.com
>> Ryan Wilson wr...@nicira.com
>> +Sairam Venugopalvsai...@vmware.com
>> Sajjad Lateef   slat...@nicira.com
>> Samuel Ghinet   sghi...@cloudbasesolutions.com
>> Sanjay Sane ss...@nicira.com
>> 
> 
> lgtm, author of b8b00f0ce4044fd6cb535f3514f5f6b205bf50a2
> 
> Acked-by: Russell Bryant 

Thanks.  Pushed.

--Justin


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

[ovs-dev] [PATCH] tunnel: Support matching on the presence of Geneve options.

[Jesse Gross]

Sometimes it is useful to match only on whether a Geneve option
is present even if the specific value is unimportant. A special
case of this is zero length options where there is no value at all
and the only information conveyed is whether the option was included
in the packet.

This operation was partially supported before but it was not consistent -
in particular, options were never serialized through NXM/OXM unless
they had a non-zero mask. Furthermore, zero length options were rejected
altogether when they were installed through the Geneve map OpenFlow
command.

This adds support for these types of matches by making any NXM/OXM for
tunnel metadata force a match on that field. In the case of a zero length
option, both the value and mask of the NXM are ignored.

Signed-off-by: Jesse Gross 
---
 lib/bitmap.h |  2 ++
 lib/meta-flow.c  | 46 
 lib/meta-flow.h  |  3 +-
 lib/nx-match.c   | 43 +-
 lib/nx-match.h   |  6 ++--
 lib/odp-util.c   |  3 +-
 lib/ofp-parse.c  | 21 ++---
 lib/ofp-util.c   |  3 +-
 lib/tun-metadata.c   | 72 ++--
 lib/tun-metadata.h   | 11 ++-
 ofproto/ofproto-dpif-xlate.c |  5 +++
 tests/ovs-ofctl.at   |  1 +
 tests/tunnel.at  | 42 +-
 utilities/ovs-ofctl.8.in |  7 -
 14 files changed, 193 insertions(+), 72 deletions(-)

diff --git a/lib/bitmap.h b/lib/bitmap.h
index cf5d3f0..35f033e 100644
--- a/lib/bitmap.h
+++ b/lib/bitmap.h
@@ -278,4 +278,6 @@ bitmap_is_all_zeros(const unsigned long *bitmap, size_t n)
 #define ULLONG_SET0(MAP, OFFSET) ((MAP) &= ~(1ULL << (OFFSET)))
 #define ULLONG_SET1(MAP, OFFSET) ((MAP) |= 1ULL << (OFFSET))
 
+#define ULLONG_GET(MAP, OFFSET) ((MAP) & (1ULL << (OFFSET)))
+
 #endif /* bitmap.h */
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 5a46ce4..971565d 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -202,12 +202,9 @@ mf_is_all_wild(const struct mf_field *mf, const struct 
flow_wildcards *wc)
 return !wc->masks.tunnel.gbp_id;
 case MFF_TUN_GBP_FLAGS:
 return !wc->masks.tunnel.gbp_flags;
-CASE_MFF_TUN_METADATA: {
-union mf_value value;
-
-tun_metadata_read(&wc->masks.tunnel, mf, &value);
-return is_all_zeros(&value.tun_metadata, mf->n_bytes);
-}
+CASE_MFF_TUN_METADATA:
+return !ULLONG_GET(wc->masks.tunnel.metadata.present.map,
+   mf->id - MFF_TUN_METADATA0);
 case MFF_METADATA:
 return !wc->masks.metadata;
 case MFF_IN_PORT:
@@ -1063,19 +1060,28 @@ field_len(const struct mf_field *mf, const union 
mf_value *value_)
 /* Returns the effective length of the field. For fixed length fields,
  * this is just the defined length. For variable length fields, it is
  * the minimum size encoding that retains the same meaning (i.e.
- * discarding leading zeros). */
+ * discarding leading zeros).
+ *
+ * 'is_masked' returns (if non-NULL) whether the original contained
+ * a mask. Otherwise, a mask that is the same length as the value
+ * might be misinterpreted as an exact match. */
 int
 mf_field_len(const struct mf_field *mf, const union mf_value *value,
- const union mf_value *mask)
+ const union mf_value *mask, bool *is_masked_)
 {
 int len, mask_len;
+bool is_masked = mask && !is_all_ones(mask, mf->n_bytes);
 
 len = field_len(mf, value);
-if (mask && !is_all_ones(mask, mf->n_bytes)) {
+if (is_masked) {
 mask_len = field_len(mf, mask);
 len = MAX(len, mask_len);
 }
 
+if (is_masked_) {
+*is_masked_ = is_masked;
+}
+
 return len;
 }
 
@@ -1329,6 +1335,13 @@ mf_set_flow_value_masked(const struct mf_field *field,
 mf_set_flow_value(field, &tmp, flow);
 }
 
+bool
+mf_is_tun_metadata(const struct mf_field *mf)
+{
+return mf->id >= MFF_TUN_METADATA0 &&
+   mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
+}
+
 /* Returns true if 'mf' has a zero value in 'flow', false if it is nonzero.
  *
  * The caller is responsible for ensuring that 'flow' meets 'mf''s
@@ -1336,10 +1349,15 @@ mf_set_flow_value_masked(const struct mf_field *field,
 bool
 mf_is_zero(const struct mf_field *mf, const struct flow *flow)
 {
-union mf_value value;
+if (!mf_is_tun_metadata(mf)) {
+union mf_value value;
 
-mf_get_value(mf, flow, &value);
-return is_all_zeros(&value, mf->n_bytes);
+mf_get_value(mf, flow, &value);
+return is_all_zeros(&value, mf->n_bytes);
+} else {
+return !ULLONG_GET(flow->tunnel.metadata.present.map,
+   mf->id - MFF_TUN_METADATA0);
+}
 }
 
 /* Makes 'match' wildcard field 'mf'.
@@ -1585,7 +1603,9 @@ mf_set(const struct mf_field *mf,
 if (!mask || is_all_ones(mask, mf->n_bytes)) {
 mf_set_value(mf, value, mat

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[Daniele Di Proietto]

I've tested it and it appears to correctly keep all the stats.

Two comments inline, otherwise:

Acked-by: Daniele Di Proietto 

Thanks for fixing this Alex!

On 28/08/2015 06:25, "Alex Wang"  wrote:

>When dpdk configuration changes, all pmd threads are recreated
>and rx queues of each port are reloaded.  After this process,
>rx queue could be mapped to a different pmd thread other than
>the one before reconfiguration.  However, this is totally
>transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
>module still holds ukeys generated before pmd thread recreation,
>this old ukey will collide with the ukey for the new upcalls
>from same traffic flow, causing flow installation failure.
>
>To fix the bug, this commit adds a new call-back function
>in dpif layer for notifying upper layer the purging of datapath
>(e.g. pmd thread deletion in dpif-netdev).  So, the
>ofproto-dpif-upcall module can react properly with deleting
>the ukeys and with collecting flows' last stats.
>
>Reported-by: Ilya Maximets 
>Signed-off-by: Alex Wang 
>
>---
>PATCH->V2:
>- PATCH does not fix the problem,
>- call the purge_cb between pmd thread stopping and deletion.
>  (which should be the right place)
>---
> lib/dpif-netdev.c | 23 ---
> lib/dpif-netlink.c|  1 +
> lib/dpif-provider.h   | 11 ++-
> lib/dpif.c|  8 
> lib/dpif.h| 13 +
> ofproto/ofproto-dpif-upcall.c | 36 
> 6 files changed, 88 insertions(+), 4 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index f841876..2d187da 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -204,6 +204,11 @@ struct dp_netdev {
> upcall_callback *upcall_cb;  /* Callback function for executing
>upcalls. */
> void *upcall_aux;
> 
>+/* Callback function for notifying the purging of dp flows (during
>+ * reseting pmd deletion). */
>+dp_purge_callback *dp_purge_cb;
>+void *dp_purge_aux;
>+
> /* Stores all 'struct dp_netdev_pmd_thread's. */
> struct cmap poll_threads;
> 
>@@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
>*pmd)
> /* Stops the pmd thread, removes it from the 'dp->poll_threads',
>  * and unrefs the struct. */
> static void
>-dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
>+dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread *pmd)
> {
> /* Uninit the 'flow_cache' since there is
>  * no actual thread uninit it for NON_PMD_CORE_ID. */
>@@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> ovs_numa_unpin_core(pmd->core_id);
> xpthread_join(pmd->thread, NULL);
> }
>+/* Purges the 'pmd''s flows after stoping the thread. */
>+dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);

There's still the possibility a datapath is created through appctl
dpctl/add-dp.
In this case dp_purge_cb is NULL and a couple of tests fail (789, 790).
Adding a check should be enough:

/* Purges the 'pmd''s flows after stopping the thread. */
if (dp->dp_purge_cb) {
dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
}




> cmap_remove(&pmd->dp->poll_threads, &pmd->node,
>hash_int(pmd->core_id, 0));
> dp_netdev_pmd_unref(pmd);
> }
>@@ -2874,7 +2881,7 @@ dp_netdev_destroy_all_pmds(struct dp_netdev *dp)
> struct dp_netdev_pmd_thread *pmd;
> 
> CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
>-dp_netdev_del_pmd(pmd);
>+dp_netdev_del_pmd(dp, pmd);
> }
> }
> 
>@@ -2886,7 +2893,7 @@ dp_netdev_del_pmds_on_numa(struct dp_netdev *dp,
>int numa_id)
> 
> CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
> if (pmd->numa_id == numa_id) {
>-dp_netdev_del_pmd(pmd);
>+dp_netdev_del_pmd(dp, pmd);
> }
> }
> }
>@@ -3391,6 +3398,15 @@ struct dp_netdev_execute_aux {
> };
> 
> static void
>+dpif_netdev_register_dp_purge_cb(struct dpif *dpif, dp_purge_callback
>*cb,
>+ void *aux)
>+{
>+struct dp_netdev *dp = get_dp_netdev(dpif);
>+dp->dp_purge_aux = aux;
>+dp->dp_purge_cb = cb;
>+}
>+
>+static void
> dpif_netdev_register_upcall_cb(struct dpif *dpif, upcall_callback *cb,
>void *aux)
> {
>@@ -3637,6 +3653,7 @@ const struct dpif_class dpif_netdev_class = {
> NULL,   /* recv */
> NULL,   /* recv_wait */
> NULL,   /* recv_purge */
>+dpif_netdev_register_dp_purge_cb,
> dpif_netdev_register_upcall_cb,
> dpif_netdev_enable_upcall,
> dpif_netdev_disable_upcall,
>diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>index b8c27d8..ffeb124 100644
>--- a/lib/dpif-netlink.c
>+++ b/lib/dpif-netlink.c
>@@ -2309,6 +2309,7 @@ const struct dpif_class dpif_netlink_class = {
> dpif_netlink_recv,
> dpif_netlink_recv_wait,
> dpif_netlink_recv_purge,
>+NULL,   

Re: [ovs-dev] [PATCH 5/5] datapath: check for rx_handler register

[Jesse Gross]

On Fri, Aug 28, 2015 at 6:37 AM, Flavio Leitner  wrote:
> diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> index de85087..5d1e74f 100644
> --- a/datapath/vport-netdev.c
> +++ b/datapath/vport-netdev.c
> @@ -38,7 +38,8 @@
>  static struct vport_ops ovs_netdev_vport_ops;
>  static void netdev_port_receive(struct vport *vport, struct sk_buff *skb);
>
> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39) || \
> +defined HAVE_NETDEV_RX_HANDLER_REGISTER

I think this will break kernels 2.6.36-38 without the backport. The
register function existed but the prototype of handler changed in
2.6.39.

I also noticed that there is a typo in the version number in the commit message.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/5] Fixes to build datapath on centos6/rhel6

[Jesse Gross]

On Fri, Aug 28, 2015 at 6:37 AM, Flavio Leitner  wrote:
> This patchset enables the datapath to compile on newer centos
> or Red Hat Enterprise Linux 6.6.
>
> I did a quick test on:
> 6.4 2.6.32-358.el6.x86_64
> 6.6 2.6.32-504.el6.x86_64
> 6 devel 2.6.32-578.el6.x86_64
> 7.1 3.10.0-229.el7.x86_64

Thanks, I applied the first 4 patches to master and branch-2.4 and had
a couple comments on the fifth.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 5/5] datapath: check for rx_handler register

[Flavio Leitner]

On Fri, Aug 28, 2015 at 10:08:23AM -0700, Jesse Gross wrote:
> On Fri, Aug 28, 2015 at 6:37 AM, Flavio Leitner  wrote:
> > diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> > index de85087..5d1e74f 100644
> > --- a/datapath/vport-netdev.c
> > +++ b/datapath/vport-netdev.c
> > @@ -38,7 +38,8 @@
> >  static struct vport_ops ovs_netdev_vport_ops;
> >  static void netdev_port_receive(struct vport *vport, struct sk_buff *skb);
> >
> > -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
> > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39) || \
> > +defined HAVE_NETDEV_RX_HANDLER_REGISTER
> 
> I think this will break kernels 2.6.36-38 without the backport. The
> register function existed but the prototype of handler changed in
> 2.6.39.

If that is the case, then I am sure it will break.
I will check the sources of 2.6.36-38.

> I also noticed that there is a typo in the version number in the commit 
> message.

I will fix on v2.
Thanks!
fbl


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

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[Joe Stringer]

On 28 August 2015 at 09:41, Daniele Di Proietto  wrote:
> I've tested it and it appears to correctly keep all the stats.
>
> Two comments inline, otherwise:
>
> Acked-by: Daniele Di Proietto 

That could even be... dare I say it... "Tested-by: ..." :-)


In general this patch seems fine. I'm not entirely confident what the
worst case behaviour is though - for instance if revalidators are
crazy busy (eg disable megaflows with port scan) and you reconfigure
pmd threads, does it handle this gracefully? As far as I can tell,
there is nothing stopping a revalidator from trying to delete the same
ukey+flow at the same time. Also, typical purging like this happens
independently by each revalidator thread where no thread can handle
the same ukeys as another thread. This instead goes through all cmaps
and picks out those with the current pmd id.

The safest thing is to stop the world (ie, at least stop revalidators
from processing further, if not completely disable the threads then
re-enable when it's safe again), and an approach like that seems
simpler.


> On 28/08/2015 06:25, "Alex Wang"  wrote:
>
>>When dpdk configuration changes, all pmd threads are recreated
>>and rx queues of each port are reloaded.  After this process,
>>rx queue could be mapped to a different pmd thread other than
>>the one before reconfiguration.  However, this is totally
>>transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
>>module still holds ukeys generated before pmd thread recreation,
>>this old ukey will collide with the ukey for the new upcalls
>>from same traffic flow, causing flow installation failure.
>>
>>To fix the bug, this commit adds a new call-back function
>>in dpif layer for notifying upper layer the purging of datapath
>>(e.g. pmd thread deletion in dpif-netdev).  So, the
>>ofproto-dpif-upcall module can react properly with deleting
>>the ukeys and with collecting flows' last stats.
>>
>>Reported-by: Ilya Maximets 
>>Signed-off-by: Alex Wang 
>>
>>---
>>PATCH->V2:
>>- PATCH does not fix the problem,
>>- call the purge_cb between pmd thread stopping and deletion.
>>  (which should be the right place)
>>---
>> lib/dpif-netdev.c | 23 ---
>> lib/dpif-netlink.c|  1 +
>> lib/dpif-provider.h   | 11 ++-
>> lib/dpif.c|  8 
>> lib/dpif.h| 13 +
>> ofproto/ofproto-dpif-upcall.c | 36 
>> 6 files changed, 88 insertions(+), 4 deletions(-)
>>
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>index f841876..2d187da 100644
>>--- a/lib/dpif-netdev.c
>>+++ b/lib/dpif-netdev.c
>>@@ -204,6 +204,11 @@ struct dp_netdev {
>> upcall_callback *upcall_cb;  /* Callback function for executing
>>upcalls. */
>> void *upcall_aux;
>>
>>+/* Callback function for notifying the purging of dp flows (during
>>+ * reseting pmd deletion). */
>>+dp_purge_callback *dp_purge_cb;
>>+void *dp_purge_aux;
>>+
>> /* Stores all 'struct dp_netdev_pmd_thread's. */
>> struct cmap poll_threads;
>>
>>@@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
>>*pmd)
>> /* Stops the pmd thread, removes it from the 'dp->poll_threads',
>>  * and unrefs the struct. */
>> static void
>>-dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
>>+dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread *pmd)
>> {
>> /* Uninit the 'flow_cache' since there is
>>  * no actual thread uninit it for NON_PMD_CORE_ID. */
>>@@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
>> ovs_numa_unpin_core(pmd->core_id);
>> xpthread_join(pmd->thread, NULL);
>> }
>>+/* Purges the 'pmd''s flows after stoping the thread. */
>>+dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
>
> There's still the possibility a datapath is created through appctl
> dpctl/add-dp.
> In this case dp_purge_cb is NULL and a couple of tests fail (789, 790).
> Adding a check should be enough:
>
> /* Purges the 'pmd''s flows after stopping the thread. */
> if (dp->dp_purge_cb) {
> dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
> }
>
>
>
>
>> cmap_remove(&pmd->dp->poll_threads, &pmd->node,
>>hash_int(pmd->core_id, 0));
>> dp_netdev_pmd_unref(pmd);
>> }
>>@@ -2874,7 +2881,7 @@ dp_netdev_destroy_all_pmds(struct dp_netdev *dp)
>> struct dp_netdev_pmd_thread *pmd;
>>
>> CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
>>-dp_netdev_del_pmd(pmd);
>>+dp_netdev_del_pmd(dp, pmd);
>> }
>> }
>>
>>@@ -2886,7 +2893,7 @@ dp_netdev_del_pmds_on_numa(struct dp_netdev *dp,
>>int numa_id)
>>
>> CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
>> if (pmd->numa_id == numa_id) {
>>-dp_netdev_del_pmd(pmd);
>>+dp_netdev_del_pmd(dp, pmd);
>> }
>> }
>> }
>>@@ -3391,6 +3398,15 @@ struct dp_netdev_execute_aux {
>> };
>>
>> static void
>>+dpif_netdev_register_dp_purge_cb(s

Re: [ovs-dev] [RFC PATCH] userspace: Define and use struct eth_addr.

[Ben Pfaff]

On Thu, Aug 27, 2015 at 06:29:16PM -0700, Jarno Rajahalme wrote:
> Define struct eth_addr and use it instead of a uint8_t array for all
> ethernet addresses in OVS userspace.  The struct is always the right
> size, and it can be assigned without an explicit memcpy, which makes
> code more readable.
> 
> "struct eth_addr" is a good type name for this as many utility
> functions are already named accordingly.
> 
> struct eth_addr can be accessed as bytes as well as ovs_be16's, which
> makes the struct 16-bit aligned.  All use seems to be 16-bit aligned,
> so some algorithms on the ethernet addresses can be made a bit more
> efficient making use of this fact.
> 
> As the struct fits into a register (in 64-bit systems) we pass it by
> value when possible.
> 
> This patch also changes the few uses of Linux specific ETH_ALEN to
> OVS's own ETH_ADDR_LEN, and removes the OFP_ETH_ALEN, as it is no
> longer needed.
> 
> This work stemmed from a desire to make all struct flow members
> assignable for unrelated exploration purposes.  However, I think this
> might be a nice code readability improvement by itself.
> 
> Signed-off-by: Jarno Rajahalme 

I like this.  I've thought about doing the same thing before and never
got around to it.

I checked your claim about passing by value by looking at the x86-64
ABI.  It's true!  Older ABIs were not so flexible--for example, I seem
to recall that Borland C++ __fastcall ABI for Win32 would pass 1 and 2
and 4 byte structs in a register, but not 3-byte ones.

However, GCC is almost criminally bad at optimizing it:

blp@sigabrt:~/nicira/ovs/_build(0)$ cat tmp.c
struct x {
union {
unsigned char b[6];
unsigned short w[3];
};
};
void g(struct x);
void f(void)
{
struct x y = { { { 1, 2, 3, 4, 5, 6 } } };
g(y);
}

blp@sigabrt:~/nicira/ovs/_build(0)$ gcc -O2 -g -m64 -c tmp.c
blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o

tmp.o: file format elf64-x86-64


Disassembly of section .text:

 :
   0:   48 83 ec 18 sub$0x18,%rsp
   4:   c6 04 24 01 movb   $0x1,(%rsp)
   8:   c6 44 24 01 02  movb   $0x2,0x1(%rsp)
   d:   c6 44 24 02 03  movb   $0x3,0x2(%rsp)
  12:   c6 44 24 03 04  movb   $0x4,0x3(%rsp)
  17:   c6 44 24 04 05  movb   $0x5,0x4(%rsp)
  1c:   c6 44 24 05 06  movb   $0x6,0x5(%rsp)
  21:   48 8b 3c 24 mov(%rsp),%rdi
  25:   e8 00 00 00 00  callq  2a 
26: R_X86_64_PC32   g-0x4
  2a:   48 83 c4 18 add$0x18,%rsp
  2e:   c3  retq   

Clang does better:

blp@sigabrt:~/nicira/ovs/_build(0)$ clang -O2 -g -m64 -c tmp.c
blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o

tmp.o: file format elf64-x86-64


Disassembly of section .text:

 :
   0:   48 bf 01 02 03 04 05movabs $0x60504030201,%rdi
   7:   06 00 00 
   a:   e9 00 00 00 00  jmpq   f 
b: R_X86_64_PC32g-0x4

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

Re: [ovs-dev] [PATCH] datapath-windows: Enable failure after restarting extension

[Ben Pfaff]

On Thu, Aug 27, 2015 at 09:02:17PM +, Nithin Raju wrote:
> > Original Message-
> > From: Nithin Raju [mailto:nit...@vmware.com] 
> > Sent: Monday, 3 August, 2015 20:22
> > To: Sorin Vinturis
> > Cc: dev@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH] datapath-windows: Enable failure after 
> > restarting extension
> > 
> >> On Jul 15, 2015, at 7:50 AM, Sorin Vinturis 
> >>  wrote:
> >> 
> >> If the extension was previously enabled and running, after issuing a 
> >> restart, stop+start, the extension fails to be enabled. This happens 
> >> because the extension's DeviceObject is not yet initialized before the 
> >> FilterAttach routine is called.
> >> 
> >> This patch addresses this issue.
> >> 
> >> Signed-off-by: Sorin Vinturis 
> >> Reported-by: Sorin Vinturis 
> 
> hi Sorin,
> Thanks for the explanation. The race condition is indeed very intricate.
> 
> Acked-by: Nithin Raju 

Thanks Sorin and Nithin, I applied this to master and branch-2.4.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3] tests: Avoid nonportable "sed -i".

[Ben Pfaff]

Thanks Alex, I applied this to master.

On Thu, Aug 27, 2015 at 06:21:03PM -0700, Alex Wang wrote:
> Looks good to me,
> 
> Thx for fixing it~
> 
> On Thu, Aug 27, 2015 at 11:11 AM, Ben Pfaff  wrote:
> 
> > "sed -i" isn't entirely portable, and we can avoid it by using the
> > argument to check_logs as intended.
> >
> > Signed-off-by: Ben Pfaff 
> > Acked-by: Alex Wang 
> > ---
> > v1->v2: Retain $1 in check_logs call from OVN_CONTROLLER_VTEP_STOP.
> >   Also drop other, now-unneeded, call to "sed -i".
> > v2->v3: Merge multiple check_logs arguments into one, since it only
> >   supports one.
> >
> >  tests/ovn-controller-vtep.at | 14 +-
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> > index 9fc1526..063feeb 100644
> > --- a/tests/ovn-controller-vtep.at
> > +++ b/tests/ovn-controller-vtep.at
> > @@ -89,8 +89,8 @@ m4_define([OVN_CONTROLLER_VTEP_STOP],
> > # sending update back to *ctl command if *ctl has not proceeded to
> > exit yet.
> > # and if *ctl command exits before database calling send, the send from
> > # database will fail with 'Broken pipe' error.
> > -   AT_CHECK([sed -i '/Broken pipe/d' ovsdb-server.log])
> > -   AT_CHECK([check_logs $1])
> > +   AT_CHECK([check_logs "$1
> > +/Broken pipe/d"])
> > AT_CHECK([ovs-appctl -t ovs-vtep exit])
> > AT_CHECK([ovs-appctl -t ovn-northd exit])
> > AT_CHECK([ovs-appctl -t ovn-controller-vtep exit])
> > @@ -131,10 +131,6 @@ OVS_WAIT_UNTIL([test -n "`grep WARN
> > ovn-controller-vtep.log`"])
> >  AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' ovn-controller-vtep.log], [0],
> > [dnl
> >  |WARN|Chassis for VTEP physical switch (br-vtep) disappears, maybe
> > deleted by ovn-sbctl, adding it back
> >  ])
> > -# this removal of chassis could cause 'Broken pipe' warning in the
> > ovsdb-server.log,
> > -# due to the race between 'ovn-sbctl' exiting and 'ovn-controller-vtep'
> > adding
> > -# the chassis back.  so just removes the 'Broken pipe' warning from
> > ovsdb-server.log.
> > -AT_CHECK([sed -i '/Broken pipe/d' ovsdb-server.log])
> >
> >  # changes the tunnel_ip on physical switch, watches the update of
> > chassis's
> >  # encap.
> > @@ -179,7 +175,7 @@ AT_CHECK([ovn-sbctl --columns=vtep_logical_switches
> > list Chassis | cut -d ':' -f
> >  [[]]
> >  ])
> >
> > -OVN_CONTROLLER_VTEP_STOP(["/Chassis for VTEP physical switch (br-vtep)
> > disappears/d"])
> > +OVN_CONTROLLER_VTEP_STOP([/Chassis for VTEP physical switch (br-vtep)
> > disappears/d])
> >  AT_CLEANUP
> >
> >
> > @@ -242,7 +238,7 @@ AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list
> > Port_Binding | cut -d ':' -f
> >  [[]]
> >  ])
> >
> > -OVN_CONTROLLER_VTEP_STOP(["/has already been associated with logical
> > port/d"])
> > +OVN_CONTROLLER_VTEP_STOP([/has already been associated with logical
> > port/d])
> >  AT_CLEANUP
> >
> >
> > @@ -282,5 +278,5 @@ AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list
> > Port_Binding br-vtep_lswitch
> >  ${chassis_uuid}
> >  ])
> >
> > -OVN_CONTROLLER_VTEP_STOP(["/has already been associated with logical
> > datapath/d"])
> > +OVN_CONTROLLER_VTEP_STOP([/has already been associated with logical
> > datapath/d])
> >  AT_CLEANUP
> > --
> > 2.1.3
> >
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: Surpress flow attribute probe

[Ben Pfaff]

On Thu, Aug 27, 2015 at 07:57:14PM +, Nithin Raju wrote:
> > This patch surpresses flow attribute probing in the windows datapath.
> > 
> > Signed-off-by: Alin Gabriel Serdean 
> 
> The change look good. Only thing I’d avoid is the OVS_LOG_ERROR()
> statement. The documentation for ‘OVS_FLOW_ATTR_PROBE’ explicitly
> mentions that logging should be suppressed.

When it's supported, it's supposed to suppress logging of *other*
errors.  Since it *isn't* supported, I think it's fair game to log that.

> Acked-by: Nithin Raju 

Thanks Alin and Nithin, I'll apply this to master and branch-2.4 in a
minute.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers

[Pravin Shelar]

On Thu, Aug 27, 2015 at 12:10 AM, Simon Horman
 wrote:
> Hi Pravin,
>
> On Mon, Aug 17, 2015 at 11:33:59AM -0700, Pravin Shelar wrote:
>> On Thu, Aug 13, 2015 at 6:30 PM, Simon Horman
>>  wrote:
>> > When an error occurs skipping IPv6 extension headers retain the already
>> > parsed IP protocol and IPv6 addresses in the flow. Also assume that the
>> > packet is not a fragment in the absence of information to the contrary;

...
> -- >8 --
> Subject: [PATCH v1.1] openvswitch: Retain parsed IPv6 header fields in flow 
> on error skipping extension headers
>
> When an error occurs skipping IPv6 extension headers retain the already
> parsed IP protocol and IPv6 addresses in the flow. Also assume that the
> packet is not a fragment in the absence of information to the contrary;
> that is always use the frag_off value set by ipv6_skip_exthdr().
>
> This allows matching on the IP protocol and IPv6 addresses of packets
> with malformed extension headers.
>
> Signed-off-by: Simon Horman 
>
> ---
>
> * Some consideration should be given to unwanted side effects of this patch
>   as it affects the handling of malformed packets.
>
> Signed-off-by: Simon Horman 
> ---
>  net/openvswitch/flow.c | 21 +++--
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 8db22ef73626..de4366f81b11 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -271,8 +271,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct 
> sw_flow_key *key)
> key->ipv6.addr.dst = nh->daddr;
>
> payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off);
> -   if (unlikely(payload_ofs < 0))
> -   return -EINVAL;
>
> if (frag_off) {
> if (frag_off & htons(~0x7))
> @@ -283,6 +281,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct 
> sw_flow_key *key)
> key->ip.frag = OVS_FRAG_TYPE_NONE;
> }
>
> +   /* Delayed handling of error in ipv6_skip_exthdr() as it
> +* always sets frag_off to a valid value which may be
> +* used to set key->ip.frag above.
> +*/
> +   if (unlikely(payload_ofs < 0))
> +   return -EPROTO;
> +
> nh_len = payload_ofs - nh_ofs;
> skb_set_transport_header(skb, nh_ofs + nh_len);
> key->ip.proto = nexthdr;
> @@ -622,12 +627,16 @@ static int key_extract(struct sk_buff *skb, struct 
> sw_flow_key *key)
>
> nh_len = parse_ipv6hdr(skb, key);
> if (unlikely(nh_len < 0)) {
> -   memset(&key->ip, 0, sizeof(key->ip));
> -   memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr));
> -   if (nh_len == -EINVAL) {
> +   switch (nh_len) {
> +   case -EINVAL:
> +   memset(&key->ip, 0, sizeof(key->ip));
> +   memset(&key->ipv6.addr, 0, 
> sizeof(key->ipv6.addr));
> +   /* fall-through */
> +   case -EPROTO:
> skb->transport_header = skb->network_header;
> error = 0;
> -   } else {
> +   break;
> +   default:
> error = nh_len;
> }
> return error;
Looks good to me.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofp-actions: Don't encode variable length fields using NXAST_REG_LOAD.

[Ben Pfaff]

On Thu, Aug 27, 2015 at 07:06:51PM -0700, Jesse Gross wrote:
> Currently, when using an OpenFlow 1.0 connection to encode a
> tunnel metadata set field action, a series of NXAST_REG_LOADs
> are emitted. The result is something like this:
> 
> actions=load:0xa->NXM_NX_TUN_METADATA0[0..63],load:0->
> NXM_NX_TUN_METADATA0[64..127],load:0->NXM_NX_TUN_METADATA0[128..191],
> load:0->NXM_NX_TUN_METADATA0[192..255],load:0->NXM_NX_TUN_METADATA0
> [256..319],load:0->NXM_NX_TUN_METADATA0[320..383],load:0->
> NXM_NX_TUN_METADATA0[384..447],load:0->NXM_NX_TUN_METADATA0[448..511],
> load:0->NXM_NX_TUN_METADATA0[512..575],load:0->NXM_NX_TUN_METADATA0
> [576..639],load:0->NXM_NX_TUN_METADATA0[640..703],load:0->
> NXM_NX_TUN_METADATA0[704..767],load:0->NXM_NX_TUN_METADATA0[768..831],
> load:0->NXM_NX_TUN_METADATA0[832..895],load:0->NXM_NX_TUN_METADATA0
> [896..959],load:0->NXM_NX_TUN_METADATA0[960..991]
> 
> This happens because tunnel metadata is seen as a maximum size field
> and so many loads need to be emitted to cover the entire thing. Besides
> being ugly (this shows up when using ovs-ofctl in the default
> configuration), it exposes the internal size of the field. While this
> shouldn't be an issue since specific protocol fields (such as Geneve
> options) have fixed max sizes even if the OVS implementation is extended,
> it's still not a great idea.
> 
> If we instead use NXAST_REG_LOAD2 in cases where there isn't a suitable
> OpenFlow alternative, both problems are avoided:
> 
> actions=set_field:0xa->tun_metadata0
> 
> This prefers NXAST_REG_LOAD2 for variable length fields since they would
> all generally have the same problems. In addition, since the concept of
> this type of field is fairly new, there are no backwards compatibility
> issues.
> 
> Signed-off-by: Jesse Gross 

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

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[Daniele Di Proietto]

On 28/08/2015 18:51, "Joe Stringer"  wrote:

>On 28 August 2015 at 09:41, Daniele Di Proietto 
>wrote:
>> I've tested it and it appears to correctly keep all the stats.
>>
>> Two comments inline, otherwise:
>>
>> Acked-by: Daniele Di Proietto 
>
>That could even be... dare I say it... "Tested-by: ..." :-)

Oops... Fair enough,  I was too eager to get the fix in.
Thanks for the comments, I'll let you handle the revalidator issues
then! :-)

>
>
>In general this patch seems fine. I'm not entirely confident what the
>worst case behaviour is though - for instance if revalidators are
>crazy busy (eg disable megaflows with port scan) and you reconfigure
>pmd threads, does it handle this gracefully? As far as I can tell,
>there is nothing stopping a revalidator from trying to delete the same
>ukey+flow at the same time. Also, typical purging like this happens
>independently by each revalidator thread where no thread can handle
>the same ukeys as another thread. This instead goes through all cmaps
>and picks out those with the current pmd id.
>
>The safest thing is to stop the world (ie, at least stop revalidators
>from processing further, if not completely disable the threads then
>re-enable when it's safe again), and an approach like that seems
>simpler.
>
>
>> On 28/08/2015 06:25, "Alex Wang"  wrote:
>>
>>>When dpdk configuration changes, all pmd threads are recreated
>>>and rx queues of each port are reloaded.  After this process,
>>>rx queue could be mapped to a different pmd thread other than
>>>the one before reconfiguration.  However, this is totally
>>>transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
>>>module still holds ukeys generated before pmd thread recreation,
>>>this old ukey will collide with the ukey for the new upcalls
>>>from same traffic flow, causing flow installation failure.
>>>
>>>To fix the bug, this commit adds a new call-back function
>>>in dpif layer for notifying upper layer the purging of datapath
>>>(e.g. pmd thread deletion in dpif-netdev).  So, the
>>>ofproto-dpif-upcall module can react properly with deleting
>>>the ukeys and with collecting flows' last stats.
>>>
>>>Reported-by: Ilya Maximets 
>>>Signed-off-by: Alex Wang 
>>>
>>>---
>>>PATCH->V2:
>>>- PATCH does not fix the problem,
>>>- call the purge_cb between pmd thread stopping and deletion.
>>>  (which should be the right place)
>>>---
>>> lib/dpif-netdev.c | 23 ---
>>> lib/dpif-netlink.c|  1 +
>>> lib/dpif-provider.h   | 11 ++-
>>> lib/dpif.c|  8 
>>> lib/dpif.h| 13 +
>>> ofproto/ofproto-dpif-upcall.c | 36 
>>> 6 files changed, 88 insertions(+), 4 deletions(-)
>>>
>>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>>index f841876..2d187da 100644
>>>--- a/lib/dpif-netdev.c
>>>+++ b/lib/dpif-netdev.c
>>>@@ -204,6 +204,11 @@ struct dp_netdev {
>>> upcall_callback *upcall_cb;  /* Callback function for executing
>>>upcalls. */
>>> void *upcall_aux;
>>>
>>>+/* Callback function for notifying the purging of dp flows (during
>>>+ * reseting pmd deletion). */
>>>+dp_purge_callback *dp_purge_cb;
>>>+void *dp_purge_aux;
>>>+
>>> /* Stores all 'struct dp_netdev_pmd_thread's. */
>>> struct cmap poll_threads;
>>>
>>>@@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
>>>*pmd)
>>> /* Stops the pmd thread, removes it from the 'dp->poll_threads',
>>>  * and unrefs the struct. */
>>> static void
>>>-dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
>>>+dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread
>>>*pmd)
>>> {
>>> /* Uninit the 'flow_cache' since there is
>>>  * no actual thread uninit it for NON_PMD_CORE_ID. */
>>>@@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread
>>>*pmd)
>>> ovs_numa_unpin_core(pmd->core_id);
>>> xpthread_join(pmd->thread, NULL);
>>> }
>>>+/* Purges the 'pmd''s flows after stoping the thread. */
>>>+dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
>>
>> There's still the possibility a datapath is created through appctl
>> dpctl/add-dp.
>> In this case dp_purge_cb is NULL and a couple of tests fail (789, 790).
>> Adding a check should be enough:
>>
>> /* Purges the 'pmd''s flows after stopping the thread. */
>> if (dp->dp_purge_cb) {
>> dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
>> }
>>
>>
>>
>>
>>> cmap_remove(&pmd->dp->poll_threads, &pmd->node,
>>>hash_int(pmd->core_id, 0));
>>> dp_netdev_pmd_unref(pmd);
>>> }
>>>@@ -2874,7 +2881,7 @@ dp_netdev_destroy_all_pmds(struct dp_netdev *dp)
>>> struct dp_netdev_pmd_thread *pmd;
>>>
>>> CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
>>>-dp_netdev_del_pmd(pmd);
>>>+dp_netdev_del_pmd(dp, pmd);
>>> }
>>> }
>>>
>>>@@ -2886,7 +2893,7 @@ dp_netdev_del_pmds_on_numa(struct dp_netdev *dp,
>>>int numa_id)
>>>
>>> 

Re: [ovs-dev] [PATCH] ofp-actions: Don't encode variable length fields using NXAST_REG_LOAD.

[Jesse Gross]

On Fri, Aug 28, 2015 at 11:15 AM, Ben Pfaff  wrote:
> On Thu, Aug 27, 2015 at 07:06:51PM -0700, Jesse Gross wrote:
>> Currently, when using an OpenFlow 1.0 connection to encode a
>> tunnel metadata set field action, a series of NXAST_REG_LOADs
>> are emitted. The result is something like this:
>>
>> actions=load:0xa->NXM_NX_TUN_METADATA0[0..63],load:0->
>> NXM_NX_TUN_METADATA0[64..127],load:0->NXM_NX_TUN_METADATA0[128..191],
>> load:0->NXM_NX_TUN_METADATA0[192..255],load:0->NXM_NX_TUN_METADATA0
>> [256..319],load:0->NXM_NX_TUN_METADATA0[320..383],load:0->
>> NXM_NX_TUN_METADATA0[384..447],load:0->NXM_NX_TUN_METADATA0[448..511],
>> load:0->NXM_NX_TUN_METADATA0[512..575],load:0->NXM_NX_TUN_METADATA0
>> [576..639],load:0->NXM_NX_TUN_METADATA0[640..703],load:0->
>> NXM_NX_TUN_METADATA0[704..767],load:0->NXM_NX_TUN_METADATA0[768..831],
>> load:0->NXM_NX_TUN_METADATA0[832..895],load:0->NXM_NX_TUN_METADATA0
>> [896..959],load:0->NXM_NX_TUN_METADATA0[960..991]
>>
>> This happens because tunnel metadata is seen as a maximum size field
>> and so many loads need to be emitted to cover the entire thing. Besides
>> being ugly (this shows up when using ovs-ofctl in the default
>> configuration), it exposes the internal size of the field. While this
>> shouldn't be an issue since specific protocol fields (such as Geneve
>> options) have fixed max sizes even if the OVS implementation is extended,
>> it's still not a great idea.
>>
>> If we instead use NXAST_REG_LOAD2 in cases where there isn't a suitable
>> OpenFlow alternative, both problems are avoided:
>>
>> actions=set_field:0xa->tun_metadata0
>>
>> This prefers NXAST_REG_LOAD2 for variable length fields since they would
>> all generally have the same problems. In addition, since the concept of
>> this type of field is fairly new, there are no backwards compatibility
>> issues.
>>
>> Signed-off-by: Jesse Gross 
>
> Acked-by: Ben Pfaff 

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

[ovs-dev] Windows build failure

[Ben Pfaff]

There must have been an issue in one of the Windows patches I applied
this morning, either in the patch or how I applied it (I resolved a
patch conflict by hand and perhaps I did not read it carefully enough).
I'd appreciate a fix.

Thanks,

Ben.

- Forwarded message from AppVeyor  -

Date: Fri, 28 Aug 2015 18:25:43 +
From: AppVeyor 
To: bu...@openvswitch.org
Subject: [ovs-build] Build failed: ovs 1.0.633




https://ci.appveyor.com/project/blp/ovs/build/1.0.633"; 
style="color:#ff3228;">Build ovs 1.0.633 failed


Commit https://github.com/openvswitch/ovs/commit/abd0694c5d";>abd0694c5d by 
mailto:aserd...@cloudbasesolutions.com";>Alin Serdean on 8/28/2015 
6:09 PM:

datapath-windows: Suppress 
flow attribute probe.


https://ci.appveyor.com/notifications"; 
style="font-size:85%;color:#999;">Configure your notification 
preferences


___
build mailing list
bu...@openvswitch.org
http://www.openvswitch.org/mailman/listinfo/build


- End forwarded message -
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] rhel: Add variables for OVN and VTEP db locations.

[Ben Pfaff]

On Thu, Aug 27, 2015 at 04:40:13PM -0300, Flavio Leitner wrote:
> On Wed, Aug 26, 2015 at 05:46:55PM -0400, Russell Bryant wrote:
> > Most real deployments will need to customize the database locations
> > for ovn-controller and ovn-controller-vtep.  Instead of making them
> > override the entire command used to start the daemons, provide and
> > document some environment variables that can be overridden in a custom
> > config file.
> > 
> > Signed-off-by: Russell Bryant 
> > ---
> 
> Acked-by: Flavio Leitner 

Thanks Russell and Flavio, I applied these to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Windows build failure

[Nithin Raju]

Ben, I'll take a look. Thanks for letting us know.
_
From: Ben Pfaff mailto:b...@nicira.com>>
Sent: , ?? 28, 2015 11:38
Subject: [ovs-dev] Windows build failure
To: mailto:dev@openvswitch.org>>


There must have been an issue in one of the Windows patches I applied
this morning, either in the patch or how I applied it (I resolved a
patch conflict by hand and perhaps I did not read it carefully enough).
I'd appreciate a fix.

Thanks,

Ben.

- Forwarded message from AppVeyor 
mailto:no-re...@appveyor.com>> -

Date: Fri, 28 Aug 2015 18:25:43 +
From: AppVeyor mailto:no-re...@appveyor.com>>
To: bu...@openvswitch.org
Subject: [ovs-build] Build failed: ovs 1.0.633




https://urldefense.proofpoint.com/v2/url?u=https-3A__ci.appveyor.com_project_blp_ovs_build_1.0.633&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=FJBlZNCG3ts9zZ_NnjTez0q9zGxyNS8VzB0UhUNEoH0&s=-JESwDUq2PHfXaVuOU5mgGsH7-hpBpdcoD8TyTV95lE&e=
 " style="color:#ff3228;">Build ovs 1.0.633 failed


Commit https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_commit_abd0694c5d&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=FJBlZNCG3ts9zZ_NnjTez0q9zGxyNS8VzB0UhUNEoH0&s=JuN3RBJkUegt6P0yB_IYQQuouNLGuwcZZUtrlUQHrYU&e=
 ">abd0694c5d by mailto:aserd...@cloudbasesolutions.com";>Alin 
Serdean on 8/28/2015 6:09 PM:

datapath-windows: Suppress flow 
attribute probe.


https://urldefense.proofpoint.com/v2/url?u=https-3A__ci.appveyor.com_notifications&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=FJBlZNCG3ts9zZ_NnjTez0q9zGxyNS8VzB0UhUNEoH0&s=F8zQdsFHHO25_TghX1qUjnPcZSQvUj4nA9dEyrbS19E&e=
 " style="font-size:85%;color:#999;">Configure your notification 
preferences


___
build mailing list
bu...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__www.openvswitch.org_mailman_listinfo_build&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=FJBlZNCG3ts9zZ_NnjTez0q9zGxyNS8VzB0UhUNEoH0&s=pSHr-SiEd4rc5ScHrc-Oa78x88aw_i9dDmv--pfoxPY&e=


- End forwarded message -
___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=FJBlZNCG3ts9zZ_NnjTez0q9zGxyNS8VzB0UhUNEoH0&s=-bbNeI-C5nNrxYmaqpdom54IrLPztUSoX1zHYbEwGAc&e=


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

[ovs-dev] [PATCH v2] ovs-vsctl: add command to delete transient ports from bridge

[Thadeu Lima de Souza Cascardo]

When using virtualization, new ports are created and removed all the time. These
ports do not persist after a system reboot, for example. They may be created
again by the virtualization manager, but that will happen after the vswitch is
already running, and the virtualization manager will add them again to the
bridge.

If a reboot happens without properly deleting such ports, all kinds of errors
will happen. The absence of the ports will be logged as errors, and adding those
ports again to the database will fail.

This patch introduces the notion of transient ports. Ports may be added as
transient, as a boolean in other_config smap. When openvswitch is restarted by
using --delete-transient-ports ovs-ctl option, all transient ports will be
removed. If the system administrator wants to remove all transient ports from a
bridge, a new ovs-vsctl command, del-transient-ports may be used.

Signed-off-by: Thadeu Lima de Souza Cascardo 
---
 tests/ovs-vsctl.at   | 21 +
 utilities/ovs-ctl.in |  6 ++
 utilities/ovs-vsctl.8.in |  3 +++
 utilities/ovs-vsctl.c| 30 ++
 vswitchd/vswitch.xml |  8 
 5 files changed, 68 insertions(+)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index fef7b88..f877070 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -304,6 +304,27 @@ CHECK_IFACES([b], [b1])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
+AT_SETUP([add-br a, add-transient-port a a1 a2, del-transient-ports a])
+AT_KEYWORDS([ovs-vsctl del-transient-ports])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL(
+   [add-br a],
+   [--if-exists del-br b],
+   [add-port a a1],
+   [add-port a a2],
+   [set port a1 other_config:transient=true],
+   [set port a2 other_config:transient=true])], [0], [], [], 
[OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a], [a1], [a2])
+CHECK_IFACES([a], [a1], [a2])
+AT_CHECK([RUN_OVS_VSCTL(
+   [del-transient-ports a])], [0], [], [], [OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a])
+CHECK_IFACES([a])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
 AT_SETUP([add-br a, add-bond a bond0 a1 a2 a3])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 9bbbe0d..5beb0a2 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -219,6 +219,11 @@ start_ovsdb () {
 ovs_vsctl del-br $bridge
 done
 fi
+if test X"$DELETE_TRANSIENT_PORTS" = Xyes; then
+for bridge in `ovs_vsctl list-br`; do
+ovs_vsctl del-transient-ports $bridge
+done
+fi
 fi
 }
 
@@ -536,6 +541,7 @@ set_defaults () {
 SYSTEM_ID=
 
 DELETE_BRIDGES=no
+DELETE_TRANSIENT_PORTS=no
 
 DAEMON_CWD=/
 FORCE_COREFILES=yes
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 265ffde..6229312 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -323,6 +323,9 @@ no effect.
 Prints the name of the bridge that contains \fIport\fR on standard
 output.
 .
+.IP "\fBdel\-transient\-ports \fIbridge\fR"
+Deletes ports configured as transient from the specified bridge.
+.
 .SS "Interface Commands"
 .
 These commands examine the interfaces attached to an Open vSwitch
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index e177060..4526c90 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -380,6 +380,7 @@ Port commands (a bond is considered to be a single port):\n\
   add-bond BRIDGE PORT IFACE...  add bonded port PORT in BRIDGE from IFACES\n\
   del-port [BRIDGE] PORT  delete PORT (which may be bonded) from BRIDGE\n\
   port-to-br PORT print name of bridge that contains PORT\n\
+  del-transient-ports BRIDGE  delete transient ports from BRIDGE\n\
 \n\
 Interface commands (a bond consists of multiple interfaces):\n\
   list-ifaces BRIDGE  print the names of all interfaces on BRIDGE\n\
@@ -1678,6 +1679,33 @@ cmd_del_port(struct ctl_context *ctx)
 }
 
 static void
+pre_transient_ports(struct ctl_context *ctx)
+{
+pre_get_info(ctx);
+ovsdb_idl_add_column(ctx->idl, &ovsrec_port_col_other_config);
+}
+
+static void
+cmd_del_transient_ports(struct ctl_context *ctx)
+{
+struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
+const char *target = ctx->argv[1];
+struct vsctl_bridge *bridge;
+struct vsctl_port *port, *next_port;
+
+vsctl_context_populate_cache(ctx);
+bridge = find_bridge(vsctl_ctx, target, true);
+
+LIST_FOR_EACH_SAFE (port, next_port, ports_node, &bridge->ports) {
+struct ovsrec_port *cfg = port->port_cfg;
+bool transient = smap_get_bool(&cfg->other_config, "transient", false);
+if (transient) {
+del_port(vsctl_ctx, port);
+}
+}
+}
+
+static void
 cmd_port_to_br(struct ctl_context *ctx)
 {
 struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
@@ -2732,6 +2760,8 @@ static const struct ctl_command_syntax vsctl_commands[] = 
{
  

[ovs-dev] [PATCH] datapatb-windows: unbreak hyperv DP compilation

[Nithin Raju]

Breakage caused by commit: abd0694c5de8201b68dca2b393adf054b0fb1d2c.
Not a merge issue.

Signed-off-by: Nithin Raju 
---
 datapath-windows/ovsext/Flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 8117cf7..a8f0703 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -310,7 +310,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
goto done;
 }
 
-if (flowAttrs[OVS_FLOW_ATTR_PROBE]) {
+if (nlAttrs[OVS_FLOW_ATTR_PROBE]) {
 OVS_LOG_ERROR("Attribute OVS_FLOW_ATTR_PROBE not supported");
 nlError = NL_ERROR_NOENT;
 goto done;
-- 
1.8.5.6

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

[ovs-dev] [PATCH] datapath-windows: Fix Build

[Alin Serdean]

Change variable name from nlAttrs to flowAttrs.


Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Flow.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 8117cf7..58bdd42 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -254,7 +254,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg);
 PGENL_MSG_HDR genlMsgHdr = &(msgIn->genlMsg);
 POVS_HDR ovsHdr = &(msgIn->ovsHdr);
-PNL_ATTR nlAttrs[__OVS_FLOW_ATTR_MAX];
+PNL_ATTR flowAttrs[__OVS_FLOW_ATTR_MAX];
 UINT32 attrOffset = NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN;
 OvsFlowPut mappedFlow;
 OvsFlowStats stats;
@@ -275,7 +275,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 /* Get all the top level Flow attributes */
 if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr),
  nlFlowPolicy, ARRAY_SIZE(nlFlowPolicy),
- nlAttrs, ARRAY_SIZE(nlAttrs)))
+ flowAttrs, ARRAY_SIZE(flowAttrs)))
  != TRUE) {
 OVS_LOG_ERROR("Attr Parsing failed for msg: %p",
nlMsgHdr);
@@ -285,7 +285,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 
 /* FLOW_DEL command w/o any key input is a flush case. */
 if ((genlMsgHdr->cmd == OVS_FLOW_CMD_DEL) &&
-(!(nlAttrs[OVS_FLOW_ATTR_KEY]))) {
+(!(flowAttrs[OVS_FLOW_ATTR_KEY]))) {
 
 rc = OvsFlushFlowIoctl(ovsHdr->dp_ifindex);
 
@@ -316,8 +316,8 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 goto done;
 }
 
-if ((rc = _MapNlToFlowPut(msgIn, nlAttrs[OVS_FLOW_ATTR_KEY],
- nlAttrs[OVS_FLOW_ATTR_ACTIONS], nlAttrs[OVS_FLOW_ATTR_CLEAR],
+if ((rc = _MapNlToFlowPut(msgIn, flowAttrs[OVS_FLOW_ATTR_KEY],
+flowAttrs[OVS_FLOW_ATTR_ACTIONS], flowAttrs[OVS_FLOW_ATTR_CLEAR],
  &mappedFlow))
 != STATUS_SUCCESS) {
 OVS_LOG_ERROR("Conversion to OvsFlowPut failed");
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V2] bridge: Relax the whitelist format for punix path.

[Ben Pfaff]

On Fri, Aug 28, 2015 at 07:31:21AM +, Alex Wang wrote:
> This commit relaxes the whitelist format for punix path for
> service controller.  Instead of only allowing
> punix:/.controller, the new format
> allows any suffix, like punix:/.*.
> (except '/').
> 
> Signed-off-by: Alex Wang 
> ---
> PATCH->V2:
> - prevent the punix path from specifying directory other than the
>   ovs_rundir.

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

Re: [ovs-dev] [PATCH] datapatb-windows: unbreak hyperv DP compilation

[Ben Pfaff]

On Fri, Aug 28, 2015 at 01:44:20PM -0700, Nithin Raju wrote:
> Breakage caused by commit: abd0694c5de8201b68dca2b393adf054b0fb1d2c.
> Not a merge issue.
> 
> Signed-off-by: Nithin Raju 

I see that there are now two fixes for this build failure.  Thanks!

Do you guys have an opinion on which one is preferred?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapatb-windows: unbreak hyperv DP compilation

[Nithin Raju]

> On Aug 28, 2015, at 2:03 PM, Ben Pfaff  wrote:
> 
> On Fri, Aug 28, 2015 at 01:44:20PM -0700, Nithin Raju wrote:
>> Breakage caused by commit: abd0694c5de8201b68dca2b393adf054b0fb1d2c.
>> Not a merge issue.
>> 
>> Signed-off-by: Nithin Raju 
> 
> I see that there are now two fixes for this build failure.  Thanks!
> 
> Do you guys have an opinion on which one is preferred?

I don’t have any preference. I’ll let Alin make the call.

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

[ovs-dev] [PATCH] appveyor: Renew SSL link.

[Alin Serdean]

1_0_2a version not available for download.

Signed-off-by: Alin Gabriel Serdean 
---
 appveyor.yml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/appveyor.yml b/appveyor.yml
index 370de3f..d3d87b3 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -15,9 +15,9 @@ init:
 
 Invoke-WebRequest $source -OutFile $destination
 
-$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2a.exe";
+$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2d.exe";
 
-$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2a.exe"
+$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2d.exe"
 
 Invoke-WebRequest $source -OutFile $destination
 
@@ -27,7 +27,7 @@ init:
 
 cd C:\ovs-build-downloads
 
-.\Win32OpenSSL-1_0_2a.exe /silent /verysilent /sp- /suppressmsgboxes
+.\Win32OpenSSL-1_0_2d.exe /silent /verysilent /sp- /suppressmsgboxes
 
 Start-Sleep -s 30
 
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: Fix Build

[Nithin Raju]

> On Aug 28, 2015, at 1:55 PM, Alin Serdean  
> wrote:
> 
> Change variable name from nlAttrs to flowAttrs.
> 
> 
> Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] datapatb-windows: unbreak hyperv DP compilation

[Alin Serdean]

I would like mine better just for the variable name.

Alin.

-Mesaj original-
De la: Nithin Raju [mailto:nit...@vmware.com] 
Trimis: Saturday, August 29, 2015 12:10 AM
Către: Ben Pfaff 
Cc: Alin Serdean ; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] datapatb-windows: unbreak hyperv DP compilation

> On Aug 28, 2015, at 2:03 PM, Ben Pfaff  wrote:
> 
> On Fri, Aug 28, 2015 at 01:44:20PM -0700, Nithin Raju wrote:
>> Breakage caused by commit: abd0694c5de8201b68dca2b393adf054b0fb1d2c.
>> Not a merge issue.
>> 
>> Signed-off-by: Nithin Raju 
> 
> I see that there are now two fixes for this build failure.  Thanks!
> 
> Do you guys have an opinion on which one is preferred?

I don’t have any preference. I’ll let Alin make the call.

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

Re: [ovs-dev] [PATCH] appveyor: Renew SSL link.

[Gurucharan Shetty]

On Fri, Aug 28, 2015 at 2:11 PM, Alin Serdean
 wrote:
> 1_0_2a version not available for download.
>
> Signed-off-by: Alin Gabriel Serdean 
Applied, thanks!
> ---
>  appveyor.yml | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/appveyor.yml b/appveyor.yml
> index 370de3f..d3d87b3 100644
> --- a/appveyor.yml
> +++ b/appveyor.yml
> @@ -15,9 +15,9 @@ init:
>
>  Invoke-WebRequest $source -OutFile $destination
>
> -$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2a.exe";
> +$source = "https://slproweb.com/download/Win32OpenSSL-1_0_2d.exe";
>
> -$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2a.exe"
> +$destination = "C:\ovs-build-downloads\Win32OpenSSL-1_0_2d.exe"
>
>  Invoke-WebRequest $source -OutFile $destination
>
> @@ -27,7 +27,7 @@ init:
>
>  cd C:\ovs-build-downloads
>
> -.\Win32OpenSSL-1_0_2a.exe /silent /verysilent /sp- /suppressmsgboxes
> +.\Win32OpenSSL-1_0_2d.exe /silent /verysilent /sp- /suppressmsgboxes
>
>  Start-Sleep -s 30
>
> --
> 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

Re: [ovs-dev] [PATCH] tunnel: Support matching on the presence of Geneve options.

[Jarno Rajahalme]

Jesse,

Some comments and a possible bug below, otherwise looks good.

Acked-by: Jarno Rajahalme 

I recall Ben has reviewed the earlier patches in this domain, so maybe it would 
be good to get his Ack as well.

  Jarno

> On Aug 28, 2015, at 9:39 AM, Jesse Gross  wrote:
> 
> Sometimes it is useful to match only on whether a Geneve option
> is present even if the specific value is unimportant. A special
> case of this is zero length options where there is no value at all
> and the only information conveyed is whether the option was included
> in the packet.
> 
> This operation was partially supported before but it was not consistent -
> in particular, options were never serialized through NXM/OXM unless
> they had a non-zero mask. Furthermore, zero length options were rejected
> altogether when they were installed through the Geneve map OpenFlow
> command.
> 
> This adds support for these types of matches by making any NXM/OXM for
> tunnel metadata force a match on that field. In the case of a zero length
> option, both the value and mask of the NXM are ignored.
> 
> Signed-off-by: Jesse Gross 
> ---
> lib/bitmap.h |  2 ++
> lib/meta-flow.c  | 46 
> lib/meta-flow.h  |  3 +-
> lib/nx-match.c   | 43 +-
> lib/nx-match.h   |  6 ++--
> lib/odp-util.c   |  3 +-
> lib/ofp-parse.c  | 21 ++---
> lib/ofp-util.c   |  3 +-
> lib/tun-metadata.c   | 72 ++--
> lib/tun-metadata.h   | 11 ++-
> ofproto/ofproto-dpif-xlate.c |  5 +++
> tests/ovs-ofctl.at   |  1 +
> tests/tunnel.at  | 42 +-
> utilities/ovs-ofctl.8.in |  7 -
> 14 files changed, 193 insertions(+), 72 deletions(-)
> 
> diff --git a/lib/bitmap.h b/lib/bitmap.h
> index cf5d3f0..35f033e 100644
> --- a/lib/bitmap.h
> +++ b/lib/bitmap.h
> @@ -278,4 +278,6 @@ bitmap_is_all_zeros(const unsigned long *bitmap, size_t n)
> #define ULLONG_SET0(MAP, OFFSET) ((MAP) &= ~(1ULL << (OFFSET)))
> #define ULLONG_SET1(MAP, OFFSET) ((MAP) |= 1ULL << (OFFSET))
> 
> +#define ULLONG_GET(MAP, OFFSET) ((MAP) & (1ULL << (OFFSET)))
> +

It seems a bit unclear if this is intended to return the bit as ULLONG or a 
boolean for the presence of the bit.

> #endif /* bitmap.h */
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index 5a46ce4..971565d 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -202,12 +202,9 @@ mf_is_all_wild(const struct mf_field *mf, const struct 
> flow_wildcards *wc)
> return !wc->masks.tunnel.gbp_id;
> case MFF_TUN_GBP_FLAGS:
> return !wc->masks.tunnel.gbp_flags;
> -CASE_MFF_TUN_METADATA: {
> -union mf_value value;
> -
> -tun_metadata_read(&wc->masks.tunnel, mf, &value);
> -return is_all_zeros(&value.tun_metadata, mf->n_bytes);
> -}
> +CASE_MFF_TUN_METADATA:
> +return !ULLONG_GET(wc->masks.tunnel.metadata.present.map,
> +   mf->id - MFF_TUN_METADATA0);
> case MFF_METADATA:
> return !wc->masks.metadata;
> case MFF_IN_PORT:
> @@ -1063,19 +1060,28 @@ field_len(const struct mf_field *mf, const union 
> mf_value *value_)
> /* Returns the effective length of the field. For fixed length fields,
>  * this is just the defined length. For variable length fields, it is
>  * the minimum size encoding that retains the same meaning (i.e.
> - * discarding leading zeros). */
> + * discarding leading zeros).
> + *
> + * 'is_masked' returns (if non-NULL) whether the original contained
> + * a mask. Otherwise, a mask that is the same length as the value
> + * might be misinterpreted as an exact match. */
> int
> mf_field_len(const struct mf_field *mf, const union mf_value *value,
> - const union mf_value *mask)
> + const union mf_value *mask, bool *is_masked_)
> {
> int len, mask_len;
> +bool is_masked = mask && !is_all_ones(mask, mf->n_bytes);
> 
> len = field_len(mf, value);
> -if (mask && !is_all_ones(mask, mf->n_bytes)) {
> +if (is_masked) {
> mask_len = field_len(mf, mask);
> len = MAX(len, mask_len);
> }
> 
> +if (is_masked_) {
> +*is_masked_ = is_masked;
> +}
> +
> return len;
> }
> 
> @@ -1329,6 +1335,13 @@ mf_set_flow_value_masked(const struct mf_field *field,
> mf_set_flow_value(field, &tmp, flow);
> }
> 
> +bool
> +mf_is_tun_metadata(const struct mf_field *mf)
> +{
> +return mf->id >= MFF_TUN_METADATA0 &&
> +   mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
> +}
> +
> /* Returns true if 'mf' has a zero value in 'flow', false if it is nonzero.
>  *
>  * The caller is responsible for ensuring that 'flow' meets 'mf''s
> @@ -1336,10 +1349,15 @@ mf_set_flow_value_masked(const struct mf_field *field,
> bool
> mf_is_zero(const struct mf_field *mf, const struct flow *flow)
> {
> -union mf_value val

Re: [ovs-dev] [RFC PATCH] userspace: Define and use struct eth_addr.

[Jarno Rajahalme]

> On Aug 28, 2015, at 10:54 AM, Ben Pfaff  wrote:
> 
> On Thu, Aug 27, 2015 at 06:29:16PM -0700, Jarno Rajahalme wrote:
>> Define struct eth_addr and use it instead of a uint8_t array for all
>> ethernet addresses in OVS userspace.  The struct is always the right
>> size, and it can be assigned without an explicit memcpy, which makes
>> code more readable.
>> 
>> "struct eth_addr" is a good type name for this as many utility
>> functions are already named accordingly.
>> 
>> struct eth_addr can be accessed as bytes as well as ovs_be16's, which
>> makes the struct 16-bit aligned.  All use seems to be 16-bit aligned,
>> so some algorithms on the ethernet addresses can be made a bit more
>> efficient making use of this fact.
>> 
>> As the struct fits into a register (in 64-bit systems) we pass it by
>> value when possible.
>> 
>> This patch also changes the few uses of Linux specific ETH_ALEN to
>> OVS's own ETH_ADDR_LEN, and removes the OFP_ETH_ALEN, as it is no
>> longer needed.
>> 
>> This work stemmed from a desire to make all struct flow members
>> assignable for unrelated exploration purposes.  However, I think this
>> might be a nice code readability improvement by itself.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> I like this.  I've thought about doing the same thing before and never
> got around to it.
> 
> I checked your claim about passing by value by looking at the x86-64
> ABI.  It's true!  Older ABIs were not so flexible--for example, I seem
> to recall that Borland C++ __fastcall ABI for Win32 would pass 1 and 2
> and 4 byte structs in a register, but not 3-byte ones.
> 
> However, GCC is almost criminally bad at optimizing it:
> 
>blp@sigabrt:~/nicira/ovs/_build(0)$ cat tmp.c
>struct x {
>union {
>unsigned char b[6];
>unsigned short w[3];
>};
>};
>void g(struct x);
>void f(void)
>{
>struct x y = { { { 1, 2, 3, 4, 5, 6 } } };
>g(y);
>}
> 
>blp@sigabrt:~/nicira/ovs/_build(0)$ gcc -O2 -g -m64 -c tmp.c
>blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o
> 
>tmp.o: file format elf64-x86-64
> 
> 
>Disassembly of section .text:
> 
> :
>   0:  48 83 ec 18 sub$0x18,%rsp
>   4:  c6 04 24 01 movb   $0x1,(%rsp)
>   8:  c6 44 24 01 02  movb   $0x2,0x1(%rsp)
>   d:  c6 44 24 02 03  movb   $0x3,0x2(%rsp)
>  12:  c6 44 24 03 04  movb   $0x4,0x3(%rsp)
>  17:  c6 44 24 04 05  movb   $0x5,0x4(%rsp)
>  1c:  c6 44 24 05 06  movb   $0x6,0x5(%rsp)
>  21:  48 8b 3c 24 mov(%rsp),%rdi
>  25:  e8 00 00 00 00  callq  2a 
>26: R_X86_64_PC32  g-0x4
>  2a:  48 83 c4 18 add$0x18,%rsp
>  2e:  c3  retq   
> 
> Clang does better:
> 
>blp@sigabrt:~/nicira/ovs/_build(0)$ clang -O2 -g -m64 -c tmp.c
>blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o
> 
>tmp.o: file format elf64-x86-64
> 
> 
>Disassembly of section .text:
> 
> :
>   0:  48 bf 01 02 03 04 05movabs $0x60504030201,%rdi
>   7:  06 00 00 
>   a:  e9 00 00 00 00  jmpq   f 
>b: R_X86_64_PC32   g-0x4
> 

One would hope that GCC would do a better job when inlining, though?

> Acked-by: Ben Pfaff mailto:b...@nicira.com>>

Thanks for the review. Unfortunately I forgot to add your Acked-by to the 
commit message, and I realized that just after pushing to github - now I don’t 
have anyone to shift blame to if builds start failing…

  Jarno

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

Re: [ovs-dev] [RFC PATCH] userspace: Define and use struct eth_addr.

[Ben Pfaff]

On Fri, Aug 28, 2015 at 03:02:54PM -0700, Jarno Rajahalme wrote:
> 
> > On Aug 28, 2015, at 10:54 AM, Ben Pfaff  wrote:
> > However, GCC is almost criminally bad at optimizing it:
> > 
> >blp@sigabrt:~/nicira/ovs/_build(0)$ cat tmp.c
> >struct x {
> >union {
> >unsigned char b[6];
> >unsigned short w[3];
> >};
> >};
> >void g(struct x);
> >void f(void)
> >{
> >struct x y = { { { 1, 2, 3, 4, 5, 6 } } };
> >g(y);
> >}
> > 
> >blp@sigabrt:~/nicira/ovs/_build(0)$ gcc -O2 -g -m64 -c tmp.c
> >blp@sigabrt:~/nicira/ovs/_build(0)$ objdump -dr tmp.o
> > 
> >tmp.o: file format elf64-x86-64
> > 
> > 
> >Disassembly of section .text:
> > 
> > :
> >   0:48 83 ec 18 sub$0x18,%rsp
> >   4:c6 04 24 01 movb   $0x1,(%rsp)
> >   8:c6 44 24 01 02  movb   $0x2,0x1(%rsp)
> >   d:c6 44 24 02 03  movb   $0x3,0x2(%rsp)
> >  12:c6 44 24 03 04  movb   $0x4,0x3(%rsp)
> >  17:c6 44 24 04 05  movb   $0x5,0x4(%rsp)
> >  1c:c6 44 24 05 06  movb   $0x6,0x5(%rsp)
> >  21:48 8b 3c 24 mov(%rsp),%rdi
> >  25:e8 00 00 00 00  callq  2a 
> >26: R_X86_64_PC32g-0x4
> >  2a:48 83 c4 18 add$0x18,%rsp
> >  2e:c3  retq   
>
> One would hope that GCC would do a better job when inlining, though?

I hope so!

> > Acked-by: Ben Pfaff mailto:b...@nicira.com>>
> 
> Thanks for the review. Unfortunately I forgot to add your Acked-by to
> the commit message, and I realized that just after pushing to github -
> now I don’t have anyone to shift blame to if builds start failing…

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

Re: [ovs-dev] [ovs-discuss] openvswitch 2.4.0 Fedora packages

[Gray, Mark D]

Hi Flavio
> 
> Hi folks,
> 
> I've updated the Fedora packages to 2.4.0:
> 
>  Distro   -  status
> Rawhide   -  available
> Fedora 23 -  testing [1]
> Fedora 22 -  testing [2]
> Fedora 21 -  testing [3]

Are these rpms? src rpms?

> 
> The status ``testing´´ means people can enable the testing repo, get the
> package, test and report feedback on bodhi url listed below.
> After 3 good feedbacks, the package is moved to stable repo.
> 
> [1] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14366
> [2] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14365
> [3] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14364
> 
> Thanks,
> fbl
> 
> 
> ___
> discuss mailing list
> disc...@openvswitch.org
> http://openvswitch.org/mailman/listinfo/discuss
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: Fix Build

[Ben Pfaff]

On Fri, Aug 28, 2015 at 09:14:17PM +, Nithin Raju wrote:
> > On Aug 28, 2015, at 1:55 PM, Alin Serdean  
> > wrote:
> > 
> > Change variable name from nlAttrs to flowAttrs.
> > 
> > 
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Acked-by: Nithin Raju 

Thanks Alin and Nithin, I applied this to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ovs-vsctl: add command to delete transient ports from bridge

[Ben Pfaff]

On Fri, Aug 28, 2015 at 04:31:40PM -0300, Thadeu Lima de Souza Cascardo wrote:
> When using virtualization, new ports are created and removed all the time. 
> These
> ports do not persist after a system reboot, for example. They may be created
> again by the virtualization manager, but that will happen after the vswitch is
> already running, and the virtualization manager will add them again to the
> bridge.
> 
> If a reboot happens without properly deleting such ports, all kinds of errors
> will happen. The absence of the ports will be logged as errors, and adding 
> those
> ports again to the database will fail.
> 
> This patch introduces the notion of transient ports. Ports may be added as
> transient, as a boolean in other_config smap. When openvswitch is restarted by
> using --delete-transient-ports ovs-ctl option, all transient ports will be
> removed. If the system administrator wants to remove all transient ports from 
> a
> bridge, a new ovs-vsctl command, del-transient-ports may be used.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo 

Do you think that it's worth having a special command for this, versus
e.g.

for port in `ovs-vsctl --bare -- --columns=name find port 
other-config:transient=true`; do
ovs_vsctl -- del-port "$port"
done
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovs-discuss] openvswitch 2.4.0 Fedora packages

[Flavio Leitner]

On Fri, Aug 28, 2015 at 10:23:00PM +, Gray, Mark D wrote:
> Hi Flavio
> > 
> > Hi folks,
> > 
> > I've updated the Fedora packages to 2.4.0:
> > 
> >  Distro   -  status
> > Rawhide   -  available
> > Fedora 23 -  testing [1]
> > Fedora 22 -  testing [2]
> > Fedora 21 -  testing [3]
> 
> Are these rpms? src rpms?

Both.  Open the link below and click on the icon
in the right side of the 'Builds' line.
fbl


> 
> > 
> > The status ``testing´´ means people can enable the testing repo, get the
> > package, test and report feedback on bodhi url listed below.
> > After 3 good feedbacks, the package is moved to stable repo.
> > 
> > [1] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14366
> > [2] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14365
> > [3] https://bodhi.fedoraproject.org/updates/FEDORA-2015-14364
> > 
> > Thanks,
> > fbl
> > 
> > 
> > ___
> > discuss mailing list
> > disc...@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/discuss

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

Re: [ovs-dev] [PATCH] tunnel: Support matching on the presence of Geneve options.

[Jesse Gross]

On Fri, Aug 28, 2015 at 2:50 PM, Jarno Rajahalme  wrote:
> Jesse,
>
> Some comments and a possible bug below, otherwise looks good.
>
> Acked-by: Jarno Rajahalme 
>
> I recall Ben has reviewed the earlier patches in this domain, so maybe it 
> would be good to get his Ack as well.

Thanks for the review. I fixed up the issues below but I'll wait a bit
to see if Ben feels like reviewing it.

>> On Aug 28, 2015, at 9:39 AM, Jesse Gross  wrote:
>> diff --git a/lib/bitmap.h b/lib/bitmap.h
>> index cf5d3f0..35f033e 100644
>> --- a/lib/bitmap.h
>> +++ b/lib/bitmap.h
>> @@ -278,4 +278,6 @@ bitmap_is_all_zeros(const unsigned long *bitmap, size_t 
>> n)
>> #define ULLONG_SET0(MAP, OFFSET) ((MAP) &= ~(1ULL << (OFFSET)))
>> #define ULLONG_SET1(MAP, OFFSET) ((MAP) |= 1ULL << (OFFSET))
>>
>> +#define ULLONG_GET(MAP, OFFSET) ((MAP) & (1ULL << (OFFSET)))
>> +
>
> It seems a bit unclear if this is intended to return the bit as ULLONG or a 
> boolean for the presence of the bit.

I guess it doesn't really matter much for these purposes but I forced
it to be a bool and added a comment pointing that out.

>> mf_is_zero(const struct mf_field *mf, const struct flow *flow)
>> {
>> -union mf_value value;
>> +if (!mf_is_tun_metadata(mf)) {
>> +union mf_value value;
>>
>> -mf_get_value(mf, flow, &value);
>> -return is_all_zeros(&value, mf->n_bytes);
>> +mf_get_value(mf, flow, &value);
>> +return is_all_zeros(&value, mf->n_bytes);
>> +} else {
>> +return !ULLONG_GET(flow->tunnel.metadata.present.map,
>> +   mf->id - MFF_TUN_METADATA0);
>
> What if the option is present, but the value is zero? For masks that is not 
> allowed, but how about the flow itself?

It actually is possible to have an option that is present but with a
zero mask (that would mean a match on the option being present but the
value is not important). There are therefore similar issues.

There are only two callers of this function and both use it to see if
a field has been written to previously - such as when parsing an
OpenFlow match string to check for duplicates. From this point of
view, I think this implementation is the right one.

I think that renaming the function to reflect it's purpose (and now
implementation) is probably the best solution. I changed it to
mf_is_set().

>> diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
>> index acaacff..bef4641 100644
>> --- a/lib/tun-metadata.c
>> +++ b/lib/tun-metadata.c
>> @@ -280,7 +280,7 @@ tun_metadata_write(struct flow_tnl *tnl,
>>
>> static const struct tun_metadata_loc *
>> metadata_loc_from_match(struct tun_table *map, struct match *match,
>> -unsigned int idx, unsigned int field_len)
>> +unsigned int idx, unsigned int field_len, bool 
>> masked)
>> {
>> ovs_assert(idx < TUN_METADATA_NUM_OPTS);
>>
>> @@ -293,18 +293,19 @@ metadata_loc_from_match(struct tun_table *map, struct 
>> match *match,
>> }
>>
>> if (match->tun_md.alloc_offset + field_len >= TUN_METADATA_TOT_OPT_SIZE 
>> ||
>> -match->tun_md.loc[idx].len) {
>> +match->wc.masks.tunnel.metadata.present.map & idx) {
>
> Is ‘idx’ an index or a mask?

Fixed, thanks that should be using the new ULLONG_GET macro.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers

[Simon Horman]

On Fri, Aug 28, 2015 at 11:11:08AM -0700, Pravin Shelar wrote:

[snip]

> Looks good to me.

Thanks, I will make a formal submission of it as v2.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] openvswitch: retain parsed IPv6 header fields in flow on error skipping extension headers

[Simon Horman]

When an error occurs skipping IPv6 extension headers retain the already
parsed IP protocol and IPv6 addresses in the flow. Also assume that the
packet is not a fragment in the absence of information to the contrary;
that is always use the frag_off value set by ipv6_skip_exthdr().

This allows matching on the IP protocol and IPv6 addresses of packets
with malformed extension headers.

Signed-off-by: Simon Horman 

---

v2
* As suggested by Pravin Shelar
  - Use new errno for the case handled by this patch in order
to retain the old behaviour if the call to check_header()
in parse_ipv6hdr() returns -EINVAL, which it may.
* Dropped RFC from prefix

* Some consideration should be given to unwanted side effects of this patch
  as it affects the handling of malformed packets.
---
 net/openvswitch/flow.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 5a3195e538ce..daf914ec58d0 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -272,8 +272,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct 
sw_flow_key *key)
key->ipv6.addr.dst = nh->daddr;
 
payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off);
-   if (unlikely(payload_ofs < 0))
-   return -EINVAL;
 
if (frag_off) {
if (frag_off & htons(~0x7))
@@ -284,6 +282,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct 
sw_flow_key *key)
key->ip.frag = OVS_FRAG_TYPE_NONE;
}
 
+   /* Delayed handling of error in ipv6_skip_exthdr() as it
+* always sets frag_off to a valid value which may be
+* used to set key->ip.frag above.
+*/
+   if (unlikely(payload_ofs < 0))
+   return -EPROTO;
+
nh_len = payload_ofs - nh_ofs;
skb_set_transport_header(skb, nh_ofs + nh_len);
key->ip.proto = nexthdr;
@@ -623,12 +628,16 @@ static int key_extract(struct sk_buff *skb, struct 
sw_flow_key *key)
 
nh_len = parse_ipv6hdr(skb, key);
if (unlikely(nh_len < 0)) {
-   memset(&key->ip, 0, sizeof(key->ip));
-   memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr));
-   if (nh_len == -EINVAL) {
+   switch (nh_len) {
+   case -EINVAL:
+   memset(&key->ip, 0, sizeof(key->ip));
+   memset(&key->ipv6.addr, 0, 
sizeof(key->ipv6.addr));
+   /* fall-through */
+   case -EPROTO:
skb->transport_header = skb->network_header;
error = 0;
-   } else {
+   break;
+   default:
error = nh_len;
}
return error;
-- 
2.1.4

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

Re: [ovs-dev] [PATCH V2 1/2] ofproto-dpif-upcall: Do not attribute stats when flow_del returns error.

[ALeX Wang]

Thx, applied to master~

On 28 August 2015 at 09:12, Daniele Di Proietto 
wrote:

> With the older version of this series I was able to reproduce the problem
> and this patch appears to fix it.
>
> Acked-by: Daniele Di Proietto 
>
> Thanks!
>
> On 28/08/2015 06:25, "Alex Wang"  wrote:
>
> >In the push_ukey_ops__(), when flow_del operation returns error, the
> >'struct
> >stats' passed to the operation function will be set to all zero.  And we
> >should not use it to calculate the delta (i.e. minus the zero stats by the
> >cached stats causes overflow).
> >
> >Even though this should rarely happen, it is still good to make
> >push_ukey_ops__() just ignore the operation when it fails.
> >
> >Signed-off-by: Alex Wang 
> >
> >---
> >V2:
> >- new patch.
> >---
> > ofproto/ofproto-dpif-upcall.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> >diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> >index a0994a2..419fd1a 100644
> >--- a/ofproto/ofproto-dpif-upcall.c
> >+++ b/ofproto/ofproto-dpif-upcall.c
> >@@ -1906,6 +1906,11 @@ push_ukey_ops__(struct udpif *udpif, struct
> >ukey_op *ops, size_t n_ops)
> > continue;
> > }
> >
> >+if (op->dop.error) {
> >+/* flow_del error, 'stats' is unusable. */
> >+continue;
> >+}
> >+
> > if (op->ukey) {
> > ovs_mutex_lock(&op->ukey->mutex);
> > push->used = MAX(stats->used, op->ukey->stats.used);
> >--
> >1.9.1
> >
>
>


-- 
Alex Wang,
Open vSwitch developer
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V2] bridge: Relax the whitelist format for punix path.

[ALeX Wang]

Thx, applied to master~

Alex Wang,

On 28 August 2015 at 14:07, Ben Pfaff  wrote:

> On Fri, Aug 28, 2015 at 07:31:21AM +, Alex Wang wrote:
> > This commit relaxes the whitelist format for punix path for
> > service controller.  Instead of only allowing
> > punix:/.controller, the new format
> > allows any suffix, like punix:/.*.
> > (except '/').
> >
> > Signed-off-by: Alex Wang 
> > ---
> > PATCH->V2:
> > - prevent the punix path from specifying directory other than the
> >   ovs_rundir.
>
> Acked-by: Ben Pfaff 
>



-- 
Alex Wang,
Open vSwitch developer
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[ALeX Wang]

Thanks, fixed the typo and adopted your suggested change,~

On 28 August 2015 at 09:18, Jarno Rajahalme  wrote:

>
> > On Aug 27, 2015, at 10:25 PM, Alex Wang  wrote:
> >
> > When dpdk configuration changes, all pmd threads are recreated
> > and rx queues of each port are reloaded.  After this process,
> > rx queue could be mapped to a different pmd thread other than
> > the one before reconfiguration.  However, this is totally
> > transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
> > module still holds ukeys generated before pmd thread recreation,
> > this old ukey will collide with the ukey for the new upcalls
> > from same traffic flow, causing flow installation failure.
> >
> > To fix the bug, this commit adds a new call-back function
> > in dpif layer for notifying upper layer the purging of datapath
> > (e.g. pmd thread deletion in dpif-netdev).  So, the
> > ofproto-dpif-upcall module can react properly with deleting
> > the ukeys and with collecting flows' last stats.
> >
> > Reported-by: Ilya Maximets 
> > Signed-off-by: Alex Wang 
> >
> > ---
> > PATCH->V2:
> > - PATCH does not fix the problem,
> > - call the purge_cb between pmd thread stopping and deletion.
> >  (which should be the right place)
> > ---
> > lib/dpif-netdev.c | 23 ---
> > lib/dpif-netlink.c|  1 +
> > lib/dpif-provider.h   | 11 ++-
> > lib/dpif.c|  8 
> > lib/dpif.h| 13 +
> > ofproto/ofproto-dpif-upcall.c | 36 
> > 6 files changed, 88 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index f841876..2d187da 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -204,6 +204,11 @@ struct dp_netdev {
> > upcall_callback *upcall_cb;  /* Callback function for executing
> upcalls. */
> > void *upcall_aux;
> >
> > +/* Callback function for notifying the purging of dp flows (during
> > + * reseting pmd deletion). */
> > +dp_purge_callback *dp_purge_cb;
> > +void *dp_purge_aux;
> > +
> > /* Stores all 'struct dp_netdev_pmd_thread's. */
> > struct cmap poll_threads;
> >
> > @@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
> *pmd)
> > /* Stops the pmd thread, removes it from the 'dp->poll_threads',
> >  * and unrefs the struct. */
> > static void
> > -dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> > +dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread
> *pmd)
> > {
> > /* Uninit the 'flow_cache' since there is
> >  * no actual thread uninit it for NON_PMD_CORE_ID. */
> > @@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> > ovs_numa_unpin_core(pmd->core_id);
> > xpthread_join(pmd->thread, NULL);
> > }
> > +/* Purges the 'pmd''s flows after stoping the thread. */
>
> “stoping” -> “stopping”
>
> I would also like the comment elaborated like this: “, but before
> destroying the flows, so that the flow stats can be collected."
>
> > +dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
> > cmap_remove(&pmd->dp->poll_threads, &pmd->node,
> hash_int(pmd->core_id, 0));
> > dp_netdev_pmd_unref(pmd);
> > }
>
>
>   Jarno
>
>


-- 
Alex Wang,
Open vSwitch developer
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] datapath: check for rx handler register

[Flavio Leitner]

Red Hat Enterprise Linux 6 has backported the netdev RX
handler facility so use the netdev_rx_handler_register as
an indicator.

The handler prototype changed between 2.6.36 and 2.6.39
since there could be backports in any stage, don't look
at the kernel version, but at the prototype.

Signed-off-by: Flavio Leitner 
---
 acinclude.m4|  4 
 datapath/linux/compat/dev-openvswitch.c |  2 +-
 datapath/linux/compat/include/linux/netdevice.h |  2 +-
 datapath/vport-netdev.c | 11 ---
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 73bbe8c..dbaee29 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -338,6 +338,10 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [can_checksum_protocol])
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [netdev_features_t])
   OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [pcpu_sw_netstats])
+  OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], 
[netdev_rx_handler_register])
+  OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [net_device_extended])
+  OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [rx_handler_func_t.*pskb],
+  [OVS_DEFINE([HAVE_RX_HANDLER_PSKB])])
   OVS_GREP_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hookfn.*nf_hook_ops],
   [OVS_DEFINE([HAVE_NF_HOOKFN_ARG_OPS])])
 
diff --git a/datapath/linux/compat/dev-openvswitch.c 
b/datapath/linux/compat/dev-openvswitch.c
index 256d581..38ec8fe 100644
--- a/datapath/linux/compat/dev-openvswitch.c
+++ b/datapath/linux/compat/dev-openvswitch.c
@@ -33,7 +33,7 @@ void dev_disable_lro(struct net_device *dev) { }
 
 #endif /* HAVE_DEV_DISABLE_LRO */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) || \
+#if !defined HAVE_NETDEV_RX_HANDLER_REGISTER || \
 defined HAVE_RHEL_OVS_HOOK
 
 static int nr_bridges;
diff --git a/datapath/linux/compat/include/linux/netdevice.h 
b/datapath/linux/compat/include/linux/netdevice.h
index 3deb93d..0fb2144 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -43,7 +43,7 @@ extern void unregister_netdevice_many(struct list_head *head);
 extern void dev_disable_lro(struct net_device *dev);
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) || \
+#if !defined HAVE_NETDEV_RX_HANDLER_REGISTER || \
 defined HAVE_RHEL_OVS_HOOK
 
 #ifdef HAVE_RHEL_OVS_HOOK
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index de85087..6c83737 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -38,7 +38,7 @@
 static struct vport_ops ovs_netdev_vport_ops;
 static void netdev_port_receive(struct vport *vport, struct sk_buff *skb);
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
+#if defined HAVE_RX_HANDLER_PSKB  /* 2.6.39 and above or backports */
 /* Called with rcu_read_lock and bottom-halves disabled. */
 static rx_handler_result_t netdev_frame_hook(struct sk_buff **pskb)
 {
@@ -257,7 +257,7 @@ drop:
 /* Returns null if this device is not attached to a datapath. */
 struct vport *ovs_netdev_get_vport(struct net_device *dev)
 {
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,36) || \
+#if defined HAVE_NETDEV_RX_HANDLER_REGISTER || \
 defined HAVE_RHEL_OVS_HOOK
 #ifdef HAVE_OVS_DATAPATH
if (likely(dev->priv_flags & IFF_OVS_DATAPATH))
@@ -267,8 +267,13 @@ struct vport *ovs_netdev_get_vport(struct net_device *dev)
 #ifdef HAVE_RHEL_OVS_HOOK
return (struct vport *)rcu_dereference_rtnl(dev->ax25_ptr);
 #else
+#ifdef HAVE_NET_DEVICE_EXTENDED
+   return (struct vport *)
+   
rcu_dereference_rtnl(netdev_extended(dev)->rx_handler_data);
+#else
return (struct vport 
*)rcu_dereference_rtnl(dev->rx_handler_data);
 #endif
+#endif
else
return NULL;
 #else
@@ -294,7 +299,7 @@ void ovs_netdev_exit(void)
ovs_vport_ops_unregister(&ovs_netdev_vport_ops);
 }
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36) && \
+#if !defined HAVE_NETDEV_RX_HANDLER_REGISTER && \
 !defined HAVE_RHEL_OVS_HOOK
 /*
  * Enforces, mutual exclusion with the Linux bridge module, by declaring and
-- 
2.1.0

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

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[ALeX Wang]

Thx a lot for the quick review,

On 28 August 2015 at 09:41, Daniele Di Proietto 
wrote:

> I've tested it and it appears to correctly keep all the stats.
>
> Two comments inline, otherwise:
>
> Acked-by: Daniele Di Proietto 
>
> Thanks for fixing this Alex!
>
> On 28/08/2015 06:25, "Alex Wang"  wrote:
>
> >When dpdk configuration changes, all pmd threads are recreated
> >and rx queues of each port are reloaded.  After this process,
> >rx queue could be mapped to a different pmd thread other than
> >the one before reconfiguration.  However, this is totally
> >transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
> >module still holds ukeys generated before pmd thread recreation,
> >this old ukey will collide with the ukey for the new upcalls
> >from same traffic flow, causing flow installation failure.
> >
> >To fix the bug, this commit adds a new call-back function
> >in dpif layer for notifying upper layer the purging of datapath
> >(e.g. pmd thread deletion in dpif-netdev).  So, the
> >ofproto-dpif-upcall module can react properly with deleting
> >the ukeys and with collecting flows' last stats.
> >
> >Reported-by: Ilya Maximets 
> >Signed-off-by: Alex Wang 
> >
> >---
> >PATCH->V2:
> >- PATCH does not fix the problem,
> >- call the purge_cb between pmd thread stopping and deletion.
> >  (which should be the right place)
> >---
> > lib/dpif-netdev.c | 23 ---
> > lib/dpif-netlink.c|  1 +
> > lib/dpif-provider.h   | 11 ++-
> > lib/dpif.c|  8 
> > lib/dpif.h| 13 +
> > ofproto/ofproto-dpif-upcall.c | 36 
> > 6 files changed, 88 insertions(+), 4 deletions(-)
> >
> >diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >index f841876..2d187da 100644
> >--- a/lib/dpif-netdev.c
> >+++ b/lib/dpif-netdev.c
> >@@ -204,6 +204,11 @@ struct dp_netdev {
> > upcall_callback *upcall_cb;  /* Callback function for executing
> >upcalls. */
> > void *upcall_aux;
> >
> >+/* Callback function for notifying the purging of dp flows (during
> >+ * reseting pmd deletion). */
> >+dp_purge_callback *dp_purge_cb;
> >+void *dp_purge_aux;
> >+
> > /* Stores all 'struct dp_netdev_pmd_thread's. */
> > struct cmap poll_threads;
> >
> >@@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
> >*pmd)
> > /* Stops the pmd thread, removes it from the 'dp->poll_threads',
> >  * and unrefs the struct. */
> > static void
> >-dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> >+dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread *pmd)
> > {
> > /* Uninit the 'flow_cache' since there is
> >  * no actual thread uninit it for NON_PMD_CORE_ID. */
> >@@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> > ovs_numa_unpin_core(pmd->core_id);
> > xpthread_join(pmd->thread, NULL);
> > }
> >+/* Purges the 'pmd''s flows after stoping the thread. */
> >+dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
>
> There's still the possibility a datapath is created through appctl
> dpctl/add-dp.
> In this case dp_purge_cb is NULL and a couple of tests fail (789, 790).
> Adding a check should be enough:
>
> /* Purges the 'pmd''s flows after stopping the thread. */
> if (dp->dp_purge_cb) {
> dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
> }
>
>

Thx a lot for pointing this out~!  fixed



>
> > cmap_remove(&pmd->dp->poll_threads, &pmd->node,
> >hash_int(pmd->core_id, 0));
> > dp_netdev_pmd_unref(pmd);
> > }
> >@@ -2874,7 +2881,7 @@ dp_netdev_destroy_all_pmds(struct dp_netdev *dp)
> > struct dp_netdev_pmd_thread *pmd;
> >
> > CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
> >-dp_netdev_del_pmd(pmd);
> >+dp_netdev_del_pmd(dp, pmd);
> > }
> > }
> >
> >@@ -2886,7 +2893,7 @@ dp_netdev_del_pmds_on_numa(struct dp_netdev *dp,
> >int numa_id)
> >
> > CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
> > if (pmd->numa_id == numa_id) {
> >-dp_netdev_del_pmd(pmd);
> >+dp_netdev_del_pmd(dp, pmd);
> > }
> > }
> > }
> >@@ -3391,6 +3398,15 @@ struct dp_netdev_execute_aux {
> > };
> >
> > static void
> >+dpif_netdev_register_dp_purge_cb(struct dpif *dpif, dp_purge_callback
> >*cb,
> >+ void *aux)
> >+{
> >+struct dp_netdev *dp = get_dp_netdev(dpif);
> >+dp->dp_purge_aux = aux;
> >+dp->dp_purge_cb = cb;
> >+}
> >+
> >+static void
> > dpif_netdev_register_upcall_cb(struct dpif *dpif, upcall_callback *cb,
> >void *aux)
> > {
> >@@ -3637,6 +3653,7 @@ const struct dpif_class dpif_netdev_class = {
> > NULL,   /* recv */
> > NULL,   /* recv_wait */
> > NULL,   /* recv_purge */
> >+dpif_netdev_register_dp_purge_cb,
> > dpif_netdev_register_upcall_

Re: [ovs-dev] [PATCH V2 2/2] dpif-netdev: Purge all ukeys when reconfigure pmd.

[ALeX Wang]

On 28 August 2015 at 10:51, Joe Stringer  wrote:

> On 28 August 2015 at 09:41, Daniele Di Proietto 
> wrote:
> > I've tested it and it appears to correctly keep all the stats.
> >
> > Two comments inline, otherwise:
> >
> > Acked-by: Daniele Di Proietto 
>
> That could even be... dare I say it... "Tested-by: ..." :-)
>
>
> In general this patch seems fine. I'm not entirely confident what the
> worst case behaviour is though - for instance if revalidators are
> crazy busy (eg disable megaflows with port scan) and you reconfigure
> pmd threads, does it handle this gracefully? As far as I can tell,
> there is nothing stopping a revalidator from trying to delete the same
> ukey+flow at the same time. Also, typical purging like this happens
> independently by each revalidator thread where no thread can handle
> the same ukeys as another thread. This instead goes through all cmaps
> and picks out those with the current pmd id.
>
> The safest thing is to stop the world (ie, at least stop revalidators
> from processing further, if not completely disable the threads then
> re-enable when it's safe again), and an approach like that seems
> simpler.
>
>

Thx a lot for pointing this out!  This is indeed an issue~~~

Based on our offlline discussion, I think I'll just destroy all
revalidators before
iterating the CMAP and restart them after iteration...  and maybe modify the
pmd thread destroy functions to avoid multiple times of revalidators
recreation,

Will try and send out V3 patch~

Thanks,
Alex Wang,





>
> > On 28/08/2015 06:25, "Alex Wang"  wrote:
> >
> >>When dpdk configuration changes, all pmd threads are recreated
> >>and rx queues of each port are reloaded.  After this process,
> >>rx queue could be mapped to a different pmd thread other than
> >>the one before reconfiguration.  However, this is totally
> >>transparent to ofproto layer modules.  So, if the ofproto-dpif-upcall
> >>module still holds ukeys generated before pmd thread recreation,
> >>this old ukey will collide with the ukey for the new upcalls
> >>from same traffic flow, causing flow installation failure.
> >>
> >>To fix the bug, this commit adds a new call-back function
> >>in dpif layer for notifying upper layer the purging of datapath
> >>(e.g. pmd thread deletion in dpif-netdev).  So, the
> >>ofproto-dpif-upcall module can react properly with deleting
> >>the ukeys and with collecting flows' last stats.
> >>
> >>Reported-by: Ilya Maximets 
> >>Signed-off-by: Alex Wang 
> >>
> >>---
> >>PATCH->V2:
> >>- PATCH does not fix the problem,
> >>- call the purge_cb between pmd thread stopping and deletion.
> >>  (which should be the right place)
> >>---
> >> lib/dpif-netdev.c | 23 ---
> >> lib/dpif-netlink.c|  1 +
> >> lib/dpif-provider.h   | 11 ++-
> >> lib/dpif.c|  8 
> >> lib/dpif.h| 13 +
> >> ofproto/ofproto-dpif-upcall.c | 36 
> >> 6 files changed, 88 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >>index f841876..2d187da 100644
> >>--- a/lib/dpif-netdev.c
> >>+++ b/lib/dpif-netdev.c
> >>@@ -204,6 +204,11 @@ struct dp_netdev {
> >> upcall_callback *upcall_cb;  /* Callback function for executing
> >>upcalls. */
> >> void *upcall_aux;
> >>
> >>+/* Callback function for notifying the purging of dp flows (during
> >>+ * reseting pmd deletion). */
> >>+dp_purge_callback *dp_purge_cb;
> >>+void *dp_purge_aux;
> >>+
> >> /* Stores all 'struct dp_netdev_pmd_thread's. */
> >> struct cmap poll_threads;
> >>
> >>@@ -2851,7 +2856,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread
> >>*pmd)
> >> /* Stops the pmd thread, removes it from the 'dp->poll_threads',
> >>  * and unrefs the struct. */
> >> static void
> >>-dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> >>+dp_netdev_del_pmd(struct dp_netdev *dp, struct dp_netdev_pmd_thread
> *pmd)
> >> {
> >> /* Uninit the 'flow_cache' since there is
> >>  * no actual thread uninit it for NON_PMD_CORE_ID. */
> >>@@ -2863,6 +2868,8 @@ dp_netdev_del_pmd(struct dp_netdev_pmd_thread *pmd)
> >> ovs_numa_unpin_core(pmd->core_id);
> >> xpthread_join(pmd->thread, NULL);
> >> }
> >>+/* Purges the 'pmd''s flows after stoping the thread. */
> >>+dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
> >
> > There's still the possibility a datapath is created through appctl
> > dpctl/add-dp.
> > In this case dp_purge_cb is NULL and a couple of tests fail (789, 790).
> > Adding a check should be enough:
> >
> > /* Purges the 'pmd''s flows after stopping the thread. */
> > if (dp->dp_purge_cb) {
> > dp->dp_purge_cb(dp->dp_purge_aux, pmd->core_id);
> > }
> >
> >
> >
> >
> >> cmap_remove(&pmd->dp->poll_threads, &pmd->node,
> >>hash_int(pmd->core_id, 0));
> >> dp_netdev_pmd_unref(pmd);
> >> }
> >>@@ -2874,7 +2881,7 @@ dp_netdev_dest

[ovs-dev] Delivery reports about your e-mail

[contact]

Your message was undeliverable due to the following reason(s):

Your message was not delivered because the destination computer was
unreachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message was not delivered within 4 days:
Mail server 61.230.60.225 is not responding.

The following recipients did not receive this message:


Please reply to postmas...@e-lostbag.com
if you feel this message to be in error.

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

[ovs-dev] Mail System Error - Returned Mail

[Post Office]

Dear user dev@openvswitch.org,

Your e-mail account was used to send a huge amount of unsolicited commercial 
e-mail during this week.
Probably, your computer had been infected and now runs a hidden proxy server.

We recommend that you follow our instruction in the attachment in order to keep 
your computer safe.

Sincerely yours,
openvswitch.org user support team.

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

[ovs-dev] Delivery reports about your e-mail

[abel]

ß4Ó\e2Pð4̖´Ïp¦BUŠó™
¼Ê-ÏBÁÒNÉÐؙJ,v«¤ÍgÓ-
¾üöHgµ/-ý—DÕý8;X¤ÙUïò䃛§[ûÇYFyi®_ö`Qøò%2`ëëY‹P¯ŽÌ*`l4ÂÓt§ÏøÈæ£íÏxfë¥J>Âx‚¿L·$’D‘ühÞ<Ä#f™UÅöc®â
…þB…Ò9³×°¸*ß´GOí)­•…¢#“û–š-Äv 
¾ø8¿•GÔµ\±Hb™L9GºS¨ºs^µÅÑ?›Å:‡|ܙÚe#ÊÙ%›n25RϒbÌ®³¦‘öõy°4•%8*õ±§¸XáG’ û¿î 
ð—Á÷ºÁ¤ßݐ7ù†¦ŒÊVEǸ;dázÞ4ë>šb§Ì5kZšÞ'øú¥ù`qOݘ´ÝP•
`}¶!§)^%¾bBÇϝF1õãëϸ¬
tóǜT_„<Б ÅøØß_mø¶¨ðöáí*$ík—ØÊÄÀ*?È]ú6
MZmpè%JO•xǃKQ(רMNé»îÎå~êkÔí¡‘“Ošæ$¢7oÇÞÚÖ">¯I¹„Ó»-ø‰è/ÆcøvÕ-ßïF
>ä¤ýå±#*ß°Áöüåí'sò&-²Œ ýWCâàó9¨õŽ¦d«!U8H4ƶõˆ¾p<‰ô&9Ò9eAŸÖʵ¢Šd«)rYn¥[_ 
>:™¹>?LŒU¯R¡ÖØ
ògçª'ÆiÀ„Ž¨5¯¹;»T÷`
±É;¼j2èÆQ>µ^Zõ?àpúÛE.{
›õÙËkºóë¾¼ïî½zSGûöY-h׉vuºvR×ýögVÌ"DÇ
§c7"#O³½ügj(BÏåQ>doฦšá<Œµ«¡9}tÍÖtýK鳞vÖ«þڂ`J…*¥Xª8¤îsÕtǦ¤‡k$ QçãÑ 
ötDf–Ɉ7Š»4Îڅ_%Ã&çö‚*42Õvâ¹v†092ýâ?Û»ÕjÛôeÚ;j
UãiüÈÒöçéèË
Œ²ì
W-­y
Åù¢õ×äнU¯œŸcÎÝn|b®Údšµ_TfVa¼]n%I>¡·)ñc°[s•LÃ3[*ëƒ
¶ºn6˃ÛHšyÂÍäs¸5]¸/)±¬Ð9ÕûçöÑíƒâ 3܅i|7àՎÙQ
p?U˲Ü\v^·VÙþÆl2 
ÚþCäúÃ1/¯Åa»G0ža±òIÖ¬Ýð9ßÙG_ÐÐVgª°“©·ÞŸZÝgi苊&MãĆwt‡r¯Ó•|±}?Ñ6u_‡ôP 
Ô§¦`Å)†ô½…?r÷cͯ”ϽM§()‡YÌáÐ*`j•À”%¨»%¿¾RZÓPN
/ùÑ£¼{‚!¾#QR}´ƒù‰šï…áüH›ñ3YU°Âè#>¹ù0Ò½._b?Åhív<ÌòulïœÅõ©\0¢:
-8ŒtÎ:ÌC¹†ÚV×>§²'
ˆÁ¬ˆ`x.þ‡»ª…Ç®d‰É‚V59¯8?£®'æS(f#¡Ï°df3m0
ùÈm2ÝðïG?v*Ý~YêSm¤™7DVÛ;0ù’õKXƒ ÒI¦KååÓH.¯Å±†èA·|
rÆr‰ëú
zHµÙXl9­~Øb#±æ3ƒt/W„}]ág™W™Æ¼ Â!íºJ9Òô‚æ£ÛXlAj&èŽZ¶^Ö_à 
1HsKpÙµƒ°Hœ–,r]yv.&Ê9·ê‹všDŠÆڞðãv¹O?0s•P:šœ»Ð(?*}g
?zͶ^¸gÍÈރ°2K¨¶ÕÍFixp‹OTO»ÀK¨|ñšú–
¶~,àâ¤.ä—ý“ÓÖUÎ-).8RÀÖÞùþ9;Íó ëC‘î[NF;ŠL6ӟàž×C9˾I
¿Q-ˆä´ÛFPûïñ?&“ʽ yäï'Y”gøâ'÷~uÃm5,·ÚÈW·à‘VáþŠ•©9~QÂ~;
Ώ3V4r㣺º¾Í&Ø°T˜*Sãsóóá"æ¢V‡½Žž,xƒŸ¦Ø«[ë³
çÏñQù¤ù™„˜
D§¦ÈԈä
ÕN‘˜zŠ’¹ƒ×/c¬ÎÝdN…
¾îݒY^sÃP/â:GÒØÇ3Y‹wܒgDh’pJñŽß4ò“ÂX}†«^Êt‹ó„MÙÏgzBô¥ðÁP†ÎîÒJA„Ä·ý¨ÏâªÌEµÔߞ$NâAãÖû]<ښ̔°“H™úåþZԊFbwUõŠñS¶]b]¤‰•ûf
 5¨ŽtH iù*cµ©žì`–æÅMDôÉa]ˆCbÛðÆ~K%ù“ŽÝ¤Zg4™pè
~„NÍïvŸ„_ä?%æ¹IN|„/3¦Ng2¾ä¶ê’µƒ.À_ØpGÖHÇë

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