[ovs-dev] beneficial proposition

[dev]

Hello!

We are looking for employees working remotely.

My name is Lucas, am the personnel manager of a large International company.
Most of the work you can do from home, that is, at a distance.

Salary is $2000-$5000.

If you are interested in this offer, please visit 
Our Web Page


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

Re: [ovs-dev] [PATCH v6 06/12] dpif-netdev: Wait an RCU grace period before freeing ports.

[Daniele Di Proietto]

On 01/04/2016 09:52, "Jarno Rajahalme"  wrote:

>
>> On Mar 30, 2016, at 8:08 PM, Daniele Di Proietto
>> wrote:
>> 
>> 
>> On 30/03/2016 16:01, "Ben Pfaff"  wrote:
>> 
>>> (I'm taking a look at this patch specifically because Daniele asked me;
>>> I'm not planning to review the whole series.)
>>> 
>>> On Mon, Mar 28, 2016 at 12:41:40PM -0700, Daniele Di Proietto wrote:
 The dpif-netdev datapath keeps ports in a cmap which is written only
by
 the main thread (holding port_mutex), but which is read concurrently
by
 many threads (most notably the pmd threads).
 
 When removing ports from the datapath we should postpone the deletion,
 otherwise another thread might access invalid memory while reading the
 cmap.
 
 This commit splits do_port_del() in do_port_remove() and
 do_port_destroy(): the former removes the port from the cmap, while
the
 latter reclaims the memory and drops the reference to the underlying
 netdev.
>>> 
>>> s/del_port/port_del/ here:
>> 
>> Thanks, changed
>> 
>>> 
 dpif_netdev_del_port() now uses ovsrcu_synchronize() before calling
 do_port_destroy(), to avoid memory corruption in concurrent readers.
>>> 
>>> ovsrcu_synchronize() requires that nothing in the thread that calls it
>>> is relying on RCU to keep objects around.  That means that no caller of
>>> dfpi_port_del()--there are a few of them--can rely on it.  This is
>>> usually a risky assumption, especially because this assumption can
>>> change later.  Is there reason to believe that it isn't important in
>>>all
>>> of these cases?
>> 
>> I agree that's risky, but I think it's the only way to keep the ports
>>RCU
>> protected, because a port needs to be effectively deleted before
>> dpif_netdev_port_del() can return.
>> 
>
>If this is because otherwise a following port_add can fail, as the old
>port is still around, maybe we could make the highest possible level of
>port_add detect the failure and then rcu_synchronize and try again? Would
>that work?
>
>  Jarno

That would work for deleting the port, but there are other reasons we need
to synchronize.  When a netdev has to be reconfigured (in the last patch
of the series) and we remove it from the cmap, we need to synchronize to
make sure that other threads have stopped using it.

I'm trying to add some compile-time RCU checks using clang thread safety
annotations, but for those to be effective we have to introduce
ovsrcu_read_lock() and ovsrcu_read_unlock() on every block that keeps RCU
references and I'm not sure we want to go down that path.

I've also remembered that dpif_netdev_port_add() and
dpif_netdev_port_del() might already quiesce, because they could call
ovs_mutex_cond_wait().  I'll try to post a patch to fix that, if we
believe it's an issue.

If ovsrcu_synchronize() is not an acceptable solution, I guess we should
just use an hmap for ports and have pmdthread-local copies.  This means
that every port_add or port_del (even for non DPDK ports) would need to
stop every pmd thread, but I guess there's no way around it.

Thanks,

Daniele

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

[ovs-dev] [PATCH v2] ofp-actions: Add a new action to truncate a packet.

[William Tu]

The patch proposes adding a new action to support packet truncation.  The new
action is formatted as 'output(port=n,max_len=m)', as output to port n, with
packet size being MIN(original_size, m).

One use case is to enable port mirroring to send smaller packets to the
destination port so that only useful packet information is mirrored/copied,
saving some performance overhead of copying entire packet payload.  Example
use case is below as well as shown in the testcases:

- Output to port 1 with max_len 100 bytes.
- The output packet size on port 1 will be MIN(original_packet_size, 100).
# ovs-ofctl add-flow br0 'actions=output(port=1,max_len=100)'

- The scope of max_len is limited to output action itself.  The following
  output:1 and output:2 will be the original packet size.
# ovs-ofctl add-flow br0 
'actions=output(port=1,max_len=100),output:1,output:2'

Implementation/Limitaions:

- The patch adds a new OpenFlow extension action OFPACT_OUTPUT_TRUNC, and
  a new datapath action, OVS_ACTION_ATTR_OUTPUT_TRUNC.
- The minimum value of max_len is 60 byte (minimum Ethernet frame size).
  This is defined in OVS_ACTION_OUTPUT_MIN.
- Only "output(port=[0-9]*,max_len=)" is supported.  Output to IN_PORT,
  TABLE, NORMAL, FLOOD, ALL, and LOCAL are not supported.

Signed-off-by: William Tu 
---
v1->v2
- Create new OpenFlow action, do not reuse OFPACT_OUTPUT
- Create new datapath action

---
 datapath/actions.c|  18 +++-
 datapath/datapath.h   |   1 +
 datapath/flow_netlink.c   |   9 ++
 datapath/linux/compat/include/linux/openvswitch.h |   9 ++
 datapath/vport.c  |   6 ++
 lib/dp-packet.c   |   1 +
 lib/dp-packet.h   |   1 +
 lib/dpif-netdev.c |  28 ++
 lib/dpif.c|   1 +
 lib/netdev.c  |   8 ++
 lib/odp-execute.c |   2 +
 lib/odp-util.c|  11 ++-
 lib/ofp-actions.c | 102 ++
 lib/ofp-actions.h |  10 +++
 ofproto/ofproto-dpif-sflow.c  |   6 ++
 ofproto/ofproto-dpif-xlate.c  |  39 +
 tests/ofproto-dpif.at |  53 +++
 tests/system-traffic.at   |  66 ++
 18 files changed, 366 insertions(+), 5 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index dcf8591..a1a1241 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -738,10 +738,13 @@ err:
 }
 
 static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port,
- struct sw_flow_key *key)
+ uint16_t max_len, struct sw_flow_key *key)
 {
struct vport *vport = ovs_vport_rcu(dp, out_port);
 
+   if (unlikely(max_len != 0))
+   OVS_CB(skb)->max_len = max_len;
+
if (likely(vport)) {
u16 mru = OVS_CB(skb)->mru;
 
@@ -1034,6 +1037,7 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
 * is slightly obscure just to avoid that.
 */
int prev_port = -1;
+   uint16_t max_len = 0;
const struct nlattr *a;
int rem;
 
@@ -1045,9 +1049,10 @@ static int do_execute_actions(struct datapath *dp, 
struct sk_buff *skb,
struct sk_buff *out_skb = skb_clone(skb, GFP_ATOMIC);
 
if (out_skb)
-   do_output(dp, out_skb, prev_port, key);
+   do_output(dp, out_skb, prev_port, max_len, key);
 
prev_port = -1;
+   max_len = 0;
}
 
switch (nla_type(a)) {
@@ -1055,6 +1060,13 @@ static int do_execute_actions(struct datapath *dp, 
struct sk_buff *skb,
prev_port = nla_get_u32(a);
break;
 
+case OVS_ACTION_ATTR_OUTPUT_TRUNC: {
+   struct ovs_action_output_trunc *output_trunc = 
nla_data(a);
+   prev_port = output_trunc->port;
+   max_len = output_trunc->max_len;
+   break;
+}
+
case OVS_ACTION_ATTR_USERSPACE:
output_userspace(dp, skb, key, a, attr, len);
break;
@@ -1126,7 +1138,7 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
}
 
if (prev_port != -1)
-   do_output(dp, skb, prev_port, key);
+   do_output(dp, skb, prev_port, max_len, key);
else
consume_skb(skb);
 
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 

Re: [ovs-dev] [PATCH v13 0/8] Add incremental processing

[Han Zhou]

On Thu, Mar 31, 2016 at 8:05 AM, Ryan Moats  wrote:

> From: RYAN D. MOATS 
>
> It looks like v11 and v12 had some interesting rebase issues,
> so v13 is a rebase back to master only
>
> RYAN D. MOATS (8):
>   Make flow table persistent in ovn controller
>   Persist lport and mcgroup indexes
>   Persist local_datapaths and patched_datapaths
>   Add incremental proessing to lflow_run
>   Change encaps_run to work incrementally
>   Convert binding_run to incremental processing.
>   Reset lflow processing when adding/removing patch ports
>   Change physical_run to incremental processing
>
>  lib/ofp-actions.c   |   12 ++
>  lib/ofp-actions.h   |2 +
>  ovn/controller/binding.c|   99 +--
>  ovn/controller/binding.h|1 +
>  ovn/controller/encaps.c |  163 +---
>  ovn/controller/lflow.c  |  116 +++--
>  ovn/controller/lflow.h  |6 +-
>  ovn/controller/lport.c  |  142 +++---
>  ovn/controller/lport.h  |   20 +++-
>  ovn/controller/ofctrl.c |  264
> +++---
>  ovn/controller/ofctrl.h |   18 ++-
>  ovn/controller/ovn-controller.c |   52 +++-
>  ovn/controller/ovn-controller.h |2 +
>  ovn/controller/patch.c  |7 +-
>  ovn/controller/physical.c   |  201 +++---
>  ovn/controller/physical.h   |6 +-
>  16 files changed, 855 insertions(+), 256 deletions(-)
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>

Here is the profiling result from Lei. Based on the testing result, this
version is great! Thanks Ryan.
version

This test uses commit f48e869 on ovs/master as base and OVN incremental
patch v13.

Below patches are already included in ovs/master:

   - ovn-controller: Loopback prevention flows for local ports only.
   -

   ovn-controller: Optimize processing for non-local datapath without patch
   ports.
   -

   ovn-controller: Optimize lex_token memory usage.

Test case running time

For the create and bind 500 lports (on top of 20k lports on 2k HVs), the
running time as below:
ovn_network.create_network   1.846 1.834
ovn.create_lswitch   1.086 1.084
ovn.create_lport 0.77 0.804
ovn_network.bind_port77.012 71.74
ovn_network.wait_port_up 0.788 0.704
ovn.create_lport (2) 0.758 0.75
ovn_network.bind_port (2)77.33 70.228
ovn_network.wait_port_up (2) 0.803 0.698
ovn.create_lport (3) 0.821 0.836
ovn_network.bind_port (3)79.842 72.493
ovn_network.wait_port_up (3) 0.956 0.782
ovn.create_lport (4) 0.795 0.843
ovn_network.bind_port (4)81.584 72.866
ovn_network.wait_port_up (4) 0.78 0.807
ovn.create_lport (5) 0.837 0.823
ovn_network.bind_port (5)83.603 72.483
ovn_network.wait_port_up (5) 0.856 0.867
total405.199 365.701


Profiling

Before apply ovn incremental processing v13 patch:
-  15.71%  ovn-controller  ovn-controller  [.]
next_real_row

   - next_real_row
  - 47.10% add_logical_flows.isra.2
   lflow_run
   main
  + 21.55% patch_run
  + 10.13% physical_run
  + 9.85% binding_run
  + 9.77% lport_index_init
  + 1.03% encaps_run
-  12.98%  ovn-controller  ovn-controller  [.] ovsdb_idl_next_row
   - ovsdb_idl_next_row
  + 56.97% add_logical_flows.isra.2
  + 29.25% patch_run
  + 5.90% lport_index_init
  + 3.29% binding_run
  + 2.95% physical_run
  + 1.35% encaps_run
-   8.54%  ovn-controller  ovn-controller  [.] physical_run
 physical_run
 main
-   7.89%  ovn-controller  ovn-controller  [.] add_logical_flows.isra.2
 add_logical_flows.isra.2
 lflow_run
 main
-   7.15%  ovn-controller  libc-2.19.so[.] strlen
   - strlen
  + 26.47% simap_find
  + 24.20% shash_find
  + 23.55% lport_lookup_by_name
  + 12.42% sset_add
  + 4.70% smap_get_node
  + 3.77% port_hash
  + 1.83% physical_run
  + 1.35% lport_index_init
  + 0.84% chassis_tunnel_find
  + 0.54% mcgroup_lookup_by_dp_name
-   4.71%  ovn-controller  ovn-controller  [.] hash_bytes
   - hash_bytes
  + 42.36% smap_get_node
  + 15.57% simap_find_len
  + 9.25% shash_find
  + 8.28% lport_index_init
  + 7.98% lport_lookup_by_name
  + 7.60% sset_add
  + 4.08% port_hash
  + 3.78% chassis_tunnel_find
  + 0.66% physical_run
-   3.26%  ovn-controller  ovn-controller  [.] smap_find__
   + smap_find__
   + smap_get



After apply the patch:
-   5.82%  ovn-controller  libc-2.19.so[.] __memcmp_sse4_1
   - __memcmp_sse4_1
  - 43.03% ofpacts_equal
   ofctrl_put
   main
  - 30.66% match_equal
   ovn_flow_lookup
   ofctrl_put
   main
  - 26.31% flow_wildcards_equal
   

[ovs-dev] [PATCH V3] Configure ovn-controller SB probe_timer on the fly and disable other unix domain socket based connections

[nghosh]

Configure ovn-controller SB probe_timer on the fly and disable other unix 
domain socket based connections

There are four sessions established from ovn-controller to the following:
OVN Southbound — JSONRPC based
Local ovsdb — JSONRPC based
Local vswitchd — openflow based from ofctrl
Local vswitchd — openflow based from pinctrl

All of these sessions have their own probe_interval, and currently only one
[SB] of them can be configured using ovn-vsctl command, and that is not
effective on the fly —in other words, ovn-controller has to be restarted to
use that probe_timer value. For the rest, probe timers need to be disabled
as they are over unix domain socket. ovsdb was already doing that.

This patch will take care of disabling probe timer for ofctrl and pinctrl,
and make sure SB timer changes take effect on the fly.

Using NB database’s external-id, SB timer settings are stored, here is how 
to
set it:
ovs-vsctl --no-wait set open_vswitch . \
   external-ids:ovn-remote-probe-interval=7000

Signed-off-by: Nirapada Ghosh 

diff --git a/lib/rconn.c b/lib/rconn.c
index 6de4c63..3ebe0e8 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -324,6 +324,15 @@ rconn_get_probe_interval(const struct rconn *rc)
 return rc->probe_interval;
 }
 
+/* For unix domain socket, target starts with "unix:", the following function 
returns
+ * True when the argument indicates that it is unix domain socket
+ */
+static inline bool
+unix_domain_socket(const char *target)
+{
+   return (!(strncmp(target,"unix:",5)));
+}
+
 /* Drops any existing connection on 'rc', then sets up 'rc' to connect to
  * 'target' and reconnect as needed.  'target' should be a remote OpenFlow
  * target in a form acceptable to vconn_open().
@@ -339,6 +348,9 @@ rconn_connect(struct rconn *rc, const char *target, const 
char *name)
 rconn_disconnect__(rc);
 rconn_set_target__(rc, target, name);
 rc->reliable = true;
+if (unix_domain_socket(target)) {
+   rc->probe_interval =0; /* we do not need probe_timer for unix domain 
sockets */
+}
 reconnect(rc);
 ovs_mutex_unlock(>mutex);
 }
@@ -461,6 +473,7 @@ reconnect(struct rconn *rc)
 if (!retval) {
 rc->backoff_deadline = time_now() + rc->backoff;
 state_transition(rc, S_CONNECTING);
+VLOG_INFO("connected with probe-interval %d", rc->probe_interval);
 } else {
 VLOG_WARN("%s: connection failed (%s)",
   rc->name, ovs_strerror(retval));
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 6027011..9751e13 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -52,16 +52,52 @@
 
 VLOG_DEFINE_THIS_MODULE(main);
 
+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
+   const struct ovsdb_idl *sb_idl);
+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value_ptr);
+
 static unixctl_cb_func ovn_controller_exit;
 static unixctl_cb_func ct_zone_list;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_PROBE_INTERVAL 5
 
 static void parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);
 
 static char *ovs_remote;
 
+/* Given key_name, the following function retrieves probe_timer value from the
+ * configuration passed, this configuration comes from the "external-ids"
+ * which were configured via ovs-vsctl command.
+ *
+ * cfg: Holding the external-id values read from NB database.
+ * keyname: Name to extract the value for.
+ * ret_value_ptr: Pointer to integer location where the value read
+ * should be copied.
+ * The function returns true if success, keeps the original
+ * value of ret_value_ptr intact in case of a failure.
+ */
+static bool
+extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value_ptr)
+{
+const char *probe_interval= smap_get(>external_ids, key_name);
+int ret_value_temp=0; /* Temporary location to hold the value, in case of
+   * failure, str_to_int() sets the ret_value_temp to 
0,
+   * which is a valid value for us */
+if ((!probe_interval) ||  (!str_to_int(probe_interval, 10, 
_value_temp)))  {
+VLOG_WARN("OVN OVSDB invalid remote probe interval:%s for %s",
+ probe_interval, key_name);
+return false;
+}
+*ret_value_ptr = ret_value_temp;
+return true;
+}
+
 const struct sbrec_chassis *
 get_chassis(struct ovsdb_idl *ovnsb_idl, const char *chassis_id)
 {
@@ -198,30 +234,27 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 }
 }
 
-/* Retrieves the OVN Southbound remote's json session probe interval from the
- * "external-ids:ovn-remote-probe-interval" 

[ovs-dev] [PATCH V2] Configure ovn-controller SB probe_timer on the fly and disable other unix domain socket based connections

[nghosh]

Configure ovn-controller SB probe_timer on the fly and disable other unix 
domain socket based connections

There are four sessions established from ovn-controller to the following:
OVN Southbound — JSONRPC based
Local ovsdb — JSONRPC based
Local vswitchd — openflow based from ofctrl
Local vswitchd — openflow based from pinctrl

All of these sessions have their own probe_interval, and currently only one
[SB] of them can be configured using ovn-vsctl command, and that is not
effective on the fly —in other words, ovn-controller has to be restarted to
use that probe_timer value. For the rest, probe timers need to be disabled
as they are over unix domain socket. ovsdb was already doing that.

This patch will take care of disabling probe timer for ofctrl and pictrl,
and make sure SB timer changes take effect on the fly.

Using NB database’s external-id, SB timer settings are stored, here is how 
to
set it:
ovs-vsctl --no-wait set open_vswitch . \
   external-ids:ovn-remote-probe-interval=7000

Signed-off-by: Nirapada Ghosh 

diff --git a/lib/rconn.c b/lib/rconn.c
index 6de4c63..3ebe0e8 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -324,6 +324,15 @@ rconn_get_probe_interval(const struct rconn *rc)
 return rc->probe_interval;
 }
 
+/* For unix domain socket, target starts with "unix:", the following function 
returns
+ * True when the argument indicates that it is unix domain socket
+ */
+static inline bool
+unix_domain_socket(const char *target)
+{
+   return (!(strncmp(target,"unix:",5)));
+}
+
 /* Drops any existing connection on 'rc', then sets up 'rc' to connect to
  * 'target' and reconnect as needed.  'target' should be a remote OpenFlow
  * target in a form acceptable to vconn_open().
@@ -339,6 +348,9 @@ rconn_connect(struct rconn *rc, const char *target, const 
char *name)
 rconn_disconnect__(rc);
 rconn_set_target__(rc, target, name);
 rc->reliable = true;
+if (unix_domain_socket(target)) {
+   rc->probe_interval =0; /* we do not need probe_timer for unix domain 
sockets */
+}
 reconnect(rc);
 ovs_mutex_unlock(>mutex);
 }
@@ -461,6 +473,7 @@ reconnect(struct rconn *rc)
 if (!retval) {
 rc->backoff_deadline = time_now() + rc->backoff;
 state_transition(rc, S_CONNECTING);
+VLOG_INFO("connected with probe-interval %d", rc->probe_interval);
 } else {
 VLOG_WARN("%s: connection failed (%s)",
   rc->name, ovs_strerror(retval));
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 6027011..cfb7b0f 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -52,16 +52,52 @@
 
 VLOG_DEFINE_THIS_MODULE(main);
 
+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
+   const struct ovsdb_idl *sb_idl);
+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value_ptr);
+
 static unixctl_cb_func ovn_controller_exit;
 static unixctl_cb_func ct_zone_list;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_PROBE_INTERVAL 5
 
 static void parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);
 
 static char *ovs_remote;
 
+/* Given key_name, the following function retrieves probe_timer value from the
+ * configuration passed, this configuration comes from the "external-ids"
+ * which were configured via ovs-vsctl command.
+ *
+ * cfg: Holding the external-id values read from NB database.
+ * keyname: Name to extract the value for.
+ * ret_value_ptr: Pointer to integer location where the value read
+ * should be copied.
+ * The function returns true if success, keeps the original
+ * value of ret_value_ptr intact in case of a failure.
+ */
+static bool
+extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value_ptr)
+{
+const char *probe_interval= smap_get(>external_ids, key_name);
+int ret_value_temp=0; /* Temporary location to hold the value, in case of
+   * failure, str_to_int() sets the ret_value_temp to 
0,
+   * which is a valid value for us */
+if ((!probe_interval) ||  (!str_to_int(probe_interval, 10, 
_value_temp)))  {
+VLOG_WARN("OVN OVSDB invalid remote probe interval:%s for %s",
+ probe_interval, key_name);
+return false;
+}
+*ret_value_ptr = ret_value_temp;
+return true;
+}
+
 const struct sbrec_chassis *
 get_chassis(struct ovsdb_idl *ovnsb_idl, const char *chassis_id)
 {
@@ -198,30 +234,27 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 }
 }
 
-/* Retrieves the OVN Southbound remote's json session probe interval from the
- * "external-ids:ovn-remote-probe-interval" key 

Re: [ovs-dev] [PATCH] stt: linearize for CONFIG_SLUB case

[pravin shelar]

On Thu, Mar 31, 2016 at 9:06 PM, Jesse Gross  wrote:
> On Thu, Mar 31, 2016 at 2:30 PM, Pravin B Shelar  wrote:
>> STT implementation we saw performance improvements with linearizing
>> skb for SLUB case.  So following patch skips zero copy operation
>> for such a case.
>>
>> Tested-By: Vasmi Abidi 
>> Signed-off-by: Pravin B Shelar 
>
> Can you add some performance numbers to the commit message?
>
ok.

>> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
>> index eb397e8..ae33d5e 100644
>> --- a/datapath/linux/compat/stt.c
>> +++ b/datapath/linux/compat/stt.c
>>  static int coalesce_skb(struct sk_buff **headp)
>>  {
>> -   struct sk_buff *frag, *head, *prev;
>> +   struct sk_buff *frag, *head;
>> +#ifndef SKIP_ZERO_COPY
>> +   struct sk_buff *prev;
>> +#endif
>> int err;
>>
>> err = straighten_frag_list(headp);
>
> I don't think that we need to straighten the frag list in the
> SKIP_ZERO_COPY case. It's basically just undoing what the for loop
> that forms the frag list will do. __skb_linearize() will be able to
> handle it in any case.
>
I can skip straightening and work on skb with frag-list, but I do not
want to complicate the code.  This case is pretty rare after the
change in reassemble where complete skb is allocated on first set
segment.

>> +#ifdef SKIP_ZERO_COPY
>> +   if (skb_shinfo(head)->frag_list) {
>> +   err = __skb_linearize(head);
>> +   return err;
>> +   }
>> +#endif
>
> Maybe move this to try_to_segment()? It seems like it is a little more
> consistent.
>
But it is not same. I think we only need to linearize of there is
frag-list, AFAIK non linear data in skb_shinfo can be safely handled
in skb-segment.

>>  static int segment_skb(struct sk_buff **headp, bool csum_partial,
>>bool ipv4, bool tcp, int l4_offset)
>>  {
>> +#ifndef SKIP_ZERO_COPY
>> int err;
>>
>> err = coalesce_skb(headp);
>> @@ -543,6 +575,7 @@ static int segment_skb(struct sk_buff **headp, bool 
>> csum_partial,
>> if (skb_shinfo(*headp)->frag_list)
>> return __try_to_segment(*headp, csum_partial,
>> ipv4, tcp, l4_offset);
>> +#endif
>> return 0;
>>  }
>
> Is this OK? It will retain a skb with a frag_list on the transmit path
> where this wasn't possible before. This used to cause kernel panics
> since the skb geometry wasn't even when the packet went to be
> segmented. I don't remember if this is still the case but if not then
> it seems like we should be able to simply the code regardless of this
> patch.
>
right, I missed it. I will keep the segmentation here.

>> @@ -1093,6 +1141,15 @@ static struct sk_buff *reassemble(struct sk_buff *skb)
>>
>> frag = lookup_frag(dev_net(skb->dev), stt_percpu, , hash);
>> if (!frag->skbs) {
>> +   int err;
>> +
>> +   err = pskb_expand_head(skb, skb_headroom(skb),
>> +  skb->data_len + tot_len, GFP_ATOMIC);
>> +   if (likely(!err)) {
>> +   if (unlikely(!__pskb_pull_tail(skb, skb->data_len)))
>> +   BUG();
>> +   }
>
> This linearizes the packet even in non-SKIP_ZERO_COPY cases. I guess
> we probably don't want to do that. It's also possible that the first
> skb received isn't necessarily the first packet in the reassembled
> packet.
>
ok, I will fix it.

> This is effectively optimizing for the case where all packets are
> large and will generate a frag list after reassembly. However, it's
> possible in many situations that packets can be reassembled with zero
> copy that doesn't later need to be split (maybe the total packet is
> around 20k). Do you know how the performance compares vs. just
> linearizing at the end in that case? Is there a benefit to doing an
> early copy?
>
We are not trying to coalesce skb in case of SKIP_ZERO_COPY, so any
partial skb segment will generate multiple inner segments. I have seen
better performance with this approach rather than linearizing at the
end.

>> @@ -1154,8 +1216,10 @@ static struct sk_buff *reassemble(struct sk_buff *skb)
>>
>> FRAG_CB(frag->skbs)->first.set_ecn_ce |= INET_ECN_is_ce(iph->tos);
>> FRAG_CB(frag->skbs)->first.rcvd_len += skb->len;
>> -   FRAG_CB(frag->skbs)->first.mem_used += skb->truesize;
>> -   stt_percpu->frag_mem_used += skb->truesize;
>> +   if (!copied_skb) {
>> +   FRAG_CB(frag->skbs)->first.mem_used += skb->truesize;
>> +   stt_percpu->frag_mem_used += skb->truesize;
>> +   }
>
> pskb_expand_head() doesn't actually change the truesize so our count
> will be more inaccurate than before. However, we don't have to worry
> about the attack case of very small packets consuming a large amount
> of memory due to having many copies of struct sk_buff.

ok. I will 

Re: [ovs-dev] [PATCH] Configure or disable ovn-controller probe timer on the fly

[Ben Pfaff]

On Fri, Apr 01, 2016 at 10:53:24AM -0800, Nirapada Ghosh wrote:
> So, in summary, here is what I am going to do:
> 
> 1) Take out the changes [monitoring and setting the probe_timer] for all
> but the first one [I believe we still need this]
> 2) Disable probe timers from rconn_connect() if the connection is over
> "unix domain sockets"
> 
> Would you please confirm that my understanding is right ?

Sounds right to me.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovsdb-server multithreading RFC 6/9] ovsdb: Add IPC messages for thread communication

[Andy Zhou]

On Fri, Apr 1, 2016 at 11:45 AM, Ryan Moats  wrote:

> I gave this patch set a spin this week and I agree
> that a less granular lock would be a good thing.
>
Thanks for the report.  Really appreciate the feedback.
I did some testing as well and reached the same conclusion. I am now
revising the series to use rwlock.

>
> In addition (and this is a nit), when I looked at
> the logs, I think it would help if the main thread
> were identified by (main_thread) for consistency
> with how other threads report.  Here's an example
> of what shows up now:
>
> 2016-04-01T16:11:28.489Z|8|ovsdb_jsonrpc_server(sessions_thread1)|ERR|thread
> receive command msg
> 2016-04-01T16:11:28.489Z|00015|memory|INFO|cells:266325 json-caches:3
> monitors:3 sessions:5
> 2016-04-01T16:21:27.781Z|00016|ovsdb_file|INFO|/opt/stack/data/ovs/ovnsb.db:
> compacting database online (1458329582.712 seconds old, 1573 transactions,
> 12266630 bytes)
> 2016-04-01T16:31:28.950Z|9|ovsdb_file(sessions_thread1)|INFO|/opt/stack/data/ovs/ovnsb.db:
> compacting database online (1458329582.971 seconds old, 1623 transactions,
> 15439453 bytes)
> 2016-04-01T16:31:30.078Z|00010|timeval(sessions_thread1)|WARN|Unreasonably
> long 1129ms poll interval (589ms user, 46ms system)
>
> Make sense. I will take a look to see if this can be implemented cleanly.

> Lastly, I'm not sure what the first line above
> is conveying, but there were a *LOT* of them,
> so I'd suggest either removing or lowering the log
> level to DEBUG.
>
I noticed this too.  It is now removed. The log message is not that useful.

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

Re: [ovs-dev] [ovsdb-server multithreading RFC 6/9] ovsdb: Add IPC messages for thread communication

[Ryan Moats]

I gave this patch set a spin this week and I agree
that a less granular lock would be a good thing.
In addition (and this is a nit), when I looked at
the logs, I think it would help if the main thread
were identified by (main_thread) for consistency
with how other threads report.  Here's an example
of what shows up now:

2016-04-01T16:11:28.489Z|8|ovsdb_jsonrpc_server(sessions_thread1)|ERR|
thread receive command msg
2016-04-01T16:11:28.489Z|00015|memory|INFO|cells:266325 json-caches:3
monitors:3 sessions:5
2016-04-01T16:21:27.781Z|00016|ovsdb_file|
INFO|/opt/stack/data/ovs/ovnsb.db: compacting database online
(1458329582.712 seconds old, 1573 transactions, 12266630 bytes)
2016-04-01T16:31:28.950Z|9|ovsdb_file(sessions_thread1)|
INFO|/opt/stack/data/ovs/ovnsb.db: compacting database online
(1458329582.971 seconds old, 1623 transactions, 15439453 bytes)
2016-04-01T16:31:30.078Z|00010|timeval(sessions_thread1)|WARN|Unreasonably
long 1129ms poll interval (589ms user, 46ms system)

Lastly, I'm not sure what the first line above
is conveying, but there were a *LOT* of them,
so I'd suggest either removing or lowering the log
level to DEBUG.

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

Re: [ovs-dev] ich Sie, der als der Erbe meiner 1.700.000Euro gewählt wurde

[igvar.harmburg]

Es ist mit einem von Verzweiflung vollen Herzen, dass ich Ihnen diese Nachricht 
übermittele, um Ihr Abkommen für die Verwirklichung einer Schenkung zu 
ersuchen, ich sind momentan krank, und angesichts meines Alters und meines 
derzeitigen Gesundheitszustandes wünsche ich, Spende meiner Güter zu machen.Ich 
bin Herr Igvar HARMBURG, ex-Berater und Dolmetscher, die im 75008e, Paris 
bleiben, Avenue Matignon und nicht weit weg von den Élysée-Feldern in 
Frankreich, aber momentan unter am 27. März 1944 entstandener medizinischer 
Beobachtung.Meine Ehelage ist so, dass ich noch Frau habe, und noch weniger als 
die Kinder, an denen mein Erbe zu hinterlassen, weil ich meine Ehefrau verloren 
habe er habe dort von das 5 Jahre, was mich viel betroffen hat und ich nicht 
mich bis heute wieder heiraten konnten, wir hatte keine Kinder.Es ist für das, 
den ich unentgeltlich möchte, Ihnen mein Erbe zu hinterlassen, das sich auf 
einen Wert von 1.700.000 Euro beläuft, damit Sie die Werke fortsetzen können, 
die ich so sehr habe zu wünschen, auf dieser Erde zu machen. Wissen Sie, dass 
ich diese Schenkung mache, weil es mein geistiger Vater ist, der es mir hat zu 
verlangen. Wenn ich gewählt habe, Sie zu kontaktieren, ist es, weil ich 
wünsche, dass das sich um ohne zu viel Werbung wegen meiner früheren 
Aktivitäten macht, ich Sie Kontakt heute, damit Sie akzeptieren, als legaler 
Verwalter (Empfänger) zu dienen, und ein Aktionsprogramm zu verwirklichen, das 
heißt eine Stiftung oder eine NRO, um in Altenhilfe und zu den Kindern in 
schwieriger Lage zu kommen.Ich möchte sterben, indem ich dem Profil der Bank 
oder des öffentlichen Schatzes ohne legitimer Empfänger diese Summe ließe. Es 
ist für das, den ich Ihnen, indem ich mich an diesem Tag schreibe, um Ihnen 
mein Werk, ganz mitzuteilen in bewusst bin, das wir wir nicht kennen.Welches 
Projekt beabsichtigen Sie, mit diesen Fonds zu verwirklichen? In erster Linie 
ist es wichtig, dass wir eine Vertrauensbasis zwischen Ihnen und mir 
aufstellten, um dieses Projekt gut zu führen.Dass Gott Sie segnet.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] employees needed

[dev]

Hello!

We are looking for employees working remotely.

My name is Lucas, am the personnel manager of a large International company.
Most of the work you can do from home, that is, at a distance.

Salary is $2000-$5000.

If you are interested in this offer, please visit 
Our Web Page

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

[ovs-dev] [PATCH v4 2/2] ovn: Apply ACL changes to existing connections.

[Russell Bryant]

Prior to this commit, once a connection had been committed to the
connection tracker, the connection would continue to be allowed, even
if the policy defined in the ACL table changed.  This patch changes
the implementation so that existing connections are affected by policy
changes.

The implementation is based on the suggested approach in this mailing
list thread:

http://openvswitch.org/pipermail/dev/2016-February/065716.html

Instead of always allowing packets associated with an established
connection, we now put all packets in the request direction through
the flows generated based on OVN ACLs.  If a packet associated with an
established connection hits a "drop" ACL, that means we have
encountered a policy change and should drop packets associated with
this connection from now on.  We handle this by setting "ct_label" on
the associated connection tracking entry.

These changes also account for re-allowing a known connection after
ct_label had been set on it. This can happen if you delete an ACL and
then re-create it while connection state is still known.

The proposal on the mailing list also discussed the idea that
ovn-controller could periodically sweep the connection tracker and
delete entries with ct_label set.  That is not implemented in this
patch.  Instead, we rely on connections dying since we're dropping
its packets and then allowing the connection tracking entry to
eventually time out.  More proactively clearing them out could be a
future enhancement.

As a realistic example of how this works, consider this security policy
from an OpenStack+OVN development environment.

+-+---+
| name| security_group_rules  |
+-+---+
| default | egress, IPv4  |
| | egress, IPv6  |
| | ingress, IPv4, 22/tcp |
| | ingress, IPv4, icmp   |
+-+---+

The OpenStack Neutron plugin creates ACLs that drop traffic by default
and higher priority ACLs for each type of traffic that is allowed.  In
this case, the ACLs for a port using the "default" security group are:

  from-lport  1002 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4) 
allow-related
  from-lport  1002 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6) 
allow-related
  from-lport  1001 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip) drop
to-lport  1002 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 && 
icmp4) allow-related
to-lport  1002 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 && 
tcp && tcp.dst == 22) allow-related
to-lport  1001 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip) 
drop

which results in the following logical flows:

  table=1(   ls_in_pre_acl), priority=  100, match=(ip), action=(ct_next;)
  table=1(   ls_in_pre_acl), priority=0, match=(1), action=(next;)
  table=2(   ls_in_acl), priority=65535, match=(!ct.est && ct.rel && 
!ct.new && !ct.inv && ct_label[0] == 0), action=(next;)
  table=2(   ls_in_acl), priority=65535, match=(ct.est && !ct.rel && 
!ct.new && !ct.inv && ct.rpl && ct_label[0] == 0), action=(next;)
  table=2(   ls_in_acl), priority=65535, match=(ct.inv || (ct.est && ct.rpl 
&& ct_label[0] == 1)), action=(drop;)
  table=2(   ls_in_acl), priority= 2002, match=(!ct.new && ct.est && 
!ct.rpl && ct_label[0] == 0 && (inport == 
"0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4)), action=(next;)
  table=2(   ls_in_acl), priority= 2002, match=(!ct.new && ct.est && 
!ct.rpl && ct_label[0] == 0 && (inport == 
"0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6)), action=(next;)
  table=2(   ls_in_acl), priority= 2002, match=(((ct.new && !ct.est) || 
(!ct.new && ct.est && !ct.rpl && ct_label[0] == 1)) && (inport == 
"0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4)), action=(ct_commit(ct_label=0); 
next;)
  table=2(   ls_in_acl), priority= 2002, match=(((ct.new && !ct.est) || 
(!ct.new && ct.est && !ct.rpl && ct_label[0] == 1)) && (inport == 
"0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6)), action=(ct_commit(ct_label=0); 
next;)
  table=2(   ls_in_acl), priority= 2001, match=((!ct.est || (ct.est && 
ct_label[0] == 1)) && (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && 
ip)), action=(drop;)
  table=2(   ls_in_acl), priority= 2001, match=(ct.est && ct_label[0] == 0 
&& (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip)), 
action=(ct_commit(ct_label=1);)
  table=2(   ls_in_acl), priority=1, match=(ip && (!ct.est || (ct.est 
&& ct_label[0] == 1))), action=(ct_commit(ct_label=0); next;)
  table=2(   ls_in_acl), priority=0, match=(1), action=(next;)

  table=0(  ls_out_pre_acl), priority=  100, match=(ip), action=(ct_next;)
  table=0(  ls_out_pre_acl), priority=0, match=(1), action=(next;)
  table=1(  ls_out_acl), priority=65535, match=(!ct.est && ct.rel && 
!ct.new && !ct.inv && ct_label[0] == 0), action=(next;)
  table=1(  ls_out_acl), 

[ovs-dev] [PATCH v4 1/2] ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action.

[Russell Bryant]

Update the "ct_commit;" logical flow action to optionally take
one or two parameters, setting the value of "ct_mark" or "ct_label".
Supported ct_commit syntax now includes:

ct_commit;
ct_commit();
ct_commit(ct_mark=1);
ct_commit(ct_label=1);
ct_commit(ct_mark=1, ct_label=1);

We would like to eventually also allow setting ct_mark and ct_label with
a masked value.  There are currently problems with this that Joe is
working on, so that support will be added later.

Setting ct_mark via this type of logical flow results in an OpenFlow
flow that looks like:

actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_mark))

Similarly, setting ct_label results in:

actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_label))

A future commit will make use of this feature.

Signed-off-by: Russell Bryant 
Acked-by: Ben Pfaff 
---
 ovn/lib/actions.c | 120 --
 ovn/ovn-sb.xml|  13 +-
 tests/ovn.at  |   4 ++
 3 files changed, 132 insertions(+), 5 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 44957c7..9d6f557 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -412,7 +412,8 @@ parse_put_arp_action(struct action_context *ctx)
 }
 
 static void
-emit_ct(struct action_context *ctx, bool recirc_next, bool commit)
+emit_ct(struct action_context *ctx, bool recirc_next, bool commit,
+int *ct_mark, ovs_be128 *ct_label)
 {
 struct ofpact_conntrack *ct = ofpact_put_CT(ctx->ofpacts);
 ct->flags |= commit ? NX_CT_F_COMMIT : 0;
@@ -438,6 +439,119 @@ emit_ct(struct action_context *ctx, bool recirc_next, 
bool commit)
 
 /* CT only works with IP, so set up a prerequisite. */
 add_prerequisite(ctx, "ip");
+
+size_t set_field_offset = ctx->ofpacts->size;
+ofpbuf_pull(ctx->ofpacts, set_field_offset);
+
+if (ct_mark) {
+struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ctx->ofpacts);
+sf->field = mf_from_id(MFF_CT_MARK);
+sf->value.be32 = htonl(*ct_mark);
+sf->mask.be32 = OVS_BE32_MAX;
+}
+
+if (ct_label) {
+struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ctx->ofpacts);
+sf->field = mf_from_id(MFF_CT_LABEL);
+sf->value.be128 = *ct_label;
+sf->mask.be128 = OVS_BE128_MAX;
+}
+
+ctx->ofpacts->header = ofpbuf_push_uninit(ctx->ofpacts, set_field_offset);
+ct = ctx->ofpacts->header;
+ofpact_finish(ctx->ofpacts, >ofpact);
+}
+
+/* Parse an argument to the ct_commit(); action.  Supported arguments include:
+ *
+ *  ct_mark=0
+ *  ct_label=0
+ *
+ * If a comma separates the current argument from the next argument, this
+ * function will consume it.
+ *
+ * Return true after successfully parsing an argument.  false on failure. */
+static bool
+parse_ct_commit_arg(struct action_context *ctx,
+bool *set_mark, int *mark_value,
+bool *set_label, ovs_be128 *label_value)
+{
+if (lexer_match_id(ctx->lexer, "ct_mark")) {
+if (!lexer_match(ctx->lexer, LEX_T_EQUALS)) {
+action_error(ctx, "Expected '=' after argument to ct_commit");
+return false;
+}
+if (!action_get_int(ctx, mark_value)) {
+return false;
+}
+*set_mark = true;
+} else if (lexer_match_id(ctx->lexer, "ct_label")) {
+if (!lexer_match(ctx->lexer, LEX_T_EQUALS)) {
+action_error(ctx, "Expected '=' after argument to ct_commit");
+return false;
+}
+/* XXX We only support a ct_label value specified as decimal.
+ * ct_label is 128-bit, so we should eventually also support specifying
+ * full 128-bit values as hex.  Hex support isn't really needed until
+ * we need more than 32 bits. */
+int val;
+if (!action_get_int(ctx, )) {
+return false;
+}
+label_value->be32[3] = htonl(val);
+*set_label = true;
+} else {
+action_error(ctx, "Expected argument to ct_commit()");
+return false;
+}
+
+if (lexer_match(ctx->lexer, LEX_T_COMMA)) {
+/* A comma is valid after an argument, but only if another
+ * argument is present (not a closing paren) */
+if (lexer_lookahead(ctx->lexer) == LEX_T_RPAREN) {
+action_error(ctx, "Another argument to ct_commit() expected "
+  "after comma.");
+return false;
+}
+}
+
+return true;
+}
+
+static void
+parse_ct_commit_action(struct action_context *ctx)
+{
+if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+/* ct_commit; */
+emit_ct(ctx, false, true, NULL, NULL);
+return;
+}
+
+if (lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+/* ct_commit(); */
+emit_ct(ctx, false, true, NULL, NULL);
+return;
+}
+
+/* ct_commit(ct_mark=0); */
+/* ct_commit(ct_label=0); */

[ovs-dev] [PATCH v4 0/2] ovn: Apply ACL changes to existing connections.

[Russell Bryant]

Prior to this commit, once a connection had been committed to the
connection tracker, the connection would continue to be allowed, even
if the policy defined in the ACL table changed.  This patch changes
the implementation so that existing connections are affected by policy
changes.

The implementation is based on the suggested approach in this mailing
list thread:

http://openvswitch.org/pipermail/dev/2016-February/065716.html

The implementation is covered in much more detail in the commit message
for patch 3, as well as code comments and doc updates.

v1->v2:
 - Address issue pointed out by Han Zhou where removing and then re-creating
   an ACL did not allow an established connection to continue.  The changes
   are in patch 3.
v2->v3:
 - rebase and resolve conflicts with master.
 - Use ct_label instead of ct_mark.
 - patch 1: add ACK from han, otherwise unchanged
 - patch 2: add support for setting ct_label. v2 only included ct_mark.
   I did not include Han's ACK here because the changes were non trivial.
 - patch 3: add ACK from han. The rest of the changes are trivial
   replacement of ct_mark with ct_label.
v3->v4:
 - Added tests for additions to the ct_commit() logical flow action.
 - Simplified ct_commit() logical flow action additions as suggested by Ben.
 - Lots of doc cleanup as suggested by Justin.

Russell Bryant (2):
  ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action.
  ovn: Apply ACL changes to existing connections.

 ovn/lib/actions.c   | 120 -
 ovn/northd/ovn-northd.8.xml |  58 +++---
 ovn/northd/ovn-northd.c | 182 +---
 ovn/ovn-sb.xml  |  13 +++-
 tests/ovn.at|   4 +
 5 files changed, 317 insertions(+), 60 deletions(-)

-- 
2.5.5

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

Re: [ovs-dev] [PATCH v3 3/3] ovn: Apply ACL changes to existing connections.

[Russell Bryant]

On Tue, Mar 29, 2016 at 7:26 PM, Justin Pettit  wrote:

>
> > On Mar 21, 2016, at 7:54 AM, Russell Bryant  wrote:
> >
> > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> > index 2cc9c34..c8cca54 100644
> > --- a/ovn/northd/ovn-northd.8.xml
> > +++ b/ovn/northd/ovn-northd.8.xml
> >
> > +allow ACLs translate into logical flows with
> > +the next; action.  If there are any stateful ACLs
> > +on this datapath, then alloc ACLs translate to
>
> Should that be "allow" instead of "alloc"?


That was just a test to see who read the docs very closely.  You passed.


>
> > +ct_commit; next;.
> > +  
> > +  
> > +allow-related ACLs translate into logical
> > +flows with the ct_commit(ct_label=0); next; actions
> > +for new connections and "next;" for existing connections.
>
> Should that "next;" be surrounded by a  declaration instead of
> quotes?


Done, thanks.


>
> > -A priority-65535 flow that allows any traffic that has been
> > -committed to the connection tracker (i.e., established flows).
> > +A priority-65535 flow that allows any traffic in the reply
> > +direction for a connection that has been committed to the
> > +connection tracker (i.e., established flows), as long as
> > +the committed flow does not have ct_label=1 set.
>
> I realize it has the same effect since we set the whole field, but I think
> it actually just matches on "ct_label[0]".


You're right, thanks.


>
> > +We only handle traffic in the reply direction here because
> > +we want all packets going in the request direction to still
> > +go through the flows that implement the currently defined
> > +policy based on ACLs.  If a connection is no longer allowed by
> > +policy, ct_label will get set and packets in the
> > +reply direction will no longer be allowed, either.
>
> I think it would be good to specify which bit gets set here, since we may
> use other bits in the future, and we may forget to update this comment.
>

Done.


>
> >   
> >
> >   
> > A priority-65535 flow that allows any traffic that is considered
> > related to a committed flow in the connection tracker (e.g., an
> > -ICMP Port Unreachable from a non-listening UDP port).
> > +ICMP Port Unreachable from a non-listening UDP port), as long
> > +as the committed flow does not have ct_label=1 set.
>
> Same comment about "ct_label[0]".


Done.

> A priority-65535 flow that drops all traffic marked by the
> > -connection tracker as invalid.
> > +connection tracker as invalid or reply packets with
> > +ct_label=1 set meaning that the connection should
>
> Ditto.
>

Done.


>
> > +no longer be allowed due to a policy change.  Packets in the
> > +request direction are skipped here to let a newly created
> > +ACL re-allow this connection.
>
> This sentence makes it sound like invalid packets will be allowed in the
> request direction, which I don't think is desired behavior, and it doesn't
> appear to be what the code does (ie, the code looks right to me).
>

Yeah, that sentence was pretty awkward.  I split it into two sections to
hopefully make it more clear.


> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -1384,48 +1384,77 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows, struct hmap *ports)
> > ...
> > + * We use "ct_commit" for a connection that is not already known
> > + * by the connection tracker.  Once a connection is committed,
> > + * subsequent packets will hit the flow at priority 0 that just
> > + * uses "next;"
> > + *
> > + * We also check for established connections that have ct_label
> set
>
> Once again, I would mention "ct_label[0]".
>
>
Done.


> > + * on them.  That's a connection that was disallowed, but is now
> > + * allowed by policy again since it hit this default-allow flow.
> > + * We need to clear the mark to let the connection continue.
>
> "mark" is correct generically, but it might be clearer to say "label".
> Also, I would mention that it's the "special" bit.
>

Done.  That was probably left over from when I was using ct_mark and my
search for "ct_mark" didn't catch it.


>
> > /* Ingress and Egress ACL Table (Priority 65535).
> >  *
> > - * Always allow traffic that is established to a committed
> > - * conntrack entry.  This is enforced at a higher priority than
> > + * Allow reply traffic that is part of an established
> > + * conntrack entry that has not been marked for deletion
> > + * (bit 0 of ct_label).  We only match traffic in the
> > + * reply direction because we want traffic in the request
> > + * direction to hit the currently defined 

Re: [ovs-dev] [PATCH 1/4] docs: OVSDB replication design document

[Ben Pfaff]

I don't think it makes sense to stack replication and Raft-based HA.

Thinking about OpenSwitch, I guess that your use case is something like
this: an OpenSwitch instance maintains, on-box, an authoritative
database instance, and then the replication feature allows that
database's content to be backed up somewhere else.  I see how that
differs from the use case for Raft, where there is no preference for a
particular database server to be authoritative.  What I don't see yet is
how or why that's useful.  What is the use case?

Thanks,

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

Re: [ovs-dev] [PATCH v3 2/3] ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action.

[Russell Bryant]

On Tue, Mar 29, 2016 at 6:30 PM, Justin Pettit  wrote:

> >>> > Bit-level twiddling would indeed be nice.  I didn't have a need for
> it in this series, though.  Are you OK with it coming as a future
> enhancement, or would you like to see it now?
> >>>
> >> In general, I think its nicer to provide a completed interface if it
> doesn't add too much complexity.  (I don't think it requires much (if any)
> more code since we have functions that parse bit-level setting.)  Also, the
> intent of the next patch is really to do flag manipulation and not setting
> a 128-bit field.  However, there should have been some bit-level tests for
> the functionality that's already checked in, so I'm happy to just add the
> tests and this flexibility to my to-do list.
> >>
> > Yeah, you're right, it does matching on the bit.  It's setting the whole
> value, which only works because this was the first use of the field.  It'll
> be needed before we add a second bit.
> >
> > I can add it now.  I was mostly just being lazy.  It may be next week
> before I get to it, though.  I'm traveling all week and hacking time is hit
> or miss.
>
> I definitely understand.  :-)  If you can do that, it would be great, but
> I'm okay if we want to add the support once we clearly document and test
> the capability in OVS itself.  I've sent Joe a message confirming whether
> this is indeed supported.
>

Joe has a couple of patches on the list.  I checked with on IRC and it
sounds like there's still an issue he's working through.  Right now I'm
planning on holding off on adding that unless those fixes are completed
before I post this next revision.

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

Re: [ovs-dev] [PATCH v3 2/3] ovn: Add ct_commit(ct_mark=INT, ct_label=INT); action.

[Russell Bryant]

On Wed, Mar 23, 2016 at 4:48 PM, Ben Pfaff  wrote:

> On Mon, Mar 21, 2016 at 10:54:58AM -0400, Russell Bryant wrote:
> > Update the "ct_commit;" logical flow action to optionally take
> > one or two parameters, setting the value of "ct_mark" or "ct_label".
> > Supported ct_commit syntax now includes:
> >
> > ct_commit;
> > ct_commit();
> > ct_commit(ct_mark=1);
> > ct_commit(ct_label=1);
> > ct_commit(ct_mark=1, ct_label=1);
> >
> > Setting ct_mark via this type of logical flow results in an OpenFlow
> > flow that looks like:
> >
> >
>  actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_mark))
> >
> > Similarly, setting ct_label results in:
> >
> >
>  actions=ct(commit,zone=NXM_NX_REG5[0..15],exec(set_field:0x1->ct_label))
> >
> > A future commit will make use of this feature.
> >
> > Signed-off-by: Russell Bryant 
>
> I have one comment to add to Guru's.
>
> I think that the complicated dance around adding actions to set ct_mark
> and ct_label could be done just once, like this:
>
> size_t set_field_offset = ctx->ofpacts->size;
> ofpbuf_pull(ctx->ofpacts, set_field_offset);
>
> if (ct_mark) {
> struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ctx->ofpacts);
> sf->field = mf_from_id(MFF_CT_MARK);
> sf->value.be32 = htonl(*ct_mark);
> sf->mask.be32 = OVS_BE32_MAX;
> }
>
> if (ct_label) {
> struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ctx->ofpacts);
> sf->field = mf_from_id(MFF_CT_LABEL);
> sf->value.be128 = *ct_label;
> sf->mask.be128 = OVS_BE128_MAX;
> }
>
> ctx->ofpacts->header = ofpbuf_push_uninit(ctx->ofpacts,
> set_field_offset);
> ct = ctx->ofpacts->header;
> ofpact_finish(ctx->ofpacts, >ofpact);
>
> I think that this is conceptually better because I'm a little
> uncomfortable with the idea of ofpact_finish()ing an action more than
> once.  It's harmless in this case, but...
>
> Although I'm acking this, please make sure that you work with Guru to
> get it to a place you both are happy with; I'm not trying to "override"
> him or anything.
>
> Acked-by: Ben Pfaff 
>

Thanks for the suggestion.  I incorporated this for the next revision.

I also added tests.

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

Re: [ovs-dev] [PATCH v3 1/3] ovn: Update ACL flow docs.

[Russell Bryant]

On Wed, Mar 23, 2016 at 4:30 PM, Ben Pfaff  wrote:

> On Mon, Mar 21, 2016 at 10:54:57AM -0400, Russell Bryant wrote:
> > Apply some minor updates to the description of flows related to ACLs.
> >
> > Signed-off-by: Russell Bryant 
> > Acked-by: Han Zhou 
>
> Acked-by: Ben Pfaff 
>

Thanks.  I applied this first patch to master.

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

Re: [ovs-dev] meters support in datapath

[Jarno Rajahalme]

You may want to have a look on meter patches I sent out earlier to ovs dev list:

http://openvswitch.org/pipermail/dev/2015-November/062428.html

The patches are to be applied to ovs master branch at the time of when the 
email was sent.

The meters in the patch series should work fine, but they have a flaw that a 
DROP meter drop drops the whole rest of the pipeline, which is bad if the meter 
is part of a resubmit that results to an output, and there are multiple of such 
resubmits. In this case only the output of the resubmit with the meter action 
should be dropped, not all of them.

 Jarno

> On Apr 1, 2016, at 12:02 AM, Deepanshu Saxena1/CHN/TCS 
>  wrote:
> 
> 
> 
> Hi all, 
> 
> 
> 
> I want to implement and contribute B.19.13-Meter action (EXT-379) of OF 1.5.1 
> to openvswitch 
> 
> 
> According to FAQ: 
> "Q: Does Open vSwitch support OpenFlow meters? 
> 
> 
> A: Since version 2.0, Open vSwitch has OpenFlow protocol support for OpenFlow 
> meters. There is no implementation of meters in the Open vSwitch software 
> switch (neither the kernel-based nor userspace switches)." 
> 
> 
> As per my understanding, meter support in openvswitch datapath is not 
> present. Could you please specify the reason for the same, as I could see the 
> support of meter in datapath of other software switches such as CPqD 
> Softswitch and ofsoftswitch. 
> 
> 
> Also since no datapath support is present, so is there any other way to test 
> meters in openvswitch without using hardware switches, as I would be 
> requiring this for development and testing of "(EXT-379) - meter Action" 
> Thanks & Regards 
> -- 
> Deepanshu Saxena 
> Assistant System Engineer, 
> Tata Consultancy Services 
> Mailto: deepanshu.saxe...@tcs.com 
> Website: http://www.tcs.com 
> =-=-=
> Notice: The information contained in this e-mail
> message and/or attachments to it may contain 
> confidential or privileged information. If you are 
> not the intended recipient, any dissemination, use, 
> review, distribution, printing or copying of the 
> information contained in this e-mail message 
> and/or attachments to it are strictly prohibited. If 
> you have received this communication in error, 
> please notify us by reply e-mail or telephone and 
> immediately and permanently delete the message 
> and any attachments. Thank you
> 
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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

Re: [ovs-dev] [PATCH v6 06/12] dpif-netdev: Wait an RCU grace period before freeing ports.

[Jarno Rajahalme]

> On Mar 30, 2016, at 8:08 PM, Daniele Di Proietto  
> wrote:
> 
> 
> On 30/03/2016 16:01, "Ben Pfaff"  wrote:
> 
>> (I'm taking a look at this patch specifically because Daniele asked me;
>> I'm not planning to review the whole series.)
>> 
>> On Mon, Mar 28, 2016 at 12:41:40PM -0700, Daniele Di Proietto wrote:
>>> The dpif-netdev datapath keeps ports in a cmap which is written only by
>>> the main thread (holding port_mutex), but which is read concurrently by
>>> many threads (most notably the pmd threads).
>>> 
>>> When removing ports from the datapath we should postpone the deletion,
>>> otherwise another thread might access invalid memory while reading the
>>> cmap.
>>> 
>>> This commit splits do_port_del() in do_port_remove() and
>>> do_port_destroy(): the former removes the port from the cmap, while the
>>> latter reclaims the memory and drops the reference to the underlying
>>> netdev.
>> 
>> s/del_port/port_del/ here:
> 
> Thanks, changed
> 
>> 
>>> dpif_netdev_del_port() now uses ovsrcu_synchronize() before calling
>>> do_port_destroy(), to avoid memory corruption in concurrent readers.
>> 
>> ovsrcu_synchronize() requires that nothing in the thread that calls it
>> is relying on RCU to keep objects around.  That means that no caller of
>> dfpi_port_del()--there are a few of them--can rely on it.  This is
>> usually a risky assumption, especially because this assumption can
>> change later.  Is there reason to believe that it isn't important in all
>> of these cases?
> 
> I agree that's risky, but I think it's the only way to keep the ports RCU
> protected, because a port needs to be effectively deleted before
> dpif_netdev_port_del() can return.
> 

If this is because otherwise a following port_add can fail, as the old port is 
still around, maybe we could make the highest possible level of port_add detect 
the failure and then rcu_synchronize and try again? Would that work?

  Jarno

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

Re: [ovs-dev] [PATCH 1/1] ovsdb-idl: Retain column values of deleted rows until change-track is cleared.

[Ryan Moats]

"Ansari, Shad"  wrote on 03/31/2016 06:55:52 PM:

> From: "Ansari, Shad" 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: "dev@openvswitch.org" 
> Date: 03/31/2016 06:55 PM
> Subject: RE: [ovs-dev] [PATCH 1/1] ovsdb-idl: Retain column values
> of deleted rows until change-track is cleared.
>
> > > My experience with trying to clear the change track and re-establish
it
> > > leads
> > > to history being lost [this is likely from my not fully understanding
what
> > > happens with sequence numbers over turning tracking on and off] and
so it
> > > is
> > > more efficient to just leave change tracking on.  Under this
scenario, I'm
> > > pretty convinced that this code would amount to a memory leak.
> > >
> >
> > Could you provide more exact description of the problem and the
> steps/code to
> > reproduce.
>
> I ran valgrind on idl test cases with this change and they all came out
clean:
> make check-valgrind TESTSUITEFLAGS='-k idl'
> find . -name "valgrind.*" | xargs cat
>

Apply this patch set:


diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index bcad318..f9bcdc0 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -204,7 +204,7 @@ add_logical_flows(struct controller_ctx *ctx, const
struct lport_index *lports,
 uint32_t conj_id_ofs = 1;

 const struct sbrec_logical_flow *lflow;
-SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
+SBREC_LOGICAL_FLOW_FOR_EACH_TRACKED (lflow, ctx->ovnsb_idl) {
 /* Determine translation of logical table IDs to physical table
IDs. */
 bool ingress = !strcmp(lflow->pipeline, "ingress");

@@ -391,7 +391,7 @@ add_neighbor_flows(struct controller_ctx *ctx,
 ofpbuf_init(, 0);

 const struct sbrec_mac_binding *b;
-SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) {
+SBREC_MAC_BINDING_FOR_EACH_TRACKED (b, ctx->ovnsb_idl) {
 const struct sbrec_port_binding *pb
 = lport_lookup_by_name(lports, b->logical_port);
 if (!pb) {
diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index 6027011..d0a5cbe 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -285,6 +285,7 @@ main(int argc, char *argv[])
 char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
 struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
 ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
+ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
 ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);

 int probe_interval = 0;
@@ -379,6 +380,7 @@ main(int argc, char *argv[])
 }
 ovsdb_idl_loop_commit_and_wait(_idl_loop);
 ovsdb_idl_loop_commit_and_wait(_idl_loop);
+ovsdb_idl_track_clear(ovnsb_idl_loop.idl);
 poll_block();
 if (should_service_stop()) {
 exiting = true;


Run make check TESTSUITEFLAGS='-k ovn'

It looks like the calls to SBREC_*_EACH_TRACKED only show you the
incremental changes since tracking was turned on.  Now, that may
in fact be how it was intended to work, but if so, then I question
it's usefulness.  In ovn-controller, I can foresee code paths that
differ only in that one uses SBREC_*_FOR_EACH_TRACKED and the other
uses SBREC_*_FOR_EACH, and that's going to be a bit ugly.

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

Re: [ovs-dev] [PATCH] ovn-northd: Fix peering of routers.

[Guru Shetty]

On 30 March 2016 at 16:18, Ben Pfaff  wrote:

> On Mon, Mar 28, 2016 at 02:31:41PM -0700, Gurucharan Shetty wrote:
> > 1. Currently, the ovn-nb man page says that the 'peer'
> > in a logical_router_port table should point to the name
> > of the peer's logical router port. But the schema had declared
> > this column as a uuid. This looks not to be the intention as peers
> > for logical switches connected to routers is a name (and not a uuid).
> > So this patch changes the schema to be name.
> >
> > 2. In the southbound database, in the port_binding table, for a
> > logical_router_port, the peer was pointing back to itself. This
> > was causing ovn-controller to create patch ports where the peer
> > was wrongly pointing back to the source itself. This clearly looks
> > to be an error. So this patch fixes the peer in southbound database
> > to correclty point to the real peer.
> >
> > 3. ovn-northd.c currently skips generating logical flows to transfer
> > packets between two peers with comment about needing 'ARP for
> > neighboring routers'. It looked to me that since the router peer
> > is a logical object that has to be created in OVN-NB database, we
> > always need to statically assign the mac address. So this patch
> > picks the mac address from the database.
> >
> > Signed-off-by: Gurucharan Shetty 
>
> Thank you.  The lack of this feature was due to my laziness.
>
> I suspect that you should update ovn-northd.8.xml also.
>

True. I will add the following incremental
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 7954e22..a89b88e 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -746,6 +746,15 @@ icmp4 {
   A has actions eth.dst = E;
   next;.
 
+
+
+  For each logical router port with an IP address A and
+  a mac address of E that is reachable via a different
+  logical router port P, a priority-100 flow with
+  match outport === P  reg0 ==
+  A has actions eth.dst = E;
+  next;.
+
   

   

>
> Acked-by: Ben Pfaff 
>

Thanks, I will apply this in a while.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order

[Russell Bryant]

On Friday, April 1, 2016, Ben Pfaff  wrote:

> On Fri, Apr 01, 2016 at 09:55:35AM -0600, Ryan Moats wrote:
> > "dev" > wrote on 04/01/2016
> 10:36:57 AM:
> >
> > > From: Numan Siddique >
> > > To: Russell Bryant >
> > > Cc: ovs dev >
> > > Date: 04/01/2016 10:37 AM
> > > Subject: Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order
> > > Sent by: "dev" >
> > >
> > > Its the same for me.
> > >
> > >
> > >
> > > On Fri, Apr 1, 2016 at 9:03 PM, Russell Bryant  > wrote:
> > >
> > > > This patch made it to patchwork, but not my inbox.  Ryan pinged me
> > about it
> > > > on IRC.  I just applied it to master.
> > > >
> > > > https://patchwork.ozlabs.org/patch/604889/
> > > >
> > > > --
> > > > Russell Bryant
> >
> > Just to defend myself ...
> >
> > http://openvswitch.org/pipermail/dev/2016-April/069022.html
> >
> > So it did make it to the mailing list
>
> patchwork gets patches via the mailing list so there's no question of
> that.  It doesn't mean that every subscriber gets the patches since
> there can be email problems especially anti-spam type things.
>
>
Right. I was just explaining why It wasn't a proper reply.

Russell


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

Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order

[Ben Pfaff]

On Fri, Apr 01, 2016 at 09:55:35AM -0600, Ryan Moats wrote:
> "dev"  wrote on 04/01/2016 10:36:57 AM:
> 
> > From: Numan Siddique 
> > To: Russell Bryant 
> > Cc: ovs dev 
> > Date: 04/01/2016 10:37 AM
> > Subject: Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order
> > Sent by: "dev" 
> >
> > Its the same for me.
> >
> >
> >
> > On Fri, Apr 1, 2016 at 9:03 PM, Russell Bryant  wrote:
> >
> > > This patch made it to patchwork, but not my inbox.  Ryan pinged me
> about it
> > > on IRC.  I just applied it to master.
> > >
> > > https://patchwork.ozlabs.org/patch/604889/
> > >
> > > --
> > > Russell Bryant
> 
> Just to defend myself ...
> 
> http://openvswitch.org/pipermail/dev/2016-April/069022.html
> 
> So it did make it to the mailing list

patchwork gets patches via the mailing list so there's no question of
that.  It doesn't mean that every subscriber gets the patches since
there can be email problems especially anti-spam type things.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] debian : upstream_version fix - resubmitted

[Ben Pfaff]

Will you submit a revised version of the patch, then?

Thanks,

Ben.

On Fri, Apr 01, 2016 at 07:43:09AM +, Zoltán Balogh wrote:
> Hi Simon,
> 
> This is a simpler and better solution. For me it's ok, since our team uses 
> debian_revision too.
> 
> Best regards,
> Zoltán
> 
> -Original Message-
> From: Simon Horman [mailto:simon.hor...@netronome.com]
> Sent: Tuesday, March 29, 2016 2:51 AM
> To: Zoltán Balogh
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] debian : upstream_version fix - resubmitted
> 
> Hi Zoltánm
> 
> On Thu, Mar 24, 2016 at 08:28:53AM +, Zoltán Balogh wrote:
> > Hi,
> > 
> > The Debian Policy Manual 
> > (https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version)
> >  says that the upstream_version may contain only alphanumerics and the 
> > characters . + - : ~ (full stop, plus, hyphen, colon, tilde) and should 
> > start with a digit.
> > 
> > Currently, the upstream_version is defined in the debian/rules file:
> > 
> > DEB_UPSTREAM_VERSION=$(shell dpkg-parsechangelog | sed -rne
> > 's,^Version: ([0-9]:)*([^-]+).*,\2,p')
> > 
> > The version number is taken from the dpkg-parsechangelog printout then the 
> > first part of the version number which does not contain hyphen is filtered 
> > out with sed. However the Debian Policy Manual says that hyphen is allowed 
> > in the upstream_version. 
> > 
> > This is not a problem with current vanilla OVS debian version. But, if a 
> > postfix string including a hyphen is added to the upstream_version then 
> > installation of datapath-dkms package will fail. 
> > 
> > I think the following patch solves this problem.
> > 
> > Signed-off-by: Zoltán Balogh 
> 
> I wonder if the version manipulation could be expressed using sed, as the 
> code existing code does, rather than awk, sed, expr and shell.
> 
> Perhaps something like this:
> 
> sed -rne 's,^Version: 
> ([0-9]+:)?([0-9][a-zA-Z0-9.+:~-]*)(-[a-zA-Z0-9*.~]*),\2,p'
> 
> Which I tested as follows:
> 
> Input: Version: 2.4.90-1
> Output: 2.4.90
> 
> Input: Version: 1:2.4.90-1
> Output: 2.4.90
> 
> Input: Version: 1:3:2.4.90-1
> Output: 3:2.4.90
> 
> Input: Version: 2.4.90-xyz-1
> Output: 2.4.90-xyz
> 
> Input: Version: 1:2.4.90-xyz-1
> Output: 2.4.90-xyz
> 
> Input: Version: 1:3:2.4.90-xyz-1
> Output: 3:2.4.90-xyz
> 
> N.B: Does not work without debian_version present
> Input: Version: 2.4.90
> Output: 
> 
> > 
> > ---
> > 
> > diff --git a/debian/rules b/debian/rules index d8e90c7..70539ab 100755
> > --- a/debian/rules
> > +++ b/debian/rules
> > @@ -13,7 +13,9 @@
> > 
> >  PACKAGE=openvswitch
> >  PACKAGE_DKMS=openvswitch-datapath-dkms
> > -DEB_UPSTREAM_VERSION=$(shell dpkg-parsechangelog | sed -rne
> > 's,^Version: ([0-9]:)*([^-]+).*,\2,p')
> > +DEB_VERSION=$(shell dpkg-parsechangelog | awk '/^Version: / {print 
> > +$$2}' | sed -rne 's,([0-9]:)+([.])*,\2,p') DEB_REVISION=$(shell expr 
> > +"$(DEB_VERSION)" : '.*\(-.*\)' ) DEB_UPSTREAM_VERSION=$(shell 
> > +version=$(DEB_VERSION); expr + $${version%"$(DEB_REVISION)"})
> > 
> >  ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))  PARALLEL = 
> > -j$(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
> > 
> > 
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> ___
> 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] Fix bug in ovn-ctl argument order

[Ryan Moats]

"dev"  wrote on 04/01/2016 10:36:57 AM:

> From: Numan Siddique 
> To: Russell Bryant 
> Cc: ovs dev 
> Date: 04/01/2016 10:37 AM
> Subject: Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order
> Sent by: "dev" 
>
> Its the same for me.
>
>
>
> On Fri, Apr 1, 2016 at 9:03 PM, Russell Bryant  wrote:
>
> > This patch made it to patchwork, but not my inbox.  Ryan pinged me
about it
> > on IRC.  I just applied it to master.
> >
> > https://patchwork.ozlabs.org/patch/604889/
> >
> > --
> > Russell Bryant

Just to defend myself ...

http://openvswitch.org/pipermail/dev/2016-April/069022.html

So it did make it to the mailing list

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

Re: [ovs-dev] [PATCH] Configure or disable ovn-controller probe timer on the fly

[Ben Pfaff]

On Thu, Mar 31, 2016 at 03:00:59PM -0700, ngh...@us.ibm.com wrote:
> Configure or disable ovn-controller probe_timer on the fly.
> 
> There are four sessions established from ovn-controller to the following:
> OVN Southbound — jsonrpc based
> Local vswitchd — jsonrpc based
> Local vswitchd — openflow based from ofctrl
> Local vswitchd — openflow based from pinctrl
> 
> All of these sessions have their own probe_interval, and currently only 
> one
> [SB] of them can be configured using ovn-vsctl command, and even that is 
> not
> effective on the fly —in other words, ovn-controller has to be restarted 
> to
> use that probe_timer value.

There's no value in configuring the other ones.  First, they're all
connections to local processes so there's little or no cost.  Second,
there's probes are unnecessary for Unix domain sockets, because the
kernel will notify us immediately if the connection is dropped.  JSONRPC
sessions automatically disable probing for Unix domain sockets (see
jsonrpc_session_open()).  I thought that our OpenFlow connections did
the same thing, but the code isn't there.  Probably, a better patch to
submit would be one to do that.

Thanks,

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

Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order

[Numan Siddique]

Its the same for me.



On Fri, Apr 1, 2016 at 9:03 PM, Russell Bryant  wrote:

> This patch made it to patchwork, but not my inbox.  Ryan pinged me about it
> on IRC.  I just applied it to master.
>
> https://patchwork.ozlabs.org/patch/604889/
>
> --
> Russell Bryant
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V6] Add Passive TCP connection to IDL.

[Ben Pfaff]

On Mon, Mar 28, 2016 at 06:25:51AM +, Ofer Ben-Yacov wrote:
> I understand that you met Ayal for a short talk during ONS.
> From what he told me I think there might be some misunderstanding so I
> would like to make it clear what the intent here is.
> First, in case I did not explain it clearly before, I do *not* mean that
> the right sequence is "connect -> disconnect -> connect".
> I completely agree with you that we should connect only once and the code
> does exactly that.
> In my opinion, the only point that we need to agree upon is how to get the
> schema.
> What the patch does right now is open the connection once, retrieve the
> schema and continue to work using the same connection.
> 
> I think the point of disagreement here is about whether we should maintain
> a local copy of the schema or not.
> Since we want to retrieve the *entire* schema, we do not need to keep a
> copy of it locally. To validate that the schema hasn't changed, we just
> need to verify that the schema version hasn't changed.
> 
> Does this make sense?

Can you please separate the actual support for passive TCP from the rest
of this?  You've conflated two issues.  Submit a patch that supports
passive TCP, then one that adds the rest of what you want.  Then we can
have a sensible discussion.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Fix bug in ovn-ctl argument order

[Russell Bryant]

This patch made it to patchwork, but not my inbox.  Ryan pinged me about it
on IRC.  I just applied it to master.

https://patchwork.ozlabs.org/patch/604889/

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

[ovs-dev] [PATCH v11 7/8] netdev-dpdk: Check dpdk-extra when reading db

[Aaron Conole]

A previous patch introduced the ability to pass arbitrary EAL command
line options via the dpdk_extras database entry. This commit enhances
that by warning the user when such a configuration is detected and
prefering the value in the database.

Suggested-by: Sean K Mooney 
Signed-off-by: Aaron Conole 
Tested-by: Sean K Mooney 
Tested-by: Kevin Traynor 
Acked-by: Panu Matilainen 
Acked-by: Flavio Leitner 
---
v9:
* Added as suggested by Sean K Mooney

v10:
* no change

v11:
* Minor cleanups as suggested by Daniele

 lib/netdev-dpdk.c | 66 +--
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index dc421c6..69abbdf 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2779,6 +2779,17 @@ dpdk_option_extend(char ***argv, int argc, const char 
*option,
 newargv[argc+1] = xstrdup(value);
 }
 
+static char **
+move_argv(char ***argv, size_t cur_size, char **src_argv, size_t src_argc)
+{
+char **newargv = grow_argv(argv, cur_size, src_argc);
+while (src_argc--) {
+newargv[cur_size+src_argc] = src_argv[src_argc];
+src_argv[src_argc] = 0;
+}
+return newargv;
+}
+
 static int
 extra_dpdk_args(const char *ovs_extra_config, char ***argv, int argc)
 {
@@ -2796,9 +2807,21 @@ extra_dpdk_args(const char *ovs_extra_config, char 
***argv, int argc)
 return ret;
 }
 
+static bool
+argv_contains(char **argv_haystack, const size_t argc_haystack,
+  const char *needle)
+{
+for (size_t i = 0; i < argc_haystack; ++i) {
+if (!strcmp(argv_haystack[i], needle))
+return true;
+}
+return false;
+}
+
 static int
 construct_dpdk_options(const struct ovsrec_open_vswitch *ovs_cfg,
-   char ***argv, const int initial_size)
+   char ***argv, const int initial_size,
+   char **extra_args, const size_t extra_argc)
 {
 struct dpdk_options_map {
 const char *ovs_configuration;
@@ -2821,8 +2844,13 @@ construct_dpdk_options(const struct ovsrec_open_vswitch 
*ovs_cfg,
 }
 
 if (lookup) {
-dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
-ret += 2;
+if (!argv_contains(extra_args, extra_argc, opts[i].dpdk_option)) {
+dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
+ret += 2;
+} else {
+VLOG_WARN("Ignoring database defined option '%s' due to "
+  "dpdk_extras config", opts[i].dpdk_option);
+}
 }
 }
 
@@ -2833,7 +2861,8 @@ construct_dpdk_options(const struct ovsrec_open_vswitch 
*ovs_cfg,
 
 static int
 construct_dpdk_mutex_options(const struct ovsrec_open_vswitch *ovs_cfg,
- char ***argv, const int initial_size)
+ char ***argv, const int initial_size,
+ char **extra_args, const size_t extra_argc)
 {
 struct dpdk_exclusive_options_map {
 const char *category;
@@ -2881,9 +2910,15 @@ construct_dpdk_mutex_options(const struct 
ovsrec_open_vswitch *ovs_cfg,
  popt->category);
 }
 
-dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
-   found_value);
-ret += 2;
+if (!argv_contains(extra_args, extra_argc,
+   popt->eal_dpdk_options[found_pos])) {
+dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
+   found_value);
+ret += 2;
+} else {
+VLOG_WARN("Ignoring database defined option '%s' due to "
+  "dpdk_extras config", popt->eal_dpdk_options[found_pos]);
+}
 }
 
 return ret;
@@ -2894,14 +2929,23 @@ get_dpdk_args(const struct ovsrec_open_vswitch 
*ovs_cfg, char ***argv,
   int argc)
 {
 const char *extra_configuration;
-int i = construct_dpdk_options(ovs_cfg, argv, argc);
-i = construct_dpdk_mutex_options(ovs_cfg, argv, i);
+char **extra_args = NULL;
+int i;
+size_t extra_argc = 0;
 
 extra_configuration = smap_get(_cfg->other_config, "dpdk-extra");
 if (extra_configuration) {
-i = extra_dpdk_args(extra_configuration, argv, i);
+extra_argc = extra_dpdk_args(extra_configuration, _args, 0);
 }
-return i;
+
+i = construct_dpdk_options(ovs_cfg, argv, argc, extra_args, extra_argc);
+i = construct_dpdk_mutex_options(ovs_cfg, argv, i, extra_args, extra_argc);
+
+if (extra_configuration) {
+*argv = move_argv(argv, i, extra_args, extra_argc);
+}
+
+return i + extra_argc;
 }
 
 static char **dpdk_argv;
-- 
2.5.5

___
dev mailing list

[ovs-dev] [PATCH v11 8/8] NEWS: Announce the DPDK EAL configuration change

[Aaron Conole]

Previous commits have converted dpdk EAL initialization from
requiring a ``--dpdk ... --`` command line arguments to using the Open
vSwitch database. This change announces that as significant NEWS.

Signed-off-by: Aaron Conole 
Tested-by: Sean K Mooney 
Tested-by: RobertX Wojciechowicz 
Acked-by: Panu Matilainen 
Acked-by: Kevin Traynor 
Acked-by: Flavio Leitner 
---
Typically this is included in the patch which introduces the NEWS-worthy
change, but in this case, I thought it was more appropriate to keep it
isolated. This patch has not significantly changed since it was
introduced (in v7 on Jan 27th)

 NEWS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/NEWS b/NEWS
index ea7f3a1..861dec1 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,9 @@ Post-v2.5.0
assignment.
  * Type of log messages from PMD threads changed from INFO to DBG.
  * QoS functionality with sample egress-policer implementation.
+ * The mechanism for configuring DPDK has changed from cli based, to
+   using the database.
+ * Some sensible defaults have been introduced for ease of use.
- ovs-benchmark: This utility has been removed due to lack of use and
  bitrot.
- ovs-appctl:
-- 
2.5.5

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

[ovs-dev] [PATCH v11 6/8] netdev-dpdk: Allow arbitrary eal arguments

[Aaron Conole]

A previous change moved some commonly used arguments from commandline to
the database, and with it the ability to pass arbitrary arguments to
EAL. This change allows arbitrary eal arguments to be provided
via a new db entry 'other_config:dpdk-extra' which will tokenize the
string and add it to the argument list. The only argument which will not
be supported with this change is '--no-huge', which appears to break the
system in other ways.

Signed-off-by: Aaron Conole 
Tested-by: Sean K Mooney 
Tested-by: RobertX Wojciechowicz 
Tested-by: Kevin Traynor 
Acked-by: Panu Matilainen 
Acked-by: Kevin Traynor 
Acked-by: Flavio Leitner 
---
v4:
* Added by suggestion of Panu, making socket-mem non-default

v5:
* Keep the socket-mem as default parameter, and mention that we
  do not support --no-huge
* Update ovs-dev.py with the new mechanism for passing arbitrary dpdk
  options

v6->v9:
* No change

v10:
* INSTALL.DPDK.md - removed the note since a future commit in the series makes
  that documentation invalid (and it seems silly to add it here, only to remove
  in in the next commit)

v11:
* Restructured the arguments to a single line (using dynamic-string)
* Moved argument print to info level

 INSTALL.DPDK.md  |  5 +
 lib/netdev-dpdk.c| 39 ++-
 utilities/ovs-dev.py |  7 +--
 vswitchd/vswitch.xml | 11 +++
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 73fc876..f37205f 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -178,6 +178,11 @@ Using the DPDK with ovs-vswitchd:
* dpdk-hugepage-dir
Directory where hugetlbfs is mounted
 
+   * dpdk-extra
+   Extra arguments to provide to DPDK EAL, as previously specified on the
+   command line. Do not pass '--no-huge' to the system in this way. Support
+   for running the system without hugepages is nonexistent.
+
* cuse-dev-name
Option to set the vhost_cuse character device name.
 
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index af5f765..dc421c6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -51,6 +51,7 @@
 #include "unaligned.h"
 #include "timeval.h"
 #include "unixctl.h"
+#include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
 #include "vswitch-idl.h"
 
@@ -2779,6 +2780,23 @@ dpdk_option_extend(char ***argv, int argc, const char 
*option,
 }
 
 static int
+extra_dpdk_args(const char *ovs_extra_config, char ***argv, int argc)
+{
+int ret = argc;
+char *release_tok = xstrdup(ovs_extra_config);
+char *tok = release_tok, *endptr = NULL;
+
+for (tok = strtok_r(release_tok, " ", ); tok != NULL;
+ tok = strtok_r(NULL, " ", )) {
+char **newarg = grow_argv(argv, ret, 1);
+*argv = newarg;
+newarg[ret++] = xstrdup(tok);
+}
+free(release_tok);
+return ret;
+}
+
+static int
 construct_dpdk_options(const struct ovsrec_open_vswitch *ovs_cfg,
char ***argv, const int initial_size)
 {
@@ -2875,8 +2893,14 @@ static int
 get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv,
   int argc)
 {
+const char *extra_configuration;
 int i = construct_dpdk_options(ovs_cfg, argv, argc);
 i = construct_dpdk_mutex_options(ovs_cfg, argv, i);
+
+extra_configuration = smap_get(_cfg->other_config, "dpdk-extra");
+if (extra_configuration) {
+i = extra_dpdk_args(extra_configuration, argv, i);
+}
 return i;
 }
 
@@ -2899,7 +2923,7 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
 {
 char **argv = NULL;
 int result;
-int argc, argc_tmp;
+int argc = 0, argc_tmp;
 bool auto_determine = true;
 int err;
 cpu_set_t cpuset;
@@ -2976,6 +3000,19 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
 
 optind = 1;
 
+if (VLOG_IS_INFO_ENABLED()) {
+struct ds eal_args;
+int opt;
+ds_init(_args);
+ds_put_cstr(_args, "EAL ARGS:");
+for (opt = 0; opt < argc; ++opt) {
+ds_put_cstr(_args, " ");
+ds_put_cstr(_args, argv[opt]);
+}
+VLOG_INFO("%s", ds_cstr_ro(_args));
+ds_destroy(_args);
+}
+
 /* Make sure things are initialized ... */
 result = rte_eal_init(argc, argv);
 if (result < 0) {
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index a74b528..524f574 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -267,6 +267,8 @@ def run():
 if options.dpdk:
 _sh("ovs-vsctl --no-wait set Open_vSwitch %s " \
 "other_config:dpdk-init=true" % root_uuid)
+_sh("ovs-vsctl --no-wait set Open_vSwitch %s other_config:" \
+"dpdk-extra=\"%s\"" % (root_uuid, ' '.join(options.dpdk)))
 else:
 _sh("ovs-vsctl --no-wait set Open_vSwitch %s " 

[ovs-dev] [PATCH v11 4/8] netdev-dpdk: Restrict vhost_sock_dir

[Aaron Conole]

Since the vhost-user sockets directory now comes from the database, it is
possible for any user with database access to program an arbitrary filesystem
location for the sockets directory. This could result in unprivileged users
creating or deleting arbitrary filesystem files by using specially crafted
names. To prevent this, use the introduced ovs_realpath function to resolve
the filesystem location and ensure that it resolves under the ovs_rundir.

Signed-off-by: Aaron Conole 
---
v11 (correct):
* Added

 INSTALL.DPDK.md  |  3 ++-
 lib/netdev-dpdk.c| 30 ++
 vswitchd/vswitch.xml |  3 ++-
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 8397688..73fc876 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -182,7 +182,8 @@ Using the DPDK with ovs-vswitchd:
Option to set the vhost_cuse character device name.
 
* vhost-sock-dir
-   Option to set the path to the vhost_user unix socket files.
+   Option to set the path to the vhost_user unix socket files. This path must
+   exist and must be a sub-tree of the Open vSwitch running directory.
 
NOTE: Changing any of these options requires restarting the ovs-vswitchd
application.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 152b66f..397a3fa 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -812,6 +812,12 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev_)
 const char *name = netdev_->name;
 int err;
 
+if (!vhost_sock_dir) {
+VLOG_ERR("Unable to setup a valid path to vhost-user sockets; "
+ "Check the 'vhost-sock-dir' option.");
+return EINVAL;
+}
+
 /* 'name' is appended to 'vhost_sock_dir' and used to create a socket in
  * the file system. '/' or '\' would traverse directories, so they're not
  * acceptable in 'name'. */
@@ -2896,6 +2902,9 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
 int argc;
 int err;
 cpu_set_t cpuset;
+#ifndef VHOST_CUSE
+char *sock_dir_subcomponent;
+#endif
 
 if (!smap_get_bool(_cfg->other_config, "dpdk-init", false)) {
 VLOG_INFO("DPDK Disabled - to change this requires a restart.\n");
@@ -2908,15 +2917,20 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
 if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
 PATH_MAX, ovs_cfg, _dev_name)) {
 #else
-if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
-NAME_MAX, ovs_cfg, _sock_dir)) {
-struct stat s;
-
-err = stat(vhost_sock_dir, );
-if (err) {
-VLOG_ERR("vhost-user sock directory '%s' does not exist.",
- vhost_sock_dir);
+if (process_vhost_flags("vhost-sock-dir", xstrdup(""),
+NAME_MAX, ovs_cfg, _dir_subcomponent)) {
+char *full_sockdir = xasprintf("%s/%s", ovs_rundir(),
+   sock_dir_subcomponent);
+vhost_sock_dir = ovs_realpath(full_sockdir);
+if (!vhost_sock_dir
+|| strncmp(vhost_sock_dir, ovs_rundir(), strlen(ovs_rundir( {
+VLOG_ERR("Invalid vhost-user socket directory '%s/%s'.",
+ ovs_rundir(), sock_dir_subcomponent);
+free(vhost_sock_dir);
+vhost_sock_dir = 0;
 }
+free(full_sockdir);
+free(sock_dir_subcomponent);
 #endif
 }
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index c350247..2a5f8fa 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -311,7 +311,8 @@
   
 
-  Specifies the path to the vhost-user unix domain socket files.
+  Specifies the path to the vhost-user unix domain socket files. This
+  path must exist and be located within the working directory subtree.
 
 
   Defaults to the working directory of the application. Changing this
-- 
2.5.5

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

[ovs-dev] [PATCH v11 1/8] netdev-dpdk: Restore thread affinity after DPDK init

[Aaron Conole]

When the DPDK init function is called, it changes the executing thread's
CPU affinity to a single core specified in -c. This will result in the
userspace bridge configuration thread being rebound, even if that is not
the intent.

This change fixes that behavior by rebinding to the original thread
affinity after calling dpdk_init().

Co-authored-by: Kevin Traynor 
Signed-off-by: Kevin Traynor 
Signed-off-by: Aaron Conole 
Tested-by: RobertX Wojciechowicz 
Tested-by: Sean K Mooney 
Acked-by: Panu Matilainen 
Acked-by: Flavio Leitner 
---
v2:
* Removed trailing whitespace

v3->v10:
* No change

v11:
* Added missing Co-authored-by tag

 lib/netdev-dpdk.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..3b954e5 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2762,6 +2762,9 @@ dpdk_init(int argc, char **argv)
 int result;
 int base = 0;
 char *pragram_name = argv[0];
+int err;
+int isset;
+cpu_set_t cpuset;
 
 if (argc < 2 || strcmp(argv[1], "--dpdk"))
 return 0;
@@ -2803,6 +2806,14 @@ dpdk_init(int argc, char **argv)
 base = 2;
 }
 
+/* Get the main thread affinity */
+CPU_ZERO();
+err = pthread_getaffinity_np(pthread_self(), sizeof(cpu_set_t), );
+if (err) {
+VLOG_ERR("Thread getaffinity error %d.", err);
+return err;
+}
+
 /* Keep the program name argument as this is needed for call to
  * rte_eal_init()
  */
@@ -2814,6 +2825,13 @@ dpdk_init(int argc, char **argv)
 ovs_abort(result, "Cannot init EAL");
 }
 
+/* Set the main thread affinity back to pre rte_eal_init() value */
+err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), );
+if (err) {
+VLOG_ERR("Thread setaffinity error %d", err);
+return err;
+}
+
 rte_memzone_dump(stdout);
 rte_eal_init_ret = 0;
 
-- 
2.5.5

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

[ovs-dev] [PATCH v11 5/8] netdev-dpdk: Autofill lcore coremask if absent

[Aaron Conole]

The user has control over the DPDK internal lcore coremask, but this
parameter can be autofilled with a bit more intelligence. If the user
does not fill this parameter in, we use the lowest set bit in the
current task CPU affinity. Otherwise, we will reassign the current
thread to the specified lcore mask, in addition to the dpdk lcore
threads.

Signed-off-by: Aaron Conole 
Tested-by: Sean K Mooney 
Tested-by: RobertX Wojciechowicz 
Tested-by: Kevin Traynor 
Acked-by: Panu Matilainen 
Acked-by: Kevin Traynor 
Acked-by: Flavio Leitner 
---
v2:
* Fix a conditional branch coding standard issue
* When lcore coremask is set, do not reset the affinities as
  suggested by Kevin Traynor

v3:
* Fixed grow_argv calls
* Fixed an error in argc assignment after 'break;' introduced
  in v2
* Switched to using xstrdup

v4->v7:
* No change

v8:
* Assign the lcore only when resetting the affinity.

v9,v10:
* No change

v11:
* Minor refactors due to previous changes
* Cleanups suggested by Daniele

 lib/netdev-dpdk.c | 40 +---
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 397a3fa..af5f765 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2792,7 +2792,6 @@ construct_dpdk_options(const struct ovsrec_open_vswitch 
*ovs_cfg,
 {"dpdk-mem-channels", "-n", false, NULL},
 {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
 };
-
 int i, ret = initial_size;
 
 /*First, construct from the flat-options (non-mutex)*/
@@ -2873,9 +2872,10 @@ construct_dpdk_mutex_options(const struct 
ovsrec_open_vswitch *ovs_cfg,
 }
 
 static int
-get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv)
+get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv,
+  int argc)
 {
-int i = construct_dpdk_options(ovs_cfg, argv, 1);
+int i = construct_dpdk_options(ovs_cfg, argv, argc);
 i = construct_dpdk_mutex_options(ovs_cfg, argv, i);
 return i;
 }
@@ -2899,7 +2899,8 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
 {
 char **argv = NULL;
 int result;
-int argc;
+int argc, argc_tmp;
+bool auto_determine = true;
 int err;
 cpu_set_t cpuset;
 #ifndef VHOST_CUSE
@@ -2941,9 +2942,34 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
 VLOG_ERR("Thread getaffinity error %d.", err);
 }
 
-argv = grow_argv(, 0, 1);
+argv = grow_argv(, argc, 1);
 argv[0] = xstrdup(ovs_get_program_name());
-argc = get_dpdk_args(ovs_cfg, );
+argc_tmp = get_dpdk_args(ovs_cfg, , argc);
+
+while (argc_tmp != argc) {
+if (!strcmp("-c", argv[argc]) || !strcmp("-l", argv[argc])) {
+auto_determine = false;
+break;
+}
+argc++;
+}
+argc = argc_tmp;
+
+/**
+ * NOTE: This is an unsophisticated mechanism for determining the DPDK
+ * lcore for the DPDK Master.
+ */
+if (auto_determine) {
+int i;
+for (i = 0; i < CPU_SETSIZE; i++) {
+if (CPU_ISSET(i, )) {
+argv = grow_argv(, argc, 2);
+argv[argc++] = xstrdup("-c");
+argv[argc++] = xasprintf("0x%08llX", (1ULL<

[ovs-dev] [PATCH v11 3/8] netdev-dpdk: Convert initialization from cmdline to db

[Aaron Conole]

Existing DPDK integration is provided by use of command line options which
must be split out and passed to librte in a special manner. However, this
forces any configuration to be passed by way of a special DPDK flag, and
interferes with ovs+dpdk packaging solutions.

This commit delays dpdk initialization until after the OVS database
connection is established, at which point ovs initializes librte. It
pulls all of the config data from the OVS database, and assembles a
new argv/argc pair to be passed along.

Signed-off-by: Aaron Conole 
---
v2:
* Removed trailing whitespace
* Followed for() loop brace coding style
* Automatically enable DPDK when adding a DPDK enabled port
* Fixed an issue on startup when DPDK enabled ports are present
* Updated the documentation (including vswitch.xml) and documented all
  new parameters
* Dropped the premature initialization test

v3:
* Improved description language in INSTALL.DPDK.md
* Fixed the ovs-vsctl examples for DPDK
* Returned to the global dpdk-init (bullet 3 from v2)
* Fixed a build error when compiling without dpdk support enabled
* converted to xstrdup, for consistency after rebasing

v4:
* No change

v5:
* Adjust the ovs-dev script to account for the new dpdk configuration
* Update the ovs-vswitchd.8.in pointing to INSTALL.DPDK.md

v6:
* Remove excess whitespace addition
* Correct INSTALL.DPDK.md regarding when DPDK is initialized
* Used incorrect variable in the non-dpdk case for testing against
  dpdk

v7:
* Account for mutually exclusive options;

v8:
* ``make check`` testing revealed a number of flaws in the initialization
  which resulted in memory corruption
* Fixed the ovs-vswitchd startup during testing

v9:
* Re-arrange the added headers in netdev-dpdk.c to try and be alphabetical
* Convert '-c' and '-n' options to be default non-inserted

v10:
* Documentation adjustment in vswitch.xml explicitly stating these values
  are not runtime configurable.
* Wrapped vhost_cuse_dev in #ifdef

v11:
* Whitespace cleanup
* Numerous error path cleanups
* Removed ACKs and Tested-bys due to the error path changes

 FAQ.md |   6 +-
 INSTALL.DPDK.md|  81 +---
 lib/automake.mk|   4 +
 lib/netdev-dpdk.c  | 301 +
 lib/netdev-dpdk.h  |  13 +-
 lib/netdev-nodpdk.c|  21 
 tests/ofproto-macros.at|   3 +-
 utilities/ovs-dev.py   |  13 +-
 vswitchd/bridge.c  |   3 +
 vswitchd/ovs-vswitchd.8.in |   6 +-
 vswitchd/ovs-vswitchd.c|  25 +---
 vswitchd/vswitch.xml   | 131 +++-
 12 files changed, 463 insertions(+), 144 deletions(-)
 create mode 100644 lib/netdev-nodpdk.c

diff --git a/FAQ.md b/FAQ.md
index 04ffc84..0750956 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -432,9 +432,9 @@ A: Yes.  How you configure it depends on what you mean by 
"promiscuous
 
 A: Firstly, you must have a DPDK-enabled version of Open vSwitch.
 
-   If your version is DPDK-enabled it will support the --dpdk
-   argument on the command line and will display lines with
-   "EAL:..." during startup when --dpdk is supplied.
+   If your version is DPDK-enabled it will support the other_config:dpdk-init
+   configuration in the database and will display lines with
+   "EAL:..." during startup when other_config:dpdk-init is set to 'true'.
 
Secondly, when adding a DPDK port, unlike a system port, the
type for the interface must be specified. For example;
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 9ec8bf6..8397688 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -143,22 +143,64 @@ Using the DPDK with ovs-vswitchd:
 
 5. Start vswitchd:
 
-   DPDK configuration arguments can be passed to vswitchd via `--dpdk`
-   argument. This needs to be first argument passed to vswitchd process.
-   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
-   for dpdk initialization.
+   DPDK configuration arguments can be passed to vswitchd via Open_vSwitch
+   other_config column. The recognized configuration options are listed.
+   Defaults will be provided for all values not explicitly set.
+
+   * dpdk-init
+   Specifies whether OVS should initialize and support DPDK ports. This is
+   a boolean, and defaults to false.
+
+   * dpdk-lcore-mask
+   Specifies the CPU cores on which dpdk lcore threads should be spawned.
+   The DPDK lcore threads are used for DPDK library tasks, such as
+   library internal message processing, logging, etc. Value should be in
+   the form of a hex string (so '0x123') similar to the 'taskset' mask
+   input.
+   If not specified, the value will be determined by choosing the lowest
+   CPU core from initial cpu affinity list. Otherwise, the value will be
+   passed directly to the DPDK library.
+   For performance reasons, it is best to set this to a single core on
+   the system, rather than allow lcore threads to float.
+
+   * dpdk-mem-channels
+   This sets the number of memory spread 

[ovs-dev] [PATCH v11 2/8] util: Add a path canonicalizer

[Aaron Conole]

This commit adds a new function (ovs_realpath) to perform the role of
realpath on various operating systems. The purpose is to ensure that a
given path to file exists, and to return a completely resolved path (sans
'.' and '..').

Signed-off-by: Aaron Conole 
---
v11:
* Added

 configure.ac  |  2 +-
 lib/util.c| 52 
 lib/util.h|  1 +
 tests/library.at  |  5 +
 tests/test-util.c | 23 +++
 5 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 05d80d5..a490260 100644
--- a/configure.ac
+++ b/configure.ac
@@ -105,7 +105,7 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include 
]])
 AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec],
   [], [], [[#include ]])
 AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include ]])
-AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r])
+AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r realpath])
 AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h 
stdatomic.h])
 AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
 #include ]])
diff --git a/lib/util.c b/lib/util.c
index 94311ac..6bb6d9e 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -2098,6 +2098,58 @@ is_stdout_a_tty(void)
 return (isatty(STDOUT_FILENO) && t && strcmp(t, "dumb") != 0);
 }
 
+/* Derives, from the pathname pointed to by path, an absolute pathname that
+ * names the same file, whose resolution does not involve '.', '..' or
+ * symbolic links. If path is null or does not exist, returns NULL.
+ */
+char *
+ovs_realpath(const char *path)
+{
+if (!path) {
+return NULL;
+}
+char *realpath_res = NULL;
+
+#ifdef HAVE_REALPATH
+realpath_res = realpath(path, NULL);
+if (realpath_res == NULL) {
+goto error;
+}
+struct stat s;
+int err = stat(realpath_res, );
+if (err) {
+goto error;
+}
+return realpath_res;
+#elif defined(_WIN32)
+realpath_res = xstrdup(path);
+DWORD ret = GetFullPathName(path, strlen(realpath_res), realpath_res,
+NULL);
+if (ret == 0) {
+goto error;
+} else if (ret > strlen(realpath_res)) {
+DWORD new_ret;
+char *new_path = (char *)xrealloc(realpath_res, ret);
+if (new_path == NULL) {
+goto error;
+}
+realpath_res = new_path;
+new_ret = GetFullPathName(path, strlen(realpath_res), realpath_res,
+  NULL);
+if (new_ret == 0 || new_ret != ret ) {
+goto error;
+}
+}
+return realpath_res;
+#else
+#error Need realpath() on this platform
+#endif
+
+error:
+free(realpath_res);
+return NULL;
+}
+
 #ifdef _WIN32
 
 char *
diff --git a/lib/util.h b/lib/util.h
index 41c5ea6..8d7f335 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -220,6 +220,7 @@ char *dir_name(const char *file_name);
 char *base_name(const char *file_name);
 #endif
 char *abs_file_name(const char *dir, const char *file_name);
+char *ovs_realpath(const char *path);
 
 char *follow_symlinks(const char *filename);
 
diff --git a/tests/library.at b/tests/library.at
index e1bac92..b6561e5 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -234,3 +234,8 @@ AT_CLEANUP
 AT_SETUP([test rcu])
 AT_CHECK([ovstest test-rcu-quiesce], [0], [])
 AT_CLEANUP
+
+AT_SETUP([test ovs_realpath])
+AT_CHECK([ovstest test-util realpath \
+/tmp/../tmp/../usr/bin/.//../share:/usr/share /tmpNOEXISTS:], [0], [])
+AT_CLEANUP
diff --git a/tests/test-util.c b/tests/test-util.c
index ef45903..2755200 100644
--- a/tests/test-util.c
+++ b/tests/test-util.c
@@ -1128,6 +1128,28 @@ test_snprintf(struct ovs_cmdl_context *ctx OVS_UNUSED)
 ovs_assert(snprintf(NULL, 0, "abcde") == 5);
 }
 
+static void
+test_ovs_realpath(struct ovs_cmdl_context *ctx)
+{
+int i;
+for (i = 1; i < ctx->argc; i++) {
+char *str_toks = NULL;
+char *path = strtok_r(ctx->argv[i], ":", _toks);
+ovs_assert(path != NULL);
+
+char *check = strtok_r(NULL, ":", _toks);
+char *rpath = ovs_realpath(path);
+
+if (check) {
+ovs_assert(rpath != NULL);
+ovs_assert(!strcmp(check, rpath));
+free(rpath);
+} else {
+ovs_assert(rpath == NULL);
+}
+}
+}
+
 #ifndef _WIN32
 static void
 test_file_name(struct ovs_cmdl_context *ctx)
@@ -1164,6 +1186,7 @@ static const struct ovs_cmdl_command commands[] = {
 {"assert", NULL, 0, 0, test_assert},
 {"ovs_scan", NULL, 0, 0, test_ovs_scan},
 {"snprintf", NULL, 0, 0, test_snprintf},
+{"realpath", NULL, 1, INT_MAX, test_ovs_realpath},
 #ifndef _WIN32
 {"file_name", NULL, 1, INT_MAX, test_file_name},
 #endif
-- 
2.5.5

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

[ovs-dev] [PATCH v11 0/8] Convert DPDK configuration from command line to DB based

[Aaron Conole]

Currently, configuration of DPDK parameters is done via the command line
through a --dpdk **OPTIONS** -- command line argument. This has a number of
challenges, including:
* It must be the first option passed to ovs-vswitchd
* It is the only datapath feature in OVS to be configured on the command
  line
* It requires specialized knowledge of sub-component command switches
* It also inteprets non-EAL arguments (confusing users)
* It is a broken model for datapath configuration.

This series brings the following changes to openvswitch:
* All DPDK options are taken from the ovs database rather than the
  command line
* Non-EAL arguments also have separate database entries
* DPDK lcores are optionally auto-assigned to a single core based on the
  bridge coremask.
* DPDK options have default behaviors
* Updated documentation

This series has been build tested (including `make check`) on OSX, Fedora 23,
Windows (via appveyor), and FreeBSD 10.3; the v11 has had very basic testing
applied (start, configure some of the settings). I have removed ACKs and
Tested-bys for some of the patches since they underwent changes that I felt
disqualified continued use of the Acked-by: and Tested-by: tags.

Travis-ci build: https://travis-ci.org/orgcandman/ovs/builds/120081527
Appveyor build: https://ci.appveyor.com/project/orgcandman/ovs/build/1.0.9

This is a resend due to an accidentally omitted hunk in 4/8.

A huge round of thanks on the work so far should be given to the following
folks (in alphabetical order):
* Ben Pfaff (Reviews, vhost-sock-dir escape suggestion)
* Christian Erhardt (Testing)
* Daniele Di Proietto   (Reviews, general suggestions)
* Flavio Leitner(Original efforts, reviews)
* Kevin Traynor (Testing, general suggestions, reviews, doc reviews)
* Panu Matilainen   (Initialization ideas, eal arguments ideas, reviews)
* RobertX Wojciechowicz (Testing, general suggestions)
* Sean Mooney   (Testing, general suggestions)

v2:
* Dropped the vhost-user socket configuration options. Those can be re-added
  as an extension
* Incorporated feedback from Kevin Traynor.

v3:
* Went back to a global dpdk-init
* Language cleanup and various minor fixes

v4:
* Added a way to pass arbitrary eal arguments

v5:
* Restore the socket-mem default, and fix up the ovs-dev.py script, along
  with the manpage for ovsdb-server

v6:
* Correct a documentation issue with INSTALL.DPDK.md
* Correct a non-dpdk enabled OVS incorrect warning variable
* Remove an excess whitespace

v7:
* After testing by Christian with dpdk-alloc-mem

v8:
* Confirmed ``make check`` operation with and without dpdk.
  Retested on live-host

v9:
* Cleanup of comments
* Cleanup of one place where headers are specified
* Mark the dpdk coremask and numa config as optional
* Added 5/6 to scan the extras and warn the user when conflicting
  DB entries are present
* Acks given for all but patch 5/6

v10:
* Rebased against latest upstream
* ACK or Tested-by for all patches
* Code cleanup on patch 2/6 (vhost-cuse warning)
* DB options documentation cleanup.

v11:
* Spacing cleanups (verified with checkpatch)
* Introduced a realpath() call
* Validate the vhost-sock-dir is in a protected area of the filesystem
* Split the netdev-dpdk into a netdev-nodpdk
* Converted most of the ovs_abort() error paths into VLOG_ERR()s

Aaron Conole (8):
  netdev-dpdk: Restore thread affinity after DPDK init
  util: Add a path canonicalizer
  netdev-dpdk: Convert initialization from cmdline to db
  netdev-dpdk: Restrict vhost_sock_dir
  netdev-dpdk: Autofill lcore coremask if absent
  netdev-dpdk: Allow arbitrary eal arguments
  netdev-dpdk: Check dpdk-extra when reading db
  NEWS: Announce the DPDK EAL configuration change

 FAQ.md |   6 +-
 INSTALL.DPDK.md|  87 +++--
 NEWS   |   3 +
 configure.ac   |   2 +-
 lib/automake.mk|   4 +
 lib/netdev-dpdk.c  | 430 +
 lib/netdev-dpdk.h  |  13 +-
 lib/netdev-nodpdk.c|  21 +++
 lib/util.c |  52 ++
 lib/util.h |   1 +
 tests/library.at   |   5 +
 tests/ofproto-macros.at|   3 +-
 tests/test-util.c  |  23 +++
 utilities/ovs-dev.py   |   8 +-
 vswitchd/bridge.c  |   3 +
 vswitchd/ovs-vswitchd.8.in |   6 +-
 vswitchd/ovs-vswitchd.c|  25 +--
 vswitchd/vswitch.xml   | 143 ++-
 18 files changed, 697 insertions(+), 138 deletions(-)
 create mode 100644 lib/netdev-nodpdk.c

-- 
2.5.5

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

Re: [ovs-dev] [RFC 2/2] ovn: Add minimal software l2 gateway.

[Russell Bryant]

On Tue, Mar 29, 2016 at 9:47 PM, Russell Bryant  wrote:

> This patch implements one approach to using ovn-controller to implement
> a software l2 gateway between logical and physical networks.
>
> A new logical port type called "gateway" is introduced here.  It is very
> close to how localnet ports work, with the following exception:
> A localnet port makes OVN use the physical network as the
> transport between hypervisors instead of tunnels. A gateway port still
> uses tunnels between all hypervisors, and packets only go to/from the
> specified physical network as needed via the chassis the gateway port
> is bound to.
>
> Signed-off-by: Russell Bryant 
> ---
>  ovn/controller/binding.c|   5 ++
>  ovn/controller/ovn-controller.8.xml |  15 ++--
>  ovn/controller/ovn-controller.c |   2 +-
>  ovn/controller/patch.c  |  24 --
>  ovn/controller/patch.h  |   2 +-
>  ovn/controller/physical.c   |  10 ++-
>  ovn/ovn-nb.xml  |  19 +
>  ovn/ovn-sb.xml  |  38 +
>  tests/ovn.at| 164
> 
>  9 files changed, 259 insertions(+), 20 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index 69d4cc0..277d5b4 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -169,18 +169,19 @@
>  
>The presence of this key identifies a patch port as one created
> by
>ovn-controller to connect the integration bridge
> and
> -  another bridge to implement a localnet logical
> port.
> -  Its value is the name of the logical port with type=localnet
> that
> -  the port implements.
> +  another bridge to implement a localnet or
> +  gateway logical port.  Its value is the name of the
> +  logical port with type set to localnet
> +  or gateway that the port implements.
>See external_ids:ovn-bridge-mappings, above,
>for more information.
>  
>
>  
> -  Each localnet logical port is implemented as a
> pair of
> -  patch ports, one in the integration bridge, one in a different
> -  bridge, with the same
> external-ids:ovn-localnet-port
> -  value.
> +  Each localnet and gateway logical port
> +  is implemented as a pair of patch ports, one in the integration
> +  bridge, one in a different bridge, with the same
> +  external-ids:ovn-localnet-port value.
>  
>
>

Suggestion to self:

It probably makes sense to rename the "ovn-localnet-port" external-id to
something like "ovn-bridge-mapping".  That may help make it more clear that
the patch port connects br-int and another ovs bridge as defined by
ovn-bridge-mappings and that these patch ports are no longer specific to
"type=localnet" ports.

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

Re: [ovs-dev] Question about assess_weak_refs( ) function implementation

[Ben Pfaff]

On Thu, Mar 24, 2016 at 12:21:04AM +, Esquivel, Randall Jose wrote:
> Hello,
> 
> I have been looking at the code used to update the weak references in OVS
> and would like to clarify what is the correct usage of the function 
> assess_weak_refs().
> This function is called inside for_each_txn_row():
> ...
> while (t->n_processed < hmap_count(>txn_rows)) {
> ...
> assess_weak_refs()  <-callback
> ...
> 
> The function for_each_txn_row() iterates internally calling on each cycle to
> assess_weak_refs() controlled by the counter
> 
> hmap_count(>txn_rows)
> 
> This counter represents the number of nodes currently in 'hmap'. This counter
> is incremented each time a row is processed inside assess_weak_refs() for rows
> deleted or modified in the original code. The call hierarchy used to update 
> the
> hmap is shown:
> 
> ovsdb_txn_row_modify()
> ovsdb_txn_row_create()
> hmap_insert()
> hmap_insert_at()
> hmap_insert_fast()
> ...
> hmap->n++;
> 
> What the code is apparently doing...
> 
> 1.   In the case a row has been deleted assess_weak_refs() enters and 
> modify the rows with references to the row that has been deleted and exit. 
> These rows are now in the hmap.
> 2.   For each of the modified rows (which have weak references to the 
> deleted row),
> a.   Run assess_weak_refs() and modify rows that have weak references to 
> this row. [dst_refs are the weak references to current row] ***
>i.ovsdb_table_get_row() 
> checks for rows in the hmap matching a uuid and if the row exists a weak 
> reference is added.
> 
> The implementation modifies the rows with weak references using:
> 
> if (txn_row->old) {
> ...
> LIST_FOR_EACH_SAFE (weak, next, dst_node, _row->old->dst_refs) {
> ...
> 
> but it evaluates deleted or modified rows.
> 
> ***When a row has been deleted, the rows with weak references (*) to it get 
> modified
> and its weak references updated. But this produces that the rows with weak 
> references
> to the rows in (*) get also updated, and so on.
> 
> Could the weak references be updated after modifying  *only* the rows with 
> weak
> references to the deleted row, or is it required that all dependent rows get 
> updated?

I think that the former is true.

> Is this the expected behavior of this function?

Is what the expected behavior of this function?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] Fix bug in ovn-ctl argument order

[Ryan Moats]

From: RYAN D. MOATS 

Commit 31491a53116a6c2fcd19f888f5f7ce71e0ccdd51 got the port and
address order backwards.  Restore it to keep ovsdb-server happy.

Signed-off-by: RYAN D. MOATS 
---
 ovn/utilities/ovn-ctl |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 0a54f7f..dad6db6 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -52,7 +52,7 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE 
--remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_ADDR:$DB_NB_PORT 
--pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
+set "$@" --detach $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE 
--remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR 
--pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
 
 $@ $DB_NB_FILE
 fi
@@ -63,7 +63,7 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE 
--remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_ADDR:$DB_SB_PORT 
--pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
+set "$@" --detach $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE 
--remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR 
--pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
 $@ $DB_SB_FILE
 fi
 }
-- 
1.7.1

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

Re: [ovs-dev] [CudaMailTagged] Match table

[Ben Pfaff]

On Fri, Apr 01, 2016 at 12:40:23PM +0200, Amrane Ait Zeouay wrote:
> I'm new to OVS and I'm working on an OVS project, and i read a lot of
> documents about it but i  didn't find anything that can make things clear
> to me about matching a packet and in userspace and datapath and if there is
> a match then what the ovs do ?

If there's a match, OVS executes the actions.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: add hotplug support

[Mauricio Vásquez]

Hello Li,

On Fri, Apr 1, 2016 at 11:08 AM, Li Wei  wrote:

> Hello,
>
> On 03/28/2016 04:52 PM, Mauricio Vasquez B wrote:
> > In order to use dpdk ports in ovs they have to be bound to a DPDK
> > compatible driver before ovs is started.
> >
> > This patch adds the possibility to hotplug (or hot-unplug) a device
> > after ovs has been started. The implementation adds an appctl command:
> > netdev-dpdk/port-clt
> >
> > After the user attaches a new device, it has to be added to a bridge
> > using the to use the add-port command, similarly, before detaching a
> > device, it has to be removed using the del-port command.
>
> Do we need some additional check in dpdk_eth_dev_init(), because the
> port_id
> maybe discontinuous after a hot-unplug?
>
>
Yes, we need. Thanks for noticing it, it is also necessary to do some minor
modifications in the dpdk_ring_create() function due to the same reason.

I sent a v2: http://openvswitch.org/pipermail/dev/2016-April/069019.html

Mauricio Vasquez,

Thanks.
>
>
> >
> > Signed-off-by: Mauricio Vasquez B <
> mauricio.vasquezber...@studenti.polito.it>
> > ---
> >  lib/netdev-dpdk.c | 73
> +++
> >  1 file changed, 73 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 7c4cd07..05fa0df 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1982,6 +1982,75 @@ netdev_dpdk_set_admin_state(struct unixctl_conn
> *conn, int argc,
> >  unixctl_command_reply(conn, "OK");
> >  }
> >
> > +static void
> > +netdev_dpdk_port_ctl(struct unixctl_conn *conn, int argc OVS_UNUSED,
> > + const char *argv[], void *aux OVS_UNUSED)
> > +{
> > +int ret;
> > +uint8_t port_id;
> > +unsigned int parsed_port;
> > +char devname[RTE_ETH_NAME_MAX_LEN];
> > +char response[512];
> > +
> > +ovs_mutex_lock(_mutex);
> > +
> > +if (strcmp(argv[1], "attach") == 0) {
> > +ret = rte_eth_dev_attach(argv[2], _id);
> > +if (ret < 0) {
> > +snprintf(response, sizeof(response),
> > + "Error attaching device '%s'", argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +snprintf(response, sizeof(response),
> > + "Device '%s' has been attached as 'dpdk%d'", argv[2],
> port_id);
> > +unixctl_command_reply(conn, response);
> > +
> > +} else if (strcmp(argv[1], "detach") == 0) {
> > +ret = dpdk_dev_parse_name(argv[2], "dpdk", _port);
> > +if (ret) {
> > +snprintf(response, sizeof(response),
> > + "'%s' is not a valid dpdk device", argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +port_id = parsed_port;
> > +
> > +struct netdev * netdev = netdev_from_name(argv[2]);
> > +if (netdev) {
> > +netdev_close(netdev);
> > +snprintf(response, sizeof(response),
> > + "Port '%s' is being used. Remove it before
> detaching",
> > + argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +rte_eth_dev_close(port_id);
> > +
> > +ret = rte_eth_dev_detach(port_id, devname);
> > +if (ret < 0) {
> > +snprintf(response, sizeof(response),
> > + "Port '%s' can not be detached", argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +snprintf(response, sizeof(response),
> > + "Port '%s' has been detached", argv[2]);
> > +unixctl_command_reply(conn, response);
> > +} else {
> > +snprintf(response, sizeof(response),
> > + "'%s' is not a valid argument", argv[1]);
> > +unixctl_command_reply_error(conn, response);
> > +}
> > +
> > +unlock:
> > +ovs_mutex_unlock(_mutex);
> > +}
> > +
> >  /*
> >   * Set virtqueue flags so that we do not receive interrupts.
> >   */
> > @@ -2262,6 +2331,10 @@ dpdk_common_init(void)
> >   "[netdev] up|down", 1, 2,
> >   netdev_dpdk_set_admin_state, NULL);
> >
> > +unixctl_command_register("netdev-dpdk/port-ctl",
> > + "attach/detach device", 2, 2,
> > + netdev_dpdk_port_ctl, NULL);
> > +
> >  ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
> >  }
> >
> >
>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] netdev-dpdk: add hotplug support

[Mauricio Vasquez B]

In order to use dpdk ports in ovs they have to be bound to a DPDK
compatible driver before ovs is started.

This patch adds the possibility to hotplug (or hot-unplug) a device
after ovs has been started. The implementation adds an appctl command:
netdev-dpdk/port-clt

After the user attaches a new device, it has to be added to a bridge
using the to use the add-port command, similarly, before detaching a
device, it has to be removed using the del-port command.

Signed-off-by: Mauricio Vasquez B 
---
 INSTALL.DPDK.md   | 26 +++-
 NEWS  |  1 +
 lib/netdev-dpdk.c | 88 ++-
 3 files changed, 107 insertions(+), 8 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 9ec8bf6..4095402 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -81,7 +81,7 @@ Using the DPDK with ovs-vswitchd:
 
 1. Setup system boot
Add the following options to the kernel bootline:
-   
+
`default_hugepagesz=1GB hugepagesz=1G hugepages=1`
 
 2. Setup DPDK devices:
@@ -227,6 +227,29 @@ Using the DPDK with ovs-vswitchd:
For more details regarding egress-policer parameters please refer to the
vswitch.xml.
 
+9. Port Hotplug
+
+   ovs supports port hotplugging, it allows to use ports that were not bound
+   to DPDK when vswitchd was started.
+   In order to attach a port, it has to be bound to DPDK using the
+   dpdk_nic_bind.py script:
+
+   `$DPDK_DIR/tools/dpdk_nic_bind.py --bind=igb_uio :01:00.0`
+
+   Then it can be attached to OVS:
+
+   `ovs-appctl netdev-dpdk/port-ctl attach :01:00.0`
+
+   At this point, the user can create a ovs port using the add-port command.
+
+   It is also possible to detach a port from ovs, the user has to remove the
+   port using the del-port command, then it can be detached using:
+
+   `ovs-appctl netdev-dpdk/port-ctl detach dpdk0`
+
+   This feature is not supported by all the NICs, please refer to the
+   [DPDK Port Hotplug Framework] in order to get more information.
+
 Performance Tuning:
 ---
 
@@ -959,3 +982,4 @@ Please report problems to b...@openvswitch.org.
 [INSTALL.md]:INSTALL.md
 [DPDK Linux GSG]: 
http://www.dpdk.org/doc/guides/linux_gsg/build_dpdk.html#binding-and-unbinding-network-ports-to-from-the-igb-uioor-vfio-modules
 [DPDK Docs]: http://dpdk.org/doc
+[DPDK Port Hotplug Framework]: 
http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html
diff --git a/NEWS b/NEWS
index ea7f3a1..2ba8659 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.5.0
assignment.
  * Type of log messages from PMD threads changed from INFO to DBG.
  * QoS functionality with sample egress-policer implementation.
+ * Port Hotplug is now supported.
- ovs-benchmark: This utility has been removed due to lack of use and
  bitrot.
- ovs-appctl:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..4fcab36 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -599,7 +599,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) 
OVS_REQUIRES(dpdk_mutex)
 int diag;
 int n_rxq, n_txq;
 
-if (dev->port_id < 0 || dev->port_id >= rte_eth_dev_count()) {
+if (!rte_eth_dev_is_valid_port(dev->port_id)) {
 return ENODEV;
 }
 
@@ -1985,6 +1985,75 @@ netdev_dpdk_set_admin_state(struct unixctl_conn *conn, 
int argc,
 unixctl_command_reply(conn, "OK");
 }
 
+static void
+netdev_dpdk_port_ctl(struct unixctl_conn *conn, int argc OVS_UNUSED,
+ const char *argv[], void *aux OVS_UNUSED)
+{
+int ret;
+uint8_t port_id;
+unsigned int parsed_port;
+char devname[RTE_ETH_NAME_MAX_LEN];
+char response[512];
+
+ovs_mutex_lock(_mutex);
+
+if (strcmp(argv[1], "attach") == 0) {
+ret = rte_eth_dev_attach(argv[2], _id);
+if (ret < 0) {
+snprintf(response, sizeof(response),
+ "Error attaching device '%s'", argv[2]);
+unixctl_command_reply_error(conn, response);
+goto unlock;
+}
+
+snprintf(response, sizeof(response),
+ "Device '%s' has been attached as 'dpdk%d'", argv[2], 
port_id);
+unixctl_command_reply(conn, response);
+
+} else if (strcmp(argv[1], "detach") == 0) {
+ret = dpdk_dev_parse_name(argv[2], "dpdk", _port);
+if (ret) {
+snprintf(response, sizeof(response),
+ "'%s' is not a valid dpdk device", argv[2]);
+unixctl_command_reply_error(conn, response);
+goto unlock;
+}
+
+port_id = parsed_port;
+
+struct netdev *netdev = netdev_from_name(argv[2]);
+if (netdev) {
+netdev_close(netdev);
+snprintf(response, sizeof(response),
+ "Port '%s' is being used. Remove it before detaching",
+ argv[2]);
+unixctl_command_reply_error(conn, response);
+goto unlock;

[ovs-dev] Documents

[Aurora Berger]

Dear dev,

We obtained such documents from your bank, please view the attached documents.

Yours sincerely,
Aurora Berger
VP Analytic Services
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] tunneling: Fix for concomitant IPv4 and IPv6 tunnels

[Thadeu Lima de Souza Cascardo]

When using an IPv6 tunnel on the same bridge as an IPv4 tunnel, the flow
received from the IPv6 tunnel would have an IPv4 address added to it, causing
problems when trying to put or execute the action on Linux datapath.

Clearing the IPv6 address when we have a valid IPv4 address fixes this problem.

Signed-off-by: Thadeu Lima de Souza Cascardo 
---
 ofproto/tunnel.c |  4 
 tests/tunnel.at  | 27 +++
 2 files changed, 31 insertions(+)

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 7430994..18297b2 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -419,12 +419,16 @@ tnl_port_send(const struct ofport_dpif *ofport, struct 
flow *flow,
 flow->tunnel.ip_src = 
in6_addr_get_mapped_ipv4(_port->match.ipv6_src);
 if (!flow->tunnel.ip_src) {
 flow->tunnel.ipv6_src = tnl_port->match.ipv6_src;
+} else {
+flow->tunnel.ipv6_src = in6addr_any;
 }
 }
 if (!cfg->ip_dst_flow) {
 flow->tunnel.ip_dst = 
in6_addr_get_mapped_ipv4(_port->match.ipv6_dst);
 if (!flow->tunnel.ip_dst) {
 flow->tunnel.ipv6_dst = tnl_port->match.ipv6_dst;
+} else {
+flow->tunnel.ipv6_dst = in6addr_any;
 }
 }
 flow->pkt_mark = tnl_port->match.pkt_mark;
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 0c033da..9f17194 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -524,3 +524,30 @@ Datapath actions: 
set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,geneve({class=0x,
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([tunnel - concomitant IPv6 and IPv4 tunnels])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
+options:remote_ip=1.1.1.1 ofport_request=1 \
+-- add-port br0 p2 -- set Interface p2 type=vxlan \
+options:remote_ip=2001:cafe::1 ofport_request=2])
+AT_DATA([flows.txt], [dnl
+in_port=1,actions=2
+in_port=2,actions=1
+])
+OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'tunnel(tun_id=0,src=1.1.1.1,dst=1.1.1.2,ttl=64),in_port(4789)'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 
set(tunnel(tun_id=0x0,ipv6_dst=2001:cafe::1,ttl=64,flags(df|key))),4789
+])
+
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'tunnel(tun_id=0x0,ipv6_src=2001:cafe::1,ipv6_dst=2001:cafe::2,ttl=64),in_port(4789)'],
 [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 
set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,flags(df|key))),4789
+])
+
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
-- 
2.5.0

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

Re: [ovs-dev] [PATCH monitor_cond V5 04/18] ovsdb: generate update notifications for monitor_cond session

[Liran Schour]

Ben Pfaff  wrote on 30/03/2016 07:56:01 PM:

> On Tue, Mar 22, 2016 at 11:24:14PM +0200, Liran Schour wrote:
> > Ben Pfaff  wrote on 22/03/2016 07:23:33 PM:
> > 
> > > On Fri, Mar 04, 2016 at 08:08:59AM +, Liran Schour wrote:
> > > > Hold session's conditions in ovsdb_monitor_session_condition. Pass 
it
> > > > to ovsdb_monitor for generating "update2" notifications.
> > > > Add functions that can generate "update2" notification for a
> > > > "monitor_cond" session.
> > > > JSON cache is enabled only for session's with true condition only.
> > > > "monitor_cond" and "monitor_cond_change" are RFC 7047 extensions
> > > > described by ovsdb-server(1) manpage.
> > > > 
> > > > Signed-off-by: Liran Schour 
> > > 
> > > I think that ovsdb_monitor_table_condition_set() has a memory leak 
in
> > > the error case; it does not free the 'error' returned to it.
> > > 
> > 
> > Right. Will fix that.
> > 
> > > I'm a little confused about ovsdb_monitor_session_condition.  Why is
> > > this a separate data structure?  That is, why is there a separate 
shash
> > > of conditions rather than incorporating a condition into
> > > ovsdb_monitor_table?
> > > 
> > 
> > A single ovsdb_monitor instance can be used by many jsonrpc sessions 
(as 
> > long as they share the same set of tables and columns). 
> > Ovsdb_monitor_condition_session is holding the condition state per 
each 
> > jsonrpc session. Each session, in ovsdb_monitor, can have a different 
set 
> > of conditions and it is being represented by 
> > ovsdb_monitor_session_condition with ovsdb_monitor_table_condition per 

> > table.
> > The trivial option was to divide ovsdb_monitor into 2 different 
instances 
> > if we have 2 sessions with different sets of conditions. But, 
condition is 
> > changing during the lifetime of monitor session. And by holding 
sessions 
> > with different conditions under the same ovsdb_monitor we can reduce 
the 
> > computation needed to insert row changes to ovsdb_monitor. (insert 
once 
> > instead of twice)
> 
> Those downsides of having a monitor per (monitored columns, condition)
> seem pretty minor.  It also seems to me like the "obvious" way to
> implement this.  It seems like, for example, the problem of caching with
> conditions is much easier if the data structures are organized this
> way.  Did you actually try implementing it that way and determine that
> there was some insuperable difficulty?

No, I did not try implementing it in the 'trivial' way. The reason is 
twofold: first, to gain the ability to share the computed updates between 
sessions, also as per-session conditions change; second, to gain the 
ability to 'fall back' to using a Json cache for condition-less tables. (A 
typical monitor session includes conditioned tables and condition-less 
tables).

My implementation choice achieves the above and has the complexity of 
processing a DB update in O(#conditioned columns). The 'trivial' 
alternative would not achieve the above in the general case and would have 
the complexity of processing a DB update in O(#jsonrpc sessions with 
matched conditions), in the best case.

As I see it, the above stated benefits of the more complex design worth 
the complexity increase in the code, given also that the memory complexity 
is similar.

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

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

[Bounced mail]

N]éIšÑ¥|U Ó
úLBIuèCߨ¿YvañúÓy;ËüsÍ2wû®qÓ3{©Žl’ìÌ~ΚýÛÜljå¾X/]NY r„Ò.RQˆèÙ\
Q
ƒ^ˁ•´ 
k%ÙgõùúIðZ£f/¼ò²:õn3sTTJÝfv°&‰`š^^ËôÖ!†CM‡á\G(09‰þNwŸê‹×М¾Bõ)¼9ÇÂø֎h„óHèUqŽ~s4ñ–
ÌÏñY~mI
h½0FFÔqÚ6IE¤õªêY<°zc’t;úÜ 
ªpIØØ!µÓG§#%ÃÀ’ŠŠûuÚ7lNªéKN¼"
ò†j&Ÿ$ìböG옯kYïҞê9f
v»>ô֔ô!t{LlÂUtÛ-§3º0i±%Òë—XµÏ‚¦#0ÎQ2«ã(ØÊÌôlÓap")£ŸY|ßdã#
üw÷¹!´¶·œéQÝ#fŸ­KÊí—ýFxKÞ/[†ä‘I2œ&¬Ý{‰óvԙñkœø_2íŒWăۘ漢®9Õ¦I1hr0õ,WWÔ&¿¤.ýz32ê¯×ôÇÙãi2ˆ_ٽވ
an}dyòÔUžZóžî¾·½¿Þ‰?™•ŠpšçG˜bH¿]·^
Ûֿփ~ÍU‚Œt{¶TQWk<.šzS­Ù‡¶b vˆ
¦TbÀµšŒÁZç”în¹â4~¨»e©Bï;Šgg*1«hˆ³ÉwQã–dŽ-0ãÏE–KJøYž[,«ñ2;ÚpEZÀÃrBÙTx®ÉÒyáÔ°1¼Ìf¿ßA«”Ç-ç|ñ¢ûúTÓïCÒ^†—˜´^®ÊŒ£¡ˆ0lÛôÊ>31?t#ZÏH¸í¹ü£¾*ž#::¶ãU“J/ËÅhz›Ã
 
؆f‡„•é5\kÇ`ˆqß7Nb.ù_(ôÃ?­œ(¸”'<ªYhxn4L$ï“<ºÑÏ4)CÛHT&̂J6`r‘-!LÆ6ê¼Óú“R†mŠËߟdü䋙K¬ÁÙ¥Ã܋¦
 Ë²u‰Û%$È¢ôhL)LRN¨ÓÚLdª^e Á)Q¹޷Åeñ¸¸M,e.,DΔÏzÍ/ú¤§Ø¼µ
„òš,¢96¹#íÖÍ"ã]¿¬ÃÂ`ž¬‰!T(ÂЅeiDÐí^Zõäs[Þ#ó
‹°¦Ã³ ÝÐ%ŒœŠÕšfÞ(ó\ýi§j¿ý\úÑdô¹·•p$Ϭn©¿°nÚêÃãÅÞ¼¨Ó¤
‡â[o,®*QQOÛלs“¿Q3~mAäóàA
ŽÝTsbߊQ(§’®Ñ
! 6w»:M
³>qUõj|4×¹J®3BðèÃGõTh4àú¸o—ã0Ìm
ùSíäé„|fD÷S„v]±Ñá´«ù³„öðE¨!èì¿Á½Wú(YÒ,|þ¦{ŸPàr¥’ÎEÜ6©Ýßí­²m¥)T7¦2k”ÜS´/ފçy´“
ú¤È»msÜëTWÆ>†“"\¼Â$´V·¡*¯ݧ¬›Mºg˜²èBáéÍÛ9®øvk/Ru¥h•c:BK÷{ö#𴦓ÆîÒÒCµj…
¾šuéQz½œE¢´ÖïÔÁ몎$T¢R•ú¡¾gÑ`1®è?Ÿ6ø !î¶F«Õ'¥Dí¼s(]Â¢÷¿k†ÎÐyI;nVep 
‹®ÛxºIí²)­Õîäùg#©§ý×réÂñʽÜfüÐ'EprIŒ¶ðRkNÏ‚î  ÞGJK¢ò¦ž]Ø
£ˆË­íQ½-éÜ6&1Ü9þԚ„æIØ©
ŠÀæA6¨·¾÷rwI;.58ȉÑväèØäy`²Øi¤c½úJ1¬ çCÇk§§éþU'kµ¿†éNv†[<1uLTŒi¬Ø~·x
·p¶˜â7A‘k÷¥~_m»´­2<”Õáœltƒ³ÍåÓo¾rbò|88t÷§NÀRæÑہ©
z?Ç£{žYosãßÊåóãȳ#%c¬¾WÙAá,Ñ~¥‘Ï1`Z‘ǶëŠ)¬ïÑÇX-Óf»Ìcf»öèÞñå/Õ`HْQñ7>wŽó" 
›L„g—…
NçB¨z“z#™®tX®wEŽâ}­µ9»ŸÑ¼Æ÷j¼Ÿ,²‘ÊSqߏ8Þ̂s:ƒ)Y¼ðß²póJˆ-οrLQTŠ¤æÖsb¯£Ù)·µOÈ7ò%z4ˆ¸ñ"þjìÒGŒç
>Ñ9HCÁ™œôI1Ž^'À¡e¢I &ҍä6ï•(­Ø)œºòÐLC±ÞàAu3•é|š¼ú:¸ÔKGîPé¨üOS«
2MÕgɘ¢˜;
N¤?\¹½0pÙMe¾!})Øïæ[øcÍÜIóíu{OÅv1p9Ë gÆd»Ë^c}q¬
Jl}­1ùºKݼ¨š,p±ý•nù·Õ#— b¾¹žòÇåöXHwP~g¹C®Ì™*ô¾(PÛpOœR>-êß°†Æ0

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

[ovs-dev] [CudaMailTagged] Match table

[Amrane Ait Zeouay]

Hey Everyone,

I'm new to OVS and I'm working on an OVS project, and i read a lot of
documents about it but i  didn't find anything that can make things clear
to me about matching a packet and in userspace and datapath and if there is
a match then what the ovs do ?

Thank you.

Best regards


-- 

Amrane Ait Zeouay

Engineer Student in The Development of Software and Systems

University of Western Brittany

Tel:  +33 7 62 25 56 03 <+33+7+62+25+56+03>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/8] Convert DPDK configuration from command line to DB based

[Aaron Conole]

Aaron Conole  writes:

> Currently, configuration of DPDK parameters is done via the command line
> through a --dpdk **OPTIONS** -- command line argument. This has a number of
> challenges, including:
> * It must be the first option passed to ovs-vswitchd
> * It is the only datapath feature in OVS to be configured on the command
>   line
> * It requires specialized knowledge of sub-component command switches
> * It also inteprets non-EAL arguments (confusing users)
> * It is a broken model for datapath configuration.
>
> This series brings the following changes to openvswitch:
> * All DPDK options are taken from the ovs database rather than the
>   command line
> * Non-EAL arguments also have separate database entries
> * DPDK lcores are optionally auto-assigned to a single core based on the
>   bridge coremask.
> * DPDK options have default behaviors
> * Updated documentation
>
> This series has been build tested (including `make check`) on OSX, Fedora 23,
> Windows (via appveyor), and FreeBSD 10.3; the v11 has had very basic testing
> applied (start, configure some of the settings). I have removed ACKs and
> Tested-bys for some of the patches since they underwent changes that I felt
> disqualified continued use of the Acked-by: and Tested-by: tags.
>
> Travis-ci build: https://travis-ci.org/orgcandman/ovs/builds/119975234
> Appveyor build: https://ci.appveyor.com/project/orgcandman/ovs/build/1.0.8
>
> A huge round of thanks on the work so far should be given to the following
> folks (in alphabetical order):
> * Ben Pfaff (Reviews, vhost-sock-dir escape suggestion)
> * Christian Erhardt (Testing)
> * Daniele Di Proietto   (Reviews, general suggestions)
> * Flavio Leitner(Original efforts, reviews)
> * Kevin Traynor (Testing, general suggestions, reviews, doc reviews)
> * Panu Matilainen   (Initialization ideas, eal arguments ideas, reviews)
> * RobertX Wojciechowicz (Testing, general suggestions)
> * Sean Mooney   (Testing, general suggestions)
>
> v2:
> * Dropped the vhost-user socket configuration options. Those can be re-added
>   as an extension
> * Incorporated feedback from Kevin Traynor.
>
> v3:
> * Went back to a global dpdk-init
> * Language cleanup and various minor fixes
>
> v4:
> * Added a way to pass arbitrary eal arguments
>
> v5:
> * Restore the socket-mem default, and fix up the ovs-dev.py script, along
>   with the manpage for ovsdb-server
>
> v6:
> * Correct a documentation issue with INSTALL.DPDK.md
> * Correct a non-dpdk enabled OVS incorrect warning variable
> * Remove an excess whitespace
>
> v7:
> * After testing by Christian with dpdk-alloc-mem
>
> v8:
> * Confirmed ``make check`` operation with and without dpdk.
>   Retested on live-host
>
> v9:
> * Cleanup of comments
> * Cleanup of one place where headers are specified
> * Mark the dpdk coremask and numa config as optional
> * Added 5/6 to scan the extras and warn the user when conflicting
>   DB entries are present
> * Acks given for all but patch 5/6
>
> v10:
> * Rebased against latest upstream
> * ACK or Tested-by for all patches
> * Code cleanup on patch 2/6 (vhost-cuse warning)
> * DB options documentation cleanup.
>
> v11:
> * Spacing cleanups (verified with checkpatch)
> * Introduced a realpath() call
> * Validate the vhost-sock-dir is in a protected area of the filesystem
> * Split the netdev-dpdk into a netdev-nodpdk
> * Converted most of the ovs_abort() error paths into VLOG_ERR()s
>
> Aaron Conole (8):
>   netdev-dpdk: Restore thread affinity after DPDK init
>   util: Add a path canonicalizer
>   netdev-dpdk: Convert initialization from cmdline to db
>   netdev-dpdk: Restrict vhost_sock_dir
>   netdev-dpdk: Autofill lcore coremask if absent
>   netdev-dpdk: Allow arbitrary eal arguments
>   netdev-dpdk: Check dpdk-extra when reading db
>   NEWS: Announce the DPDK EAL configuration change
>
>  FAQ.md |   6 +-
>  INSTALL.DPDK.md|  87 +++--
>  NEWS   |   3 +
>  configure.ac   |   2 +-
>  lib/automake.mk|   4 +
>  lib/netdev-dpdk.c  | 430 
> +
>  lib/netdev-dpdk.h  |  13 +-
>  lib/netdev-nodpdk.c|  21 +++
>  lib/util.c |  52 ++
>  lib/util.h |   1 +
>  tests/library.at   |   5 +
>  tests/ofproto-macros.at|   3 +-
>  tests/test-util.c  |  23 +++
>  utilities/ovs-dev.py   |   8 +-
>  vswitchd/bridge.c  |   3 +
>  vswitchd/ovs-vswitchd.8.in |   6 +-
>  vswitchd/ovs-vswitchd.c|  25 +--
>  vswitchd/vswitch.xml   | 143 ++-
>  18 files changed, 697 insertions(+), 138 deletions(-)
>  create mode 100644 lib/netdev-nodpdk.c

Self NAK - patch 4/8 is missing an important security fix. I will
resubmit with a proper formatted subject and the missing hunk.

Apologies for the noise.
___
dev mailing 

[ovs-dev] We offer new vacancy

[dev]

Hello!

We are looking for employees working remotely.

My name is Lucas, am the personnel manager of a large International company.
Most of the work you can do from home, that is, at a distance.

Salary is $2000-$5000.

If you are interested in this offer, please visit 
Our Web Page

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

Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: add hotplug support

[Li Wei]

Hello,

On 03/28/2016 04:52 PM, Mauricio Vasquez B wrote:
> In order to use dpdk ports in ovs they have to be bound to a DPDK
> compatible driver before ovs is started.
> 
> This patch adds the possibility to hotplug (or hot-unplug) a device
> after ovs has been started. The implementation adds an appctl command:
> netdev-dpdk/port-clt
> 
> After the user attaches a new device, it has to be added to a bridge
> using the to use the add-port command, similarly, before detaching a
> device, it has to be removed using the del-port command.

Do we need some additional check in dpdk_eth_dev_init(), because the port_id
maybe discontinuous after a hot-unplug?

Thanks.


> 
> Signed-off-by: Mauricio Vasquez B 
> ---
>  lib/netdev-dpdk.c | 73 
> +++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 7c4cd07..05fa0df 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1982,6 +1982,75 @@ netdev_dpdk_set_admin_state(struct unixctl_conn *conn, 
> int argc,
>  unixctl_command_reply(conn, "OK");
>  }
>  
> +static void
> +netdev_dpdk_port_ctl(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[], void *aux OVS_UNUSED)
> +{
> +int ret;
> +uint8_t port_id;
> +unsigned int parsed_port;
> +char devname[RTE_ETH_NAME_MAX_LEN];
> +char response[512];
> +
> +ovs_mutex_lock(_mutex);
> +
> +if (strcmp(argv[1], "attach") == 0) {
> +ret = rte_eth_dev_attach(argv[2], _id);
> +if (ret < 0) {
> +snprintf(response, sizeof(response),
> + "Error attaching device '%s'", argv[2]);
> +unixctl_command_reply_error(conn, response);
> +goto unlock;
> +}
> +
> +snprintf(response, sizeof(response),
> + "Device '%s' has been attached as 'dpdk%d'", argv[2], 
> port_id);
> +unixctl_command_reply(conn, response);
> +
> +} else if (strcmp(argv[1], "detach") == 0) {
> +ret = dpdk_dev_parse_name(argv[2], "dpdk", _port);
> +if (ret) {
> +snprintf(response, sizeof(response),
> + "'%s' is not a valid dpdk device", argv[2]);
> +unixctl_command_reply_error(conn, response);
> +goto unlock;
> +}
> +
> +port_id = parsed_port;
> +
> +struct netdev * netdev = netdev_from_name(argv[2]);
> +if (netdev) {
> +netdev_close(netdev);
> +snprintf(response, sizeof(response),
> + "Port '%s' is being used. Remove it before detaching",
> + argv[2]);
> +unixctl_command_reply_error(conn, response);
> +goto unlock;
> +}
> +
> +rte_eth_dev_close(port_id);
> +
> +ret = rte_eth_dev_detach(port_id, devname);
> +if (ret < 0) {
> +snprintf(response, sizeof(response),
> + "Port '%s' can not be detached", argv[2]);
> +unixctl_command_reply_error(conn, response);
> +goto unlock;
> +}
> +
> +snprintf(response, sizeof(response),
> + "Port '%s' has been detached", argv[2]);
> +unixctl_command_reply(conn, response);
> +} else {
> +snprintf(response, sizeof(response),
> + "'%s' is not a valid argument", argv[1]);
> +unixctl_command_reply_error(conn, response);
> +}
> +
> +unlock:
> +ovs_mutex_unlock(_mutex);
> +}
> +
>  /*
>   * Set virtqueue flags so that we do not receive interrupts.
>   */
> @@ -2262,6 +2331,10 @@ dpdk_common_init(void)
>   "[netdev] up|down", 1, 2,
>   netdev_dpdk_set_admin_state, NULL);
>  
> +unixctl_command_register("netdev-dpdk/port-ctl",
> + "attach/detach device", 2, 2,
> + netdev_dpdk_port_ctl, NULL);
> +
>  ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
>  }
>  
> 


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

Re: [ovs-dev] [PATCH] debian : upstream_version fix - resubmitted

[Zoltán Balogh]

Hi Simon,

This is a simpler and better solution. For me it's ok, since our team uses 
debian_revision too.

Best regards,
Zoltán

-Original Message-
From: Simon Horman [mailto:simon.hor...@netronome.com]
Sent: Tuesday, March 29, 2016 2:51 AM
To: Zoltán Balogh
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] debian : upstream_version fix - resubmitted

Hi Zoltánm

On Thu, Mar 24, 2016 at 08:28:53AM +, Zoltán Balogh wrote:
> Hi,
> 
> The Debian Policy Manual 
> (https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version) 
> says that the upstream_version may contain only alphanumerics and the 
> characters . + - : ~ (full stop, plus, hyphen, colon, tilde) and should start 
> with a digit.
> 
> Currently, the upstream_version is defined in the debian/rules file:
> 
> DEB_UPSTREAM_VERSION=$(shell dpkg-parsechangelog | sed -rne
> 's,^Version: ([0-9]:)*([^-]+).*,\2,p')
> 
> The version number is taken from the dpkg-parsechangelog printout then the 
> first part of the version number which does not contain hyphen is filtered 
> out with sed. However the Debian Policy Manual says that hyphen is allowed in 
> the upstream_version. 
> 
> This is not a problem with current vanilla OVS debian version. But, if a 
> postfix string including a hyphen is added to the upstream_version then 
> installation of datapath-dkms package will fail. 
> 
> I think the following patch solves this problem.
> 
> Signed-off-by: Zoltán Balogh 

I wonder if the version manipulation could be expressed using sed, as the code 
existing code does, rather than awk, sed, expr and shell.

Perhaps something like this:

sed -rne 's,^Version: ([0-9]+:)?([0-9][a-zA-Z0-9.+:~-]*)(-[a-zA-Z0-9*.~]*),\2,p'

Which I tested as follows:

Input: Version: 2.4.90-1
Output: 2.4.90

Input: Version: 1:2.4.90-1
Output: 2.4.90

Input: Version: 1:3:2.4.90-1
Output: 3:2.4.90

Input: Version: 2.4.90-xyz-1
Output: 2.4.90-xyz

Input: Version: 1:2.4.90-xyz-1
Output: 2.4.90-xyz

Input: Version: 1:3:2.4.90-xyz-1
Output: 3:2.4.90-xyz

N.B: Does not work without debian_version present
Input: Version: 2.4.90
Output: 

> 
> ---
> 
> diff --git a/debian/rules b/debian/rules index d8e90c7..70539ab 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -13,7 +13,9 @@
> 
>  PACKAGE=openvswitch
>  PACKAGE_DKMS=openvswitch-datapath-dkms
> -DEB_UPSTREAM_VERSION=$(shell dpkg-parsechangelog | sed -rne
> 's,^Version: ([0-9]:)*([^-]+).*,\2,p')
> +DEB_VERSION=$(shell dpkg-parsechangelog | awk '/^Version: / {print 
> +$$2}' | sed -rne 's,([0-9]:)+([.])*,\2,p') DEB_REVISION=$(shell expr 
> +"$(DEB_VERSION)" : '.*\(-.*\)' ) DEB_UPSTREAM_VERSION=$(shell 
> +version=$(DEB_VERSION); expr + $${version%"$(DEB_REVISION)"})
> 
>  ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))  PARALLEL = 
> -j$(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS)))
> 
> 
> ___
> 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 1/3] netdev-dpdk: add hotplug support

[Mauricio Vásquez]

Hi Ian,

On Thu, Mar 31, 2016 at 2:02 PM, Stokes, Ian  wrote:

> Hi Mauricio,
>
> This patch is quite useful. Some minor comments inline. I've also tested
> the patch and can confirm it works without issue.
>

Great!


>
> Thanks
> Ian
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Mauricio
> > Vasquez B
> > Sent: Monday, March 28, 2016 9:52 AM
> > To: dev@openvswitch.org
> > Subject: [ovs-dev] [PATCH 1/3] netdev-dpdk: add hotplug support
> >
> > In order to use dpdk ports in ovs they have to be bound to a DPDK
> > compatible driver before ovs is started.
> >
> > This patch adds the possibility to hotplug (or hot-unplug) a device
> > after ovs has been started. The implementation adds an appctl command:
> > netdev-dpdk/port-clt
> >
> > After the user attaches a new device, it has to be added to a bridge
> > using the to use the add-port command, similarly, before detaching a
> > device, it has to be removed using the del-port command.
> >
> > Signed-off-by: Mauricio Vasquez B
> > 
> > ---
> >  lib/netdev-dpdk.c | 73
> > +++
> >  1 file changed, 73 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > 7c4cd07..05fa0df 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1982,6 +1982,75 @@ netdev_dpdk_set_admin_state(struct unixctl_conn
> > *conn, int argc,
> >  unixctl_command_reply(conn, "OK");
> >  }
> >
> > +static void
> > +netdev_dpdk_port_ctl(struct unixctl_conn *conn, int argc OVS_UNUSED,
> > + const char *argv[], void *aux OVS_UNUSED) {
> > +int ret;
> > +uint8_t port_id;
> > +unsigned int parsed_port;
> > +char devname[RTE_ETH_NAME_MAX_LEN];
> > +char response[512];
> > +
> > +ovs_mutex_lock(_mutex);
> > +
> > +if (strcmp(argv[1], "attach") == 0) {
> > +ret = rte_eth_dev_attach(argv[2], _id);
> > +if (ret < 0) {
> > +snprintf(response, sizeof(response),
> > + "Error attaching device '%s'", argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +snprintf(response, sizeof(response),
> > + "Device '%s' has been attached as 'dpdk%d'", argv[2],
> > port_id);
> > +unixctl_command_reply(conn, response);
> > +
> > +} else if (strcmp(argv[1], "detach") == 0) {
> > +ret = dpdk_dev_parse_name(argv[2], "dpdk", _port);
> > +if (ret) {
> > +snprintf(response, sizeof(response),
> > + "'%s' is not a valid dpdk device", argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +port_id = parsed_port;
> > +
>
> Very minor style change here, the extra space between '*' and 'netdev'
> below can be removed.
>

I will correct it in a future v2.


>
> > +struct netdev * netdev = netdev_from_name(argv[2]);
> > +if (netdev) {
>
> So we should only enter here if the netdev device exists?
>

Yes, it is because if the device is being used it can not be detached
because it will cause a crash on the PMD threads that are using the device.


> However is there a specific reason you call netdev_close() before
> reporting the device is busy?
> I've tested with and without the call below and didn’t notice any
> functional difference; the port was still able to send/receive traffic.
> In the case the device is busy, is it required? If busy should the device
> be left as is and the reply error logged?
>

netdev_from_name() increases the count reference for that device, then
netdev_close() is called just to decrease the ref_cnt to the original value
avoiding a memory leakage.


>
> > +netdev_close(netdev);
> > +snprintf(response, sizeof(response),
> > + "Port '%s' is being used. Remove it before
> > detaching",
> > + argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +rte_eth_dev_close(port_id);
> > +
> > +ret = rte_eth_dev_detach(port_id, devname);
> > +if (ret < 0) {
> > +snprintf(response, sizeof(response),
> > + "Port '%s' can not be detached", argv[2]);
> > +unixctl_command_reply_error(conn, response);
> > +goto unlock;
> > +}
> > +
> > +snprintf(response, sizeof(response),
> > + "Port '%s' has been detached", argv[2]);
> > +unixctl_command_reply(conn, response);
> > +} else {
> > +snprintf(response, sizeof(response),
> > + "'%s' is not a valid argument", argv[1]);
> > +unixctl_command_reply_error(conn, response);
> > +}
> > +
> > +unlock:
> > +ovs_mutex_unlock(_mutex);
> > +}
> 

Re: [ovs-dev] [PATCH] datapath-windows: Hot add CPU support.

[Paul Boca]

Hi Sorin!

The only issue I see here is in OvsCpuChange. This function is called twice for 
each processor added to the system, once with KeProcessorAddStartNotify and 
second with KeProcessorAddCompleteNotify or KeProcessorAddFailureNotify.
You make reallocation on StartNotify and AddFailureNotify, but it's possible 
that for current processor the callback OvsCpuChange will be called with both 
states, sequentially.
And KeQueryActiveProcessorCountEx will return you the old processor count on 
KeProcessorAddStartNotify - the current added processor is not completely 
active yet.

If you change KeProcessorAddStartNotify with KeProcessorAddCompleteNotify, all 
should be fine.

Thanks,
Paul

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis
Sent: Thursday, March 31, 2016 3:03 PM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: Hot add CPU support.

Hot add CPU is the ability to dynamically add CPUs to a running system. Adding 
CPUs can occur physically by adding new hardware, logically by online hardware 
partitioning, or virtually through a virtualization layer.

This patch add support to reallocate any per-cpu resources, in case a new 
processor is added.

Signed-off-by: Sorin Vinturis 
Reported-by: Sorin Vinturis 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/112
---
 datapath-windows/ovsext/Actions.c  |  11 +-
 datapath-windows/ovsext/Datapath.c |   4 -
 datapath-windows/ovsext/Driver.c   |   9 ++
 datapath-windows/ovsext/Recirc.c   | 245 +++--
 datapath-windows/ovsext/Recirc.h   |  87 ++---
 datapath-windows/ovsext/Util.c | 122 +-
 datapath-windows/ovsext/Util.h |  27 +++-
 7 files changed, 410 insertions(+), 95 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index a91454d..513a1a4 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -40,6 +40,8 @@
 
 #define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2
 
+extern PNDIS_RW_LOCK_EX ovsDeferredActionLevelLock;
+
 typedef struct _OVS_ACTION_STATS {
 UINT64 rxGre;
 UINT64 txGre;
@@ -1973,14 +1975,19 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 
 flow = OvsLookupFlow(>datapath, key, , 
FALSE);
 if (flow) {
-UINT32 level = OvsDeferredActionsLevelGet();
+UINT32 level = 0;
+LOCK_STATE_EX lockState;
+
+NdisAcquireRWLockRead(ovsDeferredActionLevelLock, , 
+ 0);
 
+level = OvsDeferredActionsLevelGet();
 if (level > DEFERRED_ACTION_EXEC_LEVEL) {
 OvsCompleteNBLForwardingCtx(,
 L"OVS-Dropped due to deferred actions execution level limit \
   reached");
 ovsActionStats.deferredActionsExecLimit++;
 ovsFwdCtx.curNbl = NULL;
+NdisReleaseRWLock(ovsDeferredActionLevelLock, );
 return NDIS_STATUS_FAILURE;
 }
 
@@ -1999,6 +2006,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 ovsFwdCtx.curNbl = NULL;
 
 OvsDeferredActionsLevelDec();
+
+NdisReleaseRWLock(ovsDeferredActionLevelLock, );
 } else {
 POVS_VPORT_ENTRY vport = NULL;
 LIST_ENTRY missedPackets;
diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 464fa97..5725114 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -385,8 +385,6 @@ OvsInit()
 gOvsCtrlLock = 
 NdisAllocateSpinLock(gOvsCtrlLock);
 OvsInitEventQueue();
-OvsDeferredActionsQueueAlloc();
-OvsDeferredActionsLevelAlloc();
 }
 
 VOID
@@ -397,8 +395,6 @@ OvsCleanup()
 NdisFreeSpinLock(gOvsCtrlLock);
 gOvsCtrlLock = NULL;
 }
-OvsDeferredActionsQueueFree();
-OvsDeferredActionsLevelFree();
 }
 
 VOID
diff --git a/datapath-windows/ovsext/Driver.c b/datapath-windows/ovsext/Driver.c
index 853886e..f5d3f9c 100644
--- a/datapath-windows/ovsext/Driver.c
+++ b/datapath-windows/ovsext/Driver.c
@@ -152,6 +152,12 @@ DriverEntry(PDRIVER_OBJECT driverObject,
 goto cleanup;
 }
 
+/* Allocate per-cpu structures and register processor change callback. */
+status = OvsPerCpuDataInit(gOvsExtDriverHandle);
+if (!NT_SUCCESS(status)) {
+goto cleanup;
+}
+
 cleanup:
 if (status != NDIS_STATUS_SUCCESS){
 OvsCleanup();
@@ -180,6 +186,9 @@ OvsExtUnload(struct _DRIVER_OBJECT *driverObject)
 
 OvsDeleteDeviceObject();
 
+/* Release per-cpu structures and deregister processor change callback. */
+OvsPerCpuDataCleanup();
+
 NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
 }
 
diff --git a/datapath-windows/ovsext/Recirc.c b/datapath-windows/ovsext/Recirc.c
index 86e6f51..267e051 100644
--- a/datapath-windows/ovsext/Recirc.c
+++ b/datapath-windows/ovsext/Recirc.c
@@ -18,71 +18,214 @@
 #include "Flow.h"
 

[ovs-dev] meters support in datapath

[Deepanshu Saxena1/CHN/TCS]

Hi all, 



I want to implement and contribute B.19.13-Meter action (EXT-379) of OF 1.5.1 
to openvswitch 


According to FAQ: 
"Q: Does Open vSwitch support OpenFlow meters? 


A: Since version 2.0, Open vSwitch has OpenFlow protocol support for OpenFlow 
meters. There is no implementation of meters in the Open vSwitch software 
switch (neither the kernel-based nor userspace switches)." 


As per my understanding, meter support in openvswitch datapath is not present. 
Could you please specify the reason for the same, as I could see the support of 
meter in datapath of other software switches such as CPqD Softswitch and 
ofsoftswitch. 


Also since no datapath support is present, so is there any other way to test 
meters in openvswitch without using hardware switches, as I would be requiring 
this for development and testing of "(EXT-379) - meter Action" 
Thanks & Regards 
-- 
Deepanshu Saxena 
Assistant System Engineer, 
Tata Consultancy Services 
Mailto: deepanshu.saxe...@tcs.com 
Website: http://www.tcs.com 
=-=-=
Notice: The information contained in this e-mail
message and/or attachments to it may contain 
confidential or privileged information. If you are 
not the intended recipient, any dissemination, use, 
review, distribution, printing or copying of the 
information contained in this e-mail message 
and/or attachments to it are strictly prohibited. If 
you have received this communication in error, 
please notify us by reply e-mail or telephone and 
immediately and permanently delete the message 
and any attachments. Thank you


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