[ovs-dev] Повідомлення про переказ платежів

2019-07-15 Thread PrivatBank
Дорогий сер 

Ми отримали вказівку зробити цей переказ вам. 

Прикріплений швидкий примірник платежу. 

Best regards 

 Julian 

 [1] 

 PrivatBank 

 

 Jl. Boulevard Barat Raya Blok LC 6 No. 1-3
 Kelapa Gading Permai, Jakarta 14240
 (021) 252  

Links:
--
[1]
https://www.google.com/imgres?imgurl=https%3A%2F%2Fimg.112.international%2Foriginal%2F2017%2F08%2F21%2F266907.jpg&imgrefurl=https%3A%2F%2F112.international%2Ffinance%2Fukraines-cabinet-of-ministers-appeals-against-cancelation-of-nationalization-of-privatbank-39898.html&docid=BO6hdHP7X06ILM&tbnid=zcmrSy9lozDr1M%3A&vet=10ahUKEwih_JeryLXjAhWEQc0KHe8aApYQMwhEKAQwBA..i&w=650&h=351&client=opera&bih=598&biw=1205&q=PrivatBank&ved=0ahUKEwih_JeryLXjAhWEQc0KHe8aApYQMwhEKAQwBA&iact=mrc&uact=8___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread 0-day Robot
Bleep bloop.  Greetings Vasu Dasari, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 82 characters long (recommended limit is 79)
#71 FILE: Documentation/topics/testing.rst:97:
   export 
OVS_RUNDIR=/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139

WARNING: Line is 99 characters long (recommended limit is 79)
#76 FILE: Documentation/topics/testing.rst:102:
   export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139

Lines checked: 133, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] Аудиокниги "Модель для сборки" - Самая полная и качественная коллекция. 13_05_2019 02_03 198707

2019-07-15 Thread Роман Колунов via dev
АУДИОКНИГИ "МОДЕЛЬ ДЛЯ СБОРКИ"
Самая полная и качественная коллекция !

Программа "Модель для сборки" появилась вскоре после открытия радио <Станция> - 
в конце 1995 года. <Станция> была в то время свежей волной в радио, выдав в 
эфир танцевальную клубную музыку всех стилей и направлений - от хауса до 
хардкора. Данная передача представляет собой сложную смесь, состоящую из 
музыки, которую играет DJ Incognito, и чтения различных фантастических (а 
иногда и не очень) произведений отечественных и зарубежных авторов, чем 
занимается Влад Копп. В принципе, идея этой передачи не первична по своей сути. 
Многие знают, что подобного рода радиопередачи в течение длительного времени 
выходили на частоте радиостанции "Маяк". Однако они были неинтересны, 
во-первых, абсолютно другой направленностью читаемых произведений, а во-вторых 
- и это главное - почти полным отсутствием какого бы то ни было музыкального 
сопровождения. В "Модели для сборки" же музыка в большой степени определяет то, 
например, как слушатель воспримет произведение, играемое под эту музыку, 
понравится оно ему, или нет, а значит - определяет и всю радиопередачу. Музыка 
несет большую эмоциональную нагрузку, к тому же авторы передачи стараются так 
сочетать музыку и текст, чтобы музыка являлась отражением каждого конкретного 
объема текста, несла ту же смысловую нагрузку. Что касается самой музыки, то 
она очень разнообразна. Она включает в себя такие музыкальные направления, как 
Ambient, Psychedelic Trance, Dub, Trip-Hop, Electronic и так далее... Наша 
коллекция послужит прекрасным подарком, не лишайте себя и своих близких 
удовольствия!

! Полный список аудиокниг находится в прикрепленном к письму файле !

Коллекция состоит из всех передач с первой до последней. Общий обьём коллекции 
= 57 гигабайт. Записи в формате MP3 с битрейтом 128-224 kbps (преобладает 
высокий битрейт). Общее количество авторов = 473. Общее количество рассказов 
~1200. Все рассказы поименованы. Записана на внешний USB накопитель (флешка). 
Проблем с воспроизведением не возникнет, можно слушать на компьютере, планшете, 
смартфоне, телевизоре и т.д. Запись на внешний USB накопитель имеет ряд 
преимуществ в сравнении с обычными DVD дисками, USB накопитель гораздо легче, 
занимает меньше места, обладает высокой надёжностью сохранности записей, а это 
значит, что наша коллекция будет радовать Вас много лет. Мы гарантируем 
отличное качество всех записей. На самом носителе создана продуманная 
структура, все записи разнесены по каталогам, имеются плейлисты, прописаны 
теги, а также полный список вошедших записей, поэтому проблем с поиском и 
навигацией не возникнет.

Стоимость коллекции на внешнем USB накопителе — 6500 (Шесть Тысяч Пятьсот) 
Рублей.
Продаются только вместе. Доставка включена в стоимость.

Доставка только почтой по всей России, сроки 7-14 суток с момента отправки. 
Оплата в момент получения заказа на почте наложенным платежом. У нас нет 
курьерской доставки — только почтой, в том числе и по Москве.

Для оформления заказа просьба не забывать указывать:
 --- Ваш почтовый индекс (пишите правильный индекс — это ускорит доставку);
 --- Ваш город и точный адрес (название улицы, номер дома и номер квартиры);
 --- Ф.И.О. получателя и ОБЯЗАТЕЛЬНО номер контактного телефона (лучше сотовый);
Заказы\вопросы направляйте по адресу: mdsb...@cwhflash.ru

Мы очень ответственно относимся к качеству нашего товара, поэтому перед 
отправкой всё дополнительно проверяется, как следствие отправка бракованной 
продукции сведена к нулю. Товар упаковывается в специальный ударостойкий 
материал, что в значительной степени уменьшает риск повреждения при 
транспортировке. Если вдруг с полученным товаром возникнут проблемы, то все 
наши покупатели всегда могут рассчитывать на квалифицированную техническую 
поддержку. Мы никогда не отказываемся от гарантийных обязательств, в случае 
проблемы Вы можете рассчитывать на замену, почтовые расходы мы берём на себя.

По вашему желанию, данная коллекция может быть записана на DVD диски. Для 
записи используются надёжные DVD диски со специальным покрытием, которое 
повышает устойчивость диска к механическим повреждениям, таким как трещины и 
царапины, а это значит, что наша коллекция будет радовать Вас много лет. 
Коллекция упакована в пластиковые боксы (slim-dvd), имеет красивые и 
продуманные обложки, с обратной стороны которых указан список вошедших на 
каждый диск записей и другая полезная информация, поэтому проблем с поиском и 
навигацией не возникнет. Если хотите приобрести коллекцию, записанную на DVD 
дисках, то в этом случае просьба сообщить нам об этом в своей заявке, цена 
прежняя, как у версии на внешнем USB накопителе (флешка) — 6500 (Шесть Тысяч 
Пятьсот) Рублей.

Если вы не хотите больше получать от нас письма, отправьте нам письмо с темой 
“deletemail” и Ваш адрес навсегда будет удален автоматически.

13_05_2019 02_03 198707

ovs-dev@openvswitch.org
1001 íî÷ü: Èñòîðèÿ Àáäóëû íèùåãî ñëåïöà.
À. Ëåíòèíè: Äåðåâî
Àäàì Ðîáåðòñ: Ïî÷òè ïî Ñâ

Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Done. Patch is at:

[ovs-dev] [PATCH v2] ovs-macros: An option to suspend test execution on
error 



*Vasu Dasari*


On Mon, Jul 15, 2019 at 4:12 PM Ben Pfaff  wrote:

> I'm not Aaron but that does seems reasonable to me.
>
> On Mon, Jul 15, 2019 at 04:01:26PM -0400, Vasu Dasari wrote:
> > Aaron,
> >
> > I see that documentation for topics/testing is mostly one liners and
> > precise. And hence I chose one liner as well. If I have to put all your
> > recommended test, I think it is better to have a subsection, "Debugging
> > unit tests". What do you think?
> >
> > -Vasu
> >
> > *Vasu Dasari*
> >
> >
> > On Mon, Jul 15, 2019 at 3:48 PM Aaron Conole  wrote:
> >
> > > Hi Vasu,
> > >
> > > Vasu Dasari  writes:
> > >
> > > > Origins for this patch are captured at
> > > >
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html
> > > .
> > > >
> > > > Summarizing here, when a test fails, it would be good to pause test
> > > execution
> > > > and let the developer poke around the system to see current status of
> > > system.
> > > >
> > > > As part of this patch, made a small tweaks to ovs-macros.at, so that
> > > when test
> > > > suite fails, ovs_on_exit() function will be called. And in this
> > > function, a check
> > > > is made to see if an environment variable to OVS_PAUSE_TEST is set.
> If
> > > it is
> > > > set, then test suite is paused and will continue to wait for user
> input
> > > > Ctrl-D. Meanwhile user can poke around the system to see why test
> case
> > > has
> > > > failed. Once done with investigation, user can press ctrl-d to
> cleanup
> > > the
> > > > test suite.
> > > >
> > >
> > > From this point
> > >vv
> > >
> > > > For example, to re-run test case 139:
> > > >
> > > > export OVS_PAUSE_TEST=1
> > > > cd tests/system-userspace-testsuite.dir/139
> > > > sudo -E ./run
> > > >
> > > > When error occurs, above command would display something like this:
> > > > =
> > > > Set environment variable to use various ovs utilities
> > > > export
> > >
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> > > > Press ctrl-d to continue:
> > > >
> > > > =
> > > > And from another window, one can execute ovs-xxx commands like:
> > > > export
> > >
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> > > > $ ovs-ofctl dump-ports br0
> > > > .
> > > > .
> > > >
> > > > To be able to pause while performing `make check`, one can do:
> > > > $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'
> > > >
> > >
> > > To here ^^
> > >
> > > I would propose to put this information in the documentation.  A new
> > > developer may not actually trawl through the commits, but this
> > > information would help such a developer (or even a not-so-new
> developer,
> > > like me).
> > >
> > > > Signed-off-by: Vasu Dasari 
> > > > Reviewed-by: Ben Pfaff 
> > > > ---
> > > > v0 -> v1:
> > > > Discussion at:
> > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
> > > > 1. Incorporated review commanets from Ben.
> > > > 2. Changed topics/testing.rst to document this trick
> > > > 3. Made this trick work via "make check" as well, by issuing command:
> > > "OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"
> > > >
> > > > ---
> > > >  Documentation/topics/testing.rst |  6 ++
> > > >  tests/ovs-macros.at  | 24 +++-
> > > >  2 files changed, 29 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/topics/testing.rst
> > > b/Documentation/topics/testing.rst
> > > > index a8892e1c1..058b3986f 100644
> > > > --- a/Documentation/topics/testing.rst
> > > > +++ b/Documentation/topics/testing.rst
> > > > @@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``,
> run::
> > > >
> > > >  $ make check TESTSUITEFLAGS='-k ovsdb'
> > > >
> > > > +To pause at a failing test (e.g. ``ovsdb``) and inspect the system
> in
> > > failed
> > > > +state, follow the instructions given out by issuing following
> command
> > > to use
> > > > +various ovs-* utilities::
> > > > +
> > > > +$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
> > > > +
> > > >  To see a complete list of test options, run::
> > > >
> > > >  $ make check TESTSUITEFLAGS=--help
> > > > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> > > > index 10593429d..e58b0a1a0 100644
> > > > --- a/tests/ovs-macros.at
> > > > +++ b/tests/ovs-macros.at
> > > > @@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
> > > >  # directory.
> > > >  ovs_init() {
> > > >  ovs_base=`pwd`
> > > > -trap '. "$ovs_base/cleanup"' 0
> > > > +trap ovs_on_exit 0
> > > >  : > cleanup
> > > >  ovs_setenv
> > > >  }
> > 

[ovs-dev] [PATCH v2] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Origins for this patch are captured at
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html.

Summarizing here, when a test fails, it would be good to pause test execution
and let the developer poke around the system to see current status of system.

As part of this patch, made a small tweaks to ovs-macros.at, so that when test
suite fails, ovs_on_exit() function will be called. And in this function, a 
check
is made to see if an environment variable to OVS_PAUSE_TEST is set. If it is
set, then test suite is paused and will continue to wait for user input
Ctrl-D. Meanwhile user can poke around the system to see why test case has
failed. Once done with investigation, user can press ctrl-d to cleanup the
test suite.

For example, to re-run test case 139:

export OVS_PAUSE_TEST=1
cd tests/system-userspace-testsuite.dir/139
sudo -E ./run

When error occurs, above command would display something like this:
=
Set environment variable to use various ovs utilities
export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
Press ENTER to continue:

=
And from another window, one can execute ovs-xxx commands like:
export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
$ ovs-ofctl dump-ports br0
.
.

To be able to pause while performing `make check`, one can do:
$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'

Signed-off-by: Vasu Dasari 
Reviewed-by: Ben Pfaff 
Reviewed-by: Aaron Conole 
---
v0 -> v1:
Discussion at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
1. Incorporated review commanets from Ben.
2. Changed topics/testing.rst to document this trick
3. Made this trick work via "make check" as well, by issuing command: 
"OVS_PAUSE_TEST=1
make check TESTSUITEFLAGS='-v'"

v1 -> v2:
Discussion at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360705.html
1. Added a subsection, "Debugging unit tests"
2. Renamed $as_echo to just echo. AS_ECHO is not suported in autoconf 2.69.11. 
And, we are not gainin
g anything from AS_ECHO in this code, just using echo.

---
 Documentation/topics/testing.rst | 31 +++
 tests/ovs-macros.at  | 24 +++-
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst
index a8892e1c1..73077042c 100644
--- a/Documentation/topics/testing.rst
+++ b/Documentation/topics/testing.rst
@@ -81,6 +81,37 @@ To see a complete list of test options, run::
 The results of a testing run are reported in ``tests/testsuite.log``. Report
 report test failures as bugs and include the ``testsuite.log`` in your report.
 
+Debugging unit tests
+
+
+To initiate debugging from artifacts generated from `make check` run, use
+following instructions. For example, to re-run test case 139::
+
+   export OVS_PAUSE_TEST=1
+   cd tests/system-userspace-testsuite.dir/139
+   sudo -E ./run
+
+When error occurs, above command would display something like this::
+
+   Set environment variable to use various ovs utilities
+   export 
OVS_RUNDIR=/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
+   Press ENTER to continue:
+
+And from another window, one can execute ovs-xxx commands like::
+
+   export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
+   $ ovs-ofctl dump-ports br0
+   .
+   .
+
+Once done with investigation, press ENTER to perform cleanup operation.
+
+To be able to pause on error while performing `make check`, one can do::
+
+  $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'
+
+and then follow similar on screen instructions
+
 .. note::
   Sometimes a few tests may fail on some runs but not others. This is usually a
   bug in the testsuite, not a bug in Open vSwitch itself. If you find that a
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 10593429d..b6add7fda 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
 # directory.
 ovs_init() {
 ovs_base=`pwd`
-trap '. "$ovs_base/cleanup"' 0
+trap ovs_on_exit 0
 : > cleanup
 ovs_setenv
 }
 
+# Catch testsuite error condition and cleanup test environment by tearing down
+# all interfaces and processes spawned.
+# User has an option to leave the test environment in error state so that 
system
+# can be poked around to get more information. User can enable this option by 
setting
+# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to resume 
the
+# cleanup operation.
+ovs_pause() {
+echo "="
+echo "Set following environment variable to use various ovs utilities"
+echo "export OVS_RUNDIR=$ovs_base"
+echo "Press ENTER to continue: "
+read
+}
+
+ovs_on_exit (

Re: [ovs-dev] [PATCHv16 1/2] ovs-thread: Add pthread spin lock support.

2019-07-15 Thread William Tu
On Mon, Jul 15, 2019 at 11:54 AM Ben Pfaff  wrote:
>
> On Mon, Jul 15, 2019 at 05:28:44PM +0300, Ilya Maximets wrote:
> > On 13.07.2019 2:50, William Tu wrote:
> > > The patch adds the basic spin lock functions:
> > > ovs_spin_{lock, try_lock, unlock, init, destroy}.
> > > OSX does not support pthread spin lock, so make it
> > > linux only.
> >
> > IIUC, pthread spinlock requires some specific glibc verions (>= 2.2)
> > so it could be not supported even on Linux. Instead of checking
> > _POSIX_C_SOURCE version, I think it's better to just check for
> > pthread_spin_lock function in configure script with AC_CHECK_FUNC and
> > check for the resulted macro.
> > Additionally we could check for this macro while checking AF_XDP support
> > to not enable it if we have no spinlocks.
>
> glibc 2.2 was released in 2000.  I don't think that it is worth worrying
> that a user might have such an ancient version of glibc.
>
> However, I think that feature checks are generally better, so I support
> the change.

Thanks Ben and Ilya.

I will add AC_CHECK_FUNC for checking the existence of
pthread_spin_lock. If not exist, this patch won't get compile
in and the following AF_XDP will be disable.

Feature Test Macro Requirements for pthread_spin_lock
_POSIX_C_SOURCE >= 200112L
Since it's been a while, I won't add the _POSIX_C_SOURCE  check.

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


Re: [ovs-dev] Soft freeze exception request for OVN IGMP support in 2.12

2019-07-15 Thread Dumitru Ceara
On Fri, Jul 12, 2019 at 7:37 PM Ben Pfaff  wrote:
>
> On Tue, Jul 09, 2019 at 03:40:02PM +0200, Dumitru Ceara wrote:
> > Would it be possible to have an exception for IGMP support in 2.12
> > once review is done? Ben already did a preliminary review of v1 and I
> > posted v2 [1] today.
>
> Seems fine to me, since this has been in progress for a while, as long
> as we do something to ensure stable allocation.

Hi Ben,

I just sent v4 which fixes the ID allocation issue:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=119600

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


Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread 0-day Robot
Bleep bloop.  Greetings Vasu Dasari, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line has trailing whitespace
#58 FILE: Documentation/topics/testing.rst:78:
state, follow the instructions given out by issuing following command to use 

WARNING: Line has trailing whitespace
#91 FILE: tests/ovs-macros.at:54:
read 

Lines checked: 107, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v4 3/3] OVN: Add ovn-northd IGMP support

2019-07-15 Thread Dumitru Ceara
New IP Multicast Snooping Options are added to the Northbound DB
Logical_Switch:other_config column. These allow enabling IGMP snooping and
querier on the logical switch and get translated by ovn-northd to rows in
the IP_Multicast Southbound DB table.

ovn-northd monitors for changes done by ovn-controllers in the Southbound DB
IGMP_Group table. Based on the entries in IGMP_Group ovn-northd creates
Multicast_Group entries in the Southbound DB, one per IGMP_Group address X,
containing the list of logical switch ports (aggregated from all controllers)
that have IGMP_Group entries for that datapath and address X. ovn-northd
also creates a logical flow that matches on IP multicast traffic destined
to address X and outputs it on the tunnel key of the corresponding
Multicast_Group entry.

Signed-off-by: Dumitru Ceara 
Acked-by: Mark Michelson 
---
 ovn/controller/ovn-controller.c |5 
 ovn/controller/pinctrl.c|1 
 ovn/lib/automake.mk |2 
 ovn/lib/ip-mcast-index.h|3 
 ovn/lib/mcast-group-index.c |   43 +++
 ovn/lib/mcast-group-index.h |   32 ++
 ovn/northd/ovn-northd.c |  548 ---
 ovn/ovn-nb.xml  |   54 
 tests/ovn.at|  270 +++
 tests/system-ovn.at |  119 
 10 files changed, 1033 insertions(+), 44 deletions(-)
 create mode 100644 ovn/lib/mcast-group-index.c
 create mode 100644 ovn/lib/mcast-group-index.h

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 080a357..cf6c8ae 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -45,6 +45,7 @@
 #include "ovn/lib/chassis-index.h"
 #include "ovn/lib/extend-table.h"
 #include "ovn/lib/ip-mcast-index.h"
+#include "ovn/lib/mcast-group-index.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn/lib/ovn-util.h"
 #include "patch.h"
@@ -1743,9 +1744,7 @@ main(int argc, char *argv[])
 struct ovsdb_idl_index *sbrec_chassis_by_name
 = chassis_index_create(ovnsb_idl_loop.idl);
 struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath
-= ovsdb_idl_index_create2(ovnsb_idl_loop.idl,
-  &sbrec_multicast_group_col_name,
-  &sbrec_multicast_group_col_datapath);
+= mcast_group_index_create(ovnsb_idl_loop.idl);
 struct ovsdb_idl_index *sbrec_port_binding_by_name
 = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
   &sbrec_port_binding_col_logical_port);
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index f9730a6..d857067 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -45,6 +45,7 @@
 #include "ovn/lex.h"
 #include "ovn/lib/acl-log.h"
 #include "ovn/lib/ip-mcast-index.h"
+#include "ovn/lib/mcast-group-index.h"
 #include "ovn/lib/ovn-l7.h"
 #include "ovn/lib/ovn-util.h"
 #include "ovn/logical-fields.h"
diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
index 8f69982..7cac67f 100644
--- a/ovn/lib/automake.mk
+++ b/ovn/lib/automake.mk
@@ -14,6 +14,8 @@ ovn_lib_libovn_la_SOURCES = \
ovn/lib/extend-table.c \
ovn/lib/ip-mcast-index.c \
ovn/lib/ip-mcast-index.h \
+   ovn/lib/mcast-group-index.c \
+   ovn/lib/mcast-group-index.h \
ovn/lib/lex.c \
ovn/lib/ovn-l7.h \
ovn/lib/ovn-util.c \
diff --git a/ovn/lib/ip-mcast-index.h b/ovn/lib/ip-mcast-index.h
index 15141ea..a23b4a7 100644
--- a/ovn/lib/ip-mcast-index.h
+++ b/ovn/lib/ip-mcast-index.h
@@ -28,9 +28,6 @@ struct sbrec_datapath_binding;
 #define OVN_MCAST_DEFAULT_QUERY_MAX_RESPONSE_S 1
 #define OVN_MCAST_DEFAULT_MAX_ENTRIES  2048
 
-#define OVN_MCAST_FLOOD_TUNNEL_KEY   65535
-#define OVN_MCAST_UNKNOWN_TUNNEL_KEY (OVN_MCAST_FLOOD_TUNNEL_KEY - 1)
-
 struct ovsdb_idl_index *ip_mcast_index_create(struct ovsdb_idl *);
 const struct sbrec_ip_multicast *ip_mcast_lookup(
 struct ovsdb_idl_index *ip_mcast_index,
diff --git a/ovn/lib/mcast-group-index.c b/ovn/lib/mcast-group-index.c
new file mode 100644
index 000..740311e
--- /dev/null
+++ b/ovn/lib/mcast-group-index.c
@@ -0,0 +1,43 @@
+/* Copyright (c) 2019, Red Hat, Inc.
+ *
+ * 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.
+ */
+
+#include 
+
+#include "ovn/lib/mcast-group-index.h"
+#include "ovn/lib/ovn-sb-idl.h"
+
+struct ovsdb_idl_index *
+mcast_group_index_create(struct ovsdb_idl *idl)
+{
+return ovsdb

[ovs-dev] [PATCH v4 2/3] OVN: Add IGMP SB definitions and ovn-controller support

2019-07-15 Thread Dumitru Ceara
A new IP_Multicast table is added to Southbound DB. This table stores the
multicast related configuration for each datapath. Each row will be
populated by ovn-northd and will control:
- if IGMP Snooping is enabled or not, the snooping table size and multicast
  group idle timeout.
- if IGMP Querier is enabled or not (only if snooping is enabled too), query
  interval, query source addresses (Ethernet and IP) and the max-response
  field to be stored in outgoing queries.
- an additional "seq_no" column is added such that ovn-sbctl or if needed a
  CMS can flush currently learned groups. This can be achieved by incrementing
  the "seq_no" value.

A new IGMP_Group table is added to Southbound DB. This table stores all the
multicast groups learned by ovn-controllers. The table is indexed by
datapath, group address and chassis. For a learned multicast group on a
specific datapath each ovn-controller will store its own row in this table.
Each row contains the list of chassis-local ports on which the group was
learned. Rows in the IGMP_Group table are updated or deleted only by the
ovn-controllers that created them.

A new action ("igmp") is added to punt IGMP packets on a specific logical
switch datapath to ovn-controller if IGMP snooping is enabled.

Per datapath IGMP multicast snooping support is added to pinctrl:
- incoming IGMP reports are processed and multicast groups are maintained
  (using the OVS mcast-snooping library).
- each OVN controller syncs its in-memory IGMP groups to the Southbound DB
  in the IGMP_Group table.
- pinctrl also sends periodic IGMPv3 general queries for all datapaths where
  querier is enabled.

Signed-off-by: Mark Michelson 
Co-authored-by: Mark Michelson 
Signed-off-by: Dumitru Ceara 
Acked-by: Mark Michelson 
---
 include/ovn/actions.h   |7 
 ovn/controller/automake.mk  |2 
 ovn/controller/ip-mcast.c   |  164 
 ovn/controller/ip-mcast.h   |   52 +++
 ovn/controller/ovn-controller.c |   23 +
 ovn/controller/pinctrl.c|  784 +++
 ovn/controller/pinctrl.h|2 
 ovn/lib/actions.c   |   16 +
 ovn/lib/automake.mk |2 
 ovn/lib/ip-mcast-index.c|   40 ++
 ovn/lib/ip-mcast-index.h|   39 ++
 ovn/lib/logical-fields.c|2 
 ovn/ovn-sb.ovsschema|   44 ++
 ovn/ovn-sb.xml  |   80 
 ovn/utilities/ovn-sbctl.c   |   53 +++
 ovn/utilities/ovn-trace.c   |4 
 tests/ovn.at|4 
 17 files changed, 1312 insertions(+), 6 deletions(-)
 create mode 100644 ovn/controller/ip-mcast.c
 create mode 100644 ovn/controller/ip-mcast.h
 create mode 100644 ovn/lib/ip-mcast-index.c
 create mode 100644 ovn/lib/ip-mcast-index.h

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 5ed70e7..30551ad 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -67,6 +67,7 @@ struct ovn_extend_table;
 OVNACT(ICMP4, ovnact_nest)\
 OVNACT(ICMP4_ERROR,   ovnact_nest)\
 OVNACT(ICMP6, ovnact_nest)\
+OVNACT(IGMP,  ovnact_null)\
 OVNACT(TCP_RESET, ovnact_nest)\
 OVNACT(ND_NA, ovnact_nest)\
 OVNACT(ND_NA_ROUTER,  ovnact_nest)\
@@ -498,6 +499,12 @@ enum action_opcode {
 
 /* "trigger_event (event_type)" */
 ACTION_OPCODE_EVENT,
+
+/* "igmp()".
+ *
+ * Snoop IGMP, learn the multicast participants
+ */
+ACTION_OPCODE_IGMP,
 };
 
 /* Header. */
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index fcdf7a4..193ea69 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -10,6 +10,8 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/encaps.h \
ovn/controller/ha-chassis.c \
ovn/controller/ha-chassis.h \
+   ovn/controller/ip-mcast.c \
+   ovn/controller/ip-mcast.h \
ovn/controller/lflow.c \
ovn/controller/lflow.h \
ovn/controller/lport.c \
diff --git a/ovn/controller/ip-mcast.c b/ovn/controller/ip-mcast.c
new file mode 100644
index 000..ef36be2
--- /dev/null
+++ b/ovn/controller/ip-mcast.c
@@ -0,0 +1,164 @@
+/* Copyright (c) 2019, Red Hat, Inc.
+ *
+ * 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.
+ */
+
+#include 
+
+#include "ip-mcast.h"
+#include "lport.h"
+#include "ovn/lib/ovn-sb-idl.h"
+
+/*
+ 

[ovs-dev] [PATCH v4 1/3] packets: Add IGMPv3 query packet definitions

2019-07-15 Thread Dumitru Ceara
Signed-off-by: Dumitru Ceara 
Acked-by: Mark Michelson 
---
 lib/packets.c |   44 
 lib/packets.h |   19 ++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/lib/packets.c b/lib/packets.c
index a8fd61f..ab0b1a3 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1281,6 +1281,50 @@ packet_set_icmp(struct dp_packet *packet, uint8_t type, 
uint8_t code)
 }
 }
 
+/* Sets the IGMP type to IGMP_HOST_MEMBERSHIP_QUERY and populates the
+ * v3 query header fields in 'packet'. 'packet' must be a valid IGMPv3
+ * query packet with its l4 offset properly populated.
+ */
+void
+packet_set_igmp3_query(struct dp_packet *packet, uint8_t max_resp,
+   ovs_be32 group, bool srs, uint8_t qrv, uint8_t qqic)
+{
+struct igmpv3_query_header *igh = dp_packet_l4(packet);
+ovs_be16 orig_type_max_resp =
+htons(igh->type << 8 | igh->max_resp);
+ovs_be16 new_type_max_resp =
+htons(IGMP_HOST_MEMBERSHIP_QUERY << 8 | max_resp);
+
+if (orig_type_max_resp != new_type_max_resp) {
+igh->type = IGMP_HOST_MEMBERSHIP_QUERY;
+igh->max_resp = max_resp;
+igh->csum = recalc_csum16(igh->csum, orig_type_max_resp,
+  new_type_max_resp);
+}
+
+ovs_be32 old_group = get_16aligned_be32(&igh->group);
+
+if (old_group != group) {
+put_16aligned_be32(&igh->group, group);
+igh->csum = recalc_csum32(igh->csum, old_group, group);
+}
+
+/* See RFC 3376 4.1.6. */
+if (qrv > 7) {
+qrv = 0;
+}
+
+ovs_be16 orig_srs_qrv_qqic = htons(igh->srs_qrv << 8 | igh->qqic);
+ovs_be16 new_srs_qrv_qqic = htons(srs << 11 | qrv << 8 | qqic);
+
+if (orig_srs_qrv_qqic != new_srs_qrv_qqic) {
+igh->srs_qrv = (srs << 3 | qrv);
+igh->qqic = qqic;
+igh->csum = recalc_csum16(igh->csum, orig_srs_qrv_qqic,
+  new_srs_qrv_qqic);
+}
+}
+
 void
 packet_set_nd_ext(struct dp_packet *packet, const ovs_16aligned_be32 rso_flags,
   const uint8_t opt_type)
diff --git a/lib/packets.h b/lib/packets.h
index d293b35..4124490 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -681,6 +681,7 @@ char *ip_parse_cidr_len(const char *s, int *n, ovs_be32 *ip,
 #define IP_ECN_ECT_0 0x02
 #define IP_ECN_CE 0x03
 #define IP_ECN_MASK 0x03
+#define IP_DSCP_CS6 0xc0
 #define IP_DSCP_MASK 0xfc
 
 static inline int
@@ -763,6 +764,20 @@ struct igmpv3_header {
 };
 BUILD_ASSERT_DECL(IGMPV3_HEADER_LEN == sizeof(struct igmpv3_header));
 
+#define IGMPV3_QUERY_HEADER_LEN 12
+struct igmpv3_query_header {
+uint8_t type;
+uint8_t max_resp;
+ovs_be16 csum;
+ovs_16aligned_be32 group;
+uint8_t srs_qrv;
+uint8_t qqic;
+ovs_be16 nsrcs;
+};
+BUILD_ASSERT_DECL(
+IGMPV3_QUERY_HEADER_LEN == sizeof(struct igmpv3_query_header
+));
+
 #define IGMPV3_RECORD_LEN 8
 struct igmpv3_record {
 uint8_t type;
@@ -1543,7 +1558,9 @@ void packet_set_nd(struct dp_packet *, const struct 
in6_addr *target,
 void packet_set_nd_ext(struct dp_packet *packet,
const ovs_16aligned_be32 rso_flags,
const uint8_t opt_type);
-
+void packet_set_igmp3_query(struct dp_packet *, uint8_t max_resp,
+ovs_be32 group, bool srs, uint8_t qrv,
+uint8_t qqic);
 void packet_format_tcp_flags(struct ds *, uint16_t);
 const char *packet_tcp_flag_to_string(uint32_t flag);
 void compose_arp__(struct dp_packet *);

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


[ovs-dev] [PATCH v4 0/3] OVN: Add IGMP support

2019-07-15 Thread Dumitru Ceara
This series introduces support for IGMP Snooping and IGMP Querier. IGMP
versions v1-v3 are supported for snooping and IGMP queries originated by
ovn-controller are general IGMPv3 queries. The rationale behind using v3 for
querier is that it's backward compatible with v1-v2.

The majority of the code is IP version independent with the thought in mind
that support for MLD snooping for IPv6 will be added next.

Dumitru Ceara (3):
  packets: Add IGMPv3 query packet definitions
  OVN: Add IGMP SB definitions and ovn-controller support
  OVN: Add ovn-northd IGMP support


 include/ovn/actions.h   |7 
 lib/packets.c   |   44 ++
 lib/packets.h   |   19 +
 ovn/controller/automake.mk  |2 
 ovn/controller/ip-mcast.c   |  164 
 ovn/controller/ip-mcast.h   |   52 +++
 ovn/controller/ovn-controller.c |   28 +
 ovn/controller/pinctrl.c|  785 +++
 ovn/controller/pinctrl.h|2 
 ovn/lib/actions.c   |   16 +
 ovn/lib/automake.mk |4 
 ovn/lib/ip-mcast-index.c|   40 ++
 ovn/lib/ip-mcast-index.h|   36 ++
 ovn/lib/logical-fields.c|2 
 ovn/lib/mcast-group-index.c |   43 ++
 ovn/lib/mcast-group-index.h |   32 ++
 ovn/northd/ovn-northd.c |  548 +--
 ovn/ovn-nb.xml  |   54 +++
 ovn/ovn-sb.ovsschema|   44 ++
 ovn/ovn-sb.xml  |   80 
 ovn/utilities/ovn-sbctl.c   |   53 +++
 ovn/utilities/ovn-trace.c   |4 
 tests/ovn.at|  274 ++
 tests/system-ovn.at |  119 ++
 24 files changed, 2404 insertions(+), 48 deletions(-)
 create mode 100644 ovn/controller/ip-mcast.c
 create mode 100644 ovn/controller/ip-mcast.h
 create mode 100644 ovn/lib/ip-mcast-index.c
 create mode 100644 ovn/lib/ip-mcast-index.h
 create mode 100644 ovn/lib/mcast-group-index.c
 create mode 100644 ovn/lib/mcast-group-index.h


---
v4:
- fix multicast_group tunnel key allocation so that the old group keys
  from the SB database are reused
- add mcast-group-index.[ch] to avoid code duplication between ovn-northd
  and ovn-controller

v3:
- add acks from Mark Michelson
- fix action "igmp": no need for nested actions
- fix overlap between unknown multicast tunnel key and IP multicast tunnel
  keys

v2: 
- address reviewer comments.
- fix a memory corruption when reallocating multicast ports in
  ovn_multicast_add_ports.
- add missing NULL checks in pinctrl.c for the case when a logical switch
  configuration gets deleted by ovn-northd and controllers are updating
  IGMP_Group entries.
- Fix allocation of multicast group IDs in ovn-northd. The multicast group
  IDs were allocated globally instead of per-datapath  which was limiting
  the total number of IGMP Groups. At most 32K IGMP groups can be learned
  per datapath regardless of how many datapaths are configured.
- add system-ovn.at test.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Ben Pfaff
I'm not Aaron but that does seems reasonable to me.

On Mon, Jul 15, 2019 at 04:01:26PM -0400, Vasu Dasari wrote:
> Aaron,
> 
> I see that documentation for topics/testing is mostly one liners and
> precise. And hence I chose one liner as well. If I have to put all your
> recommended test, I think it is better to have a subsection, "Debugging
> unit tests". What do you think?
> 
> -Vasu
> 
> *Vasu Dasari*
> 
> 
> On Mon, Jul 15, 2019 at 3:48 PM Aaron Conole  wrote:
> 
> > Hi Vasu,
> >
> > Vasu Dasari  writes:
> >
> > > Origins for this patch are captured at
> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html
> > .
> > >
> > > Summarizing here, when a test fails, it would be good to pause test
> > execution
> > > and let the developer poke around the system to see current status of
> > system.
> > >
> > > As part of this patch, made a small tweaks to ovs-macros.at, so that
> > when test
> > > suite fails, ovs_on_exit() function will be called. And in this
> > function, a check
> > > is made to see if an environment variable to OVS_PAUSE_TEST is set. If
> > it is
> > > set, then test suite is paused and will continue to wait for user input
> > > Ctrl-D. Meanwhile user can poke around the system to see why test case
> > has
> > > failed. Once done with investigation, user can press ctrl-d to cleanup
> > the
> > > test suite.
> > >
> >
> > From this point
> >vv
> >
> > > For example, to re-run test case 139:
> > >
> > > export OVS_PAUSE_TEST=1
> > > cd tests/system-userspace-testsuite.dir/139
> > > sudo -E ./run
> > >
> > > When error occurs, above command would display something like this:
> > > =
> > > Set environment variable to use various ovs utilities
> > > export
> > OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> > > Press ctrl-d to continue:
> > >
> > > =
> > > And from another window, one can execute ovs-xxx commands like:
> > > export
> > OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> > > $ ovs-ofctl dump-ports br0
> > > .
> > > .
> > >
> > > To be able to pause while performing `make check`, one can do:
> > > $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'
> > >
> >
> > To here ^^
> >
> > I would propose to put this information in the documentation.  A new
> > developer may not actually trawl through the commits, but this
> > information would help such a developer (or even a not-so-new developer,
> > like me).
> >
> > > Signed-off-by: Vasu Dasari 
> > > Reviewed-by: Ben Pfaff 
> > > ---
> > > v0 -> v1:
> > > Discussion at:
> > https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
> > > 1. Incorporated review commanets from Ben.
> > > 2. Changed topics/testing.rst to document this trick
> > > 3. Made this trick work via "make check" as well, by issuing command:
> > "OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"
> > >
> > > ---
> > >  Documentation/topics/testing.rst |  6 ++
> > >  tests/ovs-macros.at  | 24 +++-
> > >  2 files changed, 29 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/topics/testing.rst
> > b/Documentation/topics/testing.rst
> > > index a8892e1c1..058b3986f 100644
> > > --- a/Documentation/topics/testing.rst
> > > +++ b/Documentation/topics/testing.rst
> > > @@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``, run::
> > >
> > >  $ make check TESTSUITEFLAGS='-k ovsdb'
> > >
> > > +To pause at a failing test (e.g. ``ovsdb``) and inspect the system in
> > failed
> > > +state, follow the instructions given out by issuing following command
> > to use
> > > +various ovs-* utilities::
> > > +
> > > +$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
> > > +
> > >  To see a complete list of test options, run::
> > >
> > >  $ make check TESTSUITEFLAGS=--help
> > > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> > > index 10593429d..e58b0a1a0 100644
> > > --- a/tests/ovs-macros.at
> > > +++ b/tests/ovs-macros.at
> > > @@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
> > >  # directory.
> > >  ovs_init() {
> > >  ovs_base=`pwd`
> > > -trap '. "$ovs_base/cleanup"' 0
> > > +trap ovs_on_exit 0
> > >  : > cleanup
> > >  ovs_setenv
> > >  }
> > >
> > > +# Catch testsuite error condition and cleanup test environment by
> > tearing down
> > > +# all interfaces and processes spawned.
> > > +# User has an option to leave the test environment in error state so
> > that system
> > > +# can be poked around to get more information. User can enable this
> > option by setting
> > > +# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to
> > resume the
> > > +# cleanup operation.
> > > +ovs_pause() {
> > > +$as_echo "="
> > > +$as_echo "Set following

Re: [ovs-dev] [PATCH] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread 0-day Robot
Bleep bloop.  Greetings Vasu Dasari, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line has trailing whitespace
#58 FILE: Documentation/topics/testing.rst:78:
state, follow the instructions given out by issuing following command to use 

WARNING: Line has trailing whitespace
#91 FILE: tests/ovs-macros.at:54:
read 

Lines checked: 107, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Aaron,

I see that documentation for topics/testing is mostly one liners and
precise. And hence I chose one liner as well. If I have to put all your
recommended test, I think it is better to have a subsection, "Debugging
unit tests". What do you think?

Ben,

Will do. Also, note that I am using at_verbose variable if we are in
verbose mode or not. If user has not run the test suite in verbose mode,
none of the instructions to set the OVS_RUNDIR variable, etc will appear on
screen, and hence user will not have a clue why script is pausing. So, I am
also qualifying the check for at_verbose variable. I hope it is ok to use
the at_verbose variable.

-Vasu

*Vasu Dasari*


On Mon, Jul 15, 2019 at 3:59 PM Ben Pfaff  wrote:

> On Mon, Jul 15, 2019 at 03:22:42PM -0400, Vasu Dasari wrote:
> > Origins for this patch are captured at
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html
> .
>
> ...
>
> Aaron provided helpful comments.  In addition:
>
> > +ovs_pause() {
> > +$as_echo "="
> > +$as_echo "Set following environment variable to use various ovs
> utilities"
> > +$as_echo "export OVS_RUNDIR=$ovs_base"
> > +$as_echo "Press ENTER to continue: "
> > +read
> > +}
>
> The above uses $as_echo, which is what the AS_ECHO([...]) macro expands
> into.  It is better to use AS_ECHO itself, since in theory the
> implementation could change in a later version of Autoconf.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Aaron,

I see that documentation for topics/testing is mostly one liners and
precise. And hence I chose one liner as well. If I have to put all your
recommended test, I think it is better to have a subsection, "Debugging
unit tests". What do you think?

-Vasu

*Vasu Dasari*


On Mon, Jul 15, 2019 at 3:48 PM Aaron Conole  wrote:

> Hi Vasu,
>
> Vasu Dasari  writes:
>
> > Origins for this patch are captured at
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html
> .
> >
> > Summarizing here, when a test fails, it would be good to pause test
> execution
> > and let the developer poke around the system to see current status of
> system.
> >
> > As part of this patch, made a small tweaks to ovs-macros.at, so that
> when test
> > suite fails, ovs_on_exit() function will be called. And in this
> function, a check
> > is made to see if an environment variable to OVS_PAUSE_TEST is set. If
> it is
> > set, then test suite is paused and will continue to wait for user input
> > Ctrl-D. Meanwhile user can poke around the system to see why test case
> has
> > failed. Once done with investigation, user can press ctrl-d to cleanup
> the
> > test suite.
> >
>
> From this point
>vv
>
> > For example, to re-run test case 139:
> >
> > export OVS_PAUSE_TEST=1
> > cd tests/system-userspace-testsuite.dir/139
> > sudo -E ./run
> >
> > When error occurs, above command would display something like this:
> > =
> > Set environment variable to use various ovs utilities
> > export
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> > Press ctrl-d to continue:
> >
> > =
> > And from another window, one can execute ovs-xxx commands like:
> > export
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> > $ ovs-ofctl dump-ports br0
> > .
> > .
> >
> > To be able to pause while performing `make check`, one can do:
> > $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'
> >
>
> To here ^^
>
> I would propose to put this information in the documentation.  A new
> developer may not actually trawl through the commits, but this
> information would help such a developer (or even a not-so-new developer,
> like me).
>
> > Signed-off-by: Vasu Dasari 
> > Reviewed-by: Ben Pfaff 
> > ---
> > v0 -> v1:
> > Discussion at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
> > 1. Incorporated review commanets from Ben.
> > 2. Changed topics/testing.rst to document this trick
> > 3. Made this trick work via "make check" as well, by issuing command:
> "OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"
> >
> > ---
> >  Documentation/topics/testing.rst |  6 ++
> >  tests/ovs-macros.at  | 24 +++-
> >  2 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/topics/testing.rst
> b/Documentation/topics/testing.rst
> > index a8892e1c1..058b3986f 100644
> > --- a/Documentation/topics/testing.rst
> > +++ b/Documentation/topics/testing.rst
> > @@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``, run::
> >
> >  $ make check TESTSUITEFLAGS='-k ovsdb'
> >
> > +To pause at a failing test (e.g. ``ovsdb``) and inspect the system in
> failed
> > +state, follow the instructions given out by issuing following command
> to use
> > +various ovs-* utilities::
> > +
> > +$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
> > +
> >  To see a complete list of test options, run::
> >
> >  $ make check TESTSUITEFLAGS=--help
> > diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> > index 10593429d..e58b0a1a0 100644
> > --- a/tests/ovs-macros.at
> > +++ b/tests/ovs-macros.at
> > @@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
> >  # directory.
> >  ovs_init() {
> >  ovs_base=`pwd`
> > -trap '. "$ovs_base/cleanup"' 0
> > +trap ovs_on_exit 0
> >  : > cleanup
> >  ovs_setenv
> >  }
> >
> > +# Catch testsuite error condition and cleanup test environment by
> tearing down
> > +# all interfaces and processes spawned.
> > +# User has an option to leave the test environment in error state so
> that system
> > +# can be poked around to get more information. User can enable this
> option by setting
> > +# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to
> resume the
> > +# cleanup operation.
> > +ovs_pause() {
> > +$as_echo "="
> > +$as_echo "Set following environment variable to use various ovs
> utilities"
> > +$as_echo "export OVS_RUNDIR=$ovs_base"
> > +$as_echo "Press ENTER to continue: "
> > +read
> > +}
> > +
> > +ovs_on_exit () {
> > +if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then
> > +trap '' INT
> > +ovs_pause
> > +fi
> > +. "$ovs_base/cleanup"
> > +}
> > +
> >  # With no paramet

Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Ben Pfaff
On Mon, Jul 15, 2019 at 03:22:42PM -0400, Vasu Dasari wrote:
> Origins for this patch are captured at
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html.

...

Aaron provided helpful comments.  In addition:

> +ovs_pause() {
> +$as_echo "="
> +$as_echo "Set following environment variable to use various ovs 
> utilities"
> +$as_echo "export OVS_RUNDIR=$ovs_base"
> +$as_echo "Press ENTER to continue: "
> +read 
> +}

The above uses $as_echo, which is what the AS_ECHO([...]) macro expands
into.  It is better to use AS_ECHO itself, since in theory the
implementation could change in a later version of Autoconf.

Thanks,

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


Re: [ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Aaron Conole
Hi Vasu,

Vasu Dasari  writes:

> Origins for this patch are captured at
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html.
>
> Summarizing here, when a test fails, it would be good to pause test execution
> and let the developer poke around the system to see current status of system.
>
> As part of this patch, made a small tweaks to ovs-macros.at, so that when test
> suite fails, ovs_on_exit() function will be called. And in this function, a 
> check
> is made to see if an environment variable to OVS_PAUSE_TEST is set. If it is
> set, then test suite is paused and will continue to wait for user input
> Ctrl-D. Meanwhile user can poke around the system to see why test case has
> failed. Once done with investigation, user can press ctrl-d to cleanup the
> test suite.
>

>From this point  
   vv

> For example, to re-run test case 139:
>
> export OVS_PAUSE_TEST=1
> cd tests/system-userspace-testsuite.dir/139
> sudo -E ./run
>
> When error occurs, above command would display something like this:
> =
> Set environment variable to use various ovs utilities
> export 
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> Press ctrl-d to continue:
>
> =
> And from another window, one can execute ovs-xxx commands like:
> export 
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> $ ovs-ofctl dump-ports br0
> .
> .
>
> To be able to pause while performing `make check`, one can do:
> $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'
>

To here ^^

I would propose to put this information in the documentation.  A new
developer may not actually trawl through the commits, but this
information would help such a developer (or even a not-so-new developer,
like me).

> Signed-off-by: Vasu Dasari 
> Reviewed-by: Ben Pfaff 
> ---
> v0 -> v1:
> Discussion at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
> 1. Incorporated review commanets from Ben.
> 2. Changed topics/testing.rst to document this trick
> 3. Made this trick work via "make check" as well, by issuing command: 
> "OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"
>
> ---
>  Documentation/topics/testing.rst |  6 ++
>  tests/ovs-macros.at  | 24 +++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/topics/testing.rst 
> b/Documentation/topics/testing.rst
> index a8892e1c1..058b3986f 100644
> --- a/Documentation/topics/testing.rst
> +++ b/Documentation/topics/testing.rst
> @@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``, run::
>  
>  $ make check TESTSUITEFLAGS='-k ovsdb'
>  
> +To pause at a failing test (e.g. ``ovsdb``) and inspect the system in failed
> +state, follow the instructions given out by issuing following command to use 
> +various ovs-* utilities::
> +
> +$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
> +
>  To see a complete list of test options, run::
>  
>  $ make check TESTSUITEFLAGS=--help
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 10593429d..e58b0a1a0 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
>  # directory.
>  ovs_init() {
>  ovs_base=`pwd`
> -trap '. "$ovs_base/cleanup"' 0
> +trap ovs_on_exit 0
>  : > cleanup
>  ovs_setenv
>  }
>  
> +# Catch testsuite error condition and cleanup test environment by tearing 
> down
> +# all interfaces and processes spawned.
> +# User has an option to leave the test environment in error state so that 
> system
> +# can be poked around to get more information. User can enable this option 
> by setting
> +# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to 
> resume the
> +# cleanup operation.
> +ovs_pause() {
> +$as_echo "="
> +$as_echo "Set following environment variable to use various ovs 
> utilities"
> +$as_echo "export OVS_RUNDIR=$ovs_base"
> +$as_echo "Press ENTER to continue: "
> +read 
> +}
> +
> +ovs_on_exit () {
> +if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then
> +trap '' INT
> +ovs_pause
> +fi
> +. "$ovs_base/cleanup"
> +}
> +
>  # With no parameter or an empty parameter, sets the OVS_*DIR
>  # environment variables to point to $ovs_base, the base directory in
>  # which the test is running.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v1] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Origins for this patch are captured at
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html.

Summarizing here, when a test fails, it would be good to pause test execution
and let the developer poke around the system to see current status of system.

As part of this patch, made a small tweaks to ovs-macros.at, so that when test
suite fails, ovs_on_exit() function will be called. And in this function, a 
check
is made to see if an environment variable to OVS_PAUSE_TEST is set. If it is
set, then test suite is paused and will continue to wait for user input
Ctrl-D. Meanwhile user can poke around the system to see why test case has
failed. Once done with investigation, user can press ctrl-d to cleanup the
test suite.

For example, to re-run test case 139:

export OVS_PAUSE_TEST=1
cd tests/system-userspace-testsuite.dir/139
sudo -E ./run

When error occurs, above command would display something like this:
=
Set environment variable to use various ovs utilities
export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
Press ctrl-d to continue:

=
And from another window, one can execute ovs-xxx commands like:
export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
$ ovs-ofctl dump-ports br0
.
.

To be able to pause while performing `make check`, one can do:
$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'

Signed-off-by: Vasu Dasari 
Reviewed-by: Ben Pfaff 
---
v0 -> v1:
Discussion at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
1. Incorporated review commanets from Ben.
2. Changed topics/testing.rst to document this trick
3. Made this trick work via "make check" as well, by issuing command: 
"OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"

---
 Documentation/topics/testing.rst |  6 ++
 tests/ovs-macros.at  | 24 +++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst
index a8892e1c1..058b3986f 100644
--- a/Documentation/topics/testing.rst
+++ b/Documentation/topics/testing.rst
@@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``, run::
 
 $ make check TESTSUITEFLAGS='-k ovsdb'
 
+To pause at a failing test (e.g. ``ovsdb``) and inspect the system in failed
+state, follow the instructions given out by issuing following command to use 
+various ovs-* utilities::
+
+$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
+
 To see a complete list of test options, run::
 
 $ make check TESTSUITEFLAGS=--help
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 10593429d..e58b0a1a0 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
 # directory.
 ovs_init() {
 ovs_base=`pwd`
-trap '. "$ovs_base/cleanup"' 0
+trap ovs_on_exit 0
 : > cleanup
 ovs_setenv
 }
 
+# Catch testsuite error condition and cleanup test environment by tearing down
+# all interfaces and processes spawned.
+# User has an option to leave the test environment in error state so that 
system
+# can be poked around to get more information. User can enable this option by 
setting
+# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to resume 
the
+# cleanup operation.
+ovs_pause() {
+$as_echo "="
+$as_echo "Set following environment variable to use various ovs utilities"
+$as_echo "export OVS_RUNDIR=$ovs_base"
+$as_echo "Press ENTER to continue: "
+read 
+}
+
+ovs_on_exit () {
+if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then
+trap '' INT
+ovs_pause
+fi
+. "$ovs_base/cleanup"
+}
+
 # With no parameter or an empty parameter, sets the OVS_*DIR
 # environment variables to point to $ovs_base, the base directory in
 # which the test is running.
-- 
2.17.2 (Apple Git-113)

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


Re: [ovs-dev] [PATCH] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Please ignore this.

*Vasu Dasari*


On Mon, Jul 15, 2019 at 3:22 PM Vasu Dasari  wrote:

> Origins for this patch are captured at
> https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html.
>
> Summarizing here, when a test fails, it would be good to pause test
> execution
> and let the developer poke around the system to see current status of
> system.
>
> As part of this patch, made a small tweaks to ovs-macros.at, so that when
> test
> suite fails, ovs_on_exit() function will be called. And in this function,
> a check
> is made to see if an environment variable to OVS_PAUSE_TEST is set. If it
> is
> set, then test suite is paused and will continue to wait for user input
> Ctrl-D. Meanwhile user can poke around the system to see why test case has
> failed. Once done with investigation, user can press ctrl-d to cleanup the
> test suite.
>
> For example, to re-run test case 139:
>
> export OVS_PAUSE_TEST=1
> cd tests/system-userspace-testsuite.dir/139
> sudo -E ./run
>
> When error occurs, above command would display something like this:
> =
> Set environment variable to use various ovs utilities
> export
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> Press ctrl-d to continue:
>
> =
> And from another window, one can execute ovs-xxx commands like:
> export
> OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
> $ ovs-ofctl dump-ports br0
> .
> .
>
> To be able to pause while performing `make check`, one can do:
> $ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'
>
> Signed-off-by: Vasu Dasari 
> Reviewed-by: Ben Pfaff 
> ---
> v0 -> v1:
> Discussion at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
> 1. Incorporated review commanets from Ben.
> 2. Changed topics/testing.rst to document this trick
> 3. Made this trick work via "make check" as well, by issuing command:
> "OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"
>
> ---
>  Documentation/topics/testing.rst |  6 ++
>  tests/ovs-macros.at  | 24 +++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/topics/testing.rst
> b/Documentation/topics/testing.rst
> index a8892e1c1..058b3986f 100644
> --- a/Documentation/topics/testing.rst
> +++ b/Documentation/topics/testing.rst
> @@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``, run::
>
>  $ make check TESTSUITEFLAGS='-k ovsdb'
>
> +To pause at a failing test (e.g. ``ovsdb``) and inspect the system in
> failed
> +state, follow the instructions given out by issuing following command to
> use
> +various ovs-* utilities::
> +
> +$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
> +
>  To see a complete list of test options, run::
>
>  $ make check TESTSUITEFLAGS=--help
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 10593429d..e58b0a1a0 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
>  # directory.
>  ovs_init() {
>  ovs_base=`pwd`
> -trap '. "$ovs_base/cleanup"' 0
> +trap ovs_on_exit 0
>  : > cleanup
>  ovs_setenv
>  }
>
> +# Catch testsuite error condition and cleanup test environment by tearing
> down
> +# all interfaces and processes spawned.
> +# User has an option to leave the test environment in error state so that
> system
> +# can be poked around to get more information. User can enable this
> option by setting
> +# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to
> resume the
> +# cleanup operation.
> +ovs_pause() {
> +$as_echo "="
> +$as_echo "Set following environment variable to use various ovs
> utilities"
> +$as_echo "export OVS_RUNDIR=$ovs_base"
> +$as_echo "Press ENTER to continue: "
> +read
> +}
> +
> +ovs_on_exit () {
> +if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then
> +trap '' INT
> +ovs_pause
> +fi
> +. "$ovs_base/cleanup"
> +}
> +
>  # With no parameter or an empty parameter, sets the OVS_*DIR
>  # environment variables to point to $ovs_base, the base directory in
>  # which the test is running.
> --
> 2.17.2 (Apple Git-113)
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ovs-macros: An option to suspend test execution on error

2019-07-15 Thread Vasu Dasari
Origins for this patch are captured at
https://mail.openvswitch.org/pipermail/ovs-discuss/2019-June/048923.html.

Summarizing here, when a test fails, it would be good to pause test execution
and let the developer poke around the system to see current status of system.

As part of this patch, made a small tweaks to ovs-macros.at, so that when test
suite fails, ovs_on_exit() function will be called. And in this function, a 
check
is made to see if an environment variable to OVS_PAUSE_TEST is set. If it is
set, then test suite is paused and will continue to wait for user input
Ctrl-D. Meanwhile user can poke around the system to see why test case has
failed. Once done with investigation, user can press ctrl-d to cleanup the
test suite.

For example, to re-run test case 139:

export OVS_PAUSE_TEST=1
cd tests/system-userspace-testsuite.dir/139
sudo -E ./run

When error occurs, above command would display something like this:
=
Set environment variable to use various ovs utilities
export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
Press ctrl-d to continue:

=
And from another window, one can execute ovs-xxx commands like:
export 
OVS_RUNDIR=/opt/vdasari/Developer/ovs/_build-gcc/tests/system-userspace-testsuite.dir/139
$ ovs-ofctl dump-ports br0
.
.

To be able to pause while performing `make check`, one can do:
$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'

Signed-off-by: Vasu Dasari 
Reviewed-by: Ben Pfaff 
---
v0 -> v1:
Discussion at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360589.html
1. Incorporated review commanets from Ben.
2. Changed topics/testing.rst to document this trick
3. Made this trick work via "make check" as well, by issuing command: 
"OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v'"

---
 Documentation/topics/testing.rst |  6 ++
 tests/ovs-macros.at  | 24 +++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst
index a8892e1c1..058b3986f 100644
--- a/Documentation/topics/testing.rst
+++ b/Documentation/topics/testing.rst
@@ -74,6 +74,12 @@ To run tests matching a keyword, e.g. ``ovsdb``, run::
 
 $ make check TESTSUITEFLAGS='-k ovsdb'
 
+To pause at a failing test (e.g. ``ovsdb``) and inspect the system in failed
+state, follow the instructions given out by issuing following command to use 
+various ovs-* utilities::
+
+$ OVS_PAUSE_TEST=1 make check TESTSUITEFLAGS='-v -k ovsdb'
+
 To see a complete list of test options, run::
 
 $ make check TESTSUITEFLAGS=--help
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 10593429d..e58b0a1a0 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -35,11 +35,33 @@ m4_divert_push([PREPARE_TESTS])
 # directory.
 ovs_init() {
 ovs_base=`pwd`
-trap '. "$ovs_base/cleanup"' 0
+trap ovs_on_exit 0
 : > cleanup
 ovs_setenv
 }
 
+# Catch testsuite error condition and cleanup test environment by tearing down
+# all interfaces and processes spawned.
+# User has an option to leave the test environment in error state so that 
system
+# can be poked around to get more information. User can enable this option by 
setting
+# environment variable OVS_PAUSE_TEST=1. User needs to press CTRL-D to resume 
the
+# cleanup operation.
+ovs_pause() {
+$as_echo "="
+$as_echo "Set following environment variable to use various ovs utilities"
+$as_echo "export OVS_RUNDIR=$ovs_base"
+$as_echo "Press ENTER to continue: "
+read 
+}
+
+ovs_on_exit () {
+if [ ! -z "${OVS_PAUSE_TEST}" ] && [ -z $at_verbose ]; then
+trap '' INT
+ovs_pause
+fi
+. "$ovs_base/cleanup"
+}
+
 # With no parameter or an empty parameter, sets the OVS_*DIR
 # environment variables to point to $ovs_base, the base directory in
 # which the test is running.
-- 
2.17.2 (Apple Git-113)

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


Re: [ovs-dev] [PATCHv16 1/2] ovs-thread: Add pthread spin lock support.

2019-07-15 Thread Ben Pfaff
On Mon, Jul 15, 2019 at 05:28:44PM +0300, Ilya Maximets wrote:
> On 13.07.2019 2:50, William Tu wrote:
> > The patch adds the basic spin lock functions:
> > ovs_spin_{lock, try_lock, unlock, init, destroy}.
> > OSX does not support pthread spin lock, so make it
> > linux only.
> 
> IIUC, pthread spinlock requires some specific glibc verions (>= 2.2)
> so it could be not supported even on Linux. Instead of checking
> _POSIX_C_SOURCE version, I think it's better to just check for
> pthread_spin_lock function in configure script with AC_CHECK_FUNC and
> check for the resulted macro.
> Additionally we could check for this macro while checking AF_XDP support
> to not enable it if we have no spinlocks.

glibc 2.2 was released in 2000.  I don't think that it is worth worrying
that a user might have such an ancient version of glibc.

However, I think that feature checks are generally better, so I support
the change.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next] net: openvswitch: do not update max_headroom if new headroom is equal to old headroom

2019-07-15 Thread Pravin Shelar
On Fri, Jul 5, 2019 at 9:08 AM Taehee Yoo  wrote:
>
> When a vport is deleted, the maximum headroom size would be changed.
> If the vport which has the largest headroom is deleted,
> the new max_headroom would be set.
> But, if the new headroom size is equal to the old headroom size,
> updating routine is unnecessary.
>
> Signed-off-by: Taehee Yoo 

Sorry for late reply. This patch looks good to me too.

I am curious about reason to avoid adjustment to headroom. why are you
trying to avoid unnecessary changes to headroom.

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


Re: [ovs-dev] [PATCH] userspace: Fix nsh test failures.

2019-07-15 Thread William Tu
On Mon, Jul 15, 2019 at 7:12 AM Ilya Maximets  wrote:
>
> On 13.07.2019 17:25, William Tu wrote:
> > When doing
> >  $ make check-kmod TESTSUITEFLAGS='-k nsh'
> >
> > every test passes, but when doing
> >  $ make check-system-userspace TESTSUITEFLAGS='-k nsh'
> >
> > All test fails due to
> >  odp_util(revalidator7)|WARN| OVS_NSH_KEY_ATTR_MD1 present but \
> >  declared mdtype 0 is not 1 (NSH_M_TYPE1)
> >  odp_util(revalidator7)|WARN| the flow mask in error is: \
> >  
> > skb_priority(0),tunnel(ttl=0,flags(0)),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),recirc_id(0x),dp_hash(0),in_port(4294967295),packet_type(ns=65535,id=0x),eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),eth_type(0x),nsh(flags=0ttl=0,mdtype=0,np=255,spi=0xff,si=255),
> >  for the following flow key: 
> > in_port=2,vlan_tci=0x,dl_src=f2:ff:00:00:00:01,dl_dst=f2:ff:00:00:00:02,dl_type=0x894f,nsh_flags=0,nsh_ttl=8,nsh_mdtype=1
> >
> > The patch adds '-n' option to tcpdump, to avoid the delay of
> > coverting address add more sleep time after the tcpdump starts
> > to capture packets. But I don't know why more sleep time makes
> > the revalidator warning disappears.
>
> Hi.
> Thanks for working on this!
>
> I re-checked the code and found a bug that causes revalidator failures:
> https://patchwork.ozlabs.org/patch/1132013/
>

Hi Ilya,

Thanks for finding out the root cause.
I tested and acked to the above patch.

> So, the additional sleep seems not needed with above patch applied.
>
> Regarding the '-n', I think it's a good enhancement.
> I'd suggest passing '-n -xx' directly to the first tcpdump like this:
> tcpdump -n -xx -U -i ovs-p1 > ovs-p1.pcap &
>
> And use simple checking with 'cat' and 'OVS_WAIT_UNTIL', like all other
> tests does:
> OVS_WAIT_UNTIL([cat ovs-p1.pcap 2>&1 | egrep "0x: *f2ff * *0002 
> *f2ff * *0001 *894f *0fc6" 2>&1 1>/dev/null])
>

OK, I will send out another patch
Regards,
William

> This way tests should be more reliable.
>
> Best regards, Ilya Maximets.
>
> >
> > Signed-off-by: William Tu 
> > --->  tests/system-traffic.at | 64 
> > -
> >  1 file changed, 32 insertions(+), 32 deletions(-)
> >
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 8ea450887076..84e73b82bd4f 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -5873,7 +5873,7 @@ AT_CHECK([ovs-ofctl -Oopenflow13 add-flow br0 
> > "table=0,priority=100,in_port=ovs-
> >
> >  rm ovs-p1.pcap
> >  tcpdump -U -i ovs-p1 -w ovs-p1.pcap &
> > -sleep 1
> > +sleep 2
> >
> >  dnl The hex dump is a TCP syn packet. pkt=eth/ip/tcp
> >  dnl The packet is sent from p0(at_ns0) interface directed to
> > @@ -5883,12 +5883,12 @@ NS_CHECK_EXEC([at_ns0], [$PYTHON $srcdir/sendpkt.py 
> > p0 f2 00 00 00 00 02 f2 00 0
> >  sleep 1
> >
> >  dnl Check the expected nsh encapsulated packet on the egress interface
> > -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x: *f2ff * 
> > *0002 *f2ff * *0001 *894f *0fc6" 2>&1 1>/dev/null])
> > -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0010: *0103 *0012 
> > *34ff *1122 *3344 * * *" 2>&1 1>/dev/null])
> > -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0020: * * 
> > * *f200 * *0002 *f200 *" 2>&1 1>/dev/null])
> > -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0030: *0001 *0800 
> > *4500 *0028 *0001 * *4006 *b013" 2>&1 1>/dev/null])
> > -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0040: *c0a8 *000a 
> > *0a00 *000a *0400 *0800 * *00c8" 2>&1 1>/dev/null])
> > -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0050: * * 
> > *5002 *2000 *b85e *" 2>&1 1>/dev/null])
> > +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x: *f2ff * 
> > *0002 *f2ff * *0001 *894f *0fc6" 2>&1 1>/dev/null])
> > +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0010: *0103 *0012 
> > *34ff *1122 *3344 * * *" 2>&1 1>/dev/null])
> > +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0020: * * 
> > * *f200 * *0002 *f200 *" 2>&1 1>/dev/null])
> > +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0030: *0001 *0800 
> > *4500 *0028 *0001 * *4006 *b013" 2>&1 1>/dev/null])
> > +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0040: *c0a8 *000a 
> > *0a00 *000a *0400 *0800 * *00c8" 2>&1 1>/dev/null])
> > +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0050: * * 
> > *5002 *2000 *b85e *" 2>&1 1>/dev/null])
> >
> >
> >  OVS_TRAFFIC_VSWITCHD_STOP
> > @@ -5908,7 +5908,7 @@ AT_CHECK([ovs-ofctl -Oopenflow13 add-flow br0 
> > "table=0,priority=100,in_port=ovs-
> >
> >  rm ovs-p1.pcap
> >  tcpdump -U -i ovs-p1 -w ovs-p1.pcap &
> > -sleep 1
> > +sleep 2
> >
> >  dnl The hex dump is NSH packet with TCP syn payload. pkt=eth/nsh/eth/ip/tcp
> >  dnl The packet is sent from p0(at_ns0) interface directed to
> > @@ -5918,

Re: [ovs-dev] [PATCH] odp-util: Fix NSH mask parsing.

2019-07-15 Thread William Tu
On Mon, Jul 15, 2019 at 6:58 AM Ilya Maximets  wrote:
>
> 'odp_flow_key_to_flow__' is used to parse both keys and masks.
> However, 'odp_nsh_key_from_attr' expects only 'key' as an argument
> and fails to parse masks with ODP_FIT_ERROR which causes userspace
> system tests failures:
>
>  # make check-system-userspace TESTSUITEFLAGS='-k nsh'
>
>  |odp_util(revalidator)|WARN|
>  OVS_NSH_KEY_ATTR_MD1 present but declared mdtype 0 is not 1 (NSH_M_TYPE1)
>
>  |odp_util(revalidator)|WARN|
>  the flow mask in error is:
>  <...> nsh(flags=0ttl=0,mdtype=0,np=255,spi=0x0,si=0),
>  for the following flow key:
>  <...> nsh_ttl=8,nsh_mdtype=1,nsh_np=3,nsh_spi=0x64,nsh_si=3,
>  nsh_c1=0x1020304,nsh_c2=0x5060708,nsh_c3=0x90a0b0c,nsh_c4=0xd0e0f10
>  <...>
>
> Fix that by passing the additional argument 'is_mask' to make it be
> like all other parsing functions.
>
> Additionally fixed missing comma in the 'format_nsh_key'.
>
> CC: Yi Yang 
> Fixes: f59cb331c481 ("nsh: rework NSH netlink keys and actions")
> Signed-off-by: Ilya Maximets 
> ---

LGTM, thanks for the fix.
Acked-by: William Tu 

>  lib/odp-util.c | 33 -
>  1 file changed, 24 insertions(+), 9 deletions(-)
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] Protocolo y etiqueta corporativa

2019-07-15 Thread Russel Cohuo
22 de Agosto | Horario de 10:00 a 14:00 hrs.  |  (hora del centro de México) 
 - Protocolo y etiqueta corporativa - 

¿De qué hablaremos?

A concientizar sobre la importancia de la imagen y el protocolo profesional
en el mundo empresarial y cómo utilizar estas herramientas para influir en la 
percepción del
cliente para posicionar a nuestra empresa de manera positiva.

¿Qué aprenderás?:

- Conocerá el impacto de la imagen, los protocolos elementales para un buen 
desarrollo empresarial y los tratamientos empresariales en diferentes 
plataformas.
- Identificará los estímulos necesarios para cumplir su objetivo profesional y 
recibirá guías para mejorar la vestimenta profesional.
- Aprenderá cómo lograr una imagen coherente y adecuada, así como reconocer la 
importancia de la comunicación no verbal.


Solicita información respondiendo a este correo con la palabra ETIQUETA, junto 
con los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Dirigido a: Profesionistas, emprendedores, personal encargado de atención al 
cliente y
personas que deseen mejorar, optimizar y elevar su imagen profesional.

Números de Atención: 

(045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder con la palabra baja o 
enviar un correo a ba...@innovalearn.net
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v11 5/5] dpif-netdev: Add specialized generic scalar functions

2019-07-15 Thread 0-day Robot
Bleep bloop.  Greetings Harry van Haaren, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Inappropriate bracing around statement
#112 FILE: lib/dpif-netdev-lookup-generic.c:266:
if (!f && u0_bits == U0 && u1_bits == U1) {   \

Lines checked: 178, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v11 2/5] dpif-netdev: Move dpcls lookup structures to .h

2019-07-15 Thread 0-day Robot
Bleep bloop.  Greetings Harry van Haaren, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Improper whitespace around control block
#139 FILE: lib/dpif-netdev-private.h:101:
#define NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP(VALUE, KEY, FLOWMAP)   \

ERROR: Inappropriate bracing around statement
#140 FILE: lib/dpif-netdev-private.h:102:
MINIFLOW_FOR_EACH_IN_FLOWMAP (VALUE, &(KEY)->mf, FLOWMAP)

Lines checked: 281, Warnings: 0, Errors: 2


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v11 5/5] dpif-netdev: Add specialized generic scalar functions

2019-07-15 Thread Harry van Haaren
This commit adds a number of specialized functions, that handle
common miniflow fingerprints. This enables compiler optimization,
resulting in higher performance. Below a quick description of
how this optimization actually works;

"Specialized functions" are "instances" of the generic implementation,
but the compiler is given extra context when compiling. In the case of
iterating miniflow datastructures, the most interesting value to enable
compile time optimizations is the loop trip count per unit.

In order to create a specialized function, there is a generic implementation,
which uses a for() loop without the compiler knowing the loop trip count at
compile time. The loop trip count is passed in as an argument to the function:

uint32_t miniflow_impl_generic(struct miniflow *mf, uint32_t loop_count)
{
for(uint32_t i = 0; i < loop_count; i++)
// do work
}

In order to "specialize" the function, we call the generic implementation
with hard-coded numbers - these are compile time constants!

uint32_t miniflow_impl_loop5(struct miniflow *mf, uint32_t loop_count)
{
// use hard coded constant for compile-time constant-propogation
return miniflow_impl_generic(mf, 5);
}

Given the compiler is aware of the loop trip count at compile time,
it can perform an optimization known as "constant propogation". Combined
with inlining of the miniflow_impl_generic() function, the compiler is
now enabled to *compile time* unroll the loop 5x, and produce "flat" code.

The last step to using the specialized functions is to utilize a
function-pointer to choose the specialized (or generic) implementation.
The selection of the function pointer is performed at subtable creation
time, when miniflow fingerprint of the subtable is known. This technique
is known as "multiple dispatch" in some literature, as it uses multiple
items of information (miniflow bit counts) to select the dispatch function.

By pointing the function pointer at the optimized implementation, OvS
benefits from the compile time optimizations at runtime.

Signed-off-by: Harry van Haaren 
Tested-by: Malvika Gupta 

---

v11:
- Use MACROs to declare and check optimized functions (Ilya)
- Use captial letter for commit message (Ilya)
- Rebase onto latest patchset changes
- Added NEWS entry for data-path subtable specialization (Ian/Harry)
- Checkpatch notes an "incorrect bracketing" in the MACROs, however I
  didn't find a solution that it does like.

v10:
- Rebase changes from previous patches.
- Remove "restrict" keyword as windows CI failed, see here for details:
  https://ci.appveyor.com/project/istokes/ovs-q8fvv/builds/24398228

v8:
- Rework to use blocks_cache from the dpcls instance, to avoid variable
  lenght arrays in the data-path.
---
 NEWS |  4 +++
 lib/dpif-netdev-lookup-generic.c | 51 
 lib/dpif-netdev-private.h|  8 +
 lib/dpif-netdev.c|  9 --
 4 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 81130e667..4cfffb1bc 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,10 @@ Post-v2.11.0
  * 'ovs-appctl exit' now implies cleanup of non-internal ports in userspace
datapath regardless of '--cleanup' option. Use '--cleanup' to remove
internal ports too.
+ * Datapath classifer code refactored to enable function pointers to select
+   the lookup implementation at runtime. This enables specialization of
+   specific subtables based on the miniflow attributes, enhancing the
+   performance of the subtable search.
- OVSDB:
  * OVSDB clients can now resynchronize with clustered servers much more
quickly after a brief disconnection, saving bandwidth and CPU time.
diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-generic.c
index abd166fc3..259c36645 100644
--- a/lib/dpif-netdev-lookup-generic.c
+++ b/lib/dpif-netdev-lookup-generic.c
@@ -233,7 +233,58 @@ dpcls_subtable_lookup_generic(struct dpcls_subtable 
*subtable,
   const struct netdev_flow_key *keys[],
   struct dpcls_rule **rules)
 {
+/* Here the runtime subtable->mf_bits counts are used, which forces the
+ * compiler to iterate normal for() loops. Due to this limitation in the
+ * compilers available optimizations, this function has lower performance
+ * than the below specialized functions.
+ */
 return lookup_generic_impl(subtable, blocks_scratch, keys_map, keys, rules,
subtable->mf_bits_set_unit0,
subtable->mf_bits_set_unit1);
 }
+
+/* Expand out specialized functions with U0 and U1 bit attributes */
+#define DECLARE_OPTIMIZED_LOOKUP_FUNCTION(U0, U1) \
+static uint32_t   \
+dpcls_subtable_lookup_mf_u0w##U0##_u1w##U1(   \
+ 

[ovs-dev] [PATCH v11 4/5] dpif-netdev: Refactor generic implementation

2019-07-15 Thread Harry van Haaren
This commit refactors the generic implementation. The
goal of this refactor is to simplify the code to enable
"specialization" of the functions at compile time.

Given compile-time optimizations, the compiler is able
to unroll loops, and create optimized code sequences due
to compile time knowledge of loop-trip counts.

In order to enable these compiler optimizations, we must
refactor the code to pass the loop-trip counts to functions
as compile time constants.

This patch allows the number of miniflow-bits set per "unit"
in the miniflow to be passed around as a function argument.

Note that this patch does NOT yet take advantage of doing so,
this is only a refactor to enable it in the next patches.

Signed-off-by: Harry van Haaren 
Tested-by: Malvika Gupta 

---

v11:
- Rebased to previous changes
- Fix typo in commit message (Ian)
- Fix variable declaration spacing (Ian)
- Remove function names from comments (Ian)
- Replace magic 8 with sizeof(uint64_t) (Ian)
- Captialize and end comments with a stop. (Ian/Ilya)
- Add build time assert to validate FLOWMAP_UNITS (Ilya)
- Add note on ALWAYS_INLINE operation
- Add space after ULLONG_FOR_EACH_1 (Ilya)
- Use hash_add_words64() instead of rolling own loop (Ilya)
Note that hash_words64_inline() calls hash_finish() with an
fixed value, so it was not the right hash function for this
usage. Used hash_add_words64() and manual hash_finish() to
re-use as much of hashing code as we can.

v10:
- Rebase updates from previous patches
- Fix whitespace indentation of func params
- Removed restrict keyword, Windows CI failing when it is used (Ian)
- Fix integer 0 used to set NULL pointer (Ilya)
- Postpone free() call on cls->blocks_scratch (Ilya)
- Fix indentation of a function

v9:
- Use count_1bits in favour of __builtin_popcount (Ilya)
- Use ALWAYS_INLINE instead of __attribute__ synatx (Ilya)

v8:
- Rework block_cache and mf_masks to avoid variable-lenght array
  due to compiler issues. Provisioning for worst case is not a
  good solution due to magnitue of over-provisioning required.
- Rework netdev_flatten function removing unused parameter
---
 lib/dpif-netdev-lookup-generic.c | 214 +--
 lib/dpif-netdev-private.h|   8 +-
 lib/dpif-netdev.c|  77 ++-
 3 files changed, 258 insertions(+), 41 deletions(-)

diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-generic.c
index 833abf54f..abd166fc3 100644
--- a/lib/dpif-netdev-lookup-generic.c
+++ b/lib/dpif-netdev-lookup-generic.c
@@ -30,68 +30,210 @@
 #include "packets.h"
 #include "pvector.h"
 
-/* Returns a hash value for the bits of 'key' where there are 1-bits in
- * 'mask'. */
-static inline uint32_t
-netdev_flow_key_hash_in_mask(const struct netdev_flow_key *key,
- const struct netdev_flow_key *mask)
+VLOG_DEFINE_THIS_MODULE(dpif_lookup_generic);
+
+/* Lookup functions below depends on the internal structure of flowmap. */
+BUILD_ASSERT_DECL(FLOWMAP_UNITS == 2);
+
+/* Given a packet, table and mf_masks, this function iterates over each bit
+ * set in the subtable, and calculates the appropriate metadata to store in the
+ * blocks_scratch[].
+ *
+ * The results of the blocks_scratch[] can be used for hashing, and later for
+ * verification of if a rule matches the given packet.
+ */
+static inline void
+netdev_flow_key_flatten_unit(const uint64_t *pkt_blocks,
+ const uint64_t *tbl_blocks,
+ const uint64_t *mf_masks,
+ uint64_t *blocks_scratch,
+ const uint64_t pkt_mf_bits,
+ const uint32_t count)
 {
-const uint64_t *p = miniflow_get_values(&mask->mf);
-uint32_t hash = 0;
-uint64_t value;
+uint32_t i;
+
+for (i = 0; i < count; i++) {
+uint64_t mf_mask = mf_masks[i];
+/* Calculate the block index for the packet metadata. */
+uint64_t idx_bits = mf_mask & pkt_mf_bits;
+const uint32_t pkt_idx = count_1bits(idx_bits);
+
+/* Check if the packet has the subtable miniflow bit set. If yes, the
+ * block at the above pkt_idx will be stored, otherwise it is masked
+ * out to be zero.
+ */
+uint64_t pkt_has_mf_bit = (mf_mask + 1) & pkt_mf_bits;
+uint64_t no_bit = ((!pkt_has_mf_bit) > 0) - 1;
 
-NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP (value, key, mask->mf.map) {
-hash = hash_add64(hash, value & *p);
-p++;
+/* Mask packet block by table block, and mask to zero if packet
+ * doesn't actually contain this block of metadata.
+ */
+blocks_scratch[i] = pkt_blocks[pkt_idx] & tbl_blocks[i] & no_bit;
 }
+}
+
+/* This function takes a packet, and subtable and writes an array of uint64_t
+ * blocks. The blocks contain the metadata that the subtable matches on, in
+ * the same order as the subtable, allowing linear iteration over the bl

[ovs-dev] [PATCH v11 3/5] dpif-netdev: Split out generic lookup function

2019-07-15 Thread Harry van Haaren
This commit splits the generic hash-lookup-verify function to
its own file, for cleaner seperation between optimized versions.

Signed-off-by: Harry van Haaren 
Tested-by: Malvika Gupta 

---

v11:
- Rebase fixups from previous patches
- Spacing around ULLONG_FOR_EACH_1 (Ilya)

v10:
- Rebase fixups from previous patch changes

v6:
- Fixup some checkpatch warnings on whitespace with MACROs (Ilya)
- Other MACROs function incorrectly when checkpatch is happy, so using
  the functional version without space after the ( character.
  This prints a checkpatch warning, but I see no way to fix it.
---
 lib/automake.mk  |  1 +
 lib/dpif-netdev-lookup-generic.c | 97 
 lib/dpif-netdev.c| 69 +--
 3 files changed, 99 insertions(+), 68 deletions(-)
 create mode 100644 lib/dpif-netdev-lookup-generic.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 6f216efe0..29d3458da 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -78,6 +78,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dp-packet.h \
lib/dp-packet.c \
lib/dpdk.h \
+   lib/dpif-netdev-lookup-generic.c \
lib/dpif-netdev.c \
lib/dpif-netdev.h \
lib/dpif-netdev-private.h \
diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-generic.c
new file mode 100644
index 0..833abf54f
--- /dev/null
+++ b/lib/dpif-netdev-lookup-generic.c
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2016, 2017 Nicira, Inc.
+ * Copyright (c) 2019 Intel Corporation.
+ *
+ * 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.
+ */
+
+#include 
+#include "dpif-netdev.h"
+#include "dpif-netdev-private.h"
+
+#include "bitmap.h"
+#include "cmap.h"
+
+#include "dp-packet.h"
+#include "dpif.h"
+#include "dpif-netdev-perf.h"
+#include "dpif-provider.h"
+#include "flow.h"
+#include "packets.h"
+#include "pvector.h"
+
+/* Returns a hash value for the bits of 'key' where there are 1-bits in
+ * 'mask'. */
+static inline uint32_t
+netdev_flow_key_hash_in_mask(const struct netdev_flow_key *key,
+ const struct netdev_flow_key *mask)
+{
+const uint64_t *p = miniflow_get_values(&mask->mf);
+uint32_t hash = 0;
+uint64_t value;
+
+NETDEV_FLOW_KEY_FOR_EACH_IN_FLOWMAP (value, key, mask->mf.map) {
+hash = hash_add64(hash, value & *p);
+p++;
+}
+
+return hash_finish(hash, (p - miniflow_get_values(&mask->mf)) * 8);
+}
+
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+  uint32_t keys_map,
+  const struct netdev_flow_key *keys[],
+  struct dpcls_rule **rules)
+{
+int i;
+uint32_t found_map;
+
+/* Compute hashes for the remaining keys.  Each search-key is
+ * masked with the subtable's mask to avoid hashing the wildcarded
+ * bits. */
+uint32_t hashes[NETDEV_MAX_BURST];
+ULLONG_FOR_EACH_1 (i, keys_map) {
+hashes[i] = netdev_flow_key_hash_in_mask(keys[i], &subtable->mask);
+}
+
+/* Lookup. */
+const struct cmap_node *nodes[NETDEV_MAX_BURST];
+found_map = cmap_find_batch(&subtable->rules, keys_map, hashes, nodes);
+
+/* Check results.  When the i-th bit of found_map is set, it means
+ * that a set of nodes with a matching hash value was found for the
+ * i-th search-key.  Due to possible hash collisions we need to check
+ * which of the found rules, if any, really matches our masked
+ * search-key. */
+ULLONG_FOR_EACH_1 (i, found_map) {
+struct dpcls_rule *rule;
+
+CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
+if (OVS_LIKELY(dpcls_rule_matches_key(rule, keys[i]))) {
+rules[i] = rule;
+/* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
+ * within one second optimization interval. */
+subtable->hit_cnt++;
+goto next;
+}
+}
+/* None of the found rules was a match.  Reset the i-th bit to
+ * keep searching this key in the next subtable. */
+ULLONG_SET0(found_map, i);  /* Did not match. */
+next:
+; /* Keep Sparse happy. */
+}
+
+return found_map;
+}
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 749a478a8..b42ca35e3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -252,8 +252,7 @@ static

[ovs-dev] [PATCH v11 2/5] dpif-netdev: Move dpcls lookup structures to .h

2019-07-15 Thread Harry van Haaren
This commit moves some data-structures to be available
in the dpif-netdev-private.h header. This allows specific
implementations of the subtable lookup function to include
just that header file, and not require that the code exists
in dpif-netdev.c

Signed-off-by: Harry van Haaren 
Tested-by: Malvika Gupta 

---

v11:
- Rebase to latest master
- Split netdev private header to own file (Ilya)

v10:
- Rebase updates from previous patch in code that moved.
- Move cmap.h include into alphabetical order (Ian)
- Fix comment and typo (Ian)
- Restructure function typedef to fit in 80 chars

v6:
- Fix double * in code comments (Eelco)
- Reword comment on lookup_func for clarity (Harry)
- Rebase fixups
---
 lib/automake.mk   |   1 +
 lib/dpif-netdev-private.h | 119 ++
 lib/dpif-netdev.c |  68 +-
 3 files changed, 123 insertions(+), 65 deletions(-)
 create mode 100644 lib/dpif-netdev-private.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 1b89cac8c..6f216efe0 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -80,6 +80,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpdk.h \
lib/dpif-netdev.c \
lib/dpif-netdev.h \
+   lib/dpif-netdev-private.h \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
lib/dpif-provider.h \
diff --git a/lib/dpif-netdev-private.h b/lib/dpif-netdev-private.h
new file mode 100644
index 0..b235e23c8
--- /dev/null
+++ b/lib/dpif-netdev-private.h
@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2019 Intel Corperation.
+ *
+ * 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.
+ */
+
+#ifndef DPIF_NETDEV_PRIVATE_H
+#define DPIF_NETDEV_PRIVATE_H 1
+
+#include 
+#include 
+
+#include "dpif.h"
+#include "cmap.h"
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
+/* Forward declaration for lookup_func typedef. */
+struct dpcls_subtable;
+struct dpcls_rule;
+
+/* Must be public as it is instantiated in subtable struct below. */
+struct netdev_flow_key {
+uint32_t hash;   /* Hash function differs for different users. */
+uint32_t len;/* Length of the following miniflow (incl. map). */
+struct miniflow mf;
+uint64_t buf[FLOW_MAX_PACKET_U64S];
+};
+
+/* A rule to be inserted to the classifier. */
+struct dpcls_rule {
+struct cmap_node cmap_node;   /* Within struct dpcls_subtable 'rules'. */
+struct netdev_flow_key *mask; /* Subtable's mask. */
+struct netdev_flow_key flow;  /* Matching key. */
+/* 'flow' must be the last field, additional space is allocated here. */
+};
+
+/* Lookup function for a subtable in the dpcls. This function is called
+ * by each subtable with an array of packets, and a bitmask of packets to
+ * perform the lookup on. Using a function pointer gives flexibility to
+ * optimize the lookup function based on subtable properties and the
+ * CPU instruction set available at runtime.
+ */
+typedef
+uint32_t (*dpcls_subtable_lookup_func)(struct dpcls_subtable *subtable,
+   uint32_t keys_map,
+   const struct netdev_flow_key *keys[],
+   struct dpcls_rule **rules);
+
+/* Prototype for generic lookup func, using same code path as before. */
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+  uint32_t keys_map,
+  const struct netdev_flow_key *keys[],
+  struct dpcls_rule **rules);
+
+/* A set of rules that all have the same fields wildcarded. */
+struct dpcls_subtable {
+/* The fields are only used by writers. */
+struct cmap_node cmap_node OVS_GUARDED; /* Within dpcls 'subtables_map'. */
+
+/* These fields are accessed by readers. */
+struct cmap rules;   /* Contains "struct dpcls_rule"s. */
+uint32_t hit_cnt;/* Number of match hits in subtable in current
+optimization interval. */
+
+/* Miniflow fingerprint that the subtable matches on. The miniflow "bits"
+ * are used to select the actual dpcls lookup implementation at subtable
+ * creation time.
+ */
+uint8_t mf_bits_set_unit0;
+uint8_t mf_bits_set_unit1;
+
+/* the lookup function to use for this subtable. If there is a known
+ * property of the subtable (eg: only 3 bits of min

[ovs-dev] [PATCH v11 1/5] dpif-netdev: Implement function pointers/subtable

2019-07-15 Thread Harry van Haaren
This allows plugging-in of different subtable hash-lookup-verify
routines, and allows special casing of those functions based on
known context (eg: # of bits set) of the specific subtable.

Signed-off-by: Harry van Haaren 
Tested-by: Malvika Gupta 

---

v11:
- Rebased to latest master
- Added space to ULLONG_FOR_EACH_1 (Ilya)
- Use capital letter in commit message (Ilya)

v10:
- Fix capitalization of comments, and punctuation. (Ian)
- Variable declarations up top before use (Ian)
- Fix alignment of function parameters, had to newline after typedef (Ian)
- Some mailing-list questions relpied to on-list (Ian)

v9:
- Use count_1bits in favour of __builtin_popcount (Ilya)

v6:
- Implement subtable effort per packet "lookups_match" counter  (Ilya)
- Remove double newline (Eelco)
- Remove double * before comments (Eelco)
- Reword comments in dpcls_lookup() for clarity (Harry)
---
 lib/dpif-netdev.c | 138 --
 1 file changed, 96 insertions(+), 42 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6b99a3c44..123f04577 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7683,6 +7683,28 @@ dpif_dummy_register(enum dummy_level level)
 
 /* Datapath Classifier. */
 
+/* Forward declaration for lookup_func typedef. */
+struct dpcls_subtable;
+
+/* Lookup function for a subtable in the dpcls. This function is called
+ * by each subtable with an array of packets, and a bitmask of packets to
+ * perform the lookup on. Using a function pointer gives flexibility to
+ * optimize the lookup function based on subtable properties and the
+ * CPU instruction set available at runtime.
+ */
+typedef
+uint32_t (*dpcls_subtable_lookup_func)(struct dpcls_subtable *subtable,
+   uint32_t keys_map,
+   const struct netdev_flow_key *keys[],
+   struct dpcls_rule **rules);
+
+/* Prototype for generic lookup func, using same code path as before. */
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+  uint32_t keys_map,
+  const struct netdev_flow_key *keys[],
+  struct dpcls_rule **rules);
+
 /* A set of rules that all have the same fields wildcarded. */
 struct dpcls_subtable {
 /* The fields are only used by writers. */
@@ -7692,6 +7714,13 @@ struct dpcls_subtable {
 struct cmap rules;   /* Contains "struct dpcls_rule"s. */
 uint32_t hit_cnt;/* Number of match hits in subtable in current
 optimization interval. */
+
+/* The lookup function to use for this subtable. If there is a known
+ * property of the subtable (eg: only 3 bits of miniflow metadata is
+ * used for the lookup) then this can point at an optimized version of
+ * the lookup function for this particular subtable. */
+dpcls_subtable_lookup_func lookup_func;
+
 struct netdev_flow_key mask; /* Wildcards for fields (const). */
 /* 'mask' must be the last field, additional space is allocated here. */
 };
@@ -7751,6 +7780,10 @@ dpcls_create_subtable(struct dpcls *cls, const struct 
netdev_flow_key *mask)
 cmap_init(&subtable->rules);
 subtable->hit_cnt = 0;
 netdev_flow_key_clone(&subtable->mask, mask);
+
+/* Decide which hash/lookup/verify function to use. */
+subtable->lookup_func = dpcls_subtable_lookup_generic;
+
 cmap_insert(&cls->subtables_map, &subtable->cmap_node, mask->hash);
 /* Add the new subtable at the end of the pvector (with no hits yet) */
 pvector_insert(&cls->subtables, subtable, 0);
@@ -7911,6 +7944,55 @@ dpcls_rule_matches_key(const struct dpcls_rule *rule,
 return true;
 }
 
+uint32_t
+dpcls_subtable_lookup_generic(struct dpcls_subtable *subtable,
+  uint32_t keys_map,
+  const struct netdev_flow_key *keys[],
+  struct dpcls_rule **rules)
+{
+int i;
+uint32_t found_map;
+
+/* Compute hashes for the remaining keys.  Each search-key is
+ * masked with the subtable's mask to avoid hashing the wildcarded
+ * bits. */
+uint32_t hashes[NETDEV_MAX_BURST];
+ULLONG_FOR_EACH_1 (i, keys_map) {
+hashes[i] = netdev_flow_key_hash_in_mask(keys[i],
+ &subtable->mask);
+}
+
+/* Lookup. */
+const struct cmap_node *nodes[NETDEV_MAX_BURST];
+found_map = cmap_find_batch(&subtable->rules, keys_map, hashes, nodes);
+
+/* Check results.  When the i-th bit of found_map is set, it means
+ * that a set of nodes with a matching hash value was found for the
+ * i-th search-key.  Due to possible hash collisions we need to check
+ * which of the found rules, if any, really matches our masked
+ * search-ke

[ovs-dev] [PATCH v11 0/5] dpcls func ptrs & optimizations

2019-07-15 Thread Harry van Haaren
Hey Folks,

Here a v11 of the DPCLS Function Pointer patchset, as has been
presented at OVS Conf in Nov '18, and discussed on the ML since then.
I'm aware of the soft-freeze for 2.12, I feel this patchset has had
enough reviews/versions/testing to be merged in 2.12.

Thanks Ilya and Ian for review comments on v10, they should all be addressed
in this v11. Patchset details below, summary of v11 changes as follows:
- Reworked to use hash_add_words64(), unfortunatly hash_words64_inline() did
  not provide the same has due to it calling hash_finish() with a different
  "final" value. Refactored to re-use as much as we can of the existing code.
- Reworked specialized functions to use MACROs as suggested in review. This
  makes the specialization of functions much more tidy - good suggestion!
- Reworked lots of little fixes, Captials and stops.
- Added NEWS entry in "Userspace Datapath" section on DPCLS function pointers
  and how specialization of subtables minfilows gains search performance.
- See per patch --- v11 notes for details :)

Regards, -Harry


Patchset details;
1) Refactor dpcls_lookup and the subtable for flexibility.
In particular, add a function pointer to the subtable
structure, which enables "plugging-in" a lookup function
at runtime. This enables a number of optimizations in future.

2) and 3)
With the function pointer in place, we refactor the existing
dpcls_lookup matching code into its own function, and later its
own file. To split it to its own file requires making various
dpcls data-structures available in dpif-netdev-private.h.

4)
Refactor the existing code, to favour compute of flat arrays of
miniflows, instead of the MACRO based iteration. This simplifies
the code itself, and makes future optimizations possible due to
simplified loop structures, and loop trip counts pass in via
function arguments. See commit message for more details.

5)
This patch implements a select few specialized functions, for handling
miniflows with 5-1, 4-1, and 4-0 miniflow unit bit patterns. More of
these types of functions can (and should) be added to accelerate other
patterns of subtable lookups! See commit message for more details.


Harry van Haaren (5):
  dpif-netdev: Implement function pointers/subtable
  dpif-netdev: Move dpcls lookup structures to .h
  dpif-netdev: Split out generic lookup function
  dpif-netdev: Refactor generic implementation
  dpif-netdev: Add specialized generic scalar functions

 NEWS |   4 +
 lib/automake.mk  |   2 +
 lib/dpif-netdev-lookup-generic.c | 290 +++
 lib/dpif-netdev-private.h| 129 ++
 lib/dpif-netdev.c| 197 ++---
 5 files changed, 525 insertions(+), 97 deletions(-)
 create mode 100644 lib/dpif-netdev-lookup-generic.c
 create mode 100644 lib/dpif-netdev-private.h

-- 
2.17.1

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


Re: [ovs-dev] Update on DDlog port of ovn-northd

2019-07-15 Thread Leonid Ryzhyk
Hi Mark,

Thanks for the feedback. We will add the `--version` flag.

Yes, `stack install` merely copies the two DDlog executable to
`~/.local/bin/` (or a custom path specified using `--local-bin-path
`).

Also, just want to mention that we maintain up-to-date binary releases of
DDlog at `https://github.com/vmware/differential-datalog/releases`, so most
users should not need to install the Haskell tool stack and compile DDlog.
When using binary releases, all you have to do is add the `ddlog/bin`
directory to `$PATH`.

Leonid

On Mon, Jul 15, 2019 at 6:30 AM Mark Michelson  wrote:

> I did install DDlog originally back in early December last year. So
> there are probably remnants of that still present. However, I did
> perform a `stack install` using an updated pull of master on Friday. So
> I guess the `stack install` didn't get rid of the old installation?
>
> Also, there's no `ddlog --version` or anything similar to see what
> version of DDlog is installed. That could be a nice feature to have in
> the near future.
>
> On 7/12/19 12:40 PM, Leonid Ryzhyk wrote:
> > Thanks for trying it out!  Sounds like you have a very old version of
> > ddlog. If you install from source, please make sure that you run `stack
> > install` and that there is no other version off ddlog in your path other
> > than the one created by `stack install`.
> >
> > Leonid
> >
> > On Fri, Jul 12, 2019, 9:05 AM Mark Michelson  > > wrote:
> >
> > On 7/12/19 3:29 AM, Leonid Ryzhyk wrote:
> >  > Dear OVN developers,
> >  >
> >  > This is a brief update on the state of the DDlog port of
> ovn-northd.
> >  >
> >  > We completed the initial implementation of ovn-northd in DDlog
> > few months
> >  > ago.  Justin kindly
> >  > helped to integrate it with OVN, so that it can be used as a
> drop-in
> >  > replacement for the C
> >  > version (and passes all the tests in the OVN test suite).  The
> DDlog
> >  > implementation does
> >  > not have any of the new features/improvements added in April 2019
> > or later.
> >  >
> >  > ## Repository
> >  >
> >  > The code is in the `ddlog-dev` branch of the `ovn-org/ovn`
> > repository:
> >  > https://github.com/ovn-org/ovn/tree/ddlog-dev
> >
> > Hi Leonid,
> >
> > I ran into an issue when attempting to build ovn-northd. I
> successfully
> > installed DDLog, but then I encountered this issue when building OVN:
> >
> > ddlog -i ovn/northd/ovn_northd.dl -L
> > /home/putnopvut/differential-datalog/lib
> > ddlog: Failed to parse input file: "ovn/northd/ovn_northd.dl" (line
> 94,
> > column 5):
> > unexpected "&"
> > expecting "not", variable name, relation name, "var", expression or
> "."
> >
> > The line in question looks like this:
> >
> >   &SwitchPort(.lsp = lsp, .sw = &sw),
> >
> > Any idea what's gone wrong here?
> >
> > Thanks,
> >
> > Mark Michelson
> >
> >
> >
> >  >
> >  > ## Documentation
> >  >
> >  > Building and using ovn-northd-ddlog:
> >  >
> >
> https://github.com/ovn-org/ovn/blob/ddlog-dev/ovn/northd/docs/design.md
> >  >
> >  > Debugging ovn-northd-ddlog:
> >  >
> >
> https://github.com/ovn-org/ovn/blob/ddlog-dev/ovn/northd/docs/debugging.md
> >  >
> >  > ## Preliminary performance results
> >  >
> >  > Han Zhou kindly tested ovn-northd-ddlog with his OVN scale test
> > and even
> >  > found a nasty
> >  > performance bug in the process (thanks, Han!).  He reports that
> DDlog
> >  > speeds up the test
> >  > by almost a factor of 10:
> >  >
> >  > - ddlog version: 7:39min
> >  > - C version: 67:47min
> >  >
> >  > This is great, and in fact profiling shows that there is still
> > plenty of
> >  > space for
> >  > improvement.  He also reports a 10+ times increase in memory
> > footprint:
> >  >
> >  > - ddlog version: 1944696KB
> >  > - C version: 147984KB
> >  >
> >  > Again, we are working on a number of optimizations, which should
> > reduce
> >  > this overhead; although it will never be as low as C, since DDlog
> >  > fundamentally
> >  > needs to cache more state to enable fast incremental computation.
> >  >
> >  > Han also used DDlog's record&replay feature to capture all northd
> >  > transactions
> >  > performed by the scale test in a format that can be replayed
> > against the
> >  > standalone DDlog executable without having to reproduce Han's
> > OpenStack
> >  > setup.
> >  > The replay file is here: http://ryzhyk.net/replay.tgz
> >  >
> >  > Instructions for replaying this script:
> >  >
> >
> https://github.com/ovn-org/ovn/blob/ddlog-dev/ovn/northd/docs/debugging.md#record-and-replay-ddlog-execution
> >  > The script will run for a few
> >  > minutes and finally print some profiling information, including
> the
>

Re: [ovs-dev] [PATCH v3 3/3] OVN: Add ovn-northd IGMP support

2019-07-15 Thread Dumitru Ceara
On Fri, Jul 12, 2019 at 7:34 PM Ben Pfaff  wrote:
>
> On Thu, Jul 11, 2019 at 05:20:58PM +0200, Dumitru Ceara wrote:
> > While doing some scale testing I realized that walking the rows of the
> > IGMP_Group table in ovn-northd in the order we get them from the
> > database might create an issue: ovn_igmp_group_add will create a new
> > multicast_group for every unique IGMP group address and allocate a
> > tunnel-id for it. However, because rows are not processed in the order
> > they were added to the database, it can happen that multicast groups
> > that didn't actually change will get a different tunnel-id triggering
> > a change in the associated logical flows.
>
> Ouch.  Let's avoid that problem!
>
> > 2. Maintain a cache (hashtable) of allocated multicast group
> > tunnel-ids between subsequent  runs of the ovn-northd loop:
> > - Once all IGMP_Group entries are processed and their corresponding
> > Multicast_Group entries are collected we'd need to store a mapping
> > (per datapath) between IGMP group address and multicast group
> > tunnel-id.
> > - Next time ovn-northd walks the IGMP_Group table, before allocating a
> > new tunnel-id for a multicast group entry it would check the "cache"
> > from the previous run. If there's already an entry it would reuse the
> > tunnel-id. If not, it will have to allocate a tunnel-id. Store the
> > (IGMP group address, tunnel-id) mapping for next run.
>
> This is the customary way, so it's what I recommend.

I'm working on it and I'll send out a v4 soon.

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


Re: [ovs-dev] [PATCHv16 1/2] ovs-thread: Add pthread spin lock support.

2019-07-15 Thread Ilya Maximets
On 13.07.2019 2:50, William Tu wrote:
> The patch adds the basic spin lock functions:
> ovs_spin_{lock, try_lock, unlock, init, destroy}.
> OSX does not support pthread spin lock, so make it
> linux only.

IIUC, pthread spinlock requires some specific glibc verions (>= 2.2)
so it could be not supported even on Linux. Instead of checking
_POSIX_C_SOURCE version, I think it's better to just check for
pthread_spin_lock function in configure script with AC_CHECK_FUNC and
check for the resulted macro.
Additionally we could check for this macro while checking AF_XDP support
to not enable it if we have no spinlocks.

What do you think?

Best regards, Ilya Maximets.

> 
> Signed-off-by: William Tu 
> ---
>  include/openvswitch/thread.h | 22 ++
>  lib/ovs-thread.c | 31 +++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h
> index 2987db37c9dc..14cc9ad73900 100644
> --- a/include/openvswitch/thread.h
> +++ b/include/openvswitch/thread.h
> @@ -33,6 +33,13 @@ struct OVS_LOCKABLE ovs_mutex {
>  const char *where;  /* NULL if and only if uninitialized. */
>  };
>  
> +#ifdef __linux__
> +struct OVS_LOCKABLE ovs_spin {
> +pthread_spinlock_t lock;
> +const char *where;  /* NULL if and only if uninitialized. */
> +};
> +#endif
> +
>  /* "struct ovs_mutex" initializer. */
>  #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
>  #define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \
> @@ -70,6 +77,21 @@ int ovs_mutex_trylock_at(const struct ovs_mutex *mutex, 
> const char *where)
>  
>  void ovs_mutex_cond_wait(pthread_cond_t *, const struct ovs_mutex *mutex)
>  OVS_REQUIRES(mutex);
> +
> +#ifdef __linux__
> +void ovs_spin_init(const struct ovs_spin *);
> +void ovs_spin_destroy(const struct ovs_spin *);
> +void ovs_spin_unlock(const struct ovs_spin *spin) OVS_RELEASES(spin);
> +void ovs_spin_lock_at(const struct ovs_spin *spin, const char *where)
> +OVS_ACQUIRES(spin);
> +#define ovs_spin_lock(spin) \
> +ovs_spin_lock_at(spin, OVS_SOURCE_LOCATOR)
> +
> +int ovs_spin_trylock_at(const struct ovs_spin *spin, const char *where)
> +OVS_TRY_LOCK(0, spin);
> +#define ovs_spin_trylock(spin) \
> +ovs_spin_trylock_at(spin, OVS_SOURCE_LOCATOR)
> +#endif
>  
>  /* Convenient once-only execution.
>   *
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index 159d87e5b0ca..29a0b9e57acd 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -75,6 +75,9 @@ static bool multithreaded;
>  LOCK_FUNCTION(mutex, lock);
>  LOCK_FUNCTION(rwlock, rdlock);
>  LOCK_FUNCTION(rwlock, wrlock);
> +#ifdef __linux__
> +LOCK_FUNCTION(spin, lock);
> +#endif
>  
>  #define TRY_LOCK_FUNCTION(TYPE, FUN) \
>  int \
> @@ -103,6 +106,9 @@ LOCK_FUNCTION(rwlock, wrlock);
>  TRY_LOCK_FUNCTION(mutex, trylock);
>  TRY_LOCK_FUNCTION(rwlock, tryrdlock);
>  TRY_LOCK_FUNCTION(rwlock, trywrlock);
> +#ifdef __linux__
> +TRY_LOCK_FUNCTION(spin, trylock);
> +#endif
>  
>  #define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \
>  void \
> @@ -125,6 +131,10 @@ UNLOCK_FUNCTION(mutex, unlock, "");
>  UNLOCK_FUNCTION(mutex, destroy, NULL);
>  UNLOCK_FUNCTION(rwlock, unlock, "");
>  UNLOCK_FUNCTION(rwlock, destroy, NULL);
> +#ifdef __linux__
> +UNLOCK_FUNCTION(spin, unlock, "");
> +UNLOCK_FUNCTION(spin, destroy, NULL);
> +#endif
>  
>  #define XPTHREAD_FUNC1(FUNCTION, PARAM1)\
>  void\
> @@ -268,6 +278,27 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const struct 
> ovs_mutex *mutex_)
>  }
>  }
>  
> +#ifdef __linux__
> +static void
> +ovs_spin_init__(const struct ovs_spin *l_, int pshared)
> +{
> +struct ovs_spin *l = CONST_CAST(struct ovs_spin *, l_);
> +int error;
> +
> +l->where = "";
> +error = pthread_spin_init(&l->lock, pshared);
> +if (OVS_UNLIKELY(error)) {
> +ovs_abort(error, "pthread_spin_failed");
> +}
> +}
> +
> +void
> +ovs_spin_init(const struct ovs_spin *spin)
> +{
> +ovs_spin_init__(spin, PTHREAD_PROCESS_PRIVATE);
> +}
> +#endif
> +
>  /* Initializes the 'barrier'.  'size' is the number of threads
>   * expected to hit the barrier. */
>  void
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] userspace: Fix nsh test failures.

2019-07-15 Thread Ilya Maximets
On 13.07.2019 17:25, William Tu wrote:
> When doing
>  $ make check-kmod TESTSUITEFLAGS='-k nsh'
> 
> every test passes, but when doing
>  $ make check-system-userspace TESTSUITEFLAGS='-k nsh'
> 
> All test fails due to
>  odp_util(revalidator7)|WARN| OVS_NSH_KEY_ATTR_MD1 present but \
>  declared mdtype 0 is not 1 (NSH_M_TYPE1)
>  odp_util(revalidator7)|WARN| the flow mask in error is: \
>  
> skb_priority(0),tunnel(ttl=0,flags(0)),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),recirc_id(0x),dp_hash(0),in_port(4294967295),packet_type(ns=65535,id=0x),eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),eth_type(0x),nsh(flags=0ttl=0,mdtype=0,np=255,spi=0xff,si=255),
>  for the following flow key: 
> in_port=2,vlan_tci=0x,dl_src=f2:ff:00:00:00:01,dl_dst=f2:ff:00:00:00:02,dl_type=0x894f,nsh_flags=0,nsh_ttl=8,nsh_mdtype=1
> 
> The patch adds '-n' option to tcpdump, to avoid the delay of
> coverting address add more sleep time after the tcpdump starts
> to capture packets. But I don't know why more sleep time makes
> the revalidator warning disappears.

Hi.
Thanks for working on this!

I re-checked the code and found a bug that causes revalidator failures:
https://patchwork.ozlabs.org/patch/1132013/

So, the additional sleep seems not needed with above patch applied.

Regarding the '-n', I think it's a good enhancement.
I'd suggest passing '-n -xx' directly to the first tcpdump like this:
tcpdump -n -xx -U -i ovs-p1 > ovs-p1.pcap &

And use simple checking with 'cat' and 'OVS_WAIT_UNTIL', like all other
tests does:
OVS_WAIT_UNTIL([cat ovs-p1.pcap 2>&1 | egrep "0x: *f2ff * *0002 
*f2ff * *0001 *894f *0fc6" 2>&1 1>/dev/null])

This way tests should be more reliable.

Best regards, Ilya Maximets.

> 
> Signed-off-by: William Tu 
> --->  tests/system-traffic.at | 64 
> -
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 8ea450887076..84e73b82bd4f 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -5873,7 +5873,7 @@ AT_CHECK([ovs-ofctl -Oopenflow13 add-flow br0 
> "table=0,priority=100,in_port=ovs-
>  
>  rm ovs-p1.pcap
>  tcpdump -U -i ovs-p1 -w ovs-p1.pcap &
> -sleep 1
> +sleep 2
>  
>  dnl The hex dump is a TCP syn packet. pkt=eth/ip/tcp
>  dnl The packet is sent from p0(at_ns0) interface directed to
> @@ -5883,12 +5883,12 @@ NS_CHECK_EXEC([at_ns0], [$PYTHON $srcdir/sendpkt.py 
> p0 f2 00 00 00 00 02 f2 00 0
>  sleep 1
>  
>  dnl Check the expected nsh encapsulated packet on the egress interface
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x: *f2ff * *0002 
> *f2ff * *0001 *894f *0fc6" 2>&1 1>/dev/null])
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0010: *0103 *0012 *34ff 
> *1122 *3344 * * *" 2>&1 1>/dev/null])
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0020: * * * 
> *f200 * *0002 *f200 *" 2>&1 1>/dev/null])
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0030: *0001 *0800 *4500 
> *0028 *0001 * *4006 *b013" 2>&1 1>/dev/null])
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0040: *c0a8 *000a *0a00 
> *000a *0400 *0800 * *00c8" 2>&1 1>/dev/null])
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0050: * * *5002 
> *2000 *b85e *" 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x: *f2ff * 
> *0002 *f2ff * *0001 *894f *0fc6" 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0010: *0103 *0012 
> *34ff *1122 *3344 * * *" 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0020: * * 
> * *f200 * *0002 *f200 *" 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0030: *0001 *0800 
> *4500 *0028 *0001 * *4006 *b013" 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0040: *c0a8 *000a 
> *0a00 *000a *0400 *0800 * *00c8" 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -n -xx -r ovs-p1.pcap 2>&1 | egrep "0x0050: * * 
> *5002 *2000 *b85e *" 2>&1 1>/dev/null])
>  
>  
>  OVS_TRAFFIC_VSWITCHD_STOP
> @@ -5908,7 +5908,7 @@ AT_CHECK([ovs-ofctl -Oopenflow13 add-flow br0 
> "table=0,priority=100,in_port=ovs-
>  
>  rm ovs-p1.pcap
>  tcpdump -U -i ovs-p1 -w ovs-p1.pcap &
> -sleep 1
> +sleep 2
>  
>  dnl The hex dump is NSH packet with TCP syn payload. pkt=eth/nsh/eth/ip/tcp
>  dnl The packet is sent from p0(at_ns0) interface directed to
> @@ -5918,10 +5918,10 @@ NS_CHECK_EXEC([at_ns0], [$PYTHON $srcdir/sendpkt.py 
> p0 f2 ff 00 00 00 02 f2 ff 0
>  sleep 1
>  
>  dnl Check the expected de-capsulated TCP packet on the egress interface
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x: *f200 * *0002 
> *f200 * *0001 *0800 *4500" 2>&1 1>/dev/null])
> -AT_CHECK([tcpdump -xx -r ovs-p1.pcap 2>&1 | egrep "0x0010

[ovs-dev] [PATCH] odp-util: Fix NSH mask parsing.

2019-07-15 Thread Ilya Maximets
'odp_flow_key_to_flow__' is used to parse both keys and masks.
However, 'odp_nsh_key_from_attr' expects only 'key' as an argument
and fails to parse masks with ODP_FIT_ERROR which causes userspace
system tests failures:

 # make check-system-userspace TESTSUITEFLAGS='-k nsh'

 |odp_util(revalidator)|WARN|
 OVS_NSH_KEY_ATTR_MD1 present but declared mdtype 0 is not 1 (NSH_M_TYPE1)

 |odp_util(revalidator)|WARN|
 the flow mask in error is:
 <...> nsh(flags=0ttl=0,mdtype=0,np=255,spi=0x0,si=0),
 for the following flow key:
 <...> nsh_ttl=8,nsh_mdtype=1,nsh_np=3,nsh_spi=0x64,nsh_si=3,
 nsh_c1=0x1020304,nsh_c2=0x5060708,nsh_c3=0x90a0b0c,nsh_c4=0xd0e0f10
 <...>

Fix that by passing the additional argument 'is_mask' to make it be
like all other parsing functions.

Additionally fixed missing comma in the 'format_nsh_key'.

CC: Yi Yang 
Fixes: f59cb331c481 ("nsh: rework NSH netlink keys and actions")
Signed-off-by: Ilya Maximets 
---
 lib/odp-util.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 3a28877e3..84ea4c148 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -264,7 +264,7 @@ static void
 format_nsh_key(struct ds *ds, const struct ovs_key_nsh *key)
 {
 ds_put_format(ds, "flags=%d", key->flags);
-ds_put_format(ds, "ttl=%d", key->ttl);
+ds_put_format(ds, ",ttl=%d", key->ttl);
 ds_put_format(ds, ",mdtype=%d", key->mdtype);
 ds_put_format(ds, ",np=%d", key->np);
 ds_put_format(ds, ",spi=0x%x",
@@ -2798,11 +2798,14 @@ odp_parse_error(struct vlog_rate_limit *rl, char 
**errorp,
 }
 
 /* Parses OVS_KEY_ATTR_NSH attribute 'attr' into 'nsh' and 'nsh_mask' and
- * returns fitness.  If 'errorp' is nonnull and the function returns
- * ODP_FIT_ERROR, stores a malloc()'d error message in '*errorp'. */
-enum odp_key_fitness
-odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh,
-  struct ovs_key_nsh *nsh_mask, char **errorp)
+ * returns fitness.  If the attribute is a key, 'is_mask' should be false;
+ * if it is a mask, 'is_mask' should be true.  If 'errorp' is nonnull and the
+ * function returns ODP_FIT_ERROR, stores a malloc()'d error message in
+ * '*errorp'. */
+static enum odp_key_fitness
+odp_nsh_key_from_attr__(const struct nlattr *attr, bool is_mask,
+struct ovs_key_nsh *nsh, struct ovs_key_nsh *nsh_mask,
+char **errorp)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 if (errorp) {
@@ -2877,7 +2880,7 @@ odp_nsh_key_from_attr(const struct nlattr *attr, struct 
ovs_key_nsh *nsh,
 return ODP_FIT_TOO_MUCH;
 }
 
-if (has_md1 && nsh->mdtype != NSH_M_TYPE1 && !nsh_mask) {
+if (!is_mask && has_md1 && nsh->mdtype != NSH_M_TYPE1 && !nsh_mask) {
 odp_parse_error(&rl, errorp, "OVS_NSH_KEY_ATTR_MD1 present but "
 "declared mdtype %"PRIu8" is not %d (NSH_M_TYPE1)",
 nsh->mdtype, NSH_M_TYPE1);
@@ -2887,6 +2890,17 @@ odp_nsh_key_from_attr(const struct nlattr *attr, struct 
ovs_key_nsh *nsh,
 return ODP_FIT_PERFECT;
 }
 
+/* Parses OVS_KEY_ATTR_NSH attribute 'attr' into 'nsh' and 'nsh_mask' and
+ * returns fitness.  The attribute should be a key (not a mask).  If 'errorp'
+ * is nonnull and the function returns ODP_FIT_ERROR, stores a malloc()'d error
+ * message in '*errorp'. */
+enum odp_key_fitness
+odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh,
+  struct ovs_key_nsh *nsh_mask, char **errorp)
+{
+return odp_nsh_key_from_attr__(attr, false, nsh, nsh_mask, errorp);
+}
+
 /* Parses OVS_KEY_ATTR_TUNNEL attribute 'attr' into 'tun' and returns fitness.
  * If the attribute is a key, 'is_mask' should be false; if it is a mask,
  * 'is_mask' should be true.  If 'errorp' is nonnull and the function returns
@@ -6750,8 +6764,9 @@ parse_l2_5_onward(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
 *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_NSH;
 }
 if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_NSH)) {
-if (odp_nsh_key_from_attr(attrs[OVS_KEY_ATTR_NSH], &flow->nsh,
-  NULL, errorp) == ODP_FIT_ERROR) {
+if (odp_nsh_key_from_attr__(attrs[OVS_KEY_ATTR_NSH],
+is_mask, &flow->nsh,
+NULL, errorp) == ODP_FIT_ERROR) {
 return ODP_FIT_ERROR;
 }
 if (is_mask) {
-- 
2.17.1

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


Re: [ovs-dev] Update on DDlog port of ovn-northd

2019-07-15 Thread Mark Michelson
I did install DDlog originally back in early December last year. So 
there are probably remnants of that still present. However, I did 
perform a `stack install` using an updated pull of master on Friday. So 
I guess the `stack install` didn't get rid of the old installation?


Also, there's no `ddlog --version` or anything similar to see what 
version of DDlog is installed. That could be a nice feature to have in 
the near future.


On 7/12/19 12:40 PM, Leonid Ryzhyk wrote:
Thanks for trying it out!  Sounds like you have a very old version of 
ddlog. If you install from source, please make sure that you run `stack 
install` and that there is no other version off ddlog in your path other 
than the one created by `stack install`.


Leonid

On Fri, Jul 12, 2019, 9:05 AM Mark Michelson > wrote:


On 7/12/19 3:29 AM, Leonid Ryzhyk wrote:
 > Dear OVN developers,
 >
 > This is a brief update on the state of the DDlog port of ovn-northd.
 >
 > We completed the initial implementation of ovn-northd in DDlog
few months
 > ago.  Justin kindly
 > helped to integrate it with OVN, so that it can be used as a drop-in
 > replacement for the C
 > version (and passes all the tests in the OVN test suite).  The DDlog
 > implementation does
 > not have any of the new features/improvements added in April 2019
or later.
 >
 > ## Repository
 >
 > The code is in the `ddlog-dev` branch of the `ovn-org/ovn`
repository:
 > https://github.com/ovn-org/ovn/tree/ddlog-dev

Hi Leonid,

I ran into an issue when attempting to build ovn-northd. I successfully
installed DDLog, but then I encountered this issue when building OVN:

ddlog -i ovn/northd/ovn_northd.dl -L
/home/putnopvut/differential-datalog/lib
ddlog: Failed to parse input file: "ovn/northd/ovn_northd.dl" (line 94,
column 5):
unexpected "&"
expecting "not", variable name, relation name, "var", expression or "."

The line in question looks like this:

      &SwitchPort(.lsp = lsp, .sw = &sw),

Any idea what's gone wrong here?

Thanks,

Mark Michelson



 >
 > ## Documentation
 >
 > Building and using ovn-northd-ddlog:
 >
https://github.com/ovn-org/ovn/blob/ddlog-dev/ovn/northd/docs/design.md
 >
 > Debugging ovn-northd-ddlog:
 >
https://github.com/ovn-org/ovn/blob/ddlog-dev/ovn/northd/docs/debugging.md
 >
 > ## Preliminary performance results
 >
 > Han Zhou kindly tested ovn-northd-ddlog with his OVN scale test
and even
 > found a nasty
 > performance bug in the process (thanks, Han!).  He reports that DDlog
 > speeds up the test
 > by almost a factor of 10:
 >
 > - ddlog version: 7:39min
 > - C version: 67:47min
 >
 > This is great, and in fact profiling shows that there is still
plenty of
 > space for
 > improvement.  He also reports a 10+ times increase in memory
footprint:
 >
 > - ddlog version: 1944696KB
 > - C version: 147984KB
 >
 > Again, we are working on a number of optimizations, which should
reduce
 > this overhead; although it will never be as low as C, since DDlog
 > fundamentally
 > needs to cache more state to enable fast incremental computation.
 >
 > Han also used DDlog's record&replay feature to capture all northd
 > transactions
 > performed by the scale test in a format that can be replayed
against the
 > standalone DDlog executable without having to reproduce Han's
OpenStack
 > setup.
 > The replay file is here: http://ryzhyk.net/replay.tgz
 >
 > Instructions for replaying this script:
 >

https://github.com/ovn-org/ovn/blob/ddlog-dev/ovn/northd/docs/debugging.md#record-and-replay-ddlog-execution
 > The script will run for a few
 > minutes and finally print some profiling information, including the
 > breakdown of
 > DDlog's CPU and memory usage.
 >
 > ## Next steps
 >
 > We seek help from the OVN community in maintaining
ovn-northd-ddlog.  The
 > first
 > step is to start porting new OVN features introduced in the last
few months
 > to
 > DDlog.
 >
 > Leonid

Hi Leonid,

I attempted

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



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


Re: [ovs-dev] [PATCH v3] Detailed packet drop statistics per dpdk and vhostuser ports

2019-07-15 Thread 0-day Robot
Bleep bloop.  Greetings Sriram Vatala via dev, I am a robot and I have tried 
out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 138 characters long (recommended limit is 79)
#344 FILE: utilities/bugtool/plugins/network-status/openvswitch.xml:37:
/usr/share/openvswitch/scripts/ovs-bugtool-get-iface-stats

Lines checked: 367, Warnings: 1, Errors: 0


build:
reading sources... [ 88%] topics/ovsdb-replication
reading sources... [ 88%] topics/porting
reading sources... [ 89%] topics/role-based-access-control
reading sources... [ 90%] topics/testing
reading sources... [ 91%] topics/tracing
reading sources... [ 92%] topics/windows
reading sources... [ 93%] tutorials/faucet
reading sources... [ 94%] tutorials/index
reading sources... [ 94%] tutorials/ipsec
reading sources... [ 95%] tutorials/ovn-ipsec
reading sources... [ 96%] tutorials/ovn-openstack
reading sources... [ 97%] tutorials/ovn-rbac
reading sources... [ 98%] tutorials/ovn-sandbox
reading sources... [ 99%] tutorials/ovs-advanced
reading sources... [100%] tutorials/ovs-conntrack


Warning, treated as error:
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/Documentation/topics/dpdk/bridge.rst:85:
 WARNING: Definition list ends without a blank line; unexpected unindent.

make[2]: *** [docs-check] Error 1
make[2]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v3] Detailed packet drop statistics per dpdk and vhostuser ports

2019-07-15 Thread Sriram Vatala via dev
OVS may be unable to transmit packets for multiple reasons and
today there is a single counter to track packets dropped due to
any of those reasons. The most common reason is that a VM is
unable to read packets fast enough causing the vhostuser port
transmit queue on the OVS side to become full. This manifests
as a problem with VNFs not receiving all packets. Having a
separate drop counter to track packets dropped because the
transmit queue is full will clearly indicate that the problem
is on the VM side and not in OVS. Similarly maintaining separate
counters for all possible drops helps in indicating sensible
cause for packet drops.

This patch adds counters to track packets dropped due to all
possible reasons and these counters are displayed along with
other stats in "ovs-vsctl get interface  statistics"
command. The detailed stats will be available for both dpdk and
vhostuser ports.

Following are the details of the new counters :

tx_failed_drops : Sometimes DPDK physical/vHost port transmit
API fails to send all/some packets. These untransmited packets
are dropped.The most likely reason for this to happen is
because of transmit queue overrun. Besides transmit queue
overrun, there are other unlikely reasons such as invalid
queue id etc.

tx_mtu_exceeded_drops : These are the packets dropped due
to MTU mismatch (i.e Pkt len > Max Dev MTU).

tx_qos_drops/rx_qos_drops : These are the packets dropped due
to transmission/reception rate exceeding the configured
Egress/Ingress policer rate on the interface.

Signed-off-by: Sriram Vatala 
---
 Documentation/topics/dpdk/bridge.rst   | 25 ++
 include/openvswitch/netdev.h   |  9 
 lib/netdev-dpdk.c  | 56 ++
 utilities/bugtool/automake.mk  |  3 +-
 utilities/bugtool/ovs-bugtool-get-iface-stats  | 25 ++
 .../bugtool/plugins/network-status/openvswitch.xml |  1 +
 vswitchd/bridge.c  |  6 ++-
 7 files changed, 115 insertions(+), 10 deletions(-)
 create mode 100755 utilities/bugtool/ovs-bugtool-get-iface-stats

diff --git a/Documentation/topics/dpdk/bridge.rst 
b/Documentation/topics/dpdk/bridge.rst
index a3ed926..df87c7a 100644
--- a/Documentation/topics/dpdk/bridge.rst
+++ b/Documentation/topics/dpdk/bridge.rst
@@ -74,6 +74,31 @@ OpenFlow14`` option::
 
 $ ovs-ofctl -O OpenFlow14 dump-ports br0
 
+Detail statistics counters for transmit dropped packets and receive
+dropped packets are implemented and supported only for DPDK physical
+and vHost ports.
+
+Following are the details of the new counters :
+
+tx_failure_drops : Sometimes DPDK transmit API for physical/vHost ports
+   may fail to transmit all/some packets in its packet
+buffer which is passed as input argument to the dpdk xmit API. These
+untransmitted packets are dropped. The most likely reason for this to
+happens is when the transmit queue is full or has been filled up.
+There are other unlikely reasons such as invalid Tx queue id etc.
+
+tx_mtu_exceeded_drops : These are the packets dropped due to MTU mismatch
+(i.e Pkt len > Max Dev MTU).
+
+tx_qos_drops/rx_qos_drops : These are the packets dropped due to
+transmission/reception rate exceeding the
+configured Egress/Ingress policer rate on the interface.
+
+These statistic counters can be viewed with the following commands:
+
+$ ovs-vsctl get interface  statistics
+$ ovs-vsctl list interface
+
 EMC Insertion Probability
 -
 
diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
index 0c10f7b..7851736 100644
--- a/include/openvswitch/netdev.h
+++ b/include/openvswitch/netdev.h
@@ -61,6 +61,15 @@ struct netdev_stats {
 uint64_t tx_heartbeat_errors;
 uint64_t tx_window_errors;
 
+/* Detailed receive drops. */
+uint64_t rx_qos_drops;  /* rx rate exceeded conf'd qos rate */
+
+/* Detailed transmit drops. */
+uint64_t tx_failure_drops;  /* Dpdk tx failure, probably tx
+ * queue is full. */
+uint64_t tx_qos_drops;  /* tx rate exceeded conf'd qos rate */
+uint64_t tx_mtu_drops;  /* tx pkt len exceeded max dev MTU */
+
 /* Extended statistics based on RFC2819. */
 uint64_t rx_1_to_64_packets;
 uint64_t rx_65_to_127_packets;
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4805783..3f22701 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -68,6 +68,7 @@
 #include "uuid.h"
 
 enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
+enum {QOS_DROPS, MTU_DROPS, TX_FAILURE_DROPS, MAX_DROPS};
 
 VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
@@ -2170,6 +2171,7 @@ netdev_dpdk_vhost_update_rx_counters(struct netdev_stats 
*stats,
 
 stats->rx_packets += count;
 stats->rx_dropped += dropped;
+stats->rx_qos_drops += dropped;
 for (i = 0; i 

[ovs-dev] Kindly Respond

2019-07-15 Thread Donald Douglas
Hello,

I am Barr Fredrick Mbogo a business consultant i have a lucrative
business to discuss with you from the Eastern part of Africa Uganda to
be precise aimed at agreed percentage upon your acceptance of my hand
in business and friendship. Kindly respond to me if you are interested
to partner with me for an update. Very important.

Yours Sincerely,
Donald Douglas,
For,
Barr Frederick Mbogo
Legal Consultant.
Reply to: barrfredmb...@consultant.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v4] Avoid dp_hash recirculation for balance-tcp bond selection mode

2019-07-15 Thread Vishal Deep Ajmera
Problem:

In OVS-DPDK, flows with output over a bond interface of type “balance-tcp”
(using a hash on TCP/UDP 5-tuple) get translated by the ofproto layer into
"HASH" and "RECIRC" datapath actions. After recirculation, the packet is
forwarded to the bond member port based on 8-bits of the datapath hash
value computed through dp_hash. This causes performance degradation in the
following ways:

1. The recirculation of the packet implies another lookup of the packet’s
flow key in the exact match cache (EMC) and potentially Megaflow classifier
(DPCLS). This is the biggest cost factor.

2. The recirculated packets have a new “RSS” hash and compete with the
original packets for the scarce number of EMC slots. This implies more
EMC misses and potentially EMC thrashing causing costly DPCLS lookups.

3. The 256 extra megaflow entries per bond for dp_hash bond selection put
additional load on the revalidation threads.

Owing to this performance degradation, deployments stick to “balance-slb”
bond mode even though it does not do active-active load balancing for
VXLAN- and GRE-tunnelled traffic because all tunnel packet have the same
source MAC address.

Proposed optimization:
--
This proposal introduces a new load-balancing output action instead of
recirculation.

Maintain one table per-bond (could just be an array of uint16's) and
program it the same way internal flows are created today for each possible
hash value(256 entries) from ofproto layer. Use this table to load-balance
flows as part of output action processing.

Currently xlate_normal() -> output_normal() -> bond_update_post_recirc_rules()
-> bond_may_recirc() and compose_output_action__() generate
“dp_hash(hash_l4(0))” and “recirc()” actions. In this case the
RecircID identifies the bond. For the recirculated packets the ofproto layer
installs megaflow entries that match on RecircID and masked dp_hash and send
them to the corresponding output port.

Instead, we will now generate actions as
"hash(l4(0)),lb_output(bond,)"

This combines hash computation (only if needed, else re-use RSS hash) and
inline load-balancing over the bond. This action is used *only* for balance-tcp
bonds in OVS-DPDK datapath (the OVS kernel datapath remains unchanged).

Example:

Current scheme:
---
With 1 IP-UDP flow:

flow-dump from pmd on cpu core: 2
recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no),
 packets:2828969, bytes:181054016, used:0.000s, 
actions:hash(hash_l4(0)),recirc(0x1)

recirc_id(0x1),dp_hash(0x113683bd/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:2828937, bytes:181051968, used:0.000s, actions:2

With 8 IP-UDP flows (with random UDP src port): (all hitting same DPCL):

flow-dump from pmd on cpu core: 2
recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no),
 packets:2674009, bytes:171136576, used:0.000s, 
actions:hash(hash_l4(0)),recirc(0x1)

recirc_id(0x1),dp_hash(0xf8e02b7e/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:377395, bytes:24153280, used:0.000s, actions:2
recirc_id(0x1),dp_hash(0xb236c260/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:333486, bytes:21343104, used:0.000s, actions:1
recirc_id(0x1),dp_hash(0x7d89eb18/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:348461, bytes:22301504, used:0.000s, actions:1
recirc_id(0x1),dp_hash(0xa78d75df/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:633353, bytes:40534592, used:0.000s, actions:2
recirc_id(0x1),dp_hash(0xb58d846f/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:319901, bytes:20473664, used:0.001s, actions:2
recirc_id(0x1),dp_hash(0x24534406/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:334985, bytes:21439040, used:0.001s, actions:1
recirc_id(0x1),dp_hash(0x3cf32550/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
 packets:326404, bytes:20889856, used:0.001s, actions:1

New scheme:
---
We can do with a single flow entry (for any number of new flows):

in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no),
 packets:2674009, bytes:171136576, used:0.000s, 
actions:hash(l4(0)),lb_output(bond,1)

A new CLI has been added to dump the per-PMD bond cache as given below.

“sudo ovs-appctl dpif-netdev/pmd-bond-show”

root@ubuntu-190:performance_scripts # sudo ovs-appctl dpif-netdev/pmd-bond-show
pmd thread numa_id 0 core_id 4:
Bond cache:
bond-id 1 :
bucket 0 - slave 2
bucket 1 - slave 1
bucket 2 - slave 2
bucket 3 - slave 1

Performance improvement:

With a prototype of the proposed idea, the f

[ovs-dev] [PATCH v4] Balance-tcp bond mode optimization

2019-07-15 Thread Vishal Deep Ajmera
v1->v2:
 Updated datapath action to hash + lb-output.
 Updated throughput test observations.
 Rebased to OVS master.

v2->v3:
 Rebased to OVS master.
 Fixed git merge issue.

v3->v4:
 Addressed Ilya Maximets comments.
 https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360452.html

Vishal Deep Ajmera (1):
  Avoid dp_hash recirculation for balance-tcp bond selection mode

 datapath/linux/compat/include/linux/openvswitch.h |   2 +
 lib/dpif-netdev.c | 462 --
 lib/dpif-netlink.c|   2 +
 lib/dpif-provider.h   |   5 +
 lib/dpif.c|  36 ++
 lib/dpif.h|   5 +
 lib/odp-execute.c |   2 +
 lib/odp-util.c|   4 +
 ofproto/bond.c|  35 +-
 ofproto/bond.h|   9 +
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  65 ++-
 ofproto/ofproto-dpif.c|  31 ++
 ofproto/ofproto-dpif.h|  12 +-
 tests/lacp.at |   9 +
 vswitchd/bridge.c |   4 +
 vswitchd/vswitch.xml  |  10 +
 18 files changed, 640 insertions(+), 55 deletions(-)

-- 
1.9.1

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