[ovs-dev] [RFC 2/2] rhel: tell ovsctl to freeze the datapath

[Aaron Conole]

This commit uses the previous to stop it from deleting active flows in the
datapath.

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
 rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 2 +-
 utilities/ovs-ctl.in| 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
index c6d9aa1b8..4c0703b23 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
@@ -23,7 +23,7 @@ ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
-  --no-monitor --system-id=random \
+  --no-monitor --system-id=random --freeze-datapath \
   --ovs-user=${OVS_USER_ID} \
   restart $OPTIONS
 TimeoutSec=300
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index f1b01d1d3..3e02613eb 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -306,6 +306,9 @@ ovs_save () {
 }
 
 save_flows_if_required () {
+if test X"$FREEZE_DATAPATH" != Xyes; then
+action "Freezing datapath" ovs-appctl dpctl/datapath-icing 
system@ovs-system enabled
+fi
 if test X"$DELETE_BRIDGES" != Xyes; then
 action "Saving flows" ovs_save save-flows "${script_flows}"
 fi
@@ -494,6 +497,7 @@ set_defaults () {
 
 DELETE_BRIDGES=no
 DELETE_TRANSIENT_PORTS=no
+FREEZE_DATAPATH=no
 
 DAEMON_CWD=/
 FORCE_COREFILES=yes
-- 
2.14.3

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

[ovs-dev] [PATCH] flake8: Ignore bare except violations

[Aaron Conole]

And update the URL which lists error codes (and also points to
additional sources for error codes).

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
 Makefile.am | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index d7cfdcd52..2c655df2e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -359,7 +359,7 @@ endif
 
 if HAVE_FLAKE8
 ALL_LOCAL += flake8-check
-# http://flake8.readthedocs.org/en/latest/warnings.html
+# http://flake8.readthedocs.org/en/latest/user/error-codes.html
 # All warnings explicitly selected or ignored should be listed below.
 #
 # E***, W*** -- warnings from pep8
@@ -371,6 +371,7 @@ ALL_LOCAL += flake8-check
 #   E128 continuation line under-indented for visual indent
 #   E129 visually indented line with same indent as next logical line
 #   E131 continuation line unaligned for hanging indent
+#   E722 do not use bare except, specify exception instead
 #   W503 line break before binary operator
 # F*** -- warnings native to flake8
 #   F811 redefinition of unused  from line  (only from flake8 v2.0)
@@ -381,7 +382,7 @@ ALL_LOCAL += flake8-check
 #   H233 Python 3.x incompatible use of print operator
 #   H238 old style class declaration, use new style (inherit from `object`)
 FLAKE8_SELECT = H231,H232,H233,H238
-FLAKE8_IGNORE = E121,E123,E125,E126,E127,E128,E129,E131,W503,F811,D,H,I
+FLAKE8_IGNORE = E121,E123,E125,E126,E127,E128,E129,E131,E722,W503,F811,D,H,I
 flake8-check: $(FLAKE8_PYFILES)
$(FLAKE8_WERROR)$(AM_V_GEN) \
  src='$^' && \
-- 
2.14.3

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

[ovs-dev] [PATCH] ovs-tcpundump: fix a conversion issue

[Aaron Conole]

When I tried using ovs-tcpundump, I got the following error message:
Traceback (most recent call last):
  File ./ovs-tcpundump, line 64, in 
if m is None or int(m.group(1)) == 0:
ValueError: invalid literal for int() with base 10: '00a0'

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
 utilities/ovs-tcpundump.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utilities/ovs-tcpundump.in b/utilities/ovs-tcpundump.in
index 57300cdc1..c99015b5b 100755
--- a/utilities/ovs-tcpundump.in
+++ b/utilities/ovs-tcpundump.in
@@ -61,7 +61,7 @@ if __name__ == "__main__":
 break
 
 m = regex.match(line)
-if m is None or int(m.group(1)) == 0:
+if m is None or int(m.group(1), 16) == 0:
 if packet != '':
 print packet
 packet = ''
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] flake8: Ignore bare except violations

[Aaron Conole]

Ben Pfaff <b...@ovn.org> writes:

> On Fri, Jan 12, 2018 at 04:38:18PM -0500, Aaron Conole wrote:
>> Ben Pfaff <b...@ovn.org> writes:
>> 
>> > On Fri, Jan 12, 2018 at 03:39:58PM -0500, Aaron Conole wrote:
>> >> And update the URL which lists error codes (and also points to
>> >> additional sources for error codes).
>> >> 
>> >> Signed-off-by: Aaron Conole <acon...@redhat.com>
>> >
>> > Would you mind explaining a bit more?  Do we have violations of this
>> > rule?  I don't get failures here myself, so is it a rule added in some
>> > newer version of flake8 than what I'm running?
>> 
>> I think it's probably from a newer version of flake8 (3.5.0, mccabe:
>> 0.6.1, pycodestyle: 2.3.1, pyflakes: 1.6.0).  Without this patch, I get
>> the following errors (probably I should have included this in my commit
>> message - apologies):
>> 
>>   utilities/checkpatch.py:476:5: E722 do not use bare except'
>>   utilities/checkpatch.py:514:5: E722 do not use bare except'
>>   utilities/ovs-dev.py:189:5: E722 do not use bare except'
>>   utilities/ovs-dev.py:192:9: E722 do not use bare except'
>>   utilities/ovs-dev.py:197:5: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:360:13: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:434:5: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:470:13: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:609:9: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:679:5: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:712:13: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:735:5: E741 ambiguous variable name 'l'
>>   utilities/bugtool/ovs-bugtool.in:744:9: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:751:9: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:825:5: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1006:13: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1041:13: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1079:5: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1124:5: E741 ambiguous variable name 'l'
>>   utilities/bugtool/ovs-bugtool.in:1202:5: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1247:9: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1257:13: E722 do not use bare except'
>>   utilities/bugtool/ovs-bugtool.in:1328:9: E722 do not use bare except'
>>   tests/test-daemon.py:60:5: E722 do not use bare except'
>>   tests/test-l7.py:23:1: E722 do not use bare except'
>>   tests/test-unixctl.py:96:5: E722 do not use bare except'
>>   xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync:404:5: E722
>> do not use bare except'
>>   python/ovs/fcntl_win.py:39:9: E722 do not use bare except'
>>   python/ovs/poller.py:38:1: E722 do not use bare except'
>>   python/ovs/socket_util.py:151:13: E722 do not use bare except'
>>   python/ovs/stream.py:169:17: E722 do not use bare except'
>>   python/ovs/stream.py:578:17: E722 do not use bare except'
>>   python/ovs/timeval.py:51:1: E722 do not use bare except'
>>   python/ovstest/util.py:52:5: E722 do not use bare except'
>>   vtep/ovs-vtep:767:5: E722 do not use bare except'
>> 
>> I assumed that since it appears in many files, it's probably okay to be
>> ignoring this error (bare except hides the exception, which is usually
>> 'bad form').  Maybe another approach would be to try and fix them all,
>> but that means understanding which exceptions should be handled and
>> allowing the rest to bubble up to the user.  That would take more time
>> than I have, though.
>
> Thanks for the extra info.  Would you mind reposting with that added
> information?

Sure, just did.  Almost forgot to send this, though.  Thanks for the
review, Ben!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] flake8: Ignore bare except violations

[Aaron Conole]

Newer versions of flake8 (3.5.0, mccabe: 0.6.1, pycodestyle: 2.3.1,
pyflakes: 1.6.0) add an error code for 'bare exception'.  The OvS
codebase does use bare exceptions in places, especially when the
specific exception isn't important (ie: the program will be
terminating, so the specific exception isn't important).

Without this change, the following error messages appear:
   utilities/checkpatch.py:476:5: E722 do not use bare except'
   utilities/checkpatch.py:514:5: E722 do not use bare except'
   utilities/ovs-dev.py:189:5: E722 do not use bare except'
   utilities/ovs-dev.py:192:9: E722 do not use bare except'
   utilities/ovs-dev.py:197:5: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:360:13: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:434:5: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:470:13: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:609:9: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:679:5: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:712:13: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:744:9: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:751:9: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:825:5: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1006:13: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1041:13: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1079:5: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1202:5: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1247:9: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1257:13: E722 do not use bare except'
   utilities/bugtool/ovs-bugtool.in:1328:9: E722 do not use bare except'
   tests/test-daemon.py:60:5: E722 do not use bare except'
   tests/test-l7.py:23:1: E722 do not use bare except'
   tests/test-unixctl.py:96:5: E722 do not use bare except'
   xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync:404:5: E722 do not use 
bare except'
   python/ovs/fcntl_win.py:39:9: E722 do not use bare except'
   python/ovs/poller.py:38:1: E722 do not use bare except'
   python/ovs/socket_util.py:151:13: E722 do not use bare except'
   python/ovs/stream.py:169:17: E722 do not use bare except'
   python/ovs/stream.py:578:17: E722 do not use bare except'
   python/ovs/timeval.py:51:1: E722 do not use bare except'
   python/ovstest/util.py:52:5: E722 do not use bare except'
   vtep/ovs-vtep:767:5: E722 do not use bare except'

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
v1->v2: Include more information about the change.

 Makefile.am | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index d7cfdcd52..2c655df2e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -359,7 +359,7 @@ endif
 
 if HAVE_FLAKE8
 ALL_LOCAL += flake8-check
-# http://flake8.readthedocs.org/en/latest/warnings.html
+# http://flake8.readthedocs.org/en/latest/user/error-codes.html
 # All warnings explicitly selected or ignored should be listed below.
 #
 # E***, W*** -- warnings from pep8
@@ -371,6 +371,7 @@ ALL_LOCAL += flake8-check
 #   E128 continuation line under-indented for visual indent
 #   E129 visually indented line with same indent as next logical line
 #   E131 continuation line unaligned for hanging indent
+#   E722 do not use bare except, specify exception instead
 #   W503 line break before binary operator
 # F*** -- warnings native to flake8
 #   F811 redefinition of unused  from line  (only from flake8 v2.0)
@@ -381,7 +382,7 @@ ALL_LOCAL += flake8-check
 #   H233 Python 3.x incompatible use of print operator
 #   H238 old style class declaration, use new style (inherit from `object`)
 FLAKE8_SELECT = H231,H232,H233,H238
-FLAKE8_IGNORE = E121,E123,E125,E126,E127,E128,E129,E131,W503,F811,D,H,I
+FLAKE8_IGNORE = E121,E123,E125,E126,E127,E128,E129,E131,E722,W503,F811,D,H,I
 flake8-check: $(FLAKE8_PYFILES)
$(FLAKE8_WERROR)$(AM_V_GEN) \
  src='$^' && \
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] flake8: Ignore bare except violations

[Aaron Conole]

Ben Pfaff <b...@ovn.org> writes:

> On Fri, Jan 12, 2018 at 03:39:58PM -0500, Aaron Conole wrote:
>> And update the URL which lists error codes (and also points to
>> additional sources for error codes).
>> 
>> Signed-off-by: Aaron Conole <acon...@redhat.com>
>
> Would you mind explaining a bit more?  Do we have violations of this
> rule?  I don't get failures here myself, so is it a rule added in some
> newer version of flake8 than what I'm running?

I think it's probably from a newer version of flake8 (3.5.0, mccabe:
0.6.1, pycodestyle: 2.3.1, pyflakes: 1.6.0).  Without this patch, I get
the following errors (probably I should have included this in my commit
message - apologies):

  utilities/checkpatch.py:476:5: E722 do not use bare except'
  utilities/checkpatch.py:514:5: E722 do not use bare except'
  utilities/ovs-dev.py:189:5: E722 do not use bare except'
  utilities/ovs-dev.py:192:9: E722 do not use bare except'
  utilities/ovs-dev.py:197:5: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:360:13: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:434:5: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:470:13: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:609:9: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:679:5: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:712:13: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:735:5: E741 ambiguous variable name 'l'
  utilities/bugtool/ovs-bugtool.in:744:9: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:751:9: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:825:5: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1006:13: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1041:13: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1079:5: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1124:5: E741 ambiguous variable name 'l'
  utilities/bugtool/ovs-bugtool.in:1202:5: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1247:9: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1257:13: E722 do not use bare except'
  utilities/bugtool/ovs-bugtool.in:1328:9: E722 do not use bare except'
  tests/test-daemon.py:60:5: E722 do not use bare except'
  tests/test-l7.py:23:1: E722 do not use bare except'
  tests/test-unixctl.py:96:5: E722 do not use bare except'
  xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync:404:5: E722 do not use 
bare except'
  python/ovs/fcntl_win.py:39:9: E722 do not use bare except'
  python/ovs/poller.py:38:1: E722 do not use bare except'
  python/ovs/socket_util.py:151:13: E722 do not use bare except'
  python/ovs/stream.py:169:17: E722 do not use bare except'
  python/ovs/stream.py:578:17: E722 do not use bare except'
  python/ovs/timeval.py:51:1: E722 do not use bare except'
  python/ovstest/util.py:52:5: E722 do not use bare except'
  vtep/ovs-vtep:767:5: E722 do not use bare except'

I assumed that since it appears in many files, it's probably okay to be
ignoring this error (bare except hides the exception, which is usually
'bad form').  Maybe another approach would be to try and fix them all,
but that means understanding which exceptions should be handled and
allowing the rest to bubble up to the user.  That would take more time
than I have, though.

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

Re: [ovs-dev] [PATCH] rhel: Fix support for root user using DPDK

[Aaron Conole]

Marcos Felipe Schwarz <marcos.f@gmail.com> writes:

> Since 2.8.0 OVS runs as non-root user on rhel distros, but the current
> implementation breaks the ability to run as root with DPDK and as a
> consequence there is no way possible to use UIO drivers on kernel 4.0 and
> newer [1, 2].
> [1]
> http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766bd95a658b5e0686a62ec04f5b0
> [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt
>
> Signed-off-by: Marcos Schwarz <marcos.f@gmail.com>
> ---

The patchwork version of this had problems applying (so I cc'd Stephen).
I manually applied this, and it works for me.

Acked-by: Aaron Conole <acon...@redhat.com>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2] rhel: Fix support for root user using DPDK

[Aaron Conole]

Marcos Felipe Schwarz  writes:

>  Since 2.8.0 OVS runs as non-root user on rhel distros, but the current
> implementation breaks the ability to run as root with DPDK and as a
> consequence there is no way possible to use UIO drivers on kernel 4.0 and
> newer [1, 2].
> [1] http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766bd95a658b5e0686a
> 62ec04f5b0
> [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt
>
> Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user")
> Signed-off-by: Marcos Schwarz 
> ---
>  lib/daemon-unix.c   | 3 ++-
>  rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index adb549c98..06528e9ab 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec)
>  }
>  }
>
> -switch_user = true;
> +if (!uid_verify(uid) || !gid_verify(gid))
> +switch_user = true;
>  }
> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in b/rhel/
> usr_lib_systemd_system_ovs-vswitchd.service.in
> index c6d9aa1b8..e8b81e707 100644
> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> @@ -14,7 +14,7 @@ Environment=HOME=/var/run/openvswitch
>  EnvironmentFile=/etc/openvswitch/default.conf
>  EnvironmentFile=-/etc/sysconfig/openvswitch
>  @begin_dpdk@
> -ExecStartPre=-/usr/bin/chown :hugetlbfs /dev/hugepages
> +ExecStartPre=-/bin/sh -c '/usr/bin/chown :${OVS_USER_ID##*:}

I think part of this hunk was lost when moving to v2.

If you post a v3, add my Acked-by.

Thanks, Marcos!

>  ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
>  @end_dpdk@
>  ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
> --
> 2.14.3
> ___
> 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] rhel: Fix support for root user using DPDK

[Aaron Conole]

Aaron Conole <acon...@redhat.com> writes:

> Ben Pfaff <b...@ovn.org> writes:
>
>> Well, is it ever useful to be able to drop unneeded capabilities while
>> retaining the same uid/gid?  It certainly sounds like a reasonable thing
>> to want to do.  I'm reluctant to apply this without at least considering
>> that possibility.
>
> Let me think about it a bit more.  When I originally suggested shunting
> the setuid code-path, I didn't consider this case.  There could be an
> alternative.
>
> I suggested this in response to the original proposal (add CAP_SYS_ADMIN
> to the list of retained privs).  Certainly, I don't want to allow
> CAP_SYS_ADMIN to be retained (after all, with CAP_NET_ADMIN and
> CAP_SYS_ADMIN, there's really not much reason to change uid from root at
> all - for all functional purposes the process will be root).
>
> Maybe there's a way to see that the user will be root from the
> systemd scripts and not pass the "--user=XXX:YYY" option.

Is the following patch a workable solution?  If so, I can post it
formally.  Marcos, please confirm that this resolves your issue?

---

diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
index c6d9aa1b8..889740f1a 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
@@ -13,17 +13,18 @@ Restart=on-failure
 Environment=HOME=/var/run/openvswitch
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
+EnvironmentFile=-/run/openvswitch/useropts
 @begin_dpdk@
-ExecStartPre=-/usr/bin/chown :hugetlbfs /dev/hugepages
+ExecStartPre=-/bin/sh -c '/usr/bin/chown :${OVS_USER_ID##*:} /dev/hugepages'
 ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
 @end_dpdk@
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovsdb-server --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
   --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   restart $OPTIONS
 TimeoutSec=300
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 234d39355..e05742d87 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -11,13 +11,15 @@ Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
 ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
+ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ 
"${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo 
"OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
+EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovs-vswitchd --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
-   --ovs-user=${OVS_USER_ID} \
+   ${OVSUSER} \
--no-monitor restart $OPTIONS
 RuntimeDirectory=openvswitch
 RuntimeDirectoryMode=0755
---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v5 00/11] Userspace datapath: Add fragmentation support.

[Aaron Conole]

Hi Darrell,

Darrell Ball  writes:

> Fragmentation support for userspace datapath conntrack is added; both
> v4 and v6 are supported. See the patches for additional details.

Very pumped about this!

I went to start reviewing this, but found out that 04/ didn't apply
correctly.  No problem, I'll proceed - just figured I'd let you know
that:

  1. I'm looking at it :)
  2. it'll need at least one more spin (but no need to rush that since
 I'll move myself past the error)

Thanks,
-Aaron

> v4->v5: Added a sub-feature to optionally dump fragmentation lists.
> This is useful for DOS forensics and debugging.
>
> The testing coverage was also extended including checking
> more counters and frag list occupancies.
>
> Fixed a few bugs: 
> 1/ Handle dpdk mempool source restrictions for a batch of
>packets from multiple sources; this also brings in a purge
>frag list function to handle pathological cases of stuck frags.
> 2/ ipf_destroy was missing packet frees for frag lists.
> 3/ A setting of CS_INVALID was missing for expired packets -
>I mentioned this earlier for version 4.
>
> Some enhancements and coding standards changes for Patch 3.   
>
> v3->v4: Add V6 support to the patches.
> Fix possible race cleanup bug when the user disables
>fragmentation and there are list occupancies, not cleaned up
>yet.
> Add missed orig tuple fields for copy from reassembled packet
> to fragments.
> Fix an fragment list increment check - shoiuld have been "> 0"
> rather then "!= 0".
> Fix max frags calculation in case of theoretical corner case.
> Add proper lock annotations.
> Made some other improvements while adding V6 support.
>
> v2->v3: Patch 2 was updated:
> Remove "XXX" todo items by implementing the ones needed,
> including realloc frag_list contexts to save memory.
> Fix related bug with max_frag_list_size when min_frag_size is
> reconfigured.
>
> Tighten ip_tot_len sanity check for reassembled packets which
> was more loose than intended.
>
> Add another sanity check for fragment ip_tot_len; even though
> it be redundant, add for completeness.
>
> v1->v2: Few fixes, improvements and cleanups.
>
> Darrell Ball (11):
>   dp-packet: Add const qualifiers for checksum apis.
>   flow: Enhance parse_ipv6_ext_hdrs.
>   Userspace datapath: Add fragmentation handling.
>   conntrack: Support fragmentation.
>   ipf: Add command to enable fragmentation handling.
>   ipf: Add set minimum fragment size command.
>   ipf: Add set maximum fragments supported command.
>   ipf: Add command to get fragmentation handling status.
>   ipf: Enhance ipf_get_status.
>   tests: Add missed local stack checks.
>   tests: Enable fragmentation for userspace datapath.
>
>  NEWS |   10 +
>  include/sparse/netinet/ip6.h |1 +
>  lib/automake.mk  |2 +
>  lib/conntrack.c  |   10 +-
>  lib/ct-dpif.c|   69 ++
>  lib/ct-dpif.h|   13 +
>  lib/dp-packet.h  |4 +-
>  lib/dpctl.c  |  216 ++
>  lib/dpctl.man|   32 +
>  lib/dpif-netdev.c|   83 +++
>  lib/dpif-netlink.c   |7 +
>  lib/dpif-provider.h  |   18 +
>  lib/flow.c   |   23 +-
>  lib/flow.h   |3 +-
>  lib/ipf.c| 1390 
> ++
>  lib/ipf.h|   86 +++
>  tests/system-kmod-macros.at  |   30 +-
>  tests/system-traffic.at  |   45 +-
>  tests/system-userspace-macros.at |  125 +++-
>  19 files changed, 2129 insertions(+), 38 deletions(-)
>  create mode 100644 lib/ipf.c
>  create mode 100644 lib/ipf.h
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/8] doc: Add an overview of the 'dpdk' port

[Aaron Conole]

Hi Stephen,

Stephen Finucane  writes:

> These ports are used to allow ingress/egress from the host and are
> therefore _reasonably_ important. However, there is no clear overview of
> what these ports actually are or why things are done the way they are.
> Start closing this gap by providing a standalone example of using these
> ports along with a little more detailed overview of the binding process.
>
> There is additional cleanup to be done for the DPDK howto, but that will
> be done separately.
>
> Signed-off-by: Stephen Finucane 
> Cc: Ciara Loftus 
> Cc: Kevin Traynor 
> ---
>  Documentation/topics/dpdk/index.rst |   1 +
>  Documentation/topics/dpdk/phy.rst   | 111 
> 
>  2 files changed, 112 insertions(+)
>  create mode 100644 Documentation/topics/dpdk/phy.rst
>
> diff --git a/Documentation/topics/dpdk/index.rst 
> b/Documentation/topics/dpdk/index.rst
> index da148b323..5f836a6e9 100644
> --- a/Documentation/topics/dpdk/index.rst
> +++ b/Documentation/topics/dpdk/index.rst
> @@ -28,5 +28,6 @@ The DPDK Datapath
>  .. toctree::
> :maxdepth: 2
>  
> +   phy
> vhost-user
> ring
> diff --git a/Documentation/topics/dpdk/phy.rst 
> b/Documentation/topics/dpdk/phy.rst
> new file mode 100644
> index 0..1c18e4e3d
> --- /dev/null
> +++ b/Documentation/topics/dpdk/phy.rst
> @@ -0,0 +1,111 @@
> +..
> +  Copyright 2018, 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.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +DPDK Physical Ports
> +===
> +
> +The DPDK datapath provides a way to attach DPDK-backed physical
> interfaces to

I think it may make more sense:

  The netdev datapath allows attaching DPDK-backed physical interfaces...

> +allow high-performance ingress/egress from the host.
> +
> +.. versionchanged:: 2.7.0
> +
> +   Before Open vSwitch 2.7.0, it was necessary to prefix port names with a
> +   ``dpdk`` prefix. Starting with 2.7.0, this is no longer necessary.
> +
> +Quick Example
> +-
> +
> +This example demonstrates how to bind two ``dpdk`` ports, bound to physical
> +interfaces identified by hardware IDs ``:01:00.0`` and ``:01:00.1``, 
> to
> +an existing bridge called ``br0``::
> +
> +$ ovs-vsctl add-port br0 dpdk-p0 \
> +   -- set Interface dpdk-p0 type=dpdk options:dpdk-devargs=:01:00.0
> +$ ovs-vsctl add-port br0 dpdk-p1 \
> +   -- set Interface dpdk-p1 type=dpdk options:dpdk-devargs=:01:00.1
> +
> +For the above example to work, the two physical interfaces must be bound to
> +the DPDK poll-mode drivers in userspace rather than the traditional kernel
> +drivers. See the `binding NIC drivers ` section for 
> details.
> +
> +.. _dpdk-binding-nics:
> +
> +Binding NIC Drivers
> +---
> +
> +DPDK operates entirely in userspace and, as a result, requires use of its own
> +poll-mode drivers in user space for physical interfaces and a 
> passthrough-style
> +driver for the devices in kernel space.
> +
> +There are two different tools for binding drivers: :command:`driverctl` which
> +is a generic tool for persistently configuring alternative device drivers, 
> and
> +:command:`dpdk-devbind` which is a DPDK-specific tool and whose changes do 
> not
> +persist across reboots. In addition, there are two options available for this
> +kernel space driver - VFIO (Virtual Function I/O) and UIO (Userspace I/O) -
> +along with a number of drivers for each option. We will demonstrate examples 
> of
> +both tools and will use the ``vfio-pci`` driver, which is the more secure,
> +robust driver of those available. More information can be found in the `DPDK
> +documentation `__.

Just commentary - really happy to see driverctl getting some love.

> +
> +To list devices using :command:`driverctl`, run::
> +
> +$ driverctl -v list-devices | grep -i net
> +:07:00.0 igb (I350 Gigabit Network Connection (Ethernet Server 
> Adapter I350-T2))
> +:07:00.1 igb (I350 Gigabit Network Connection (Ethernet Server 

[ovs-dev] [PATCH] rhel: don't drop capabilities when running as root

[Aaron Conole]

Currently, regardless of which user is being set as the running user,
Open vSwitch daemons on RHEL systems drop capabilities.  This means the
very powerful CAP_SYS_ADMIN is dropped, even when the user is 'root'.

For the majority of use cases this behavior works, as the user can
enable or disable various configurations, regardless of which datapath
functions are desired.  However, when using certain DPDK PMDs, the
enablement and configuration calls require CAP_SYS_ADMIN.

Instead of retaining CAP_SYS_ADMIN in all cases, which would practically
nullify the uid/gid and privilege drop, we don't pass the --ovs-user
option to the daemons.  This shunts the capability and privilege
dropping code.

Reported-by: Marcos Felipe Schwarz <marcos.f@gmail.com>
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045955.html
Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user")
Signed-off-by: Aaron Conole <acon...@redhat.com>
---
NOTE: I did test this a little bit on my system, passing packets, etc.
  But more eyes can't be bad.

 rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 7 ---
 rhel/usr_lib_systemd_system_ovsdb-server.service| 6 --
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
index c6d9aa1..889740f 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
@@ -13,17 +13,18 @@ Restart=on-failure
 Environment=HOME=/var/run/openvswitch
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
+EnvironmentFile=-/run/openvswitch/useropts
 @begin_dpdk@
-ExecStartPre=-/usr/bin/chown :hugetlbfs /dev/hugepages
+ExecStartPre=-/bin/sh -c '/usr/bin/chown :${OVS_USER_ID##*:} /dev/hugepages'
 ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
 @end_dpdk@
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovsdb-server --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
   --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   restart $OPTIONS
 TimeoutSec=300
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 234d393..e05742d 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -11,13 +11,15 @@ Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
 ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
+ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ 
"${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo 
"OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
+EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovs-vswitchd --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
-   --ovs-user=${OVS_USER_ID} \
+   ${OVSUSER} \
--no-monitor restart $OPTIONS
 RuntimeDirectory=openvswitch
 RuntimeDirectoryMode=0755
--
2.9.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] selinux: add a new target to build the policy

[Aaron Conole]

The selinux policy currently builds manually, as a process that either
the user or distribution maintainer undertakes.  That process consists
of:

  1. Convert the intermediary files into their file form through
 'make' statements at the top level.

  2. Change to the selinux directory and issue the selinux "make -f"
 directive.

This commit introduces a new target 'selinux-policy' which builds the
openvswitch-custom policy files.

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
 rhel/openvswitch-fedora.spec.in | 4 +---
 rhel/openvswitch.spec.in| 4 +---
 selinux/automake.mk | 5 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index ed991cf07..a57abf616 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -239,9 +239,7 @@ build-aux/dpdkstrip.py \
> rhel/usr_lib_systemd_system_ovs-vswitchd.service
 
 make %{?_smp_mflags}
-make selinux/openvswitch-custom.te
-cd selinux
-make -f %{_datadir}/selinux/devel/Makefile
+make selinux-policy
 
 %install
 rm -rf $RPM_BUILD_ROOT
diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
index e510d351b..a15868f9f 100644
--- a/rhel/openvswitch.spec.in
+++ b/rhel/openvswitch.spec.in
@@ -68,9 +68,7 @@ Tailored Open vSwitch SELinux policy
 ./configure --prefix=/usr --sysconfdir=/etc --localstatedir=%{_localstatedir} \
 --libdir=%{_libdir} --enable-ssl --enable-shared
 make %{_smp_mflags}
-make selinux/openvswitch-custom.te
-cd selinux
-make -f %{_datadir}/selinux/devel/Makefile
+make selinux-policy
 
 %install
 rm -rf $RPM_BUILD_ROOT
diff --git a/selinux/automake.mk b/selinux/automake.mk
index e8871aa97..48853cdc0 100644
--- a/selinux/automake.mk
+++ b/selinux/automake.mk
@@ -7,3 +7,8 @@
 
 EXTRA_DIST += \
 selinux/openvswitch-custom.te.in
+
+PHONY: selinux-policy
+
+selinux-policy: selinux/openvswitch-custom.te
+   $(MAKE) -C selinux/ -f /usr/share/selinux/devel/Makefile
-- 
2.14.3

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

[ovs-dev] [PATCH] selinux: allow dpdkvhostuserclient sockets with newer libvirt

[Aaron Conole]

Newer libvirt and openstack versions will now label the unix socket as
an `svirt_tmpfs_t` object.  This means that in order to support
deploying with the recommended configuration (using a
dpdkvhostuserclient socket), additional permissions need to be
installed as part of the selinux policy.

An example of some of the AVC violations:

type=AVC msg=audit(1518752799.102:978): avc:  denied  { write }
for  pid=14368 comm="ovs-vswitchd" name="vhost0" dev="dm-0" ino=94
scontext=system_u:system_r:openvswitch_t:s0
tcontext=system_u:object_r:svirt_tmp_t:s0 tclass=sock_file

type=AVC msg=audit(1518816172.126:1318): avc:  denied  { connectto }
for  pid=32717 comm="ovs-vswitchd" path="/tmp/vhost0"
scontext=system_u:system_r:openvswitch_t:s0
tcontext=system_u:system_r:svirt_t:s0:c106,c530
    tclass=unix_stream_socket

Signed-off-by: Aaron Conole <acon...@redhat.com>
---
 selinux/openvswitch-custom.te.in | 5 +
 1 file changed, 5 insertions(+)

diff --git a/selinux/openvswitch-custom.te.in b/selinux/openvswitch-custom.te.in
index c1a774f0e..7b9c1c7a0 100644
--- a/selinux/openvswitch-custom.te.in
+++ b/selinux/openvswitch-custom.te.in
@@ -14,6 +14,7 @@ require {
 type hugetlbfs_t;
 type kernel_t;
 type svirt_image_t;
+type svirt_tmpfs_t;
 type vfio_device_t;
 @end_dpdk@
 
@@ -26,6 +27,7 @@ require {
 class unix_stream_socket { write getattr read connectto connect setopt 
getopt sendto accept bind recvfrom acceptfrom };
 
 @begin_dpdk@
+class sock_file { read write append getattr open };
 class tun_socket { relabelfrom relabelto create };
 @end_dpdk@
 }
@@ -50,5 +52,8 @@ allow openvswitch_t hugetlbfs_t:file { create unlink };
 allow openvswitch_t kernel_t:unix_stream_socket { write getattr read connectto 
connect setopt getopt sendto accept bind recvfrom acceptfrom };
 allow openvswitch_t self:tun_socket { relabelfrom relabelto create };
 allow openvswitch_t svirt_image_t:file { getattr read write };
+allow openvswitch_t svirt_tmpfs_t:file { read write };
+allow openvswitch_t svirt_tmpfs_t:sock_file { read write append getattr open };
+allow openvswitch_t svirt_t:unix_stream_socket { connectto read write getattr 
sendto recvfrom setopt };
 allow openvswitch_t vfio_device_t:chr_file { read write open ioctl getattr };
 @end_dpdk@
-- 
2.14.3

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

Re: [ovs-dev] [patch v5 00/11] Userspace datapath: Add fragmentation support.

[Aaron Conole]

Darrell Ball <dlu...@gmail.com> writes:

> On Fri, Feb 9, 2018 at 1:35 PM, Aaron Conole <acon...@redhat.com> wrote:
>
>  Hi Darrell,
>
>  Darrell Ball <dlu...@gmail.com> writes:
>
>  > Fragmentation support for userspace datapath conntrack is added; both
>  > v4 and v6 are supported. See the patches for additional details.
>
>  Very pumped about this!
>
>  I went to start reviewing this, but found out that 04/ didn't apply
>  correctly.  No problem, I'll proceed - just figured I'd let you know
>  that:
>
> The merge conflict in NEWS was due to 2 subsequent applies on Feb 6, while
> the series was posted on Feb 4.

Yep.  I've worked around it.

>1. I'm looking at it :)
>
> Thanks Aaron

I have a question on patches 5-7:

  Do you think it's worthwhile having these as configuration options in
  the database?

I can see where it could be considered that they aren't appropriate
(after all, the linux fragmentation support is enabled/disabled not via
the netlink interface, but by the system administrator outside of the
ovs configuration).  On the other hand, from a user perspective it's
probably more friendly to have that configuration knob be persistent.

I think there is value in having the configuration for ipfrag be stored
in a database so that each 'restart' doesn't require an extra set of
commands be run.  I envision a situation where users have their system
configured, restart, and get confused because they have to manually
re-enable the configuration.  There are a few ways this could be done
(for instance, a post-start script that runs after ovs-ctl), but I think
the database might be a useful way of accomplishing that goal - it
exists and we use it for dpdk configurations, ex.

Something to think on.

I'm planning on running performance tests this coming week, and will
give my patch 3/11 comments then.

Thanks for working on this, Darrell!

> Darrell
>
>  
>2. it'll need at least one more spin (but no need to rush that since
>   I'll move myself past the error)  
>
>  Thanks,
>  -Aaron
>
>  > v4->v5: Added a sub-feature to optionally dump fragmentation lists.
>  > This is useful for DOS forensics and debugging.
>  >
>  > The testing coverage was also extended including checking
>  > more counters and frag list occupancies.
>  >
>  > Fixed a few bugs:
>  > 1/ Handle dpdk mempool source restrictions for a batch of
>  >packets from multiple sources; this also brings in a purge
>  >frag list function to handle pathological cases of stuck frags.
>  > 2/ ipf_destroy was missing packet frees for frag lists.
>  > 3/ A setting of CS_INVALID was missing for expired packets -
>  >I mentioned this earlier for version 4.
>  >
>  > Some enhancements and coding standards changes for Patch 3.
>  >
>  > v3->v4: Add V6 support to the patches.
>  > Fix possible race cleanup bug when the user disables
>  >fragmentation and there are list occupancies, not cleaned up
>  >yet.
>  > Add missed orig tuple fields for copy from reassembled packet
>  > to fragments.
>  > Fix an fragment list increment check - shoiuld have been "> 0"
>  > rather then "!= 0".
>  > Fix max frags calculation in case of theoretical corner case.
>  > Add proper lock annotations.
>  > Made some other improvements while adding V6 support.
>  >
>  > v2->v3: Patch 2 was updated:
>  > Remove "XXX" todo items by implementing the ones needed,
>  > including realloc frag_list contexts to save memory.
>  > Fix related bug with max_frag_list_size when min_frag_size is
>  > reconfigured.
>  >
>  > Tighten ip_tot_len sanity check for reassembled packets which
>  > was more loose than intended.
>  >
>  > Add another sanity check for fragment ip_tot_len; even though
>  > it be redundant, add for completeness.
>  >
>  > v1->v2: Few fixes, improvements and cleanups.
>  >
>  > Darrell Ball (11):
>  >   dp-packet: Add const qualifiers for checksum apis.
>  >   flow: Enhance parse_ipv6_ext_hdrs.
>  >   Userspace datapath: Add fragmentation handling.
>  >   conntrack: Support fragmentation.
>  >   ipf: Add command to enable fragmentation handling.
>  >   ipf: Add set minimum fragment size command.
>  >   ipf: Add set maximum fragments supported command.
>  >   ipf: Add command to get fragmentation handling status.
>  >   ipf: Enhance ipf_g

Re: [ovs-dev] [PATCH] Set release dates for 2.9.0.

[Aaron Conole]

Justin Pettit <jpet...@ovn.org> writes:

> Signed-off-by: Justin Pettit <jpet...@ovn.org>
> ---
>  NEWS | 2 +-
>  debian/changelog | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 62a979e1e9f6..311f3a426cf3 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,4 +1,4 @@
> -v2.9.0 - xx xxx 
> +v2.9.0 - 19 Feb 2018
>  
> - NSH implementation now conforms to latest draft (draft-ietf-sfc-nsh-28).
>   * Add ttl field.
> diff --git a/debian/changelog b/debian/changelog
> index 64b8f08ee931..e68407834c61 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -2,7 +2,7 @@ openvswitch (2.9.0-1) unstable; urgency=low
>  
> * New upstream version
>  
> - -- Open vSwitch team <d...@openvswitch.org>  Wed, 17 Jan 2018 09:49:31 -0700
> + -- Open vSwitch team <d...@openvswitch.org>  Wed, 19 Feb 2018 11:03:12 -0700

Just a nit - 19 Feb is a Mon. :)

Otherwise,

Acked-by: Aaron Conole <acon...@redhat.com>

>  openvswitch (2.8.0-1) unstable; urgency=medium
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [branch-2.8 1/2] Set release date for 2.8.2.

[Aaron Conole]

Justin Pettit <jpet...@ovn.org> writes:

> Signed-off-by: Justin Pettit <jpet...@ovn.org>
> ---

Acked-by: Aaron Conole <acon...@redhat.com>

>  NEWS | 4 ++--
>  debian/changelog | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 17495dd6e75a..48dc04daeb43 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,7 +1,7 @@
> -v2.8.2 - xx xxx 
> +v2.8.2 - 19 Feb 2018
>  -
> - NSH implementation now conforms to latest draft (draft-ietf-sfc-nsh-28).
> -
> +   - Bug fixes
>  
>  v2.8.1 - 26 Sep 2017
>  -
> diff --git a/debian/changelog b/debian/changelog
> index b536a44dc528..d1298f5e3187 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -3,7 +3,7 @@ openvswitch (2.8.2-1) unstable; urgency=medium
> [ Open vSwitch team ]
> * New upstream version
>  
> - -- Open vSwitch team <d...@openvswitch.org>  Tue, 26 Sep 2017 13:34:23 -0700
> + -- Open vSwitch team <d...@openvswitch.org>  Mon, 19 Feb 2018 11:12:33 -0700
>  
>  openvswitch (2.8.1-1) unstable; urgency=medium
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [branch-2.8 2/2] Prepare for 2.8.3.

[Aaron Conole]

Justin Pettit <jpet...@ovn.org> writes:

> Signed-off-by: Justin Pettit <jpet...@ovn.org>
> ---

Acked-by: Aaron Conole <acon...@redhat.com>

>  NEWS | 4 
>  configure.ac | 2 +-
>  debian/changelog | 8 
>  3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index 48dc04daeb43..7c219f4034d4 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,7 @@
> +v2.8.3 - xx xxx 
> +-
> +
> +
>  v2.8.2 - 19 Feb 2018
>  -
> - NSH implementation now conforms to latest draft (draft-ietf-sfc-nsh-28).
> diff --git a/configure.ac b/configure.ac
> index 46c00ad2dc25..a90318e639ae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>  
>  AC_PREREQ(2.63)
> -AC_INIT(openvswitch, 2.8.2, b...@openvswitch.org)
> +AC_INIT(openvswitch, 2.8.3, b...@openvswitch.org)
>  AC_CONFIG_SRCDIR([datapath/datapath.c])
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
> diff --git a/debian/changelog b/debian/changelog
> index d1298f5e3187..6bf6ed584eeb 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,11 @@
> +openvswitch (2.8.3-1) unstable; urgency=medium
> +
> +   [ Open vSwitch team ]
> +   * New upstream version
> +
> + -- Open vSwitch team <d...@openvswitch.org>  Mon, 19 Feb 2018 11:14:43 -0700
> +
> +
>  openvswitch (2.8.2-1) unstable; urgency=medium
>  
> [ Open vSwitch team ]
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [branch-2.9] Prepare for 2.9.1.

[Aaron Conole]

Justin Pettit <jpet...@ovn.org> writes:

> Signed-off-by: Justin Pettit <jpet...@ovn.org>
> ---

Acked-by: Aaron Conole <acon...@redhat.com>

>  NEWS | 4 
>  configure.ac | 2 +-
>  debian/changelog | 6 ++
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index 311f3a426cf3..180099379548 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,3 +1,7 @@
> +v2.9.1 - xx xxx 
> +
> +
> +
>  v2.9.0 - 19 Feb 2018
>  
> - NSH implementation now conforms to latest draft (draft-ietf-sfc-nsh-28).
> diff --git a/configure.ac b/configure.ac
> index 635beb6ea962..4d7bd8d754d0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>  
>  AC_PREREQ(2.63)
> -AC_INIT(openvswitch, 2.9.0, b...@openvswitch.org)
> +AC_INIT(openvswitch, 2.9.1, b...@openvswitch.org)
>  AC_CONFIG_SRCDIR([datapath/datapath.c])
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
> diff --git a/debian/changelog b/debian/changelog
> index 5b880d0bde0f..a2c95eb5bf5b 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +openvswitch (2.9.1-1) unstable; urgency=low
> +
> +   * New upstream version
> +
> + -- Open vSwitch team <d...@openvswitch.org>  Mon, 19 Feb 2018 14:34:32 -0700
> +
>  openvswitch (2.9.0-1) unstable; urgency=low
>  
> * New upstream version
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Set release dates for 2.9.0.

[Aaron Conole]

Justin Pettit <jpet...@ovn.org> writes:

>> On Feb 19, 2018, at 1:38 PM, Aaron Conole <acon...@redhat.com> wrote:
>> 
>> Justin Pettit <jpet...@ovn.org> writes:
>> 
>>> Signed-off-by: Justin Pettit <jpet...@ovn.org>
>>> ---
>>> NEWS | 2 +-
>>> debian/changelog | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/NEWS b/NEWS
>>> index 62a979e1e9f6..311f3a426cf3 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -1,4 +1,4 @@
>>> -v2.9.0 - xx xxx 
>>> +v2.9.0 - 19 Feb 2018
>>> 
>>>- NSH implementation now conforms to latest draft 
>>> (draft-ietf-sfc-nsh-28).
>>>  * Add ttl field.
>>> diff --git a/debian/changelog b/debian/changelog
>>> index 64b8f08ee931..e68407834c61 100644
>>> --- a/debian/changelog
>>> +++ b/debian/changelog
>>> @@ -2,7 +2,7 @@ openvswitch (2.9.0-1) unstable; urgency=low
>>> 
>>>* New upstream version
>>> 
>>> - -- Open vSwitch team <d...@openvswitch.org>  Wed, 17 Jan 2018 09:49:31 
>>> -0700
>>> + -- Open vSwitch team <d...@openvswitch.org>  Wed, 19 Feb 2018 11:03:12 
>>> -0700
>> 
>> Just a nit - 19 Feb is a Mon. :)
>
> Yeah, I noticed that right after I sent it.  :-)
>
>> Otherwise,
>> 
>> Acked-by: Aaron Conole <acon...@redhat.com>
>
> I pushed it to branch-2.9 with your and Flavio's Acked-bys.
>
> I forgot the patch to prep for 2.9.1.   Can one of you reivew "Prepare for 
> 2.9.1."?
>
> Thanks!

Got you covered.

$release is dead!  Long live $release!

> --Justin
>
>
> ___
> 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] ovs-vsctl: show DPDK version

[Aaron Conole]

Hi Matteo,

Matteo Croce  writes:

> Show DPDK version if Open vSwitch is compiled with DPDK support.
> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py
> to populate the field with the right value.
>
> Signed-off-by: Matteo Croce 
> ---

Sorry I missed some of the development for this - I think it's better to
have the dpdk version retrieved via an appctl command.  I can't think of
a need for this information to be persistent - if it's for historical
information when debugging, we can put it in with the logs when dpdk is
started.  The user cannot influence DPDK version dynamically - it is not
just read-only, but it's read-only from a compile time decision.  I
might be misunderstanding the point of putting this in the DB, though?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] ovs-vsctl: show DPDK version

[Aaron Conole]

Matteo Croce <mcr...@redhat.com> writes:

> On Tue, Jan 2, 2018 at 6:08 PM, Aaron Conole <acon...@redhat.com> wrote:
>> Hi Matteo,
>>
>> Matteo Croce <mcr...@redhat.com> writes:
>>
>>> Show DPDK version if Open vSwitch is compiled with DPDK support.
>>> Add a `dpdk_version` column in the datamodel, change ovs-ctl and ovs-dev.py
>>> to populate the field with the right value.
>>>
>>> Signed-off-by: Matteo Croce <mcr...@redhat.com>
>>> ---
>>
>> Sorry I missed some of the development for this - I think it's better to
>> have the dpdk version retrieved via an appctl command.  I can't think of
>> a need for this information to be persistent - if it's for historical
>> information when debugging, we can put it in with the logs when dpdk is
>> started.  The user cannot influence DPDK version dynamically - it is not
>> just read-only, but it's read-only from a compile time decision.  I
>> might be misunderstanding the point of putting this in the DB, though?
>
> Hi Aaron,
>
> I was threating dpdk_version the same way as ovs_version.

Ahh, okay.  I had forgotten that ovs_version (and other information) was
in the db as well.  Sorry for that - next time I'll have coffee before
reviewing. :)

> ovs_version is saved into the DB by ovs-ctl and it isn't read-only either.
> What about putting it only in `ovs-vswitchd --version` output and not
> into the DB?

What about using the datapath_version field?  That field already gets
populated, and I think it may be okay to modify the return value of
dpif_netdev_get_datapath_version function when dpdk is enabled.  Just a
thought.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/3] Initial support for new SIP Alg.

[Aaron Conole]

Tiago Lam  writes:

> This patch-set is an initial approach at implementing the new SIP Alg,
> mentioned by Aaron at [1].

Thanks for this work, Tiago!

> I'm mostly interested in getting to know your thoughts of how this is
> headed. There are a couple of points that are worth bringing up:
> - As mentioned in patches 1/3 and 2/3, this is still a preliminary
>   implementation, and some work will be needed to move away from some
>   assuptions, like assuming the SIP traffic is always going over IPv4
>   and TCP;
> - At the moment, the sip state is being stored in the conn struct. I
>   followed the example of seq_skew_dir here, which is also stored there,
>   but realise this is not ideal. It seems storing it somewhere agnostic
>   will be ideal in the future, to avoid polluting that struct with
>   different Alg's details;
> - The SIP helpers functions and structures are in conntrack-sip.h and
>   conntrack-sip.c. This can create confusion when comparing to
>   conntrack-tcp.c and other protocols since SIP is an Alg and is at a
>   different level.
>
> With regards to testing, for now, this has been tested manually, by
> setting up the flows mentioned in patch 2/3 and having two VMs connected
> to OvS, both using SIPp to simulate real traffic both ways. I'm going to
> have a look at how this can be automated and added to
> tests/system-traffic.at, together with the rest of the already existing
> tests.

Please do.  I would consider the test-suite an important part for
acceptance.

> [1] [CONNTRACK] Discussions at OvS 2017:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341089.html
>
> Tiago Lam (3):
>   Conntrack: Add new API for future SIP Alg.
>   Conntrack: Add initial support for new SIP Alg.
>   Conntrack: Support asymmetric RTP port for SIP.
>
>  include/openvswitch/ofp-actions.h |   4 +
>  lib/automake.mk   |   2 +
>  lib/conntrack-private.h   |   2 +
>  lib/conntrack-sip.c   | 491 
> ++
>  lib/conntrack-sip.h   | 123 ++
>  lib/conntrack.c   | 254 +++-
>  lib/ofp-parse.c   |   5 +
>  ofproto/ofproto-dpif-xlate.c  |   3 +
>  8 files changed, 883 insertions(+), 1 deletion(-)
>  create mode 100644 lib/conntrack-sip.c
>  create mode 100644 lib/conntrack-sip.h
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Automated robotic reply. Re: ...

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Justin Pettit, I am a robot and I have tried out your 
> patch
> with message ID  <1529454160-88413-1-git-send-email-jpet...@ovn.org>
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> checkpatch:
> == Checking ".patch" ==
> ERROR: Too many signoffs; are you missing Co-authored-by lines?
> Lines checked: 118, Warnings: 0, Errors: 1

I'll fix the sign off check in checkpatch.  It shouldn't flag on these
kinds of patches.

Sorry for the noise.

>
> Please check this out.  If you feel there has been an error, please email me 
> back.
>
> Thanks,
> 0-day Robot
> ___
> 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] AUTHORS: Update email address for Jakub Sitnicki.

[Aaron Conole]

Jakub Sitnicki  writes:

> Signed-off-by: Jakub Sitnicki 
> ---
>
> The j...@redhat.com address will be valid only until August 4th.

:'(

With sadness:

Acked-by: Aaron Conole 

>  AUTHORS.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/AUTHORS.rst b/AUTHORS.rst
> index 25ee39d75..f3ac8e32b 100644
> --- a/AUTHORS.rst
> +++ b/AUTHORS.rst
> @@ -161,7 +161,7 @@ Isaku Yamahata yamah...@valinux.co.jp
>  Ivan Dyukovi.dyu...@samsung.com
>  IWASE Yusuke   iwase.yus...@gmail.com
>  Jakub Libosvar libos...@redhat.com
> -Jakub Sitnicki j...@redhat.com
> +Jakub Sitnicki jsitni...@gmail.com
>  James P.   roamp...@gmail.com
>  James Page james.p...@ubuntu.com
>  Jamie Lennox   jamielen...@gmail.com
> --
> 2.14.4
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Ben Pfaff  writes:

> I don't understand the conclusion in this thread.  Does anyone want me
> to apply some patch?  Or should I stay tuned for further discussion?

Stay tuned for the exciting conclusion, please :)

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

Re: [ovs-dev] [PATCH 1/9] tests: Simplify the setting of aliases.

[Aaron Conole]

Hi Ilya,

Ilya Maximets  writes:

> There is no need to create a separate function for each alias.
> This will simplify adding new default options and utils.
>
> Signed-off-by: Ilya Maximets 
> ---
>  tests/ovs-macros.at | 35 +--
>  1 file changed, 5 insertions(+), 30 deletions(-)
>
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 677eea7..e3365b6 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -124,36 +124,11 @@ fi
>  # Set default timeout for 30 seconds.
>  # This should be sufficient on all platforms.
>  OVS_TIMEOUT=30
> -alias ovs-vsctl='OVS_VSCTL_TIMEOUT' >/dev/null 2>&1
> -if [ $? -eq 0 ]; then
> -OVS_VSCTL_TIMEOUT () {
> -command ovs-vsctl --timeout=$OVS_TIMEOUT "$@"
> -}
> -alias ovs-ofctl='OVS_OFCTL_TIMEOUT'
> -alias ovs-appctl='OVS_APPCTL_TIMEOUT'
> -alias ovn-sbctl='OVS_SBCTL_TIMEOUT'
> -alias ovn-nbctl='OVN_NBCTL_TIMEOUT'
> -alias vtep-ctl='VTEP_CTL_TIMEOUT'
> -alias ovsdb-client='OVSDB_CLIENT_TIMEOUT'
> -OVS_OFCTL_TIMEOUT () {
> -command ovs-ofctl --timeout=$OVS_TIMEOUT "$@"
> -}
> -OVS_APPCTL_TIMEOUT () {
> -command ovs-appctl --timeout=$OVS_TIMEOUT "$@"
> -}
> -OVS_SBCTL_TIMEOUT () {
> -command ovn-sbctl --timeout=$OVS_TIMEOUT "$@"
> -}
> -OVN_NBCTL_TIMEOUT () {
> -command ovn-nbctl --timeout=$OVS_TIMEOUT "$@"
> -}
> -VTEP_CTL_TIMEOUT () {
> -command vtep-ctl --timeout=$OVS_TIMEOUT "$@"
> -}
> -OVSDB_CLIENT_TIMEOUT () {
> -command ovsdb-client --timeout=$OVS_TIMEOUT "$@"
> -}
> -fi
> +OVS_UTILS_LIST="ovs-vsctl ovs-ofctl ovs-appctl ovn-sbctl ovn-nbctl
> +vtep-ctl ovsdb-client"
> +for util in $OVS_UTILS_LIST; do
> +alias $util="$util $OVS_TIMEOUT" >/dev/null 2>&1

Maybe I misunderstood something - should this be?

+alias $util="$util --timeout=$OVS_TIMEOUT" >/dev/null 2>&1

> +done
>  
>  # parent_pid PID
>  #
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: don't use a bashism to check that datapath exists

[Aaron Conole]

Timothy Redaelli  writes:

> [[ ]] syntax is not supported, at least, by dash that Debian, Ubuntu and other
> linux distributions may use instead of bash.
>
> This commit uses, instead, a POSIX way that is compatible with any POSIX
> shell (bash, dash, busybox sh, etc).
>
> CC: Martin Xu 
> Fixes: 9763d17fbd05 ("utilities: check datapath exists before conntrack 
> flush")
>
> Signed-off-by: Timothy Redaelli 
> ---

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

Re: [ovs-dev] Revert "dp-packet: Handle multi-seg mbufs in resize__()."

[Aaron Conole]

Ian Stokes  writes:

> On 7/25/2018 2:56 PM, Aaron Conole wrote:
>> 0-day Robot  writes:
>>
>>> Bleep bloop.  Greetings Tiago Lam, 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.
>>>
>>
>> Hi Ian (and all),
>>
>> Given there are currently two trees (mainline and dpdk_merge), do you
>> have any preferences, suggestions, or comments on managing the robot
>> w.r.t. these kind of tree-specific patches?  I suspect that we'll have
>> others in the future, and I'd like to make the robot be a bit friendlier
>> this way (plus it helps to build confidence when there are fewer
>> false-positives).
>>
>> -Aaron
>
> Is it that you'd like to see 0-day running on dpdk_merge also?

I think I was wondering if you would like to see it running against
dpdk_merge. :)

> From my side I think the revert here was an exception, as the pull
> request had not been merged at this point but the assumption was that
> it had been. I encourage people to base patches on what is in master,
> not dpdk_merge.

Good to know.  As a side, maybe it's time to formally document things so
that folks will base their contributions on the mainline always?
The growing pains of multi-tree projects :)

> It's a valid point however, I'm in favor of keeping the mainline as
> clean as possible so typically with a pull request I'll rebase
> dpdk_merge to the head of master, then apply dpdk specific patches on
> top of that so as to ensure its a clean pull request when applying to
> mainline.
>
> If an error does occurs (such as the revert, or a major bug discovery)
> I'll typically rework the patches at the head of dpdk_merge to take
> this into account. I didnt want to see a comit and a revert going into
> the mainline if possible.
>
> I'm happy for 0-day to work on master as is for the moment and
> coordinate with a submitter if a patch is needed and how we handle it?

Okay, that makes sense to me.  For now, I'll leave it running as-is.

Thanks, Ian!

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

Re: [ovs-dev] [PATCH v2 0/9] tests: Clean up syslog.

[Aaron Conole]

Ilya Maximets  writes:

> Each run of the testsuite produces millions lines in a system
> log. This is completely unnecessary and makes it difficult to
> use system logs on test / build servers.
>
> This series is aimed to disable most of the syslog messages.
> There are still few logs that requires significant changes in
> tests or code to disable. They will be removed separately if
> needed.
>
> Some testing results:
>   OS : RHEL 7.5
>   CPU: Xeon E5 v4 2.6GHz
>   Cmd: make check TESTSUITEFLAGS='-j20'
>
>   Without patches:
>   * 3.350.097 Lines of logs in journalctl
>   * Execution time: 11 minutes
>   * jourlald eats 100% of one cpu core.
>
>   With patch-set applied:
>   * 226 Lines of logs in journalctl
>   * Execution time: 2.5 minutes
>
> So, in addition to clean logs, this patch-set significantly
> speeds up the testsuite execution in parralel builds (more
> than 4 times! in my case).
>
> Side effects:
>   * default timeout applied to control utils in a subshell.
>   * tests refactored to be more readable.
>   * testsuite execution speed up.
>
> Version 2:
>   * Fixed accidentially missed '--timeout' in patches 1 and 2. [Aaron]
>
> Ilya Maximets (9):
>   tests: Simplify the setting of aliases.
>   tests: Set default timeout for utils in subshell.
>   tests: Disable syslog by default for control utils.
>   tests: Disable syslog for daemons.
>   tests: Enable only file logging by vlog/set appctl.
>   tests: Drop full logging for ovs-ofctl.
>   tests: Disable syslog for test utils.
>   tests: Reorder logging args for ovn-sbctl in a subshell.
>   tests: Disable syslog for ovsdb-tool.
>
>  tests/bridge.at   |   3 +-
>  tests/dpif-netdev.at  |  10 +-
>  tests/learn.at|   6 +-
>  tests/mpls-xlate.at   |   4 +-
>  tests/ofproto-dpif.at | 138 +
>  tests/ofproto-macros.at   |  22 +++-
>  tests/ofproto.at  |  20 ++--
>  tests/ovn-controller-vtep.at  |  35 +--
>  tests/ovn-nbctl.at|   4 +-
>  tests/ovn-sbctl.at|  19 +++-
>  tests/ovs-macros.at   |  39 ++--
>  tests/ovs-ofctl.at|   4 +-
>  tests/ovs-vsctl.at|  21 +++-
>  tests/ovs-vswitchd.at |  17 +++-
>  tests/ovsdb-cluster.at|   8 +-
>  tests/ovsdb-idl.at|  37 +--
>  tests/ovsdb-lock.at   |   6 +-
>  tests/ovsdb-monitor.at|  23 -
>  tests/ovsdb-rbac.at   |   3 +-
>  tests/ovsdb-server.at | 227 
> --
>  tests/ovsdb.at|   2 +-
>  tests/pmd.at  |  20 ++--
>  tests/stp.at  |   6 +-
>  tests/system-traffic.at   |   4 +-
>  tests/tunnel-push-pop-ipv6.at |   2 +-
>  tests/tunnel-push-pop.at  |   2 +-
>  tests/vlog.at |  15 +--
>  tests/vtep-ctl.at |   4 +-
>  28 files changed, 463 insertions(+), 238 deletions(-)

For the series:

Acked-by: Aaron Conole 

As a note, I saw a mix of '--detach' and '&' being used to background
various invocations of ovs-vswitchd and ovsdb-server.  Is there a
reason to prefer one vs the other?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, v3, 2 of 6] ipsec: reintroduce IPsec support for tunneling

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Qiuyu Xiao, 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: Too many signoffs; are you missing Co-authored-by lines?

My understanding is that there should be a 'Signed-off-by' tag from the
principal or submitter, and a Signed-off-by: and Co-authored-by from
each co-author.

Maybe it would be better match the tags and ignore these kinds of cases
(where each 'co-author' adds both tags)?

> WARNING: Line has trailing whitespace
> #522 FILE: ipsec/ovs-monitor-ipsec:456:
> self.secrets_file.write('%s %s : PSK "%s"\n' % 
>
> WARNING: Line is 80 characters long (recommended limit is 79)
> WARNING: Line has trailing whitespace
> #523 FILE: ipsec/ovs-monitor-ipsec:457:
> (tunnel.conf["local_ip"], 
> tunnel.conf["remote_ip"], 
>
> Lines checked: 1228, Warnings: 3, 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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] selinux: more changes to support newer hugetlbfs restrictions

[Aaron Conole]

Timothy Redaelli  writes:

> The new 'map' action is needed for 'hugetlbfs_t:file' too.
>
> CC: Aaron Conole 
> Fixes: d2675a146130 ("selinux: changes to support newer hugetlbfs 
> restrictions")
>
> Signed-off-by: Timothy Redaelli 
> ---

Good catch.

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

Re: [ovs-dev] [PATCH] ovn: Clean up log() action parsing errors.

[Aaron Conole]

Justin Pettit  writes:

>> On Jul 31, 2018, at 5:21 AM, Mark Michelson  wrote:
>> 
>> The 0-day Robot needs to lay off the pipe.
>
> Yeah, I'm not sure what was happening there.  I emailed Aaron about it.

Sorry about that.  I'll work on protecting the list for when the robot
indulges in compile-altering substances.  It's a young bot -
experimentation is to be expected.

>> Acked-by: Mark Michelson 
>
> Thanks!  I pushed this to master and branch-2.10.
>
> --Justin
>
>
> ___
> 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] robot suggestion

[Aaron Conole]

Ben Pfaff  writes:

> Today we've seen some failure reports from the robot because the build
> was broken on master.  That will happen occasionally (often, as in this
> case, because the robot uses a GCC version different from the
> submitter).  Maybe the robot should even report it, probably in a
> general email to the list rather than a followup to a particular patch.
> But it's not helpful to report those errors as related to a particular
> patch.
>
> So, maybe the robot should build the upstream branch before it applies
> the patch.  If that build fails, it shouldn't bother to report the error
> relative to the patch in question (but maybe as a general error to the
> list).  It could then apply the patch and report anything from
> checkpatch.

Good idea.  I'll try to implement it as soon as I can.  In the mean time
apologies for the bot going haywire.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/6] system-dpdk: Use a different character marker for sed commands

[Aaron Conole]

The default marker for sed commands according to the manual is /, but this
is inconvenient when working with paths.  The solution is either to escape
all instances of / or use sed's \cREGEXc feature.

Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 723ba794f..834ba06fb 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -65,12 +65,12 @@ AT_CHECK([grep "VHOST_CONFIG: /tmp/dpdkvhostclient0: 
reconnecting..." ovs-vswitc
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
-OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel module is 
probably not loaded./d
-/Failed to enable flow control/d
-/failed to connect to \/tmp\/dpdkvhostclient0: No such file or directory/d
-/Global register is changed during/d
-/EAL:   Invalid NUMA socket, default to 0/d
-/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
-/EAL: No free hugepages reported in hugepages-1048576kB/d"])
+OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is 
probably not loaded.@d
+\@Failed to enable flow control@d
+\@VHOST_CONFIG: failed to connect to /tmp/dpdkvhostclient0: No such file or 
directory@d
+\@Global register is changed during@d
+\@EAL:   Invalid NUMA socket, default to 0@d
+\@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
+\@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH 0/6] system-dpdk: add support to ping two namespaces

[Aaron Conole]

This allows system-dpdk test suite to ping two namespaces via a pair
of dpdkvhostuserclient ports, using testpmd as a forwarding agent.

Currently, the final patch in the series (which adds the test) isn't
reliable enough yet, so it is still RFC.  Submitted for early feedback.

Aaron Conole (3):
  system-dpdk: update test suite for non-phy testing
  system-dpdk: Allow running the dpdk tests from a VM
  system-dpdk: Use a different character marker for sed commands

Bala Sankaran (3):
  system-dpdk: skip all tests if there are no hugepages
  system-dpdk: Convert /tmp to use OVS_RUNDIR
  system-dpdk: Connect two namespaces via virtio

 tests/system-dpdk-macros.at |  20 +++--
 tests/system-dpdk.at| 104 ++--
 2 files changed, 107 insertions(+), 17 deletions(-)

-- 
2.14.3

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

[ovs-dev] [PATCH 1/6] system-dpdk: update test suite for non-phy testing

[Aaron Conole]

This allows a system that doesn't have a dedicated DPDK nic to
execute some DPDK tests.  In this fashion, tests that operate on
virtual ports (such as dpdkvhostuserclient) can be executed in
a wider set of environments.

Signed-off-by: Aaron Conole 
---
 tests/system-dpdk-macros.at | 18 +++---
 tests/system-dpdk.at| 16 
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index 0762ee055..2e5571fc4 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -2,7 +2,6 @@
 #
 # Check prerequisites for DPDK tests. Following settings are checked:
 #  - Hugepages
-#  - UIO driver
 #
 m4_define([OVS_DPDK_PRE_CHECK],
   [dnl Check Hugepages
@@ -11,13 +10,26 @@ m4_define([OVS_DPDK_PRE_CHECK],
AT_CHECK([mount], [], [stdout])
AT_CHECK([grep 'hugetlbfs' stdout], [], [stdout], [])
 
+])
+
+
+# OVS_DPDK_PRE_PHY_SKIP()
+#
+# Skip any phy related tests if the PHY variable is not set.
+# This is done by checking for a bound driver.
+#
+m4_define([OVS_DPDK_PRE_PHY_SKIP],
+  [dnl Perform the precheck
+   OVS_DPDK_PRE_CHECK()
+
dnl Check if VFIO or UIO driver is loaded
-   AT_CHECK([lsmod | grep -E "igb_uio|vfio"], [], [stdout])
+   AT_SKIP_IF([ ! (lsmod | grep -E "igb_uio|vfio") ], [], [stdout])
 
dnl Find PCI address candidate, skip if there is no DPDK-compatible NIC
AT_CHECK([$DPDK_DIR/usertools/dpdk-devbind.py -s | head -n +4 | tail -1], 
[], [stdout])
AT_CHECK([cat stdout | cut -d" " -s -f1 > PCI_ADDR])
-   AT_CHECK([test -s PCI_ADDR || exit 77])
+   AT_SKIP_IF([ ! test -s PCI_ADDR ])
+
 ])
 
 
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 3d21b0136..6901d19e6 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -4,14 +4,14 @@ dnl 
--
 dnl Check if EAL init is successfull
 AT_SETUP([OVS-DPDK datapath - EAL init])
 AT_KEYWORDS([dpdk])
-dnl OVS_DPDK_PRE_CHECK()
+OVS_DPDK_PRE_CHECK()
 OVS_DPDK_START()
 AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [], 
[stdout])
 AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
 AT_CHECK([grep "DPDK Enabled - initialized" ovs-vswitchd.log], [], [stdout])
-OVS_VSWITCHD_STOP("/Global register is changed during/d
-/EAL: No free hugepages reported in hugepages-1048576kB/d
-")
+OVS_VSWITCHD_STOP(["/Global register is changed during/d
+/EAL:   Invalid NUMA socket, default to 0/d
+/EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
 
@@ -22,7 +22,7 @@ dnl Add standard DPDK PHY port
 AT_SETUP([OVS-DPDK datapath - add standard DPDK port])
 AT_KEYWORDS([dpdk])
 
-OVS_DPDK_PRE_CHECK()
+OVS_DPDK_PRE_PHY_SKIP()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
@@ -63,11 +63,11 @@ AT_CHECK([grep "VHOST_CONFIG: /tmp/dpdkvhostclient0: 
reconnecting..." ovs-vswitc
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
-OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel module is probably 
not loaded./d
+OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel module is 
probably not loaded./d
 /Failed to enable flow control/d
 /failed to connect to \/tmp\/dpdkvhostclient0: No such file or directory/d
 /Global register is changed during/d
-/EAL: No free hugepages reported in hugepages-1048576kB/d
-")
+/EAL:   Invalid NUMA socket, default to 0/d
+/EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH 3/6] system-dpdk: Allow running the dpdk tests from a VM

[Aaron Conole]

Some VM configurations result in CPU flags that cause warnings to be issued by
the DPDK libraries.  When these warnings are issued, the tests will fail.

This commit adds the unreliable tsc warning to the list of ignored warnings.

Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index c1c908411..723ba794f 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -11,6 +11,7 @@ AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
 AT_CHECK([grep "DPDK Enabled - initialized" ovs-vswitchd.log], [], [stdout])
 OVS_VSWITCHD_STOP(["/Global register is changed during/d
 /EAL:   Invalid NUMA socket, default to 0/d
+/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
@@ -36,6 +37,7 @@ AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], 
[stderr])
 OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel module is probably 
not loaded./d
 /Failed to enable flow control/d
 /Global register is changed during/d
+/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d
 ")
 AT_CLEANUP
@@ -68,6 +70,7 @@ OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel 
module is probably
 /failed to connect to \/tmp\/dpdkvhostclient0: No such file or directory/d
 /Global register is changed during/d
 /EAL:   Invalid NUMA socket, default to 0/d
+/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH 2/6] system-dpdk: skip all tests if there are no hugepages

[Aaron Conole]

From: Bala Sankaran 

A failure is quite harsh in this scenario.  It's better to
simply skip all the tests and let the user look at the logs
to understand the missing hugepages.

Signed-off-by: Bala Sankaran 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
 tests/system-dpdk-macros.at | 2 +-
 tests/system-dpdk.at| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index 2e5571fc4..f772a1945 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -6,7 +6,7 @@
 m4_define([OVS_DPDK_PRE_CHECK],
   [dnl Check Hugepages
AT_CHECK([cat /proc/meminfo], [], [stdout])
-   AT_CHECK([grep HugePages_ stdout], [], [stdout])
+   AT_SKIP_IF([egrep 'HugePages_Free: *0' stdout], [], [stdout])
AT_CHECK([mount], [], [stdout])
AT_CHECK([grep 'hugetlbfs' stdout], [], [stdout], [])
 
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 6901d19e6..c1c908411 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -47,7 +47,7 @@ dnl 
--
 dnl Add vhost-user-client port
 AT_SETUP([OVS-DPDK datapath - add vhost-user-client port])
 AT_KEYWORDS([dpdk])
-
+OVS_DPDK_PRE_CHECK()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
-- 
2.14.3

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

[ovs-dev] [RFC 6/6] system-dpdk: Connect two namespaces via virtio

[Aaron Conole]

From: Bala Sankaran 

This adds a new test to the 'check-dpdk' subsystem that will exercise
allocations, PMDs, and the vhost-user code path.

Signed-off-by: Bala Sankaran 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
NOTE: This currently is broken, and needs some help to fix.  The
biggest issue we currently see is that the ovs-vswitchd does not
successfully configure memory regions, failing with the generic:
"Cannot allocate memory" from the mmap() call in
vhost_user_set_mem_table.

We have used dpdk 17.11 for ovs-vswitchd, and 18.05+ for the
testpmd version.

 tests/system-dpdk.at | 75 
 1 file changed, 75 insertions(+)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 58dc8aaae..14ea2edce 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -74,3 +74,78 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
+
+
+
+dnl --
+dnl Add vhost-user-client port
+AT_SETUP([OVS-DPDK datapath - ping vhost-user-client ports])
+AT_KEYWORDS([dpdk])
+OVS_DPDK_PRE_CHECK()
+OVS_DPDK_START()
+
+dnl Add userspace bridge and attach it to OVS
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface 
dpdkvhostuserclient0 type=dpdkvhostuserclient 
options:vhost-server-path=$OVS_RUNDIR/vhu0], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient1 -- set Interface 
dpdkvhostuserclient1 type=dpdkvhostuserclient 
options:vhost-server-path=$OVS_RUNDIR/vhu1], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+sleep 2
+
+dnl Parse log file
+AT_CHECK([grep "VHOST_CONFIG: vhost-user client: socket created" 
ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' 
mode, using client socket" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "vHost User device 'dpdkvhostuserclient1' created in 'client' 
mode, using client socket" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: $OVS_RUNDIR/vhu0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: $OVS_RUNDIR/vhu1: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
+
+dnl Set up namespaces
+ADD_NAMESPACES(ns1, ns2)
+
+dnl execute testpmd in background
+on_exit "pkill -f -x -9 'tail -f /dev/null'"
+(echo start && tail -f /dev/null) | /usr/local/bin/testpmd --socket-mem=512 \
+   --vdev="net_virtio_user,path=$OVS_RUNDIR/vhu0,server=1" \
+   --vdev="net_tap0,iface=tap0" --file-prefix page0 \
+   --single-file-segments -- -a >$OVS_RUNDIR/testpmd-vhu0.log 2>&1 &
+(echo start && tail -f /dev/null) | /usr/local/bin/testpmd --socket-mem=512 \
+   --vdev="net_virtio_user,path=$OVS_RUNDIR/vhu1,server=1" \
+   --vdev=net_tap1,iface=tap1 --file-prefix page1 \
+   --single-file-segments -- -a >$OVS_RUNDIR/testpmd-vhu1.log 2>&1 &
+
+dnl give settling time to the testpmd processes
+sleep 2
+
+dnl move the tap devices to the namespaces
+AT_CHECK([ip link set tap0 netns ns1], [], [stdout], [stderr])
+AT_CHECK([ip link set tap1 netns ns2], [], [stdout], [stderr])
+
+AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 ip link show | grep tap0], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 ip link set tap0 up], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 ip addr add 172.31.110.11/24 dev tap0], [], 
[stdout], [stderr])
+
+AT_CHECK([ip netns exec ns2 ip link show], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns2 ip link show | grep tap1], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns2 ip link set tap1 up], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns2 ip addr add 172.31.110.12/24 dev tap1], [], 
[stdout], [stderr])
+
+ip netns exec ns1 arping -c 4 -I tap0 172.31.110.12
+
+ovs-vsctl show
+ovs-appctl dpctl/dump-dps
+ovs-appctl dpctl/show netdev@ovs-netdev
+ps aux | grep testpmd
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
+AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient1], [], [stdout], 
[stderr])
+OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is 
probably not loaded.@d
+\@Failed to enable flow control@d
+\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/hu0: No such file or 
directory@d
+\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/vhu1: No such file or 
directory@d
+\@Global register is changed during@d
+\@EAL:   Invalid NUMA socket, default to 0@d
+\@EAL: WARNING: 

[ovs-dev] [PATCH 5/6] system-dpdk: Convert /tmp to use OVS_RUNDIR

[Aaron Conole]

From: Bala Sankaran 

When multiple users run the DPDK testsuite ther dependence on /tmp
will cause conflicts. Use the RUNDIR as a dynamic path to overcome
this.

NOTE: This still doesn't solve the dependency on /var/run that
DPDK requires.

Signed-off-by: Bala Sankaran 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 834ba06fb..58dc8aaae 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -54,20 +54,20 @@ OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
-AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface 
dpdkvhostuserclient0 type=dpdkvhostuserclient 
options:vhost-server-path=/tmp/dpdkvhostclient0], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface 
dpdkvhostuserclient0 type=dpdkvhostuserclient 
options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl show], [], [stdout])
 sleep 2
 
 dnl Parse log file
 AT_CHECK([grep "VHOST_CONFIG: vhost-user client: socket created" 
ovs-vswitchd.log], [], [stdout])
 AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' 
mode, using client socket" ovs-vswitchd.log], [], [stdout])
-AT_CHECK([grep "VHOST_CONFIG: /tmp/dpdkvhostclient0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: $OVS_RUNDIR/dpdkvhostclient0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
 OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is 
probably not loaded.@d
 \@Failed to enable flow control@d
-\@VHOST_CONFIG: failed to connect to /tmp/dpdkvhostclient0: No such file or 
directory@d
+\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/dpdkvhostclient0: No such 
file or directory@d
 \@Global register is changed during@d
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] selinux: changes to support newer hugetlbfs restrictions

[Aaron Conole]

Aaron Conole  writes:

> Newer selinux base policies now split out 'map' actions, as well as
> adding more explicit checks for hugetlbfs objects.  Where previously these
> weren't required, recent changes have flagged the allocation of hugepages
> and subsequent clearing.  This means that the hugepage storage information
> for the DPDK .rte_config, and clearing actions copying from /dev/zero will
> trigger selinux denials.
>
> This commit allows openvswitch to have more permissions for the hugetlbfs
> allocation and use.
>
> Signed-off-by: Aaron Conole 
> ---

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

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Flavio Leitner  writes:

> On Wed, Jul 18, 2018 at 11:24:43AM -0400, Aaron Conole wrote:
>> Markos Chandras  writes:
>> 
>> > When ovsdb-server is starting, it performs some DB steps such as
>> > creating and upgrading the OvS DB. When we are running as
>> > 'non-root' user, the 'runuser' tool is used to manage the privileges.
>> > However, when this happens during systemd boot, we observe the following
>> > errors in journald:
>> >
>> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add
>> > PIDs to scope's control group: No such process
>> > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
>> > openvswitch.
>> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed 
>> > state.
>> >
>> > According to the analysis performed on openSUSE bugzilla[1], it seems
>> > that ovsdb-server.service creates (via the call to runuser) a user
>> > session and therefore call pam_systemd which in its turn tries to start
>> > a systemd user instance: "user@474.service". However "user@474.service"
>> > is supposed to be started after systemd-user-sessions.service which is
>> > supposed to be started after network.target. Additionally,
>> > ovsdb-server.service uses Before=network.target hence the deadlock.
>> >
>> > We can workaround this by switching to 'root' user when we are
>> > performing this pre-startup steps and fixup the DB permissions before
>> > we start the actual ovsdb-server daemon.
>> >
>> > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
>> > Cc: Aaron Conole 
>> > Signed-off-by: Markos Chandras 
>> > ---
>> > Probably not the cleanest option so I am open to suggestions :)
>> 
>> I think there's actually a race condition here.  Most likely,
>> ovsdb-server doesn't need to be started before network.service.
>
> Unfortunately it does because network.service will ifup OVS bridges
> and ports and then we need ovsdb already running.

Good point.  Someday, I'll remember that the first time.

> fbl
>
>> Looking at the bug, I think we can unroll some of the dependencies that
>> each unit file has and get a cleaner systemd dependency chain, and
>> preserve the existing user downgrade.
>> 
>> I will install an OpenSUSE VM and work on it.  Strange that we don't see
>> this on the RHEL side.  I'll also try to reproduce that.
>> 
>> Thanks for pointing the issue out (and thanks to David and Franck on
>> your side).
>> 
>> -Aaron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Markos Chandras  writes:

> Hello Aaron,
>
> On 18/07/18 16:24, Aaron Conole wrote:
>> 
>> I think there's actually a race condition here.  Most likely,
>> ovsdb-server doesn't need to be started before network.service.
>> 
>> Looking at the bug, I think we can unroll some of the dependencies that
>> each unit file has and get a cleaner systemd dependency chain, and
>> preserve the existing user downgrade.
>> 
>> I will install an OpenSUSE VM and work on it.  Strange that we don't see
>> this on the RHEL side.  I'll also try to reproduce that.
>> 
>> Thanks for pointing the issue out (and thanks to David and Franck on
>> your side).
>> 
>> -Aaron
>> 
>
> Great thank you. If you are using vagrant you can use the following
> steps for a reproducer
>
> vagrant init opensuse/openSUSE-15.0-x86_64
> vagrant up
> vagrant ssh
> (inside vagrant)
> sudo -s
> zypper -n in openvswitch
> systemctl enable openvswitch
> systemctl reboot
>
> checking the journald logs after the reboot should reveal the issue.
>
> Let me know if you need any help.

Is it possible that the provided diff can fix most of the issue (rather
than needing the corrective block in ovs-ctl)?  If so, I'd advocate for
the smaller change instead.  I need to double check it on RHEL/Fedora.

Flavio?  Timothy?

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 92f98ad92..8db887ef6 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,7 @@ move_ip_routes () {
 
 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+setpriv --reuid "${OVS_USER%:*}" ovsdb-tool -vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
--
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] unixctl: Style fix.

[Aaron Conole]

Ben Pfaff  writes:

> Reported-by: Aaron Conole 
> Signed-off-by: Ben Pfaff 
> ---

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

Re: [ovs-dev] [PATCH v2 1/3] unixctl: Make path to unixctl_server socket available to the client.

[Aaron Conole]

Ben Pfaff  writes:

> Acked-by: Alin Gabriel Serdean 
> Signed-off-by: Ben Pfaff 
> ---
>  lib/unixctl.c   | 52 
>  lib/unixctl.h   |  2 ++
>  tests/daemon.at |  4 ++--
>  3 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/lib/unixctl.c b/lib/unixctl.c
> index bd9c1caeedef..9b3b0671f33c 100644
> --- a/lib/unixctl.c
> +++ b/lib/unixctl.c
> @@ -56,6 +56,7 @@ struct unixctl_conn {
>  struct unixctl_server {
>  struct pstream *listener;
>  struct ovs_list conns;
> +char *path;
>  };
>  
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -216,48 +217,44 @@ unixctl_command_reply_error(struct unixctl_conn *conn, 
> const char *error)
>  int
>  unixctl_server_create(const char *path, struct unixctl_server **serverp)
>  {
> -struct unixctl_server *server;
> -struct pstream *listener;
> -char *punix_path;
> -int error;
> -
>  *serverp = NULL;
>  if (path && !strcmp(path, "none")) {
>  return 0;
>  }
>  
> -if (path) {
> -char *abs_path;
> -abs_path = abs_file_name(ovs_rundir(), path);
> -punix_path = xasprintf("punix:%s", abs_path);
> -free(abs_path);
> -} else {
> -#ifndef _WIN32
> -punix_path = xasprintf("punix:%s/%s.%ld.ctl", ovs_rundir(),
> -   program_name, (long int) getpid());
> +#ifdef _WIN32
> +enum { WINDOWS = 1 };
>  #else
> -punix_path = xasprintf("punix:%s/%s.ctl", ovs_rundir(), 
> program_name);
> +enum { WINDOWS = 0 };
>  #endif
> -}
>  
> -error = pstream_open(punix_path, , 0);
> +long int pid = getpid();
> +char *abs_path
> += (path ? abs_file_name(ovs_rundir(), path)
> +   : WINDOWS ? xasprintf("%s/%s.ctl", ovs_rundir(), program_name)
> +   : xasprintf("%s/%s.%ld.ctl", ovs_rundir(), program_name, pid));
> +
> +struct pstream *listener;
> +char *punix_path = xasprintf("punix:%s", abs_path);
> +int error = pstream_open(punix_path, , 0);
> +free(punix_path);
> +
>  if (error) {
> -ovs_error(error, "could not initialize control socket %s", 
> punix_path);
> -goto exit;
> +ovs_error(error, "%s: could not initialize control socket", 
> abs_path);
> +free(abs_path);
> +return error;
>  }
>  
>  unixctl_command_register("list-commands", "", 0, 0, 
> unixctl_list_commands,
>   NULL);
>  unixctl_command_register("version", "", 0, 0, unixctl_version, NULL);
>  
> -server = xmalloc(sizeof *server);
> +struct unixctl_server *server = xmalloc(sizeof *server);
>  server->listener = listener;
> +server->path = abs_path;
>  ovs_list_init(>conns);
>  *serverp = server;
> -
> -exit:
> -free(punix_path);
> -return error;
> +return 0;
>  }
>  
>  static void
> @@ -429,10 +426,17 @@ unixctl_server_destroy(struct unixctl_server *server)
>  kill_connection(conn);
>  }
>  
> +free (server->path);

Just a small nit, this looks like gnu style.

>  pstream_close(server->listener);
>  free(server);
>  }
>  }
> +
> +const char *
> +unixctl_server_get_path(const struct unixctl_server *server)
> +{
> +return server ? server->path : NULL;
> +}
>  
>  /* On POSIX based systems, connects to a unixctl server socket.  'path' 
> should
>   * be the name of a unixctl server socket.  If it does not start with '/', it
> diff --git a/lib/unixctl.h b/lib/unixctl.h
> index ce43893c6a7d..4562dbc49113 100644
> --- a/lib/unixctl.h
> +++ b/lib/unixctl.h
> @@ -28,6 +28,8 @@ void unixctl_server_run(struct unixctl_server *);
>  void unixctl_server_wait(struct unixctl_server *);
>  void unixctl_server_destroy(struct unixctl_server *);
>  
> +const char *unixctl_server_get_path(const struct unixctl_server *);
> +
>  /* Client for Unix domain socket control connection. */
>  struct jsonrpc;
>  int unixctl_client_create(const char *path, struct jsonrpc **client);
> diff --git a/tests/daemon.at b/tests/daemon.at
> index 952d5a7c7bbe..b379fa83f9aa 100644
> --- a/tests/daemon.at
> +++ b/tests/daemon.at
> @@ -149,7 +149,7 @@ AT_SETUP([daemon --detach startup errors])
>  AT_CAPTURE_FILE([pid])
>  OVSDB_INIT([db])
>  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile 
> --unixctl=nonexistent/unixctl db], [1], [], [stderr])
> -AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
> +AT_CHECK([grep 'could not initialize control socket' stderr],
>[0], [ignore])
>  AT_CHECK([test ! -s pid])
>  AT_CLEANUP
> @@ -159,7 +159,7 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  AT_CAPTURE_FILE([pid])
>  OVSDB_INIT([db])
>  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --monitor 
> --unixctl=nonexistent/unixctl db], [1], [], [stderr])
> -AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
> +AT_CHECK([grep 'could not initialize control socket' stderr],
>[0], 

Re: [ovs-dev] [PATCH v5 3/6] debian and rhel: Create IPsec package.

[Aaron Conole]

Ben Pfaff  writes:

> On Thu, Aug 09, 2018 at 06:31:31PM -0400, Aaron Conole wrote:
>> Ben Pfaff  writes:
>> 
>> > On Thu, Aug 09, 2018 at 12:40:39PM -0700, Ansis Atteka wrote:
>> >> On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao  wrote:
>> >> >
>> >> > Added rules and files to create debian and rpm ovs-ipsec packages.
>> >> >
>> >> > Signed-off-by: Qiuyu Xiao 
>> >> > Signed-off-by: Ansis Atteka 
>> >> > Co-authored-by: Ansis Atteka 
>> >> 
>> >> Did you test this patch on Fedora with SElinux enabled?
>> >> ovs-monitor-ipsec daemon fails to start. You need to create SElinux
>> >> policy too:
>> >

Looking at the documentation and playing around here are my thoughts:

1. We probably can squelch the .local and ldconfig AVCs that pop out.
These seem to be related more to the python environment of the ipsec
monitor.

  dontaudit openvswitch_t gconf_home_t:dir { search };
  dontaudit openvswitch_t ldconfig_exec_t:file { execute };

I don't think there's any harm in them, so the above would simply keep
the alert log quiet.

2. The actual ipsec side seems a bit more complicated.

Since the openvswitch-ipsec daemon writes configurations to /etc, it
would be best to build a transition domain that has the ability just to
modify those files and start the ipsec daemon.  I'm not sure it makes
sense to allow openvswitch_t domain to write to all of /etc.  We can
certainly grant that for now and make the transition domain something to
do in the future.  I'll write that policy up and send it out (but it's a
bit bigger - even the non-domain transition one - just because of the
extra headache to allow /etc access).

On the other hand, it might be possible to use an existing ipsec service
and use the ipsec dbus interface.  Can you take a look to see if we
could integrate that by default and fall back to the manual monitoring
mode.  That would be my preferred solution (but I don't know if it has
all of the support needed).  The selinux policy for that is much simpler
as well (just a few macros).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Improve accuracy and specificity of sign-off checking.

[Aaron Conole]

Aaron Conole  writes:

> Ben Pfaff  writes:
>
>> This also makes a start at a testsuite for checkpatch.
>>
>> CC: Aaron Conole 
>> Signed-off-by: Ben Pfaff 
>> ---
>
> I think it's close, but I still get this sort of error:
>
> 03:07:46 aconole {ipsec_selinux} ~/git/ovs$ ./utilities/checkpatch.py -1
> == Checking fa10857dafcc ("checkpatch: Improve accuracy and
> specificity of sign-off checking.") ==
> ERROR: Committer None needs to sign off.
> Lines checked: 268, Warnings: 0, Errors: 1
>

Actually, it was correct - I did forget to sign off.  However, even when
I do, I see:

WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Aaron Conole 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] checkpatch: Improve accuracy and specificity of sign-off checking.

[Aaron Conole]

Ben Pfaff  writes:

> This also makes a start at a testsuite for checkpatch.
>
> CC: Aaron Conole 
> Signed-off-by: Ben Pfaff 
> ---

I think it's close, but I still get this sort of error:

03:07:46 aconole {ipsec_selinux} ~/git/ovs$ ./utilities/checkpatch.py -1
== Checking fa10857dafcc ("checkpatch: Improve accuracy and specificity of 
sign-off checking.") ==
ERROR: Committer None needs to sign off.
Lines checked: 268, Warnings: 0, Errors: 1


Very excited to play with the test suite.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to configure flow control at netdev-init.

[Aaron Conole]

Hi Sugesh (and robot),

0-day Robot  writes:

> Bleep bloop.  Greetings Sugesh Chandran, 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.
>
>
> git-am:
> Failed to merge in the changes.
> Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at 
> netdev-init.
> The copy of the patch that failed is found in:
>
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

This happened because Branch2.8 doesn't match any such branch in the
repository.  It isn't a problem with your patch, per-se.

In the future, if you use branch-x.y the bot will detect the branch and
switch to using that branch.  Then the checks will be more robust.

> 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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to configure flow control at netdev-init.

[Aaron Conole]

"Chandran, Sugesh"  writes:

> Hi Aron,
> Thank you for pointing out.
> I will send out the patch again with right branch convention to make the 
> robot happy.

No need.  We can ignore the bot in this case.  A committer can do the
right thing.  Just an FYI for future commits.

Thanks!

> Regards
> _Sugesh
>
>
>> -----Original Message-
>> From: Aaron Conole [mailto:acon...@bytheb.org]
>> Sent: Thursday, August 9, 2018 2:15 PM
>> To: 0-day Robot 
>> Cc: Chandran, Sugesh ; d...@openvswitch.org
>> Subject: Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to 
>> configure
>> flow control at netdev-init.
>> 
>> Hi Sugesh (and robot),
>> 
>> 0-day Robot  writes:
>> 
>> > Bleep bloop.  Greetings Sugesh Chandran, 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.
>> >
>> >
>> > git-am:
>> > Failed to merge in the changes.
>> > Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at
>> netdev-init.
>> > The copy of the patch that failed is found in:
>> >
>> > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-app
>> > ly/patch When you have resolved this problem, run "git am --resolved".
>> > If you prefer to skip this patch, run "git am --skip" instead.
>> > To restore the original branch and stop patching, run "git am --abort".
>> 
>> This happened because Branch2.8 doesn't match any such branch in the
>> repository.  It isn't a problem with your patch, per-se.
>> 
>> In the future, if you use branch-x.y the bot will detect the branch and 
>> switch to
>> using that branch.  Then the checks will be more robust.
>> 
>> > 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
> ___
> 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] vswitch.xml: Update dpdk-init documentation.

[Aaron Conole]

Kevin Traynor  writes:

> dpdk-init is now a string. Add description of 'true' and 'try'.
>
> Fixes: 3e52fa5644cd ("dpdk: reflect status and version in the database")
> Cc: acon...@redhat.com
> Signed-off-by: Kevin Traynor 
> ---

Acked-by: Aaron Conole 

D'oh!  Thanks, Kevin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.

[Aaron Conole]

Ben Pfaff  writes:

> This also makes a start at a testsuite for checkpatch.
>
> CC: Aaron Conole 
> Signed-off-by: Ben Pfaff 
> ---

Acked-by: Aaron Conole 

Thanks for this work, Ben!

There's one more case that we could possibly consider (but it's rare
enough that maybe it's not that big of a deal to let it stand for now),
encompassed by commit 52e20a3d6c8b.

Once this goes in I'll fix the bot up to add a signoff (so that the
warning is squelched :).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/2] table: fix html buffer output

[Aaron Conole]

Aaron Conole  writes:

> Prior to this commit, html output exhibiits a doppler effect for

Ironically, I duplicated an 'i' in the word exhibits here.  Let me know
if you want a respin or if it can be fixed while applying. :-x

> content by continually printing strings passed from
> table_print_html_cell.
>
> Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a 
> table as a string.")
> Cc: Ben Pfaff 
> Cc: Jakub Sitnicki 
> Signed-off-by: Aaron Conole 
> ---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 2/2] table: fix html buffer output

[Aaron Conole]

Prior to this commit, html output exhibiits a doppler effect for
content by continually printing strings passed from
table_print_html_cell.

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table 
as a string.")
Cc: Ben Pfaff 
Cc: Jakub Sitnicki 
Signed-off-by: Aaron Conole 
---
 lib/table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/table.c b/lib/table.c
index 19bf89262..ab72668c7 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -349,7 +349,7 @@ static void
 table_escape_html_text__(const char *content, size_t n, struct ds *s)
 {
 if (!strpbrk(content, "&<>\"")) {
-ds_put_cstr(s, content);
+ds_put_buffer(s, content, n);
 } else {
 size_t i;
 
-- 
2.14.3

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

[ovs-dev] [PATCH v2 1/2] table: append newline when printing tables

[Aaron Conole]

With commit cb139fa8b3a1 ("table: New function table_format() for
formatting a table as a string.") a new mechanism for formatting
tables was introduced, and the table_print method was refactored to
use this.

During that refactor, calls to 'puts' were replaced with
'ds_put_cstr', and table print was changed to use 'fputs(...,
stdout)'.  Unfortunately, fputs() does not append a newline to the
string provided, and changes the output strings of, for example,
ovsdb-client dump to print all on one line.  This means
post-processing scripts that are chained after ovsdb-client would
either block indefinitely (if they don't detect EOF), or process the
entire bundle at once (rather than seeing each table on a separate
line).

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table 
as a string.")
Cc: Ben Pfaff 
Cc: Jakub Sitnicki 
Reported-by: Terry Wilson 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
Signed-off-by: Aaron Conole 
Suggested-by: Ben Pfaff 
---
 lib/table.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/table.c b/lib/table.c
index cd811caf5..19bf89262 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -547,6 +547,7 @@ table_print_json__(const struct table *table, const struct 
table_style *style,
 json_object_put(json, "data", data);
 
 json_to_ds(json, style->json_flags, s);
+ds_put_char(s, '\n');
 json_destroy(json);
 }
 
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] table: append newline when printing tables

[Aaron Conole]

Ben Pfaff  writes:

> On Tue, Aug 07, 2018 at 06:23:12PM -0400, Aaron Conole wrote:
>> With commit cb139fa8b3a1 ("table: New function table_format() for
>> formatting a table as a string.") a new mechanism for formatting
>> tables was introduced, and the table_print method was refactored to
>> use this.
>> 
>> During that refactor, calls to 'puts' were replaced with
>> 'ds_put_cstr', and table print was changed to use 'fputs(...,
>> stdout)'.  Unfortunately, fputs() does not append a newline to the
>> string provided, and changes the output strings of, for example,
>> ovsdb-client dump to print all on one line.  This means
>> post-processing scripts that are chained after ovsdb-client would
>> either block indefinitely (if they don't detect EOF), or process the
>> entire bundle at once (rather than seeing each table on a separate
>> line).
>> 
>> Fixes: cb139fa8b3a1 ("table: New function table_format() for
>> formatting a table as a string.")
>> Cc: Ben Pfaff 
>> Cc: Jakub Sitnicki 
>> Reported-by: Terry Wilson 
>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
>> Signed-off-by: Aaron Conole 
>> ---
>> NOTE: I chose to keep the fputs and insert an additional fputs.
>>   It might alternately be appropriate to change fputs() to
>>   puts().  Dealer's choice, I guess :)
>
> It looks to me like this is a problem specifically in the formatting for
> JSON tables.  If so, then the fix is more like this:
>
> diff --git a/lib/table.c b/lib/table.c
> index cd811caf5b88..19bf89262cfc 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -547,6 +547,7 @@ table_print_json__(const struct table *table, const 
> struct table_style *style,
>  json_object_put(json, "data", data);
>  
>  json_to_ds(json, style->json_flags, s);
> +ds_put_char(s, '\n');
>  json_destroy(json);
>  }
>  
>
> Can you check?

Whoops!  Looks like you're right - I rushed to the wrong routine.

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

Re: [ovs-dev] [ovs-dev,v3] tests: Test for ovs-ofctl snoop command

[Aaron Conole]

Ashish Varma  writes:

> Hi Aaron,
>
> My patch is already committed by Ben. Not sure what is wrong here.

Thanks for the heads up.

This happens - usually if the patch is committed before the bot has run
against it.  I have it on my list of TODOs, so as soon as my next 'work
on the bot' time comes, I'll fix it.

> Is there something that I need to do now? (or should have done correctly)

Without looking too deeply, most likely a false positive.  Feel free to
not worry about it :)  I'll ping if it's a real issue somewhere.

> Thanks,
> Ashish 
> VMware OVS team
>
> On Mon, Aug 6, 2018 at 7:15 PM, 0-day Robot  wrote:
>
>  Bleep bloop.  Greetings Ashish Varma, 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.
>
>  git-am:
>  Failed to merge in the changes.
>  Patch failed at 0001 tests: Test for ovs-ofctl snoop command
>  The copy of the patch that failed is found in:
> 
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
>  When you have resolved this problem, run "git am --resolved".
>  If you prefer to skip this patch, run "git am --skip" instead.
>  To restore the original branch and stop patching, run "git am --abort".
>
>  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] table: append newline when printing tables

[Aaron Conole]

With commit cb139fa8b3a1 ("table: New function table_format() for
formatting a table as a string.") a new mechanism for formatting
tables was introduced, and the table_print method was refactored to
use this.

During that refactor, calls to 'puts' were replaced with
'ds_put_cstr', and table print was changed to use 'fputs(...,
stdout)'.  Unfortunately, fputs() does not append a newline to the
string provided, and changes the output strings of, for example,
ovsdb-client dump to print all on one line.  This means
post-processing scripts that are chained after ovsdb-client would
either block indefinitely (if they don't detect EOF), or process the
entire bundle at once (rather than seeing each table on a separate
line).

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table 
as a string.")
Cc: Ben Pfaff 
Cc: Jakub Sitnicki 
Reported-by: Terry Wilson 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
Signed-off-by: Aaron Conole 
---
NOTE: I chose to keep the fputs and insert an additional fputs.
  It might alternately be appropriate to change fputs() to
  puts().  Dealer's choice, I guess :)

 lib/table.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/table.c b/lib/table.c
index cd811caf5..a572c3446 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -620,6 +620,7 @@ table_print(const struct table *table, const struct 
table_style *style)
 struct ds s = DS_EMPTY_INITIALIZER;
 table_format(table, style, );
 fputs(ds_cstr(), stdout);
+fputs("\n", stdout);
 ds_destroy();
 }
 
-- 
2.14.3

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

[ovs-dev] [PATCH] ovn-ctl: allow configuring user:group for daemons

[Aaron Conole]

Add two options, one for controlling the ovs daemon user/group, and the
other for controlling the ovn daemon user/group.  This allows a fine-grained
split between OVN and OVS daemons, and keeps the syntax and user/group
separation from ovs-ctl when running ovn-ctl.

Signed-off-by: Aaron Conole 
---
 NEWS|  3 ++-
 ovn/utilities/ovn-ctl   | 14 ++
 ovn/utilities/ovn-ctl.8.xml |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 7875f6673..64d4ed5e3 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,7 @@
 Post-v2.10.0
 -
-
+   - ovn:
+ * ovn-ctl: allow passing user:group ids to the OVN daemons.
 
 v2.10.0 - xx xxx 
 -
diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 296e5b82c..3ff0df68e 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -172,6 +172,8 @@ $cluster_remote_port
 set "$@" --remote=punix:$sock --pidfile=$pid
 set "$@" --unixctl=ovn${db}_db.ctl
 
+[ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
+
 if test X"$detach" != Xno; then
 set "$@" --detach --monitor
 else
@@ -293,6 +295,8 @@ start_northd () {
 set "$@" --log-file=$OVN_NORTHD_LOGFILE
 fi
 
+[ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
+
 set "$@" $OVN_NORTHD_LOG $ovn_northd_params
 
 OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_NORTHD_PRIORITY" 
"$OVN_NORTHD_WRAPPER" "$@"
@@ -314,6 +318,9 @@ start_controller () {
 if test X"$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT" != X; then
 set "$@" --bootstrap-ca-cert=$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT
 fi
+
+[ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
+
 OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_CONTROLLER_PRIORITY" 
"$OVN_CONTROLLER_WRAPPER" "$@"
 }
 
@@ -332,6 +339,9 @@ start_controller_vtep () {
 if test X"$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT" != X; then
 set "$@" --bootstrap-ca-cert=$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT
 fi
+
+[ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
+
 OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_CONTROLLER_PRIORITY" 
"$OVN_CONTROLLER_WRAPPER" "$@"
 }
 
@@ -429,6 +439,8 @@ set_defaults () {
 
 OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
 OVN_RUNDIR=${OVN_RUNDIR:-${OVS_RUNDIR}}
+OVN_USER=
+OVS_USER=
 
 OVN_CONTROLLER_LOG="-vconsole:emer -vsyslog:err -vfile:info"
 OVN_NORTHD_LOG="-vconsole:emer -vsyslog:err -vfile:info"
@@ -535,6 +547,8 @@ Options:
   --ovn-northd-logfile=STRINGovn northd process log file (default: 
$OVN_NORTHD_LOGFILE)
   --ovn-nb-log=STRING ovn NB ovsdb-server processes logging params 
(default: $OVN_NB_LOG)
   --ovn-sb-log=STRING ovn SB ovsdb-server processes logging params 
(default: $OVN_SB_LOG)
+  --ovn-user="user[:group]"  pass the --user flag to the ovn daemons
+  --ovs-user="user[:group]"  pass the --user flag to ovs daemons
   -h, --help display this help message
 
 File location options:
diff --git a/ovn/utilities/ovn-ctl.8.xml b/ovn/utilities/ovn-ctl.8.xml
index 02235fe1e..3b0e67a45 100644
--- a/ovn/utilities/ovn-ctl.8.xml
+++ b/ovn/utilities/ovn-ctl.8.xml
@@ -44,6 +44,8 @@
 --ovn-northd-wrapper=WRAPPER
 --ovn-controller-priority=NICE
 --ovn-controller-wrapper=WRAPPER
+--ovn-user=USER:GROUP
+--ovs-user=USER:GROUP
 -h | --help
 
 File location options
-- 
2.14.3

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

Re: [ovs-dev] [PATCH v3] checkpatch: Improve accuracy and specificity of sign-off checking.

[Aaron Conole]

Ben Pfaff  writes:

> On Sun, Aug 12, 2018 at 07:43:14AM -0700, Ben Pfaff wrote:
>> On Sat, Aug 11, 2018 at 09:54:01AM -0400, Aaron Conole wrote:
>> > Ben Pfaff  writes:
>> > 
>> > > This also makes a start at a testsuite for checkpatch.
>> > >
>> > > CC: Aaron Conole 
>> > > Signed-off-by: Ben Pfaff 
>> > > ---
>> > 
>> > Acked-by: Aaron Conole 
>> > 
>> > Thanks for this work, Ben!
>> > 
>> > There's one more case that we could possibly consider (but it's rare
>> > enough that maybe it's not that big of a deal to let it stand for now),
>> > encompassed by commit 52e20a3d6c8b.
>> > 
>> > Once this goes in I'll fix the bot up to add a signoff (so that the
>> > warning is squelched :).
>> 
>> I guess another way would be to make checkpatch ignore some particular
>> committer, e.g. "Nobody " and not complain about the
>> lack of sign-off.
>
> Or the robot could just use the author as committer as well.

I'll use that option.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, v5, 08 of 20] ovn-controller: Incremental logical flow processing

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Han Zhou, 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.
>
>
> git-am:
> fatal: sha1 information is lacking or useless (ovn/controller/lflow.c).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 ovn-controller: Incremental logical flow processing
> The copy of the patch that failed is found in:
>
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
>
> Please check this out.  If you feel there has been an error, please
> email acon...@bytheb.org
>
> Thanks,
> 0-day Robot

Ignore this.

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

Re: [ovs-dev] [ovs-dev, v5, 08 of 20] ovn-controller: Incremental logical flow processing

[Aaron Conole]

Aaron Conole  writes:

> 0-day Robot  writes:
>
>> Bleep bloop.  Greetings Han Zhou, 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.
>>
>>
>> git-am:
>> fatal: sha1 information is lacking or useless (ovn/controller/lflow.c).
>> Repository lacks necessary blobs to fall back on 3-way merge.
>> Cannot fall back to three-way merge.
>> Patch failed at 0001 ovn-controller: Incremental logical flow processing
>> The copy of the patch that failed is found in:
>>
>> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
>> When you have resolved this problem, run "git am --resolved".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>>
>>
>> Please check this out.  If you feel there has been an error, please
>> email acon...@bytheb.org
>>
>> Thanks,
>> 0-day Robot
>
> Ignore this.
>
> Same issue as before.

I believe this issue is now fixed.  The bot has a second check before
submitting the series for processing where it checks for all of the
patches received.  If the answer is 'false', it delays processing that
series.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, v4, 05 of 20] ovn-controller: Incremental processing engine

[Aaron Conole]

Han Zhou  writes:

> On Mon, Aug 13, 2018 at 1:04 AM, 0-day Robot  wrote:
>>
>> Bleep bloop.  Greetings Han Zhou, 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
>> #439 FILE: ovn/lib/inc-proc-eng.h:194:
>> if (first_run) { \
>>
>> ERROR: Inappropriate bracing around statement
>> #446 FILE: ovn/lib/inc-proc-eng.h:201:
>> if (DB_NAME##rec_##TBL_NAME##_table_track_get_first(table)) { \
>>
>> Lines checked: 489, Warnings: 0, Errors: 2
>>
> I don't see a problem here. The bracing is in macro, so has to add " \"
> after it. Any suggestions?

None at the moment.  It's case we should account for - being in a macro.

It's probably safe to whitelist something like

  if ([^\s]) { \

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

Re: [ovs-dev] [ovs-dev, v4, 20 of 20] ovn-performance.at: Test port group incremental processing.

[Aaron Conole]

Han Zhou  writes:

> On Mon, Aug 13, 2018 at 1:06 AM, 0-day Robot  wrote:
>>
>> Bleep bloop.  Greetings Han Zhou, 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.
>>
>>
>> git-am:
>> fatal: sha1 information is lacking or useless (tests/ovn-performance.at).
>> Repository lacks necessary blobs to fall back on 3-way merge.
>> Cannot fall back to three-way merge.
>> Patch failed at 0001 ovn-performance.at: Test port group incremental
> processing.
>> The copy of the patch that failed is found in:
>>
>  
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
>> When you have resolved this problem, run "git am --resolved".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>>
>>
>> Please check this out.  If you feel there has been an error, please email
> acon...@bytheb.org
>>
>> Thanks,
>> 0-day Robot
>
> This should not fail if patching following the order of the patch series,
> unless it skipped the 2 patches authored by Jakub.

So, the curl response the bot received from the patchwork server is
missing patch 19, which explains why the bot failed to apply this.

It looks like the bot grabbed the series, but for some reason patchwork
hadn't seen the final patch yet (possibly stuck processing):

  "total":20,"received_total":19

It's strange, because the times are so far apart.

I'll put in a catch for this case and make the bot resubmit the job when
this happens.

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

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Aaron Conole]

Ian Stokes  writes:

> On 8/15/2018 10:51 AM, Ophir Munk wrote:
>> Hi,
>> Please find comments inline.
>>
>>> -Original Message-
>>> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>>> boun...@openvswitch.org] On Behalf Of Stokes, Ian
>>> Sent: Tuesday, August 14, 2018 7:42 PM
>>> To: Ben Pfaff (b...@ovn.org) 
>>> Cc: d...@openvswitch.org
>>> Subject: [ovs-dev] OVS DPDK Latest & HWOL Branches
>>>
>>> Hi Ben,
>>>
>>> Recently at the OVS DPDK community meeting the case for 2 new branches
>>> was raised.
>>>
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
>>> l.openvswitch.org%2Fpipermail%2Fovs-dev%2F2018-
>>> August%2F350898.htmldata=02%7C01%7Cophirmu%40mellanox.com
>>> %7C61e0d527081c4e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149
>>> 256f461b%7C0%7C1%7C636698617407744747sdata=0tKB9%2BdVH%
>>> 2BIfWaCHzaD6oy1mldVs9FUeZVbR2T7Ym1E%3Dreserved=0
>>>
>>> These branches would be:
>>>
>>> (i) OVS DPDK Latest: This branch would essentially be OVS master using the
>>> latest DPDK release (Including non LTS releases).
>>>
>>> The purpose of this branch would be to allow OVS DPDK developers to assess
>>> the latest DPDK releases with OVS and provide feedback to the DPDK
>>> community if changes are required. Currently OVS transitions between
>>> supported DPDK releases using DPDK LTS releases only. DPDK LTS releases
>>> happen annually. The next DPDK LTS release would be 18.11. However the
>>> other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs 
>>> that
>>> impact OVS DPDK (Such as the HWOL). This feedback would be in place for
>>> the next LTS release before OVS transitions to the next x.11 LTS.
>>>
>>> (ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but
>>> would encompass the HWOL development work that is ongoing.
>>>
>>> The feeling as regards the need for a OVS DPDK HWOL branch is that it
>>> requires new features only available in the latest DPDK releases and that
>>> there will be a lot of code rework required as its validated with various HW
>>> devices over time before an acceptable solution will be in place.
>>>
>>> There was a question as regards the logistics of where the branches should
>>> reside. It was suggested that they could be part of the OVS Repo to
>>> centralize the development work but that is obviously something that would
>>> have to be raised with yourself and the other project maintainers.
>>>
>>
>> If using OVS Repo for the 2 branches will also enable having
>> frequent builds and
>> running automated CI tests - that would greatly help to make sure the 
>> branches
>> are backward compatible with master OVS.
>
> I think this should be simple enough to enable.
>
>>
>>> An alternative would be that it would be hosted on a developers GitHub repo
>>> similar to how the dpdk_merge branches currently work.
>>>
>>
>> Where ever the branches are - I suggest using the patchwork mailing list
>> for sending patches to these branches.
>> We will need to decide on a header prefix to designate a specific patch to
>> its relevant branch (e.g. "HWOL PATCH v1" or "DPDKLATEST PATCH v1").
>
> Sure, I'm not familiar with the inner workings of patchwork but I
> believe it should work once a patch is sent to d...@openvswitch.org.
>
> As you said we just need a to agree the header prefix to distinguish
> that it's not intended for the main OVS repo. (I was thinking
> "OVS_DPDK_LATEST" and "OVS_DPDK_HWOL"), using just DPDK could confuse
> people into think it's a patch for DPDK itself rather than ovs.

Probably best to follow the existing branch convention:

  "branch-XXX"

So, "branch-dpdk-next" and "branch-dpdk-hwol"

Then patches get posted as:
[PATCH branch-dpdk-next]
[PATCH branch-hwol]

I don't think it should be confusing to omit the ovs - after all they're
OVS branches / trees / whatever you want to consider them.  I don't
think anyone would mistake them that way (but maybe they would if there
was an xpost?  the file layout is radically different so the patch won't
even apply accidentally.. I dunno).

> Ian
>>
>>> Neither of the branches would be subject to releases as the end goal of the
>>> development work carried out on them would make its way into OVS Master
>>> eventually.
>>>
>>> Curious as to what your thoughts on this would be?

I think it makes sense - it can help give early feedback.  We can
probably be a bit more 'lax' in that tree as well, since some of the
dpdk compatibility work might need to be thrown away (ie: change in
XX.05, then XX.08, then XX.11-LTS which would merge mainline might have
3 different sets of changes, but hopefully not).

I also like the idea that we could do a nightly integration with dpdk
mainline without needing to affect the ovs master branch.  That means we
could detect breakages in the ovs<->dpdk interface much earlier.

All that said - I really hope that people don't start thinking of
whatever you want to call those branches as anything other than
experimentation / early 

Re: [ovs-dev] [PATCH] conntrack.c: Add missing return value check to prevent nptr dereference.

[Aaron Conole]

Jiecheng Wu  writes:

> Function ovs_ct_limit_cmd_get() defined in net/openvswitch/conntrack.c may 
> cause a null pointer dereference as it calls nla_nest_start which may return 
> NULL. The returned value is used in function nla_nest_end() later where the 
> pointer is dereferenced.
> ---
>  net/openvswitch/conntrack.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 284aca2..dad0456 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -2132,6 +2132,10 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb, 
> struct genl_info *info)
>   return PTR_ERR(reply);
>  
>   nla_reply = nla_nest_start(reply, OVS_CT_LIMIT_ATTR_ZONE_LIMIT);
> + if (!nla_reply) {
> + err = -ENOMEM;
> + goto exit_err;
> + }
>  
>   if (a[OVS_CT_LIMIT_ATTR_ZONE_LIMIT]) {
>   err = ovs_ct_limit_get_zone_limit(

This patch is appropriate to the net...@vger.kernel.org mailing list.  A
version was submitted already by Stephen Hemminger (Cc'd).

See:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349816.html

Looks like these were not accepted per David's response at:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349929.html

Stephen, are you going to resubmit your patches to netdev?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] compat: Substitute more dependable define

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Greg Rose, 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: Committer 0-day Robot  needs to sign off.
> Lines checked: 70, 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

I believe I've updated the robot to do the right thing, now.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Fix socket permissions on Linux

[Aaron Conole]

Terry Wilson  writes:

> On Thu, Aug 16, 2018 at 4:57 PM, Ben Pfaff  wrote:
>> On Thu, Aug 16, 2018 at 07:55:09PM +, Terry Wilson wrote:
>>> Unix sockets were not being created with the permission 0770,
>>> instead using the current umask value. The manpage for fchmod()
>>> states that that if filedes refers to a socket, the behavior is
>>> undefined. Insetad, use the same code as *BSD to ensure the 0770
>>> permission is set on unix sockets.
>>>
>>> Signed-off-by: Terry Wilson 
>>
>> It's extraordinarily expensive to fork() to make a single system call.
>
> I agree it is ridiculously ugly, though it isn't like this is
> something that is done in a tight loop anywhere either.
>
>> As far as I can tell, the existing code actually works on Linux, in the
>> same way as the third 'mode' parameter to open(2).
>
> It doesn't (and never has) on my Centos 7 machine. I ran into this a
> couple of years ago and ended up just working around it. As an example
> after make rpm-fedora and installing:
> [centos@test x86_64]$ ls -al /var/run/openvswitch/db.sock
> srwxr-x---.  1 openvswitch openvswitch0 Aug 16 22:09 db.sock
>
> So we've got 0750 and not 0770 like the hardcoded value in the source.
>
>> Surely there's a better way to do this.
>
> I *hope* so. I mean it certainly seems like something one would want
> to be able to do, but I remember looking for a couple of days 2 years
> ago and giving up. umask seemed like the only reliable option.
> Whatever the solution is, fchmod is *not* it since it is specifically
> undefined behavior to use it on a socket. I'll try with ubuntu and see
> what happens there, but wouldn't imagine it to be different.

So...

Gather 'round folks, and let me tell you the tale of a series long
ago posted:

https://mail.openvswitch.org/pipermail/ovs-dev/2016-August/321866.html

Something... something ... black magic...
I think the fchmod needs to happen after the bind for the permissions
to actually be changed.  That's how the unit tests in that series are
coded.

> From man 3 fchmod:
> DESCRIPTION
> ...
> If fildes refers to a socket, the behavior of fchmod() is unspecified.
> ...

I think that's because some unixes don't even honor permissions on
sockets, and some don't allow any changing of those permissions.

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

Re: [ovs-dev] [PATCH] utilities: Launch ovsdb-tool without using PAM

[Aaron Conole]

Timothy Redaelli  writes:

> When ovsdb-server is starting, it performs some DB steps such as
> creating and upgrading the OvS DB. When we are running as
> 'non-root' user, the 'runuser' tool is used to manage the privileges.
> However, when this happens during systemd boot, we observe the following
> errors in journald:
>
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to
> scope's control group: No such process
> Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> openvswitch.
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.
>
> According to the analysis performed on openSUSE bugzilla[1], it seems
> that ovsdb-server.service creates (via the call to runuser) a user
> session and therefore call pam_systemd which in its turn tries to start
> a systemd user instance: "user@474.service". However "user@474.service"
> is supposed to be started after systemd-user-sessions.service which is
> supposed to be started after network.target. Additionally,
> ovsdb-server.service uses Before=network.target hence the deadlock.
>
> This commit uses "setpriv" instead of "runuser" to launch "ovsdb-tool" that
> doesn't use PAM and so it permits to launch "ovsdb-tool" as a user without
> having the deadlock. Since some old versions for "setpriv" (such as the
> one used by RHEL7) doesn't support the username / groupname, but only the
> user ids / group ids, "id" is used to get the user ID and the group IDs.
> To replicate the same behaviour of "runuser", the effective group ID of
> the user is used as GID (usually "openvswitch") and the remaining group
> IDs are used as supplementary groups (usually "hugetlbfs", if OVS is
> built with DPDK support).
>
> [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> Reported-by: Markos Chandras 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349716.html
> Co-authored-by: Aaron Conole 
> Signed-off-by: Timothy Redaelli 
> ---

Thanks all.

Signed-off-by: Aaron Conole 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Better validate OpenFlow message length in "ofp-parse-pcap".

[Aaron Conole]

Ben Pfaff  writes:

> Reported-by: Oscar Wilde 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047070.html
> Signed-off-by: Ben Pfaff 
> ---

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

Re: [ovs-dev] [PATCH v5 3/6] debian and rhel: Create IPsec package.

[Aaron Conole]

Ben Pfaff  writes:

> On Thu, Aug 09, 2018 at 12:40:39PM -0700, Ansis Atteka wrote:
>> On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao  wrote:
>> >
>> > Added rules and files to create debian and rpm ovs-ipsec packages.
>> >
>> > Signed-off-by: Qiuyu Xiao 
>> > Signed-off-by: Ansis Atteka 
>> > Co-authored-by: Ansis Atteka 
>> 
>> Did you test this patch on Fedora with SElinux enabled?
>> ovs-monitor-ipsec daemon fails to start. You need to create SElinux
>> policy too:
>
> Is that something you can help with?  I doubt that Qiuyu has much
> experience with SELinux (and I don't either).

I'll throw something together tomorrow, if Ansis isn't able to do so.

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

Re: [ovs-dev] [PATCH] Fix socket permissions on Linux

[Aaron Conole]

Terry Wilson  writes:

>> Gather 'round folks, and let me tell you the tale of a series long
>> ago posted:
>>
>> https://mail.openvswitch.org/pipermail/ovs-dev/2016-August/321866.html
>>
>> Something... something ... black magic...
>> I think the fchmod needs to happen after the bind for the permissions
>> to actually be changed.  That's how the unit tests in that series are
>> coded.
>
> If you move fchmod after the bind, you get 0755 and not 0750 on my
> system. It seems weird fchmod interacting with umask since it seems
> like the whole purpose of chmod/fchmod are to change permissions
> ignoring umask. You certainly aren't going to get the same behavior
> with fchmod on a regular file anyway. It seemed like the patch that
> specifically changed 0700 -> 0770 intended group permissions to always
> be there. I can see arguments both ways, though. The problem with
> modifying umask before calling is that you are changing the
> permissions for every created file (log files, etc.). Maybe that isn't
> a problem, I just know that it was something that came up in our
> internal discussions hacking around things with umask.

Strange.  I've tested on regular files, and find that umask is *correctly*
ignored.  The test program I'm using:

--- 8< ---
#include 
#include 
#include 
#include 

int main(int argc, char *argv[])
{
/* Change the mode */
if (argc < 2) {
printf("pass a filename\n");
return -1;
}

int fd = open(argv[1], O_RDWR);
if (fd < 0) {
perror("open");
return -1;
}

fchmod(fd, 0770);
close(fd);
return 0;
}
--- >8 ---

output
---

10:42:20 aconole /tmp$ umask
0022
10:42:22 aconole /tmp$ touch tmp.foo
10:42:25 aconole /tmp$ ls -lah | grep tmp.foo
-rw-r--r--.  1 aconole aconole0 Aug 20 10:42 tmp.foo
10:42:29 aconole /tmp$ ./test tmp.foo
10:42:36 aconole /tmp$ ls -lah | grep tmp.foo
-rwxrwx---.  1 aconole aconole0 Aug 20 10:42 tmp.foo

---

of course, /tmp has some other sticky bits set, but I tried in my own
home directory, with the same results.

Then, I played a bit with unix sockets.  What I found was interesting.

If I use fchmod on the actual socket, it seems not possible to adjust
the permissions after creation time.  That means post bind, the
permissions are locked for fd, at least as far as fchmod goes.

HOWEVER, if I independently call 'chmod(path, mode)' the mode bits are
changed without factoring the umask (as I expect).  There might need to
be some additional work to get this to do the right thing under linux,
but I think it's possible without going through the clone path.  Looking
at the old patches I had, I did a traversal and eventually fchmodat(),
which probably does the right thing (but I haven't dusted those patches
off to actually try them out).

Ben, Terry, how would you feel if I did some minor necromancy on that
code?  Any objections?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] utilities: Drop shebang from bash completion script

[Aaron Conole]

Markos Chandras  writes:

> On 28/08/18 12:57, 0-day Robot wrote:
>> Bleep bloop.  Greetings Markos Chandras, 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: Author Markos Chandras  needs to sign off.
>> Lines checked: 25, Warnings: 0, Errors: 1
>> 
>> 
>
> But it is signed off :)
>
> I guess the script may have been confused by the '---' I used to quote
> the OBS warning.

Yes - actually, even git mailinfo gets confused by it:

   09:16:48 aconole /tmp$ git mailinfo msg ptch < 
ovs-dev-utilities-Drop-shebang-from-bash-completion-script.patch
   Author: Markos Chandras
   Email: mchand...@suse.de
   Subject: utilities: Drop shebang from bash completion script
   Date: Tue, 28 Aug 2018 12:21:24 +0100

   09:17:03 aconole /tmp$ cat msg 
   This fixes the following warning when building Open vSwitch on the
   openSUSE Build Service:
   09:17:07 aconole /tmp$ 


In the future, it's best to indent the lines you intend to be quoting.

Patchwork will process the patch slightly more liberally than
git-mailinfo, but if I use git-am to apply your mbox file, the message
gets truncated and the commit log only shows the same as the above 'msg'
file from git mailinfo.

I think it can be fixed when applying, though (by making the change I've
outlined above).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Aaron Conole]

Ian Stokes  writes:

> On 8/27/2018 5:16 PM, Ben Pfaff wrote:
>> I can help with some of these.
>>
>> On Mon, Aug 27, 2018 at 04:05:39PM +, Ophir Munk wrote:
>>> Ian, can you please specify the practical steps regarding the new branches?
>>> Specifically, what is the procedure for adding a new patch for
>>> either of the branches (OVS DPDK latest or HWOK)?
>>> 1. What should the patch title include?
>>
>> I guess this is up to Ian, although he should coordinate with Aaron to
>> make sure that the patch robot understands too.
>
> We discussed this at the community call last week but it's good to
> raise it on the ML again for wider input.
>
> The process we would follow is that patches would be submitted to the
> d...@openvswitch.org.
>
> As the patches affects a particular branch and not master,
> contributors should submit the change with the target branch listed in
> the subject line of the patch.
>
> The git format-patch argument --subject-prefix may be used when
> posting the patch, for example:
>
> $ git format-patch HEAD --subject-prefix="PATCH branch-dpdk-latest"
>
> or
>
> $ git format-patch HEAD --subject-prefix="PATCH branch-dpdk-hwol"
>
> @Aaron, would it be possible to setup the 0-day robot to recognize
> patches with the above subject header and apply/build the correlating
> branch?

Absolutely do-able.  I'm getting ready for DPDK Userspace summit, so it
*might* get delayed a bit, but I'll have it implemented asap.

>>
>>> 2. Who is going to merge a new patch (as well as ongoing master
>>> branch updates) into the relevant branch?
>>
>> I expect that Ian will be the only one pushing to the new branches.
>
> Yes, I'll handle merging the patches as well as the typical validation
> via vsperf to help identify any issues a patch may cause to
> performance or functionality.
>
>>
>>> 3. Can I have write permissions in the new branches?
>>
>> I'd prefer to have just Ian doing this work for now.
>>
>>> 4. How can I inspect the new branches? Currently I am not seeing them.
>>
>> I do not think that Ian has created the new branches yet.
>
> I can create these today.
>
> There was some discussion as regards the branch names. Before creating
> them are people happy with 'branch-dpdk-latest' and 'branch-dpdk-hwol'
> ?
>
> If there are no objections then I'll go ahead with these today.
>
> Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 5/6] system-dpdk: Convert /tmp to use OVS_RUNDIR

[Aaron Conole]

From: Bala Sankaran 

When multiple users run the DPDK testsuite ther dependence on /tmp
will cause conflicts. Use the RUNDIR as a dynamic path to overcome
this.

NOTE: This still doesn't solve the dependency on /var/run that
DPDK requires.

Signed-off-by: Bala Sankaran 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 834ba06fb..58dc8aaae 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -54,20 +54,20 @@ OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
-AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface 
dpdkvhostuserclient0 type=dpdkvhostuserclient 
options:vhost-server-path=/tmp/dpdkvhostclient0], [], [stdout], [stderr])
+AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface 
dpdkvhostuserclient0 type=dpdkvhostuserclient 
options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl show], [], [stdout])
 sleep 2
 
 dnl Parse log file
 AT_CHECK([grep "VHOST_CONFIG: vhost-user client: socket created" 
ovs-vswitchd.log], [], [stdout])
 AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' 
mode, using client socket" ovs-vswitchd.log], [], [stdout])
-AT_CHECK([grep "VHOST_CONFIG: /tmp/dpdkvhostclient0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: $OVS_RUNDIR/dpdkvhostclient0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
 OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is 
probably not loaded.@d
 \@Failed to enable flow control@d
-\@VHOST_CONFIG: failed to connect to /tmp/dpdkvhostclient0: No such file or 
directory@d
+\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/dpdkvhostclient0: No such 
file or directory@d
 \@Global register is changed during@d
 \@EAL:   Invalid NUMA socket, default to 0@d
 \@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
-- 
2.14.3

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

[ovs-dev] [PATCH v2 4/6] system-dpdk: Use a different character marker for sed commands

[Aaron Conole]

The default marker for sed commands according to the manual is /, but this
is inconvenient when working with paths.  The solution is either to escape
all instances of / or use sed's \cREGEXc feature.

Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 723ba794f..834ba06fb 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -65,12 +65,12 @@ AT_CHECK([grep "VHOST_CONFIG: /tmp/dpdkvhostclient0: 
reconnecting..." ovs-vswitc
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
-OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel module is 
probably not loaded./d
-/Failed to enable flow control/d
-/failed to connect to \/tmp\/dpdkvhostclient0: No such file or directory/d
-/Global register is changed during/d
-/EAL:   Invalid NUMA socket, default to 0/d
-/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
-/EAL: No free hugepages reported in hugepages-1048576kB/d"])
+OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is 
probably not loaded.@d
+\@Failed to enable flow control@d
+\@VHOST_CONFIG: failed to connect to /tmp/dpdkvhostclient0: No such file or 
directory@d
+\@Global register is changed during@d
+\@EAL:   Invalid NUMA socket, default to 0@d
+\@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
+\@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH v2 6/6] system-dpdk: Execute testpmd on the background

[Aaron Conole]

From: Bala Sankaran 

This adds a new test to the 'check-dpdk' subsystem that will exercise
allocations, PMDs, and the vhost-user code path.

Signed-off-by: Bala Sankaran 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 77 
 1 file changed, 77 insertions(+)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 58dc8aaae..914a1b644 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -1,3 +1,6 @@
+m4_define([CONFIGURE_VETH_OFFLOADS],
+   [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
+
 AT_BANNER([OVS-DPDK unit tests])
 
 dnl --
@@ -74,3 +77,77 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch 
kernel module is probably
 \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
 AT_CLEANUP
 dnl --
+
+
+
+dnl --
+dnl Ping vhost-user-client port
+AT_SETUP([OVS-DPDK datapath - ping vhost-user-client ports])
+AT_KEYWORDS([dpdk])
+OVS_DPDK_PRE_CHECK()
+OVS_DPDK_START()
+
+dnl Add userspace bridge and attach it to OVS
+AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br10 vhu0 -- set Interface vhu0 \
+  type=dpdkvhostuser], [],
+ [stdout], [stderr])
+AT_CHECK([ovs-vsctl show], [], [stdout])
+
+dnl Parse log file
+AT_CHECK([grep "VHOST_CONFIG: vhost-user server: socket created" \
+  ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "Socket $OVS_RUNDIR/vhu0 created for vhost-user port vhu0" \
+  ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: bind to $OVS_RUNDIR/vhu0" ovs-vswitchd.log], [],
+ [stdout])
+
+dnl Set up namespaces
+ADD_NAMESPACES(ns1, ns2)
+
+dnl execute testpmd in background
+on_exit "pkill -f -x -9 'tail -f /dev/null'"
+tail -f /dev/null | testpmd --socket-mem=512 \
+   --vdev="net_virtio_user,path=$OVS_RUNDIR/vhu0" \
+   --vdev="net_tap0,iface=tap0" --file-prefix page0 \
+   --single-file-segments -- -a >$OVS_RUNDIR/testpmd-vhu0.log 2>&1 &
+
+dnl add veth device
+ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
+
+dnl give settling time to the testpmd processes - NOTE: this is bad form.
+sleep 10
+
+dnl move the tap devices to the namespaces
+AT_CHECK([ps aux | grep testpmd], [], [stdout], [stderr])
+AT_CHECK([ip link show], [], [stdout], [stderr])
+AT_CHECK([ip link set tap0 netns ns1], [], [stdout], [stderr])
+
+AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 ip link show | grep tap0], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 ip link set tap0 up], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 ip addr add 172.31.110.11/24 dev tap0], [],
+ [stdout], [stderr])
+
+AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns2 ip link show], [], [stdout], [stderr])
+AT_CHECK([ip netns exec ns1 arping -c 4 -I tap0 172.31.110.12], [], [stdout],
+ [stderr])
+
+dnl clean up the testpmd now
+pkill -f -x -9 'tail -f /dev/null'
+
+dnl Clean up
+AT_CHECK([ovs-vsctl del-port br10 vhu0], [], [stdout], [stderr])
+OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is 
probably not loaded.@d
+\@Failed to enable flow control@d
+\@VHOST_CONFIG: recvmsg failed@d
+\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/vhu0: No such file or 
directory@d
+\@Global register is changed during@d
+\@dpdkvhostuser ports are considered deprecated;  please migrate to 
dpdkvhostuserclient ports.@d
+\@failed to enumerate system datapaths: No such file or directory@d
+\@EAL:   Invalid NUMA socket, default to 0@d
+\@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !@d
+\@EAL: No free hugepages reported in hugepages-1048576kB@d"])
+AT_CLEANUP
+dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH v2 0/6] system-dpdk: add support to ping two namespaces

[Aaron Conole]

This allows system-dpdk test suite to ping two namespaces via a veth
and dpdkvhostuser port, using testpmd as a forwarding agent.

For the initial test, testpmd included with 18.11-rc0 is used, while ovs
is linked against DPDK 17.11 LTS.

Some additional enhancements are added to the dpdk testsuite to make it
easier to use.

Aaron Conole (3):
  system-dpdk: update test suite for non-phy testing
  system-dpdk: Allow running the dpdk tests from a VM
  system-dpdk: Use a different character marker for sed commands

Bala Sankaran (3):
  system-dpdk: skip all tests if there are no hugepages
  system-dpdk: Convert /tmp to use OVS_RUNDIR
  system-dpdk: Execute testpmd on the background

 tests/system-dpdk-macros.at |  20 ++--
 tests/system-dpdk.at| 108 ++--
 2 files changed, 110 insertions(+), 18 deletions(-)

-- 
2.14.3

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

[ovs-dev] [PATCH v2 3/6] system-dpdk: Allow running the dpdk tests from a VM

[Aaron Conole]

Some VM configurations result in CPU flags that cause warnings to be issued by
the DPDK libraries.  When these warnings are issued, the tests will fail.

This commit adds the unreliable tsc warning to the list of ignored warnings.

Signed-off-by: Aaron Conole 
---
 tests/system-dpdk.at | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index c1c908411..723ba794f 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -11,6 +11,7 @@ AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
 AT_CHECK([grep "DPDK Enabled - initialized" ovs-vswitchd.log], [], [stdout])
 OVS_VSWITCHD_STOP(["/Global register is changed during/d
 /EAL:   Invalid NUMA socket, default to 0/d
+/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
@@ -36,6 +37,7 @@ AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], 
[stderr])
 OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel module is probably 
not loaded./d
 /Failed to enable flow control/d
 /Global register is changed during/d
+/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d
 ")
 AT_CLEANUP
@@ -68,6 +70,7 @@ OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel 
module is probably
 /failed to connect to \/tmp\/dpdkvhostclient0: No such file or directory/d
 /Global register is changed during/d
 /EAL:   Invalid NUMA socket, default to 0/d
+/EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable 
clock cycles !/d
 /EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH v2 1/6] system-dpdk: update test suite for non-phy testing

[Aaron Conole]

This allows a system that doesn't have a dedicated DPDK nic to
execute some DPDK tests.  In this fashion, tests that operate on
virtual ports (such as dpdkvhostuserclient) can be executed in
a wider set of environments.

Signed-off-by: Aaron Conole 
---
 tests/system-dpdk-macros.at | 18 +++---
 tests/system-dpdk.at| 16 
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index 0762ee055..2e5571fc4 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -2,7 +2,6 @@
 #
 # Check prerequisites for DPDK tests. Following settings are checked:
 #  - Hugepages
-#  - UIO driver
 #
 m4_define([OVS_DPDK_PRE_CHECK],
   [dnl Check Hugepages
@@ -11,13 +10,26 @@ m4_define([OVS_DPDK_PRE_CHECK],
AT_CHECK([mount], [], [stdout])
AT_CHECK([grep 'hugetlbfs' stdout], [], [stdout], [])
 
+])
+
+
+# OVS_DPDK_PRE_PHY_SKIP()
+#
+# Skip any phy related tests if the PHY variable is not set.
+# This is done by checking for a bound driver.
+#
+m4_define([OVS_DPDK_PRE_PHY_SKIP],
+  [dnl Perform the precheck
+   OVS_DPDK_PRE_CHECK()
+
dnl Check if VFIO or UIO driver is loaded
-   AT_CHECK([lsmod | grep -E "igb_uio|vfio"], [], [stdout])
+   AT_SKIP_IF([ ! (lsmod | grep -E "igb_uio|vfio") ], [], [stdout])
 
dnl Find PCI address candidate, skip if there is no DPDK-compatible NIC
AT_CHECK([$DPDK_DIR/usertools/dpdk-devbind.py -s | head -n +4 | tail -1], 
[], [stdout])
AT_CHECK([cat stdout | cut -d" " -s -f1 > PCI_ADDR])
-   AT_CHECK([test -s PCI_ADDR || exit 77])
+   AT_SKIP_IF([ ! test -s PCI_ADDR ])
+
 ])
 
 
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 3d21b0136..6901d19e6 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -4,14 +4,14 @@ dnl 
--
 dnl Check if EAL init is successfull
 AT_SETUP([OVS-DPDK datapath - EAL init])
 AT_KEYWORDS([dpdk])
-dnl OVS_DPDK_PRE_CHECK()
+OVS_DPDK_PRE_CHECK()
 OVS_DPDK_START()
 AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [], 
[stdout])
 AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
 AT_CHECK([grep "DPDK Enabled - initialized" ovs-vswitchd.log], [], [stdout])
-OVS_VSWITCHD_STOP("/Global register is changed during/d
-/EAL: No free hugepages reported in hugepages-1048576kB/d
-")
+OVS_VSWITCHD_STOP(["/Global register is changed during/d
+/EAL:   Invalid NUMA socket, default to 0/d
+/EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
 
@@ -22,7 +22,7 @@ dnl Add standard DPDK PHY port
 AT_SETUP([OVS-DPDK datapath - add standard DPDK port])
 AT_KEYWORDS([dpdk])
 
-OVS_DPDK_PRE_CHECK()
+OVS_DPDK_PRE_PHY_SKIP()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
@@ -63,11 +63,11 @@ AT_CHECK([grep "VHOST_CONFIG: /tmp/dpdkvhostclient0: 
reconnecting..." ovs-vswitc
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
-OVS_VSWITCHD_STOP("/does not exist. The Open vSwitch kernel module is probably 
not loaded./d
+OVS_VSWITCHD_STOP(["/does not exist. The Open vSwitch kernel module is 
probably not loaded./d
 /Failed to enable flow control/d
 /failed to connect to \/tmp\/dpdkvhostclient0: No such file or directory/d
 /Global register is changed during/d
-/EAL: No free hugepages reported in hugepages-1048576kB/d
-")
+/EAL:   Invalid NUMA socket, default to 0/d
+/EAL: No free hugepages reported in hugepages-1048576kB/d"])
 AT_CLEANUP
 dnl --
-- 
2.14.3

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

[ovs-dev] [PATCH v2 2/6] system-dpdk: skip all tests if there are no hugepages

[Aaron Conole]

From: Bala Sankaran 

A failure is quite harsh in this scenario.  It's better to
simply skip all the tests and let the user look at the logs
to understand the missing hugepages.

Signed-off-by: Bala Sankaran 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
 tests/system-dpdk-macros.at | 2 +-
 tests/system-dpdk.at| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
index 2e5571fc4..f772a1945 100644
--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -6,7 +6,7 @@
 m4_define([OVS_DPDK_PRE_CHECK],
   [dnl Check Hugepages
AT_CHECK([cat /proc/meminfo], [], [stdout])
-   AT_CHECK([grep HugePages_ stdout], [], [stdout])
+   AT_SKIP_IF([egrep 'HugePages_Free: *0' stdout], [], [stdout])
AT_CHECK([mount], [], [stdout])
AT_CHECK([grep 'hugetlbfs' stdout], [], [stdout], [])
 
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 6901d19e6..c1c908411 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -47,7 +47,7 @@ dnl 
--
 dnl Add vhost-user-client port
 AT_SETUP([OVS-DPDK datapath - add vhost-user-client port])
 AT_KEYWORDS([dpdk])
-
+OVS_DPDK_PRE_CHECK()
 OVS_DPDK_START()
 
 dnl Add userspace bridge and attach it to OVS
-- 
2.14.3

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

Re: [ovs-dev] [PATCH] Documentation:

[Aaron Conole]

Gregory Rose  writes:

> On 7/18/2018 8:03 AM, Aaron Conole wrote:
>> Hi Greg,
>>
>> Greg Rose  writes:
>>
>>> Add netstat when mentioning testing.  Many check-kmod failures result
>>> when it is not present.
>>>
>>> Signed-off-by: Greg Rose 
>>> ---
>> Just wanted to point out that the subject looks incomplete.
>>
>> Otherwise:
>>
>> Acked-by: Aaron Conole 
>
> Yes, I bungled the title.  I submitted a V2 with the subject fixed.

Oops... I might have fat-fingered it away.

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

Re: [ovs-dev] [PATCH] Documentation:

[Aaron Conole]

Hi Greg,

Greg Rose  writes:

> Add netstat when mentioning testing.  Many check-kmod failures result
> when it is not present.
>
> Signed-off-by: Greg Rose 
> ---

Just wanted to point out that the subject looks incomplete.

Otherwise:

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

Re: [ovs-dev] [PATCH V2] Documentation: Add netstat to testing instructions

[Aaron Conole]

Greg Rose  writes:

> Add netstat when mentioning testing.  Many check-kmod failures result
> when it is not present.
>
> Signed-off-by: Greg Rose 
>
> ---

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

[ovs-dev] [PATCH] selinux: changes to support newer hugetlbfs restrictions

[Aaron Conole]

Newer selinux base policies now split out 'map' actions, as well as
adding more explicit checks for hugetlbfs objects.  Where previously these
weren't required, recent changes have flagged the allocation of hugepages
and subsequent clearing.  This means that the hugepage storage information
for the DPDK .rte_config, and clearing actions copying from /dev/zero will
trigger selinux denials.

This commit allows openvswitch to have more permissions for the hugetlbfs
allocation and use.

Signed-off-by: Aaron Conole 
---
NOTE: I seem to have lost the system with the logs that were used to
  generate this policy.  If needed, I can ask to get access again and
  recreate the scenarios.

 selinux/openvswitch-custom.te.in | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/selinux/openvswitch-custom.te.in b/selinux/openvswitch-custom.te.in
index 4678f2f57..21de1136d 100644
--- a/selinux/openvswitch-custom.te.in
+++ b/selinux/openvswitch-custom.te.in
@@ -37,13 +37,14 @@ require {
 type svirt_image_t;
 type svirt_tmpfs_t;
 type vfio_device_t;
+type zero_device_t;
 @end_dpdk@
 
 class capability { dac_override audit_write net_broadcast net_raw };
-class chr_file { write getattr read open ioctl };
+class chr_file { write getattr read open ioctl map };
 class dir { write remove_name add_name lock read getattr search open };
 class fd { use };
-class file { write getattr read open execute execute_no_trans create 
unlink map entrypoint lock ioctl };
+class file { map write getattr read open execute execute_no_trans 
create unlink map entrypoint lock ioctl };
 class fifo_file { getattr read write append ioctl lock open };
 class filesystem getattr;
 class lnk_file { read open };
@@ -83,7 +84,8 @@ allow openvswitch_t openvswitch_rw_t:dir { write remove_name 
add_name lock read
 allow openvswitch_t openvswitch_rw_t:file { write getattr read open execute 
execute_no_trans create unlink };
 allow openvswitch_t openvswitch_tmp_t:file { execute execute_no_trans };
 allow openvswitch_t openvswitch_tmp_t:unix_stream_socket { write getattr read 
connectto connect setopt getopt sendto accept bind recvfrom acceptfrom };
-allow openvswitch_t openvswitch_var_run_t:dir { getattr read open search };
+allow openvswitch_t openvswitch_var_run_t:dir { getattr read open search write 
remove_name add_name lock };
+allow openvswitch_t openvswitch_var_run_t:file { map open read write getattr 
create unlink };
 allow openvswitch_t tun_tap_device_t:chr_file { read write getattr open ioctl 
};
 
 @begin_dpdk@
@@ -96,6 +98,7 @@ allow openvswitch_t svirt_tmpfs_t:file { read write };
 allow openvswitch_t svirt_tmpfs_t:sock_file { read write append getattr open };
 allow openvswitch_t svirt_t:unix_stream_socket { connectto read write getattr 
sendto recvfrom setopt };
 allow openvswitch_t vfio_device_t:chr_file { read write open ioctl getattr };
+allow openvswitch_t zero_device_t:chr_file { read open getattr map };
 @end_dpdk@
 
 #= Transition allows =
-- 
2.14.3

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

Re: [ovs-dev] build: Add gitattribute file to build-aux

[Aaron Conole]

Hi Alin,

0-day Robot  writes:

> Bleep bloop.  Greetings Alin Gabriel Serdean, 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.
>
>

I suggest folding in something like the following (since I don't think
it makes sense to worry about distributing gitattributes files):

---
diff --git a/Makefile.am b/Makefile.am
index e02799a90..fd6620a9b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -199,6 +199,7 @@ dist-hook-git: distfiles
  (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) | \
LC_ALL=C sort -u > all-distfiles; \
  (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' | \
+   grep -v '\.gitattributes$$' | \
LC_ALL=C sort -u > all-gitfiles; \
  LC_ALL=C comm -1 -3 all-distfiles all-gitfiles > missing-distfiles; \
  if test -s missing-distfiles; then \
---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Markos Chandras  writes:

> When ovsdb-server is starting, it performs some DB steps such as
> creating and upgrading the OvS DB. When we are running as
> 'non-root' user, the 'runuser' tool is used to manage the privileges.
> However, when this happens during systemd boot, we observe the following
> errors in journald:
>
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to 
> scope's control group: No such process
> Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> openvswitch.
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.
>
> According to the analysis performed on openSUSE bugzilla[1], it seems
> that ovsdb-server.service creates (via the call to runuser) a user
> session and therefore call pam_systemd which in its turn tries to start
> a systemd user instance: "user@474.service". However "user@474.service"
> is supposed to be started after systemd-user-sessions.service which is
> supposed to be started after network.target. Additionally,
> ovsdb-server.service uses Before=network.target hence the deadlock.
>
> We can workaround this by switching to 'root' user when we are
> performing this pre-startup steps and fixup the DB permissions before
> we start the actual ovsdb-server daemon.
>
> [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> Cc: Aaron Conole 
> Signed-off-by: Markos Chandras 
> ---
> Probably not the cleanest option so I am open to suggestions :)

I think there's actually a race condition here.  Most likely,
ovsdb-server doesn't need to be started before network.service.

Looking at the bug, I think we can unroll some of the dependencies that
each unit file has and get a cleaner systemd dependency chain, and
preserve the existing user downgrade.

I will install an OpenSUSE VM and work on it.  Strange that we don't see
this on the RHEL side.  I'll also try to reproduce that.

Thanks for pointing the issue out (and thanks to David and Franck on
your side).

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

[ovs-dev] [PATCH] netlink-conntrack: undef the correct macro

[Aaron Conole]

Fixes: 6830a0c0e6bf ("netlink-conntrack: New module.")
Cc: Daniele Di Proietto 
Signed-off-by: Aaron Conole 
---
 lib/netlink-conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
index e5a5fc118..42be1d9ce 100644
--- a/lib/netlink-conntrack.c
+++ b/lib/netlink-conntrack.c
@@ -660,7 +660,7 @@ ip_ct_tcp_flags_to_dpif(uint8_t flags)
 #define CT_DPIF_TCP_FLAG(FLAG) \
 ret |= (flags & IP_CT_TCP_FLAG_##FLAG) ? CT_DPIF_TCPF_##FLAG : 0;
 CT_DPIF_TCP_FLAGS
-#undef CT_DPIF_STATUS_FLAG
+#undef CT_DPIF_TCP_FLAG
 return ret;
 #endif
 }
-- 
2.14.3

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

Re: [ovs-dev] [PATCH v2] build: Add gitattribute file to build-aux

[Aaron Conole]

Alin Gabriel Serdean  writes:

> The command: `make check-tabs` fails on Windows due to line ending conversions
> caused by the following setting: `git config --global core.autocrlf true`
> (the whitelist `build-aux/initial-tab-whitelist` becomes a blacklist)
>
> This patch adds a .gittatribute file to build-aux to force LF endings
> on Windows.
>
> Signed-off-by: Alin Gabriel Serdean 
> Co-authored-by: Aaron Conole 
> ---

Signed-off-by: Aaron Conole 

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

Re: [ovs-dev] Introduce ovs-appctl command to monitor HVs sb connection status

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Lorenzo Bianconi, 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: Too many signoffs; are you missing Co-authored-by lines?
> ERROR: Co-authored-by/Signed-off-by corruption
> Lines checked: 144, Warnings: 0, Errors: 2

Probably we can improve this log to be a bit more informative.

In the meantime, from the submitting-patches.rst:

  All co-authors must also sign off.

So, you'll need to get the sign-off tag.  I think patchwork can
automatically record it from a reply, so it would probably be sufficient
to just have aginwala (CCd) to reply to the patch with the appropriate
tag.

Thanks, Lorenzo and aginwala!

-Aaron

> 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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netlink-conntrack: undef the correct macro

[Aaron Conole]

Ben Pfaff  writes:

> On Mon, Jul 23, 2018 at 04:40:49PM -0400, Aaron Conole wrote:
>> Fixes: 6830a0c0e6bf ("netlink-conntrack: New module.")
>> Cc: Daniele Di Proietto 
>> Signed-off-by: Aaron Conole 
>
> Thanks, applied to master.
>
> I didn't backport because I don't think there's any real harm here.

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

Re: [ovs-dev] Revert "dp-packet: Handle multi-seg mbufs in resize__()."

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Tiago Lam, 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.
>

Hi Ian (and all),

Given there are currently two trees (mainline and dpdk_merge), do you
have any preferences, suggestions, or comments on managing the robot
w.r.t. these kind of tree-specific patches?  I suspect that we'll have
others in the future, and I'd like to make the robot be a bit friendlier
this way (plus it helps to build confidence when there are fewer
false-positives).

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

Re: [ovs-dev] dpif-netdev: Fix zero length keys insertion to EMC.

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Ilya Maximets, 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.
>
>

This isn't an issue with your patch, Ilya.

The current mainline has a failing build.

I think just the following would fix it, but I need to do a full test to
confirm.

---

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 13a20f023..6c3736ef5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3481,7 +3481,7 @@ dpif_netdev_flow_dump_cast(struct dpif_flow_dump *dump)
 
 static struct dpif_flow_dump *
 dpif_netdev_flow_dump_create(const struct dpif *dpif_, bool terse,
- char *type OVS_UNUSED)
+ struct dpif_flow_dump_types *type OVS_UNUSED)
 {
 struct dpif_netdev_flow_dump *dump;
 
---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] dpif-netdev: compatible function type

[Aaron Conole]

The dpif_provder flow_dump_create function signature was changed, but
the netdev dpif was not updated along with it.  This generated a build
error with the following warnings:

libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/dpif-netdev.lo -MD 
-MP -MF lib/.deps/dpif-netdev.Tpo -c lib/dpif-netdev.c -o lib/dpif-netdev.o
lib/dpif-netdev.c:6812:5: error: initialization from incompatible pointer type 
[-Werror]
 dpif_netdev_flow_dump_create,
 ^
lib/dpif-netdev.c:6812:5: error: (near initialization for 
'dpif_netdev_class.flow_dump_create') [-Werror]

Fixes: ab15e70eb587 ("dpctl: Expand the flow dump type filter")
Cc: Gavi Teitz 
Cc: Roi Dayan 
Cc: Simon Horman 
Signed-off-by: Aaron Conole 
---
 lib/dpif-netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 13a20f023..6c3736ef5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3481,7 +3481,7 @@ dpif_netdev_flow_dump_cast(struct dpif_flow_dump *dump)
 
 static struct dpif_flow_dump *
 dpif_netdev_flow_dump_create(const struct dpif *dpif_, bool terse,
- char *type OVS_UNUSED)
+ struct dpif_flow_dump_types *type OVS_UNUSED)
 {
 struct dpif_netdev_flow_dump *dump;
 
-- 
2.14.3

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

Re: [ovs-dev] [PATCH v2] tc: Fix sparse warnings.

[Aaron Conole]

Ben Pfaff  writes:

> Fixes the following warnings:
>
> ../lib/tc.c:817:37: error: incorrect type in assignment (different base 
> types)
> ../lib/tc.c:817:37:expected restricted ovs_be16 [usertype] 
> vlan_push_tpid
> ../lib/tc.c:817:37:got unsigned short
> ../lib/tc.c:1522:54: error: incorrect type in argument 2 (different base 
> types)
> ../lib/tc.c:1522:54:expected unsigned short [unsigned] [usertype] tpid
> ../lib/tc.c:1522:54:got restricted ovs_be16 [usertype] vlan_push_tpid
>
> CC: Jianbo Liu 
> CC: Simon Horman 
> Fixes: 61e8655cfc7a ("tc: Add VLAN tpid for push action")
> Signed-off-by: Ben Pfaff 
> ---

Looks good - passed with sparse 0.5.2

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

[ovs-dev] [RFC] Federating the 0-day robot, and improving the testing

[Aaron Conole]

As of June, the 0-day robot has tested over 450 patch series.
Occasionally it spams the list (apologies for that), but for the
majority of the time it has caught issues before they made it to the
tree - so it's accomplishing the initial goal just fine.

I see lots of ways it can improve.  Currently, the bot runs on a light
system.  It takes ~20 minutes to complete a set of tests, including all
the checkpatch and rebuild runs.  That's not a big issue.  BUT, it does
mean that the machine isn't able to perform all the kinds of regression
tests that we would want.  I want to improve this in a way that various
contributors can bring their own hardware and regression tests to the
party.  In that way, various projects can detect potential issues before
they would ever land on the tree and it could flag functional changes
earlier in the process.

I'm not sure the best way to do that.  One thing I'll be doing is
updating the bot to push a series that successfully builds and passes
checkpatch to a special branch on a github repository to kick off travis
builds.  That will give us a more complete regression coverage, and we
could be confident that a series won't break something major.  After
that, I'm not sure how to notify various alternate test infrastructures
how to kick off their own tests using the patched sources.

My goal is to get really early feedback on patch series.  I've sent this
out to the folks I know are involved in testing and test discussions in
the hopes that we can talk about how best to get more CI happening.  The
open questions:

1. How can we notify various downstream consumers of OvS of these
   0-day builds?  Should we just rely on people rolling their own?
   Should there be a more formalized framework?  How will these other
   test frameworks report any kind of failures?

2. What kinds of additional testing do we want to see the robot include?
   Should the test results be made available in general on some kind of
   public facing site?  Should it just stay as a "bleep bloop -
   failure!" marker?

3. What other concerns should be addressed?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 6/6] system-dpdk: Connect network namespaces via dpdkvhostuser ports

[Aaron Conole]

Bala Sankaran  writes:

> - Original Message -
>> From: "Tiago Lam" 
>> To: "Bala Sankaran" , d...@openvswitch.org
>> Sent: Wednesday, 29 August, 2018 1:36:13 PM
>> Subject: Re: [ovs-dev] [PATCH v3 6/6] system-dpdk: Connect network
>> namespaces via dpdkvhostuser ports
>> 
>> Hi Bala,
>> 
>> Thanks to both you and Aaron for working on this. Seems to be a great
>> addition.
>> 
>> As a general comment I agree with Ian that running everything on v17.11
>> would be preferable, as this would enable us to run this test on any
>> given system, and not only when v18.11 is installed. But after reading
>> through your thread on the DPDK users list on the 2MB hugepages
>> limitations around virtio_user, it seems this will have to be a
>> dependency until OvS-DPDK moves to v18.11.
> Hello Tiago,
>
> I agree, I did not happen to notice a workaround for this.
>
>> 
>> On 28/08/2018 18:47, Bala Sankaran wrote:
>> > This adds a new test to the 'check-dpdk' subsystem that will exercise
>> > allocations, PMDs, and the vhost-user code path.
>> > 
>> > Signed-off-by: Bala Sankaran 
>> > Co-authored-by: Aaron Conole 
>> > Signed-off-by: Aaron Conole 
>> > ---
>> >  tests/system-dpdk.at | 77 
>> >  1 file changed, 77 insertions(+)
>> > 
>> > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
>> > index 58dc8aaae..914a1b644 100644
>> > --- a/tests/system-dpdk.at
>> > +++ b/tests/system-dpdk.at
>> > @@ -1,3 +1,6 @@
>> > +m4_define([CONFIGURE_VETH_OFFLOADS],
>> > +   [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
>> > +
>> >  AT_BANNER([OVS-DPDK unit tests])
>> >  
>> >  dnl
>> >  --
>> > @@ -74,3 +77,77 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch
>> > kernel module is probably
>> >  \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
>> >  AT_CLEANUP
>> >  dnl
>> >  --
>> > +
>> > +
>> > +
>> > +dnl
>> > --
>> > +dnl Ping vhost-user-client port
>> > +AT_SETUP([OVS-DPDK datapath - ping vhost-user-client ports])

@bala:

This is wrong here.  These are vhost-user ports, not vhost-user-client ports.

>> 
>> Any reason why you're using vhost-user instead of vhost-user-client? If
>> we change it to "type=dpdkvhostuserclient" in the vhu0 interface added
>> to OvS and append ",server=1" to the net_virtio_user --vdev in the
>> testpmd arguments, doesn't it just work the same?
>
> I believe I encountered an error while running the tests with a vhost-user 
> client ports. That's when I switched over to vhost-user instead. I do not 
> remember the error at this moment, but now that you mentioned it, I am 
> thinking
> of adding another unit test that use vhost-user-client port which would give 
> me the error and then skip to vhost-user instead.

I also don't remember which error we hit and whether it had anything to
do with the type of port.  Maybe it makes sense to have both.  That way
we cover both.  And if we ever completely remove the server mode ports,
we can just drop the test as well (I like that it helps us also catch
the cleanup error in that case).

>> 
>> > +AT_KEYWORDS([dpdk])
>> > +OVS_DPDK_PRE_CHECK()
>> > +OVS_DPDK_START()
>> > +
>> > +dnl Add userspace bridge and attach it to OVS
>> > +AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
>> > +AT_CHECK([ovs-vsctl add-port br10 vhu0 -- set Interface vhu0 \
>> > +  type=dpdkvhostuser], [],
>> > + [stdout], [stderr])
>> > +AT_CHECK([ovs-vsctl show], [], [stdout])
>> > +
>> > +dnl Parse log file
>> > +AT_CHECK([grep "VHOST_CONFIG: vhost-user server: socket created" \
>> > +  ovs-vswitchd.log], [], [stdout])
>> > +AT_CHECK([grep "Socket $OVS_RUNDIR/vhu0 created for vhost-user port vhu0"
>> > \
>> > +  ovs-vswitchd.log], [], [stdout])
>> > +AT_CHECK([grep "VHOST_CONFIG: bind to $OVS_RUNDIR/vhu0" ovs-vswitchd.log],
>> > [],
>> > + [stdout])
>> > +
>> > +dnl Set up namespaces
>> > +ADD_NAMESPACES(ns1, ns2)
>> > +

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

[Aaron Conole]

Hi Arvind,

Aravind Prasad  writes:

> Currently, rule_insert() API doesnot have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
>
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
>
> Rule-insert errors for bundles are not handled in this pull-request.
> Will be handled in upcoming pull request.
>
> Signed-off-by: Aravind Prasad S 
> ---

Thanks for working on OVS.

As noted, the patch has some submission errors.  Please try submitting
again with 'git send-email' to eliminate any potential mail client
munging.

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

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.9

[Aaron Conole]

0-day Robot  writes:

> Bleep bloop.  Greetings Ian Stokes, 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: Too many signoffs; are you missing Co-authored-by lines?
> Lines checked: 92, Warnings: 0, Errors: 1

Greetings Robot,

I'm going to squelch you from working on pull requests and RFCs
tomorrow.  Should make get you to noopsville faster :)

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