date:20170531



On 5/31/17, 8:06 PM, "王志克"  wrote:

Hi Darrell,

In my opinion, it may be also hard for user to decide "configurable buffer 
size".


Yep, but the user knows their network or situation best.

 Also I guess the default value should be to enable the fragment support. ( If 
we give such option, I can imagine most user will enable it, right?)

I agree.
However, let us say we chose values for the memory thresholds for the user, 
like Linux.
The user thinks “the fragmentation thing is taken care of”. That is likely 
wrong, maybe they should be
thinking more upfront.

My suggestion is to use Linux kernel as best practice.

That is the stock answer; no doubt.

Just my personal thought.

Br,
Wangzhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年6月1日 10:16
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: 答复: 答复: [ovs-dev] Query for missing function



On 5/31/17, 6:07 PM, "王志克"  wrote:

Hi,

See my reply in line. Thanks.

Br,
Wangzhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年6月1日 3:29
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: 答复: [ovs-dev] Query for missing function



On 5/26/17, 6:24 PM, "王志克"  wrote:

Hi Darrell,

I indeed observed IP fragment scenario in our other product 
deployment, and resulted some critical issue. Then I am
 wondering how to handle it in OVS+DPDK alternative solution.

[Darrell]
I am not sure what critical means here; it varies widely based on 
several considerations.

Maybe you just describe what the situation was and what happened.
Let me ask a few questions to draw that out.

Q1) Was that “other deployment” using kernel datapath and experienced a 
transient input of fragmented traffic from a Vxlan tunnel that the kernel could 
not handle due to lower throughput for fragmented traffic and/or kernel 
fragmentation thresholds used and ended up dropping those packets (possibly 
retried with delay) ?
Or something else ?
[Wangzhike] I observed Linux kernel panic when handling IP fragmented 
packets both from OVS and general IP stack, like Vxlan tunnel packets and the 
overlay packets are both fragmented. I am preparing patch for that.



Q2) Was the transient input of fragmented packets an intentional hack 
used to trash other traffic or just network misconfiguration or you have no 
clue ?
[Wangzhike] I believe it is kind of attack. Lots of uncompleted IP 
fragments were received.

Yes, fragmentation is good at driving forwarding performance down and/or 
increasing attack surface.
If one has control over the network boundaries, it can be avoided within 
those boundaries.
In other scenarios, the fragments get into a control volume because of 
others fault, intentional or otherwise.
If the fragments are legitimate and in large numbers for long enough, the 
misconfiguration must be fixed at source because they can’t be handled anyways 
in software.
Fragments in low numbers/less often provide less pressure to fix the 
misconfiguration, since it is what we can handle with software, assuming they 
are legitimate.

If OVS-DPDK adds support for IP fragmentation, it will be susceptible to 
the same issues the kernel datapath has.
I don’t know of a configurable, non-zero default fragmentation buffer that 
will work in all unforeseen legitimate cases.
Bigger fragmentation buffers are more susceptible to exploits.
A zero default configurable buffer size seems most clear and then let the 
user decide how much “IP Fragmentation”
they want.

Thoughts ? 



Typical cases are:
1) VxLan segmented packet reaches Vswitch and need to pop the VxLan 
header for further handling. In kernel OVS, normally Linux kernel will 
reassemble it before sending to OVS module. Since this happens in real world, 
we need to handle it though the possibility of happening is quite low.

[Darrell]
It is possible that fragmented packets arrive from a Vxlan tunnel – 
that is obvious.


2) Segmented packets go through conntrack. In kernel OVS, it 
will reuse Kernel reassembly function to make reassembled packet go through 
conntrack.

[Darrell]
“2” is not a use case; it is simply a statement of what the kernel does 
when faced with fragmented packets, which is the topic of this thread.
[Wangzhike] Let me revise it. We set some rule on OVS DPDK conntrack, 
and one VM app sends large packet (eg 9600 size

Re: [ovs-dev] [PATCH v3 1/5] rstp: Init a recursive mutex for rstp.


> On Jun 1, 2017, at 7:01 AM, Ben Pfaff  wrote:
> 
> On Fri, May 19, 2017 at 12:20:39AM -0700, nickcooper-zhangtonghao wrote:
>> * This patch will be used for next patch. The 'rstp/show' command,
>> which uses the mutex, calls functions which also use the mutex.
>> We should init it as a recursive mutex.
>> 
>> * Because of recursive mutex, this patch remove the OVS_EXCLUDED in
>> list/rstp.[ch] files.
>> 
>> * Some rstp tests of OvS, which run with ovstest, does not run rstp_init
>> in the bridge_init. We should call rstp_init when creating the rstp
>> and stp also do that, otherwise tests fail.
>> 
>> Signed-off-by: nickcooper-zhangtonghao 
> 
> I think I'd prefer to add internal versions of functions that don't take
> the locks, instead.  Did you consider that approach?

The v4 patches has been submitted. We use the internal functions to implement 
the ‘rstp/show’. 


http://patchwork.ozlabs.org/patch/769478/ 

http://patchwork.ozlabs.org/patch/769476/ 

http://patchwork.ozlabs.org/patch/769477/ 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 1/3] rstp: Add rstp port name for human reading.

This patch is useful to debug rstp subsystem and log the
port name instead of port number. This patch will also
be used to display rstp info for next patches.

Signed-off-by: nickcooper-zhangtonghao 
Acked-by: Jarno Rajahalme 
---
 lib/rstp-common.h  |  1 +
 lib/rstp.c | 14 +-
 lib/rstp.h |  3 ++-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index 27e8079..c108232 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -262,6 +262,7 @@ struct rstp_port {
 struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
 struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
 void *aux OVS_GUARDED_BY(rstp_mutex);
+char *port_name;
 struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
 /*
  * MAC status parameters
diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..67e6912 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -751,6 +751,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
uint16_t port_number)
 }
 }
 
+static void
+rstp_port_set_port_name__(struct rstp_port *port, const char *name)
+OVS_REQUIRES(rstp_mutex)
+{
+free(port->port_name);
+port->port_name = xstrdup(name);
+}
+
 /* Converts the link speed to a port path cost [Table 17-3]. */
 uint32_t
 rstp_convert_speed_to_cost(unsigned int speed)
@@ -1164,6 +1172,7 @@ rstp_add_port(struct rstp *rstp)
 rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
 rstp_port_set_port_number__(p, 0);
 p->aux = NULL;
+p->port_name = NULL;
 rstp_initialize_port_defaults__(p);
 VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
  p->port_id);
@@ -1201,6 +1210,7 @@ rstp_port_unref(struct rstp_port *rp)
 ovs_mutex_lock(_mutex);
 rstp = rp->rstp;
 rstp_port_set_state__(rp, RSTP_DISABLED);
+free(rp->port_name);
 hmap_remove(>ports, >node);
 VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
  rp->port_id);
@@ -1439,13 +1449,15 @@ void
 rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
-  bool admin_port_state, bool do_mcheck, void *aux)
+  bool admin_port_state, bool do_mcheck, void *aux,
+  const char *name)
 OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 port->aux = aux;
 rstp_port_set_priority__(port, priority);
 rstp_port_set_port_number__(port, port_num);
+rstp_port_set_port_name__(port, name);
 rstp_port_set_path_cost__(port, path_cost);
 rstp_port_set_admin_edge__(port, is_admin_edge);
 rstp_port_set_auto_edge__(port, is_auto_edge);
diff --git a/lib/rstp.h b/lib/rstp.h
index 4942d59..5213f98 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -227,7 +227,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
 void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
enum rstp_admin_point_to_point_mac_state 
admin_p2p_mac_state,
-   bool admin_port_state, bool do_mcheck, void *aux)
+   bool admin_port_state, bool do_mcheck, void *aux,
+   const char *name)
 OVS_EXCLUDED(rstp_mutex);
 
 enum rstp_state rstp_port_get_state(const struct rstp_port *)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5d247e3..231b5b0 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2761,7 +2761,7 @@ set_rstp_port(struct ofport *ofport_,
 rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
   s->admin_edge_port, s->auto_edge,
   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
-  ofport);
+  ofport, netdev_get_name(ofport->up.netdev));
 update_rstp_port_state(ofport);
 /* Synchronize operational status. */
 rstp_port_set_mac_operational(rp, ofport->may_enable);
-- 
1.8.3.1



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 3/3] rstp: Add the 'ovs-appctl rstp/show' command.

The rstp/show command will help users and developers to
get more details about rstp. This patch works together with
the previous patches.

Signed-off-by: nickcooper-zhangtonghao 
---
 NEWS   |   3 +-
 lib/rstp.c | 107 +
 vswitchd/ovs-vswitchd.8.in |  11 -
 3 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index dd39d2a..e68d465 100644
--- a/NEWS
+++ b/NEWS
@@ -36,7 +36,8 @@ Post-v2.7.0
abbreviated to 4 hex digits.
  * "ovn-sbctl lflow-list" can now print OpenFlow flows that correspond
to logical flows.
-   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
+   - Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
+ (see ovs-vswitchd(8)).
- OpenFlow:
  * All features required by OpenFlow 1.4 are now implemented, so
ovs-vswitchd now enables OpenFlow 1.4 by default (in addition to
diff --git a/lib/rstp.c b/lib/rstp.c
index 4455c4a..e6b04f9 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -130,6 +130,8 @@ static rstp_identifier rstp_get_root_id__(const struct rstp 
*rstp)
 OVS_REQUIRES(rstp_mutex);
 static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
  const char *argv[], void *aux);
+static void rstp_unixctl_show(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -248,6 +250,8 @@ rstp_init(void)
 {
 unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
  NULL);
+unixctl_command_register("rstp/show", "[bridge]", 0, 1, rstp_unixctl_show,
+ NULL);
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -1573,3 +1577,106 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock(_mutex);
 }
+
+static void
+rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
+   const uint16_t hello_time, const uint16_t max_age,
+   const uint16_t forward_delay)
+OVS_REQUIRES(rstp_mutex)
+{
+uint16_t priority = bridge_id >> 48;
+ds_put_format(ds, "\tstp-priority\t%"PRIu16"\n", priority);
+
+struct eth_addr mac;
+const uint64_t mac_bits = (UINT64_C(1) << 48) - 1;
+eth_addr_from_uint64(bridge_id & mac_bits, );
+ds_put_format(ds, "\tstp-system-id\t"ETH_ADDR_FMT"\n", ETH_ADDR_ARGS(mac));
+ds_put_format(ds, "\tstp-hello-time\t%"PRIu16"s\n", hello_time);
+ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
+}
+
+static void
+rstp_print_details(struct ds *ds, const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+ds_put_format(ds, " %s \n", rstp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+bool is_root = rstp_is_root_bridge__(rstp);
+struct rstp_port *p = rstp_get_root_port__(rstp);
+
+rstp_identifier bridge_id =
+is_root ? rstp->bridge_identifier : rstp_get_root_id__(rstp);
+uint16_t hello_time =
+is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
+uint16_t max_age =
+is_root ? rstp->bridge_max_age : p->designated_times.max_age;
+uint16_t forward_delay =
+is_root ? rstp->bridge_forward_delay : 
p->designated_times.forward_delay;
+
+rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, forward_delay);
+if (is_root) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n",
+  rstp_get_root_path_cost__(rstp));
+}
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+rstp_bridge_id_details(ds, rstp->bridge_identifier,
+   rstp->bridge_hello_time,
+   rstp->bridge_max_age,
+   rstp->bridge_forward_delay);
+ds_put_cstr(ds, "\n");
+
+ds_put_format(ds, "\t%-11.10s%-11.10s%-11.10s%-9.8s%-8.7s\n",
+  "Interface", "Role", "State", "Cost", "Pri.Nbr");
+ds_put_cstr(ds, "\t-- -- --  ---\n");
+HMAP_FOR_EACH (p, node, >ports) {
+if (p->rstp_state != RSTP_DISABLED) {
+ds_put_format(ds, "\t%-11.10s",
+  p->port_name ? p->port_name : "null");
+ds_put_format(ds, "%-11.10s", rstp_port_role_name(p->role));
+ds_put_format(ds, "%-11.10s", rstp_state_name(p->rstp_state));
+ds_put_format(ds, "%-9d", p->port_path_cost);
+ds_put_format(ds, "%d.%d\n", p->priority, p->port_number);
+}
+}
+
+ds_put_cstr(ds, "\n");
+}
+
+static void
+rstp_unixctl_show(struct unixctl_conn *conn, int argc,
+  const

[ovs-dev] [PATCH v4 2/3] rstp: Add internal functions without locks.

This patch adds some internal functions which
does not use the locks. This patch is used for
next patch.

Signed-off-by: nickcooper-zhangtonghao 
---
 lib/rstp.c | 63 +-
 lib/rstp.h |  2 +-
 2 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 67e6912..4455c4a 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -120,6 +120,16 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
bool mcheck)
 OVS_REQUIRES(rstp_mutex);
 static void reinitialize_port__(struct rstp_port *p)
 OVS_REQUIRES(rstp_mutex);
+static bool rstp_is_root_bridge__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static uint32_t rstp_get_root_path_cost__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static struct rstp_port *rstp_get_root_port__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static rstp_identifier rstp_get_root_id__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -208,9 +218,6 @@ rstp_port_get_number(const struct rstp_port *p)
 return number;
 }
 
-static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
- const char *argv[], void *aux);
-
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
@@ -796,6 +803,13 @@ rstp_port_set_path_cost__(struct rstp_port *port, uint32_t 
path_cost)
 }
 
 /* Gets the root path cost. */
+static uint32_t
+rstp_get_root_path_cost__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+return rstp->root_priority.root_path_cost;
+}
+
 uint32_t
 rstp_get_root_path_cost(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
@@ -803,7 +817,7 @@ rstp_get_root_path_cost(const struct rstp *rstp)
 uint32_t cost;
 
 ovs_mutex_lock(_mutex);
-cost = rstp->root_priority.root_path_cost;
+cost = rstp_get_root_path_cost__(rstp);
 ovs_mutex_unlock(_mutex);
 return cost;
 }
@@ -1313,6 +1327,13 @@ rstp_get_designated_id(const struct rstp *rstp)
 }
 
 /* Returns the root bridge id. */
+static rstp_identifier
+rstp_get_root_id__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+return rstp->root_priority.root_bridge_id;
+}
+
 rstp_identifier
 rstp_get_root_id(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
@@ -1320,7 +1341,7 @@ rstp_get_root_id(const struct rstp *rstp)
 rstp_identifier root_id;
 
 ovs_mutex_lock(_mutex);
-root_id = rstp->root_priority.root_bridge_id;
+root_id = rstp_get_root_id__(rstp);
 ovs_mutex_unlock(_mutex);
 
 return root_id;
@@ -1357,6 +1378,14 @@ rstp_get_bridge_port_id(const struct rstp *rstp)
 /* Returns true if the bridge believes to the be root of the spanning tree,
  * false otherwise.
  */
+static bool
+rstp_is_root_bridge__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+return rstp->bridge_identifier ==
+rstp->root_priority.designated_bridge_id;
+}
+
 bool
 rstp_is_root_bridge(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
@@ -1364,8 +1393,7 @@ rstp_is_root_bridge(const struct rstp *rstp)
 bool is_root;
 
 ovs_mutex_lock(_mutex);
-is_root = rstp->bridge_identifier ==
-rstp->root_priority.designated_bridge_id;
+is_root = rstp_is_root_bridge__(rstp);
 ovs_mutex_unlock(_mutex);
 
 return is_root;
@@ -1388,23 +1416,32 @@ rstp_get_designated_root(const struct rstp *rstp)
 /* Returns the port connecting 'rstp' to the root bridge, or a null pointer if
  * there is no such port.
  */
-struct rstp_port *
-rstp_get_root_port(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
+static struct rstp_port *
+rstp_get_root_port__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
 {
 struct rstp_port *p;
 
-ovs_mutex_lock(_mutex);
 HMAP_FOR_EACH (p, node, >ports) {
 if (p->port_id == rstp->root_port_id) {
-ovs_mutex_unlock(_mutex);
 return p;
 }
 }
-ovs_mutex_unlock(_mutex);
 return NULL;
 }
 
+struct rstp_port *
+rstp_get_root_port(const struct rstp *rstp)
+OVS_EXCLUDED(rstp_mutex)
+{
+struct rstp_port *p;
+
+ovs_mutex_lock(_mutex);
+p = rstp_get_root_port__(rstp);
+ovs_mutex_unlock(_mutex);
+return p;
+}
+
 /* Returns the state of port 'p'. */
 enum rstp_state
 rstp_port_get_state(const struct rstp_port *p)
diff --git a/lib/rstp.h b/lib/rstp.h
index 5213f98..39a13b5 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -207,7 +207,7 @@ uint16_t rstp_get_designated_port_id(const struct rstp *)
 OVS_EXCLUDED(rstp_mutex);
 uint16_t rstp_get_bridge_port_id(const struct rstp *)
 OVS_EXCLUDED(rstp_mutex);
-struct rstp_port * rstp_get_root_port(struct rstp *)
+struct rstp_port * rstp_get_root_port(const struct rstp *)
 OVS_EXCLUDED(rstp_mutex);

[ovs-dev] 答复: 答复: 答复: 答复: Query for missing function

2017-05-31 Thread 王志克

Hi Darrell,

In my opinion, it may be also hard for user to decide "configurable buffer 
size". Also I guess the default value should be to enable the fragment support. 
( If we give such option, I can imagine most user will enable it, right?)

My suggestion is to use Linux kernel as best practice.

Just my personal thought.

Br,
Wangzhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年6月1日 10:16
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: 答复: 答复: [ovs-dev] Query for missing function



On 5/31/17, 6:07 PM, "王志克"  wrote:

Hi,

See my reply in line. Thanks.

Br,
Wangzhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年6月1日 3:29
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: 答复: [ovs-dev] Query for missing function



On 5/26/17, 6:24 PM, "王志克"  wrote:

Hi Darrell,

I indeed observed IP fragment scenario in our other product deployment, 
and resulted some critical issue. Then I am
 wondering how to handle it in OVS+DPDK alternative solution.

[Darrell]
I am not sure what critical means here; it varies widely based on several 
considerations.

Maybe you just describe what the situation was and what happened.
Let me ask a few questions to draw that out.

Q1) Was that “other deployment” using kernel datapath and experienced a 
transient input of fragmented traffic from a Vxlan tunnel that the kernel could 
not handle due to lower throughput for fragmented traffic and/or kernel 
fragmentation thresholds used and ended up dropping those packets (possibly 
retried with delay) ?
Or something else ?
[Wangzhike] I observed Linux kernel panic when handling IP fragmented 
packets both from OVS and general IP stack, like Vxlan tunnel packets and the 
overlay packets are both fragmented. I am preparing patch for that.



Q2) Was the transient input of fragmented packets an intentional hack used 
to trash other traffic or just network misconfiguration or you have no clue ?
[Wangzhike] I believe it is kind of attack. Lots of uncompleted IP 
fragments were received.

Yes, fragmentation is good at driving forwarding performance down and/or 
increasing attack surface.
If one has control over the network boundaries, it can be avoided within those 
boundaries.
In other scenarios, the fragments get into a control volume because of others 
fault, intentional or otherwise.
If the fragments are legitimate and in large numbers for long enough, the 
misconfiguration must be fixed at source because they can’t be handled anyways 
in software.
Fragments in low numbers/less often provide less pressure to fix the 
misconfiguration, since it is what we can handle with software, assuming they 
are legitimate.

If OVS-DPDK adds support for IP fragmentation, it will be susceptible to the 
same issues the kernel datapath has.
I don’t know of a configurable, non-zero default fragmentation buffer that will 
work in all unforeseen legitimate cases.
Bigger fragmentation buffers are more susceptible to exploits.
A zero default configurable buffer size seems most clear and then let the user 
decide how much “IP Fragmentation”
they want.

Thoughts ? 



Typical cases are:
1) VxLan segmented packet reaches Vswitch and need to pop the VxLan header 
for further handling. In kernel OVS, normally Linux kernel will reassemble it 
before sending to OVS module. Since this happens in real world, we need to 
handle it though the possibility of happening is quite low.

[Darrell]
It is possible that fragmented packets arrive from a Vxlan tunnel – that is 
obvious.


2) Segmented packets go through conntrack. In kernel OVS, it will 
reuse Kernel reassembly function to make reassembled packet go through 
conntrack.

[Darrell]
“2” is not a use case; it is simply a statement of what the kernel does 
when faced with fragmented packets, which is the topic of this thread.
[Wangzhike] Let me revise it. We set some rule on OVS DPDK conntrack, and 
one VM app sends large packet (eg 9600 size while MTU is 1500). In this case, 
fragmented packet will reach OVS conntrack. We hope such packet can be handled 
as kernel ovs behavior(able to be reassembled before really go through 
conntrack) instead of tagging as INV state.

FYI, there are several configuration parameters that determine the actual 
behavior obtained. The effective behavior could even be that the kernel drops 
all these fragments.




Above cases really happen in current product deployment, and we want to 
keep it work when migrating to OVS+DPDK solution.

Br,
Wang Zhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年5月27日 2:45

[ovs-dev] 答复: [spam可疑邮件]Re: 答复: [spam可疑邮件]Re: [PATCH] ovn-northd: Fix ping failure of vlan networks.

2017-05-31 Thread wang . qianyu

Thanks for you rapidly reply.

We think localnet port never be the real destination port of vm instance. 
Like patch port of route, localnet port just used for interim.

And nouse of ct to localnet will not cause the bypass of firewall. Because 
of the real destination  port of vm1 or vm2 have their own ct.

The introducing of same port or same zone in different networks maybe not 
suitable, this is not consensus with the isolation of networks.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.



On 5/31/17, 4:36 PM, "ovs-dev-boun...@openvswitch.org on behalf of Darrell 
Ball"  wrote:

I did a pass through the changes.
Few comments

On 5/31/17, 6:09 AM, "ovs-dev-boun...@openvswitch.org on behalf of 
Kavanagh, Mark B"  wrote:


>From: ovs-dev-boun...@openvswitch.org 
[mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of
>Kevin Traynor
>Sent: Wednesday, May 31, 2017 1:53 PM
>To: Weglicki, MichalX ; 
d...@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add 
support for DPDK 17.05.
>
>On 05/31/2017 10:47 AM, mweglicx wrote:
>> Following changes are applied:
>> - netdev-dpdk: Changes required by DPDK API modifications.
>> - doc: Because of DPDK API changes, backward compatibility
>>   with previous DPDK releases will be broken, thus all
>>   relevant documentation entries are updated.
>> - .travis: DPDK version change from 16.11.1 to 17.05.
>> - rhel/openvswitch-fedora.spec.in: DPDK version change
>>   from 16.11 to 17.05.
>>
>> v1->v2: Patch rework based on minor review comments.


Hi Michal,

Apart from the changes suggested by Kevin, LGTM.

Thanks,
Mark


>>
>> Signed-off-by: Michal Weglicki 
>> ---
>>  .travis/linux-build.sh   |   2 +-
>>  Documentation/faq/releases.rst   |   3 +-
>>  Documentation/intro/install/dpdk.rst |   8 +--
>>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
>>  lib/netdev-dpdk.c| 114 
---
>>  rhel/openvswitch-fedora.spec.in  |   2 +-
>>  tests/dpdk/ring_client.c |   6 +-
>>  7 files changed, 75 insertions(+), 68 deletions(-)
>>
>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
>> index 8750d68..ec15fd8 100755
>> --- a/.travis/linux-build.sh
>> +++ b/.travis/linux-build.sh
>> @@ -80,7 +80,7 @@ fi
>>
>>  if [ "$DPDK" ]; then
>>  if [ -z "$DPDK_VER" ]; then
>> -DPDK_VER="16.11.1"
>> +DPDK_VER="17.05"
>>  fi
>>  install_dpdk $DPDK_VER
>>  if [ "$CC" = "clang" ]; then
>> diff --git a/Documentation/faq/releases.rst 
b/Documentation/faq/releases.rst
>> index c85eff8..0455a43 100644
>> --- a/Documentation/faq/releases.rst
>> +++ b/Documentation/faq/releases.rst
>> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch 
release work with?
>>  2.4.x2.0
>>  2.5.x2.2
>>  2.6.x16.07.2
>> -2.7.x16.11.1
>> +2.7.016.11.1 
>> +2.7.0+   17.05
>>   ===
>
>This is about OVS releases. I think you should revert back to the
>original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
>likely not use DPDK 17.05.

That's a good point - +1 on this.

Yep


>
>When OVS 2.8 is close to release it can be updated with it's DPDK
>version, or you could set it now and it can be changed later if 
required.
>
>>
>>  Q: I get an error like this when I configure Open vSwitch:
>> diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
>> index e83f852..536450b 100644
>> --- a/Documentation/intro/install/dpdk.rst
>> +++ b/Documentation/intro/install/dpdk.rst
>> @@ -40,7 +40,7 @@ Build requirements
>>  In addition to the requirements described in :doc:`general`, 
building Open
>>  vSwitch with DPDK will require the following:
>>
>> -- DPDK 16.11
>> +- DPDK 17.05
>>
>>  - A `DPDK supported NIC`_

Why is this comment removed ?

Oops, I forgot about the rst formatting; this is fine.


>>
>> @@ -69,9 +69,9 @@ Install DPDK
>>  #. Download the `DPDK sources`_, extract the file and set 
``DPDK_DIR``::
>>
>> $ cd /usr/src/
>> -   $ wget 
https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D16.11.1.tar.xz=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs=
 
>> -   $ tar xf dpdk-16.11.1.tar.xz
>> -   $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
>> +   $

Re: [ovs-dev] [PATCH v1] netdev-linux: Refactoring the netdev_linux_send in forwarding batch packets

2017-05-31 Thread Gao Zhenyu

well, ovs-dpdk + userspace-tcp + app should be a better way. I just made
this test so we can have a conclusion that ovs-dpdk + veth is not a good
choice. And it worse than regular ovs.

I will do more performance testings on latest openvswitch and send a patch
out ASAP.


Thanks
Zhenyu Gao

2017-06-01 10:16 GMT+08:00 Ben Pfaff :

> Are you sure that this is the fastest way to interface OVS-DPDK to a
> container?  But, even if it is not, optimizations are welcome.
>
> On Thu, Jun 01, 2017 at 09:50:44AM +0800, Gao Zhenyu wrote:
> > Here is the backgroud:
> >
> > I tried to consume ovs-dpdk(ovs-2.7.0, dpdk-16.11) for container-app and
> > here is the topologic
> >
> >
> > netserver
> > |---|
> > |  |
> > |  container   |
> > |veth|
> >   |
> >   |  ||
> >   |---veth-|   dpdk-ovs
> > |   netperf
> >  |
> > |   |---|
> >
> >  |-dpdk|   |
> > bare-metal   |
> >
> > |
> > -
> >
> > |   |
> >
> > |   |
> >
> > physical-nicphysical-nic
> >
> > But the performance is worse than regular OVS and then I found sendmsg
> cost
> > 96% cpu cycles.  Some packets were dropped due to insufficient cpu.
> > So I tried to replace sendmsg with sendmmsg, it shows some performance
> > improvement like:
> >
> > netperf -H 10.100.85.242 -t TCP_STREAM -l 60
> > 335.98Mb(sendmsg + ovs-dpdk)   >   663Mb(sendmmsg + ovs-dpdk)
> >
> > (I turn off the veth's tx offloading because the dpdk would not do
> > tx-checksum which introduces tcp-checksum error.   ethtool -K eth1 tx
> off)
> >
> >
> > Thanks
> > Zhenyu Gao
> >
> >
> > 2017-05-31 23:41 GMT+08:00 Ben Pfaff :
> >
> > > On Wed, May 31, 2017 at 09:50:09AM +0800, Gao Zhenyu wrote:
> > > > BTW, I would like to submit another patch to use sendmmsg to replace
> > > > sendmsg. Sendmmsg get more benefit on throughput(my draft testing
> show
> > > 100%
> > > > improvment). Do you think it is doable?
> > >
> > > I'm surprised that it makes a big difference, because I tested a
> similar
> > > change years ago and it did not.  However, let's assume that it does.
> > > In that case, of course we'd accept a change.  It would be important to
> > > retain support for older kernels and non-Linux kernels.
> > >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 答复: [spam可疑邮件]Re: [PATCH] ovn-northd: Fix ping failure of vlan networks.

Should we fix the problem another way, by introducing a way to make sure
that the same port is used, or at least that the same zone is used?  It
does not sound like a good idea to simply bypass the firewall.

On Thu, Jun 01, 2017 at 09:22:24AM +0800, wang.qia...@zte.com.cn wrote:
> Hi Ben, thanks for your review.
> 
> Conntrack have no problem with localnet port, but the pipline hase problem 
> in the follow circumstance
> 
> --   vlan  
> |ovs1|--  |ovs2| 
> ---
>   | |
>  vm1   vm2
> 
> net1 10.0.0.0/24 has vm1 with ip 10.0.0.10, net2 10.0.0.0/24 has vm2 with 
> ip 20.0.0.10. net1 and net2 link to same route. net1 and net2 have 
> localnet ports as inport/outport when packet forwarded between ovs1 and 
> ovs2. 
> 
> when vm1 ping vm2, by the route forward, the out port of icmp request is 
> localnet port of net2 in ovs1. And in reverse, ovs1 will use localnet port 
> of net1 as inport of icmp reply from vm2.
> 
> The request and reply is not the same localnet port in ovs. Because of 
> different localnet port with different zone id, when localnet port use ct, 
> the ct state can not change to established.
> 
> So the icmp relpy will be dropped because of the error ct state.
> 
> 
> 
> 
> 
> Ben Pfaff 
> 2017/06/01 07:42
>  
> 收件人：wang.qia...@zte.com.cn, 
> 抄送：  d...@openvswitch.org, zhou.huij...@zte.com.cn, 
> xu.r...@zte.com.cn
> 主题：  [spam可疑邮件]Re: [ovs-dev]  [PATCH] ovn-northd: Fix ping 
> failure of vlan networks.
> 
> 
> On Mon, May 22, 2017 at 07:39:22PM +0800, wang.qia...@zte.com.cn wrote:
> > There are two computer node, each have one vm. And the two vms in 
> > indifferent vlan networks. The ping between the vms is not success.
> > 
> > The reason is that, acl of to-localnet port or from-localnet port is
> > signed to contrack. So the pair of icmp request and reply have different
> > zone id in one ovs node. This makes the ct state not correct.
> > 
> > This patch do the modification that localnet port do not use ct.
> > 
> > Signed-off-by: wangqianyu 
> 
> This patch was word-wrapped, but I was able to deal with that.
> 
> I don't exactly understand the problem.  Does conntrack not work at all
> with packets that go to/from localnet ports?  Or does it have something
> to do with VLAN tags?
> 
> Please document the new flows in ovn-northd.8.xml.
> 
> Also, checkpatch reported the following:
> 
> ERROR: Improper whitespace around control block
> #17 FILE: b/ovn/northd/ovn-northd.c:1355:
> if(!strcmp(nbsp->type, "localnet")) {
> 
> ERROR: Improper whitespace around control block
> #28 FILE: b/ovn/northd/ovn-northd.c:2637:
> if(od->localnet_port) {
> 
> WARNING: Line length is >79-characters long
> #32 FILE: b/ovn/northd/ovn-northd.c:2641:
> ds_put_format(_in, "ip && inport == %s", 
> od->localnet_port->json_key);
> 
> WARNING: Line length is >79-characters long
> #33 FILE: b/ovn/northd/ovn-northd.c:2642:
> ds_put_format(_out, "ip && outport == %s", 
> od->localnet_port->json_key);
> 
> Thanks a lot for working on OVN!
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 答复: 答复: 答复: Query for missing function



On 5/31/17, 6:07 PM, "王志克"  wrote:

Hi,

See my reply in line. Thanks.

Br,
Wangzhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年6月1日 3:29
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: 答复: [ovs-dev] Query for missing function



On 5/26/17, 6:24 PM, "王志克"  wrote:

Hi Darrell,

I indeed observed IP fragment scenario in our other product deployment, 
and resulted some critical issue. Then I am
 wondering how to handle it in OVS+DPDK alternative solution.

[Darrell]
I am not sure what critical means here; it varies widely based on several 
considerations.

Maybe you just describe what the situation was and what happened.
Let me ask a few questions to draw that out.

Q1) Was that “other deployment” using kernel datapath and experienced a 
transient input of fragmented traffic from a Vxlan tunnel that the kernel could 
not handle due to lower throughput for fragmented traffic and/or kernel 
fragmentation thresholds used and ended up dropping those packets (possibly 
retried with delay) ?
Or something else ?
[Wangzhike] I observed Linux kernel panic when handling IP fragmented 
packets both from OVS and general IP stack, like Vxlan tunnel packets and the 
overlay packets are both fragmented. I am preparing patch for that.



Q2) Was the transient input of fragmented packets an intentional hack used 
to trash other traffic or just network misconfiguration or you have no clue ?
[Wangzhike] I believe it is kind of attack. Lots of uncompleted IP 
fragments were received.

Yes, fragmentation is good at driving forwarding performance down and/or 
increasing attack surface.
If one has control over the network boundaries, it can be avoided within those 
boundaries.
In other scenarios, the fragments get into a control volume because of others 
fault, intentional or otherwise.
If the fragments are legitimate and in large numbers for long enough, the 
misconfiguration must be fixed at 
source because they can’t be handled anyways in software.
Fragments in low numbers/less often provide less pressure to fix the 
misconfiguration, since it is what we
can handle with software, assuming they are legitimate.

If OVS-DPDK adds support for IP fragmentation, it will be susceptible to the 
same issues the kernel datapath has.
I don’t know of a configurable, non-zero default fragmentation buffer that will 
work in all unforeseen legitimate cases.
Bigger fragmentation buffers are more susceptible to exploits.
A zero default configurable buffer size seems most clear and then let the user 
decide how much “IP Fragmentation”
they want.

Thoughts ? 



Typical cases are:
1) VxLan segmented packet reaches Vswitch and need to pop the VxLan header 
for further handling. In kernel OVS, normally Linux kernel will reassemble it 
before sending to OVS module. Since this happens in real world, we need to 
handle it though the possibility of happening is quite low.

[Darrell]
It is possible that fragmented packets arrive from a Vxlan tunnel – that is 
obvious.


2) Segmented packets go through conntrack. In kernel OVS, it will 
reuse Kernel reassembly function to make reassembled packet go through 
conntrack.

[Darrell]
“2” is not a use case; it is simply a statement of what the kernel does 
when faced with fragmented packets, which is the topic of this thread.
[Wangzhike] Let me revise it. We set some rule on OVS DPDK conntrack, and 
one VM app sends large packet (eg 9600 size while MTU is 1500). In this case, 
fragmented packet will reach OVS conntrack. We hope such packet can be handled 
as kernel ovs behavior(able to be reassembled before really go through 
conntrack) instead of tagging as INV state.

FYI, there are several configuration parameters that determine the actual 
behavior obtained. The effective behavior could even be that the kernel drops 
all these fragments.




Above cases really happen in current product deployment, and we want to 
keep it work when migrating to OVS+DPDK solution.

Br,
Wang Zhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com] 
发送时间: 2017年5月27日 2:45
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: [ovs-dev] Query for missing function



On 5/26/17, 2:00 AM, "王志克"  wrote:

Hi Darrell, Ben,

Thanks for your reply. Glad to hear that we are approaching useful 
candidate patch.

What is the plan for disassemble and fragment for OVS+DPDK? Like
   1, received underlay vxlan fragmented packets,
   2, received

Re: [ovs-dev] [PATCH v2 1/3] windows: add definition of getpid and getcwd

On Fri, May 19, 2017 at 11:16:16PM +, Alin Serdean wrote:
> > >  #define WIN32_LEAN_AND_MEAN
> > > +#include 
> > >  #include 
> > > +#include 
> > 
> > Thanks for the revised patch.
> > 
> > Does #include  make a difference?  Every .c file should already
> > start out with that #include, and so if it makes a difference then it 
> > probably
> > indicates that some .c file has forgotten it.  (But the Makefile checks for 
> > that,
> > so it is unlikely.)
> > 
> > Thanks,
> > 
> > Ben.
> [Alin Serdean] I did a clean compile and a run of unit tests and everything 
> was ok.
> I included  for 
> https://github.com/openvswitch/ovs/blob/master/include/windows/windefs.h#L41 .
> Should I change  to  ?

It looks like config.h always include windefs.h, on Windows.  Every .c
file includes config.h.  Thus, there should be no need for anything else
to ever include config.h or windefs.h.  Right?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Fwd: [PATCH] bfd: Detect Multiplier configuration

On Mon, May 22, 2017 at 01:51:17PM +0200, Gabor Szucs wrote:
> Hi,
> hereby I'm sending an implementation of configurable BFD Detect Multiplier.
> 
> Mult value (bfd.DetectMult in RFC5880) is hard-coded and equal to 3 in
> current openvswitch. As a consequence remote and local mult is the same.

Thank you for working on Open vSwitch!

This patch does not apply.  Perhaps it was corrupted as part of
forwarding?  I suggest that you re-send it using "git send-email", which
always does it the right way.

"git am" said:

Applying: Fwd: [PATCH] bfd: Detect Multiplier configuration
error: patch failed: lib/bfd.c:146
error: lib/bfd.c: patch does not apply
error: patch failed: tests/bfd.at:1
error: tests/bfd.at: patch does not apply
error: patch failed: vswitchd/vswitch.xml:2917
error: vswitchd/vswitch.xml: patch does not apply
Patch failed at 0001 Fwd: [PATCH] bfd: Detect Multiplier configuration
The copy of the patch that failed is found in: .git/rebase-apply/patch
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: Use uint8_t for port_id.

On Fri, May 26, 2017 at 12:37:14PM +, Kavanagh, Mark B wrote:
> 
> >From: ovs-dev-boun...@openvswitch.org 
> >[mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of
> >Ilya Maximets
> >Sent: Friday, May 19, 2017 2:38 PM
> >To: d...@openvswitch.org; Daniele Di Proietto ; Darrell 
> >Ball
> >
> >Cc: Ilya Maximets ; Heetae Ahn 
> >
> >Subject: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: Use uint8_t for port_id.
> >
> >Currently, signed integer is used for 'port_id' variable and
> >'-1' as identifier of bad or uninitialized 'port_id'.
> >
> >This inconsistent with dpdk library and, also, in few cases,
> >leads to passing '-1' to dpdk functions where uint8_t expected.
> >
> >Such behaviour doesn't produce any issues, but it's better to
> >use same type as in dpdk library for consistency.
> >Introduced 'dpdk_port_t' typedef for better maintainability.
> >
> >Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
> >macro.
> >
> >Signed-off-by: Ilya Maximets 
> 
> Hi Ilya,
> 
> Darrell has pretty much covered any concerns that I had with these changes in 
> his comments on V3.
> 
> On initial reading of V4, I found 'dpdk_port_t' slightly non-intuitive; 
> however, having read your rationale for this typedef, and given that it 
> adheres to existing port ID conventions, I'm happy with this change.
> I also like the introduction of 'DPDK_ETH_PORT_ID_INVALID'.
> 
> I've also conducted the following sanity checks on the patch, and found no 
> issues:
>  - checkpatch.py
>  - compilation with clang
>  - compilation with gcc
>  - sparse
>  - make check (no additional tests fail after application of this patch)
> 
> Acked-by: Mark Kavanagh 

Thanks Ilya and Mark.

I'd tend to go for a new "DPDK_PORT_FMT" macro, like this:
#define DPDK_PORT_FMT PRIu8
to better abstract dpdk_port_t for use in printf() contexts.

However, this patch makes sense to me anyway, so I applied this to
master.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 2/3] netdev-dpdk: Fix device leak on port deletion.

On Fri, May 19, 2017 at 04:37:31PM +0300, Ilya Maximets wrote:
> Currently, once created device in dpdk will exist forever
> even after del-port operation untill we manually call
> 'ovs-appctl netdev-dpdk/detach ', where  is not
> the port's name but the name of dpdk eth device or pci address.

Thanks, I applied this patch to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 1/3] netdev-dpdk: Fix double attaching of virtual devices.

Thanks Ilya and Billy, I applied this to master.

On Fri, May 26, 2017 at 12:53:04PM +, O Mahony, Billy wrote:
> Tested-by: Billy O'Mahony 
> Acked-by: Billy O'Mahony 
> 
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Ilya Maximets
> > Sent: Friday, May 19, 2017 2:38 PM
> > To: d...@openvswitch.org; Daniele Di Proietto ;
> > Darrell Ball 
> > Cc: Ilya Maximets ; Heetae Ahn
> > 
> > Subject: [ovs-dev] [PATCH v4 1/3] netdev-dpdk: Fix double attaching of
> > virtual devices.
> > 
> > 'devargs' for virtual devices contains not only name but also a list of
> > arguments like this:
> > 
> > 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
> > or
> > 'eth_af_packet0,iface=eth0'
> > 
> > We must cut off the arguments from this string before calling
> > 'rte_eth_dev_get_port_by_name()' to avoid double attaching of the same
> > device.
> > 
> > CC: Ciara Loftus 
> > Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs
> > (vdevs)")
> > Signed-off-by: Ilya Maximets 
> > ---
> >  lib/netdev-dpdk.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 9941f88..1586e41
> > 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1115,10 +1115,12 @@ netdev_dpdk_lookup_by_port_id(int port_id)
> > static int  netdev_dpdk_process_devargs(const char *devargs, char **errp)
> > {
> > +/* Get the name up to the first comma. */
> > +char *name = xmemdup0(devargs, strcspn(devargs, ","));
> >  uint8_t new_port_id = UINT8_MAX;
> > 
> >  if (!rte_eth_dev_count()
> > -|| rte_eth_dev_get_port_by_name(devargs, _port_id)
> > +|| rte_eth_dev_get_port_by_name(name, _port_id)
> >  || !rte_eth_dev_is_valid_port(new_port_id)) {
> >  /* Device not found in DPDK, attempt to attach it */
> >  if (!rte_eth_dev_attach(devargs, _port_id)) { @@ -1128,10
> > +1130,11 @@ netdev_dpdk_process_devargs(const char *devargs, char
> > **errp)
> >  /* Attach unsuccessful */
> >  VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK",
> >devargs);
> > -return -1;
> > +new_port_id = UINT8_MAX;
> >  }
> >  }
> > 
> > +free(name);
> >  return new_port_id;
> >  }
> > 
> > --
> > 2.7.4
> > 
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/6] userspace: Packet type-aware pipeline

On Fri, May 19, 2017 at 09:23:40AM +, Zoltán Balogh wrote:
> 
> This patch set is the 2nd part of an initiative presented by Jan Scheurich: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332367.html
> 
> It takes the patch set referred by the link above as a bases.

Would you mind rebasing this (again)?  It has a lot of rejects,
unfortunately.  Thanks a lot!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] Support accepting and displaying port names in OVS tools.

On Thu, May 25, 2017 at 02:41:04PM -0700, Ben Pfaff wrote:
> On Thu, May 25, 2017 at 02:29:55PM -0400, Aaron Conole wrote:
> > Hi Ben,
> > 
> > Ben Pfaff  writes:
> > 
> > > Until now, most ovs-ofctl commands have not accepted names for ports, only
> > > numbers, and have not been able to display port names either.  It's a lot
> > > easier for users if they can use and see meaningful names instead of
> > > arbitrary numbers.  This commit adds that support.
> > >
> > > For backward compatibility, only interactive ovs-ofctl commands by default
> > > display port names; to display them in scripts, use the new --names
> > > option.
> > >
> > > Signed-off-by: Ben Pfaff 
> > > ---
> > 
> > Tested-by: Aaron Conole 
> > 
> > I suggest folding in the following (otherwise, I'm happy with it):
> 
> Thanks a lot for reviewing and testing!
> 
> Since this is a big change, I'm going to leave this out for comments
> from others for a while before I consider pushing it.

No one else had comments, so I applied this.  Thanks again!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] Replace most uses of and references to "ifconfig" by "ip".

2017-05-31 Thread Greg Rose

On Wed, 2017-05-31 at 14:32 -0700, Ben Pfaff wrote:
> On Wed, May 31, 2017 at 02:13:31PM -0700, Greg Rose wrote:
> > 

[snip]

> > They both seem to be broken SFAICT.
> > 
> > I'll work on fixing them up.
> 
> Thank you!

OK, user error on ovs-vlan-test.  I fixed that up and it is working as
per instructions.

[root@munda utilities]# ./ovs-vlan-test 192.168.1.202:8080 13.0.0.5
Send size 50 successful
Receive size 50 successful
Send size 500 successful
Receive size 500 successful
Send size 1000 successful
Receive size 1000 successful
Send size 1500 successful
Receive size 1500 successful
OK

The ovs-test utility isn't finding some packages.  Let me investigate
that.  Perhaps just another user error.

Thanks,

- Greg

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 3/3] ovn-sbctl: support setting rbac role for remote connections

Add support for specifying rbac "role" when setting remote
connection configuration in the southbound database.

Prior to this change, usage examples included:

ovn-sbctl set-connection ptcp:6642
ovn-sbctl set-connection pssl:6642 \
 read-only ptcp: \
 read-write punix:/tmp.foo

With this change, in addition to the above:

ovn-sbctl set-connection role=ovn-controller pssl:6642 \
 read-only role= ptcp: \
 read-write punix:/tmp/foo

As with the "read-only"/"read-write" attributes, the specified
role is applied to all subsequent connections until changed.

Signed-off-by: Lance Richardson 
---
v3: No changes.
v2: No changes.

 ovn/utilities/ovn-sbctl.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index 4a88423..4301971 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -943,6 +943,7 @@ pre_connection(struct ctl_context *ctx)
 ovsdb_idl_add_column(ctx->idl, _sb_global_col_connections);
 ovsdb_idl_add_column(ctx->idl, _connection_col_target);
 ovsdb_idl_add_column(ctx->idl, _connection_col_read_only);
+ovsdb_idl_add_column(ctx->idl, _connection_col_role);
 }
 
 static void
@@ -960,8 +961,10 @@ cmd_get_connection(struct ctl_context *ctx)
 SBREC_CONNECTION_FOR_EACH(conn, ctx->idl) {
 char *s;
 
-s = xasprintf("%s %s", conn->read_only ? "read-only" : "read-write",
-   conn->target);
+s = xasprintf("%s role=\"%s\" %s",
+  conn->read_only ? "read-only" : "read-write",
+  conn->role,
+  conn->target);
 svec_add(, s);
 free(s);
 }
@@ -1002,6 +1005,7 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 struct sbrec_connection **connections;
 size_t i, conns=0;
 bool read_only = false;
+char *role = "";
 
 /* Insert each connection in a new row in Connection table. */
 connections = xmalloc(n * sizeof *connections);
@@ -1012,6 +1016,9 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 } else if (!strcmp(targets[i], "read-write")) {
 read_only = false;
 continue;
+} else if (!strncmp(targets[i], "role=", 5)) {
+role = targets[i] + 5;
+continue;
 } else if (stream_verify_name(targets[i]) &&
pstream_verify_name(targets[i])) {
 VLOG_WARN("target type \"%s\" is possibly erroneous", targets[i]);
@@ -1020,6 +1027,7 @@ insert_connections(struct ctl_context *ctx, char 
*targets[], size_t n)
 connections[conns] = sbrec_connection_insert(ctx->txn);
 sbrec_connection_set_target(connections[conns], targets[i]);
 sbrec_connection_set_read_only(connections[conns], read_only);
+sbrec_connection_set_role(connections[conns], role);
 conns++;
 }
 
-- 
2.9.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 2/3] ovn: add rbac tables to ovn southbound schema

Add rbac "roles" and "permissions" tables to ovn southbound
database schema, add support to ovn-northd for managing these
tables.

Signed-off-by: Lance Richardson 
---
 v3: - Rebased and fixed conflicts in ovn-sb.ovsschema.
 - Updated ovn-architecture(7) to say authorization checks apply
   to row insertion.
 - Changed initialization for rbac_perm_cfg[] to fix a travis-ci
   compilation warning.
 v2: - Rebased, added reference to ovsdb-server(1) to ovn-architecture(7).

 ovn/northd/ovn-northd.c| 197 +
 ovn/ovn-architecture.7.xml | 156 +++
 ovn/ovn-sb.ovsschema   |  26 +-
 ovn/ovn-sb.xml |  40 +
 4 files changed, 417 insertions(+), 2 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 5914988..965bc80 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -5804,6 +5804,189 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct 
northd_context *ctx)
 hmap_destroy(_opts_to_add);
 }
 
+static const char *rbac_chassis_auth[] =
+{"name"};
+static const char *rbac_chassis_update[] =
+{"nb_cfg", "external_ids", "encaps", "vtep_logical_switches"};
+
+static const char *rbac_encap_auth[] =
+{""};
+static const char *rbac_encap_update[] =
+{"type", "options", "ip"};
+
+static const char *rbac_port_binding_auth[] =
+{""};
+static const char *rbac_port_binding_update[] =
+{"chassis"};
+
+static const char *rbac_mac_binding_auth[] =
+{""};
+static const char *rbac_mac_binding_update[] =
+{"logical_port", "ip", "mac", "datapath"};
+
+static struct rbac_perm_cfg {
+const char *table;
+const char **auth;
+int n_auth;
+bool insdel;
+const char **update;
+int n_update;
+const struct sbrec_rbac_permission *row;
+} rbac_perm_cfg[] = {
+{
+.table = "Chassis",
+.auth = rbac_chassis_auth,
+.n_auth = ARRAY_SIZE(rbac_chassis_auth),
+.insdel = true,
+.update = rbac_chassis_update,
+.n_update = ARRAY_SIZE(rbac_chassis_update),
+.row = NULL
+},{
+.table = "Encap",
+.auth = rbac_encap_auth,
+.n_auth = ARRAY_SIZE(rbac_encap_auth),
+.insdel = true,
+.update = rbac_encap_update,
+.n_update = ARRAY_SIZE(rbac_encap_update),
+.row = NULL
+},{
+.table = "Port_Binding",
+.auth = rbac_port_binding_auth,
+.n_auth = ARRAY_SIZE(rbac_port_binding_auth),
+.insdel = false,
+.update = rbac_port_binding_update,
+.n_update = ARRAY_SIZE(rbac_port_binding_update),
+.row = NULL
+},{
+.table = "MAC_Binding",
+.auth = rbac_mac_binding_auth,
+.n_auth = ARRAY_SIZE(rbac_mac_binding_auth),
+.insdel = true,
+.update = rbac_mac_binding_update,
+.n_update = ARRAY_SIZE(rbac_mac_binding_update),
+.row = NULL
+},{
+.table = NULL,
+.auth = NULL,
+.n_auth = 0,
+.insdel = false,
+.update = NULL,
+.n_update = 0,
+.row = NULL
+}
+};
+
+static bool
+ovn_rbac_validate_perm(const struct sbrec_rbac_permission *perm)
+{
+struct rbac_perm_cfg *pcfg;
+int i, j, n_found;
+
+for (pcfg = rbac_perm_cfg; pcfg->table; pcfg++) {
+if (!strcmp(perm->table, pcfg->table)) {
+break;
+}
+}
+if (!pcfg->table) {
+return false;
+}
+if (perm->n_authorization != pcfg->n_auth ||
+perm->n_update != pcfg->n_update) {
+return false;
+}
+if (perm->insert_delete != pcfg->insdel) {
+return false;
+}
+/* verify perm->authorization vs. pcfg->auth */
+n_found = 0;
+for (i = 0; i < pcfg->n_auth; i++) {
+for (j = 0; j < perm->n_authorization; j++) {
+if (!strcmp(pcfg->auth[i], perm->authorization[j])) {
+n_found++;
+break;
+}
+}
+}
+if (n_found != pcfg->n_auth) {
+return false;
+}
+
+/* verify perm->update vs. pcfg->update */
+n_found = 0;
+for (i = 0; i < pcfg->n_update; i++) {
+for (j = 0; j < perm->n_update; j++) {
+if (!strcmp(pcfg->update[i], perm->update[j])) {
+n_found++;
+break;
+}
+}
+}
+if (n_found != pcfg->n_update) {
+return false;
+}
+
+/* Success, db state matches expected state */
+pcfg->row = perm;
+return true;
+}
+
+static void
+ovn_rbac_create_perm(struct rbac_perm_cfg *pcfg,
+ struct northd_context *ctx,
+ const struct sbrec_rbac_role *rbac_role)
+{
+struct sbrec_rbac_permission *rbac_perm;
+
+rbac_perm = sbrec_rbac_permission_insert(ctx->ovnsb_txn);
+sbrec_rbac_permission_set_table(rbac_perm, pcfg->table);
+sbrec_rbac_permission_set_authorization(rbac_perm,
+

[ovs-dev] [PATCH v3 1/3] ovsdb: add support for role-based access controls

Add suport for ovsdb RBAC (role-based access control). This includes:

   - Support for "RBAC_Role" table. A db schema containing a table
 by this name will enable role-based access controls using
 this table for RBAC role configuration.

 The "RBAC_Role" table has one row per role, with each row having a
 "name" column (role name) and a "permissions" column (map of
 table name to UUID of row in separate permission table.) The
 permission table has one row per access control configuration,
 with the following columns:
  "name"  - name of table to which this row applies
  "authorization" - set of column names and column:key pairs
to be compared against client ID to
determine authorization status
  "insert_delete" - boolean, true if insertions and
authorized deletions are allowed.
  "update"- Set of columns and column:key pairs for
which authorized updates are allowed.
   - Support for a new "role" column in the remote configuration
 table.
   - Logic for applying the RBAC role and permission tables, in
 combination with session role from the remote connection table
 and client id, to determine whether operations modifying database
 contents should be permitted.
   - Support for specifying RBAC role string as a command-line option
 to ovsdb-tool (Ben Pfaff).

Signed-off-by: Lance Richardson 
Co-authored-by: Ben Pfaff 
---
v3:
  - Row insertion RBAC now enforces authorization criteria.
  - Added a test case for row insertion with client ID/auth
mismatch.
  - Updated documentation (ovsdb-server(1)) to indicate that
authorization checks are applied for row insertion.
v2:
  - Folded in incremental patch from Ben Pfaff with style cleanup, a
bug fix, and addition of --rbac-role option to ovsdb-tool.
  - Documented differences with respect to RFC 7074 in ovsdb-server(1).

 lib/jsonrpc.c   |  10 ++
 lib/jsonrpc.h   |   1 +
 lib/ovsdb-error.c   |  13 ++
 lib/ovsdb-error.h   |   4 +
 lib/ovsdb-idl.c |   6 +
 ovsdb/automake.mk   |   2 +
 ovsdb/execution.c   |  44 -
 ovsdb/jsonrpc-server.c  |   6 +-
 ovsdb/jsonrpc-server.h  |   1 +
 ovsdb/ovsdb-server.1.in |  45 +
 ovsdb/ovsdb-server.c|   8 +-
 ovsdb/ovsdb-tool.1.in   |  10 +-
 ovsdb/ovsdb-tool.c  |  23 ++-
 ovsdb/ovsdb-util.c  |  31 
 ovsdb/ovsdb-util.h  |   4 +
 ovsdb/ovsdb.c   |   3 +
 ovsdb/ovsdb.h   |   3 +
 ovsdb/rbac.c| 449 
 ovsdb/rbac.h|  48 ++
 ovsdb/trigger.c |   8 +-
 ovsdb/trigger.h |   5 +-
 tests/automake.mk   |   1 +
 tests/ovsdb-rbac.at | 375 
 tests/ovsdb.at  |   1 +
 tests/test-ovsdb.c  |   5 +-
 25 files changed, 1090 insertions(+), 16 deletions(-)
 create mode 100644 ovsdb/rbac.c
 create mode 100644 ovsdb/rbac.h
 create mode 100644 tests/ovsdb-rbac.at

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index a0ade9c..2fae057 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1005,6 +1005,16 @@ jsonrpc_session_get_name(const struct jsonrpc_session *s)
 return reconnect_get_name(s->reconnect);
 }
 
+const char *
+jsonrpc_session_get_id(const struct jsonrpc_session *s)
+{
+if (s->rpc && s->rpc->stream) {
+return stream_get_peer_id(s->rpc->stream);
+} else {
+return NULL;
+}
+}
+
 /* Always takes ownership of 'msg', regardless of success. */
 int
 jsonrpc_session_send(struct jsonrpc_session *s, struct jsonrpc_msg *msg)
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index 982017a..9b4fb0e 100644
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -130,5 +130,6 @@ void jsonrpc_session_set_probe_interval(struct 
jsonrpc_session *,
 int probe_interval);
 void jsonrpc_session_set_dscp(struct jsonrpc_session *,
   uint8_t dscp);
+const char *jsonrpc_session_get_id(const struct jsonrpc_session *);
 
 #endif /* jsonrpc.h */
diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
index dfa4249..d8161e6 100644
--- a/lib/ovsdb-error.c
+++ b/lib/ovsdb-error.c
@@ -167,6 +167,19 @@ ovsdb_internal_error(struct ovsdb_error *inner_error,
 return error;
 }
 
+struct ovsdb_error *
+ovsdb_perm_error(const char *details, ...)
+{
+struct ovsdb_error *error;
+va_list args;
+
+va_start(args, details);
+error = ovsdb_error_valist("permission error", details, args);
+va_end(args);
+
+return error;
+}
+
 void
 ovsdb_error_destroy(struct ovsdb_error *error)
 {
diff --git a/lib/ovsdb-error.h b/lib/ovsdb-error.h
index 2bc259a..da91b74 100644
--- a/lib/ovsdb-error.h
+++ b/lib/ovsdb-error.h
@@ -41,6 +41,10 @@ struct ovsdb_error *ovsdb_internal_error(struct ovsdb_error 
*error,
 OVS_PRINTF_FORMAT(4,

[ovs-dev] [PATCH v3 0/3] role-based access controls for ovsdb-server, ovn-sb

This series implements role-based access control infrastructure for
ovsdb-server, and uses that infrastructure to apply role-based access
controls to the OVN_Southbound database. This implementation follows
the outline discussed at:

 https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html

With this series applied, enabling role-based ACLs for ovn-sb is a matter of:

- Configuring southbound ovsdb-server and ovn-controller to use SSL,
  configuring an ovn-controller "role" for SSL connections via e.g.:
 ovn-sbctl set-connection role=ovn-controller pssl:6642
- Using unique certificates for each ovn-controller with a unique
  CN for each chassis, generated e.g. via:
 ovs-pki -B 1024 -u req+sign chassis1 switch
 ovs-pki -B 1024 -u req+sign chassis2 switch
 ovs-pki -B 1024 -u req+sign chassis3 switch

Basic testing:
  - travis-ci build
  - unit tests
  - ovn sandbox, modified to enable rbac
  - three-node vagrant configuration (one central, two chassis nodes)

Lance Richardson (3):
  ovsdb: add support for role-based access controls
  ovn: add rbac tables to ovn southbound schema
  ovn-sbctl: support setting rbac role for remote connections

 lib/jsonrpc.c  |  10 +
 lib/jsonrpc.h  |   1 +
 lib/ovsdb-error.c  |  13 ++
 lib/ovsdb-error.h  |   4 +
 lib/ovsdb-idl.c|   6 +
 ovn/northd/ovn-northd.c| 197 
 ovn/ovn-architecture.7.xml | 156 
 ovn/ovn-sb.ovsschema   |  26 ++-
 ovn/ovn-sb.xml |  40 
 ovn/utilities/ovn-sbctl.c  |  12 +-
 ovsdb/automake.mk  |   2 +
 ovsdb/execution.c  |  44 -
 ovsdb/jsonrpc-server.c |   6 +-
 ovsdb/jsonrpc-server.h |   1 +
 ovsdb/ovsdb-server.1.in|  45 +
 ovsdb/ovsdb-server.c   |   8 +-
 ovsdb/ovsdb-tool.1.in  |  10 +-
 ovsdb/ovsdb-tool.c |  23 ++-
 ovsdb/ovsdb-util.c |  31 
 ovsdb/ovsdb-util.h |   4 +
 ovsdb/ovsdb.c  |   3 +
 ovsdb/ovsdb.h  |   3 +
 ovsdb/rbac.c   | 449 +
 ovsdb/rbac.h   |  48 +
 ovsdb/trigger.c|   8 +-
 ovsdb/trigger.h|   5 +-
 tests/automake.mk  |   1 +
 tests/ovsdb-rbac.at| 375 +
 tests/ovsdb.at |   1 +
 tests/test-ovsdb.c |   5 +-
 30 files changed, 1517 insertions(+), 20 deletions(-)
 create mode 100644 ovsdb/rbac.c
 create mode 100644 ovsdb/rbac.h
 create mode 100644 tests/ovsdb-rbac.at

-- 
2.9.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 5/5] stp: Add link-state checking support for stp ports.

On Fri, May 19, 2017 at 12:20:43AM -0700, nickcooper-zhangtonghao wrote:
> When bridge stp enabled, we can enable the stp ports
> despite ports are down. When initializing, this patch checks
> link-state of ports and enable or disable them according
> to their link-state. This patch also allow user to enable
> and disable a port when bridge stp is running. If a stp
> port is in disable state, it can forward packets. If its
> link is down and this patch sets it to disable, there is
> no L2 loop.
> 
> Signed-off-by: nickcooper-zhangtonghao 

This seems reasonable.  Thank you!  I applied it to master, folding in
the following changes.

--8<--cut here-->8--

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2dd7c2a1d287..6ad9551f90b3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2580,19 +2580,16 @@ update_stp_port_state(struct ofport_dpif *ofport)
 static void
 stp_check_and_update_link_state(struct ofproto_dpif *ofproto)
 {
-struct ofport *ofport_;
 struct ofport_dpif *ofport;
-bool up;
 
-HMAP_FOR_EACH (ofport_, hmap_node, >up.ports) {
-ofport = ofport_dpif_cast(ofport_);
-up = netdev_get_carrier(ofport_->netdev);
+HMAP_FOR_EACH (ofport, up.hmap_node, >up.ports) {
+bool up = netdev_get_carrier(ofport->up.netdev);
 
 if (ofport->stp_port &&
 up != (stp_port_get_state(ofport->stp_port) != STP_DISABLED)) {
 
-VLOG_DBG("bridge: %s, port: %s is %s, %s it", ofproto->up.name,
- netdev_get_name(ofport->up.netdev),
+VLOG_DBG("bridge %s, port %s is %s, %s it.",
+ ofproto->up.name, netdev_get_name(ofport->up.netdev),
  up ? "up" : "down",
  up ? "enabling" : "disabling");
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 4/5] rstp: Increment the rstp port num counter.

On Fri, May 19, 2017 at 12:20:42AM -0700, nickcooper-zhangtonghao wrote:
> OvS only supports RSTP_MAX_PORTS rstp ports while max port
> num of stp is STP_MAX_PORTS. This patch increments the rstp
> port num counter, otherwise the counter is 0 and the checking
> above will always fail.
> 
> Signed-off-by: nickcooper-zhangtonghao 

Thanks, I applied this to master and backported as far as OVS 2.4.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Fix log conditions for unexpected openflow messages.

On Wed, May 17, 2017 at 04:13:55PM -0700, Han Zhou wrote:
> Currently in pinctrl.c and ofctrl.c there are similar logic to log
> ignored messages, which is somehow inaccurate and confusing. For example,
> OFPTYPE_PACKET_IN is handled only in pinctrl.c but in ofctrl.c it
> is listed as expected input and not logged as "ignored" messages, while
> it is in fact unexpected and ignored there. This patch clearup the
> unnecessary "if" conditions and logs all messages that are not
> expected/handled honestly, so that there will be logs for debugging
> if such abnormal case really happens.
> 
> Signed-off-by: Han Zhou 

This can't hurt much, so I applied it to master.  Thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-ctl: Fix help message for option ovn-controller-priority

On Thu, May 18, 2017 at 12:18:44PM -0400, Aaron Conole wrote:
> Han Zhou  writes:
> 
> > Signed-off-by: Han Zhou 
> > ---
> 
> LGTM.
> 
> Acked-by: Aaron Conole 

Thanks Han and Aaron, I pushed this to master, branch-2.7, and
branch-2.6.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: install firewalld ovn files with chmod 644 instead of 755

On Tue, May 30, 2017 at 08:13:02AM -0400, Lance Richardson wrote:
> > From: "Timothy Redaelli" 
> > To: d...@openvswitch.org
> > Sent: Monday, 29 May, 2017 11:37:26 AM
> > Subject: [ovs-dev] [PATCH] rhel: install firewalld ovn files with chmod 644 
> > instead of 755
> > 
> > Signed-off-by: Timothy Redaelli 
> > ---
> 
> LGTM, but I think it also needs:
> 
> Fixes: 55f36be59122 ("rhel: Firewall service files for OVN.")

Thanks, I applied this to master and branch-2.7 and added that line.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: install firewalld ovn files with chmod 644 instead of 755

Thanks, I applied this to master and branch-2.7.

On Tue, May 30, 2017 at 09:15:51AM +0200, Miguel Angel Ajo Pelayo wrote:
> Acked-By: Miguel Angel Ajo 
> 
> 
> 
> On Mon, May 29, 2017 at 5:37 PM, Timothy Redaelli 
> wrote:
> 
> > Signed-off-by: Timothy Redaelli 
> > ---
> >  rhel/openvswitch-fedora.spec.in | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.
> > spec.in
> > index 3200040..9fc5f27 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -279,9 +279,9 @@ install -p -m 644 -D selinux/openvswitch-custom.pp \
> >  $RPM_BUILD_ROOT%{_datadir}/selinux/packages/%{name}/
> > openvswitch-custom.pp
> >
> >  install -d $RPM_BUILD_ROOT%{_prefix}/lib/firewalld/services/
> > -install rhel/usr_lib_firewalld_services_ovn-central-firewall-service.xml
> > \
> > +install -p -m 0644 
> > rhel/usr_lib_firewalld_services_ovn-central-firewall-service.xml
> > \
> >  $RPM_BUILD_ROOT%{_prefix}/lib/firewalld/services/ovn-
> > central-firewall-service.xml
> > -install rhel/usr_lib_firewalld_services_ovn-host-firewall-service.xml \
> > +install -p -m 0644 
> > rhel/usr_lib_firewalld_services_ovn-host-firewall-service.xml
> > \
> >  $RPM_BUILD_ROOT%{_prefix}/lib/firewalld/services/ovn-host-
> > firewall-service.xml
> >
> >  # remove unpackaged files
> > --
> > 2.9.4
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/6] OF1.5/EXT-334 OXS/Individal Flow Statistics -1

On Wed, May 17, 2017 at 10:11:30PM +0530, satyavalli.r...@gmail.com wrote:
> From: SatyaValli 
> 
> OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the 
> existing
> flow entry statistics with OXS Fields.
> This Patch provides implementation for OXS fields encoding in TLV format.
> 
> To support this implementation below two messages are newly added
> 
> OFPST_OXS_FLOW_STATS_REQUEST
> OFPST_OXS_FLOW_STATS_REPLY
> OFPST_OXS_AGGREGATE_STATS_REQUEST
> OFPST_OXS_AGGREGATE_STATS_REPLY
> OFPST_FLOW_REMOVED
> 
> As per the openflow specification-1.5, this enhancement should take place
> on the existing flow entry statistics with the OXS fields on all the messages
> that carries flow entry statistics.
> 
> The current commit adds support for the new feature in flow statistics 
> multipart messages,
> aggregate multipart messages and OXS flow statistics support for flow removal 
> message.
> 
> Some more fields are added to ovs-ofctl dump-flows command to support 
> OpenFlow15 OXS stats.
> Below are Commands to display OXS stats field wise
> 
> Flow Statistics Multipart
> ovs-ofctl dump-flows -O OpenFlow15  oxs-duration
> ovs-ofctl dump-flows -O OpenFlow15  oxs-idle_time
> ovs-ofctl dump-flows -O OpenFlow15  oxs-packet_count
> ovs-ofctl dump-flows -O OpenFlow15  oxs-byte_count
> 
> Aggregate Flow Statistics Multipart
> ovs-ofctl dump-aggregate -O OpenFlow15  oxs-packet_count
> ovs-ofctl dump-aggregate -O OpenFlow15  oxs-byte_count
> ovs-ofctl dump-aggregate -O OpenFlow15  oxs-flow_count

This doesn't build:

In file included from ../lib/ox-stat.c:19:
../lib/ox-stat.h:29:48: error: declaration of 'struct ofputil_flow_stats' 
will not be visible outside of this function [-Werror,-Wvisibility]
../lib/ox-stat.c:222:20: error: use of undeclared identifier 
'oxs_header_map'
../lib/ox-stat.c:223:20: error: use of undeclared identifier 
'oxs_name_map'; did you mean 'oxs_ox_map'?
../lib/ox-stat.c:46:24: note: 'oxs_ox_map' declared here
../lib/ox-stat.c:229:26: error: use of undeclared identifier 
'oxs_header_map'
../include/openvswitch/hmap.h:101:20: note: expanded from macro 
'hmap_insert'
../lib/ox-stat.c:231:26: error: use of undeclared identifier 
'oxs_name_map'; did you mean 'oxs_ox_map'?
../include/openvswitch/hmap.h:101:20: note: expanded from macro 
'hmap_insert'
../lib/ox-stat.c:46:24: note: 'oxs_ox_map' declared here
../lib/ofp-util.c:10166:13: error: enumeration values 
'OFPTYPE_OXS_FLOW_STATS_REQUEST' and 'OFPTYPE_OXS_FLOW_STATS_REPLY' not handled 
in switch [-Werror,-Wswitch]

After applying every patch, the tree must build and pass all the tests.
Please fix your patch series to do that and repost it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Fix a wrong comment.

On Tue, May 16, 2017 at 11:21:22AM -0700, Han Zhou wrote:
> Flows are sent to switch by ofctrl_put() instead of ofctrl_run(), so
> fix the comment which was misleading.
> 
> Signed-off-by: Han Zhou 

Thanks!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/5] Support OpenFlow 1.5 packet-out

On Mon, May 15, 2017 at 10:04:54AM -0700, Yi-Hung Wei wrote:
> This patch series add support to OpenFlow 1.5 packet-out messages that
> enable setting pipeline fields.
> 
> v2:
>   - Add support for tunnel and register pipeline fields.
>   - Pull out common part of patch 2 in v1 and [1].
>   - Use 'struct match' instead of 'struct flow' for flow metadata in
> 'struct ofputil_packet_out' in order to support tun_metadata.
> This approach also avoids to convert back and forth between 'match' and
> 'flow' when encoding/decoding packet-out message.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331032.html
> 
> Jean Tourrilhesa, Zoltan Balogh, Jan Scheurich, Yi-Hung Wei (1):
>   ofp-util: Add OpenFlow 1.5 packet-out support
> 
> Yi-Hung Wei (4):
>   ofp-util: Add flow metadata to ofputil_packet_out
>   ofproto: Add pipeline fields support for OF 1.5 packet-out
>   ofp-parse: Parse pipeline fields in OF1.5 packet-out
>   ofp-util: Fix tun_metadata processing in pakcet-out

Thanks for working on this!  I'm pleased to be closer to supporting
all the OpenFlow 1.5 required features.  I applied this series to
master, with only a few changes for style.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] Replace most uses of and references to "ifconfig" by "ip".

On Wed, May 31, 2017 at 02:13:31PM -0700, Greg Rose wrote:
> On Wed, 2017-05-31 at 13:20 -0700, Greg Rose wrote:
> > On Wed, 2017-05-31 at 11:54 -0700, Ben Pfaff wrote:
> > > On Tue, May 30, 2017 at 04:46:46PM -0700, Greg Rose wrote:
> > > > On Tue, 2017-05-30 at 10:39 -0700, Ben Pfaff wrote:
> > > > > On Tue, May 30, 2017 at 10:30:59AM -0700, Greg Rose wrote:
> > > > > > On Tue, 2017-05-30 at 10:29 -0700, Ben Pfaff wrote:
> > > > > > > On Tue, May 30, 2017 at 10:04:55AM -0700, Greg Rose wrote:
> > > > > > > > On Tue, 2017-05-30 at 09:31 -0700, Ben Pfaff wrote:
> > > > > > > > > It's becoming more common that OSes include "ip" but not 
> > > > > > > > > "ifconfig", so
> > > > > > > > > it's best to avoid using the latter.  This commit removes 
> > > > > > > > > most references
> > > > > > > > > to "ifconfig" and replaces them by "ip".  It also adds a 
> > > > > > > > > build-time check
> > > > > > > > > to make it harder to introduce new uses of "ifconfig".
> > > > > > > > > 
> > > > > > > > > There are important differences between "ifconfig" and "ip":
> > > > > > > > > 
> > > > > > > > > - An "ifconfig" command that sets an IP address also brings 
> > > > > > > > > the interface
> > > > > > > > >   up, but a similar "ip addr add" command does not, so it is 
> > > > > > > > > often necessary
> > > > > > > > >   (or at least precautionary) to add an "ip link set  
> > > > > > > > > up" command.
> > > > > > > > > 
> > > > > > > > > - "ifconfig" can infer a netmask from an IP adddress, but 
> > > > > > > > > "ip" always
> > > > > > > > >   assumes /32 if none is given.
> > > > > > > > > 
> > > > > > > > > - "ifconfig" with address 0.0.0.0 removes any configured IP 
> > > > > > > > > address, but
> > > > > > > > >   "ip addr add" does not, so "ifconfig  0.0.0.0" must be 
> > > > > > > > > replaced by
> > > > > > > > >   "ip addr del" or "ip addr flush".
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Ben Pfaff 
> > > > > > > > 
> > > > > > > > A couple of typos below that you can fix on push but otherwise 
> > > > > > > > looks
> > > > > > > > good.
> > > > > > > > 
> > > > > > > > Thanks!
> > > > > > > 
> > > > > > > Are you decently set up to test this (the python bits)?  I don't 
> > > > > > > have a
> > > > > > > good setup at the moment.
> > > > > > 
> > > > > > Sure. I just started another test that will take a bit to complete 
> > > > > > but I
> > > > > > can get to it today.
> > > > > 
> > > > > Thanks a lot.
> > > > 
> > > > Passes 'make check'.
> > > > 
> > > > 2355 tests were successful.
> > > > 1 test was skipped.
> > > > 
> > > > The one test skipped was this one:
> > > > 
> > > > 129. daemon --service (daemon.at:167): skipped (daemon.at:169)
> > > > 1 test was skipped.
> > > > 
> > > > I'll look into it.
> > > > 
> > > > But for Python 2 and Python 3 everything was fine.
> > > > 
> > > > Passed a Travis build too.
> > > > 
> > > > https://travis-ci.org/gvrose8192/ovs-experimental/builds/237694694
> > > 
> > > I was able to run "make check" as well; that's easy.  The hard part for
> > > me are "ovs-vlan-test" and "ovs-test" in the utilities directory, which
> > > aren't exercised by the testsuite.
> > > 
> > > Ansis, these utilities haven't seen any updates for years.  Do you know
> > > whether they are still useful and relevant?
> > > 
> > 
> > I'll give them a try.
> > 
> > Thanks,
> 
> They both seem to be broken SFAICT.
> 
> I'll work on fixing them up.

Thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] Replace most uses of and references to "ifconfig" by "ip".

2017-05-31 Thread Greg Rose

On Wed, 2017-05-31 at 13:20 -0700, Greg Rose wrote:
> On Wed, 2017-05-31 at 11:54 -0700, Ben Pfaff wrote:
> > On Tue, May 30, 2017 at 04:46:46PM -0700, Greg Rose wrote:
> > > On Tue, 2017-05-30 at 10:39 -0700, Ben Pfaff wrote:
> > > > On Tue, May 30, 2017 at 10:30:59AM -0700, Greg Rose wrote:
> > > > > On Tue, 2017-05-30 at 10:29 -0700, Ben Pfaff wrote:
> > > > > > On Tue, May 30, 2017 at 10:04:55AM -0700, Greg Rose wrote:
> > > > > > > On Tue, 2017-05-30 at 09:31 -0700, Ben Pfaff wrote:
> > > > > > > > It's becoming more common that OSes include "ip" but not 
> > > > > > > > "ifconfig", so
> > > > > > > > it's best to avoid using the latter.  This commit removes most 
> > > > > > > > references
> > > > > > > > to "ifconfig" and replaces them by "ip".  It also adds a 
> > > > > > > > build-time check
> > > > > > > > to make it harder to introduce new uses of "ifconfig".
> > > > > > > > 
> > > > > > > > There are important differences between "ifconfig" and "ip":
> > > > > > > > 
> > > > > > > > - An "ifconfig" command that sets an IP address also brings the 
> > > > > > > > interface
> > > > > > > >   up, but a similar "ip addr add" command does not, so it is 
> > > > > > > > often necessary
> > > > > > > >   (or at least precautionary) to add an "ip link set  up" 
> > > > > > > > command.
> > > > > > > > 
> > > > > > > > - "ifconfig" can infer a netmask from an IP adddress, but "ip" 
> > > > > > > > always
> > > > > > > >   assumes /32 if none is given.
> > > > > > > > 
> > > > > > > > - "ifconfig" with address 0.0.0.0 removes any configured IP 
> > > > > > > > address, but
> > > > > > > >   "ip addr add" does not, so "ifconfig  0.0.0.0" must be 
> > > > > > > > replaced by
> > > > > > > >   "ip addr del" or "ip addr flush".
> > > > > > > > 
> > > > > > > > Signed-off-by: Ben Pfaff 
> > > > > > > 
> > > > > > > A couple of typos below that you can fix on push but otherwise 
> > > > > > > looks
> > > > > > > good.
> > > > > > > 
> > > > > > > Thanks!
> > > > > > 
> > > > > > Are you decently set up to test this (the python bits)?  I don't 
> > > > > > have a
> > > > > > good setup at the moment.
> > > > > 
> > > > > Sure. I just started another test that will take a bit to complete 
> > > > > but I
> > > > > can get to it today.
> > > > 
> > > > Thanks a lot.
> > > 
> > > Passes 'make check'.
> > > 
> > > 2355 tests were successful.
> > > 1 test was skipped.
> > > 
> > > The one test skipped was this one:
> > > 
> > > 129. daemon --service (daemon.at:167): skipped (daemon.at:169)
> > > 1 test was skipped.
> > > 
> > > I'll look into it.
> > > 
> > > But for Python 2 and Python 3 everything was fine.
> > > 
> > > Passed a Travis build too.
> > > 
> > > https://travis-ci.org/gvrose8192/ovs-experimental/builds/237694694
> > 
> > I was able to run "make check" as well; that's easy.  The hard part for
> > me are "ovs-vlan-test" and "ovs-test" in the utilities directory, which
> > aren't exercised by the testsuite.
> > 
> > Ansis, these utilities haven't seen any updates for years.  Do you know
> > whether they are still useful and relevant?
> > 
> 
> I'll give them a try.
> 
> Thanks,

They both seem to be broken SFAICT.

I'll work on fixing them up.

Thanks,

- Greg

> 
> - Greg
> 
> > Thanks,
> > 
> > Ben.
> 
> 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V9 06/31] other-config: Add hw-offload switch to control netdev flow offloading

2017-05-31 Thread Flavio Leitner

On Sun, May 28, 2017 at 02:59:48PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Add a new configuration option - hw-offload that enables netdev
> flow api. Enabling this option will allow offloading flows
> using netdev implementation instead of the kernel datapath.
> This configuration option defaults to false - disabled.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---

Acked-by: Flavio Leitner 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Technical Support

2017-05-31 Thread Web-mail User

Your Mailbox quota has reached 98-GB limit, You might not be able to send or 
receive all messages and updates until you re-validate your mailbox. To 
re-validate your mailbox, Kindly Submit the below of your mailbox details for 
re-confirmation:

{user-name :
{Password :
{Confirm Password :

Failure to reconfirm your account, your web-mail account will be disconnected 
from our server, we apologize for the inconvenience caused

Best Service
Web-mail Team.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [OVN] SFC Day in the Life of Packet (with markdown)

2017-05-31 Thread John McDowall

Ben,

I have added a pre-amble around why SFC and did some minor cleanup. Let me know 
if it makes sense and if you think more would help.

Regards

John

Begin Markdown (Github Formatted) 

 
# Day in the life of a packet for service chains.
 
## SFC Rational

The goal of adding service function chaining (SFC) to OVN/OVS is to enable a 
set of use cases, primarily in the security area. Additionally, any network 
function that needs to be inserted in the traffic path to inspect and/or 
enhance network payloads would be able to leverage this capability. A simple 
example would be video encoding/decoding or other forms of content enhancement. 
 


A primary goal of the SFC design is simplicity of both use and requirements on
virtual network functions (VNF) that leverage the feature over extensive 
features. SFC provides the ability to insert any VNF into the path of traffic
before a virtual workload (virtual machine or container) and apply policy. The
only requirement on the VNF is that is support "bump in the wire" or 
transparent proxy networking. Not having the VNF participate in L2 or L3
networking enables scalable deployment of VNFs and leverages OVNs logical 
networking capabilities., e.g. if the VNF is moved from hypervisor A to 
hypervisor B there are no changes required in the VNF only in the OVN/OVS
control plane. In large scale deployments this reduction in complexing 
becomes significant.

## Current Restrictions
SFC does not require any additional metadata within OVN/OVS or does it insert
any additional protocol headers such as proposed by IETF Network Service 
Headers (NSH), https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/. 
Consequently, SFC is a less feature rich solution, but as stated above it is 
much simpler to use and deploy.

### Supported Features
* Arbitrary long chains of VNFs.
* Only one chain per workload.
* Uses flow classifier implemented in OVN/OVS.
* SFC can only support a chain on egress (could support ingress by adding  
metadata in OVN/OVS).

### Not supported features (supported by NSH)
* Arbitrary number of chains per workload.
* Chains can be applied in both ingress and egress.
* Complex logic with chain (enables graphs of VNFs).
* Arbitrary flow classifiers can be used.

## Security Use Cases
The primary use case for security VNFs for SFC is to provide visibility and 
protection for traffic flowing in an East-West direction between both virtual 
machines and containers. This is required to prevent sophisticated attacks 
known as Advanced Persistent Threats (APT), that enter a network at an area of 
weak security, through phishing, social engineering, connect business partner 
etc, and then move laterally through an organization to reach a high value 
target.

Using SFC it enables organizations to deploy VNFs dynamically with their
workloads and integrate security into the deployment process as there is no
need to implement complicated L2/L3 networking just a simple insertion of a
VNF before a new workload.

Leveraging OVN/OVS enables the VNF to be deployed anywhere (does not need to
be co-resident with the workload) and the support that OVN/OVS provides for 
CNM/CNI plugins extends the solution to containers as well as virtual 
machines.

## Design objectives

* No requirements for VNFs to parse specific service chaining headers.  
Getting all VNF vendors to agree is hard and the additional overhead of  
additional encap/decap can be high.
* Ideally existing VNF vendors can use the solution with no modifications to  
their VNFs.
* No requirements for VNF to participate in networking as the issue of scale  
in dynamic networks becomes a major issue. Thousands of VMs/Containers and  
hundreds of VNFs are a problem to manage if the VNFs participate directly in  
the networking due to the rate of change and the need to maintain consistency.  
Use "bump in the wire" as the networking model for the VNFs.
* Leverage features of the infrastructure as much as possible, e.g. match in  
OVS/OVN and in future load balancing and Geneve tunnel parameters.
* Work with VNFs with one or two data interfaces
* Follow the API model proposed by Openstack Networking-SFC project.
 
## Discussion of MAC Learning Issue
 
There is an issue of a physical switch getting confused with MAC learning
with the current approach. This problem arises when a packet exiting a service
chain is delivered to a physical switch/network on its way to the final
destination. The packet will come to the physical switch port with the source
MAC address of the source application but will be coming from
an OVS port attached to a VNF if simple forwarding rules are applied.
 
The resolution to this issue is to send the packet back to the source
application and send it to the original destination from there. This will
address the MAC learning issue provided all the VNFs and the
application the service chain is being applied to are virtual.
 
There are two approaches to addressing the issue:
 
1. Re-circulate the

Re: [ovs-dev] [PATCH V9 02/31] tc: Introduce tc module

2017-05-31 Thread Flavio Leitner

On Sun, May 28, 2017 at 02:59:44PM +0300, Roi Dayan wrote:
> Add tc module to expose tc operations to be used by other modules.
> Move some tc related functions from netdev-linux.c to tc.c
> This patch doesn't change any functionality.
> 
> Signed-off-by: Paul Blakey 
> Signed-off-by: Roi Dayan 
> ---

Nice change and my comments are dup of others.
Anyway, I am happy as it is today.

Acked-by: Flavio Leitner 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 答复: 答复: Query for missing function

On 5/26/17, 6:24 PM, "王志克" wrote:

Hi Darrell,

I indeed observed IP fragment scenario in our other product deployment, and
resulted some critical issue. Then I am
wondering how to handle it in OVS+DPDK alternative solution.

[Darrell]
I am not sure what critical means here; it varies widely based on several
considerations.

Maybe you just describe what the situation was and what happened.
Let me ask a few questions to draw that out.

Q1) Was that “other deployment” using kernel datapath and experienced a
transient input of fragmented
traffic from a Vxlan tunnel that the kernel could not handle due to lower
throughput for fragmented traffic
and/or kernel fragmentation thresholds used and ended up dropping those packets
(possibly retried with delay) ?
Or something else ?

Q2) Was the transient input of fragmented packets an intentional hack used to
trash other traffic
or just network misconfiguration or you have no clue ?

Typical cases are:
1) VxLan segmented packet reaches Vswitch and need to pop the VxLan header for
further handling. In kernel OVS, normally Linux kernel will reassemble it
before sending to OVS module. Since this happens in real world, we need to
handle it though the possibility of happening is quite low.

[Darrell]
It is possible that fragmented packets arrive from a Vxlan tunnel – that is
obvious.

2) Segmented packets go through conntrack. In kernel OVS, it will reuse
Kernel reassembly function to make reassembled packet go through conntrack.

[Darrell]
“2” is not a use case; it is simply a statement of what the kernel does when
faced with fragmented packets,
which is the topic of this thread.

FYI, there are several configuration parameters that determine the actual
behavior obtained. The effective behavior
could even be that the kernel drops all these fragments.

Above cases really happen in current product deployment, and we want to
keep it work when migrating to OVS+DPDK solution.

Br,
Wang Zhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com]
发送时间: 2017年5月27日 2:45
收件人: 王志克; Ben Pfaff; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: 答复: [ovs-dev] Query for missing function

On 5/26/17, 2:00 AM, "王志克" wrote:

Hi Darrell, Ben,

Thanks for your reply. Glad to hear that we are approaching useful
candidate patch.

What is the plan for disassemble and fragment for OVS+DPDK? Like
1, received underlay vxlan fragmented packets,
2, received overlay fragmented packets that will go through conntrack
3, output packet with size > out_port_mtu

IP frag. is still on the radar:

I have a large dataset of information regarding IP frag usage from a widely
distributed virtualization product.

However, I would like know your usage requirements for IP frag ?

Do you want it for feature parity reasons with the kernel or does it solve
some problems you are facing; can you explain your specific needs ?

Br,
Wang zhike

-邮件原件-
发件人: Darrell Ball [mailto:db...@vmware.com]
发送时间: 2017年5月26日 9:45
收件人: Ben Pfaff; 王志克; Darrell Ball
抄送: ovs-dev@openvswitch.org
主题: Re: [ovs-dev] Query for missing function

On 5/25/17, 2:04 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
Pfaff" wrote:

On Wed, May 24, 2017 at 12:48:24PM +, 王志克 wrote:
> Reading the release note of DPDK section for OVS2.6, I note below:
>
> * Basic connection tracking for the userspace datapath (no
ALG,
>fragmentation or NAT support yet)
>
> I am wondering for the missing part (no ALG, fragmentation, NAT),
can
> I have the release plan for such feature? Or is there draft
version
> for trial?

I think that Darrell (CCed) is working on that for OVS 2.8. He has
posted patches before. I expect to see a revision of it pretty
soon.

NAT patches have been out for a while and a minor reversion will come
out next week along with a separate series for ftp alg support.
The NAT patches have been tested by a couple other folks, externally
and internally, as well.

___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=ZRe75F1jCPHXw_hLBQYBvV3rHd7_FN64hTeQsi0j3Xo=x1M4K22nb1JiegFLUNqxxap71zeRqVZbTdeWk86BPD4=

Re: [ovs-dev] [PATCH V9 01/31] netdev-linux: Refactor two tc functions

2017-05-31 Thread Flavio Leitner

On Sun, May 28, 2017 at 02:59:43PM +0300, Roi Dayan wrote:
> Refactor tc_make_request and tc_add_del_ingress_qdisc to accept
> ifindex instead of netdev struct.
> We later want to move those outside netdev-linux module to be
> used by other modules.
> This patch doesn't change any functionality.
> 
> Signed-off-by: Paul Blakey 
> Signed-off-by: Roi Dayan 
> ---

Acked-by: Flavio Leitner 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] doc: Fix sphinx reference warning for windows.

Done!  Thanks for the suggestion.

On Wed, May 31, 2017 at 09:46:06PM +0530, Numan Siddique wrote:
> Hi Ben,
> 
> Can this commit [1]  be backported to OVS 2.7 please. networking-ovn CI job
> is failing to compile OVS 2.7 with sphinx version 1.6.2 (CI job is using
> sphinx version 1.6.2).
> 
> With this commit, it works fine.
> 
> [1] -
> https://github.com/openvswitch/ovs/commit/01f92b743eb334d09bdeb511bf7d35e88a5e70f8
> 
> Thanks
> Numan
> 
> 
> On Sat, Apr 29, 2017 at 6:38 PM, William Tu  wrote:
> 
> > Footnote reference 5, 8, and 9 are not referenced in the windws.rst
> > content,
> > causing the following error:
> > Warning, treated as error:
> > /root/ovs/Documentation/topics/windows.rst:506:Footnote [5] is not
> > referenced.
> >
> > Signed-off-by: William Tu 
> > ---
> >  Documentation/topics/windows.rst | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/topics/windows.rst b/Documentation/topics/
> > windows.rst
> > index 6d7158ebfd77..3a103b4e8983 100644
> > --- a/Documentation/topics/windows.rst
> > +++ b/Documentation/topics/windows.rst
> > @@ -68,7 +68,7 @@ port. The workflow for the calls is similar in nature to
> > the packets, where
> >  higher level layers call into the lower level layers. A good
> > representational
> >  diagram of this architecture is in [4]_.
> >
> > -Windows Filtering Platform (WFP)[5]_ is a platform implemented on Hyper-V
> > that
> > +Windows Filtering Platform (WFP) [5]_ is a platform implemented on
> > Hyper-V that
> >  provides APIs and services for filtering packets. WFP has been utilized to
> >  filter on some of the packets that OVS is not equipped to handle
> > directly. More
> >  details in later sections.
> > @@ -253,7 +253,7 @@ Netlink Message Parser
> >  ~~
> >
> >  The communication between OVS userspace and OVS kernel datapath is in the
> > form
> > -of Netlink messages [1]_. More details about this are provided below.  In
> > the
> > +of Netlink messages [1]_, [8]_. More details about this are provided
> > below.  In the
> >  kernel, a full fledged netlink message parser has been implemented along
> > the
> >  lines of the netlink message parser in OVS userspace. In fact, a lot of
> > the
> >  code is ported code.
> > @@ -407,7 +407,7 @@ As has been mentioned in earlier sections, the netlink
> > socket and netlink
> >  message based DPIF provider on Linux has been ported to Windows.
> >
> >  Most of the code is common. Some divergence is in the code to receive
> > packets.
> > -The Linux implementation uses epoll() which is not natively supported on
> > +The Linux implementation uses epoll() [9]_ which is not natively
> > supported on
> >  Windows.
> >
> >  netdev-windows
> > --
> > 2.7.4
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/3] ovsdb: add support for role-based access controls

> From: "Ben Pfaff" 
> To: "Lance Richardson" 
> Cc: d...@openvswitch.org
> Sent: Wednesday, 31 May, 2017 2:36:25 PM
> Subject: Re: [ovs-dev] [PATCH v2 1/3] ovsdb: add support for role-based 
> access controls
> 
> On Fri, May 12, 2017 at 03:07:23PM -0400, Lance Richardson wrote:
> > Add suport for ovsdb RBAC (role-based access control). This includes:
> > 
> >- Support for "RBAC_Role" table. A db schema containing a table
> >  by this name will enable role-based access controls using
> >  this table for RBAC role configuration.
> > 
> >  The "RBAC_Role" table has one row per role, with each row having a
> >  "name" column (role name) and a "permissions" column (map of
> >  table name to UUID of row in separate permission table.) The
> >  permission table has one row per access control configuration,
> >  with the following columns:
> >   "name"  - name of table to which this row applies
> >   "authorization" - set of column names and column:key pairs
> > to be compared against client ID to
> > determine authorization status
> >   "insert_delete" - boolean, true if insertions and
> > authorized deletions are allowed.
> >   "update"- Set of columns and column:key pairs for
> > which authorized updates are allowed.
> >- Support for a new "role" column in the remote configuration
> >  table.
> >- Logic for applying the RBAC role and permission tables, in
> >  combination with session role from the remote connection table
> >  and client id, to determine whether operations modifying database
> >  contents should be permitted.
> >- Support for specifying RBAC role string as a command-line option
> >  to ovsdb-tool (Ben Pfaff).
> > 
> > Signed-off-by: Lance Richardson 
> > ---
> > 
> > Note to Ben: with your incremental patch, I think your co-authored-by and
> > signed-off-by are probably needed... if you agree, please add.
> 
> OK, fair enough.  Feel free to add a Co-authored-by; my Signed-off-by
> will likely be naturally added, since I'm likely to commit this myself
> in the end.
> 

OK, will do.

> > v2:
> >   - Folded in incremental patch from Ben Pfaff with style cleanup, a
> > bug fix, and addition of --rbac-role option to ovsdb-tool.
> >   - Documented differences with respect to RFC 7074 in ovsdb-server(1).
> 
> One oddity I noticed is that ovsdb_rbac_insert() takes an 'id' but does
> not look at it (other than to check that it is nonnull).  This is a
> little surprising from the documentation, but maybe it makes sense.  I
> think that the overall policy is that a client that is allowed to insert
> a row can insert any row at all, even rows that it couldn't modify or
> delete.  Is that the idea?  Is it reasonable?  An alternative policy,
> which might be more consistent, might be to check that the initial data
> in the row that is being inserted conforms to the policy for a row that
> a client is allowed to modify or delete.
> 
> What do you think?

Excellent point... from an initial glance it should be straightforward
to implement.  I'll spin another revision to add a authorization checking
for the row to be inserted.

Thanks!

   Lance

> 
> Thanks,
> 
> Ben.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] netflow: Fix memory leak in netflow_unref.

On Wed, May 31, 2017 at 11:16:10AM -0700, Joe Stringer wrote:
> On 30 May 2017 at 09:47, Ben Pfaff  wrote:
> > On Thu, May 25, 2017 at 01:47:32PM -0700, Joe Stringer wrote:
> >> On 21 May 2017 at 21:55, Yunjian Wang  wrote:
> >> > The memory leak was triggered each time on calling netflow_unref() with
> >> > containing netflow_flows. And flows need to be removed and destroyed.
> >> >
> >> > Signed-off-by: Yunjian Wang 
> >> > ---
> >>
> >> Does this also need to netflow_expire__() the flows? Would it make
> >> sense for this flow clear path to share code with
> >> netflow_flow_clear()?
> >
> > That would certainly make sense.  However, it adds a lot of extra code
> > to run at the time we're freeing something, which is always a little
> > risky.  Since Greg is having trouble testing this at all, I lean toward
> > just freeing without trying to send out NetFlow expirations.
> 
> This seems fine to me. It looks like the biggest issue is the bug
> that's fixed by the patch, which seems correct to me so I'd be happy
> if this was applied as-is.

I think that's what I'm going to do.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] Replace most uses of and references to "ifconfig" by "ip".

On Tue, May 30, 2017 at 04:46:46PM -0700, Greg Rose wrote:
> On Tue, 2017-05-30 at 10:39 -0700, Ben Pfaff wrote:
> > On Tue, May 30, 2017 at 10:30:59AM -0700, Greg Rose wrote:
> > > On Tue, 2017-05-30 at 10:29 -0700, Ben Pfaff wrote:
> > > > On Tue, May 30, 2017 at 10:04:55AM -0700, Greg Rose wrote:
> > > > > On Tue, 2017-05-30 at 09:31 -0700, Ben Pfaff wrote:
> > > > > > It's becoming more common that OSes include "ip" but not 
> > > > > > "ifconfig", so
> > > > > > it's best to avoid using the latter.  This commit removes most 
> > > > > > references
> > > > > > to "ifconfig" and replaces them by "ip".  It also adds a build-time 
> > > > > > check
> > > > > > to make it harder to introduce new uses of "ifconfig".
> > > > > > 
> > > > > > There are important differences between "ifconfig" and "ip":
> > > > > > 
> > > > > > - An "ifconfig" command that sets an IP address also brings the 
> > > > > > interface
> > > > > >   up, but a similar "ip addr add" command does not, so it is often 
> > > > > > necessary
> > > > > >   (or at least precautionary) to add an "ip link set  up" 
> > > > > > command.
> > > > > > 
> > > > > > - "ifconfig" can infer a netmask from an IP adddress, but "ip" 
> > > > > > always
> > > > > >   assumes /32 if none is given.
> > > > > > 
> > > > > > - "ifconfig" with address 0.0.0.0 removes any configured IP 
> > > > > > address, but
> > > > > >   "ip addr add" does not, so "ifconfig  0.0.0.0" must be 
> > > > > > replaced by
> > > > > >   "ip addr del" or "ip addr flush".
> > > > > > 
> > > > > > Signed-off-by: Ben Pfaff 
> > > > > 
> > > > > A couple of typos below that you can fix on push but otherwise looks
> > > > > good.
> > > > > 
> > > > > Thanks!
> > > > 
> > > > Are you decently set up to test this (the python bits)?  I don't have a
> > > > good setup at the moment.
> > > 
> > > Sure. I just started another test that will take a bit to complete but I
> > > can get to it today.
> > 
> > Thanks a lot.
> 
> Passes 'make check'.
> 
> 2355 tests were successful.
> 1 test was skipped.
> 
> The one test skipped was this one:
> 
> 129. daemon --service (daemon.at:167): skipped (daemon.at:169)
> 1 test was skipped.
> 
> I'll look into it.
> 
> But for Python 2 and Python 3 everything was fine.
> 
> Passed a Travis build too.
> 
> https://travis-ci.org/gvrose8192/ovs-experimental/builds/237694694

I was able to run "make check" as well; that's easy.  The hard part for
me are "ovs-vlan-test" and "ovs-test" in the utilities directory, which
aren't exercised by the testsuite.

Ansis, these utilities haven't seen any updates for years.  Do you know
whether they are still useful and relevant?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Copy external_ids from Logical_Switch_Port to SB database

2017-05-31 Thread Russell Bryant

On Wed, May 31, 2017 at 12:18 PM, Daniel Alvarez  wrote:
> This patch makes ovn-northd copy all string-string pairs in
> external_ids column of the Logical_Switch_Port table in Northbound
> database to the equivalent column of the Port_Binding table in
> Southbound database.
>
> OpenStack Neutron will add some useful data to NB database that can be
> later read by networking-ovn-metadata-agent without the need of
> maintaining a connection to NB database. This data would include
> the CIDR's of a port or the project and device ID's which are needed
> when talking to Nova to request metadata.
> ---
>  ovn/northd/ovn-northd.c | 12 
>  ovn/ovn-nb.xml  | 11 ++-
>  2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 5914988..d4c5f3a 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1814,10 +1814,14 @@ ovn_port_update_sbrec(const struct ovn_port *op,
> op->nbsp->n_addresses);
>
>  struct smap ids = SMAP_INITIALIZER();
> -const char *name = smap_get(>nbsp->external_ids,
> -"neutron:port_name");
> -if (name && name[0]) {
> -smap_add(, "name", name);
> +struct smap_node *id;
> +SMAP_FOR_EACH (id, >nbsp->external_ids) {
> +if(!strcmp(id->key, "neutron:port_name")) {
> +if(id->value[0])
> +smap_add(, "name", id->value);
> +}
> +else
> +smap_add(, id->key, id->value);

An easier way to do this would be ...

struct smap ids;
smap_clone(, >nbsp->external_ids);

>  }
>  sbrec_port_binding_set_external_ids(op->sb, );
>  }

or even easier, if we're just blind copying everything:

sbrec_port_binding_set_external_ids(op->sb, >nbsp->external_ids);

There's a more general question of whether we want to copy everything,
or make sure we have an explicit understanding of external-ids we know
about.  Copying everything is really convenient, but it's also nice to
have a conversation about each use case in case there's something
better we can do that would benefit other projects as well.

In this case, based on our IRC conversation, there are 3 pieces of
information you'd like to have associated with a Port_Binding:

1) network prefix length for each IP address associated with the port

2) a "project ID" -- an openstack tenancy identifier

3) a "device ID" -- an ID for the openstack VM the port is associated with

#1 seems like something that could be more generally useful.  We
discussed this on IRC, but perhaps we should just update the addresses
column to optionally allow you specify a prefix length with an
address?

#2 and #3 seem OpenStack specific, and just leaving them as
external-ids that get copied over seems fine.  I'm also OK with a bulk
copy.  It may be nice to document the specific external-ids you plan
to use, just so people have a reference that explains what they are
when they go to debug an environment.

> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index eb348fe..7bb322f 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -848,7 +848,16 @@
>
>  
>
> -See External IDs at the beginning of this document.
> +
> +  See External IDs at the beginning of this document.
> +
> +
> +
> +  The ovn-northd program copies all these pairs into the
> +   column of the
> +   table in 
> +  database.
> +
>
>  
>
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] meta-flow: Remove dead condition in mf_set().

On Wed, May 31, 2017 at 02:02:43PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > mf_set() always takes a nonnull mask, but the MFF_CT_LABEL case checked
> > whether it was nonnull.
> >
> > Found by Coverity.
> >
> > Reported-at: 
> > https://scan3.coverity.com/reports.htm#v16889/p10449/fileInstanceId=14762941=4304057=179568
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> Acked-by: Aaron Conole 

Thanks!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb-client: Use correct operand to 'sizeof' in do_dump().

On Wed, May 31, 2017 at 02:09:08PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > When copying an object, one must calculate the size of the object itself,
> > not of its address.
> >
> > No visible effect, though, since both the object and its address are
> > pointers in this case.
> >
> > Found by Coverity.
> >
> > Reported-at: 
> > https://scan3.coverity.com/reports.htm#v16889/p10449/fileInstanceId=14762869=4304032=179550
> > Signed-off-by: Ben Pfaff 
> > ---
> >  ovsdb/ovsdb-client.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> > index 1df4fb4da92c..9ef3311270aa 100644
> > --- a/ovsdb/ovsdb-client.c
> > +++ b/ovsdb/ovsdb-client.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, 
> > Inc.
> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 
> > Nicira, Inc.
> 
> Maybe change this to
> + * Copyright (c) 2009-2017 Nicira, Inc.
> 
> Otherwise:
> 
> Acked-by: Aaron Conole 

Thanks, I applied this to master, with that correction.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] netflow: Fix memory leak in netflow_unref.

2017-05-31 Thread Joe Stringer

On 30 May 2017 at 09:47, Ben Pfaff  wrote:
> On Thu, May 25, 2017 at 01:47:32PM -0700, Joe Stringer wrote:
>> On 21 May 2017 at 21:55, Yunjian Wang  wrote:
>> > The memory leak was triggered each time on calling netflow_unref() with
>> > containing netflow_flows. And flows need to be removed and destroyed.
>> >
>> > Signed-off-by: Yunjian Wang 
>> > ---
>>
>> Does this also need to netflow_expire__() the flows? Would it make
>> sense for this flow clear path to share code with
>> netflow_flow_clear()?
>
> That would certainly make sense.  However, it adds a lot of extra code
> to run at the time we're freeing something, which is always a little
> risky.  Since Greg is having trouble testing this at all, I lean toward
> just freeing without trying to send out NetFlow expirations.

This seems fine to me. It looks like the biggest issue is the bug
that's fixed by the patch, which seems correct to me so I'd be happy
if this was applied as-is.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Also check switch, HMAP_FOR_EACH, etc.

On Tue, May 30, 2017 at 04:24:55PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > The switch statement and our FOR_EACH macro iteration constructs have the
> > same rules as if, for, and while.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> At first, I was thinking that the FOR_EACH constructs were treated
> separately, but actually it looks like there are just places where the
> usage is incorrect.

Yes, mistakes tend to creep in--perhaps this will help.

> Acked-by: Aaron Conole 

Thanks!  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] Replace most uses of and references to "ifconfig" by "ip".