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

[Han Zhou]

On Wed, Mar 2, 2016 at 1:43 PM, Russell Bryant  wrote:
>
> Prior to this commit, once a connection had been committed to the
> connection tracker, the connection would continue to be allowed, even
> if the policy defined in the ACL table changed.  This patch changes
> the implementation so that existing connections are affected by policy
> changes.
>
> The implementation is based on the suggested approach in this mailing
> list thread:
>
> http://openvswitch.org/pipermail/dev/2016-February/065716.html
>
> Instead of always allowing packets associated with an established
> connection, we now put all packets in the request direction through
> the flows generated based on OVN ACLs.  If a packet associated with an
> established connection hits a "drop" ACL, that means we have
> encountered a policy change and should drop packets associated with
> this connection from now on.  We handle this by setting "ct_mark" on
> the associated connection tracking entry.
>
> One difference in the implementation vs. the mailing list proposal is
> the use of "ct_mark" instead of "ct_label".  The OpenFlow interface to
> both fields is identical, other than the size of the field (32-bit for
> ct_mark, 128-bit for ct_label).  We see two uses for these fields in
> OVN: bit twiddling for state tracking (this patch) and possibly
> associating connection tracking entries with OVN ACLs.  The easiest
> way to do that sort of association would be to use a UUID, which is
> 128-bits, so we reserve ct_label for that purpose and use the 32-bits
> of ct_mark for custom state tracking.
>
> The proposal on the mailing list also discussed the idea that
> ovn-controller could periodically sweep the connection tracker and
> delete entries with ct_mark set.  That is not implemented in this
> patch.  Instead, we rely on connections dying since we're dropping
> its packets and then allowing the connection tracking entry to
> eventually time out.  More proactively clearing them out could be a
> future enhancement.
>
> As a realistic example of how this works, consider this security policy
> from an OpenStack+OVN development environment.
>
> +-+---+
> | name| security_group_rules  |
> +-+---+
> | default | egress, IPv4  |
> | | egress, IPv6  |
> | | ingress, IPv4, 22/tcp |
> | | ingress, IPv4, icmp   |
> +-+---+
>
> The OpenStack Neutron plugin creates ACLs that drop traffic by default
> and higher priority ACLs for each type of traffic that is allowed.  In
> this case, the ACLs for a port using the "default" security group are:
>
>   from-lport  1002 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" &&
ip4) allow-related
>   from-lport  1002 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" &&
ip6) allow-related
>   from-lport  1001 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" &&
ip) drop
> to-lport  1002 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" &&
ip4 && icmp4) allow-related
> to-lport  1002 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" &&
ip4 && tcp && tcp.dst == 22) allow-related
> to-lport  1001 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" &&
ip) drop
>
> which results in the following logical flows:
>
> ...
>   table=1(   ls_in_pre_acl), priority=  100, match=(ip), action=(ct_next;)
>   table=1(   ls_in_pre_acl), priority=0, match=(1), action=(next;)
>   table=2(   ls_in_acl), priority=65535, match=(!ct.est && ct.rel &&
!ct.new && !ct.inv && ct_mark[0] == 0x0), action=(next;)
>   table=2(   ls_in_acl), priority=65535, match=(ct.est && !ct.rel &&
!ct.new && !ct.inv && ct.rpl && ct_mark[0] == 0x0), action=(next;)
>   table=2(   ls_in_acl), priority=65535, match=(ct.inv || ct_mark[0]
== 0x1), action=(drop;)
>   table=2(   ls_in_acl), priority= 2002, match=(!ct.new && ct.est &&
!ct.rpl && (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4)),
action=(next;)
>   table=2(   ls_in_acl), priority= 2002, match=(!ct.new && ct.est &&
!ct.rpl && (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6)),
action=(next;)
>   table=2(   ls_in_acl), priority= 2002, match=(ct.new && !ct.est &&
(inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4)),
action=(ct_commit; next;)
>   table=2(   ls_in_acl), priority= 2002, match=(ct.new && !ct.est &&
(inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6)),
action=(ct_commit; next;)
>   table=2(   ls_in_acl), priority= 2001, match=(!ct.est && (inport ==
"0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip)), action=(drop;)
>   table=2(   ls_in_acl), priority= 2001, match=(ct.est && (inport ==
"0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip)), action=(ct_commit(ct_mark =
1);)
>   table=2(   ls_in_acl), priority=1, match=(ip && !ct.est),
action=(ct_commit; next;)
>   table=2(   ls_in_acl), priority=0, match=(1), action=(next;)
> ...
>   table=0(  

Re: [ovs-dev] [PATCH] ovsdb-server: Fix a reference count leak bug

[Liran Schour]

> When destroying an ovsdb_jsonrpc_monitor, the jsonrpc monitor still
> holds a reference count to the monitors 'changes' indexed with
> 'unflushed' transaction id.  The bug is that the reference count was
> not decremented as it should in the code path.
> 
> The bug caused 'changes' that have been flushed to all jsonrpc
> clients to linger around unnecessarily, occupying increasingly
> large amount of memory. See "Reported-at" URL for more details.
> 
> This bug is tricky to find since the memory is not leaked; they will
> eventually be freed when monitors are destroyed.
> 
> Reported-by: Lei Huang 
> Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html
> Signed-off-by: Andy Zhou 
> ---
>  AUTHORS| 1 +
>  ovsdb/jsonrpc-server.c | 4 ++--
>  ovsdb/monitor.c| 9 -
>  ovsdb/monitor.h| 6 ++
>  4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/AUTHORS b/AUTHORS
> index 1184a51..0ba0f58 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -119,6 +119,7 @@ Kyle Mesterymest...@mestery.com
>  Kyle Upton  kup...@baymicrosystems.com
>  Lance Richardsonlrich...@redhat.com
>  Lars Kellogg-Stedmanl...@redhat.com
> +Lei Huang   huang.f@gmail.com
>  Leo Altermanlalter...@nicira.com
>  Lilijun jerry.lili...@huawei.com
>  Linda Sun   l...@vmware.com
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index 15dbc4e..caaf2bf 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -1265,7 +1265,7 @@ ovsdb_jsonrpc_monitor_create(struct 
> ovsdb_jsonrpc_session *s, struct ovsdb *db,
>  dbmon = ovsdb_monitor_add(m->dbmon);
>  if (dbmon != m->dbmon) {
>  /* Found an exisiting dbmon, reuse the current one. */
> -ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
> +ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, 
m->unflushed);
>  ovsdb_monitor_add_jsonrpc_monitor(dbmon, m);
>  m->dbmon = dbmon;
>  }
> @@ -1348,7 +1348,7 @@ ovsdb_jsonrpc_monitor_destroy(struct 
> ovsdb_jsonrpc_monitor *m)
>  {
>  json_destroy(m->monitor_id);
>  hmap_remove(>session->monitors, >node);
> -ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
> +ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
>  free(m);
>  }
> 
> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index 0d81d89..6b0d13d 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -959,7 +959,8 @@ ovsdb_monitor_get_initial(const struct 
> ovsdb_monitor *dbmon)
> 
>  void
>  ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
> -   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
> +   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
> +   uint64_t unflushed)
>  {
>  struct jsonrpc_monitor_node *jm;
> 
> @@ -971,6 +972,12 @@ ovsdb_monitor_remove_jsonrpc_monitor(struct 
> ovsdb_monitor *dbmon,
>  /* Find and remove the jsonrpc monitor from the list.  */
>  LIST_FOR_EACH(jm, node, >jsonrpc_monitors) {
>  if (jm->jsonrpc_monitor == jsonrpc_monitor) {
> +/* Release the tracked changes. */
> +struct shash_node *node;
> +SHASH_FOR_EACH (node, >tables) {
> +struct ovsdb_monitor_table *mt = node->data;
> +ovsdb_monitor_table_untrack_changes(mt, unflushed);
> +}
>  list_remove(>node);
>  free(jm);
> 
> diff --git a/ovsdb/monitor.h b/ovsdb/monitor.h
> index fb10435..9fea831 100644
> --- a/ovsdb/monitor.h
> +++ b/ovsdb/monitor.h
> @@ -46,10 +46,8 @@ void ovsdb_monitor_add_jsonrpc_monitor(struct 
> ovsdb_monitor *dbmon,
> struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
> 
>  void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
> -   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
> -
> -void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
> -   struct ovsdb_jsonrpc_monitor 
> *jsonrpc_monitor);
> +   struct ovsdb_jsonrpc_monitor 
*jsonrpc_monitor,
> +   uint64_t unflushed);
> 
>  void ovsdb_monitor_add_table(struct ovsdb_monitor *m,
>   const struct ovsdb_table *table);
> -- 
> 1.9.1
> 

Acked-by: Liran Schour 

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

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

[Ofer Ben-Yacov]

Hi Ben,

I'm still waiting for you response.
Do you see the reason for the current usage of IDL, where Neutron and my
patch get the schema from the server, or do you insist that the schema will
be read from a file?

I do think that if Neutron is using is like that and I took this approach
too than it is probably feature that is needed and IDL should support it. I
would go even further and add the ability to get the schema from the server
into SchemaHelper or IDL instead of reading it outside of these classes and
send it in, like Neutron does.

Regards,
Ofer.






‫בתאריך יום ד׳, 2 במרץ 2016 ב-9:47 מאת ‪Ofer Ben-Yacov‬‏ <‪
ofer.benya...@gmail.com‬‏>:‬

> Actually, SchemaHelper supports both ways: getting the schema from the
> server or read it from file (schema_json / location parameters). Neutron
> took the first approach probably because they wanted all the tables anyway
> .
>
>
> If we want to keep supporting getting the schema from the server, than my
> change of sending the open stream to IDL is a must.
> If you want to have the file option only (even if it is currently coded
> differently), than I will change the patch to support only getting the
> schema form a file.
>
>
> Ofer.
>
>
> ‫בתאריך יום ב׳, 29 בפבר׳ 2016 ב-19:58 מאת ‪Ben Pfaff‬‏ <‪b...@ovn.org‬‏>:‬
>
>> That's not how the Python version of the IDL is meant to be used.
>> SchemaHelper operates on the schema that the client *wants*, not on the
>> schema that the database server *has*.  That's why it reads the schema
>> from a local disk file by default.  If Neutron does something else then
>> it should probably be changed.
>>
>> On Mon, Feb 29, 2016 at 08:06:33AM +, Ofer Ben-Yacov wrote:
>> > Take a loot at the contractor of IDL:
>> >
>> > ...
>> > def __init__(self, remote, schema):
>> > ...
>> >
>> >
>> > You can see that it gets schema as a parameter, which is actually
>> > SchemaHelper object:
>> >
>> > ...
>> > assert isinstance(schema, SchemaHelper)
>> > ...
>> >
>> >
>> > This object holds the schema in a json string (schema_json) until you
>> will
>> > call its register_columns function which will build the tables.
>> >
>> > You can see that the IDL, when first built, is getting the schema which
>> > means that the OVSDB server was already connected beforehand to get it.
>> >
>> > If you should like to know how this is done, you can look at Neutron's
>> > idlutils.py file and check the get_schema_helper() function.
>> >
>> > You can see that it gets the schema name as a parameter, connects to the
>> > OVSDB, get the schema, close the connection and
>> > return SchemaHelper object to be used for the above.
>> >
>> >
>> > Note that after this patch will be committed, I'm going to send a patch
>> to
>> > Neutron to add another function similar to get_schema_helper() with the
>> > the deference that it will get stream (open connection to the OVSDB
>> server)
>> > as a parameter and get the schema from the server without closing it
>> when
>> > done.
>> >
>> > Ofer.
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > ‫בתאריך שבת, 27 בפבר׳ 2016 ב-3:13 מאת ‪Ben Pfaff‬‏ <‪b...@ovn.org‬‏>:‬
>> >
>> > > On Wed, Feb 24, 2016 at 08:39:02AM +, Ofer Ben-Yacov wrote:
>> > > > The current implementation IDL is:
>> > > > 1. connects to the OVSDB
>> > > > 2. get the schema and
>> > > > 3. disconnect
>> > > > 4. connect again to work with the OVSDB server.
>> > >
>> > > I don't understand--where do you see that behavior?  I can't spot it
>> > > anywhere.  The code I see in python/ovs/db/idl.py doesn't seem to do
>> > > anything like it.
>> > >
>>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] bond: don't re-zero recirc_id when creating bond

[Simon Horman]

The bond structure is already zeroed as it is allocated
using xzalloc so there is no need to re-zero the recirc_id field.

Signed-off-by: Simon Horman 
---
Found by inspection.
---
 ofproto/bond.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index c2749e52db94..a3d0c2d6faff 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -237,8 +237,6 @@ bond_create(const struct bond_settings *s, struct 
ofproto_dpif *ofproto)
 list_init(>enabled_slaves);
 ovs_mutex_init(>mutex);
 ovs_refcount_init(>ref_cnt);
-
-bond->recirc_id = 0;
 hmap_init(>pr_rule_ops);
 
 bond_reconfigure(bond, s);
-- 
2.1.4

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

Re: [ovs-dev] [PATCH 4/4] openflow: Support matching and modifying MPLS TTL field.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 05:38:59PM -0800, Justin Pettit wrote:
> 
> > On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> > 
> > +/* Modifies 'match' so that the MPLS TTL is wildcarded. */
> > +void
> > +match_set_any_mpls_ttl(struct match *match, int idx)
> > +{
> > +match->wc.masks.mpls_lse[idx] &= ~htonl(MPLS_TTL_MASK);
> > +flow_set_mpls_ttl(>flow, idx, 0);
> > +}
> > +
> > +/* Modifies 'match' so that it matches only packets with an MPLS header 
> > whose
> > + * TTL equals 'mpls_ttl' */
> > +void
> > +match_set_mpls_ttl(struct match *match, int idx, uint8_t mpls_ttl)
> > +{
> > +match->wc.masks.mpls_lse[idx] |= htonl(MPLS_TTL_MASK);
> > +flow_set_mpls_ttl(>flow, idx, mpls_ttl);
> > +}
> 
> Do you think it's worth documenting the "idx" arguments for these two 
> functions?

Fixed, thanks.

> > +/* "mpls_ttl".
> > + *
> > + * The outermost MPLS label's time-to-level (TTL) field, or 0 if no 
> > MPLS
> 
> Shouldn't that be "time-to-live"?  Or is that how MPLS indicates that it 
> wants to have a serious talk.

Fixed, thanks.

> Acked-by: Justin Pettit 

Thanks, I'll rerun the tests and apply this to master in a minute.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/4] netdev: Improve comments on netdev_rxq_recv().

[Ben Pfaff]

On Mon, Mar 07, 2016 at 05:31:17PM -0800, Justin Pettit wrote:
> 
> > On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> > 
> > The comment was incomplete in some ways and simply wrong in others.
> > 
> > Also ensure that *cnt is set to 0 if an error is encountered.  It's nice
> > when callers can rely on this.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

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

Re: [ovs-dev] [PATCH 2/4] dpif-netdev: Fix typo in comment.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 05:26:35PM -0800, Justin Pettit wrote:
> 
> > On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

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

Re: [ovs-dev] [PATCH 1/4] ovs-ofctl: Fix command names in documentation.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 05:25:41PM -0800, Justin Pettit wrote:
> 
> > On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> > 
> > The actual command names do not capitalize "tlv".
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

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

Re: [ovs-dev] why we need op wait when add-port or delete port

[Ben Pfaff]

On Tue, Mar 08, 2016 at 11:43:53AM +0800, ychen wrote:
>   I noticed that when add or delete port, the transaction always send op wait 
> with all the ports in the same bridge.
>  If the port p0 and p1 are totally independent, and have no relationship with 
> each other, why when I add port p1, I need to wait port p0?

This implements transactional atomicity.  If two transactions each add a
different port, and they execute at the same time, one of them needs to
fail and be retried, otherwise only one of the ports will be added in
the end.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] why we need op wait when add-port or delete port

[ychen]

hi:
  I noticed that when add or delete port, the transaction always send op wait 
with all the ports in the same bridge.
 If the port p0 and p1 are totally independent, and have no relationship with 
each other, why when I add port p1, I need to wait port p0?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] netdev-dpdk: fix mbuf leaks

[Yuanhan Liu]

mbufs could be chained (by the "next" field of rte_mbuf struct), when
an mbuf is not big enough to hold a big packet, say when TSO is enabled.

rte_pktmbuf_free_seg() frees the head mbuf only, leading mbuf leaks.
This patch fix it by invoking the right API rte_pktmbuf_free(), to
free all mbufs in the chain.

Signed-off-by: Yuanhan Liu 
---
 lib/netdev-dpdk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 0233b3c..f402354 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -396,7 +396,7 @@ free_dpdk_buf(struct dp_packet *p)
 {
 struct rte_mbuf *pkt = (struct rte_mbuf *) p;
 
-rte_pktmbuf_free_seg(pkt);
+rte_pktmbuf_free(pkt);
 }
 
 static void
@@ -1089,7 +1089,7 @@ dpdk_queue_flush__(struct netdev_dpdk *dev, int qid)
 int i;
 
 for (i = nb_tx; i < txq->count; i++) {
-rte_pktmbuf_free_seg(txq->burst_pkts[i]);
+rte_pktmbuf_free(txq->burst_pkts[i]);
 }
 rte_spinlock_lock(>stats_lock);
 dev->stats.tx_dropped += txq->count-nb_tx;
-- 
1.9.0

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration

[Alin Serdean]

Still that breaks our prerequisites (elementname of the VIFs are unique) and 
does not inform the user regarding the error behind it.

We could write a wrapper over Enable-Vmswitchextension, i.e. 
Enable-OvsExtension, in which we require a switch name as a mandatory field, in 
that way we can avoid the extension to be run on two or more switches, and also 
check the elementnames of the VIFs and inform the user if the activation failed 
and why.

Alin.

> -Mesaj original-
> De la: Nithin Raju [mailto:nit...@vmware.com]
> Trimis: Tuesday, March 8, 2016 3:20 AM
> Către: Alin Serdean ;
> dev@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs
> during port enumeration
> 
> Hi Alin,
> Valid point. I’ll update the code.
> 
> The change I was trying to make was as follows. If user forgot to assign OVS
> names to for VIFs, the first VIF gets added to the hash tables but subsequent
> ones will throw errors - either a NDIS_STATUS_DATA_NOT_ACCEPTED or
> NDIS_STATUS_ADAPTER_NOT_FOUND. For these two specific errors, I didn’t
> want to panic NDIS by reporting error since it is an OVS logic to not add 
> these
> ports/NICs.
> 
> I agree that things like memory allocation are more serious issues and should
> be reported to NDIS.
> 
> -- Nithin
> 
> 
> -Original Message-
> From: Alin Serdean 
> Date: Monday, March 7, 2016 at 3:40 PM
> To: Nithin Raju , "dev@openvswitch.org"
> 
> Subject: RE: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs
> duringport enumeration
> 
> >Unless I am reading wrong: OvsAddConfiguredSwitchPorts and
> >OvsInitConfiguredSwitchNics only fail if we could not allocate memory
> >or could not issue an OID request.
> >
> >
> >
> >I am not in favor of reporting back to NDIS gracefully if any of the
> >above conditions was broken even once.
> >
> >
> >
> >Thanks,
> >
> >Alin.
> >
> >
> >
> >> -Mesaj original-
> >
> >> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju
> >
> >> Trimis: Thursday, March 3, 2016 10:51 AM
> >
> >> Către: dev@openvswitch.org
> >
> >> Subiect: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs
> >> during
> >
> >> port enumeration
> >
> >>
> >
> >> While enumerating the ports on a switch, if adding one of the ports
> >>fails, we
> >
> >> are not handling that error gracefully. Instead, we fail adding of
> >>all the ports.
> >
> >> This patch rectifies that.
> >
> >>
> >
> >> Signed-off-by: Nithin Raju 
> >
> >> ---
> >
> >>  datapath-windows/ovsext/Vport.c | 41
> >
> >> +
> >
> >>  1 file changed, 29 insertions(+), 12 deletions(-)
> >
> >>
> >
> >> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
> >
> >> windows/ovsext/Vport.c index 7b0103d..a991d10 100644
> >
> >> --- a/datapath-windows/ovsext/Vport.c
> >
> >> +++ b/datapath-windows/ovsext/Vport.c
> >
> >> @@ -125,7 +125,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> >
> >> switchContext,
> >
> >>  if (vport != NULL) {
> >
> >>  OVS_LOG_ERROR("Port add failed due to duplicate port name, "
> >
> >>"port Id: %u", portParam->PortId);
> >
> >> -status = STATUS_DATA_NOT_ACCEPTED;
> >
> >> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
> >
> >>  goto create_port_done;
> >
> >>  }
> >
> >>
> >
> >> @@ -140,7 +140,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> >
> >> switchContext,
> >
> >>  OVS_LOG_ERROR("Port add failed since a port already exists on "
> >
> >>"the specified port Id: %u, ovsName: %s",
> >
> >>portParam->PortId, vport->ovsName);
> >
> >> -status = STATUS_DATA_NOT_ACCEPTED;
> >
> >> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
> >
> >>  goto create_port_done;
> >
> >>  }
> >
> >>
> >
> >> @@ -157,7 +157,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> >
> >> switchContext,
> >
> >>  OVS_LOG_INFO("Port add failed due to PortType change,
> >>port
> >>Id: %u"
> >
> >>   " old: %u, new: %u", portParam->PortId,
> >
> >>   vport->portType, portParam->PortType);
> >
> >> -status = STATUS_DATA_NOT_ACCEPTED;
> >
> >> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
> >
> >>  goto create_port_done;
> >
> >>  }
> >
> >>  vport->isAbsentOnHv = FALSE;
> >
> >> @@ -365,7 +365,7 @@ HvCreateNic(POVS_SWITCH_CONTEXT
> >
> >> switchContext,
> >
> >>  OVS_LOG_ERROR("Create NIC without Switch Port,"
> >
> >>" PortId: %x, NicIndex: %d",
> >
> >>nicParam->PortId, nicParam->NicIndex);
> >
> >> -status = NDIS_STATUS_INVALID_PARAMETER;
> >
> >> +status = NDIS_STATUS_ADAPTER_NOT_FOUND;
> >
> >>  goto add_nic_done;
> >
> >>  }
> >
> >>  

Re: [ovs-dev] [PATCH 4/4] openflow: Support matching and modifying MPLS TTL field.

[Justin Pettit]

> On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> 
> +/* Modifies 'match' so that the MPLS TTL is wildcarded. */
> +void
> +match_set_any_mpls_ttl(struct match *match, int idx)
> +{
> +match->wc.masks.mpls_lse[idx] &= ~htonl(MPLS_TTL_MASK);
> +flow_set_mpls_ttl(>flow, idx, 0);
> +}
> +
> +/* Modifies 'match' so that it matches only packets with an MPLS header whose
> + * TTL equals 'mpls_ttl' */
> +void
> +match_set_mpls_ttl(struct match *match, int idx, uint8_t mpls_ttl)
> +{
> +match->wc.masks.mpls_lse[idx] |= htonl(MPLS_TTL_MASK);
> +flow_set_mpls_ttl(>flow, idx, mpls_ttl);
> +}

Do you think it's worth documenting the "idx" arguments for these two functions?

> +/* "mpls_ttl".
> + *
> + * The outermost MPLS label's time-to-level (TTL) field, or 0 if no MPLS

Shouldn't that be "time-to-live"?  Or is that how MPLS indicates that it wants 
to have a serious talk.

Acked-by: Justin Pettit 

--Justin


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

Re: [ovs-dev] [PATCH 3/4] netdev: Improve comments on netdev_rxq_recv().

[Justin Pettit]

> On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> 
> The comment was incomplete in some ways and simply wrong in others.
> 
> Also ensure that *cnt is set to 0 if an error is encountered.  It's nice
> when callers can rely on this.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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

Re: [ovs-dev] [PATCH 2/4] dpif-netdev: Fix typo in comment.

[Justin Pettit]

> On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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

Re: [ovs-dev] [PATCH] ovsdb-server: Fix a reference count leak bug

[Han Zhou]

On Mon, Mar 7, 2016 at 3:44 PM, Andy Zhou  wrote:
>
> When destroying an ovsdb_jsonrpc_monitor, the jsonrpc monitor still
> holds a reference count to the monitors 'changes' indexed with
> 'unflushed' transaction id.  The bug is that the reference count was
> not decremented as it should in the code path.
>
> The bug caused 'changes' that have been flushed to all jsonrpc
> clients to linger around unnecessarily, occupying increasingly
> large amount of memory. See "Reported-at" URL for more details.
>
> This bug is tricky to find since the memory is not leaked; they will
> eventually be freed when monitors are destroyed.
>
> Reported-by: Lei Huang 
> Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html
> Signed-off-by: Andy Zhou 
> ---
>  AUTHORS| 1 +
>  ovsdb/jsonrpc-server.c | 4 ++--
>  ovsdb/monitor.c| 9 -
>  ovsdb/monitor.h| 6 ++
>  4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index 1184a51..0ba0f58 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -119,6 +119,7 @@ Kyle Mesterymest...@mestery.com
>  Kyle Upton  kup...@baymicrosystems.com
>  Lance Richardsonlrich...@redhat.com
>  Lars Kellogg-Stedmanl...@redhat.com
> +Lei Huang   huang.f@gmail.com
>  Leo Altermanlalter...@nicira.com
>  Lilijun jerry.lili...@huawei.com
>  Linda Sun   l...@vmware.com
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index 15dbc4e..caaf2bf 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -1265,7 +1265,7 @@ ovsdb_jsonrpc_monitor_create(struct
ovsdb_jsonrpc_session *s, struct ovsdb *db,
>  dbmon = ovsdb_monitor_add(m->dbmon);
>  if (dbmon != m->dbmon) {
>  /* Found an exisiting dbmon, reuse the current one. */
> -ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
> +ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
>  ovsdb_monitor_add_jsonrpc_monitor(dbmon, m);
>  m->dbmon = dbmon;
>  }
> @@ -1348,7 +1348,7 @@ ovsdb_jsonrpc_monitor_destroy(struct
ovsdb_jsonrpc_monitor *m)
>  {
>  json_destroy(m->monitor_id);
>  hmap_remove(>session->monitors, >node);
> -ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
> +ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
>  free(m);
>  }
>
> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index 0d81d89..6b0d13d 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -959,7 +959,8 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor
*dbmon)
>
>  void
>  ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
> -   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
> +   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
> +   uint64_t unflushed)
>  {
>  struct jsonrpc_monitor_node *jm;
>
> @@ -971,6 +972,12 @@ ovsdb_monitor_remove_jsonrpc_monitor(struct
ovsdb_monitor *dbmon,
>  /* Find and remove the jsonrpc monitor from the list.  */
>  LIST_FOR_EACH(jm, node, >jsonrpc_monitors) {
>  if (jm->jsonrpc_monitor == jsonrpc_monitor) {
> +/* Release the tracked changes. */
> +struct shash_node *node;
> +SHASH_FOR_EACH (node, >tables) {
> +struct ovsdb_monitor_table *mt = node->data;
> +ovsdb_monitor_table_untrack_changes(mt, unflushed);

Just a question, why passing in "unflushed" as a parameter instead of using
jsonrpc_monitor->unflushed directly?

Otherwise looks good, and it is verified in the scale test env.

> +}
>  list_remove(>node);
>  free(jm);
>
> diff --git a/ovsdb/monitor.h b/ovsdb/monitor.h
> index fb10435..9fea831 100644
> --- a/ovsdb/monitor.h
> +++ b/ovsdb/monitor.h
> @@ -46,10 +46,8 @@ void ovsdb_monitor_add_jsonrpc_monitor(struct
ovsdb_monitor *dbmon,
> struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
>
>  void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
> -   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
> -
> -void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
> -   struct ovsdb_jsonrpc_monitor
*jsonrpc_monitor);
> +   struct ovsdb_jsonrpc_monitor
*jsonrpc_monitor,
> +   uint64_t unflushed);
>
>  void ovsdb_monitor_add_table(struct ovsdb_monitor *m,
>   const struct ovsdb_table *table);
> --
> 1.9.1
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

Acked-by: Han Zhou 

--
Best regards,
Han
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/4] ovs-ofctl: Fix command names in documentation.

[Justin Pettit]

> On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> 
> The actual command names do not capitalize "tlv".
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin



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

Re: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration

[Nithin Raju]

Hi Alin,
Valid point. I’ll update the code.

The change I was trying to make was as follows. If user forgot to assign
OVS names to for VIFs, the first VIF gets added to the hash tables but
subsequent ones will throw errors - either a NDIS_STATUS_DATA_NOT_ACCEPTED
or NDIS_STATUS_ADAPTER_NOT_FOUND. For these two specific errors, I didn’t
want to panic NDIS by reporting error since it is an OVS logic to not add
these ports/NICs.

I agree that things like memory allocation are more serious issues and
should be reported to NDIS.

-- Nithin


-Original Message-
From: Alin Serdean 
Date: Monday, March 7, 2016 at 3:40 PM
To: Nithin Raju , "dev@openvswitch.org"

Subject: RE: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs
during  port enumeration

>Unless I am reading wrong: OvsAddConfiguredSwitchPorts and
>OvsInitConfiguredSwitchNics only fail if we could not allocate memory or
>could not issue an OID request.
>
>
>
>I am not in favor of reporting back to NDIS gracefully if any of the
>above conditions was broken even once.
>
>
>
>Thanks,
>
>Alin.
>
>
>
>> -Mesaj original-
>
>> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju
>
>> Trimis: Thursday, March 3, 2016 10:51 AM
>
>> Către: dev@openvswitch.org
>
>> Subiect: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during
>
>> port enumeration
>
>> 
>
>> While enumerating the ports on a switch, if adding one of the ports
>>fails, we
>
>> are not handling that error gracefully. Instead, we fail adding of all
>>the ports.
>
>> This patch rectifies that.
>
>> 
>
>> Signed-off-by: Nithin Raju 
>
>> ---
>
>>  datapath-windows/ovsext/Vport.c | 41
>
>> +
>
>>  1 file changed, 29 insertions(+), 12 deletions(-)
>
>> 
>
>> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
>
>> windows/ovsext/Vport.c index 7b0103d..a991d10 100644
>
>> --- a/datapath-windows/ovsext/Vport.c
>
>> +++ b/datapath-windows/ovsext/Vport.c
>
>> @@ -125,7 +125,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
>
>> switchContext,
>
>>  if (vport != NULL) {
>
>>  OVS_LOG_ERROR("Port add failed due to duplicate port name, "
>
>>"port Id: %u", portParam->PortId);
>
>> -status = STATUS_DATA_NOT_ACCEPTED;
>
>> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
>
>>  goto create_port_done;
>
>>  }
>
>> 
>
>> @@ -140,7 +140,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
>
>> switchContext,
>
>>  OVS_LOG_ERROR("Port add failed since a port already exists on "
>
>>"the specified port Id: %u, ovsName: %s",
>
>>portParam->PortId, vport->ovsName);
>
>> -status = STATUS_DATA_NOT_ACCEPTED;
>
>> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
>
>>  goto create_port_done;
>
>>  }
>
>> 
>
>> @@ -157,7 +157,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
>
>> switchContext,
>
>>  OVS_LOG_INFO("Port add failed due to PortType change, port
>>Id: %u"
>
>>   " old: %u, new: %u", portParam->PortId,
>
>>   vport->portType, portParam->PortType);
>
>> -status = STATUS_DATA_NOT_ACCEPTED;
>
>> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
>
>>  goto create_port_done;
>
>>  }
>
>>  vport->isAbsentOnHv = FALSE;
>
>> @@ -365,7 +365,7 @@ HvCreateNic(POVS_SWITCH_CONTEXT
>
>> switchContext,
>
>>  OVS_LOG_ERROR("Create NIC without Switch Port,"
>
>>" PortId: %x, NicIndex: %d",
>
>>nicParam->PortId, nicParam->NicIndex);
>
>> -status = NDIS_STATUS_INVALID_PARAMETER;
>
>> +status = NDIS_STATUS_ADAPTER_NOT_FOUND;
>
>>  goto add_nic_done;
>
>>  }
>
>>  OvsInitVportWithNicParam(switchContext, vport, nicParam); @@
>>-1393,6
>
>> +1393,7 @@ OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT
>
>> switchContext)
>
>>  ULONG arrIndex;
>
>>  PNDIS_SWITCH_PORT_PARAMETERS portParam;
>
>>  PNDIS_SWITCH_PORT_ARRAY portArray = NULL;
>
>> +BOOLEAN portAdded = FALSE;
>
>> 
>
>>  OVS_LOG_TRACE("Enter: switchContext:%p", switchContext);
>
>> 
>
>> @@ -1402,16 +1403,24 @@
>
>> OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT switchContext)
>
>>  }
>
>> 
>
>>  for (arrIndex = 0; arrIndex < portArray->NumElements; arrIndex++) {
>
>> - portParam = NDIS_SWITCH_PORT_AT_ARRAY_INDEX(portArray,
>
>> arrIndex);
>
>> +portParam = NDIS_SWITCH_PORT_AT_ARRAY_INDEX(portArray,
>
>> + arrIndex);
>
>> 
>
>> - if (portParam->IsValidationPort) {
>
>> - continue;
>
>> - }
>
>> +if (portParam->IsValidationPort) {
>
>> +continue;
>
>> +}
>
>> 
>
>> - status = HvCreatePort(switchContext, portParam, 0);
>
>> - if (status != STATUS_SUCCESS && status !=
>
>> STATUS_DATA_NOT_ACCEPTED) {
>
>> -

[ovs-dev] [PATCH v4 1/6] datapath-windows: Added recirculation support.

[Alin Serdean]

Recirculation support for the OVS extension.

Tested using PING and iperf with Driver Verifier enabled.

Signed-off-by: Sorin Vinturis 
Co-authored-by: Alin Gabriel Serdean 
Reported-by: Sorin Vinturis 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/104
---
v2: Initialize flow key before using it.
v3: Synchronized access to deferred actions queue.
v4: Address comments.
---
 datapath-windows/automake.mk  |   3 +
 datapath-windows/ovsext/Actions.c | 224 +++---
 datapath-windows/ovsext/Actions.h |  54 +++
 datapath-windows/ovsext/Datapath.c|   4 +
 datapath-windows/ovsext/DpInternal.h  |   2 +-
 datapath-windows/ovsext/Flow.c|  76 --
 datapath-windows/ovsext/Flow.h|   5 +-
 datapath-windows/ovsext/Netlink/Netlink.h |  11 ++
 datapath-windows/ovsext/PacketIO.c|  16 ++-
 datapath-windows/ovsext/PacketIO.h|  10 --
 datapath-windows/ovsext/Recirc.c  | 171 +++
 datapath-windows/ovsext/Recirc.h  |  82 +++
 datapath-windows/ovsext/Tunnel.c  |  15 +-
 datapath-windows/ovsext/User.c|  13 +-
 datapath-windows/ovsext/ovsext.vcxproj|   3 +
 15 files changed, 623 insertions(+), 66 deletions(-)
 create mode 100644 datapath-windows/ovsext/Actions.h
 create mode 100644 datapath-windows/ovsext/Recirc.c
 create mode 100644 datapath-windows/ovsext/Recirc.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index f29f548..04fc97f 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -9,6 +9,7 @@ EXTRA_DIST += \
datapath-windows/misc/uninstall.cmd \
datapath-windows/ovsext.sln \
datapath-windows/ovsext/Actions.c \
+   datapath-windows/ovsext/Actions.h \
datapath-windows/ovsext/Atomic.h \
datapath-windows/ovsext/BufferMgmt.c \
datapath-windows/ovsext/BufferMgmt.h \
@@ -45,6 +46,8 @@ EXTRA_DIST += \
datapath-windows/ovsext/PacketIO.h \
datapath-windows/ovsext/PacketParser.c \
datapath-windows/ovsext/PacketParser.h \
+   datapath-windows/ovsext/Recirc.c \
+   datapath-windows/ovsext/Recirc.h \
datapath-windows/ovsext/Stt.c \
datapath-windows/ovsext/Stt.h \
datapath-windows/ovsext/Switch.c \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 5a04541..e2d4ba8 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -16,6 +16,7 @@
 
 #include "precomp.h"
 
+#include "Actions.h"
 #include "Debug.h"
 #include "Event.h"
 #include "Flow.h"
@@ -24,6 +25,7 @@
 #include "NetProto.h"
 #include "Offload.h"
 #include "PacketIO.h"
+#include "Recirc.h"
 #include "Stt.h"
 #include "Switch.h"
 #include "User.h"
@@ -35,6 +37,8 @@
 #endif
 #define OVS_DBG_MOD OVS_DBG_ACTION
 
+#define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2
+
 typedef struct _OVS_ACTION_STATS {
 UINT64 rxGre;
 UINT64 txGre;
@@ -66,7 +70,6 @@ OVS_ACTION_STATS ovsActionStats;
  * exercised while adding new members to the structure - only add ones that get
  * used across multiple stages in the pipeline/get used in multiple functions.
  */
-#define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2
 typedef struct OvsForwardingContext {
 POVS_SWITCH_CONTEXT switchContext;
 /* The NBL currently used in the pipeline. */
@@ -99,7 +102,7 @@ typedef struct OvsForwardingContext {
  */
 OvsIPv4TunnelKey tunKey;
 
- /*
+/*
  * Tunneling - Tx:
  * To store the output port, when it is a tunneled port. We don't foresee
  * multiple tunneled ports as outport for any given NBL.
@@ -117,7 +120,6 @@ typedef struct OvsForwardingContext {
 OVS_PACKET_HDR_INFO layers;
 } OvsForwardingContext;
 
-
 /*
  * --
  * OvsInitForwardingCtx --
@@ -564,10 +566,10 @@ OvsCompleteNBLForwardingCtx(OvsForwardingContext 
*ovsFwdCtx,
 static __inline NDIS_STATUS
 OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx)
 {
-OvsFlowKey key;
-OvsFlow *flow;
-UINT64 hash;
-NDIS_STATUS status;
+OvsFlowKey key = { 0 };
+OvsFlow *flow = NULL;
+UINT64 hash = 0;
+NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_VPORT_ENTRY vport =
 OvsFindVportByPortNo(ovsFwdCtx->switchContext, ovsFwdCtx->srcVportNo);
 if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) {
@@ -595,11 +597,13 @@ OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx)
 if (flow) {
 OvsFlowUsed(flow, ovsFwdCtx->curNbl, >layers);
 ovsFwdCtx->switchContext->datapath.hits++;
-status = OvsActionsExecute(ovsFwdCtx->switchContext,
-   ovsFwdCtx->completionList, 
ovsFwdCtx->curNbl,
-   ovsFwdCtx->srcVportNo, 

Re: [ovs-dev] [PATCH] tests: Expand 'bundle with many ports' test.

[Joe Stringer]

On 7 March 2016 at 15:39, Ben Pfaff  wrote:
> On Mon, Mar 07, 2016 at 03:36:38PM -0800, Joe Stringer wrote:
>> Explain what this test is doing, and check that the decoded action can
>> be re-encoded and dumped back out of OVS.
>>
>> Suggested-by: Ben Pfaff 
>> Signed-off-by: Joe Stringer 
>
> Acked-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH] ofp-actions: Assert variable actions have len>0.

[Joe Stringer]

On 7 March 2016 at 15:38, Ben Pfaff  wrote:
> On Mon, Mar 07, 2016 at 03:36:36PM -0800, Joe Stringer wrote:
>> Variable-length actions must have a nonzero length; if they don't,
>> something went wrong and we should bail out.
>>
>> Suggested-by: Ben Pfaff 
>> Signed-off-by: Joe Stringer 
>
> Acked-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH] Revert "ovn-controller: race between binding-run and patch-run for localnet ports"

[Ben Pfaff]

Thanks, applied.

On Mon, Mar 07, 2016 at 06:42:18PM -0500, Russell Bryant wrote:
> Acked-by: Russell Bryant 
> 
> On Monday, March 7, 2016, Ben Pfaff  wrote:
> 
> > This reverts commit 3a83007a76bbf05144cee1fda7ad81c1c717dca7.  It's really
> > nonobvious from the code why the condition added by that commit makes
> > sense.
> > The new condition should not be necessary now that binding_run() always
> > keeps
> > track of the local datapaths, since commit 7c040135cf351 (binding: Track
> > local
> > datapaths even when no transaction is possible).
> >
> > CC: Ramu Ramamurthy >
> > Signed-off-by: Ben Pfaff >
> > ---
> >  ovn/controller/patch.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> > index cfae613..753ce3e 100644
> > --- a/ovn/controller/patch.c
> > +++ b/ovn/controller/patch.c
> > @@ -280,7 +280,7 @@ void
> >  patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> >struct hmap *local_datapaths)
> >  {
> > -if (!ctx->ovs_idl_txn || !ctx->ovnsb_idl_txn) {
> > +if (!ctx->ovs_idl_txn) {
> >  return;
> >  }
> >
> > --
> > 2.1.3
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org 
> > http://openvswitch.org/mailman/listinfo/dev
> >
> 
> 
> -- 
> Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 0/3] Testsuite fixes.

[Joe Stringer]

This series looks good to me, thanks!

Ben, did you have any remaining comments?

On 3 March 2016 at 21:31, Ilya Maximets  wrote:
> version 4:
> * Reworked prohibition of parallel execution.
>
> version 3:
> * AT_SKIP_IF ---> AT_CHECK(... || return 77).
> * Using of GNU make extentions removed.
>
> version 2:
> * 'testsuite: Add timeout to add_of_br() command.' removed
>   because already applied.
> * New patch 'tests/automake.mk: Prohibition of parallel
>   system-traffic test execution.'
> * delay after ovs-vswitchd killing replaced with 'Waiting
>   for port's availability before creation.'
>
> Ilya Maximets (3):
>   system-traffic.at: Skip tests if namespaces or veths aren't supported.
>   check-system-userspace: Waiting for port's availability before
> creation.
>   tests/automake.mk: Prohibition of parallel system-traffic test
> execution.
>
>  tests/automake.mk| 4 ++--
>  tests/system-common-macros.at| 6 +++---
>  tests/system-userspace-macros.at | 2 ++
>  3 files changed, 7 insertions(+), 5 deletions(-)
>
> --
> 2.5.0
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] ovn northbound ovsdb-server’s memory usage problem

[Andy Zhou]

On Mon, Mar 7, 2016 at 1:57 PM, Han Zhou  wrote:

>
>
> On Sun, Mar 6, 2016 at 11:02 PM, Lei Huang  wrote:
> >
> > Hi,
> >
> >
> > During a scalability test, we found that the ovn northbound
> ovsdb-server’s
> > memory usage becomes very high while creating and binding ports, the test
> > step is:
> >
> > 1. Create 1000 sandboxes
> >
> > 2. Create 5 lswitches and create 200 lports for each of lswitches,  on
> > northbound
> >
> > 3. For each of lport created in step 2, bind it to a sandboxes randomly,
> >  then use ‘ovn-nbctl wait-until Logical_Port  up=true’ to wait
> > the port is ‘up’ on northbound
> >
> > 4. Goto step 2
> >
> >
> > The VmRSS memory usages as below:
> >
> >
> > lport number  ovn-northd   nb ovsdb-server   sb ovsdb-server
> >
> > 0   1292  1124  7104
> >
> >  5000  21704   6476420 17424
> >
> > 1  41680  25446148 31808
> >
> > 15000  61600  56893928133864
> >
> > 2  81844 100955012176868
> >
> >
> > The memory log as below:
> >
> > 016-03-02T09:48:32.024Z|3|memory|INFO|1124 kB peak resident set size
> > after 10.0 seconds
> >
> > 2016-03-02T16:24:46.430Z|5|memory|INFO|peak resident set size grew
> 150%
> > in last 23774.4 seconds, from 1124 kB to 2808 kB
> >
> > 2016-03-02T16:25:06.432Z|7|memory|INFO|peak resident set size grew
> 130%
> > in last 20.0 seconds, from 2808 kB to 6448 kB
> >
> > 2016-03-02T16:25:16.433Z|9|memory|INFO|peak resident set size grew
> 54%
> > in last 10.0 seconds, from 6448 kB to 9948 kB
> >
> > 2016-03-02T16:25:26.434Z|00011|memory|INFO|peak resident set size grew
> 52%
> > in last 10.0 seconds, from 9948 kB to 15072 kB
> >
> > 2016-03-02T16:25:36.436Z|00013|memory|INFO|peak resident set size grew
> 178%
> > in last 10.0 seconds, from 15072 kB to 41956 kB
> >
> > 2016-03-02T16:26:36.439Z|00015|memory|INFO|peak resident set size grew
> 140%
> > in last 60.0 seconds, from 41956 kB to 100696 kB
> >
> > 2016-03-02T16:27:36.444Z|00017|memory|INFO|peak resident set size grew
> 78%
> > in last 60.0 seconds, from 100696 kB to 179108 kB
> >
> > 2016-03-02T16:28:46.444Z|00019|memory|INFO|peak resident set size grew
> 55%
> > in last 70.0 seconds, from 179108 kB to 277412 kB
> >
> > 2016-03-02T16:39:26.953Z|00021|memory|INFO|peak resident set size grew
> 50%
> > in last 640.5 seconds, from 277412 kB to 417472 kB
> >
> > 2016-03-02T16:41:06.960Z|00023|memory|INFO|peak resident set size grew
> 67%
> > in last 100.0 seconds, from 417472 kB to 696732 kB
> >
> > 2016-03-02T16:44:17.180Z|00025|memory|INFO|peak resident set size grew
> 54%
> > in last 190.2 seconds, from 696732 kB to 1071080 kB
> >
> > 2016-03-02T16:55:02.317Z|00027|memory|INFO|peak resident set size grew
> 68%
> > in last 645.1 seconds, from 1071080 kB to 1794276 kB
> >
> > 2016-03-02T17:03:22.544Z|00029|memory|INFO|peak resident set size grew
> 50%
> > in last 500.2 seconds, from 1794276 kB to 2698100 kB
> >
> > 2016-03-02T17:12:18.359Z|00031|memory|INFO|peak resident set size grew
> 54%
> > in last 535.8 seconds, from 2698100 kB to 4158336 kB
> >
> > 2016-03-02T17:27:59.897Z|00070|memory|INFO|peak resident set size grew
> 55%
> > in last 941.5 seconds, from 4158336 kB to 6458136 kB
> >
> > 2016-03-02T17:51:04.005Z|00136|memory|INFO|peak resident set size grew
> 53%
> > in last 1384.1 seconds, from 6458136 kB to 9896844 kB
> >
> > 2016-03-02T18:37:52.913Z|00243|memory|INFO|peak resident set size grew
> 57%
> > in last 2808.9 seconds, from 9896844 kB to 15539132 kB
> >
> > 2016-03-02T19:21:33.977Z|00414|memory|INFO|peak resident set size grew
> 51%
> > in last 2621.1 seconds, from 15539132 kB to 23403744 kB
> >
> > 2016-03-02T20:27:53.938Z|00661|memory|INFO|peak resident set size grew
> 51%
> > in last 3980.0 seconds, from 23403744 kB to 35348044 kB
> >
> > 2016-03-02T22:11:13.200Z|00957|memory|INFO|peak resident set size grew
> 53%
> > in last 6199.3 seconds, from 35348044 kB to 53943084 kB
> >
> > 2016-03-03T00:49:32.196Z|01306|memory|INFO|peak resident set size grew
> 51%
> > in last 9499.0 seconds, from 53943084 kB to 81658988 kB
> >
> >
> >
> > On another small test environment, I checked northbound ovsdb-server by
> > valgrind:
> >
> > $ valgrind --leak-check=full --show-leak-kinds=all ovsdb-server
> --no-chdir
> > --pidfile=ovsdb-server-nb.pid --unixctl=ovsdb-server-nb.ctl -vconsole:off
> > -vsyslog:off -vfile:info --log-file=ovsdb-server-nb.log
> > --remote=punix:/home/rally/controller-sandbox/db-nb.sock conf-nb.db
> ovnnb.db
> >
> >
> > But no obvious memory leak found. So I make ovsdb-server exit without
> > cleanup to see what are memory used for, code changed as below:
> >
> >
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> >
> > index fa662b1..4498eaa 100644
> >
> > --- a/ovsdb/ovsdb-server.c
> >
> > +++ b/ovsdb/ovsdb-server.c
> >
> > @@ -339,7 +339,7 @@ 

[ovs-dev] [PATCH] ovsdb-server: Fix a reference count leak bug

[Andy Zhou]

When destroying an ovsdb_jsonrpc_monitor, the jsonrpc monitor still
holds a reference count to the monitors 'changes' indexed with
'unflushed' transaction id.  The bug is that the reference count was
not decremented as it should in the code path.

The bug caused 'changes' that have been flushed to all jsonrpc
clients to linger around unnecessarily, occupying increasingly
large amount of memory. See "Reported-at" URL for more details.

This bug is tricky to find since the memory is not leaked; they will
eventually be freed when monitors are destroyed.

Reported-by: Lei Huang 
Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067274.html
Signed-off-by: Andy Zhou 
---
 AUTHORS| 1 +
 ovsdb/jsonrpc-server.c | 4 ++--
 ovsdb/monitor.c| 9 -
 ovsdb/monitor.h| 6 ++
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 1184a51..0ba0f58 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -119,6 +119,7 @@ Kyle Mesterymest...@mestery.com
 Kyle Upton  kup...@baymicrosystems.com
 Lance Richardsonlrich...@redhat.com
 Lars Kellogg-Stedmanl...@redhat.com
+Lei Huang   huang.f@gmail.com
 Leo Altermanlalter...@nicira.com
 Lilijun jerry.lili...@huawei.com
 Linda Sun   l...@vmware.com
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 15dbc4e..caaf2bf 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1265,7 +1265,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session 
*s, struct ovsdb *db,
 dbmon = ovsdb_monitor_add(m->dbmon);
 if (dbmon != m->dbmon) {
 /* Found an exisiting dbmon, reuse the current one. */
-ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
+ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
 ovsdb_monitor_add_jsonrpc_monitor(dbmon, m);
 m->dbmon = dbmon;
 }
@@ -1348,7 +1348,7 @@ ovsdb_jsonrpc_monitor_destroy(struct 
ovsdb_jsonrpc_monitor *m)
 {
 json_destroy(m->monitor_id);
 hmap_remove(>session->monitors, >node);
-ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
+ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m, m->unflushed);
 free(m);
 }
 
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 0d81d89..6b0d13d 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -959,7 +959,8 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
 
 void
 ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
-   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
+   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
+   uint64_t unflushed)
 {
 struct jsonrpc_monitor_node *jm;
 
@@ -971,6 +972,12 @@ ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor 
*dbmon,
 /* Find and remove the jsonrpc monitor from the list.  */
 LIST_FOR_EACH(jm, node, >jsonrpc_monitors) {
 if (jm->jsonrpc_monitor == jsonrpc_monitor) {
+/* Release the tracked changes. */
+struct shash_node *node;
+SHASH_FOR_EACH (node, >tables) {
+struct ovsdb_monitor_table *mt = node->data;
+ovsdb_monitor_table_untrack_changes(mt, unflushed);
+}
 list_remove(>node);
 free(jm);
 
diff --git a/ovsdb/monitor.h b/ovsdb/monitor.h
index fb10435..9fea831 100644
--- a/ovsdb/monitor.h
+++ b/ovsdb/monitor.h
@@ -46,10 +46,8 @@ void ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor 
*dbmon,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
 
 void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
-   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
-
-void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
-   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
+   struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
+   uint64_t unflushed);
 
 void ovsdb_monitor_add_table(struct ovsdb_monitor *m,
  const struct ovsdb_table *table);
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free parsing matches.

[William Tu]

Hi Joe,

I've tested this patch (with modification to ofpbuf_put to force using
newly allocated address) and it works fine. Thanks!

Regards,
William

On Mon, Mar 7, 2016 at 11:31 AM, Joe Stringer  wrote:

> Address pointed by header_ptr might be free'd due to realloc
> happened in ofpbuf_put_hex(). Reported by valgrind in the test
> 379: check TCP flags expression in OXM and NXM.
>
> Invalid write of size 4
> nx_match_from_string_raw (nx-match.c:1510)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> ovs_cmdl_run_command (command-line.c:121)
> main (ovs-ofctl.c:137)
>
> Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
> free (vg_replace_malloc.c:530)
> ofpbuf_resize__ (ofpbuf.c:246)
> ofpbuf_put (ofpbuf.c:386)
> ofpbuf_put_hex (ofpbuf.c:414)
> nx_match_from_string_raw (nx-match.c:1488)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
>
> Reported-by: William Tu 
> Signed-off-by: Joe Stringer 
> ---
>  lib/nx-match.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 4999b1ac95d7..6fcc53ad07d1 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -1467,7 +1467,6 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
>  const char *name;
>  uint64_t header;
>  ovs_be64 nw_header;
> -ovs_be64 *header_ptr;
>  int name_len;
>  size_t n;
>
> @@ -1484,7 +1483,7 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
>
>  s += name_len + 1;
>
> -header_ptr = ofpbuf_put_uninit(b, nxm_header_len(header));
> +b->header = ofpbuf_put_uninit(b, nxm_header_len(header));
>  s = ofpbuf_put_hex(b, s, );
>  if (n != nxm_field_bytes(header)) {
>  const struct mf_field *field = mf_from_oxm_header(header);
> @@ -1507,7 +1506,7 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
>  }
>  }
>  nw_header = htonll(header);
> -memcpy(header_ptr, _header, nxm_header_len(header));
> +memcpy(b->header, _header, nxm_header_len(header));
>
>  if (nxm_hasmask(header)) {
>  s += strspn(s, " ");
> --
> 2.1.4
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Revert "ovn-controller: race between binding-run and patch-run for localnet ports"

[Russell Bryant]

Acked-by: Russell Bryant 

On Monday, March 7, 2016, Ben Pfaff  wrote:

> This reverts commit 3a83007a76bbf05144cee1fda7ad81c1c717dca7.  It's really
> nonobvious from the code why the condition added by that commit makes
> sense.
> The new condition should not be necessary now that binding_run() always
> keeps
> track of the local datapaths, since commit 7c040135cf351 (binding: Track
> local
> datapaths even when no transaction is possible).
>
> CC: Ramu Ramamurthy >
> Signed-off-by: Ben Pfaff >
> ---
>  ovn/controller/patch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index cfae613..753ce3e 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -280,7 +280,7 @@ void
>  patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
>struct hmap *local_datapaths)
>  {
> -if (!ctx->ovs_idl_txn || !ctx->ovnsb_idl_txn) {
> +if (!ctx->ovs_idl_txn) {
>  return;
>  }
>
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org 
> http://openvswitch.org/mailman/listinfo/dev
>


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

Re: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration

[Alin Serdean]

Unless I am reading wrong: OvsAddConfiguredSwitchPorts and 
OvsInitConfiguredSwitchNics only fail if we could not allocate memory or could 
not issue an OID request.

I am not in favor of reporting back to NDIS gracefully if any of the above 
conditions was broken even once.

Thanks,
Alin.

> -Mesaj original-
> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju
> Trimis: Thursday, March 3, 2016 10:51 AM
> Către: dev@openvswitch.org
> Subiect: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during
> port enumeration
> 
> While enumerating the ports on a switch, if adding one of the ports fails, we
> are not handling that error gracefully. Instead, we fail adding of all the 
> ports.
> This patch rectifies that.
> 
> Signed-off-by: Nithin Raju 
> ---
>  datapath-windows/ovsext/Vport.c | 41
> +
>  1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
> windows/ovsext/Vport.c index 7b0103d..a991d10 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -125,7 +125,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> switchContext,
>  if (vport != NULL) {
>  OVS_LOG_ERROR("Port add failed due to duplicate port name, "
>"port Id: %u", portParam->PortId);
> -status = STATUS_DATA_NOT_ACCEPTED;
> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
>  goto create_port_done;
>  }
> 
> @@ -140,7 +140,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> switchContext,
>  OVS_LOG_ERROR("Port add failed since a port already exists on "
>"the specified port Id: %u, ovsName: %s",
>portParam->PortId, vport->ovsName);
> -status = STATUS_DATA_NOT_ACCEPTED;
> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
>  goto create_port_done;
>  }
> 
> @@ -157,7 +157,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT
> switchContext,
>  OVS_LOG_INFO("Port add failed due to PortType change, port Id: 
> %u"
>   " old: %u, new: %u", portParam->PortId,
>   vport->portType, portParam->PortType);
> -status = STATUS_DATA_NOT_ACCEPTED;
> +status = NDIS_STATUS_DATA_NOT_ACCEPTED;
>  goto create_port_done;
>  }
>  vport->isAbsentOnHv = FALSE;
> @@ -365,7 +365,7 @@ HvCreateNic(POVS_SWITCH_CONTEXT
> switchContext,
>  OVS_LOG_ERROR("Create NIC without Switch Port,"
>" PortId: %x, NicIndex: %d",
>nicParam->PortId, nicParam->NicIndex);
> -status = NDIS_STATUS_INVALID_PARAMETER;
> +status = NDIS_STATUS_ADAPTER_NOT_FOUND;
>  goto add_nic_done;
>  }
>  OvsInitVportWithNicParam(switchContext, vport, nicParam); @@ -1393,6
> +1393,7 @@ OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT
> switchContext)
>  ULONG arrIndex;
>  PNDIS_SWITCH_PORT_PARAMETERS portParam;
>  PNDIS_SWITCH_PORT_ARRAY portArray = NULL;
> +BOOLEAN portAdded = FALSE;
> 
>  OVS_LOG_TRACE("Enter: switchContext:%p", switchContext);
> 
> @@ -1402,16 +1403,24 @@
> OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT switchContext)
>  }
> 
>  for (arrIndex = 0; arrIndex < portArray->NumElements; arrIndex++) {
> - portParam = NDIS_SWITCH_PORT_AT_ARRAY_INDEX(portArray,
> arrIndex);
> +portParam = NDIS_SWITCH_PORT_AT_ARRAY_INDEX(portArray,
> + arrIndex);
> 
> - if (portParam->IsValidationPort) {
> - continue;
> - }
> +if (portParam->IsValidationPort) {
> +continue;
> +}
> 
> - status = HvCreatePort(switchContext, portParam, 0);
> - if (status != STATUS_SUCCESS && status !=
> STATUS_DATA_NOT_ACCEPTED) {
> - break;
> - }
> +status = HvCreatePort(switchContext, portParam, 0);
> +if (status == NDIS_STATUS_SUCCESS) {
> +portAdded = TRUE;
> +} else if (status != NDIS_STATUS_DATA_NOT_ACCEPTED) {
> +/* Any other error. */
> +break;
> +}
> +}
> +
> +/* If at least one port was added, return success. */
> +if (status != NDIS_STATUS_SUCCESS && portAdded == TRUE) {
> +status = NDIS_STATUS_SUCCESS;
>  }
> 
>  cleanup:
> @@ -1438,6 +1447,7 @@
> OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT switchContext)
>  PNDIS_SWITCH_NIC_ARRAY nicArray = NULL;
>  ULONG arrIndex;
>  PNDIS_SWITCH_NIC_PARAMETERS nicParam;
> +BOOLEAN nicAdded = FALSE;
> 
>  OVS_LOG_TRACE("Enter: switchContext: %p", switchContext);
>  /*
> @@ -1459,9 +1469,16 @@
> OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT switchContext)
> 
>  status = HvCreateNic(switchContext, nicParam);
>  if (status == NDIS_STATUS_SUCCESS) {
> +nicAdded = TRUE;
>  HvConnectNic(switchContext, nicParam);
>  }
>   

Re: [ovs-dev] [PATCH] tests: Expand 'bundle with many ports' test.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 03:36:38PM -0800, Joe Stringer wrote:
> Explain what this test is doing, and check that the decoded action can
> be re-encoded and dumped back out of OVS.
> 
> Suggested-by: Ben Pfaff 
> Signed-off-by: Joe Stringer 

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

Re: [ovs-dev] [PATCH] ofp-actions: Assert variable actions have len>0.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 03:36:36PM -0800, Joe Stringer wrote:
> Variable-length actions must have a nonzero length; if they don't,
> something went wrong and we should bail out.
> 
> Suggested-by: Ben Pfaff 
> Signed-off-by: Joe Stringer 

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

[ovs-dev] [PATCH] ofp-actions: Fix use-after-free with ofpact_finish().

[Joe Stringer]

ofpact_finish() may now reallocate the buffer it is passed, but not all
callers updated their local pointers to the current action in the
buffer. This could potentially lead to several use-after-free bugs.

Update ofpact_finish() to return the new pointer to the ofpact which is
provided, and update the calling points to ensure that their local
pointers are pointing into the correct (potentially reallocated) buffer.

Fixes: 2bd318dec242 ("ofp-actions: Make composing actions harder to screw up.")
Reported-by: William Tu 
Signed-off-by: Joe Stringer 
---
 lib/bundle.c   |  4 +---
 lib/ofp-actions.c  | 14 +-
 lib/ofp-actions.h  |  2 +-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/bundle.c b/lib/bundle.c
index 1451e928fec6..07dc9f4919b2 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -178,9 +178,7 @@ bundle_parse__(const char *s, char **save_ptr,
 bundle = ofpacts->header;
 bundle->n_slaves++;
 }
-ofpact_finish(ofpacts, >ofpact);
-
-bundle = ofpacts->header;
+bundle = ofpact_finish(ofpacts, >ofpact);
 bundle->basis = atoi(basis);
 
 if (!strcasecmp(fields, "eth_src")) {
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 905469b6bbf9..a7c0388adeaa 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1256,8 +1256,7 @@ decode_bundle(bool load, const struct nx_action_bundle 
*nab,
 bundle = ofpacts->header;
 }
 
-ofpact_finish(ofpacts, >ofpact);
-
+bundle = ofpact_finish(ofpacts, >ofpact);
 if (!error) {
 error = bundle_check(bundle, OFPP_MAX, NULL);
 }
@@ -4956,7 +4955,7 @@ decode_NXAST_RAW_CT(const struct nx_action_conntrack *nac,
 
 conntrack = ofpbuf_push_uninit(out, sizeof(*conntrack));
 out->header = >ofpact;
-ofpact_finish(out, >ofpact);
+conntrack = ofpact_finish(out, >ofpact);
 
 if (conntrack->ofpact.len > sizeof(*conntrack)
 && !(conntrack->flags & NX_CT_F_COMMIT)) {
@@ -7397,8 +7396,11 @@ ofpact_init(struct ofpact *ofpact, enum ofpact_type 
type, size_t len)
 /* Finishes composing a variable-length action (begun using
  * ofpact_put_()), by padding the action to a multiple of OFPACT_ALIGNTO
  * bytes and updating its embedded length field.  See the large comment near
- * the end of ofp-actions.h for more information. */
-void
+ * the end of ofp-actions.h for more information.
+ *
+ * May reallocate 'ofpacts'. Callers should consider updating their 'ofpact'
+ * pointer to the return value of this function. */
+void *
 ofpact_finish(struct ofpbuf *ofpacts, struct ofpact *ofpact)
 {
 ptrdiff_t len;
@@ -7408,6 +7410,8 @@ ofpact_finish(struct ofpbuf *ofpacts, struct ofpact 
*ofpact)
 ovs_assert(len <= UINT16_MAX);
 ofpact->len = len;
 ofpbuf_padto(ofpacts, OFPACT_ALIGN(ofpacts->size));
+
+return ofpacts->header;
 }
 
 static char * OVS_WARN_UNUSED_RESULT
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 24143d324323..ec8b36981a56 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -968,7 +968,7 @@ OFPACTS
 #undef OFPACT
 
 /* Call after adding the variable-length part to a variable-length action. */
-void ofpact_finish(struct ofpbuf *, struct ofpact *);
+void *ofpact_finish(struct ofpbuf *, struct ofpact *);
 
 /* Additional functions for composing ofpacts. */
 struct ofpact_set_field *ofpact_put_reg_load(struct ofpbuf *ofpacts);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b963ff201782..69521a63a448 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1411,7 +1411,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
 controller->max_len = UINT16_MAX;
 controller->controller_id = 0;
 controller->reason = OFPR_IMPLICIT_MISS;
-ofpact_finish(, >ofpact);
+controller = ofpact_finish(, >ofpact);
 
 error = add_internal_miss_flow(ofproto, id++, ,
>miss_rule);
-- 
2.1.4

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

[ovs-dev] [PATCH] tests: Expand 'bundle with many ports' test.

[Joe Stringer]

Explain what this test is doing, and check that the decoded action can
be re-encoded and dumped back out of OVS.

Suggested-by: Ben Pfaff 
Signed-off-by: Joe Stringer 
---
 tests/bundle.at | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/bundle.at b/tests/bundle.at
index 298e683b0a1a..0bc179f978c8 100644
--- a/tests/bundle.at
+++ b/tests/bundle.at
@@ -193,8 +193,18 @@ AT_CHECK([ovs-ofctl parse-flow 
'actions=bundle(symmetric_l4,60,hrw,ofport,robot:
 ])
 AT_CLEANUP
 
+dnl Bundle actions with <= 2 ports typically align nicely within ofpbuf memory
+dnl allocation, so will not trigger reallocation codepaths. This test was
+dnl introduced to ensure that when bundle actions with a larger number of ports
+dnl are used, the encode/decode still works correctly. By placing the bundle
+dnl action deep within a list of actions, this test was able to trigger
+dnl Valgrind warnings for use-after-free bugs.
 AT_SETUP([bundle action with many ports])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl add-flow br0 
'actions=set_field:0x1->metadata,set_field:0x2->metadata,set_field:0x3->metadata,set_field:0x4->metadata,bundle(symmetric_l4,0,hrw,ofport,slaves:[[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40]])'])
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ 
actions=load:0x1->OXM_OF_METADATA[[]],load:0x2->OXM_OF_METADATA[[]],load:0x3->OXM_OF_METADATA[[]],load:0x4->OXM_OF_METADATA[[]],bundle(symmetric_l4,0,hrw,ofport,slaves:1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40)
+NXST_FLOW reply:
+])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
-- 
2.1.4

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

[ovs-dev] [PATCH] ofp-actions: Assert variable actions have len>0.

[Joe Stringer]

Variable-length actions must have a nonzero length; if they don't,
something went wrong and we should bail out.

Suggested-by: Ben Pfaff 
Signed-off-by: Joe Stringer 
---
 lib/ofp-actions.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index a7c0388adeaa..e5fe8e608986 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -7407,7 +7407,7 @@ ofpact_finish(struct ofpbuf *ofpacts, struct ofpact 
*ofpact)
 
 ovs_assert(ofpact == ofpacts->header);
 len = (char *) ofpbuf_tail(ofpacts) - (char *) ofpact;
-ovs_assert(len <= UINT16_MAX);
+ovs_assert(len && len <= UINT16_MAX);
 ofpact->len = len;
 ofpbuf_padto(ofpacts, OFPACT_ALIGN(ofpacts->size));
 
-- 
2.1.4

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

Re: [ovs-dev] ovn northbound ovsdb-server’s memory usage problem

[Ben Pfaff]

On Mon, Mar 07, 2016 at 01:57:22PM -0800, Han Zhou wrote:
> On Sun, Mar 6, 2016 at 11:02 PM, Lei Huang  wrote:
> >
> > Hi,
> >
> >
> > During a scalability test, we found that the ovn northbound ovsdb-server’s
> > memory usage becomes very high while creating and binding ports, the test
> > step is:
> >
> > 1. Create 1000 sandboxes
> >
> > 2. Create 5 lswitches and create 200 lports for each of lswitches,  on
> > northbound
> >
> > 3. For each of lport created in step 2, bind it to a sandboxes randomly,
> >  then use ‘ovn-nbctl wait-until Logical_Port  up=true’ to wait
> > the port is ‘up’ on northbound
> >
> > 4. Goto step 2
> >
> >
> > The VmRSS memory usages as below:
> >
> >
> > lport number  ovn-northd   nb ovsdb-server   sb ovsdb-server
> >
> > 0   1292  1124  7104
> >
> >  5000  21704   6476420 17424
> >
> > 1  41680  25446148 31808
> >
> > 15000  61600  56893928133864
> >
> > 2  81844 100955012176868
> >
> >
> > The memory log as below:
> >
> > 016-03-02T09:48:32.024Z|3|memory|INFO|1124 kB peak resident set size
> > after 10.0 seconds
> >
> > 2016-03-02T16:24:46.430Z|5|memory|INFO|peak resident set size grew
> 150%
> > in last 23774.4 seconds, from 1124 kB to 2808 kB
> >
> > 2016-03-02T16:25:06.432Z|7|memory|INFO|peak resident set size grew
> 130%
> > in last 20.0 seconds, from 2808 kB to 6448 kB
> >
> > 2016-03-02T16:25:16.433Z|9|memory|INFO|peak resident set size grew 54%
> > in last 10.0 seconds, from 6448 kB to 9948 kB
> >
> > 2016-03-02T16:25:26.434Z|00011|memory|INFO|peak resident set size grew 52%
> > in last 10.0 seconds, from 9948 kB to 15072 kB
> >
> > 2016-03-02T16:25:36.436Z|00013|memory|INFO|peak resident set size grew
> 178%
> > in last 10.0 seconds, from 15072 kB to 41956 kB
> >
> > 2016-03-02T16:26:36.439Z|00015|memory|INFO|peak resident set size grew
> 140%
> > in last 60.0 seconds, from 41956 kB to 100696 kB
> >
> > 2016-03-02T16:27:36.444Z|00017|memory|INFO|peak resident set size grew 78%
> > in last 60.0 seconds, from 100696 kB to 179108 kB
> >
> > 2016-03-02T16:28:46.444Z|00019|memory|INFO|peak resident set size grew 55%
> > in last 70.0 seconds, from 179108 kB to 277412 kB
> >
> > 2016-03-02T16:39:26.953Z|00021|memory|INFO|peak resident set size grew 50%
> > in last 640.5 seconds, from 277412 kB to 417472 kB
> >
> > 2016-03-02T16:41:06.960Z|00023|memory|INFO|peak resident set size grew 67%
> > in last 100.0 seconds, from 417472 kB to 696732 kB
> >
> > 2016-03-02T16:44:17.180Z|00025|memory|INFO|peak resident set size grew 54%
> > in last 190.2 seconds, from 696732 kB to 1071080 kB
> >
> > 2016-03-02T16:55:02.317Z|00027|memory|INFO|peak resident set size grew 68%
> > in last 645.1 seconds, from 1071080 kB to 1794276 kB
> >
> > 2016-03-02T17:03:22.544Z|00029|memory|INFO|peak resident set size grew 50%
> > in last 500.2 seconds, from 1794276 kB to 2698100 kB
> >
> > 2016-03-02T17:12:18.359Z|00031|memory|INFO|peak resident set size grew 54%
> > in last 535.8 seconds, from 2698100 kB to 4158336 kB
> >
> > 2016-03-02T17:27:59.897Z|00070|memory|INFO|peak resident set size grew 55%
> > in last 941.5 seconds, from 4158336 kB to 6458136 kB
> >
> > 2016-03-02T17:51:04.005Z|00136|memory|INFO|peak resident set size grew 53%
> > in last 1384.1 seconds, from 6458136 kB to 9896844 kB
> >
> > 2016-03-02T18:37:52.913Z|00243|memory|INFO|peak resident set size grew 57%
> > in last 2808.9 seconds, from 9896844 kB to 15539132 kB
> >
> > 2016-03-02T19:21:33.977Z|00414|memory|INFO|peak resident set size grew 51%
> > in last 2621.1 seconds, from 15539132 kB to 23403744 kB
> >
> > 2016-03-02T20:27:53.938Z|00661|memory|INFO|peak resident set size grew 51%
> > in last 3980.0 seconds, from 23403744 kB to 35348044 kB
> >
> > 2016-03-02T22:11:13.200Z|00957|memory|INFO|peak resident set size grew 53%
> > in last 6199.3 seconds, from 35348044 kB to 53943084 kB
> >
> > 2016-03-03T00:49:32.196Z|01306|memory|INFO|peak resident set size grew 51%
> > in last 9499.0 seconds, from 53943084 kB to 81658988 kB
> >
> >
> >
> > On another small test environment, I checked northbound ovsdb-server by
> > valgrind:
> >
> > $ valgrind --leak-check=full --show-leak-kinds=all ovsdb-server --no-chdir
> > --pidfile=ovsdb-server-nb.pid --unixctl=ovsdb-server-nb.ctl -vconsole:off
> > -vsyslog:off -vfile:info --log-file=ovsdb-server-nb.log
> > --remote=punix:/home/rally/controller-sandbox/db-nb.sock conf-nb.db
> ovnnb.db
> >
> >
> > But no obvious memory leak found. So I make ovsdb-server exit without
> > cleanup to see what are memory used for, code changed as below:
> >
> >
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> >
> > index fa662b1..4498eaa 100644
> >
> > --- a/ovsdb/ovsdb-server.c
> >
> > +++ b/ovsdb/ovsdb-server.c
> >
> > @@ -339,7 +339,7 @@ main(int argc, char *argv[])
> >
> > 

Re: [ovs-dev] [PATCH] unixctl: Log commands received and their replies (at debug level).

[Ben Pfaff]

On Mon, Mar 07, 2016 at 01:00:28PM -0800, Justin Pettit wrote:
> 
> > On Mar 7, 2016, at 11:19 AM, Ben Pfaff  wrote:
> > 
> > +if (VLOG_IS_DBG_ENABLED()) {
> > +char *params = json_to_string(request->params, 0);
> > +char *id = json_to_string(request->id, 0);
> > +VLOG_DBG("received request %s%s, id=%s", request->method, params, 
> > id);
> > +free(params);
> > +free(id);
> > +}
> 
> Not a big deal, but the containing function has a variable called "params" of 
> a different type.

Thank you for pointing that out.

I added an _s suffix to params and id here.

> Acked-by: Justin Pettit 

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

[ovs-dev] Say "Oh!" and "Ah!" all the nights with Rafa Camillo Dev

[Mrs . Rafa Camillo]

Heard her hand reached across from.Good evening  f#ck sensei!!i found yr 
profile via facebook!! you are rogue!!My husband cheated on me! Now I want to 
get him back 8-D I'm up for anythingMy account is here: 
http://qhegipco.DatingPrint.ruThe cure for my loneliness is in your pants, Dev 
. message me at +1 574 212 o258.. Talk soon!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] binding: Track local datapaths even when no transaction is possible.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 04:06:11PM -0500, Russell Bryant wrote:
> On Mon, Mar 7, 2016 at 3:55 PM, Ben Pfaff  wrote:
> 
> > Plenty of other code depends on the set of local datapaths.  Most notably,
> > the lflow code will drop logical flows when their logical datapaths aren't
> > present locally.
> >
> > Signed-off-by: Ben Pfaff 
> >
> 
> I think this also negates the need for this fix which I merged earlier
> today.
> 
> https://github.com/openvswitch/ovs/commit/3a83007a76bbf05144cee1fda7ad81c1c717dca7

I think that we should probably revert that.  I sent out a revert:
http://openvswitch.org/pipermail/dev/2016-March/067331.html

> Acked-by: Russell Bryant 

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

[ovs-dev] [PATCH] Revert "ovn-controller: race between binding-run and patch-run for localnet ports"

[Ben Pfaff]

This reverts commit 3a83007a76bbf05144cee1fda7ad81c1c717dca7.  It's really
nonobvious from the code why the condition added by that commit makes sense.
The new condition should not be necessary now that binding_run() always keeps
track of the local datapaths, since commit 7c040135cf351 (binding: Track local
datapaths even when no transaction is possible).

CC: Ramu Ramamurthy 
Signed-off-by: Ben Pfaff 
---
 ovn/controller/patch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index cfae613..753ce3e 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -280,7 +280,7 @@ void
 patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
   struct hmap *local_datapaths)
 {
-if (!ctx->ovs_idl_txn || !ctx->ovnsb_idl_txn) {
+if (!ctx->ovs_idl_txn) {
 return;
 }
 
-- 
2.1.3

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

Re: [ovs-dev] [PATCH 3/4] ofp-actions: Prevent integer overflow in decode.

[Joe Stringer]

On 5 March 2016 at 13:25, Ben Pfaff  wrote:
> On Thu, Mar 03, 2016 at 09:22:50PM +1300, Joe Stringer wrote:
>> When decoding a variable-length action, if the length of the action
>> exceeds the length storable in a uint16_t then something has gone
>> terribly wrong. Assert that this is not the case.
>>
>> Signed-off-by: Joe Stringer 
>> ---
>>  lib/ofp-actions.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
>> index fe1424f137a1..905469b6bbf9 100644
>> --- a/lib/ofp-actions.c
>> +++ b/lib/ofp-actions.c
>> @@ -7401,8 +7401,12 @@ ofpact_init(struct ofpact *ofpact, enum ofpact_type 
>> type, size_t len)
>>  void
>>  ofpact_finish(struct ofpbuf *ofpacts, struct ofpact *ofpact)
>>  {
>> +ptrdiff_t len;
>> +
>>  ovs_assert(ofpact == ofpacts->header);
>> -ofpact->len = (char *) ofpbuf_tail(ofpacts) - (char *) ofpact;
>> +len = (char *) ofpbuf_tail(ofpacts) - (char *) ofpact;
>> +ovs_assert(len <= UINT16_MAX);
>
> Probably worth adding "len > 0 &&" to that condition, as long as we're
> at it.
>
> Acked-by: Ben Pfaff 

Thanks for the review, I'll send a followup patch. (This was already
applied to branches 2.1+)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/4] tests: Add bundle action test with buffer realloc.

[Joe Stringer]

On 5 March 2016 at 11:26, Ben Pfaff  wrote:
> On Thu, Mar 03, 2016 at 09:22:48PM +1300, Joe Stringer wrote:
>> Add a test which causes internal reallocation of the ofpacts buffer,
>> followed by a large bundle action which should cause a subsequent
>> reallocation while decoding slave ports. Running this test under
>> valgrind reveals the issue below, which is fixed in the following
>> commit.
>>
>> Invalid read of size 4
>>at 0x4CED87: decode_bundle (ofp-actions.c:1253)
>>by 0x4CEDFC: decode_NXAST_RAW_BUNDLE (ofp-actions.c:1272)
>>by 0x4DBDE6: ofpact_decode (ofp-actions.inc2:3765)
>>by 0x4D6914: ofpacts_decode (ofp-actions.c:5735)
>>by 0x4D6A3D: ofpacts_pull_openflow_actions__ (ofp-actions.c:5772)
>>by 0x4D74F3: ofpacts_pull_openflow_instructions (ofp-actions.c:6352)
>>by 0x4F59FA: ofputil_decode_flow_mod (ofp-util.c:1704)
>>by 0x4EAD18: ofp_print_flow_mod (ofp-print.c:786)
>>by 0x4F0711: ofp_to_string__ (ofp-print.c:3220)
>>by 0x4F0D98: ofp_to_string (ofp-print.c:3453)
>>by 0x5486B3: do_recv (vconn.c:644)
>>by 0x548498: vconn_recv (vconn.c:598)
>>by 0x524582: rconn_recv (rconn.c:703)
>>by 0x45DA61: ofconn_run (connmgr.c:1370)
>>by 0x45B3B4: connmgr_run (connmgr.c:323)
>>by 0x41D1E8: ofproto_run (ofproto.c:1762)
>>by 0x40CEE0: bridge_run__ (bridge.c:2885)
>>by 0x40D093: bridge_run (bridge.c:2940)
>>by 0x412F7E: main (ovs-vswitchd.c:120)
>> Address 0x66aa460 is 1,152 bytes inside a block of size 1,184 free'd
>>at 0x4C2AF2E: realloc (vg_replace_malloc.c:692)
>>by 0x543D27: xrealloc (util.c:123)
>>by 0x5089EF: ofpbuf_resize__ (ofpbuf.c:243)
>>by 0x508B81: ofpbuf_prealloc_tailroom (ofpbuf.c:290)
>>by 0x508D5C: ofpbuf_put_uninit (ofpbuf.c:364)
>>by 0x508DEF: ofpbuf_put (ofpbuf.c:387)
>>by 0x4CED7D: decode_bundle (ofp-actions.c:1255)
>>by 0x4CEDFC: decode_NXAST_RAW_BUNDLE (ofp-actions.c:1272)
>>by 0x4DBDE6: ofpact_decode (ofp-actions.inc2:3765)
>>by 0x4D6914: ofpacts_decode (ofp-actions.c:5735)
>>by 0x4D6A3D: ofpacts_pull_openflow_actions__ (ofp-actions.c:5772)
>>by 0x4D74F3: ofpacts_pull_openflow_instructions (ofp-actions.c:6352)
>>by 0x4F59FA: ofputil_decode_flow_mod (ofp-util.c:1704)
>>by 0x4EAD18: ofp_print_flow_mod (ofp-print.c:786)
>>by 0x4F0711: ofp_to_string__ (ofp-print.c:3220)
>>by 0x4F0D98: ofp_to_string (ofp-print.c:3453)
>>by 0x5486B3: do_recv (vconn.c:644)
>>by 0x548498: vconn_recv (vconn.c:598)
>>by 0x524582: rconn_recv (rconn.c:703)
>>by 0x45DA61: ofconn_run (connmgr.c:1370)
>>
>> Signed-off-by: Joe Stringer 
>
> I'd add a comment to the test explaining why it's useful.
>
> It might be a good idea to add a second command to dump the flow back
> out, because it's never too convincing when a test just verifies that a
> command exits with status 0 and no output.

OK, thanks for the additional review. I'll send a follow-up patch.

> Usually, when a series fixes a bug and adds a test for it, we put the
> new test either in the same commit as the bug fix or just after the bug
> fix, because that maintains bisect-ability.  I guess that this is less
> important when the bug only shows up under valgrind, but I still think
> that it is a sound principle.

Ack, I realised it should have been either ordered within or after the
fix later on. Since the test passes even with valgrind, it seemed a
little less important.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [CudaMailTagged] Urgent Purchase Order

[Blaise Marquez]

Hi dev@openvswitch.org,

Please view the underlisted purchase order in Excel spreadsheet and
tell me the duration which the items could be readily made available.

 Please view the Purchase Order below.



 View | Download 15KB
  
 Kindly get back to me asap
 
Thank you,
Best Regards,

Blaise Marquez
Purchasing Assistant



DAS Companies, Inc.
724 Lawn Road, Palmyra, PA 17078
Phone: 717.964.3542 Extension 275

 
 En cumplimiento a la Ley Federal de Protección de Datos Personales en Posesión 
de los Particulares, en Pesa Sacos de Polipropileno S.A de C.V., protegemos y 
salvaguardamos sus datos personales para evitar el daño, pérdida, destrucción, 
robo, extravío, alteración, así como el tratamiento no autorizado de sus datos 
personales. Para más información respecto a nuestro aviso de privacidad lo 
invitamos a visitar nuestra página www.pesasacos.com Esta información es 
confidencial y se envía exclusivamente al destinatario. Queda estrictamente 
prohibido el uso, alteración o difusión de la misma por otra persona. Si usted 
no es el destinatario notifique al remitente y elimine el correo de su buzón. 
This message contains privileged and confidential information intended only for 
the use of the addressee named above. If you are not the intended recipient of 
this message, you are hereby notified that you may not disseminate, copy or 
take any action based on the contents thereof; kindly inform the sender 
immediately.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] ovn northbound ovsdb-server’s memory usage problem

[Han Zhou]

On Sun, Mar 6, 2016 at 11:02 PM, Lei Huang  wrote:
>
> Hi,
>
>
> During a scalability test, we found that the ovn northbound ovsdb-server’s
> memory usage becomes very high while creating and binding ports, the test
> step is:
>
> 1. Create 1000 sandboxes
>
> 2. Create 5 lswitches and create 200 lports for each of lswitches,  on
> northbound
>
> 3. For each of lport created in step 2, bind it to a sandboxes randomly,
>  then use ‘ovn-nbctl wait-until Logical_Port  up=true’ to wait
> the port is ‘up’ on northbound
>
> 4. Goto step 2
>
>
> The VmRSS memory usages as below:
>
>
> lport number  ovn-northd   nb ovsdb-server   sb ovsdb-server
>
> 0   1292  1124  7104
>
>  5000  21704   6476420 17424
>
> 1  41680  25446148 31808
>
> 15000  61600  56893928133864
>
> 2  81844 100955012176868
>
>
> The memory log as below:
>
> 016-03-02T09:48:32.024Z|3|memory|INFO|1124 kB peak resident set size
> after 10.0 seconds
>
> 2016-03-02T16:24:46.430Z|5|memory|INFO|peak resident set size grew
150%
> in last 23774.4 seconds, from 1124 kB to 2808 kB
>
> 2016-03-02T16:25:06.432Z|7|memory|INFO|peak resident set size grew
130%
> in last 20.0 seconds, from 2808 kB to 6448 kB
>
> 2016-03-02T16:25:16.433Z|9|memory|INFO|peak resident set size grew 54%
> in last 10.0 seconds, from 6448 kB to 9948 kB
>
> 2016-03-02T16:25:26.434Z|00011|memory|INFO|peak resident set size grew 52%
> in last 10.0 seconds, from 9948 kB to 15072 kB
>
> 2016-03-02T16:25:36.436Z|00013|memory|INFO|peak resident set size grew
178%
> in last 10.0 seconds, from 15072 kB to 41956 kB
>
> 2016-03-02T16:26:36.439Z|00015|memory|INFO|peak resident set size grew
140%
> in last 60.0 seconds, from 41956 kB to 100696 kB
>
> 2016-03-02T16:27:36.444Z|00017|memory|INFO|peak resident set size grew 78%
> in last 60.0 seconds, from 100696 kB to 179108 kB
>
> 2016-03-02T16:28:46.444Z|00019|memory|INFO|peak resident set size grew 55%
> in last 70.0 seconds, from 179108 kB to 277412 kB
>
> 2016-03-02T16:39:26.953Z|00021|memory|INFO|peak resident set size grew 50%
> in last 640.5 seconds, from 277412 kB to 417472 kB
>
> 2016-03-02T16:41:06.960Z|00023|memory|INFO|peak resident set size grew 67%
> in last 100.0 seconds, from 417472 kB to 696732 kB
>
> 2016-03-02T16:44:17.180Z|00025|memory|INFO|peak resident set size grew 54%
> in last 190.2 seconds, from 696732 kB to 1071080 kB
>
> 2016-03-02T16:55:02.317Z|00027|memory|INFO|peak resident set size grew 68%
> in last 645.1 seconds, from 1071080 kB to 1794276 kB
>
> 2016-03-02T17:03:22.544Z|00029|memory|INFO|peak resident set size grew 50%
> in last 500.2 seconds, from 1794276 kB to 2698100 kB
>
> 2016-03-02T17:12:18.359Z|00031|memory|INFO|peak resident set size grew 54%
> in last 535.8 seconds, from 2698100 kB to 4158336 kB
>
> 2016-03-02T17:27:59.897Z|00070|memory|INFO|peak resident set size grew 55%
> in last 941.5 seconds, from 4158336 kB to 6458136 kB
>
> 2016-03-02T17:51:04.005Z|00136|memory|INFO|peak resident set size grew 53%
> in last 1384.1 seconds, from 6458136 kB to 9896844 kB
>
> 2016-03-02T18:37:52.913Z|00243|memory|INFO|peak resident set size grew 57%
> in last 2808.9 seconds, from 9896844 kB to 15539132 kB
>
> 2016-03-02T19:21:33.977Z|00414|memory|INFO|peak resident set size grew 51%
> in last 2621.1 seconds, from 15539132 kB to 23403744 kB
>
> 2016-03-02T20:27:53.938Z|00661|memory|INFO|peak resident set size grew 51%
> in last 3980.0 seconds, from 23403744 kB to 35348044 kB
>
> 2016-03-02T22:11:13.200Z|00957|memory|INFO|peak resident set size grew 53%
> in last 6199.3 seconds, from 35348044 kB to 53943084 kB
>
> 2016-03-03T00:49:32.196Z|01306|memory|INFO|peak resident set size grew 51%
> in last 9499.0 seconds, from 53943084 kB to 81658988 kB
>
>
>
> On another small test environment, I checked northbound ovsdb-server by
> valgrind:
>
> $ valgrind --leak-check=full --show-leak-kinds=all ovsdb-server --no-chdir
> --pidfile=ovsdb-server-nb.pid --unixctl=ovsdb-server-nb.ctl -vconsole:off
> -vsyslog:off -vfile:info --log-file=ovsdb-server-nb.log
> --remote=punix:/home/rally/controller-sandbox/db-nb.sock conf-nb.db
ovnnb.db
>
>
> But no obvious memory leak found. So I make ovsdb-server exit without
> cleanup to see what are memory used for, code changed as below:
>
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
>
> index fa662b1..4498eaa 100644
>
> --- a/ovsdb/ovsdb-server.c
>
> +++ b/ovsdb/ovsdb-server.c
>
> @@ -339,7 +339,7 @@ main(int argc, char *argv[])
>
>   ovsdb_server_disable_monitor2, jsonrpc);
>
>
>
>  main_loop(jsonrpc, _dbs, unixctl, , run_process,
);
>
> -
>
> +return 0;
>
>  ovsdb_jsonrpc_server_destroy(jsonrpc);
>
>  SHASH_FOR_EACH_SAFE(node, next, _dbs) {
>
>  struct db *db = node->data;
>
>
> After 100 port created and bound, the 

Re: [ovs-dev] [PATCH] ovn-controller: add restart test

[ramu]

> What I would probably do is submit them together: the zone-ids fix + this
> test case that helps test it.  If you take out your "xyz" change, will this
> test fail currently?
>
> If i take out the change which masks the zone-ids, this test fails on
master because, zone-ids dont match before and after restart.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: add restart test

[Russell Bryant]

On Mon, Mar 7, 2016 at 4:27 PM, ramu  wrote:

>
>
>> Does this depend on your zone-ids fix?  Do you have another rev of that
>> coming?
>>
>
> Russell, This test does not depend on the zone-id fix, because it replaces
> zone-ids on flows
> with a dummy string "xyz". It passes on the master as is.
>
> I will send the next revision of the zone-id fix soon, and as part of that
> fix, this test
> will also be updated to compare zone-ids on flows.
>

I see now.  You even wrote:

+# For now, zone-ids in flows are masked out because, they are not assigned
+# consistently. This test is added to prepare for that code-change.

What I would probably do is submit them together: the zone-ids fix + this
test case that helps test it.  If you take out your "xyz" change, will this
test fail currently?

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

Re: [ovs-dev] [PATCH] ovn-controller: add restart test

[ramu]

>
> Does this depend on your zone-ids fix?  Do you have another rev of that
> coming?
>

Russell, This test does not depend on the zone-id fix, because it replaces
zone-ids on flows
with a dummy string "xyz". It passes on the master as is.

I will send the next revision of the zone-id fix soon, and as part of that
fix, this test
will also be updated to compare zone-ids on flows.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: add restart test

[Russell Bryant]

On Mon, Mar 7, 2016 at 4:03 PM, Ramu Ramamurthy 
wrote:

> Add a test to validate that a restart of the ovn-controller
> will program flows in table 0 consistently. Flows before
> restart are compared against flows after restart to detect
> problems with ofports or zone-ids.
>
> Signed-off-by: Ramu Ramamurthy 
>
>
Does this depend on your zone-ids fix?  Do you have another rev of that
coming?

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

Re: [ovs-dev] [PATCH] binding: Track local datapaths even when no transaction is possible.

[Russell Bryant]

On Mon, Mar 7, 2016 at 3:55 PM, Ben Pfaff  wrote:

> Plenty of other code depends on the set of local datapaths.  Most notably,
> the lflow code will drop logical flows when their logical datapaths aren't
> present locally.
>
> Signed-off-by: Ben Pfaff 
>

I think this also negates the need for this fix which I merged earlier
today.

https://github.com/openvswitch/ovs/commit/3a83007a76bbf05144cee1fda7ad81c1c717dca7

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

[ovs-dev] [PATCH] ovn-controller: add restart test

[Ramu Ramamurthy]

Add a test to validate that a restart of the ovn-controller
will program flows in table 0 consistently. Flows before
restart are compared against flows after restart to detect
problems with ofports or zone-ids.

Signed-off-by: Ramu Ramamurthy 
---
 tests/ovn-controller.at | 127 
 1 file changed, 127 insertions(+)

diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 2ff56af..68b78ff 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -117,3 +117,130 @@ check_patches
 # Gracefully terminate ovn-controller
 ovs-appctl -t ovn-controller exit
 AT_CLEANUP
+
+AT_SETUP([ovn-controller - restart])
+AT_KEYWORDS([ovn])
+
+# The purpose of this test is to validate flows programmed
+# (in table 0) after ovn-controller restart.
+# This test confirms that:
+# a) ofports remain the same, so, ports were not deleted/readded
+# b) zone-ids remain the same on ports
+#
+# For now, zone-ids in flows are masked out because, they are not assigned
+# consistently. This test is added to prepare for that code-change.
+#
+
+ovn_init_db ovn-sb
+net_add n1
+sim_add hv
+as hv
+ovs-vsctl \
+-- add-br br-phys \
+-- add-br br-eth0 \
+-- add-br br-eth1 \
+-- add-br br-eth2
+
+ovn_attach n1 br-phys 192.168.0.1
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . 
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1,physnet3:br-eth2])
+
+#
+# - Create 2 localnet ports
+# - Create 4 VIFs
+# - Create 5 CIFs
+#
+
+#
+# Create a localnet port, along with a vif
+#
+ovn-sbctl \
+-- --id=@dp101 create Datapath_Binding tunnel_key=101 \
+-- create Port_Binding datapath=@dp101 logical_port=localnet1 tunnel_key=1 
\
+type=localnet options:network_name=physnet1 \
+-- create Port_Binding datapath=@dp101 logical_port=localvif1 tunnel_key=2
+ovs-vsctl add-port br-int localvif1 -- set Interface localvif1 
external_ids:iface-id=localvif1
+#
+# Create another localnet port, along with a vif
+#
+ovn-sbctl \
+-- --id=@dp201 create Datapath_Binding tunnel_key=201 \
+-- create Port_Binding datapath=@dp201 logical_port=localnet201 
tunnel_key=1 \
+type=localnet options:network_name=physnet2 \
+-- create Port_Binding datapath=@dp201 logical_port=localvif201 
tunnel_key=2
+
+ovs-vsctl add-port br-int localvif201 -- set Interface localvif201 
external_ids:iface-id=localvif201
+
+#
+# Create a vif port
+#
+ovn-sbctl \
+-- --id=@dp102 create Datapath_Binding tunnel_key=102 \
+-- create Port_Binding datapath=@dp102 logical_port=localvif2 tunnel_key=3
+ovs-vsctl add-port br-int localvif2 -- set Interface localvif2 
external_ids:iface-id=localvif2
+
+#
+# Create a vif port and add 5 cifs on it
+#
+ovn-sbctl \
+-- --id=@dp103 create Datapath_Binding tunnel_key=103 \
+-- create Port_Binding datapath=@dp103 logical_port=localvif3 tunnel_key=4 
\
+-- create Port_Binding datapath=@dp103 logical_port=localcif1 tunnel_key=5 
parent_port=localvif3 tag=1 \
+-- create Port_Binding datapath=@dp103 logical_port=localcif2 tunnel_key=6 
parent_port=localvif3 tag=2 \
+-- create Port_Binding datapath=@dp103 logical_port=localcif3 tunnel_key=7 
parent_port=localvif3 tag=3 \
+-- create Port_Binding datapath=@dp103 logical_port=localcif4 tunnel_key=8 
parent_port=localvif3 tag=4 \
+-- create Port_Binding datapath=@dp103 logical_port=localcif5 tunnel_key=9 
parent_port=localvif3 tag=5
+ovs-vsctl add-port br-int localvif3 -- set Interface localvif3 
external_ids:iface-id=localvif3
+
+ovs-vsctl show
+
+# wait for flows in table 0 to get programmed
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br-int table=0 | ofctl_strip | tail 
-n+2 | sort | wc -l` -ge 13])
+
+# save the table0 flows for a later comparison after restart
+ovs-ofctl dump-flows br-int table=0 | ofctl_strip | tail -n+2 | sort | sed 
's/load.\+NXM_NX_REG5/load:xyz->NXM_NX_REG5/g' > expout
+cat expout
+
+
+#kill the ovn controller ungracefully
+kill -9 `cat "$OVS_RUNDIR"/ovn-controller.pid`
+
+echo "killed the ovn-controller"
+
+#clean the flows
+ovs-ofctl del-flows br-int
+
+#restart the ovn controller
+start_daemon ovn-controller
+
+#wait for the table 0 flows to get reprogrammed
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br-int table=0 | ofctl_strip | tail 
-n+2 | sort | wc -l` -ge 13])
+
+# save flows for a comparison
+ovs-ofctl dump-flows br-int table=0 | ofctl_strip | tail -n+2 | sort | sed 
's/load.\+NXM_NX_REG5/load:xyz->NXM_NX_REG5/g' > afterkill
+cat afterkill
+
+#compare the table 0 flows before and after restart
+AT_CHECK([ovs-ofctl diff-flows afterkill expout])
+
+
+ovs-appctl -t ovn-controller exit
+echo "shutdown the ovn-controller gracefully"
+
+#clean the flows
+ovs-ofctl del-flows br-int
+
+start_daemon ovn-controller
+echo "restarted the ovn-controller"
+
+#wait for the table 0 flows to get reprogrammed
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br-int table=0 | ofctl_strip | tail 
-n+2 | sort | wc -l` -ge 13])

Re: [ovs-dev] [PATCH v2] Use 'RUNDIR' from make for rhel/ovn-controller.service

[Russell Bryant]

On Mon, Mar 7, 2016 at 2:39 PM, Russell Bryant  wrote:

>
>
> On Mon, Mar 7, 2016 at 1:15 PM, Ben Pfaff  wrote:
>
>> On Mon, Mar 07, 2016 at 10:34:30AM -0500, Russell Bryant wrote:
>> > On Mon, Mar 7, 2016 at 9:37 AM, Russell Bryant  wrote:
>> >
>> > >
>> > >
>> > > On Mon, Mar 7, 2016 at 12:03 AM,  wrote:
>> > >
>> > >> Perviously it was using the platform's runtime directory which can be
>> > >> different from the runtime directory of ovsdb-server started by the
>> > >> openvswitch service
>> > >>
>> > >> Signed-off-by: Babu Shanmugam 
>> > >>
>> > >>
>> > > Thanks!  I added the ack from Flavio and applied this to master.
>> > >
>> > >
>> > It looks like this patch broke the build on travis-ci.  For example:
>> >
>> > https://travis-ci.org/openvswitch/ovs/jobs/114254913
>> >
>> > It probably has to do with using a separate build directory.  I'm
>> looking
>> > into how to fix it, but haven't worked it out just yet.
>>
>> It probably needs something like in $(update_rhel_spec), so that if it's
>> just old but not incorrect it just gets touched instead of overwritten.
>> (It's a nasty kluge.)
>>
>
> Thanks for the tip.  I think that has me on the right track now.
>

This is pretty annoying, actually.  I tried to apply that same fix. with
the following patch:

> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index dc53986..214cda4 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -33,7 +33,8 @@ EXTRA_DIST += \
> rhel/usr_lib_systemd_system_ovn-northd.service
>
>  update_rhel_spec = \
> -  $(AM_V_GEN)($(ro_shell) && sed -e 's,[@]VERSION[@],$(VERSION),g') \
> +  $(AM_V_GEN)($(ro_shell) && sed -e 's,[@]VERSION[@],$(VERSION),g' \
> + -e 's,[@]RUNDIR[@],$(RUNDIR),g') \
>  < $(srcdir)/rhel/$(@F).in > $(@F).tmp || exit 1; \
>if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp
$@; fi
>
> @@ -53,6 +54,7 @@ $(srcdir)/rhel/openvswitch-fedora.spec: rhel/
openvswitch-fedora.spec.in $(top_bu
> $(update_rhel_spec)
>
>  $(srcdir)/rhel/usr_lib_systemd_system_ovn-controller.service: rhel/
usr_lib_systemd_system_ovn-controller.service.in
$(top_builddir)/config.status
> +   $(update_rhel_spec)
>
>  RPMBUILD_TOP := $(abs_top_builddir)/rpm/rpmbuild

This fails because ovn-controller.service does actually change.  @RUNDIR@
is different between the original build and "make distcheck".

/usr/local/var/run/openvswitch

vs.

/home/travis/build/russellb/ovs/openvswitch-2.5.90/_inst/var/run/openvswitch

It seems pretty odd that @RUNDIR@ is different in "make distcheck", though.

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

Re: [ovs-dev] [PATCH] unixctl: Log commands received and their replies (at debug level).

[Justin Pettit]

> On Mar 7, 2016, at 11:19 AM, Ben Pfaff  wrote:
> 
> +if (VLOG_IS_DBG_ENABLED()) {
> +char *params = json_to_string(request->params, 0);
> +char *id = json_to_string(request->id, 0);
> +VLOG_DBG("received request %s%s, id=%s", request->method, params, 
> id);
> +free(params);
> +free(id);
> +}

Not a big deal, but the containing function has a variable called "params" of a 
different type.

Acked-by: Justin Pettit 

--Justin



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

Re: [ovs-dev] [PATCH v5 5/5] [RFC] lflow: Disable egress table optimization.

[Ben Pfaff]

On Fri, Feb 19, 2016 at 04:40:19PM -0800, Ben Pfaff wrote:
> I don't understand why, but without this change, the test in the previous
> commit does not pass.
> 
> Signed-off-by: Ben Pfaff 

I figured out the correct fix:
http://openvswitch.org/pipermail/dev/2016-March/067317.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] binding: Track local datapaths even when no transaction is possible.

[Ben Pfaff]

Plenty of other code depends on the set of local datapaths.  Most notably,
the lflow code will drop logical flows when their logical datapaths aren't
present locally.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/binding.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 9087052..d3ca9c9 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -154,10 +154,6 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 const struct sbrec_chassis *chassis_rec;
 const struct sbrec_port_binding *binding_rec;
 
-if (!ctx->ovnsb_idl_txn) {
-return;
-}
-
 chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id);
 if (!chassis_rec) {
 return;
@@ -177,10 +173,6 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 sset_add(_lports, node->name);
 }
 
-ovsdb_idl_txn_add_comment(
-ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for '%s'",
-chassis_id);
-
 /* Run through each binding record to see if it is resident on this
  * chassis and update the binding accordingly.  This includes both
  * directly connected logical ports and children of those ports. */
@@ -201,15 +193,24 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 if (binding_rec->chassis == chassis_rec) {
 continue;
 }
-if (binding_rec->chassis) {
-VLOG_INFO("Changing chassis for lport %s from %s to %s",
-  binding_rec->logical_port,
-  binding_rec->chassis->name,
-  chassis_rec->name);
+if (ctx->ovnsb_idl_txn) {
+if (binding_rec->chassis) {
+VLOG_INFO("Changing chassis for lport %s from %s to %s.",
+  binding_rec->logical_port,
+  binding_rec->chassis->name,
+  chassis_rec->name);
+} else {
+VLOG_INFO("Claiming lport %s for this chassis.",
+  binding_rec->logical_port);
+}
+sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
 }
-sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
 } else if (binding_rec->chassis == chassis_rec) {
-sbrec_port_binding_set_chassis(binding_rec, NULL);
+if (ctx->ovnsb_idl_txn) {
+VLOG_INFO("Releasing lport %s from this chassis.",
+  binding_rec->logical_port);
+sbrec_port_binding_set_chassis(binding_rec, NULL);
+}
 } else if (!binding_rec->chassis
&& !strcmp(binding_rec->type, "localnet")) {
 /* localnet ports will never be bound to a chassis, but we want
-- 
2.1.3

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

[ovs-dev] re

[Liza Kell]

I have a proposal for you kindly E-mail me at mrrsshhui7...@hotmail.com

Yours Faithfully
Mrs Huian Shao
















__
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
__
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Use 'RUNDIR' from make for rhel/ovn-controller.service

[Russell Bryant]

On Mon, Mar 7, 2016 at 1:15 PM, Ben Pfaff  wrote:

> On Mon, Mar 07, 2016 at 10:34:30AM -0500, Russell Bryant wrote:
> > On Mon, Mar 7, 2016 at 9:37 AM, Russell Bryant  wrote:
> >
> > >
> > >
> > > On Mon, Mar 7, 2016 at 12:03 AM,  wrote:
> > >
> > >> Perviously it was using the platform's runtime directory which can be
> > >> different from the runtime directory of ovsdb-server started by the
> > >> openvswitch service
> > >>
> > >> Signed-off-by: Babu Shanmugam 
> > >>
> > >>
> > > Thanks!  I added the ack from Flavio and applied this to master.
> > >
> > >
> > It looks like this patch broke the build on travis-ci.  For example:
> >
> > https://travis-ci.org/openvswitch/ovs/jobs/114254913
> >
> > It probably has to do with using a separate build directory.  I'm looking
> > into how to fix it, but haven't worked it out just yet.
>
> It probably needs something like in $(update_rhel_spec), so that if it's
> just old but not incorrect it just gets touched instead of overwritten.
> (It's a nasty kluge.)
>

Thanks for the tip.  I think that has me on the right track now.

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

Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free.

[Joe Stringer]

On 4 March 2016 at 17:35, William Tu  wrote:
> Address pointed by header_ptr might be free'd due to realloc
> happened at ofpbuf_put_uninit() and ofpbuf_put_hex(). Reported
> by valgrind 379: check TCP flags expression in OXM and NXM.
>
> Invalid write of size 4
> nx_match_from_string_raw (nx-match.c:1510)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> ovs_cmdl_run_command (command-line.c:121)
> main (ovs-ofctl.c:137)
>
> Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
> free (vg_replace_malloc.c:530)
> ofpbuf_resize__ (ofpbuf.c:246)
> ofpbuf_put (ofpbuf.c:386)
> ofpbuf_put_hex (ofpbuf.c:414)
> nx_match_from_string_raw (nx-match.c:1488)
> nx_match_from_string (nx-match.c:1538)
> ofctl_parse_nxm__ (ovs-ofctl.c:3325)
>
> Signed-off-by: William Tu 

As a general policy, I think it's better to avoid adding pointer
arithmetic throughout the codebase where possible.

I've proposed a slightly different fix here making use of
ofpbuf->header, although I'm not 100% on whether it's fine for this
code to overwrite it:
http://openvswitch.org/pipermail/dev/2016-March/067313.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] nx-match: Fix use-after-free parsing matches.

[Joe Stringer]

Address pointed by header_ptr might be free'd due to realloc
happened in ofpbuf_put_hex(). Reported by valgrind in the test
379: check TCP flags expression in OXM and NXM.

Invalid write of size 4
nx_match_from_string_raw (nx-match.c:1510)
nx_match_from_string (nx-match.c:1538)
ofctl_parse_nxm__ (ovs-ofctl.c:3325)
ovs_cmdl_run_command (command-line.c:121)
main (ovs-ofctl.c:137)

Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
free (vg_replace_malloc.c:530)
ofpbuf_resize__ (ofpbuf.c:246)
ofpbuf_put (ofpbuf.c:386)
ofpbuf_put_hex (ofpbuf.c:414)
nx_match_from_string_raw (nx-match.c:1488)
nx_match_from_string (nx-match.c:1538)
ofctl_parse_nxm__ (ovs-ofctl.c:3325)

Reported-by: William Tu 
Signed-off-by: Joe Stringer 
---
 lib/nx-match.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 4999b1ac95d7..6fcc53ad07d1 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1467,7 +1467,6 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 const char *name;
 uint64_t header;
 ovs_be64 nw_header;
-ovs_be64 *header_ptr;
 int name_len;
 size_t n;
 
@@ -1484,7 +1483,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 
 s += name_len + 1;
 
-header_ptr = ofpbuf_put_uninit(b, nxm_header_len(header));
+b->header = ofpbuf_put_uninit(b, nxm_header_len(header));
 s = ofpbuf_put_hex(b, s, );
 if (n != nxm_field_bytes(header)) {
 const struct mf_field *field = mf_from_oxm_header(header);
@@ -1507,7 +1506,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf *b)
 }
 }
 nw_header = htonll(header);
-memcpy(header_ptr, _header, nxm_header_len(header));
+memcpy(b->header, _header, nxm_header_len(header));
 
 if (nxm_hasmask(header)) {
 s += strspn(s, " ");
-- 
2.1.4

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

[ovs-dev] Unpaid Invoice #963125

[Norma Nieves]

Dear Valued Customer,

Please make sure you send payment for your parcel to avoid any inconvenience. 
Open the attached file to review the confirmation listing.


Sincerely,

Norma Nieves
Courier Service
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 4/4] openflow: Support matching and modifying MPLS TTL field.

[Ben Pfaff]

Occasionally we get asked about this and I don't see a reason not to
support it.

Signed-off-by: Ben Pfaff 
---
 NEWS |  1 +
 lib/match.c  | 19 ++-
 lib/match.h  |  4 +++-
 lib/meta-flow.c  | 20 
 lib/meta-flow.h  | 15 +++
 tests/ofproto.at |  3 ++-
 6 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 6d71df7..38f6b56 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,7 @@ Post-v2.5.0
  * OpenFlow 1.3 Extension 230, adding OpenFlow Bundles support, is
now implemented.  Only flow mod and port mod messages are supported
in bundles.
+ * New OpenFlow extension NXM_NX_MPLS_TTL to provide access to MPLS TTL.
- ovs-ofctl:
  * queue-get-config command now allows a queue ID to be specified.
  * '--bundle' option can now be used with OpenFlow 1.3.
diff --git a/lib/match.c b/lib/match.c
index 95d34bc..17ef752 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -591,6 +591,23 @@ match_set_mpls_bos(struct match *match, int idx, uint8_t 
mpls_bos)
 flow_set_mpls_bos(>flow, idx, mpls_bos);
 }
 
+/* Modifies 'match' so that the MPLS TTL is wildcarded. */
+void
+match_set_any_mpls_ttl(struct match *match, int idx)
+{
+match->wc.masks.mpls_lse[idx] &= ~htonl(MPLS_TTL_MASK);
+flow_set_mpls_ttl(>flow, idx, 0);
+}
+
+/* Modifies 'match' so that it matches only packets with an MPLS header whose
+ * TTL equals 'mpls_ttl' */
+void
+match_set_mpls_ttl(struct match *match, int idx, uint8_t mpls_ttl)
+{
+match->wc.masks.mpls_lse[idx] |= htonl(MPLS_TTL_MASK);
+flow_set_mpls_ttl(>flow, idx, mpls_ttl);
+}
+
 /* Modifies 'match' so that the MPLS LSE is wildcarded. */
 void
 match_set_any_mpls_lse(struct match *match, int idx)
diff --git a/lib/match.h b/lib/match.h
index 650a203..0a6ac29 100644
--- a/lib/match.h
+++ b/lib/match.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -120,6 +120,8 @@ void match_set_any_mpls_tc(struct match *, int idx);
 void match_set_mpls_tc(struct match *, int idx, uint8_t);
 void match_set_any_mpls_bos(struct match *, int idx);
 void match_set_mpls_bos(struct match *, int idx, uint8_t);
+void match_set_any_mpls_ttl(struct match *, int idx);
+void match_set_mpls_ttl(struct match *, int idx, uint8_t);
 void match_set_tp_src(struct match *, ovs_be16);
 void match_set_mpls_lse(struct match *, int idx, ovs_be32 lse);
 void match_set_tp_src_masked(struct match *, ovs_be16 port, ovs_be16 mask);
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 16b9c92..6c899e1 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -271,6 +271,8 @@ mf_is_all_wild(const struct mf_field *mf, const struct 
flow_wildcards *wc)
 return !(wc->masks.mpls_lse[0] & htonl(MPLS_TC_MASK));
 case MFF_MPLS_BOS:
 return !(wc->masks.mpls_lse[0] & htonl(MPLS_BOS_MASK));
+case MFF_MPLS_TTL:
+return !(wc->masks.mpls_lse[0] & htonl(MPLS_TTL_MASK));
 
 case MFF_IPV4_SRC:
 return !wc->masks.nw_src;
@@ -527,6 +529,7 @@ mf_is_value_valid(const struct mf_field *mf, const union 
mf_value *value)
 case MFF_ETH_DST:
 case MFF_ETH_TYPE:
 case MFF_VLAN_TCI:
+case MFF_MPLS_TTL:
 case MFF_IPV4_SRC:
 case MFF_IPV4_DST:
 case MFF_IPV6_SRC:
@@ -741,6 +744,10 @@ mf_get_value(const struct mf_field *mf, const struct flow 
*flow,
 value->u8 = mpls_lse_to_bos(flow->mpls_lse[0]);
 break;
 
+case MFF_MPLS_TTL:
+value->u8 = mpls_lse_to_ttl(flow->mpls_lse[0]);
+break;
+
 case MFF_IPV4_SRC:
 value->be32 = flow->nw_src;
 break;
@@ -995,6 +1002,10 @@ mf_set_value(const struct mf_field *mf,
 match_set_mpls_bos(match, 0, value->u8);
 break;
 
+case MFF_MPLS_TTL:
+match_set_mpls_ttl(match, 0, value->u8);
+break;
+
 case MFF_IPV4_SRC:
 match_set_nw_src(match, value->be32);
 break;
@@ -1301,6 +1312,10 @@ mf_set_flow_value(const struct mf_field *mf,
 flow_set_mpls_bos(flow, 0, value->u8);
 break;
 
+case MFF_MPLS_TTL:
+flow_set_mpls_ttl(flow, 0, value->u8);
+break;
+
 case MFF_IPV4_SRC:
 flow->nw_src = value->be32;
 break;
@@ -1623,6 +1638,10 @@ mf_set_wild(const struct mf_field *mf, struct match 
*match, char **err_str)
 match_set_any_mpls_bos(match, 0);
 break;
 
+case MFF_MPLS_TTL:
+

[ovs-dev] [PATCH] unixctl: Log commands received and their replies (at debug level).

[Ben Pfaff]

These commands are also visible through the "jsonrpc" module, but turning
up the log level there also exposes a lot of OVSDB traffic that usually
isn't interesting.

Also, enable this logging for the tests.

Signed-off-by: Ben Pfaff 
---
 lib/unixctl.c   | 17 -
 tests/ofproto-macros.at |  4 ++--
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/lib/unixctl.c b/lib/unixctl.c
index b47f35a..f5fa59d 100644
--- a/lib/unixctl.c
+++ b/lib/unixctl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -151,6 +151,13 @@ unixctl_command_reply__(struct unixctl_conn *conn,
 reply = jsonrpc_create_error(body_json, conn->request_id);
 }
 
+if (VLOG_IS_DBG_ENABLED()) {
+char *id = json_to_string(conn->request_id, 0);
+VLOG_DBG("replying with %s, id=%s: \"%s\"",
+ success ? "success" : "error", id, body);
+free(id);
+}
+
 /* If jsonrpc_send() returns an error, the run loop will take care of the
  * problem eventually. */
 jsonrpc_send(conn->rpc, reply);
@@ -268,6 +275,14 @@ process_command(struct unixctl_conn *conn, struct 
jsonrpc_msg *request)
 COVERAGE_INC(unixctl_received);
 conn->request_id = json_clone(request->id);
 
+if (VLOG_IS_DBG_ENABLED()) {
+char *params = json_to_string(request->params, 0);
+char *id = json_to_string(request->id, 0);
+VLOG_DBG("received request %s%s, id=%s", request->method, params, id);
+free(params);
+free(id);
+}
+
 params = json_array(request->params);
 command = shash_find_data(, request->method);
 if (!command) {
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 1b22c44..18114d9 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -96,7 +96,7 @@ sim_add () {
as $1 ovs-vsctl --no-wait -- init || return 1
 
# Start ovs-vswitchd
-   as $1 start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif
+   as $1 start_daemon ovs-vswitchd --enable-dummy=system -vvconn 
-vofproto_dpif -vunixctl
 }
 
 # "as $1" sets the OVS_*DIR environment variables to point to $ovs_base/$1.
@@ -272,7 +272,7 @@ m4_define([_OVS_VSWITCHD_START],
AT_CHECK([ovs-vsctl --no-wait init])
 
dnl Start ovs-vswitchd.
-   AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn 
-vofproto_dpif], [0], [], [stderr])
+   AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn 
-vofproto_dpif -vunixctl], [0], [], [stderr])
AT_CAPTURE_FILE([ovs-vswitchd.log])
on_exit "kill `cat ovs-vswitchd.pid`"
AT_CHECK([[sed < stderr '
-- 
2.1.3

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

[ovs-dev] [PATCH 2/4] dpif-netdev: Fix typo in comment.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 lib/dpif-netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a7e224a..cf574ad 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1598,7 +1598,7 @@ netdev_flow_key_equal(const struct netdev_flow_key *a,
 }
 
 /* Used to compare 'netdev_flow_key' in the exact match cache to a miniflow.
- * The maps are compared bitwise, so both 'key->mf' 'mf' must have been
+ * The maps are compared bitwise, so both 'key->mf' and 'mf' must have been
  * generated by miniflow_extract. */
 static inline bool
 netdev_flow_key_equal_mf(const struct netdev_flow_key *key,
-- 
2.1.3

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

[ovs-dev] [PATCH 3/4] netdev: Improve comments on netdev_rxq_recv().

[Ben Pfaff]

The comment was incomplete in some ways and simply wrong in others.

Also ensure that *cnt is set to 0 if an error is encountered.  It's nice
when callers can rely on this.

Signed-off-by: Ben Pfaff 
---
 lib/netdev-provider.h | 32 +++-
 lib/netdev.c  | 32 
 2 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 2aa1b5d..1952a02 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -713,23 +713,29 @@ struct netdev_class {
 void (*rxq_destruct)(struct netdev_rxq *);
 void (*rxq_dealloc)(struct netdev_rxq *);
 
-/* Attempts to receive batch of packets from 'rx' and place array of
- * pointers into '*pkts'. netdev is responsible for allocating buffers.
- * '*cnt' points to packet count for given batch. Once packets are returned
- * to caller, netdev should give up ownership of ofpbuf data.
- *
- * Implementations should allocate buffer with DP_NETDEV_HEADROOM headroom
- * and add a VLAN header which is obtained out-of-band to the packet.
- *
- * Caller is expected to pass array of size MAX_RX_BATCH.
- * This function may be set to null if it would always return EOPNOTSUPP
- * anyhow. */
+/* Attempts to receive a batch of packets from 'rx'.  The caller supplies
+ * 'pkts' as the pointer to the beginning of an array of MAX_RX_BATCH
+ * pointers to dp_packet.  If successful, the implementation stores
+ * pointers to up to MAX_RX_BATCH dp_packets into the array, transferring
+ * ownership of the packets to the caller, stores the number of received
+ * packets into '*cnt', and returns 0.
+ *
+ * The implementation does not necessarily initialize any non-data members
+ * of 'pkts'.  That is, the caller must initialize layer pointers and
+ * metadata itself, if desired, e.g. with pkt_metadata_init() and
+ * miniflow_extract().
+ *
+ * Implementations should allocate buffers with DP_NETDEV_HEADROOM bytes of
+ * headroom.
+ *
+ * Returns EAGAIN immediately if no packet is ready to be received or
+ * another positive errno value if an error was encountered. */
 int (*rxq_recv)(struct netdev_rxq *rx, struct dp_packet **pkts,
 int *cnt);
 
 /* Registers with the poll loop to wake up from the next call to
- * poll_block() when a packet is ready to be received with 
netdev_rxq_recv()
- * on 'rx'. */
+ * poll_block() when a packet is ready to be received with
+ * netdev_rxq_recv() on 'rx'. */
 void (*rxq_wait)(struct netdev_rxq *rx);
 
 /* Discards all packets waiting to be received from 'rx'. */
diff --git a/lib/netdev.c b/lib/netdev.c
index ee92e01..150f8d8 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -639,28 +639,28 @@ netdev_rxq_close(struct netdev_rxq *rx)
 }
 }
 
-/* Attempts to receive batch of packets from 'rx'.
- *
- * Returns EAGAIN immediately if no packet is ready to be received.
- *
- * Returns EMSGSIZE, and discards the packet, if the received packet is longer
- * than 'dp_packet_tailroom(buffer)'.
- *
- * It is advised that the tailroom of 'buffer' should be
- * VLAN_HEADER_LEN bytes longer than the MTU to allow space for an
- * out-of-band VLAN header to be added to the packet.  At the very least,
- * 'buffer' must have at least ETH_TOTAL_MIN bytes of tailroom.
- *
- * This function may be set to null if it would always return EOPNOTSUPP
- * anyhow. */
+/* Attempts to receive a batch of packets from 'rx'.  'pkts' should point to
+ * the beginning of an array of MAX_RX_BATCH pointers to dp_packet.  If
+ * successful, this function stores pointers to up to MAX_RX_BATCH dp_packets
+ * into the array, transferring ownership of the packets to the caller, stores
+ * the number of received packets into '*cnt', and returns 0.
+ *
+ * The implementation does not necessarily initialize any non-data members of
+ * 'pkts'.  That is, the caller must initialize layer pointers and metadata
+ * itself, if desired, e.g. with pkt_metadata_init() and miniflow_extract().
+ *
+ * Returns EAGAIN immediately if no packet is ready to be received or another
+ * positive errno value if an error was encountered. */
 int
-netdev_rxq_recv(struct netdev_rxq *rx, struct dp_packet **buffers, int *cnt)
+netdev_rxq_recv(struct netdev_rxq *rx, struct dp_packet **pkts, int *cnt)
 {
 int retval;
 
-retval = rx->netdev->netdev_class->rxq_recv(rx, buffers, cnt);
+retval = rx->netdev->netdev_class->rxq_recv(rx, pkts, cnt);
 if (!retval) {
 COVERAGE_INC(netdev_received);
+} else {
+*cnt = 0;
 }
 return retval;
 }
-- 
2.1.3

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

[ovs-dev] [PATCH 1/4] ovs-ofctl: Fix command names in documentation.

[Ben Pfaff]

The actual command names do not capitalize "tlv".

Signed-off-by: Ben Pfaff 
---
 utilities/ovs-ofctl.8.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index a414408..bc3ddae 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -466,16 +466,16 @@ be lifted in the future to allow for easier management.
 These commands are Nicira extensions to OpenFlow and require Open vSwitch
 2.5 or later.
 
-.IP "\fBadd\-TLV\-map \fIswitch option\fR[\fB,\fIoption\fR]..."
+.IP "\fBadd\-tlv\-map \fIswitch option\fR[\fB,\fIoption\fR]..."
 Add each \fIoption\fR to \fIswitch\fR's tables. Duplicate fields are
 rejected.
 .
-.IP "\fBdel\-TLV\-map \fIswitch \fR[\fIoption\fR[\fB,\fIoption\fR]]..."
+.IP "\fBdel\-tlv\-map \fIswitch \fR[\fIoption\fR[\fB,\fIoption\fR]]..."
 Delete each \fIoption\fR from \fIswitch\fR's table, or all option TLV
 mapping if no \fIoption\fR is specified.
 Fields that aren't mapped are ignored.
 .
-.IP "\fBdump\-TLV\-map \fIswitch\fR"
+.IP "\fBdump\-tlv\-map \fIswitch\fR"
 Show the currently mapped fields in the switch's option table as well
 as switch capabilities.
 .
-- 
2.1.3

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

Re: [ovs-dev] [CudaMailTagged] [PATCH] ofp-util: Fix use-after-free in group append.

[Joe Stringer]

On 4 March 2016 at 15:18, William Tu  wrote:
> It is possible for ofpbuf_put() to realloc a newly allocated address,
> casuing the previously referenced pointer, ogds, points to old/free'd
> address. The issue is generated by forcing ofpbuf_put() to use newly
> allocated buffer and valgrind reports invalid write. The similiar syndrome
> is reported at: https://patchwork.ozlabs.org/patch/591330/
>
> Invalid write of size 2
> ofputil_append_ofp15_group_desc_reply (ofp-util.c:8367)
> ofputil_append_group_desc_reply (ofp-util.c:8392)
> append_group_desc (ofproto.c:6262)
> handle_group_request (ofproto.c:6230)
> handle_group_desc_stats_request (ofproto.c:6269)
> handle_openflow__ (ofproto.c:7337)
> handle_openflow (ofproto.c:7403)
> ofconn_run (connmgr.c:1379)
> connmgr_run (connmgr.c:323)
> ofproto_run (ofproto.c:1762)
> bridge_run__ (bridge.c:2885)
> bridge_run (bridge.c:2940)
> main (ovs-vswitchd.c:120)
>
> Address 0x7cb1020 is 144 bytes inside a block of size 1,144 free'd
> free (vg_replace_malloc.c:530)
> ofpbuf_resize__ (ofpbuf.c:246)
> ofpbuf_put (ofpbuf.c:386)
> nx_put_header__ (nx-match.c:1241)
> nxm_put__ (nx-match.c:697)
> oxm_put_field_array (nx-match.c:1226)
> ofputil_put_group_prop_ntr_selection_method (ofp-util.c:8305)
> ofputil_append_ofp15_group_desc_reply (ofp-util.c:8364)
> ofputil_append_group_desc_reply (ofp-util.c:8392)
> append_group_desc (ofproto.c:6262)
>
> Signed-off-by: William Tu 

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

Re: [ovs-dev] [PATCH] ovs-sandbox: Add note about OVN to initial output.

[Russell Bryant]

On Mon, Mar 7, 2016 at 11:17 AM, Ben Pfaff  wrote:

> On Mon, Mar 07, 2016 at 10:48:21AM -0500, Russell Bryant wrote:
> > When you run ovs-sandbox, it finishes with a note describing the dummy
> > environment it has set up.  Add some additional text that indicates that
> > OVN is also enabled when that is the case.
> >
> > Signed-off-by: Russell Bryant 
>
> Acked-by: Ben Pfaff 
>

Thanks, pushed to master.

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

Re: [ovs-dev] [PATCH] ofpbuf: Fix use-after-free in bundle parse.

[Joe Stringer]

Oh! I've been looking mostly at v2.5 lately and didn't notice how
commit 2bd318dec2428ae6c0febbf79453982676ccb672 changed the
"update_len" (now ofpact_finish) function.

Thanks, I applied this to master.

As a separate thing, I was wondering about whether it's worthwhile to
do something additional to try to avoid this kind of bug in future. A
couple of ideas:
* Rearrange the parse/decode functions so that ofpact_finish() is the
final call within the decode_FOO()/parse_FOO() functions
* Amend ofpact_finish() to have an additional 'void *localptr'
parameter, so that the caller has to explicitly consider whether the
pointer needs to be updated.

Maybe the former is enough, perhaps + amend the comment above
ofpact_finish() to make it more explicit that it may reallocate the
buffer (and therefore invalidate local pointers in the caller
context).

On 4 March 2016 at 21:27, William Tu  wrote:
> Hi Joe,
>
> Thanks for your reply. The reason is that line 181:
> ofpact_finish(ofpacts, >ofpact);
> https://github.com/openvswitch/ovs/blob/34abaa3deaa430ca0b50453865d2e042a5132165/lib/bundle.c#L181
>
> Could also call into ofpbuf_put_zero, which frees the memory pointed by
> bundle.
>
> Regards,
> William
>
> On Fri, Mar 4, 2016 at 6:12 PM, Joe Stringer  wrote:
>>
>> On 4 March 2016 at 18:00, William Tu  wrote:
>> > Address pointed by bundle could be obsolete/free'd when
>> > realloc, called from ofpbuf_put_zero(), returns new address.
>> > Reported by Valgrind 367: ovs-ofctl parse-flows (NXM)
>> >
>> > Invalid write of size 4
>> > bundle_parse__ (bundle.c:200)
>> > bundle_parse_load (bundle.c:272)
>> > parse_bundle_load (ofp-actions.c:1324)
>> > ofpacts_parse__ (ofp-actions.c:7484)
>> > ofpacts_parse (ofp-actions.c:7540)
>> > ofpacts_parse_copy (ofp-actions.c:7558)
>> > parse_ofp_str__ (ofp-parse.c:491)
>> > parse_ofp_str (ofp-parse.c:544)
>> > parse_ofp_flow_mod_str (ofp-parse.c:870)
>> >
>> > Address 0x7a4e96c is 12 bytes inside a block of size 64 free'd
>> > free (vg_replace_malloc.c:530)
>> > ofpbuf_resize__ (ofpbuf.c:246) (purposely add to force using new
>> > buf)
>> > ofpbuf_put_zeros (ofpbuf.c:375)
>> > bundle_parse__ (bundle.c:181)
>> > bundle_parse_load (bundle.c:272)
>> > parse_bundle_load (ofp-actions.c:1324)
>> > ofpacts_parse__ (ofp-actions.c:7484)
>> > ofpacts_parse (ofp-actions.c:7540)
>> > ofpacts_parse_copy (ofp-actions.c:7558)
>> >
>> > Signed-off-by: William Tu 
>> > ---
>> >  lib/bundle.c | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/lib/bundle.c b/lib/bundle.c
>> > index 871a724..1451e92 100644
>> > --- a/lib/bundle.c
>> > +++ b/lib/bundle.c
>> > @@ -180,6 +180,7 @@ bundle_parse__(const char *s, char **save_ptr,
>> >  }
>> >  ofpact_finish(ofpacts, >ofpact);
>> >
>> > +bundle = ofpacts->header;
>> >  bundle->basis = atoi(basis);
>>
>> I don't understand how this would trigger (or how this would fix the
>> issue); the same line is also done directly after ofpbuf_put() inside
>> the loop above:
>>
>> https://github.com/openvswitch/ovs/blob/34abaa3deaa430ca0b50453865d2e042a5132165/lib/bundle.c#L178
>>
>> Can you explain it?
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free.

[William Tu]

Hi Jarno,

Thanks for the feedback. I forgot to mention that this issue is found by
changing the ofpbuf code to make each put reallocate the memory. I patched
the code with:

--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -383,6 +383,7 @@ ofpbuf_put_zeros(struct ofpbuf *b, size_t size)
 void *
 ofpbuf_put(struct ofpbuf *b, const void *p, size_t size)
 {
+ofpbuf_resize__(b, ofpbuf_headroom(b)+64, ofpbuf_tailroom(b));
 void *dst = ofpbuf_put_uninit(b, size);
 memcpy(dst, p, size);
 return dst;

Which forces to increase the headroom and causes allocating new buffer.
Then I re-run the testsuite with valgrind testcase, and a couple of related
issues show up as "Invalid write...".
Another idea is to modify glibc so that realloc will always use newly
allocated memory. In this way we don't need to change ofpbuf code. I'm
still experimenting this approach.

Regards,
William

On Mon, Mar 7, 2016 at 10:46 AM, Jarno Rajahalme  wrote:

> It might be super slow, but how about running the test suite with valgrind
> and ofpbuf code changed so that each put reallocates the memory? That way
> we would not have to be lucky about the timing/placement of reallocations
> to find these bugs?
>
>   Jarno
>
> > On Mar 4, 2016, at 5:35 PM, William Tu  wrote:
> >
> > Address pointed by header_ptr might be free'd due to realloc
> > happened at ofpbuf_put_uninit() and ofpbuf_put_hex(). Reported
> > by valgrind 379: check TCP flags expression in OXM and NXM.
> >
> > Invalid write of size 4
> >nx_match_from_string_raw (nx-match.c:1510)
> >nx_match_from_string (nx-match.c:1538)
> >ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> >ovs_cmdl_run_command (command-line.c:121)
> >main (ovs-ofctl.c:137)
> >
> > Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
> >free (vg_replace_malloc.c:530)
> >ofpbuf_resize__ (ofpbuf.c:246)
> >ofpbuf_put (ofpbuf.c:386)
> >ofpbuf_put_hex (ofpbuf.c:414)
> >nx_match_from_string_raw (nx-match.c:1488)
> >nx_match_from_string (nx-match.c:1538)
> >ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> >
> > Signed-off-by: William Tu 
> > ---
> > lib/nx-match.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/nx-match.c b/lib/nx-match.c
> > index 4999b1a..2615f8c 100644
> > --- a/lib/nx-match.c
> > +++ b/lib/nx-match.c
> > @@ -1470,6 +1470,7 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
> > ovs_be64 *header_ptr;
> > int name_len;
> > size_t n;
> > +ptrdiff_t header_ptr_offset;
> >
> > name = s;
> > name_len = strcspn(s, "(");
> > @@ -1485,6 +1486,7 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
> > s += name_len + 1;
> >
> > header_ptr = ofpbuf_put_uninit(b, nxm_header_len(header));
> > +header_ptr_offset = (char *)header_ptr - (char *)b->data;
> > s = ofpbuf_put_hex(b, s, );
> > if (n != nxm_field_bytes(header)) {
> > const struct mf_field *field = mf_from_oxm_header(header);
> > @@ -1507,6 +1509,10 @@ nx_match_from_string_raw(const char *s, struct
> ofpbuf *b)
> > }
> > }
> > nw_header = htonll(header);
> > +
> > +/* header_ptr might be free'd due to
> > + * ofpbuf_put_uninit() and ofpbuf_put_hex(). */
> > +header_ptr = (ovs_be64 *)((char *)b->data + header_ptr_offset);
> > memcpy(header_ptr, _header, nxm_header_len(header));
> >
> > if (nxm_hasmask(header)) {
> > --
> > 2.5.0
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] ofpbuf: Fix setting of 'msg' in ofpbuf_clone_with_headroom()

[Jarno Rajahalme]

Thanks for the review!

I split out the more substantial change you pointed out to a separate patch and 
pushed the series to master.

  Jarno

> On Mar 4, 2016, at 6:55 PM, Joe Stringer  wrote:
> 
> (Responding to both patches here as I'm having email reception problems)
> 
> On 3 March 2016 at 15:12, Jarno Rajahalme  wrote:
>> Commit 38876d31 fixed setting 'msg' when resizing an ofpbuf, but
>> failed to fix the same issue in ofpbuf_clone_with_headroom().  Without
>> this fix the newly cloned ofpbuf's 'msg', if non-NULL, will point to
>> the buffer of the original ofpbuf.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> In the first patch, there are two changes: Adjusting the header offset
> calculation, and some nice cosmetic improvements. It's not entirely
> straightforward to see, but I think that if the ptrdiff_t is unsigned
> and the new buffer is actually allocated earlier in memory than the
> old buffer, then the offset is really wrong. This change should stand
> alone from cosmetic fixes. That said, I completely agree with those
> changes.
> 
> This second patch LGTM.
> 
> Acked-by: Joe Stringer 

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

Re: [ovs-dev] [PATCH] nx-match: Fix use-after-free.

[Jarno Rajahalme]

It might be super slow, but how about running the test suite with valgrind and 
ofpbuf code changed so that each put reallocates the memory? That way we would 
not have to be lucky about the timing/placement of reallocations to find these 
bugs?

  Jarno

> On Mar 4, 2016, at 5:35 PM, William Tu  wrote:
> 
> Address pointed by header_ptr might be free'd due to realloc
> happened at ofpbuf_put_uninit() and ofpbuf_put_hex(). Reported
> by valgrind 379: check TCP flags expression in OXM and NXM.
> 
> Invalid write of size 4
>nx_match_from_string_raw (nx-match.c:1510)
>nx_match_from_string (nx-match.c:1538)
>ofctl_parse_nxm__ (ovs-ofctl.c:3325)
>ovs_cmdl_run_command (command-line.c:121)
>main (ovs-ofctl.c:137)
> 
> Address 0x7a2cc40 is 0 bytes inside a block of size 64 free'd
>free (vg_replace_malloc.c:530)
>ofpbuf_resize__ (ofpbuf.c:246)
>ofpbuf_put (ofpbuf.c:386)
>ofpbuf_put_hex (ofpbuf.c:414)
>nx_match_from_string_raw (nx-match.c:1488)
>nx_match_from_string (nx-match.c:1538)
>ofctl_parse_nxm__ (ovs-ofctl.c:3325)
> 
> Signed-off-by: William Tu 
> ---
> lib/nx-match.c | 6 ++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 4999b1a..2615f8c 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -1470,6 +1470,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf 
> *b)
> ovs_be64 *header_ptr;
> int name_len;
> size_t n;
> +ptrdiff_t header_ptr_offset;
> 
> name = s;
> name_len = strcspn(s, "(");
> @@ -1485,6 +1486,7 @@ nx_match_from_string_raw(const char *s, struct ofpbuf 
> *b)
> s += name_len + 1;
> 
> header_ptr = ofpbuf_put_uninit(b, nxm_header_len(header));
> +header_ptr_offset = (char *)header_ptr - (char *)b->data;
> s = ofpbuf_put_hex(b, s, );
> if (n != nxm_field_bytes(header)) {
> const struct mf_field *field = mf_from_oxm_header(header);
> @@ -1507,6 +1509,10 @@ nx_match_from_string_raw(const char *s, struct ofpbuf 
> *b)
> }
> }
> nw_header = htonll(header);
> +
> +/* header_ptr might be free'd due to
> + * ofpbuf_put_uninit() and ofpbuf_put_hex(). */
> +header_ptr = (ovs_be64 *)((char *)b->data + header_ptr_offset);
> memcpy(header_ptr, _header, nxm_header_len(header));
> 
> if (nxm_hasmask(header)) {
> -- 
> 2.5.0
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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

Re: [ovs-dev] [PATCH v2] Use 'RUNDIR' from make for rhel/ovn-controller.service

[Ben Pfaff]

On Mon, Mar 07, 2016 at 10:34:30AM -0500, Russell Bryant wrote:
> On Mon, Mar 7, 2016 at 9:37 AM, Russell Bryant  wrote:
> 
> >
> >
> > On Mon, Mar 7, 2016 at 12:03 AM,  wrote:
> >
> >> Perviously it was using the platform's runtime directory which can be
> >> different from the runtime directory of ovsdb-server started by the
> >> openvswitch service
> >>
> >> Signed-off-by: Babu Shanmugam 
> >>
> >>
> > Thanks!  I added the ack from Flavio and applied this to master.
> >
> >
> It looks like this patch broke the build on travis-ci.  For example:
> 
> https://travis-ci.org/openvswitch/ovs/jobs/114254913
> 
> It probably has to do with using a separate build directory.  I'm looking
> into how to fix it, but haven't worked it out just yet.

It probably needs something like in $(update_rhel_spec), so that if it's
just old but not incorrect it just gets touched instead of overwritten.
(It's a nasty kluge.)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 3/5] ovn-controller: Add data structure for indexing lports, multicast groups.

[Ben Pfaff]

On Thu, Mar 03, 2016 at 02:42:45PM -0800, Ben Pfaff wrote:
> On Thu, Mar 03, 2016 at 02:29:16PM -0800, Justin Pettit wrote:
> > 
> > > On Feb 19, 2016, at 4:40 PM, Ben Pfaff  wrote:
> > 
> > > +struct mcgroup {
> > > +struct hmap_node dp_name_node; /* Index by (logical datapath, name). 
> > > */
> > > +const struct sbrec_multicast_group *mg;
> > > +};
> > > +
> > > +void
> > > +mcgroup_index_init(struct mcgroup_index *mcgroups, struct ovsdb_idl 
> > > *ovnsb_idl)
> > > +{
> > > +hmap_init(>by_dp_name);
> > > +
> > > +const struct sbrec_multicast_group *sb;
> > 
> > Pretty minor, but when looking at the code later in the function, I
> > would have thought "sb" was a pointer to the database.  I think I was
> > particularly thrown because it's referred to as "mg" in the "mcgroup"
> > definition.
> 
> 'sb' is a pointer to a database record (any "struct sbrec_*" is a
> database record), so you would have thought correctly.
> 
> What do you mean?

After talking to you in person I understood that you thought this
variable would be better named 'mg', so I changed the name.  Thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 2/5] ovn: Use callback function instead of simap for logical port number map.

[Ben Pfaff]

On Wed, Mar 02, 2016 at 06:26:03PM -0800, Justin Pettit wrote:
> 
> > On Feb 19, 2016, at 4:40 PM, Ben Pfaff  wrote:
> 
> > @@ -2433,10 +2442,13 @@ add_cmp_flow(const struct expr *cmp, const struct 
> > simap *ports,
> >  *   caller should remap the conj_id and add the OpenFlow flow with its
> >  *   desired actions.
> >  *
> > - * 'ports' must be a map from strings (presumably names of ports) to 
> > integers.
> > - * Any comparisons against string fields in 'expr' are translated into 
> > integers
> > - * through this map.  A comparison against a string that is not in 'ports' 
> > acts
> > - * like a Boolean "false"; that is, it will always fail to match.  For a 
> > simple
> > + * 'lookup_port' must be a function to map from a port name to a port 
> > number.
> > + * When successful, 'lookup_port' stores the port number into '*portp' and
> > + * returns true; when there is no port by the given name, it returns false.
> > + * 'aux' is passed to 'lookup_port' as auxiliary data.  Any comparisons 
> > against
> > + * string fields in 'expr' are translated into integers through this this
> 
> There's a duplicated "this".
> 
> Acked-by: Justin Pettit 

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

Re: [ovs-dev] [PATCH] ovs-sandbox: Add note about OVN to initial output.

[Ryan Moats]

"dev"  wrote on 03/07/2016 09:48:21 AM:

> From: Russell Bryant 
> To: dev@openvswitch.org
> Date: 03/07/2016 09:48 AM
> Subject: [ovs-dev] [PATCH] ovs-sandbox: Add note about OVN to initial
output.
> Sent by: "dev" 
>
> When you run ovs-sandbox, it finishes with a note describing the dummy
> environment it has set up.  Add some additional text that indicates that
> OVN is also enabled when that is the case.
>
> Signed-off-by: Russell Bryant 

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

Re: [ovs-dev] [PATCH] ovs-sandbox: Add note about OVN to initial output.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 10:48:21AM -0500, Russell Bryant wrote:
> When you run ovs-sandbox, it finishes with a note describing the dummy
> environment it has set up.  Add some additional text that indicates that
> OVN is also enabled when that is the case.
> 
> Signed-off-by: Russell Bryant 

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

Re: [ovs-dev] [PATCH] AUTHORS: Add Ramu Ramamurthy.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 10:25:43AM -0500, Russell Bryant wrote:
> Ramu is the author of 3a83007a76bbf05144cee1fda7ad81c1c717dca7.
> 
> Signed-off-by: Russell Bryant 

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

[ovs-dev] [PATCH v10 0/1] Separating OVN NB and SB database processes

[Ryan Moats]

From: RYAN D. MOATS 

Changes from V9:

Updated tutorial/ovs-sandbox to work with the split database processes
Addressed russellb's comments on V9:
 - fixed vtep test case to split databases rather than change all the
   ovn-*ctl calls
 - fixed indentation on set_defaults()
 - stop_ovsdb() now uses ovs-appctl

RYAN D. MOATS (1):
  Separating OVN NB and SB database processes

 debian/ovn-central.init  |8 ++-
 ovn/northd/ovn-northd.c  |   33 +++---
 ovn/utilities/ovn-ctl|  151 +++---
 ovn/utilities/ovn-nbctl.c|2 +-
 ovn/utilities/ovn-sbctl.c|3 +-
 tests/ovn-controller-vtep.at |   11 ++-
 tests/ovn-nbctl.at   |2 +-
 tests/ovn-sbctl.at   |   12 ++--
 tutorial/ovs-sandbox |   28 +++--
 9 files changed, 197 insertions(+), 53 deletions(-)

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

[ovs-dev] [PATCH v10 1/1] Separating OVN NB and SB database processes

[Ryan Moats]

From: RYAN D. MOATS 

OVN NB & SB DB's should be run in separate ovsdb-server processes
and should run with ovn-ctl start_northd / stop_northd.  This patch
includes changes to unit tests, tutorial and debian scripts to remain
self-consistent.

Signed-off-by: RYAN D. MOATS 
Signed-off-by: Michael Arnaldi 
---
 debian/ovn-central.init  |8 ++-
 ovn/northd/ovn-northd.c  |   33 +++---
 ovn/utilities/ovn-ctl|  151 +++---
 ovn/utilities/ovn-nbctl.c|2 +-
 ovn/utilities/ovn-sbctl.c|3 +-
 tests/ovn-controller-vtep.at |   11 ++-
 tests/ovn-nbctl.at   |2 +-
 tests/ovn-sbctl.at   |   12 ++--
 tutorial/ovs-sandbox |   28 +++--
 9 files changed, 197 insertions(+), 53 deletions(-)

diff --git a/debian/ovn-central.init b/debian/ovn-central.init
index a75192f..0c5b09e 100755
--- a/debian/ovn-central.init
+++ b/debian/ovn-central.init
@@ -28,12 +28,18 @@ start () {
 "$@" || exit $?
 }
 
+stop_northd () {
+set /usr/share/openvswitch/scripts/ovn-ctl ${1-stop_northd}
+set "$@" $OVN_CTL_OPTS
+"$@" || exit $?
+}
+
 case $1 in
 start)
 start
 ;;
 stop)
-/usr/share/openvswitch/scripts/ovn-ctl stop_northd
+stop_northd
 ;;
 restart)
 start restart_northd
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 35ec267..f362fe4 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -52,7 +52,8 @@ struct northd_context {
 static const char *ovnnb_db;
 static const char *ovnsb_db;
 
-static const char *default_db(void);
+static const char *default_nb_db(void);
+static const char *default_sb_db(void);
 
 /* Pipeline stages. */
 
@@ -168,7 +169,7 @@ Options:\n\
   -h, --helpdisplay this help message\n\
   -o, --options list available options\n\
   -V, --version display version information\n\
-", program_name, program_name, default_db(), default_db());
+", program_name, program_name, default_nb_db(), default_sb_db());
 daemon_usage();
 vlog_usage();
 stream_usage("database", true, true, false);
@@ -1922,15 +1923,26 @@ ovnsb_db_run(struct northd_context *ctx)
 }
 
 
-static char *default_db_;
+static char *default_nb_db_;
 
 static const char *
-default_db(void)
+default_nb_db(void)
 {
-if (!default_db_) {
-default_db_ = xasprintf("unix:%s/db.sock", ovs_rundir());
+if (!default_nb_db_) {
+default_nb_db_ = xasprintf("unix:%s/ovnnb_db.sock", ovs_rundir());
 }
-return default_db_;
+return default_nb_db_;
+}
+
+static char *default_sb_db_;
+
+static const char *
+default_sb_db(void)
+{
+if (!default_sb_db_) {
+default_sb_db_ = xasprintf("unix:%s/ovnsb_db.sock", ovs_rundir());
+}
+return default_sb_db_;
 }
 
 static void
@@ -1992,11 +2004,11 @@ parse_options(int argc OVS_UNUSED, char *argv[] 
OVS_UNUSED)
 }
 
 if (!ovnsb_db) {
-ovnsb_db = default_db();
+ovnsb_db = default_sb_db();
 }
 
 if (!ovnnb_db) {
-ovnnb_db = default_db();
+ovnnb_db = default_nb_db();
 }
 
 free(short_options);
@@ -2112,7 +2124,8 @@ main(int argc, char *argv[])
 ovsdb_idl_loop_destroy(_idl_loop);
 service_stop();
 
-free(default_db_);
+free(default_nb_db_);
+free(default_sb_db_);
 exit(res);
 }
 
diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index b171934..15096f4 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -30,32 +30,79 @@ done
 ## start ##
 ## - ##
 
-upgrade_ovn_dbs () {
-ovn_dbs=$(ovs-appctl -t ovsdb-server ovsdb-server/list-dbs 2>/dev/null)
-for db in $ovn_dbs; do
-case $db in
-OVN*)
-action "Removing $db from ovsdb-server" \
-ovs-appctl -t ovsdb-server ovsdb-server/remove-db $db
-;;
-esac
-done
-upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA"
-upgrade_db "$DB_SB_FILE" "$DB_SB_SCHEMA"
-for db in $DB_NB_FILE $DB_SB_FILE; do
-action "Adding $db to ovsdb-server" \
-ovs-appctl -t ovsdb-server ovsdb-server/add-db $db || exit 1
-done
+pidfile_is_running () {
+pidfile=$1
+test -e "$pidfile" && pid=`cat "$pidfile"` && pid_exists "$pid"
+} >/dev/null 2>&1
+
+stop_ovsdb () {
+if pidfile_is_running $DB_NB_PID; then
+ovs-appctl -t ovnnb_db exit
+fi
+
+if pidfile_is_running $DB_SB_PID; then
+ovs-appctl -t ovnsb_db exit
+fi
+}
+
+start_ovsdb () {
+# Check and eventually start ovsdb-server for Northbound DB
+if ! pidfile_is_running $DB_NB_PID; then
+upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA" 1>/dev/null 2>/dev/null
+
+set ovsdb-server
+
+set "$@" --detach $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE 
--remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT --pidfile=$DB_NB_PID 
--unixctl=ovnnb_db
+
+$@ $DB_NB_FILE
+fi
+
+   

[ovs-dev] [PATCH] ovs-sandbox: Add note about OVN to initial output.

[Russell Bryant]

When you run ovs-sandbox, it finishes with a note describing the dummy
environment it has set up.  Add some additional text that indicates that
OVN is also enabled when that is the case.

Signed-off-by: Russell Bryant 
---
 tutorial/ovs-sandbox | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index ea3b827..99cc3bb 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -360,8 +360,16 @@ cat 

Re: [ovs-dev] [PATCH v2] Use 'RUNDIR' from make for rhel/ovn-controller.service

[Russell Bryant]

On Mon, Mar 7, 2016 at 9:37 AM, Russell Bryant  wrote:

>
>
> On Mon, Mar 7, 2016 at 12:03 AM,  wrote:
>
>> Perviously it was using the platform's runtime directory which can be
>> different from the runtime directory of ovsdb-server started by the
>> openvswitch service
>>
>> Signed-off-by: Babu Shanmugam 
>>
>>
> Thanks!  I added the ack from Flavio and applied this to master.
>
>
It looks like this patch broke the build on travis-ci.  For example:

https://travis-ci.org/openvswitch/ovs/jobs/114254913

It probably has to do with using a separate build directory.  I'm looking
into how to fix it, but haven't worked it out just yet.

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

[ovs-dev] [PATCH] AUTHORS: Add Ramu Ramamurthy.

[Russell Bryant]

Ramu is the author of 3a83007a76bbf05144cee1fda7ad81c1c717dca7.

Signed-off-by: Russell Bryant 
---
 AUTHORS | 1 +
 1 file changed, 1 insertion(+)


I meant to incldue this when I pushed Ramu's bug fix, but forgot to
amend the commit.  I have already pushed this to master.


diff --git a/AUTHORS b/AUTHORS
index 96bdc4f..1184a51 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -162,6 +162,7 @@ pritesh pritesh.koth...@cisco.com
 Pravin B Shelar pshe...@nicira.com
 Raju Subramanianrsubraman...@nicira.com
 Rami Rosen  ramir...@gmail.com
+Ramu Ramamurthy ramu.ramamur...@us.ibm.com
 Randall Sharo   andall.sh...@navy.mil
 Ravi Kerur  ravi.ke...@telekom.com
 Reid Price  r...@nicira.com
-- 
2.5.0

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

Re: [ovs-dev] weekly OVN report

[Russell Bryant]

On Mon, Mar 7, 2016 at 9:47 AM, Dan Mihai Dumitriu 
wrote:

> I would argue for a server-side OVSDB to other backend proxy. I would also
> argue against a client side (in ovn-controller) abstraction of the other-db
> - one reason for this is related to the security/safety argument, in the
> sense that other-db may not have any ACL mechanism, or any way to limit
> what the client can do, and thus would be susceptible to a compromised
> chassis. If OVN has its own RPC mechanism between the chassis
> (ovn-controller) and the control cluster, the security issues can be
> controlled more precisely, considering the particular requirements of this
> system. I think there are also advantages for doing upgrades, and just
> generally for decoupling the ovn-controller implementation from the db
> backend.
>

Good points.  Thanks for sharing.

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

[ovs-dev] Shipping Information - Your Order #357-3610

[Graciela Simon]

Dear Customer,

Your order will be shipped shortly, we apologize for the troubles. Please, 
review the invoice in the attached file.

Sincerely,

Graciela Simon
Project Manager
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] weekly OVN report

[Dan Mihai Dumitriu]

I would argue for a server-side OVSDB to other backend proxy. I would also
argue against a client side (in ovn-controller) abstraction of the other-db
- one reason for this is related to the security/safety argument, in the
sense that other-db may not have any ACL mechanism, or any way to limit
what the client can do, and thus would be susceptible to a compromised
chassis. If OVN has its own RPC mechanism between the chassis
(ovn-controller) and the control cluster, the security issues can be
controlled more precisely, considering the particular requirements of this
system. I think there are also advantages for doing upgrades, and just
generally for decoupling the ovn-controller implementation from the db
backend.

On Mon, Mar 7, 2016 at 11:34 PM, Russell Bryant  wrote:

> On Mon, Mar 7, 2016 at 9:29 AM, Russell Bryant  wrote:
>
>> On Sun, Mar 6, 2016 at 11:40 PM, Dan Mihai Dumitriu 
>> wrote:
>>
>>> I'd argue for the approach of keeping the OVSDB protocol in place,
>>> because the SB schema is already there, well understood, and making the
>>> central DB a fault tolerant cluster would have little or no impact on the
>>> ovs-controller implementation. It would also allow the current single OVSDB
>>> to continue to function while the cluster is developed.
>>>
>>
> Do you also mean keep ovsdb-server, or did you mean writing a server-side
> OVSDB-to-other-db proxy?
>
> I was thinking that if we wanted to add support for another db, doing it
> with a client side abstraction would be better instead of adding our own
> custom server-side component.
>
> --
> Russell Bryant
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: race between binding-run and patch-run for localnet ports

[Russell Bryant]

On Fri, Mar 4, 2016 at 8:40 PM, Ramu Ramamurthy 
wrote:

> when ctx->ovnsb_idl_txn is null, binding_run exits early
> and does not add any local_datapaths, but patch_run
> doesnt check this, and ends up deleting localnet ports,
> because there are no local datapaths for them,
> They get readded in a subsequent run causing unnecessary
> deletion and readdition.
>
> Signed-off-by: Ramu Ramamurthy 


Nice catch.  I added your name to the AUTHORS file and applied this patch
to master.  Congratulations on getting your first patch merged!

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

Re: [ovs-dev] weekly OVN report

[Dan Mihai Dumitriu]

Hi Russell,

Nice writeup of the issues and potential solutions. We have been thinking
along the same lines.

Cheers,
Dan


On Mon, Mar 7, 2016 at 11:29 PM, Russell Bryant  wrote:

> On Sun, Mar 6, 2016 at 11:40 PM, Dan Mihai Dumitriu 
> wrote:
>
>> I'd argue for the approach of keeping the OVSDB protocol in place,
>> because the SB schema is already there, well understood, and making the
>> central DB a fault tolerant cluster would have little or no impact on the
>> ovs-controller implementation. It would also allow the current single OVSDB
>> to continue to function while the cluster is developed.
>>
>> That said, if the current OVSDB doesn't have an ACL model, I have some
>> security and robustness concerns, once it's run at scale. Has it been
>> considered to add an ACL model to OVSDB?
>>
>
> I've put some thought into the security concerns of the southbound
> database.  I wrote a bit about this in the ovn/TODO file.  See "limiting
> the impact of a compromised chassis".
>
> https://github.com/openvswitch/ovs/blob/master/ovn/TODO#L128
>
> Since writing that, I've come to think that as a first step, making
> ovn-controller (at least optionally) only have read-only access to the
> southbound database would be a good step.  This would require:
>
>  - Instead of having ovn-controller create the Chassis and Encap rows for
> itself, make that an administrative task when bring a new chassis online.
> This can be done with the ovn-sbctl utility.  It's data that is set once
> and very rarely changed.
>
>  - Move the job of assigning port bindings.  Right now ovn-controller sees
> ports appear locally and automatically matches them up with OVN logical
> ports and updates the port bindings.  This is quite convenient, but we
> could optionally force an external entity to define the port bindings
> instead.  In the case of OpenStack, the Neutron plugin could do this.  In
> fact, Neutron already expects to be in charge of port-host bindings, and we
> just ignore that for OVN right now.
>
> If we take that route, this doesn't seem too hard to do.
>
> --
> Russell Bryant
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Use 'RUNDIR' from make for rhel/ovn-controller.service

[Russell Bryant]

On Mon, Mar 7, 2016 at 12:03 AM,  wrote:

> Perviously it was using the platform's runtime directory which can be
> different from the runtime directory of ovsdb-server started by the
> openvswitch service
>
> Signed-off-by: Babu Shanmugam 
>
>
Thanks!  I added the ack from Flavio and applied this to master.

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

Re: [ovs-dev] weekly OVN report

[Russell Bryant]

On Mon, Mar 7, 2016 at 9:29 AM, Russell Bryant  wrote:

> On Sun, Mar 6, 2016 at 11:40 PM, Dan Mihai Dumitriu 
> wrote:
>
>> I'd argue for the approach of keeping the OVSDB protocol in place,
>> because the SB schema is already there, well understood, and making the
>> central DB a fault tolerant cluster would have little or no impact on the
>> ovs-controller implementation. It would also allow the current single OVSDB
>> to continue to function while the cluster is developed.
>>
>
Do you also mean keep ovsdb-server, or did you mean writing a server-side
OVSDB-to-other-db proxy?

I was thinking that if we wanted to add support for another db, doing it
with a client side abstraction would be better instead of adding our own
custom server-side component.

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

Re: [ovs-dev] weekly OVN report

[Russell Bryant]

On Sun, Mar 6, 2016 at 11:40 PM, Dan Mihai Dumitriu 
wrote:

> I'd argue for the approach of keeping the OVSDB protocol in place, because
> the SB schema is already there, well understood, and making the central DB
> a fault tolerant cluster would have little or no impact on the
> ovs-controller implementation. It would also allow the current single OVSDB
> to continue to function while the cluster is developed.
>
> That said, if the current OVSDB doesn't have an ACL model, I have some
> security and robustness concerns, once it's run at scale. Has it been
> considered to add an ACL model to OVSDB?
>

I've put some thought into the security concerns of the southbound
database.  I wrote a bit about this in the ovn/TODO file.  See "limiting
the impact of a compromised chassis".

https://github.com/openvswitch/ovs/blob/master/ovn/TODO#L128

Since writing that, I've come to think that as a first step, making
ovn-controller (at least optionally) only have read-only access to the
southbound database would be a good step.  This would require:

 - Instead of having ovn-controller create the Chassis and Encap rows for
itself, make that an administrative task when bring a new chassis online.
This can be done with the ovn-sbctl utility.  It's data that is set once
and very rarely changed.

 - Move the job of assigning port bindings.  Right now ovn-controller sees
ports appear locally and automatically matches them up with OVN logical
ports and updates the port bindings.  This is quite convenient, but we
could optionally force an external entity to define the port bindings
instead.  In the case of OpenStack, the Neutron plugin could do this.  In
fact, Neutron already expects to be in charge of port-host bindings, and we
just ignore that for OVN right now.

If we take that route, this doesn't seem too hard to do.

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

Re: [ovs-dev] vhost-user invalid txqid cause discard of packets

[Wang, Zhihong]

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, March 4, 2016 7:14 PM
> To: Wang, Zhihong ; dev@openvswitch.org
> Cc: Flavio Leitner ; Traynor, Kevin 
> ;
> Dyasly Sergey 
> Subject: Re: vhost-user invalid txqid cause discard of packets
> 
> On 04.03.2016 13:50, Wang, Zhihong wrote:
> >
> >
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >> Sent: Friday, March 4, 2016 6:00 PM
> >> To: Wang, Zhihong ; dev@openvswitch.org
> >> Cc: Flavio Leitner ; Traynor, Kevin
> ;
> >> Dyasly Sergey 
> >> Subject: Re: vhost-user invalid txqid cause discard of packets
> >>
> >> Hi, Zhihong.
> >> I can't reproduce this in my environment.
> >> Could you please provide ovs-vswithd.log with VLOG_DBG enabled
> >> for netdev-dpdk and outputs of following commands:
> >> # ovs-vsctl show
> >> # ovs-appctl dpctl/show
> >> # ovs-appctl dpif-netdev/pmd-rxq-show
> >> in 'good' and 'bad' states?
> >>
> >> Also, are you sure that VM started with exactly 4 queues?
> >
> >
> > Yes it's exact 4 queues.
> > Please see command output below.
> >
> > In "bad" case only vhost txq 0, 1 are sending packets, I believe the other 2
> > become -1 after the lookup.
> 
> I don't think so. Reconfiguration code can't affect vHost queue mapping.
> Only distribution of queues between threads changed here.
> Can you reproduce this issue with another cpu-mask?
> For example : '00ff0' and '003f0'.


The results is always the same no matter what 8 cores & 6 cores are used, like
fc0c & f00c.

The way I launch OVS is like below, could you check if there's anything wrong?
You should be able to reproduce this issue with it.
The qemu version is 2.5, OVS and DPDK are pulled from the master Mar 1st.
-
export DB_SOCK=/var/run/openvswitch/db.sock
#cd ovsdb
./ovsdb-tool create /etc/openvswitch/conf.db 
/usr/share/openvswitch/vswitch.ovsschema
./ovsdb-server --remote=punix:$DB_SOCK 
--remote=db:Open_vSwitch,Open_vSwitch,manager_options --pidfile --detach

#cd utilities
./ovs-vsctl --no-wait init
cd $1/vswitchd
./ovs-vswitchd --dpdk -c 0xf -n 3 --socket-mem 1024 -- unix:$DB_SOCK --pidfile 
--log-file=/var/log/openvswitch/ovs-vswitchd.log &

#cd utilities
./ovs-vsctl add-br ovsbr0 -- set bridge ovsbr0 datapath_type=netdev
./ovs-vsctl add-port ovsbr0 vhost-user1 -- set Interface vhost-user1 
type=dpdkvhostuser
./ovs-vsctl add-port ovsbr0 dpdk0 -- set Interface dpdk0 type=dpdk
./ovs-vsctl set Interface vhost-user1 options:n_rxq=4
./ovs-vsctl set Interface dpdk0 options:n_rxq=4
./ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x3fc00
./ovs-ofctl del-flows ovsbr0
./ovs-ofctl add-flow ovsbr0 in_port=1,action=output:2
./ovs-ofctl add-flow ovsbr0 in_port=2,action=output:1
-

Some logs below, not sure which part you're looking for.

"bad" 6c:
2016-03-07T06:37:32.291Z|1|dpif_netdev(pmd84)|DBG|Core 12 processing port 
'vhost-user1' with queue-id 0
2016-03-07T06:37:32.291Z|2|dpif_netdev(pmd84)|DBG|Core 12 processing port 
'dpdk0' with queue-id 2
2016-03-07T06:37:32.292Z|1|dpif_netdev(pmd80)|DBG|Core 17 processing port 
'dpdk0' with queue-id 1
2016-03-07T06:37:32.292Z|1|dpif_netdev(pmd79)|DBG|Core 16 processing port 
'dpdk0' with queue-id 0
2016-03-07T06:37:32.292Z|1|dpif_netdev(pmd83)|DBG|Core 13 processing port 
'vhost-user1' with queue-id 1
2016-03-07T06:37:32.292Z|2|dpif_netdev(pmd83)|DBG|Core 13 processing port 
'dpdk0' with queue-id 3
2016-03-07T06:37:32.293Z|1|dpif_netdev(pmd81)|DBG|Core 15 processing port 
'vhost-user1' with queue-id 3
2016-03-07T06:37:32.294Z|1|dpif_netdev(pmd82)|DBG|Core 14 processing port 
'vhost-user1' with queue-id 2

"good" 8c:
2016-03-07T06:40:34.200Z|1|dpif_netdev(pmd86)|DBG|Core 10 processing port 
'vhost-user1' with queue-id 0
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd79)|DBG|Core 13 processing port 
'vhost-user1' with queue-id 3
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd80)|DBG|Core 14 processing port 
'dpdk0' with queue-id 0
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd85)|DBG|Core 11 processing port 
'vhost-user1' with queue-id 1
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd84)|DBG|Core 12 processing port 
'vhost-user1' with queue-id 2
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd81)|DBG|Core 15 processing port 
'dpdk0' with queue-id 1
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd83)|DBG|Core 17 processing port 
'dpdk0' with queue-id 3
2016-03-07T06:40:34.202Z|1|dpif_netdev(pmd82)|DBG|Core 16 processing port 
'dpdk0' with queue-id 2


> 
> We will see exact mapping in ovs-vswitchd.log with VLOG_DBG enabled for
> netdev-dpdk. (-vdpdk:all:dbg option to ovs-vswitchd).
> 
> One more comment inlined.
> 
> >
> 

[ovs-dev] Queues for DPDK interfaces in OVS-DPDK

[ravali.burra]

Hi Team,

We have the support for queues creation on QoS record in OVS.

Can anyone help in creation of queues for DPDK type interfaces in OVS-DPDK.

Thanks in advance.

Thanks & Regards,
Ravali
The information contained in this electronic message and any attachments to 
this message are intended for the exclusive use of the addressee(s) and may 
contain proprietary, confidential or privileged information. If you are not the 
intended recipient, you should not disseminate, distribute or copy this e-mail. 
Please notify the sender immediately and destroy all copies of this message and 
any attachments. WARNING: Computer viruses can be transmitted via email. The 
recipient should check this email and any attachments for the presence of 
viruses. The company accepts no liability for any damage caused by any virus 
transmitted by this email. www.wipro.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Shipping Information - Your Order #632-1504

[Bart Stark]

Dear Customer,

Your order will be shipped shortly, we apologize for the troubles. Please, 
review the invoice in the attached file.

Sincerely,

Bart Stark
Project Manager
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 11/11] netdev-dpdk: Use ->reconfigure() call to change rx/tx queues.

[Kavanagh, Mark B]

Hi Daniele,

Thanks for making the changes to this patch and also 1/11 and 7/11. 

Apart from one minor comment below, all LGTM.

Thanks,
Mark

>
>This introduces in dpif-netdev and netdev-dpdk the first use for the
>newly introduce reconfigure netdev call.
>
>When a request to change the number of queues comes, netdev-dpdk will
>remember this and notify the upper layer via
>netdev_request_reconfigure().
>
>The datapath, instead of periodically calling netdev_set_multiq(), can
>detect this and call reconfigure().
>
>This mechanism can also be used to:
>* Automatically match the number of rxq with the one provided by qemu
>  via the new_device callback.
>* Provide a way to change the MTU of dpdk devices at runtime.
>
>Signed-off-by: Daniele Di Proietto 
>---
> lib/dpif-netdev.c |  70 +--
> lib/netdev-dpdk.c | 181 +-
> lib/netdev-provider.h |  19 ++
> lib/netdev.c  |  30 ++---
> lib/netdev.h  |   3 +-
> 5 files changed, 136 insertions(+), 167 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 0c3673b..8e9181c 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -256,8 +256,6 @@ struct dp_netdev_port {
> unsigned n_rxq; /* Number of elements in 'rxq' */
> struct netdev_rxq **rxq;
> char *type; /* Port type as requested by user. */
>-int latest_requested_n_rxq; /* Latest requested from netdev number
>-   of rx queues. */
> };
>
> /* Contained by struct dp_netdev_flow's 'stats' member.  */
>@@ -1161,20 +1159,26 @@ do_add_port(struct dp_netdev *dp, const char *devname, 
>const char
>*type,
> /* There can only be ovs_numa_get_n_cores() pmd threads,
>  * so creates a txq for each, and one extra for the non
>  * pmd threads. */
>-error = netdev_set_multiq(netdev, n_cores + 1,
>-  netdev_requested_n_rxq(netdev));
>+error = netdev_set_multiq(netdev, n_cores + 1);
> if (error && (error != EOPNOTSUPP)) {
> VLOG_ERR("%s, cannot set multiq", devname);
> goto out_close;
> }
> }
>+
>+if (netdev_is_reconf_required(netdev)) {
>+error = netdev_reconfigure(netdev);
>+if (error) {
>+goto out_close;
>+}
>+}
>+
> port = xzalloc(sizeof *port);
> port->port_no = port_no;
> port->netdev = netdev;
> port->n_rxq = netdev_n_rxq(netdev);
> port->rxq = xmalloc(sizeof *port->rxq * port->n_rxq);
> port->type = xstrdup(type);
>-port->latest_requested_n_rxq = netdev_requested_n_rxq(netdev);
>
> for (i = 0; i < port->n_rxq; i++) {
> error = netdev_rxq_open(netdev, >rxq[i], i);
>@@ -2450,24 +2454,6 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op 
>**ops, size_t
>n_ops)
> }
> }
>
>-/* Returns true if the configuration for rx queues is changed. */
>-static bool
>-pmd_n_rxq_changed(const struct dp_netdev *dp)
>-{
>-struct dp_netdev_port *port;
>-
>-CMAP_FOR_EACH (port, node, >ports) {
>-int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>-
>-if (netdev_is_pmd(port->netdev)
>-&& port->latest_requested_n_rxq != requested_n_rxq) {
>-return true;
>-}
>-}
>-
>-return false;
>-}
>-
> static bool
> cmask_equals(const char *a, const char *b)
> {
>@@ -2600,14 +2586,12 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
> dp_netdev_destroy_all_pmds(dp);
>
> CMAP_FOR_EACH (port, node, >ports) {
>-struct netdev *netdev = port->netdev;
>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>-if (netdev_is_pmd(port->netdev)
>-&& port->latest_requested_n_rxq != requested_n_rxq) {
>+if (netdev_is_reconf_required(port->netdev)) {
> cmap_remove(>ports, >node, 
> hash_odp_port(port->port_no));
> hmapx_add(_reconfigure, port);
> }
> }
>+
> ovs_mutex_unlock(>port_mutex);
>
> /* Waits for the other threads to see the ports removed from the cmap,
>@@ -2616,11 +2600,9 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>
> ovs_mutex_lock(>port_mutex);
> HMAPX_FOR_EACH (node, _reconfigure) {
>-int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
> int i, err;
>
> port = node->data;
>-requested_n_rxq = netdev_requested_n_rxq(port->netdev);
> /* Closes the existing 'rxq's. */
> for (i = 0; i < port->n_rxq; i++) {
> netdev_rxq_close(port->rxq[i]);
>@@ -2628,18 +2610,15 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
> }
> port->n_rxq = 0;
>
>-/* Sets the new rx queue config. */
>-err = netdev_set_multiq(port->netdev, ovs_numa_get_n_cores() + 1,
>-requested_n_rxq);
>+/* Allows the netdev to apply the pending configuration changes. */
>+ 

[ovs-dev] Message could not be delivered

[Returned mail]

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

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

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

Your message could not be delivered within 3 days:
Mail server 141.213.243.139 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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