date:20171026

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-26 Thread Darrell Ball



On 10/19/17, 9:56 AM, "ovs-dev-boun...@openvswitch.org on behalf of 
antonio.fische...@intel.com"  wrote:

In case a mempool name could not be generated log a message
and return a null mempool pointer to the caller.

CC: Mark B Kavanagh 
CC: Darrell Ball 
CC: Ciara Loftus 
CC: Kevin Traynor 
CC: Aaron Conole 
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each 
port.")
Signed-off-by: Antonio Fischetti 
---
 lib/netdev-dpdk.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index dc1e9c3..6fc6e1b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
 int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
h, dmp->socket_id, dmp->mtu, dmp->mp_size);
 if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {

I see you copied me on an earlier version, so I’ll add one comment.
Given MTU size restriction for dpdk and rte max queues and max buf desc 
enforcement, can this condition be met ?
I think there are 11 remaining characters (out of 31) for mp_size and the 
calculation is 

dmp->mp_size = dev->requested_n_rxq * dev->requested_rxq_size
+ dev->requested_n_txq * dev->requested_txq_size
+ MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST
+ MIN_NB_MBUF;

I got 9 decimal digits max, assuming my math is correct…

Previous filtering for mtu (netdev_dpdk_set_mtu) and max queues 
(dpdk_eth_dev_init)/max buf desc (dpdk_process_queue_size)
enforcement (present and also any future design) should prevent snprintf from 
having an unexpected return value number of chars, which would
mean snprintf should only have an unexpected return value number of chars, if a 
future coding error were to be introduced.
FYI, OVS traditionally deals with other such cases with ovs_assert(), not a 
VLOG_DBG, since it is easier to find the bug earlier.
I believe snprintf at this level should not be catching user config errors.



+VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
+"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
+ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
 return NULL;
 }
 return mp_name;
@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 
 do {
 char *mp_name = dpdk_mp_name(dmp);
+if (!mp_name) {
+rte_free(dmp);
+return NULL;
+}
 
 VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
   "on socket %d for %d Rx and %d Tx queues.",
-- 
2.4.11

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

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=cKHgut93ZGYQzoLXpXQWPqcZhBX3NMdQaRnbNlfvhiU=tx4JYNmCFZ-vgGQzTIteThFJN2RSlMtqg34oTwlFEF0=


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

[ovs-dev] [PATCH] dpif-netdev: Initialize new rxqs in port_reconfigure().

2017-10-26 Thread Ben Pfaff

valgrind reported use of uninitialized data in port_reconfigure(), which
was due to xrealloc() not initializing the newly added data, combined with
dp_netdev_rxq_set_intrvl_cycles() reading 'intrvl_idx' from the added data.
This avoids the warning.

Signed-off-by: Ben Pfaff 
---
 lib/dpif-netdev.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d5eb8305c8a2..1418175bdfef 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3285,9 +3285,14 @@ port_reconfigure(struct dp_netdev_port *port)
 port->txq_used = xcalloc(netdev_n_txq(netdev), sizeof *port->txq_used);
 
 for (i = 0; i < netdev_n_rxq(netdev); i++) {
+bool new_queue = i >= last_nrxq;
+if (new_queue) {
+memset(>rxqs[i], 0, sizeof port->rxqs[i]);
+}
+
 port->rxqs[i].port = port;
-if (i >= last_nrxq) {
-/* Only reset cycle stats for new queues */
+
+if (new_queue) {
 dp_netdev_rxq_set_cycles(>rxqs[i], RXQ_CYCLES_PROC_CURR, 0);
 dp_netdev_rxq_set_cycles(>rxqs[i], RXQ_CYCLES_PROC_HIST, 0);
 for (unsigned j = 0; j < PMD_RXQ_INTERVAL_MAX; j++) {
-- 
2.10.2

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

Re: [ovs-dev] [PKG-Openstack-devel] Bug#878249: recent OVS upload

2017-10-26 Thread Thomas Goirand

Hi Ben,

On 10/27/2017 12:45 AM, Ben Pfaff wrote:
> Thanks for doing an upload.
> 
> Open vSwitch users expect to be able to build packages for whatever
> version of Open vSwitch they're using, which is most easily satisfied by
> keeping the Debian packaging with the rest of the tree.  Any other
> solution makes it harder to keep the code in sync with the packaging.
> Removing the debian directory would frustrate this.

You can simply rename it as "debian-upstream" or something. The other
way is to push the debian folder in a separate packaging branch.

I by the way saw a lot of differences between the Debian packaging and
the upstream one, which is a major issue, and the main reason why having
a debian folder upstream is a bad idea: there's 2 source of "truth", and
that should be avoided.

> I'm not a fan of the "dfsg" suffix because it implies that
> DFSG-noncompliant material was removed.  There is nothing
> DFSG-noncompliant about the Debian directory, it is simply inconvenient
> for the way you prefer to maintain the packaging.

Which is why I wrote a debian/README.source explaining the facts. Is
that enough for you? Alternatively, I could use "debian1", but I don't
think a lot of people will even notice, and even less, understand the
difference.

> I see a number of failed builds here:
> https://buildd.debian.org/status/package.php?p=openvswitch=experimental
> 
> Let me analyze them:
> 
> * mips, powerpc, and ppc64 should be fixed by this commit that is
>   already on branch-2.8:
>   https://github.com/openvswitch/ovs/commit/2906ff5e7eb1fb39b497dc05e471

I can incorporate this patch, no pb.

> * m68k is because of looser alignment rules than on other platforms.  I
>   don't care much about m68k, and it's not a Debian required platform,
>   so I don't plan to fix this.

Right, we shall not care.

> * sparc64 failures are due to bus errors.  I would like to investigate,
>   but I don't know how, because there is only one sparc64 machine listed
>   at https://db.debian.org/machines.cgi, and that machine appears to be
>   down (it is not accepting SSH connections at least when I tried just
>   now).

Sparc64 isn't an official port either. See here:
https://www.debian.org/ports/

I don't think we should care.

> * The ppc64el failure is a hang during the testsuite.  Test 2332, which
>   appears to be "ovn -- icmp_reply: 1 HVs, 2 LSs, 1 lport/LS, 1 LR",
>   hung.  I will try to reproduce and fix this.  Even if we do not fix
>   it, it might not recur in later runs, because it indicates a race
>   condition in the testsuite.  (This is almost certainly a bug in the
>   testsuite rather than in OVS itself.)

In such a case, I'd strongly suggest removing this unit test until
further investigation. Is that ok for you too?

> It has been my practice to package the tip of whatever release branch
> we're using, for example in this case to package from the tip of
> branch-2.8.  OVS release branches only accept bug fixes, so this works
> well for getting bug fixes that have not yet made it into an "official"
> release.  In this case, this would have picked up the fix that caused
> three of the builds to fail while running the testsuite.

Well, in such a case, I'd suggest upstream to release more bugfix
releases then! :)

> Thanks again,

My pleasure. I hope the other changes I made are ok with you. Did you
read the debian/changelog? I removed lots of binary packages that were
not technically needed, because that's best practice in Debian (ie: the
FTP masters recommend this). All of them have a Provides: equivalent.
This also simplifies packaging a lot (ie: all /usr/bin things and
manpages are going into openvswitch-common, etc.).

BTW, I always wanted to know: is there a way to do VXLan with OpenStack
and the OpenVSwitch plugin?

Cheers,

Thomas Goirand (zigo)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] recent OVS upload (was: Re: Bug#878249: Please package >= 2.7.0)

2017-10-26 Thread Ben Pfaff

On Fri, Oct 20, 2017 at 08:56:24PM +0200, Thomas Goirand wrote:
> I don't mind becoming a co-maintainer, though there's a few things that
> need to change to facilitate packaging. First of all, I'd like to switch
> to a Git packaging workflow, using git-buildpackage. The debian folder
> upstream here should be removed:
> 
> https://github.com/openvswitch/ovs/tree/master/debian
> 
> otherwise, it will clash with what we have in Alioth. Then I'll push
> openvswitch in the Alioth git repository under /git/third-party.
> 
> Also, I will remove the debian/automake.mk and the d/copyright hack.
> Indeed, Debian needs list of copyright holders, not authors.
> 
> Is that fine for you? I'm starting...

Thanks for doing an upload.

Open vSwitch users expect to be able to build packages for whatever
version of Open vSwitch they're using, which is most easily satisfied by
keeping the Debian packaging with the rest of the tree.  Any other
solution makes it harder to keep the code in sync with the packaging.
Removing the debian directory would frustrate this.

I'm not a fan of the "dfsg" suffix because it implies that
DFSG-noncompliant material was removed.  There is nothing
DFSG-noncompliant about the Debian directory, it is simply inconvenient
for the way you prefer to maintain the packaging.  Would you mind
choosing a different suffix?

I see a number of failed builds here:
https://buildd.debian.org/status/package.php?p=openvswitch=experimental

Let me analyze them:

* mips, powerpc, and ppc64 should be fixed by this commit that is
  already on branch-2.8:
  https://github.com/openvswitch/ovs/commit/2906ff5e7eb1fb39b497dc05e471

* m68k is because of looser alignment rules than on other platforms.  I
  don't care much about m68k, and it's not a Debian required platform,
  so I don't plan to fix this.

* sparc64 failures are due to bus errors.  I would like to investigate,
  but I don't know how, because there is only one sparc64 machine listed
  at https://db.debian.org/machines.cgi, and that machine appears to be
  down (it is not accepting SSH connections at least when I tried just
  now).

* The ppc64el failure is a hang during the testsuite.  Test 2332, which
  appears to be "ovn -- icmp_reply: 1 HVs, 2 LSs, 1 lport/LS, 1 LR",
  hung.  I will try to reproduce and fix this.  Even if we do not fix
  it, it might not recur in later runs, because it indicates a race
  condition in the testsuite.  (This is almost certainly a bug in the
  testsuite rather than in OVS itself.)

It has been my practice to package the tip of whatever release branch
we're using, for example in this case to package from the tip of
branch-2.8.  OVS release branches only accept bug fixes, so this works
well for getting bug fixes that have not yet made it into an "official"
release.  In this case, this would have picked up the fix that caused
three of the builds to fail while running the testsuite.

Thanks again,

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

[ovs-dev] Finanzas para no Financieros

2017-10-26 Thread Contabilidad y Finanzas - 12 Temas

Buenas Tardes:

Este mes de Octubre le invitamos a adquirir una de nuestras pólizas de 
Capacitación online, los cuáles constan de 12 Temas que son utilizables durante 
3 meses, las 24 hrs del día, las veces que usted así lo requiera.

En Específico le ofrecemos el plan de Contabilidad y Finanzas que consta de 12 
temas.

1. Principios Esenciales De La Administración De Empresas. 
2. Guía Práctica De Finanzas Para No Financieros: Lenguaje 100% Empresarial. 
3. Cómo Organizar Y Controlar Los Procedimientos De Contabilidad General.
4. Cómo Organizar Y Optimizar Costos/Presupuestos/Planeación De Utilidades 
(Puntos De Equilibrio).
5. Recomendaciones Críticas Para Organizar El Área Fiscal. 
6. Herramientas Y Estrategias En Las Funciones Del Contralor. 
7. Cómo Incrementar Tus Ventas, Administrando Tu Crédito Y Cuentas Por Cobrar.
8. Control Y Ahorro En Las Cuentas Por Pagar.
9. Administración Del Capital De Trabajo. 
10. Auditoría Y Control Interno Para La Prevención De Fraudes.
11. Las Funciones Del Gerente Financiero. 
12. Lo Que Todo Director General Debe Saber Sobre Finanzas Y La Administración 
De Su Compañía. 


Para que usted recibá la información completa de este plan, le solicitamos por 
favor enviarnos la palabra Contabilidad junto los siguientes datos:

Nombre:
Empresa:
Teléfono:

Y le haremos llegar el Temario Completo para que usted revise más a detalle.

centro telefónico: 018002129393


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

[ovs-dev] PAYMENT UPDATE.

2017-10-26 Thread Mr. William Wilson

United States Money Gram Office,
Address:300 7th St SW, Houston, Texas 20024, USA
Houston, Texas, United States
Phone: +1 916 581 2055

Dear Customer,

Sir,How are you doing together with your family? I hope everything is moving 
normally as you want.

Sir, it has being a while I heard from you. I email you severally regarding the 
transfer of your funds and none was replied till date. Can you kindly tell me 
what's going on?

I am asking you this so that I will know your arrangement over this your 
transfer "whether you will want these funds to be completely transferred to you 
as it was programmed or to cancel it and return it back to Federal Government 
Treasury".

I am waiting for your response and please do not hesitate to reply back as it 
is from your response that I will know whether you still interested on 
receiving those fund.

Remember: Your payment file will be cancel and divert to government purse if 
you don't need these funds. So make sure you reply back immediately you receive 
this email and tell me if you still need those funds to be transfer to you...


Use Below information to send the $220 for the securing of the required Form in 
order to proceed with your transfer as scheduled.

RECEIVER NAME:: MICHAEL HILAND
LOCATION:: ECKERT COLORADO USA
AMOUN $220

Please send the fee through Money Gram office and get back to us with transfer 
slip in other to proceed on your transfer. Feel free to call Postal code for 
more clarifications. Your rapid response is highly needed.

Here is the payment information we send on your name but due to what I been 
telling you concerning the paper, Our head Office Put it stop down till the 
paper is obtain and present to them. Remember, no one can pick it because it 
was press stop down by Our Head Office as I told you before.

Sender's Name:.Corass smith
Country:...Usa
Reference #:.92169792
Amount:...$2,500

Note: Once you send the money, the clearance document of your funds will be 
obtained as well as its Government approval documents being secured too. Then 
we will without wasting time remit your first payment and email you the info to 
go to any Money Gram Office and pick it up peacefully.

I await for your reply with the payment details of the $220 so that we will fix 
the error for you to pick up the first payment.

Yours Sincere,
Mr. William Wilson,
Managing Director (Foreign Dept)
Tell Phone: +1 916 581 2055
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] DNS support feature (was: Re: DNS support options)

2017-10-26 Thread Yifeng Sun

Thanks Mark for your reply.

There is one more thing. If we bring DNS into play, we may need a mechanism
to watch for changes of ip addresses that were already resolved and being
used.

Thanks,
Yifeng

On Thu, Oct 26, 2017 at 12:10 PM, Mark Michelson 
wrote:

> On Wed, Oct 25, 2017 at 4:16 PM Yifeng Sun  wrote:
>
>> I feel that unbound stands out in the available open source DNS resolver.
>>
>> Below is the summary for unbound:
>> * The actual resolving work is done by a background process or thread. A
>> background process or thread seems unavoidable. Linux's getaddrinfo_a
>> clones a thread similarly.
>>
> * It is ported on Linux, BSD, Windows, MacOS/X and Solaris/SPARC. This is
>> good because OVS runs on a large range of platforms.
>>
> * It complies to the standard, with optional DNSSEC support. Some of its
>> features may not be needed in our case.
>> * The unbound context is thread-safe. Its internal locks may bring some
>> overhead. But since the DNS resolving is not frequent in OVS, I suppose
>> this small overhead is not an issue.
>>
>> Unbound looks like a good option. Another option is to create a
>> background thread which processes DNS resolving requests from the main
>> thread and sends back the resulting events to the main thread. This method
>> is quite simple and straightforward.
>>
>> The above are what I got so far. Please give your thoughts, thanks a lot.
>>
>
> If portability to all of the systems you mentioned in your second bullet
> point is important, then you can rule out a couple of options:
> * getaddrinfo_a is a GNU extension and is only available with glibc
> * The resolver functions[1] are a BSD specification so they'd be available
> on most platforms, but not on Windows. I don't personally recommend these
> because of the need to manually parse the DNS responses you receive.
>
> That leaves two options:
> * Run a background thread uses getaddrinfo() to perform resolution.
> * Use a third-party library (like unbound).
>
> Of these two options, I feel like the third-party library is the better
> option. The only downside I can think of is the extra dependency for OVS.
> And as far as what third-party library to use, I was the one that suggested
> unbound in the first place, so obviously I'm fine with using it :)
>
> Mark
>
> [1] http://man7.org/linux/man-pages/man3/resolver.3.html
>
>
>>
>> Below is the link for original discussion:
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html
>>
>>
>>
>> On Wed, Oct 25, 2017 at 2:11 PM, Ben Pfaff  wrote:
>>
>>> Hello everyone, please allow me to introduce Yifeng Sun (CCed), who
>>> recently joined VMware's Open vSwitch team.  I've asked Yifeng to start
>>> out by working on DNS support for Open vSwitch.  Yifeng, can you tell us
>>> about what you've discovered so far, based on this thread from August
>>> that I'm reviving, and your tentative conclusions?
>>>
>>> Thanks,
>>>
>>> Ben.
>>>
>>
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Introduce Emeritus Committer status.

2017-10-26 Thread Justin Pettit

Acked-by: Justin Pettit 

--Justin


> On Oct 26, 2017, at 2:33 PM, Ben Pfaff  wrote:
> 
> From: Russell Bryant 
> 
> This patch introduces an Emeritus status for OVS committers. An
> Emeritus Committer is recognized as having made a significant impact
> to the project and having been a committer in the past.  It is
> intended as an option for those that do not currently have the time or
> interest to fulfill committer responsibilities based on their current
> responsibilities.  While in this status, they are not included in
> voting for governance purposes.
> 
> An emeritus committer may be re-instated as a full committer at any
> time.
> 
> The OVS committers voted approval of this change.
> 
> See documentation contents for full details.
> 
> Suggested-by: Ethan J. Jackson 
> Signed-off-by: Russell Bryant 
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2:
>  - Deleted the previous requirements for inactivity revocation,
>which are now supplanted by emeritus status.
>  - Wordsmithing.
>  - Obtained committer approval via voting.
> 
> Documentation/automake.mk  |  1 +
> Documentation/index.rst|  3 +-
> .../internals/committer-emeritus-status.rst| 63 ++
> .../internals/committer-grant-revocation.rst   | 63 ++
> Documentation/internals/index.rst  |  1 +
> MAINTAINERS.rst| 14 -
> 6 files changed, 84 insertions(+), 61 deletions(-)
> create mode 100644 Documentation/internals/committer-emeritus-status.rst
> 
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 630fdf197b7c..3be185414928 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -81,6 +81,7 @@ DOC_SOURCE = \
>   Documentation/internals/index.rst \
>   Documentation/internals/authors.rst \
>   Documentation/internals/bugs.rst \
> + Documentation/internals/committer-emeritus-status.rst \
>   Documentation/internals/committer-grant-revocation.rst \
>   Documentation/internals/committer-responsibilities.rst \
>   Documentation/internals/documentation.rst \
> diff --git a/Documentation/index.rst b/Documentation/index.rst
> index 17b7b7a0e8a0..c737a6f6c238 100644
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how 
> you can contribute:
> 
> - **Maintaining:** :doc:`internals/maintainers` |
>   :doc:`internals/committer-responsibilities` |
> -  :doc:`internals/committer-grant-revocation`
> +  :doc:`internals/committer-grant-revocation` |
> +  :doc:`internals/committer-emeritus-status`
> 
> - **Documentation:** :doc:`internals/contributing/documentation-style` |
>   :doc:`Building Open vSwitch Documentation ` |
> diff --git a/Documentation/internals/committer-emeritus-status.rst 
> b/Documentation/internals/committer-emeritus-status.rst
> new file mode 100644
> index ..b2589ac6729e
> --- /dev/null
> +++ b/Documentation/internals/committer-emeritus-status.rst
> @@ -0,0 +1,63 @@
> +..
> +  Licensed under the Apache License, Version 2.0 (the "License"); you may
> +  not use this file except in compliance with the License. You may obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See 
> the
> +  License for the specific language governing permissions and limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +==
> +Emeritus Status for OVS Committers
> +==
> +
> +OVS committers are nominated and elected based on their impact on the Open
> +vSwitch project.  Over time, as committers' responsibilities change, some may
> +become unable or uninterested in actively participating in project 
> governance.
> +Committer "emeritus" status provides a way for committers to take a leave of
> +absence from OVS governance responsibilities.  The following guidelines 
> clarify
> +the process around the emeritus status for committers:
> +
> +* A committer may choose to transition from active to emeritus, or from
> +  emeritus to active, by sending an email to the committers mailing list.
> +
> +* If a committer hasn't been heard from in 6 months, and does not respond to
> +

Re: [ovs-dev] [PATCH] faq: Better document how to add vendor extensions.

2017-10-26 Thread Yi-Hung Wei

On Mon, Oct 9, 2017 at 10:33 AM, Ben Pfaff  wrote:
> Signed-off-by: Ben Pfaff 
> ---
>  Documentation/faq/contributing.rst | 44 
> +++---
>  include/openvswitch/ofp-errors.h   |  4 +++-
>  2 files changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/faq/contributing.rst 
> b/Documentation/faq/contributing.rst
> index 6dfc8bc4d436..d59376cd615c 100644
> --- a/Documentation/faq/contributing.rst
> +++ b/Documentation/faq/contributing.rst
> @@ -33,22 +33,28 @@ Q: How do I implement a new OpenFlow message?
>  as needed.  (If you configure with ``--enable-Werror``, as described in
>  :doc:`/intro/install/general`, then it is impossible to miss any 
> warnings.)
>
> -If you need to add an OpenFlow vendor extension message for a vendor that
> -doesn't yet have any extension messages, then you will also need to edit
> -``build-aux/extract-ofp-msgs``.
> +To add an OpenFlow vendor extension message (aka experimenter message) 
> for
> +a vendor that doesn't yet have any extension messages, you will also need
> +to edit ``build-aux/extract-ofp-msgs`` and at least ``ofphdrs_decode()``
> +and ``ofpraw_put__()`` in ``lib/ofp-msgs.c``.  OpenFlow doesn't 
> standardize
> +vendor extensions very well, so it's hard to make the process simpler 
> than
> +that.  (If you have a choice of how to design your vendor extension
> +messages, it will be easier if you make them resemble the ONF and OVS
> +extension messages.)
>
>  Q: How do I add support for a new field or header?
>
>  A: Add new members for your field to ``struct flow`` in ``lib/flow.h``, 
> and
>  add new enumerations for your new field to ``enum mf_field_id`` in
> -``lib/meta-flow.h``, following the existing pattern.  Also, add support 
> to
Instead of ``lib/meta-flow.h``, maybe
``include/openvswitch/meta-flow.h``? Otherwise, looks good to me.

Acked-by: Yi-Hung Wei 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Introduce Emeritus Committer status.

2017-10-26 Thread Ethan J. Jackson

Acked-by: Ethan J. Jackson 

On Thu, Oct 26, 2017 at 2:33 PM, Ben Pfaff  wrote:
> From: Russell Bryant 
>
> This patch introduces an Emeritus status for OVS committers. An
> Emeritus Committer is recognized as having made a significant impact
> to the project and having been a committer in the past.  It is
> intended as an option for those that do not currently have the time or
> interest to fulfill committer responsibilities based on their current
> responsibilities.  While in this status, they are not included in
> voting for governance purposes.
>
> An emeritus committer may be re-instated as a full committer at any
> time.
>
> The OVS committers voted approval of this change.
>
> See documentation contents for full details.
>
> Suggested-by: Ethan J. Jackson 
> Signed-off-by: Russell Bryant 
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2:
>   - Deleted the previous requirements for inactivity revocation,
> which are now supplanted by emeritus status.
>   - Wordsmithing.
>   - Obtained committer approval via voting.
>
>  Documentation/automake.mk  |  1 +
>  Documentation/index.rst|  3 +-
>  .../internals/committer-emeritus-status.rst| 63 
> ++
>  .../internals/committer-grant-revocation.rst   | 63 
> ++
>  Documentation/internals/index.rst  |  1 +
>  MAINTAINERS.rst| 14 -
>  6 files changed, 84 insertions(+), 61 deletions(-)
>  create mode 100644 Documentation/internals/committer-emeritus-status.rst
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 630fdf197b7c..3be185414928 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -81,6 +81,7 @@ DOC_SOURCE = \
> Documentation/internals/index.rst \
> Documentation/internals/authors.rst \
> Documentation/internals/bugs.rst \
> +   Documentation/internals/committer-emeritus-status.rst \
> Documentation/internals/committer-grant-revocation.rst \
> Documentation/internals/committer-responsibilities.rst \
> Documentation/internals/documentation.rst \
> diff --git a/Documentation/index.rst b/Documentation/index.rst
> index 17b7b7a0e8a0..c737a6f6c238 100644
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how 
> you can contribute:
>
>  - **Maintaining:** :doc:`internals/maintainers` |
>:doc:`internals/committer-responsibilities` |
> -  :doc:`internals/committer-grant-revocation`
> +  :doc:`internals/committer-grant-revocation` |
> +  :doc:`internals/committer-emeritus-status`
>
>  - **Documentation:** :doc:`internals/contributing/documentation-style` |
>:doc:`Building Open vSwitch Documentation ` |
> diff --git a/Documentation/internals/committer-emeritus-status.rst 
> b/Documentation/internals/committer-emeritus-status.rst
> new file mode 100644
> index ..b2589ac6729e
> --- /dev/null
> +++ b/Documentation/internals/committer-emeritus-status.rst
> @@ -0,0 +1,63 @@
> +..
> +  Licensed under the Apache License, Version 2.0 (the "License"); you may
> +  not use this file except in compliance with the License. You may obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See 
> the
> +  License for the specific language governing permissions and limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +==
> +Emeritus Status for OVS Committers
> +==
> +
> +OVS committers are nominated and elected based on their impact on the Open
> +vSwitch project.  Over time, as committers' responsibilities change, some may
> +become unable or uninterested in actively participating in project 
> governance.
> +Committer "emeritus" status provides a way for committers to take a leave of
> +absence from OVS governance responsibilities.  The following guidelines 
> clarify
> +the process around the emeritus status for committers:
> +
> +* A committer may choose to transition from active to emeritus, or from
> +  emeritus to active, by sending an email to the committers mailing list.
> +
> +* If a committer hasn't been heard from in 6 months, and does

[ovs-dev] [PATCH v2] Introduce Emeritus Committer status.

2017-10-26 Thread Ben Pfaff

From: Russell Bryant 

This patch introduces an Emeritus status for OVS committers. An
Emeritus Committer is recognized as having made a significant impact
to the project and having been a committer in the past.  It is
intended as an option for those that do not currently have the time or
interest to fulfill committer responsibilities based on their current
responsibilities.  While in this status, they are not included in
voting for governance purposes.

An emeritus committer may be re-instated as a full committer at any
time.

The OVS committers voted approval of this change.

See documentation contents for full details.

Suggested-by: Ethan J. Jackson 
Signed-off-by: Russell Bryant 
Signed-off-by: Ben Pfaff 
---
v1->v2:
  - Deleted the previous requirements for inactivity revocation,
which are now supplanted by emeritus status.
  - Wordsmithing.
  - Obtained committer approval via voting.

 Documentation/automake.mk  |  1 +
 Documentation/index.rst|  3 +-
 .../internals/committer-emeritus-status.rst| 63 ++
 .../internals/committer-grant-revocation.rst   | 63 ++
 Documentation/internals/index.rst  |  1 +
 MAINTAINERS.rst| 14 -
 6 files changed, 84 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/internals/committer-emeritus-status.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 630fdf197b7c..3be185414928 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -81,6 +81,7 @@ DOC_SOURCE = \
Documentation/internals/index.rst \
Documentation/internals/authors.rst \
Documentation/internals/bugs.rst \
+   Documentation/internals/committer-emeritus-status.rst \
Documentation/internals/committer-grant-revocation.rst \
Documentation/internals/committer-responsibilities.rst \
Documentation/internals/documentation.rst \
diff --git a/Documentation/index.rst b/Documentation/index.rst
index 17b7b7a0e8a0..c737a6f6c238 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how you 
can contribute:
 
 - **Maintaining:** :doc:`internals/maintainers` |
   :doc:`internals/committer-responsibilities` |
-  :doc:`internals/committer-grant-revocation`
+  :doc:`internals/committer-grant-revocation` |
+  :doc:`internals/committer-emeritus-status`
 
 - **Documentation:** :doc:`internals/contributing/documentation-style` |
   :doc:`Building Open vSwitch Documentation ` |
diff --git a/Documentation/internals/committer-emeritus-status.rst 
b/Documentation/internals/committer-emeritus-status.rst
new file mode 100644
index ..b2589ac6729e
--- /dev/null
+++ b/Documentation/internals/committer-emeritus-status.rst
@@ -0,0 +1,63 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+==
+Emeritus Status for OVS Committers
+==
+
+OVS committers are nominated and elected based on their impact on the Open
+vSwitch project.  Over time, as committers' responsibilities change, some may
+become unable or uninterested in actively participating in project governance.
+Committer "emeritus" status provides a way for committers to take a leave of
+absence from OVS governance responsibilities.  The following guidelines clarify
+the process around the emeritus status for committers:
+
+* A committer may choose to transition from active to emeritus, or from
+  emeritus to active, by sending an email to the committers mailing list.
+
+* If a committer hasn't been heard from in 6 months, and does not respond to
+  reasonable attempts to contact him or her, the other committers can vote as a
+  majority to transition the committer from active to emeritus.  (If the
+  committer resurfaces, he or she can transition back to active by sending an
+  email to the committers mailing list.)
+
+* Emeritus committers may stay on the

Re: [ovs-dev] [PATCH] ovsdb-idl: fix index row setting with references.

2017-10-26 Thread Ben Pfaff

On Sat, Oct 14, 2017 at 11:53:23PM -0700, Han Zhou wrote:
> IDL index should be able to be used without having to be in a
> transaction. However, current implementation leads to crash if
> a reference type column is being set in an index row for querying
> purpose when it is not in a transaction. It is because of the
> uninitialized arcs and unnecessary updates of the arcs. This patch
> fixes it by telling the column parsing function whether it is for
> index row or not, so that when parsing index row, the arcs are not
> updated. A new test case is added to cover this scenario.
> 
> Signed-off-by: Han Zhou 

Thank you for the bug fix.  I agree that this fix should go in.

I see at least one place where "update" is misspelled "udpate".

Did you consider whether there is a way to distinguish an index row from
other rows, without adding a new member?  It might be possible to simply
consider the UUID, since an index row has UUID zero and other rows do
not.  If this is possible, then I think I'd prefer that approach.

The following incremental fixes some style violations identified by
"checkpatch".

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

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 3636a0e09e0e..451172cdcc34 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2683,8 +2683,8 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context 
*ctx)
 
 struct ovsdb_idl_index *index;
 index = ovsdb_idl_create_index(idl, _table_simple3, "uref");
-ovsdb_idl_index_add_column(index, _simple3_col_uref, 
OVSDB_INDEX_ASC,
-   NULL);
+ovsdb_idl_index_add_column(index, _simple3_col_uref,
+   OVSDB_INDEX_ASC, NULL);
 
 ovsdb_idl_get_initial_snapshot(idl);
 
@@ -2706,7 +2706,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context 
*ctx)
 printf("%03d: After add to other table + set of strong ref\n", step++);
 dump_simple3(idl, myRow, step++);
 
-myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl);
+myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl);
 printf("%03d: check simple4: %s\n", step++,
myRow2 ? "not empty" : "empty");
 
@@ -2714,7 +2714,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context 
*ctx)
 
 struct idltest_simple3 *equal;
 equal = idltest_simple3_index_init_row(idl, _table_simple3);
-myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl);
+myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl);
 idltest_simple3_index_set_uref(equal, , 1);
 printf("%03d: Query using index with reference\n", step++);
 IDLTEST_SIMPLE3_FOR_EACH_EQUAL (myRow, , equal) {
@@ -2732,7 +2732,7 @@ do_idl_compound_index_with_ref(struct ovs_cmdl_context 
*ctx)
 printf("%03d: After delete\n", step++);
 dump_simple3(idl, myRow, step++);
 
-myRow2 = (struct idltest_simple4*)idltest_simple4_first(idl);
+myRow2 = (struct idltest_simple4 *) idltest_simple4_first(idl);
 printf("%03d: check simple4: %s\n", step++,
myRow2 ? "not empty" : "empty");
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] system-traffic: Fix conntrack tests

2017-10-26 Thread Yi-Hung Wei

Three conntrack system-traffic tests are broken because of a recent
change 7827edcaebd8 ("Add dl_type to flow metadata for correct
interpretation of conntrack metadata"). It can be reproduced by
  $ make check-system-userspace TESTSUITEFLAGS='18 19 37'

This patch modifies the check messages to fix the breakage.

Fixes: 7827edcaebd8 ("Add dl_type to flow metadata for correct interpretation 
of conntrack metadata")
CC: Daniel Alvarez 
Signed-off-by: Yi-Hung Wei 
---
 tests/system-traffic.at | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 522eaa615834..fd7b6121b04f 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -764,7 +764,7 @@ dnl Check this output. We only see the latter two packets, 
not the first.
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN2 (xid=0x0): total_len=42 in_port=1 (via action) data_len=42 
(unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=1,tp_dst=2
 udp_csum:0
-NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 
ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2
 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 
ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,ip,in_port=2
 (via action) data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=2,tp_dst=1
 udp_csum:0
 ])
 
@@ -810,7 +810,7 @@ dnl Check this output. We only see the latter two packets, 
not the first.
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN2 (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) 
data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=1,tp_dst=2
 udp_csum:0
-NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=42 
ct_state=new|trk,ct_nw_src=10.1.1.2,ct_nw_dst=10.1.1.1,ct_nw_proto=17,ct_tp_src=2,ct_tp_dst=1,in_port=2
 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=42 
ct_state=new|trk,ct_nw_src=10.1.1.2,ct_nw_dst=10.1.1.1,ct_nw_proto=17,ct_tp_src=2,ct_tp_dst=1,ip,in_port=2
 (via action) data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=2,tp_dst=1
 udp_csum:0
 ])
 
@@ -1589,11 +1589,11 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 
resubmit\(,0\) 'e64c473528c9c
 
 dnl Check this output. We only see the latter two packets, not the first.
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
-NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 
ct_state=inv|trk,in_port=2 (via action) data_len=75 (unbuffered)
+NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 
ct_state=inv|trk,ip,in_port=2 (via action) data_len=75 (unbuffered)
 
icmp,vlan_tci=0x,dl_src=c6:f5:4e:cb:72:db,dl_dst=f6:4c:47:35:28:c9,nw_src=172.16.0.4,nw_dst=172.16.0.3,nw_tos=192,nw_ecn=0,nw_ttl=64,icmp_type=3,icmp_code=3
 icmp_csum:da49
-NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=47 
ct_state=new|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,in_port=1
 (via action) data_len=47 (unbuffered)
+NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=47 
ct_state=new|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,ip,in_port=1
 (via action) data_len=47 (unbuffered)
 
udp,vlan_tci=0x,dl_src=e6:4c:47:35:28:c9,dl_dst=c6:f9:4e:cb:72:db,nw_src=172.16.0.1,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=41614,tp_dst=
 udp_csum:2096
-NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 
ct_state=rel|rpl|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,in_port=2
 (via action) data_len=75 (unbuffered)
+NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=75 
ct_state=rel|rpl|trk,ct_nw_src=172.16.0.1,ct_nw_dst=172.16.0.2,ct_nw_proto=17,ct_tp_src=41614,ct_tp_dst=,ip,in_port=2
 (via action) data_len=75 (unbuffered)
 
icmp,vlan_tci=0x,dl_src=c6:f9:4e:cb:72:db,dl_dst=e6:4c:47:35:28:c9,nw_src=172.16.0.2,nw_dst=172.16.0.1,nw_tos=192,nw_ecn=0,nw_ttl=64,icmp_type=3,icmp_code=3
 icmp_csum:553f
 ])
 
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart

2017-10-26 Thread Eric Garver

On Thu, Oct 26, 2017 at 12:31:03PM -0700, William Tu wrote:
> When using the out-of-tree (openvswitch compat) geneve module,
> the first time oot tunnel probing returns true (correct).
> Without unloading the geneve module, if the userspace ovs-vswitchd
> restarts, because the 'geneve_sys_6081' still exists, the probing
> incorrectly returns false and loads the in-tree (upstream kernel)
> geneve module.  The issue also exists the other way around, where
> existing geneve module is in-tree and ovs-vswitchd restarts.
> 
> The patch fixes it by unconditionally removing the tunnel device
> before the probing.  To reproduce the issue, start the ovs
> > /etc/init.d/openvswitch-switch start
> > creat a bridge and attach a geneve port using out-of-tree geneve
> > /etc/init.d/openvswitch-switch restart
> 
> Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides 
> used interface")
> Signed-off-by: William Tu 
> Cc: Eric Garver 
> Cc: Gurucharan Shetty 
> ---
>  lib/dpif-netlink-rtnl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 0c32e7d8ccb4..148ce5ff3a3d 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -448,6 +448,8 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
>  }
>  
>  name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
> +dpif_netlink_rtnl_destroy(name);
> +
>  error = dpif_netlink_rtnl_create(tnl_cfg, name, 
> OVS_VPORT_TYPE_GENEVE,
>   "ovs_geneve",
>   (NLM_F_REQUEST | NLM_F_ACK
> -- 
> 2.7.4

Thanks for the fix!

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

[ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart

2017-10-26 Thread William Tu

When using the out-of-tree (openvswitch compat) geneve module,
the first time oot tunnel probing returns true (correct).
Without unloading the geneve module, if the userspace ovs-vswitchd
restarts, because the 'geneve_sys_6081' still exists, the probing
incorrectly returns false and loads the in-tree (upstream kernel)
geneve module.  The issue also exists the other way around, where
existing geneve module is in-tree and ovs-vswitchd restarts.

The patch fixes it by unconditionally removing the tunnel device
before the probing.  To reproduce the issue, start the ovs
> /etc/init.d/openvswitch-switch start
> creat a bridge and attach a geneve port using out-of-tree geneve
> /etc/init.d/openvswitch-switch restart

Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides used 
interface")
Signed-off-by: William Tu 
Cc: Eric Garver 
Cc: Gurucharan Shetty 
---
 lib/dpif-netlink-rtnl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 0c32e7d8ccb4..148ce5ff3a3d 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -448,6 +448,8 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
 }
 
 name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
+dpif_netlink_rtnl_destroy(name);
+
 error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE,
  "ovs_geneve",
  (NLM_F_REQUEST | NLM_F_ACK
-- 
2.7.4

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

[ovs-dev] [PATCH v2 3/3] system-traffic: Add conntrack floating IP test

2017-10-26 Thread Eric Garver

This test cases uses floating IP (FIP) addresses for each endpoint. If
the destination is a FIP, the packet will undergo a transformation of
the form (dst=FIP, src=non-FIP) --> (dst=non-FIP, src=FIP) before
egress. Otherwise the packet is untouched.

This exercises the ct_clear action in the datapath.

Signed-off-by: Eric Garver 
---
 tests/system-traffic.at | 73 +
 1 file changed, 73 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 522eaa615834..cf915d6be7cd 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3996,6 +3996,79 @@ ovs-ofctl -O OpenFlow15 dump-group-stats br0
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - floating IP])
+AT_SKIP_IF([test $HAVE_NC = no])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+OVS_CHECK_CT_CLEAR()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") dnl FIP 
10.254.254.1
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") dnl FIP 
10.254.254.2
+
+dnl Static ARPs
+NS_CHECK_EXEC([at_ns0], [ip neigh add 10.1.1.2 lladdr f0:00:00:01:01:02 dev 
p0])
+NS_CHECK_EXEC([at_ns1], [ip neigh add 10.1.1.1 lladdr f0:00:00:01:01:01 dev 
p1])
+
+dnl Static ARP and route entries for the FIP "gateway"
+NS_CHECK_EXEC([at_ns0], [ip neigh add 10.1.1.254 lladdr f0:00:00:01:01:FE dev 
p0])
+NS_CHECK_EXEC([at_ns1], [ip neigh add 10.1.1.254 lladdr f0:00:00:01:01:FE dev 
p1])
+NS_CHECK_EXEC([at_ns0], [ip route add default nexthop via 10.1.1.254])
+NS_CHECK_EXEC([at_ns1], [ip route add default nexthop via 10.1.1.254])
+
+NETNS_DAEMONIZE([at_ns0], [nc -l -k 1234 > /dev/null], [nc0.pid])
+
+AT_DATA([flows.txt], [dnl
+table=0,priority=10  ip action=ct(table=1)
+table=0,priority=1   action=drop
+dnl dst FIP
+table=1,priority=20  ip,ct_state=+trk+est,nw_dst=10.254.254.0/24 
action=goto_table:10
+table=1,priority=20  ip,ct_state=+trk+new,nw_dst=10.254.254.0/24 
action=ct(commit,table=10)
+dnl dst local
+table=1,priority=10  ip,ct_state=+trk+est action=goto_table:20
+table=1,priority=10  ip,ct_state=+trk+new action=ct(commit,table=20)
+table=1,priority=1   ip,ct_state=+trk+inv action=drop
+dnl
+dnl FIP translation (dst FIP, src local) --> (dst local, src FIP)
+table=10 ip,nw_dst=10.254.254.1 
action=set_field:10.1.1.1->nw_dst,goto_table:11
+table=10 ip,nw_dst=10.254.254.2 
action=set_field:10.1.1.2->nw_dst,goto_table:11
+table=11 ip,nw_src=10.1.1.1 
action=set_field:10.254.254.1->nw_src,goto_table:12
+table=11 ip,nw_src=10.1.1.2 
action=set_field:10.254.254.2->nw_src,goto_table:12
+dnl clear conntrack and do another lookup since we changed the tuple
+table=12,priority=10 ip action=ct_clear,ct(table=13)
+table=12,priority=1  action=drop
+table=13 ip,ct_state=+trk+est action=goto_table:20
+table=13 ip,ct_state=+trk+new action=ct(commit,table=20)
+table=13 ip,ct_state=+trk+inv action=drop
+dnl
+dnl Output
+table=20 ip,nw_src=10.1.1.1 
action=set_field:f0:00:00:01:01:01->eth_src,goto_table:21
+table=20 ip,nw_src=10.1.1.2 
action=set_field:f0:00:00:01:01:02->eth_src,goto_table:21
+table=20 ip,nw_src=10.254.254.0/24 
action=set_field:f0:00:00:01:01:FE->eth_src,goto_table:21
+table=21 ip,nw_dst=10.1.1.1 
action=set_field:f0:00:00:01:01:01->eth_dst,output:ovs-p0
+table=21 ip,nw_dst=10.1.1.2 
action=set_field:f0:00:00:01:01:02->eth_dst,output:ovs-p1
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl non-FIP case
+NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.1.1.1 1234])
+OVS_WAIT_UNTIL([[ovs-appctl dpctl/dump-conntrack | sed -e 
's/port=[0-9]*/port=/g' -e 's/id=[0-9]*/id=/g' |
+grep 
"tcp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),reply=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),protoinfo=(state=TIME_WAIT)"
+]])
+
+dnl Check that the full session ends as expected (i.e. TIME_WAIT). Otherwise it
+dnl means the datapath didn't process the ct_clear action. Ending in SYN_RECV
+dnl (OVS maps to ESTABLISHED) means the initial frame was committed, but not a
+dnl second time after the FIP translation (because ct_clear didn't occur).
+NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1 1234])
+OVS_WAIT_UNTIL([[ovs-appctl dpctl/dump-conntrack | sed -e 
's/port=[0-9]*/port=/g' -e 's/id=[0-9]*/id=/g' |
+grep 
"tcp,orig=(src=10.254.254.2,dst=10.1.1.1,sport=,dport=),reply=(src=10.1.1.1,dst=10.254.254.2,sport=,dport=),protoinfo=(state=TIME_WAIT)"
+]])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_BANNER([802.1ad])
 
 AT_SETUP([802.1ad - vlan_limit])
-- 
2.12.0

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

[ovs-dev] [PATCH v2 2/3] system-common-macros: Check for ct_clear action in datapath

2017-10-26 Thread Eric Garver

New macro OVS_CHECK_CT_CLEAR() to check if ct_clear action is supported
by the datapath.

Signed-off-by: Eric Garver 
Tested-by: William Tu 
---
 tests/system-common-macros.at | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 73ae4829dac4..f7d4adb947a0 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -319,3 +319,7 @@ m4_define([OVS_CHECK_8021AD],
 # OVS_CHECK_IPROUTE_ENCAP()
 m4_define([OVS_CHECK_IPROUTE_ENCAP],
 [AT_SKIP_IF([! ip route help 2>&1 |grep encap >/dev/null])])
+
+# OVS_CHECK_CT_CLEAR()
+m4_define([OVS_CHECK_CT_CLEAR],
+[AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
ovs-vswitchd.log])])
-- 
2.12.0

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

[ovs-dev] [PATCH v2 1/3] dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR

2017-10-26 Thread Eric Garver

This supports using the ct_clear action in the kernel datapath. To
preserve compatibility with current ct_clear behavior on old kernels, we
only pass this action down to the datapath if a probe reveals the
datapath actually supports it.

Signed-off-by: Eric Garver 
Acked-by: William Tu 
---
 datapath/linux/compat/include/linux/openvswitch.h |  1 +
 lib/conntrack.c   | 10 +++
 lib/conntrack.h   |  1 +
 lib/dpif-netdev.c |  1 +
 lib/dpif.c|  1 +
 lib/odp-execute.c |  7 +
 lib/odp-util.c|  4 +++
 lib/ofp-actions.c |  1 +
 ofproto/ofproto-dpif-ipfix.c  |  1 +
 ofproto/ofproto-dpif-sflow.c  |  1 +
 ofproto/ofproto-dpif-xlate.c  | 14 +-
 ofproto/ofproto-dpif.c| 32 +++
 ofproto/ofproto-dpif.h|  5 +++-
 13 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index bc6c94b8d52d..28f20103af81 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -924,6 +924,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_TRUNC,/* u32 struct ovs_action_trunc. */
OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
OVS_ACTION_ATTR_POP_ETH,  /* No argument. */
+   OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
 
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
diff --git a/lib/conntrack.c b/lib/conntrack.c
index e555b5501da9..ddd6de4daff8 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1242,6 +1242,16 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
 return 0;
 }
 
+int
+conntrack_clear(struct dp_packet *packet)
+{
+/* According to pkt_metadata_init(), ct_state == 0 is enough to make all of
+ * the conntrack fields invalid. */
+packet->md.ct_state = 0;
+
+return 0;
+}
+
 static void
 set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val, uint32_t mask)
 {
diff --git a/lib/conntrack.h b/lib/conntrack.h
index fbeef1c4754e..6c19f3c65804 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -97,6 +97,7 @@ int conntrack_execute(struct conntrack *, struct 
dp_packet_batch *,
   const char *helper,
   const struct nat_action_info_t *nat_action_info,
   long long now);
+int conntrack_clear(struct dp_packet *packet);
 
 struct conntrack_dump {
 struct conntrack *ct;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d5eb8305c8a2..a3046b259c2e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5640,6 +5640,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_ACTION_ATTR_CLONE:
 case OVS_ACTION_ATTR_ENCAP_NSH:
 case OVS_ACTION_ATTR_DECAP_NSH:
+case OVS_ACTION_ATTR_CT_CLEAR:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
 }
diff --git a/lib/dpif.c b/lib/dpif.c
index 79b2e6c97305..febeb816e4c4 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1273,6 +1273,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_ACTION_ATTR_CLONE:
 case OVS_ACTION_ATTR_ENCAP_NSH:
 case OVS_ACTION_ATTR_DECAP_NSH:
+case OVS_ACTION_ATTR_CT_CLEAR:
 case OVS_ACTION_ATTR_UNSPEC:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5f4d23a91a3e..01ac62b25bca 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -34,6 +34,7 @@
 #include "unaligned.h"
 #include "util.h"
 #include "csum.h"
+#include "conntrack.h"
 
 /* Masked copy of an ethernet address. 'src' is already properly masked. */
 static void
@@ -654,6 +655,7 @@ requires_datapath_assistance(const struct nlattr *a)
 case OVS_ACTION_ATTR_CLONE:
 case OVS_ACTION_ATTR_ENCAP_NSH:
 case OVS_ACTION_ATTR_DECAP_NSH:
+case OVS_ACTION_ATTR_CT_CLEAR:
 return false;
 
 case OVS_ACTION_ATTR_UNSPEC:
@@ -837,6 +839,11 @@ odp_execute_actions(void *dp, struct dp_packet_batch 
*batch, bool steal,
 }
 break;
 }
+case OVS_ACTION_ATTR_CT_CLEAR:
+DP_PACKET_BATCH_FOR_EACH (packet, batch) {
+conntrack_clear(packet);
+}
+break;
 
 case OVS_ACTION_ATTR_OUTPUT:
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 6304b3dd299a..83b936d2a0fd 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -126,6 +126,7 @@ odp_action_len(uint16_t type)
 case OVS_ACTION_ATTR_SET_MASKED: return

[ovs-dev] [PATCH v2 0/3] Add dpif support for ct_clear action

2017-10-26 Thread Eric Garver

This series adds dpif support for OVS_ACTION_ATTR_CT_CLEAR. Previously the
ct_clear action was a userspace only operation, but it has use cases in the
dpif as well. Namely changing a packet's tuple after a ct() lookup has
occurred.

Kernel support has already be accepted upstream:
  b8226962b1c4 ("openvswitch: add ct_clear action").

travis-ci: https://travis-ci.org/erig0/ovs/builds/290487503

v2:
- Specify port for nc in test (patch 3)

Eric Garver (3):
  dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR
  system-common-macros: Check for ct_clear action in datapath
  system-traffic: Add conntrack floating IP test

 datapath/linux/compat/include/linux/openvswitch.h |  1 +
 lib/conntrack.c   | 10 
 lib/conntrack.h   |  1 +
 lib/dpif-netdev.c |  1 +
 lib/dpif.c|  1 +
 lib/odp-execute.c |  7 +++
 lib/odp-util.c|  4 ++
 lib/ofp-actions.c |  1 +
 ofproto/ofproto-dpif-ipfix.c  |  1 +
 ofproto/ofproto-dpif-sflow.c  |  1 +
 ofproto/ofproto-dpif-xlate.c  | 14 -
 ofproto/ofproto-dpif.c| 32 ++
 ofproto/ofproto-dpif.h|  5 +-
 tests/system-common-macros.at |  4 ++
 tests/system-traffic.at   | 73 +++
 15 files changed, 154 insertions(+), 2 deletions(-)

-- 
2.12.0

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

Re: [ovs-dev] DNS support feature (was: Re: DNS support options)

2017-10-26 Thread Mark Michelson

On Wed, Oct 25, 2017 at 4:16 PM Yifeng Sun  wrote:

> I feel that unbound stands out in the available open source DNS resolver.
>
> Below is the summary for unbound:
> * The actual resolving work is done by a background process or thread. A
> background process or thread seems unavoidable. Linux's getaddrinfo_a
> clones a thread similarly.
>
* It is ported on Linux, BSD, Windows, MacOS/X and Solaris/SPARC. This is
> good because OVS runs on a large range of platforms.
>
* It complies to the standard, with optional DNSSEC support. Some of its
> features may not be needed in our case.
> * The unbound context is thread-safe. Its internal locks may bring some
> overhead. But since the DNS resolving is not frequent in OVS, I suppose
> this small overhead is not an issue.
>
> Unbound looks like a good option. Another option is to create a background
> thread which processes DNS resolving requests from the main thread and
> sends back the resulting events to the main thread. This method is quite
> simple and straightforward.
>
> The above are what I got so far. Please give your thoughts, thanks a lot.
>

If portability to all of the systems you mentioned in your second bullet
point is important, then you can rule out a couple of options:
* getaddrinfo_a is a GNU extension and is only available with glibc
* The resolver functions[1] are a BSD specification so they'd be available
on most platforms, but not on Windows. I don't personally recommend these
because of the need to manually parse the DNS responses you receive.

That leaves two options:
* Run a background thread uses getaddrinfo() to perform resolution.
* Use a third-party library (like unbound).

Of these two options, I feel like the third-party library is the better
option. The only downside I can think of is the extra dependency for OVS.
And as far as what third-party library to use, I was the one that suggested
unbound in the first place, so obviously I'm fine with using it :)

Mark

[1] http://man7.org/linux/man-pages/man3/resolver.3.html

>
> Below is the link for original discussion:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html
>
>
>
> On Wed, Oct 25, 2017 at 2:11 PM, Ben Pfaff  wrote:
>
>> Hello everyone, please allow me to introduce Yifeng Sun (CCed), who
>> recently joined VMware's Open vSwitch team.  I've asked Yifeng to start
>> out by working on DNS support for Open vSwitch.  Yifeng, can you tell us
>> about what you've discovered so far, based on this thread from August
>> that I'm reviving, and your tentative conclusions?
>>
>> Thanks,
>>
>> Ben.
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart

2017-10-26 Thread William Tu

On Thu, Oct 26, 2017 at 11:15 AM, Eric Garver  wrote:
> On Thu, Oct 26, 2017 at 09:12:14AM -0700, William Tu wrote:
>> On Wed, Oct 25, 2017 at 10:39 AM, Eric Garver  wrote:
>> > On Tue, Oct 24, 2017 at 02:43:31PM -0700, William Tu wrote:
>> >
>> > Hi William,
>> > Thanks for taking a look at this.
>> >
>> >> When using the out-of-tree (openvswitch compat) geneve module,
>> >> the first time oot tunnel probing returns true (correct).
>> >> Without unloading the geneve module, if the userspace ovs-vswitchd
>> >> restarts, because the 'geneve_sys_6081' still exists, the probing
>> >> incorrectly returns false and loads the in-tree (upstream kernel)
>> >> geneve module.
>> >
>> > The reason for this is because rtnl sees the existing device and tries
>> > to call ->changelink, but since the out-of-tree tunnel has no handler it
>> > returns EOPNOTSUPP.
>> >
>> >> The patch fixes it by adding NLM_F_EXCL flags, so if geneve already
>> >> exists, the dpif_netlink_rtnl_create return EEXIST and the oot tunnel
>> >> probing returns true.  To reproduce the issue, start the ovs
>> >
>> > While this fixes this scenario, it breaks another; an existing device
>> > in-tree/kernel geneve tunnel. We'll end up with the opposite occurring -
>> > it returns true when it should return false.
>> >
>> > So in addition to adding NLM_F_EXCL, this needs to also try to delete
>> > the existing device in the case of EEXISTS, then restart the probe.
>> >
>>
>> You're right, thanks!
>> How about we unconditionally delete the device before the probe?. I'm
>> simply adding one line without using NLM_F_EXCL:
>>
>> +   dpif_netlink_rtnl_destroy(name);
>>  error = dpif_netlink_rtnl_create(tnl_cfg, name, 
>> OVS_VPORT_TYPE_GENEVE,
>>   "ovs_geneve",
>>   (NLM_F_REQUEST | NLM_F_ACK
>>| NLM_F_CREATE));
>>
>> I'm testing using in-tree or out-of-tree and the result is consistent.
>>
>
> I think that will work.
thanks, I will send out v2.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Add dl_type to flow metadata for correct interpretation of conntrack metadata

2017-10-26 Thread Ben Pfaff

On Thu, Oct 26, 2017 at 02:52:22PM +0200, Daniel Alvarez wrote:
> When a packet is sent to the controller, dl_type is not stored in the
> 'ofputil_packet_in_private'. When the packet is resumed, the flow's
> dl_type is 0 and this leads to invalid value in ct_orig_tuple in the
> pkt_metadata.
> 
> This patch adds the dl_type to the metadata so that conntrack
> information can be interpreted correctly when packets are resumed.
> 
> This is a change from the ordinary practice, since flow_get_metadata() is
> only supposed to deal with metadata and dl_type is not metadata.  It is
> necessary when ct_state is involved, though, because ct_state only applies
> in the case of particular Ethertypes (IPv4 and IPv6 currently), so we need
> to add it as a kind of prerequisite.  (This isn't ideal; maybe we didn't
> think through the ct_state mechanism carefully enough.)
> 
> Reported-by: Daniel Alvarez Sanchez 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339868.html
> Signed-off-by: Daniel Alvarez 
> Signed-off-by: Numan Siddique 

Thanks a lot.  I applied this to master and branch-2.8.  Let me know if
it should also go to older branches.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().

2017-10-26 Thread Ben Pfaff

On Thu, Oct 26, 2017 at 01:57:59PM +0300, Ilya Maximets wrote:
> On 26.10.2017 10:12, Ilya Maximets wrote:
> > On 25.10.2017 20:28, Ben Pfaff wrote:
> >> On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote:
>  This fanction will provide monotonic time in microseconds.
> >>>
> >>> [BHANU] Typo here with function.
> >>>
> 
>  Signed-off-by: Ilya Maximets 
>  ---
>  lib/timeval.c | 22 ++  lib/timeval.h |  2 ++
>  2 files changed, 24 insertions(+)
> 
>  diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644
>  --- a/lib/timeval.c
>  +++ b/lib/timeval.c
>  @@ -233,6 +233,22 @@ time_wall_msec(void)
>  return time_msec__(_clock);
>  }
> 
>  +static long long int
>  +time_usec__(struct clock *c)
>  +{
>  +struct timespec ts;
>  +
>  +time_timespec__(c, );
>  +return timespec_to_usec();
>  +}
>  +
>  +/* Returns a monotonic timer, in microseconds. */ long long int
>  +time_usec(void)
>  +{
>  +return time_usec__(_clock); }
>  +
> >>>
> >>> [BHANU]  As you are introducing the support for microsecond granularity, 
> >>> can you also add time_wall_usec() and time_wall_usec__() here?
> > 
> > I'm not sure what you meant under 'time_wall_usec__()'. There is no such 
> > function for msec.
> > 
> >>> The ipfix code (ipfix_now()) can be the first one to use it for now. May 
> >>> be more in the future! 
> >>>
>  /* Configures the program to die with SIGALRM 'secs' seconds from now, if
>   * 'secs' is nonzero, or disables the feature if 'secs' is zero. */  
>  void @@ -360,6
>  +376,12 @@ timeval_to_msec(const struct timeval *tv)
>  return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000;  }
> 
>  +long long int
>  +timespec_to_usec(const struct timespec *ts) {
>  +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec /
>  +1000; }
>  +
> >>>
> >>> [BHANU] how about adding timeval_to_usec()?
> >>> Also it would be nice to have the usec_to_timespec() and  
> >>> timeval_diff_usec() implemented to make this commit complete.
> >>
> >> I'd appreciate those changes too; with those changes, I'd be happy to
> >> apply this, independent of anything else in the series.
> > 
> > 
> > OK. Cool.
> > I'm going to implement additionally:
> > 
> > time_wall_usec()
> > usec_to_timespec()
> > timeval_to_usec()
> > timeval_diff_usec()
> > 
> > Please, correct me if I missed something.
> 
> I figured out that usec_to_timespec() and timeval_diff_usec() are static and
> will produce 'defined but not used' warning. And, actually, they are used only
> for poll logging and time warping, so those are not important to have usec
> granularity.
> I will add only two functions:
> 
> time_wall_usec()
> timeval_to_usec()
> 
> because they are part of the public timeval interface.

OK.  It's only the public interface I care about.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart

2017-10-26 Thread William Tu

On Wed, Oct 25, 2017 at 10:39 AM, Eric Garver  wrote:
> On Tue, Oct 24, 2017 at 02:43:31PM -0700, William Tu wrote:
>
> Hi William,
> Thanks for taking a look at this.
>
>> When using the out-of-tree (openvswitch compat) geneve module,
>> the first time oot tunnel probing returns true (correct).
>> Without unloading the geneve module, if the userspace ovs-vswitchd
>> restarts, because the 'geneve_sys_6081' still exists, the probing
>> incorrectly returns false and loads the in-tree (upstream kernel)
>> geneve module.
>
> The reason for this is because rtnl sees the existing device and tries
> to call ->changelink, but since the out-of-tree tunnel has no handler it
> returns EOPNOTSUPP.
>
>> The patch fixes it by adding NLM_F_EXCL flags, so if geneve already
>> exists, the dpif_netlink_rtnl_create return EEXIST and the oot tunnel
>> probing returns true.  To reproduce the issue, start the ovs
>
> While this fixes this scenario, it breaks another; an existing device
> in-tree/kernel geneve tunnel. We'll end up with the opposite occurring -
> it returns true when it should return false.
>
> So in addition to adding NLM_F_EXCL, this needs to also try to delete
> the existing device in the case of EEXISTS, then restart the probe.
>

You're right, thanks!
How about we unconditionally delete the device before the probe?. I'm
simply adding one line without using NLM_F_EXCL:

+   dpif_netlink_rtnl_destroy(name);
 error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE,
  "ovs_geneve",
  (NLM_F_REQUEST | NLM_F_ACK
   | NLM_F_CREATE));

I'm testing using in-tree or out-of-tree and the result is consistent.

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

Re: [ovs-dev] [PATCH] ofproto-dpif: Delete system tunnel interface when remove ovs bridge

2017-10-26 Thread Eric Garver

On Wed, Oct 25, 2017 at 11:41:27AM +0800, ju...@redhat.com wrote:
> When there is only one bridge,create tunnel in the bridge,
> then delete the bridge directly. the system tunnel interface
> still in.
> 
> Cause of only one bridge, backer->refcount values 1, when
> delete bridge, "close_dpif_backer" will delete the backer,
> so type_run will return directly, doesn't delete the interface.
> This patch delete the system interface before free the backer.

I'll add a bit more explanation..

This occurs when a tunnel is created with rtnetlink. With the compat API
the tunnel is created via the vport tunnel interface, so it can be
implicitly cleaned up by the kernel when the dp is closed or the module
unloaded.  But with rtnetlink the kernel module is not involved with the
tunnel device creation (it's added to OVS as a netdev vport), so
userspace needs to explicitly clean up the tunnel backers - type_run
can't garbage collect them if the dpif is already deleted.

> 
> Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides 
> used interface")
> Signed-off-by: JunhanYan 
> ---
>  ofproto/ofproto-dpif.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 43d670a..72993a4 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -646,6 +646,8 @@ dealloc(struct ofproto *ofproto_)
>  static void
>  close_dpif_backer(struct dpif_backer *backer, bool del)
>  {
> +struct simap_node *node;
> +
>  ovs_assert(backer->refcount > 0);
>  
>  if (--backer->refcount) {
> @@ -654,6 +656,9 @@ close_dpif_backer(struct dpif_backer *backer, bool del)
>  
>  udpif_destroy(backer->udpif);
>  
> +SIMAP_FOR_EACH (node, >tnl_backers) {
> +dpif_port_del(backer->dpif, u32_to_odp(node->data));
> +}
>  simap_destroy(>tnl_backers);
>  ovs_rwlock_destroy(>odp_to_ofport_lock);
>  hmap_destroy(>odp_to_ofport_map);
> -- 
> 2.9.5
> 

I didn't run the testsuite, but this looks okay to me.

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

Re: [ovs-dev] [PATCH 3/3] system-traffic: Add conntrack floating IP test

2017-10-26 Thread Eric Garver

On Thu, Oct 26, 2017 at 06:06:15AM -0700, William Tu wrote:
> Hi Eric,
> Thanks for the reply.
> 
> >> >>  dnl means the datapath didn't process the ct_clear action. Ending in 
> >> >> SYN_RECV
> >> >>  dnl (OVS maps to ESTABLISHED) means the initial frame was committed, 
> >> >> but not a
> >> >>  dnl second time after the FIP translation (because ct_clear didn't 
> >> >> occur).
> >> >> -NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1])
> >> >> +NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1 
> >> >> 1234])
> >> >
> >> > Are you saying it works for you with the explicit port?
> Yes, it works for me with explicit port (sorry I didn't answer this question)

Perfect.
I'll send a v2 with only this change.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] system-traffic: Add conntrack floating IP test

2017-10-26 Thread William Tu

Hi Eric,
Thanks for the reply.

>> >>  dnl means the datapath didn't process the ct_clear action. Ending in 
>> >> SYN_RECV
>> >>  dnl (OVS maps to ESTABLISHED) means the initial frame was committed, but 
>> >> not a
>> >>  dnl second time after the FIP translation (because ct_clear didn't 
>> >> occur).
>> >> -NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1])
>> >> +NS_CHECK_EXEC([at_ns1], [echo "foobar" |nc $NC_EOF_OPT 10.254.254.1 
>> >> 1234])
>> >
>> > Are you saying it works for you with the explicit port?
Yes, it works for me with explicit port (sorry I didn't answer this question)

>> > If not, can you show the error?
>> >
>> > FWIW, nmap-ncat defaults to 31337. I guess other nc implementations may
>> > be different.
>>
>> The error message:
>>
>>  83: conntrack - floating IP FAILED
>> (system-traffic.at:4055)
>> +This is nc from the netcat-openbsd package. An alternative nc is available
>> +in the netcat-traditional package.
>> +usage: nc [-46bCDdhjklnrStUuvZz] [-I length] [-i interval] [-O length]
>> + [-P proxy_username] [-p source_port] [-q seconds] [-s source]
>> + [-T toskeyword] [-V rtable] [-w timeout] [-X proxy_protocol]
>> + [-x proxy_address[:port]] [destination] [port]
>> ./system-traffic.at:4055: exit code was 1, expected 0
>>
>> on my system (ubuntu 1604), it is using the BSD
>> tests/atlocal.in:NC_EOF_OPT="-q 1 -w 5"
>
> There are other test cases that use NC_EOF_OPT, so I expect those are
> failing for you as well. Can you verify?
>
So my other test cases work OK since it has explicit port number.

> Indeed, OpenBSD nc(1) man page does not list the -q option. Nor do the
> other BSDs. I don't know why that option is being used. The first

Actually my BSD nc (netcat-traditional package) has -q options.

> evidence of it's usage seems to be 9ac0aadab9f9 ("conntrack: Add support
> for NAT.").

I think "-q 1" is mentioned at b54971f72ec ("system-traffic: use
appropriate nc options for installed version")

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

Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack

2017-10-26 Thread Daniel Alvarez Sanchez

Thanks Ben for the review.
I have fixed the tests (basically, now ip or ipv6 is added to the flows)
and submitted a v2 of the patch.

Daniel

On Wed, Oct 25, 2017 at 7:21 PM, Ben Pfaff  wrote:

> On Wed, Oct 25, 2017 at 06:50:29PM +0530, Numan Siddique wrote:
> > On Wed, Oct 25, 2017 at 3:09 PM, Daniel Alvarez Sanchez <
> dalva...@redhat.com
> > > wrote:
> >
> > >
> > >
> > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff  wrote:
> > >
> > >> On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote:
> > >> > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez
> wrote:
> > >> > > Hi guys,
> > >> > >
> > >> > > Great job Numan!
> > >> > > As we discussed over IRC, the patch below may make more sense.
> > >> > > It essentially sets the dl_type so that when packet comes from the
> > >> > > controller, it matches
> > >> > > a valid type and OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4 is not added.
> > >> > > Maybe what Numan proposed and this patch could be a good
> combination
> > >> but I
> > >> > > think
> > >> > > that we definitely need to set the dl_type as it's later checked
> in
> > >> > > pkt_metadata_from_flow()
> > >> > > and it'll be zero there otherwise.
> > >> > > What do you guys think? I have tried the patch below and the
> kernel
> > >> error
> > >> > > is not shown
> > >> > > anymore when issuing DHCP requests.
> > >> > >
> > >> > >
> > >> > > diff --git a/lib/flow.c b/lib/flow.c
> > >> > > index b2b10aa..62b948f 100644
> > >> > > --- a/lib/flow.c
> > >> > > +++ b/lib/flow.c
> > >> > > @@ -1073,6 +1073,7 @@ flow_get_metadata(const struct flow *flow,
> > >> struct
> > >> > > match *flow_metadata)
> > >> > >
> > >> > >  if (flow->ct_state != 0) {
> > >> > >  match_set_ct_state(flow_metadata, flow->ct_state);
> > >> > > +match_set_dl_type(flow_metadata, flow->dl_type);
> > >> > >  if (is_ct_valid(flow, NULL, NULL) && flow->ct_nw_proto
> != 0)
> > >> {
> > >> > >  if (flow->dl_type == htons(ETH_TYPE_IP)) {
> > >> > >  match_set_ct_nw_src(flow_metadata,
> flow->ct_nw_src);
> > >> >
> > >> > This patch seems reasonable too.
> > >> >
> > >> > I recommend adding a comment above it to explain what's going on,
> > >> > because dl_type is not a metadata field and it's confusing to deal
> with
> > >> > it in a context that's supposed to be all about metadata.  I guess
> the
> > >> > comment would essentially say that dl_type is essential to the
> > >> > interpretation of the conntrack metadata.
> > >>
> > >> Oh, and for this patch too I'd welcome a formal patch proposal.
> > >>
> > >
> > > Done. Thanks a lot Ben.
> > > If this get merged, it would be great if we can get it into 2.8 branch
> and
> > > add a new tag (2.8.2).
> > >
> > > Thanks!!
> > >
> >
> > Ben, we have submitted both the patches. The patch from Daniel - (
> > https://patchwork.ozlabs.org/patch/830160/)  will fix the issue.
> > Not sure if this patch  https://patchwork.ozlabs.org/patch/830132/ is
> > needed any more.
> >
> > Request to review these patches if possible as RDO is blocked on these
> > patches before we can  update from OVS 2.7.2 to OVS 2.8(.2)
>
> I've reviewed both.  I wasn't able to immediately apply either one, but
> they're obviously moving in the right direction, so I'd appreciate
> follow-up from both of you so that we can get them in.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] Add dl_type to flow metadata for correct interpretation of conntrack metadata

2017-10-26 Thread Daniel Alvarez

When a packet is sent to the controller, dl_type is not stored in the
'ofputil_packet_in_private'. When the packet is resumed, the flow's
dl_type is 0 and this leads to invalid value in ct_orig_tuple in the
pkt_metadata.

This patch adds the dl_type to the metadata so that conntrack
information can be interpreted correctly when packets are resumed.

This is a change from the ordinary practice, since flow_get_metadata() is
only supposed to deal with metadata and dl_type is not metadata.  It is
necessary when ct_state is involved, though, because ct_state only applies
in the case of particular Ethertypes (IPv4 and IPv6 currently), so we need
to add it as a kind of prerequisite.  (This isn't ideal; maybe we didn't
think through the ct_state mechanism carefully enough.)

Reported-by: Daniel Alvarez Sanchez 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339868.html
Signed-off-by: Daniel Alvarez 
Signed-off-by: Numan Siddique 
---
 lib/flow.c|  3 +++
 tests/ofproto-dpif.at | 30 +++---
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index b2b10aa48..4d2b7747a 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1073,6 +1073,9 @@ flow_get_metadata(const struct flow *flow, struct match 
*flow_metadata)
 
 if (flow->ct_state != 0) {
 match_set_ct_state(flow_metadata, flow->ct_state);
+/* Match dl_type since it is required for the later interpretation of
+ * the conntrack metadata. */
+match_set_dl_type(flow_metadata, flow->dl_type);
 if (is_ct_valid(flow, NULL, NULL) && flow->ct_nw_proto != 0) {
 if (flow->dl_type == htons(ETH_TYPE_IP)) {
 match_set_ct_nw_src(flow_metadata, flow->ct_nw_src);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 28a7e827c..c75a1acbf 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -8952,7 +8952,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) 
data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2
 udp_csum:e9d6
 dnl
-NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2
 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,ip,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2
 (via action) data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=1
 udp_csum:e9d6
 ])
 
@@ -8973,7 +8973,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) 
data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=2
 udp_csum:e9d4
 dnl
-NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2
 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,ip,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2
 (via action) data_len=42 (unbuffered)
 
udp,vlan_tci=0x,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=3
 udp_csum:e9d4
 ])
 
@@ -9105,7 +9105,7 @@ dnl happens because the ct_state field is available only 
after recirc.
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=62 in_port=1 (via action) 
data_len=62 (unbuffered)
 
udp6,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,ipv6_src=2001:db8::1,ipv6_dst=2001:db8::2,ipv6_label=0x0,nw_tos=112,nw_ecn=0,nw_ttl=128,tp_src=1,tp_dst=2
 udp_csum:a466
-NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=62

Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().

2017-10-26 Thread Ilya Maximets

On 26.10.2017 10:12, Ilya Maximets wrote:
> On 25.10.2017 20:28, Ben Pfaff wrote:
>> On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote:
 This fanction will provide monotonic time in microseconds.
>>>
>>> [BHANU] Typo here with function.
>>>

 Signed-off-by: Ilya Maximets 
 ---
 lib/timeval.c | 22 ++  lib/timeval.h |  2 ++
 2 files changed, 24 insertions(+)

 diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644
 --- a/lib/timeval.c
 +++ b/lib/timeval.c
 @@ -233,6 +233,22 @@ time_wall_msec(void)
 return time_msec__(_clock);
 }

 +static long long int
 +time_usec__(struct clock *c)
 +{
 +struct timespec ts;
 +
 +time_timespec__(c, );
 +return timespec_to_usec();
 +}
 +
 +/* Returns a monotonic timer, in microseconds. */ long long int
 +time_usec(void)
 +{
 +return time_usec__(_clock); }
 +
>>>
>>> [BHANU]  As you are introducing the support for microsecond granularity, 
>>> can you also add time_wall_usec() and time_wall_usec__() here?
> 
> I'm not sure what you meant under 'time_wall_usec__()'. There is no such 
> function for msec.
> 
>>> The ipfix code (ipfix_now()) can be the first one to use it for now. May be 
>>> more in the future! 
>>>
 /* Configures the program to die with SIGALRM 'secs' seconds from now, if
  * 'secs' is nonzero, or disables the feature if 'secs' is zero. */  void 
 @@ -360,6
 +376,12 @@ timeval_to_msec(const struct timeval *tv)
 return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000;  }

 +long long int
 +timespec_to_usec(const struct timespec *ts) {
 +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec /
 +1000; }
 +
>>>
>>> [BHANU] how about adding timeval_to_usec()?
>>> Also it would be nice to have the usec_to_timespec() and  
>>> timeval_diff_usec() implemented to make this commit complete.
>>
>> I'd appreciate those changes too; with those changes, I'd be happy to
>> apply this, independent of anything else in the series.
> 
> 
> OK. Cool.
> I'm going to implement additionally:
> 
> time_wall_usec()
> usec_to_timespec()
> timeval_to_usec()
> timeval_diff_usec()
> 
> Please, correct me if I missed something.

I figured out that usec_to_timespec() and timeval_diff_usec() are static and
will produce 'defined but not used' warning. And, actually, they are used only
for poll logging and time warping, so those are not important to have usec
granularity.
I will add only two functions:

time_wall_usec()
timeval_to_usec()

because they are part of the public timeval interface.

> Best regards, Ilya Maximets.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#878757: marked as done (Openvswitch must started before networking servise)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#878757: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #878757,
regarding Openvswitch must started before networking servise
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
878757: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=878757
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#878249: marked as done (Please package >= 2.7.0)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#878249: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #878249,
regarding Please package >= 2.7.0
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
878249: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=878249
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#877543: marked as done (CVE-2017-14970)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#877543: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #877543,
regarding CVE-2017-14970
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
877543: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=877543
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#863662: marked as done (openvswitch: CVE-2017-9265)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#863662: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #863662,
regarding openvswitch: CVE-2017-9265
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
863662: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863662
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#863661: marked as done (openvswitch: CVE-2017-9264)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#863661: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #863661,
regarding openvswitch: CVE-2017-9264
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
863661: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863661
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#863655: marked as done (openvswitch: CVE-2017-9263)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#863655: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #863655,
regarding openvswitch: CVE-2017-9263
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
863655: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863655
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#863228: marked as done (openvswtich: CVE-2017-9214)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#863228: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #863228,
regarding openvswtich: CVE-2017-9214
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
863228: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863228
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Bug#771507: marked as done (openvswitch-switch: missing systemd files)

2017-10-26 Thread Debian Bug Tracking System

Your message dated Thu, 26 Oct 2017 09:00:34 +
with message-id 
and subject line Bug#771507: fixed in openvswitch 2.8.1+dfsg1-1
has caused the Debian Bug report #771507,
regarding openvswitch-switch: missing systemd files
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
771507: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=771507
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 5/7] timeval: Introduce time_usec().

2017-10-26 Thread Ilya Maximets

On 25.10.2017 20:28, Ben Pfaff wrote:
> On Fri, Oct 13, 2017 at 01:03:18PM +, Bodireddy, Bhanuprakash wrote:
>>> This fanction will provide monotonic time in microseconds.
>>
>> [BHANU] Typo here with function.
>>
>>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>> lib/timeval.c | 22 ++  lib/timeval.h |  2 ++
>>> 2 files changed, 24 insertions(+)
>>>
>>> diff --git a/lib/timeval.c b/lib/timeval.c index dd63f03..be2eddc 100644
>>> --- a/lib/timeval.c
>>> +++ b/lib/timeval.c
>>> @@ -233,6 +233,22 @@ time_wall_msec(void)
>>> return time_msec__(_clock);
>>> }
>>>
>>> +static long long int
>>> +time_usec__(struct clock *c)
>>> +{
>>> +struct timespec ts;
>>> +
>>> +time_timespec__(c, );
>>> +return timespec_to_usec();
>>> +}
>>> +
>>> +/* Returns a monotonic timer, in microseconds. */ long long int
>>> +time_usec(void)
>>> +{
>>> +return time_usec__(_clock); }
>>> +
>>
>> [BHANU]  As you are introducing the support for microsecond granularity, can 
>> you also add time_wall_usec() and time_wall_usec__() here?

I'm not sure what you meant under 'time_wall_usec__()'. There is no such 
function for msec.

>> The ipfix code (ipfix_now()) can be the first one to use it for now. May be 
>> more in the future! 
>>
>>> /* Configures the program to die with SIGALRM 'secs' seconds from now, if
>>>  * 'secs' is nonzero, or disables the feature if 'secs' is zero. */  void 
>>> @@ -360,6
>>> +376,12 @@ timeval_to_msec(const struct timeval *tv)
>>> return (long long int) tv->tv_sec * 1000 + tv->tv_usec / 1000;  }
>>>
>>> +long long int
>>> +timespec_to_usec(const struct timespec *ts) {
>>> +return (long long int) ts->tv_sec * 1000 * 1000 + ts->tv_nsec /
>>> +1000; }
>>> +
>>
>> [BHANU] how about adding timeval_to_usec()?
>> Also it would be nice to have the usec_to_timespec() and  
>> timeval_diff_usec() implemented to make this commit complete.
> 
> I'd appreciate those changes too; with those changes, I'd be happy to
> apply this, independent of anything else in the series.


OK. Cool.
I'm going to implement additionally:

time_wall_usec()
usec_to_timespec()
timeval_to_usec()
timeval_diff_usec()

Please, correct me if I missed something.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 08/10] vswitch.xml: Detail vxlanipsec user interface.

2017-10-26 Thread Stokes, Ian

> Hi Ian,
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Ian Stokes
> > Sent: Friday, August 25, 2017 5:41 PM
> > To: d...@openvswitch.org
> > Subject: [ovs-dev] [RFC PATCH v2 08/10] vswitch.xml: Detail vxlanipsec
> > user interface.
> >
> > This commit adds details to the vswitch xml regarding the use of the
> > vxlanipsec interface type. This patch is not intended for upstreaming
> > and simply seeks to solicit feedback on the user interface design of
> > the vxlanipsec port type as described in the vswitch.xml.
> >
> > This modifies the vswitch.xml with a proposed vxlanipsec interface.
> > It also provides details for the proposed interface options such as
> > SPD creation, SA creation and modification, Policy entries for the SPD
> > as well as traffic selector options for the policy.
> >
> > Signed-off-by: Ian Stokes 
> > ---
> >  vswitchd/vswitch.xml |  225
> > ++
> >  1 files changed, 225 insertions(+), 0 deletions(-)
> >
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index
> > 074535b..27c3c54 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -2227,6 +2227,13 @@
> >  A pair of virtual devices that act as a patch cable.
> >
> >
> > +  vxlanipsec
> > +  
> > +An interface type to provide IPsec RFC4301 functionality
> for
> > +traffic at the IP layer with vxlan, initially for IPv4
> > +environments.
> > +  
> > +
> >null
> >An ignored interface. Deprecated and slated for removal
> in
> >February 2013.
> > @@ -2644,6 +2651,224 @@
> >
> >  
> >
> > +
> > +  
> > +Only vxlanipsec interfaces support these options.
> > +  
> > +
> > +  
> > +  
> > +Must be an identifier for the SPD that is to be used by this
> IPsec
> > +interface. If no SPD exists with this ID then it will be
> created.
> > +  
> > +  
> [[BO'M]] Do we need different security policy databases for different
> Interfaces? One per vSwitch would be enough to start with.

My thinking was that an SPD per tunnel is easier in terms of performance and 
look up for policies. If associated on a per tunnel basis there should only be 
1 inbound and 1 outbound policy per tunnel (as the selectors will be based on 
the src/dst IP and dest port), when you start to scale the number of tunnels 
you could end up with a lot of entries in the spd if using just 1 global spd 
which I'm thinking is going to slow down performance when selecting he correct 
policy for a given tunnel.

Having an spd per tunnel I don't think it's too much overhead and from a 
security point of view it could make sense that if an SPD is compromised it 
would only compromise that tunnel, not all tunnels. Thoughts?

> > +
> > +  
> > +  
> > +An identifier representing the ID of a Security Association.
> > +If no SA with this ID exists it will be created.
> > +  
> > +  
> > +
> > +  
> > +  
> > +A 32 bit number representing the security policy index for
> > +the SA.
> [[BO'M]] should this be 'security *parameters* index'? If so the specs
> (rfc4301 sec 4.4.2.1) say "a 32-bit value selected by the
>   receiving end of an SA to uniquely identify the SA"  and " An
> arbitrary 32-bit value that is used by a receiver to identify
>   the SA to which an incoming packet should be bound." so should not
> need to be configured? I think the receiver just assigns an arbitrary
> 32bit value.

I think this would have to be configured in the manual/static case. The SPI 
should represent an index that is set in the ESP header when a packet is being 
sent outbound, the idea being that when a received receives the packet they can 
use the SPI as an index to which SA is used to decrypt the packet. For this to 
work I think we would have to assign an SPI per SA manually so that they are 
synced on both sides of the tunnel.

> > +  
> > +  
> > +
> [[BO'M]] The remaining options ipsec_mode, sa_protocol, 
> ts_remote_port_range really define an SPD (security policy database) to
> look at the packet and decide if it needs to be DISCARD, BYPASS,
> PROTECTed. Would it be feasible to configure the selectors just as regular
> OF rules that send traffic to the vxlanipsec interface and then associate
> the keys, algorithms, mode (tunnel/transport) and protocol (AH/ESP) with
> the vxlanipsec interface?
> 

Possibly, I hadn't looked at this, do you have a fleshed out example of how 
this would work?

Because We're doing vxlan over ipsec in transport mode I guess you will only 
ever have 2 SAs for the vxlanipsec port, 1 for outbound, 1 for inbound, these 
would be called at the encap and decap stages respectfully, it makes me think 
would the SPD need to be

Re: [ovs-dev] [RFC PATCH v2 09/10] Docs: Add userspace-ipsec how to guide.

2017-10-26 Thread Stokes, Ian

> Hi Ian,
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Ian Stokes
> > Sent: Friday, August 25, 2017 5:41 PM
> > To: d...@openvswitch.org
> > Subject: [ovs-dev] [RFC PATCH v2 09/10] Docs: Add userspace-ipsec how
> > to guide.
> >
> > This commit adds a how to guide for using the proposed vxlanipsec
> > userspace interface. It is not intended to be upstreamed but simply
> > seeks to solicit feed back by providing an example of the proposed
> > vxlanipsec interface design setup and usage.
> >
> > The how to usecase deals with securing vxlan traffic between 2 VMs as
> > described in the userspace-vxlan how to guide. It provides an example
> > of how the proposed ipsec interface is configured with an SPD,
> > creation of SAs including IPsec protocol, mode, crypto/authentication
> > algorithms/keys, assigning SPD entires to SAs for inbound/outbound
> > traffic with traffic selectors and finally updating the SA keys.
> >
> > Signed-off-by: Ian Stokes 
> > ---
> >  Documentation/automake.mk   |1 +
> >  Documentation/howto/index.rst   |1 +
> >  Documentation/howto/userspace-ipsec.rst |  187
> > +++
> >  3 files changed, 189 insertions(+), 0 deletions(-)  create mode
> > 100644 Documentation/howto/userspace-ipsec.rst
> >
> > diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> > index
> > 24fe63d..a8f2a01 100644
> > --- a/Documentation/automake.mk
> > +++ b/Documentation/automake.mk
> > @@ -59,6 +59,7 @@ DOC_SOURCE = \
> > Documentation/howto/tunneling.png \
> > Documentation/howto/tunneling.rst \
> > Documentation/howto/userspace-tunneling.rst \
> > +   Documentation/howto/userspace-ipsec.rst \
> > Documentation/howto/vlan.png \
> > Documentation/howto/vlan.rst \
> > Documentation/howto/vtep.rst \
> > diff --git a/Documentation/howto/index.rst
> > b/Documentation/howto/index.rst index 5859a33..97d690a 100644
> > --- a/Documentation/howto/index.rst
> > +++ b/Documentation/howto/index.rst
> > @@ -43,6 +43,7 @@ OVS
> > lisp
> > tunneling
> > userspace-tunneling
> > +   userspace-ipsec
> > vlan
> > qos
> > vtep
> > diff --git a/Documentation/howto/userspace-ipsec.rst
> > b/Documentation/howto/userspace-ipsec.rst
> > new file mode 100644
> > index 000..2ae2bd8
> > --- /dev/null
> > +++ b/Documentation/howto/userspace-ipsec.rst
> > @@ -0,0 +1,187 @@
> > +..
> > +  Licensed under the Apache License, Version 2.0 (the "License");
> you may
> > +  not use this file except in compliance with the License. You may
> obtain
> > +  a copy of the License at
> > +
> > +  http://www.apache.org/licenses/LICENSE-2.0
> > +
> > +  Unless required by applicable law or agreed to in writing,
> software
> > +  distributed under the License is distributed on an "AS IS" BASIS,
> WITHOUT
> > +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> > + implied. See
> > the
> > +  License for the specific language governing permissions and
> limitations
> > +  under the License.
> > +
> > +  Convention for heading levels in Open vSwitch documentation:
> > +
> > +  ===  Heading 0 (reserved for the title in a document)
> > +  ---  Heading 1
> > +  ~~~  Heading 2
> > +  +++  Heading 3
> > +  '''  Heading 4
> > +
> > +  Avoid deeper levels because they do not render well.
> > +
> > +==
> > +Securing VXLAN traffic between VMs Using IPsec (Userspace)
> > +==
> > +
> > +This document describes how to use IPsec in Open vSwitch to secure
> > +traffic between VMs on two different hosts communicating over VXLAN
> > +tunnels. This solution works entirely in userspace.
> > +
> > +.. note::
> > +
> > +   This guide covers the steps required to configure an IPsec interface
> to
> > +   secure VXLAN tunneling traffic. It does not cover the steps to
> configure
> > +   the vxlan tunnels in userspace. For these configuration steps please
> refer
> > +   to :doc:`userspace-tunneling`.
> > +
> > +.. TODO(stephenfin): Convert this to a (prettier) PNG with same styling
> as the
> > +   rest of the document
> > +
> > +::
> > +
> > ++--+  +--+
> > +| vm0  | 192.168.1.1/24192.168.1.2/24 | vm1  |
> > ++--+  +--+
> > +   (vm_port0)(vm_port1)
> > +   | |
> > +   | |
> > +   | |
> > ++--+  +--+
> > +|br-int|

41 matches

Mail list logo