Re: [ovs-dev] [PATCH v1] rhel: Add 'SYSTEMD_NO_WRAP=yes' in ovs init script for SLES

[Markos Chandras]

Hello,

On 10/12/2018 14:33, Martin Xu wrote:
> The variable equivalent to RHEL's 'SYSTEMCTL_SKIP_REDIRECT=yes' on SLES
> 12 is 'SYSTEMD_NO_WRAP=yes'
> 
> VMware-BZ: #2245358
> Signed-off-by: Martin Xu 
> CC: Markos Chandras 
> CC: Ansis Atteka 
> CC: Ben Pfaff 
> ---
>  rhel/etc_init.d_openvswitch | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rhel/etc_init.d_openvswitch b/rhel/etc_init.d_openvswitch
> index 20d556803..7a4cfbab5 100755
> --- a/rhel/etc_init.d_openvswitch
> +++ b/rhel/etc_init.d_openvswitch
> @@ -28,6 +28,7 @@
>  ### END INIT INFO
>  
>  SYSTEMCTL_SKIP_REDIRECT=yes
> +SYSTEMD_NO_WRAP=yes
>  
>  . /usr/share/openvswitch/scripts/ovs-lib || exit 1
>  test -e /etc/sysconfig/openvswitch && . /etc/sysconfig/openvswitch
> 

Looks good to me. Thank you

Reviewed-by: Markos Chandras 

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [v2.10 1/2] Set release date for 2.10.1.

[Markos Chandras]

Hello Justin,

On 20/10/2018 00:51, Justin Pettit wrote:
> 
>> On Oct 17, 2018, at 9:47 AM, Ben Pfaff  wrote:
>>
>> All of these seem fine to me.
> 
> Thanks. I pushed them to all the various branches. I’ll get the releases out 
> soon. 
> 
> --Justin
> 

Could you also push the tags for these releases as well? Thank you

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpif-netlink: don't allocate per thread netlink sockets

[Markos Chandras]

On 08/10/2018 17:09, Ben Pfaff wrote:
> 
> I backported:
> 
> 69c51582ff78 ("dpif-netlink: don't allocate per thread netlink sockets")
> to 2.10, 2.9, and 2.8.
> 
> 769b50349f28 ("dpif: Remove support for multiple queues per port.")
> to 2.10.  It got patch rejects on 2.9, so I skipped it there.  If you
> want to fix it up and post a backport, please feel free.
> 
> dc2c9ce38748 ("ofproto-dpif-xlate.c: Fix uninitialized variable
> warning.") to 2.10 because it fixes a warning introduce by 769b50349f28.
> 
> 790a43722974 ("dpif-netlink: Fix null pointer.")  to 2.10, 2.9 and 2.8.
> 

Thank you Ben

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpif-netlink: don't allocate per thread netlink sockets

[Markos Chandras]

On 05/10/2018 09:55, Matteo Croce wrote:
> On Fri, Oct 5, 2018 at 8:32 AM Markos Chandras  wrote:
>>
>> On 25/09/2018 22:14, Ben Pfaff wrote:
>>>
>>> Applied to master thanks!
>>>
>>> I sent a patch to remove support for multiple queues in the
>>> infrastructure layer:
>>> https://patchwork.ozlabs.org/patch/974755/
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
>> Hello Ben and Matteo,
>>
>> This patch applies cleanly to branch-2.10, branch-2.9, and branch-2.8.
>> Would it make sense to backport it to these as well?
>>
>> --
>> markos
>>
>> SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
>> HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
> 
> Hi Markos,
> 
> the file descriptor limit issue was first reported on OVS 2.8, so it's worth 
> it.
> If you backport it, please consider backporting the following commit from Ben,
> which removes some code which become unused after my patch:
> 
> commit 769b50349f28c5f9e4bff102bc61dadcb9b99c37
> Author: Ben Pfaff 
> Date:   Tue Sep 25 15:14:13 2018 -0700
> 
> dpif: Remove support for multiple queues per port.
> 
> Regards,
> 

Thank you Matteo,

Ben, please let me know if you can backport these up to branch-2.8
otherwise I can send patches for each branch myself.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] dpif-netlink: don't allocate per thread netlink sockets

[Markos Chandras]

On 25/09/2018 22:14, Ben Pfaff wrote:
> 
> Applied to master thanks!
> 
> I sent a patch to remove support for multiple queues in the
> infrastructure layer:
> https://patchwork.ozlabs.org/patch/974755/
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

Hello Ben and Matteo,

This patch applies cleanly to branch-2.10, branch-2.9, and branch-2.8.
Would it make sense to backport it to these as well?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 3/5] rhel: add allow_unsupported_modules flag for OVS kmod build

[Markos Chandras]

On 03/09/18 19:11, Flavio Leitner wrote:
> On Fri, Aug 31, 2018 at 11:52:41AM -0700, Martin Xu wrote:
>> Add "--with/without allow_unsupported_modules" flag for rpmbuild. With
>> this flag on, OVS kmod RPM sets allow_unsupported_modules to 1 if needed
>> in /etc/modprobe.d/10-unsupported-modules.conf during post-install.
>> Post-uninstall resets allow_unsupported_modules.
> 
> Sorry, but I can't find why would anyone be interested in setting
> allow_unsupported_modules to any value.
> 
> fbl
> 

I think that RPMs should not silently modify this file. This is
something that needs to be decided by the user/administrator since it
affects the security of the entire system.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 4/5] rhel: allow passing more flags to configure, fedora

[Markos Chandras]

On 03/09/18 19:34, Flavio Leitner wrote:
> On Fri, Aug 31, 2018 at 11:52:42AM -0700, Martin Xu wrote:
>> Define a variable _ovs_config_extra_flags to allow passing more flags to
>> configure when building OVS kmod RPM. For example, to build with a
>> non-standard openssl and add an RPATH, use the following command
>>
>> make rpm-fedora-kmod RPMBUILD_OPT='-D "_ovs_config_extra_flags
>> --with-openssl= LDFLAGS=\"\${LDFLAGS} -Xlinker
>> -rpath=\""'
>>
>> Signed-off-by: Martin Xu 
>> CC: Greg Rose 
>> CC: Flavio Leitner 
> 
> No objections from me.
> Acked-by: Flavio Leitner 
> 
> 

Looks good to me

Reviewed-by: Markos Chandras 

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/5] rhel: openvswitch-kmod-fedora.spec.in file bug fix

[Markos Chandras]

On 03/09/18 18:58, Flavio Leitner wrote:
> 
> The /lib/modules/${kv}/build is a symlink to /usr/src/kernels/${kv}/ 
> on Fedora, so the patch looks good.
> 
> Acked-by: Flavio Leitner 
> 
> fbl
> 

This works on SUSE as well
Reviewed-by: Markos Chandras 

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Markos Chandras]

This fixes the following warning when building Open vSwitch on the
openSUSE Build Service:

  W: non-executable-script 
/usr/share/bash-completion/completions/ovs-appctl-bashcomp.bash
  This text file contains a shebang or is located in a path dedicated
  for executables, but lacks the executable bits and cannot thus be
  executed. If the file is meant to be an executable script, add the
  executable bits, otherwise remove the shebang or move the file
  elsewhere.

The file is meant to be sourced instead of executed, so we can simply
drop the shebang.

Signed-off-by: Markos Chandras 
---
 utilities/ovs-appctl-bashcomp.bash | 1 -
 1 file changed, 1 deletion(-)

diff --git a/utilities/ovs-appctl-bashcomp.bash 
b/utilities/ovs-appctl-bashcomp.bash
index f7fb83047..4384be8ae 100755
--- a/utilities/ovs-appctl-bashcomp.bash
+++ b/utilities/ovs-appctl-bashcomp.bash
@@ -1,4 +1,3 @@
-#!/bin/bash
 #
 # A bash command completion script for ovs-appctl.
 #
-- 
2.18.0

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

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

[Markos Chandras]

On 28/08/18 14:22, Aaron Conole wrote:
> 
> 
> In the future, it's best to indent the lines you intend to be quoting.

Good idea.

> 
> 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).
> 

Let me send a v2 to address that.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Markos Chandras]

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.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Markos Chandras]

This fixes the following warning when building Open vSwitch on the
openSUSE Build Service:

---
W: non-executable-script 
/usr/share/bash-completion/completions/ovs-appctl-bashcomp.bash
This text file contains a shebang or is located in a path dedicated for
executables, but lacks the executable bits and cannot thus be executed.
If the file is meant to be an executable script, add the executable bits,
otherwise remove the shebang or move the file elsewhere.
---

The file is meant to be sourced instead of executed, so we can simply
drop the shebang.

Signed-off-by: Markos Chandras 
---
 utilities/ovs-appctl-bashcomp.bash | 1 -
 1 file changed, 1 deletion(-)

diff --git a/utilities/ovs-appctl-bashcomp.bash 
b/utilities/ovs-appctl-bashcomp.bash
index f7fb83047..4384be8ae 100755
--- a/utilities/ovs-appctl-bashcomp.bash
+++ b/utilities/ovs-appctl-bashcomp.bash
@@ -1,4 +1,3 @@
-#!/bin/bash
 #
 # A bash command completion script for ovs-appctl.
 #
-- 
2.18.0

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

Re: [ovs-dev] s/rhel/rpm/?

[Markos Chandras]

On 08/08/2018 09:01 PM, Ben Pfaff wrote:
> [asking some random SuSE and Red Hat people]
> 
> It had somehow slipped past my notice before that the spec files we have
> are useful for SuSE as well as Red Hat.  Should we make the directory or
> file names more generic?
> 
> Thanks,
> 
> Ben.
> 
Hello Ben,

The SUSE spec file[1] mostly matches the rhel/openvswitch-fedora.spec.in
one from the OvS tree, but because of the different packaging policies
between the two distributions, we need to adapt it a little bit. Our
spec file is also adapted to build on RHEL and SUSE (note all the %if
0%{?suse_version} blocks there).

The rhel/ directory currently has quite a few spec files and most of
them only make sense for RHEL. I can perhaps commit the spec file from
[1] as openvswitch-suse.spec.in and then we can rename the directory to
'rpm' since it would then hold spec files for multiple distros. Would
that work?

[1]
https://build.opensuse.org/package/view_file/network/openvswitch/openvswitch.spec?expand=1

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] rhel: Use correct user in the logrotate configuration file

[Markos Chandras]

The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:

openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir 
/var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config

In order to fix that, we should run the logrotate script as the same
user which runs the various Open vSwitch daemons. If this is a new
installation, then this user is the 'openvswitch' one, but if we are
upgrading from an older release, then the user is normally 'root'.
As such, we set the initial user to 'root' and we fix this up in the
%post scriptlet.

Cc: Aaron Conole 
Cc: Timothy Redaelli 
Signed-off-by: Markos Chandras 
---
 rhel/etc_logrotate.d_openvswitch | 1 +
 rhel/openvswitch-fedora.spec.in  | 4 +++-
 rhel/usr_lib_systemd_system_ovsdb-server.service | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index ed7d733c9..f4302ffbc 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,6 +6,7 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
+su root root
 daily
 compress
 sharedscripts
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 9f8664e95..c2d3200e1 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -405,6 +405,7 @@ exit 0
 %post
 if [ $1 -eq 1 ]; then
 sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
+sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
%{_sysconfdir}/logrotate.d/openvswitch
 
 %if %{with dpdk}
 sed -i \
@@ -414,6 +415,7 @@ if [ $1 -eq 1 ]; then
 
 # In the case of upgrade, this is not needed.
 chown -R openvswitch:openvswitch /etc/openvswitch
+chown -R openvswitch:openvswitch /var/log/openvswitch
 fi
 
 %if 0%{?systemd_post:1}
@@ -601,7 +603,7 @@ fi
 %endif
 %doc NOTICE README.rst NEWS rhel/README.RHEL.rst
 /var/lib/openvswitch
-%attr(750,openvswitch,openvswitch) /var/log/openvswitch
+%attr(750,root,root) /var/log/openvswitch
 %ghost %attr(755,root,root) %{_rundir}/openvswitch
 
 %files ovn-docker
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 0fa57a925..70da1ec95 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -10,7 +10,7 @@ Type=forking
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
-ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
+ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
/var/log/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 \
-- 
2.16.4

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

[ovs-dev] [PATCH v2] rhel: Use correct user in the logrotate configuration file

[Markos Chandras]

The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:

openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir 
/var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config

In order to fix that, we should run the logrotate script as the same
user which runs the various Open vSwitch daemons. If this is a new
installation, then this user is the 'openvswitch' one, but if we are
upgrading from an older release, then the user is normally 'root'.
As such, we set the initial user to 'root' and we fix this up in the
%post scriptlet.

Cc: Aaron Conole 
Cc: Timothy Redaelli 
Signed-off-by: Markos Chandras 
---
 rhel/etc_logrotate.d_openvswitch | 1 +
 rhel/openvswitch-fedora.spec.in  | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index ed7d733c9..f4302ffbc 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,6 +6,7 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
+su root root
 daily
 compress
 sharedscripts
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 9f8664e95..c2d3200e1 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -405,6 +405,7 @@ exit 0
 %post
 if [ $1 -eq 1 ]; then
 sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
+sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
%{_sysconfdir}/logrotate.d/openvswitch
 
 %if %{with dpdk}
 sed -i \
@@ -414,6 +415,7 @@ if [ $1 -eq 1 ]; then
 
 # In the case of upgrade, this is not needed.
 chown -R openvswitch:openvswitch /etc/openvswitch
+chown -R openvswitch:openvswitch /var/log/openvswitch
 fi
 
 %if 0%{?systemd_post:1}
@@ -601,7 +603,7 @@ fi
 %endif
 %doc NOTICE README.rst NEWS rhel/README.RHEL.rst
 /var/lib/openvswitch
-%attr(750,openvswitch,openvswitch) /var/log/openvswitch
+%attr(750,root,root) /var/log/openvswitch
 %ghost %attr(755,root,root) %{_rundir}/openvswitch
 
 %files ovn-docker
-- 
2.16.4

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

Re: [ovs-dev] [PATCH] rhel: Use openvswitch user in the logrotate configuration file

[Markos Chandras]

Hi Timothy,

On 08/07/2018 09:01 PM, Timothy Redaelli wrote:
> 
> Hi Markos,
> I agree with you that running logrotate as root is probably bad.
> 
> The problem is that, for backward compatibility, we keep OVS as "root"
> user if you upgrade OVS from an old version (older than the non-root
> user support).

Good point about the backwards compatibility. I will submit a v2

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: Use openvswitch user in the logrotate configuration file

[Markos Chandras]

The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:

openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir 
/var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config

In order to fix that, we should run the logrotate script as the
openvswitch user which ensures that the correct user is processing
the Open vSwitch log files.

Cc: Aaron Conole 
Cc: Timothy Redaelli 
Signed-off-by: Markos Chandras 
---
 rhel/etc_logrotate.d_openvswitch | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index ed7d733c9..eaf1fd5bf 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,6 +6,7 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
+su openvswitch openvswitch
 daily
 compress
 sharedscripts
-- 
2.16.4

___
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

[Markos Chandras]

Hello Timothy,

On 08/06/2018 01:03 PM, Timothy Redaelli wrote:
> 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 
> ---

Thank you for the fix

Reviewed-by: Markos Chandras 

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
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

[Markos Chandras]

Hello,

On 08/02/2018 08:06 PM, Timothy Redaelli wrote:
> 
> This is, hopefully, the correct git-diff:
> 
> diff --git a/utilities/ovs-lib.in 
> b/utilities/ovs-lib.in 
> index c3b76ec94..33776aac7 100644
> --- a/utilities/ovs-lib.in 
> +++ b/utilities/ovs-lib.in 
> @@ -389,7 +389,10 @@ move_ip_routes () {
> 
>  ovsdb_tool () {
>      if [ "$OVS_USER" != "" ]; then
> -        runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
> +        local uid=$(id -u "${OVS_USER%:*}")
> +        local gid=$(id -g "${OVS_USER%:*}")
> +        local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
> +        setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
> ovsdb-tool -vconsole:off "$@"
>      else
>          ovsdb-tool -vconsole:off "$@"
>      fi

I can also confirm that this seems to work on openSUSE as well. I will
add my Reviewed-by tag once this is properly submitted. Thank you.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
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

[Markos Chandras]

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.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Markos Chandras]

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 :)
---
 utilities/ovs-ctl.in | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 43c8f32b7..588f546fe 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -109,9 +109,15 @@ do_start_ovsdb () {
 if daemon_is_running ovsdb-server; then
 log_success_msg "ovsdb-server is already running"
 else
-# Create initial database or upgrade database schema.
-upgrade_db $DB_FILE $DB_SCHEMA || return 1
-
+# Create initial database or upgrade database schema. The runuser calls
+# in ovsdb_tool function will fail on system startup so we need to run
+# as root and fix permissions later on.
+[ "$OVS_USER" != "" ] && OVS_USER_OVSDB=${OVS_USER}
+OVS_USER="" upgrade_db $DB_FILE $DB_SCHEMA || return 1
+if [ ! -z "${OVS_USER_OVSDB+x}" ]; then
+OVS_USER=${OVS_USER_OVSDB}
+chown -R "$OVS_USER" $etcdir $dbdir
+fi
 # Start ovsdb-server.
 set ovsdb-server "$DB_FILE"
 for db in $EXTRA_DBS; do
-- 
2.18.0

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

Re: [ovs-dev] [PATCH] rhel: Use openvswitch user/group for the log directory

[Markos Chandras]

On 23/05/18 14:46, Timothy Redaelli wrote:
> Commit 94cd8383e297 ("rhel: fix log directory permissions") restored the
> old 755 permission on /var/log/openvswitch and this can result in the
> exposure of sensitive information.
> 
> Since commit f624bf23b62a ("rhel: user/group openvswitch does not exist")
> moved the user/group creations in %pre phase it's now possible to change
> /var/log/openvswitch user/group to openvswitch:openvswitch and remove
> the r/x bits for other again without having the "permission denied"
> error when the logs are rotated.
> 
> CC: Aaron Conole <acon...@redhat.com>
> Fixes: 94cd8383e297 ("rhel: fix log directory permissions")
> Signed-off-by: Timothy Redaelli <tredae...@redhat.com>
> Acked-by: Aaron Conole <acon...@redhat.com>
> ---

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Build failures with openvswitch-{2.9.1, 2.8.3} due to missing rte_mempool_ops_get_count DPDK symbol

[Markos Chandras]

On 23/05/18 10:08, Stokes, Ian wrote:
>> On 05/23/2018 09:38 AM, Markos Chandras wrote:
>>> On 23/05/18 09:14, Kevin Traynor wrote:
>>>> On 05/23/2018 08:48 AM, Markos Chandras wrote:
>>>>> On 23/05/18 08:41, Markos Chandras wrote:
>>>>>> Hello,
>>>>>> [...]
>>>>>>
>>>>>> This was added in commit 91fccdad72a253a3892dcb3c4453a31833851bb7
>>>>>> ("netdev-dpdk: Free mempool only when no in-use mbufs."). I am not
>>>>>> sure how that ever built since that symbol was never exported by
>>>>>> the rte_mempool DPDK library
>>>>>>
>>>>>
>>>>> I am also adding Kevin Traynor on CC who is the author of this commit.
>>>>>
>>>>
>>>> hi Markos, sorry for the trouble, I will send a fix this morning,
>>>>
>>>> thanks,
>>>> Kevin.
>>>>
>>> Hi Kevin,
>>>
>>> No problem. Thank you for working on this. A backport will also be
>>> needed for the following branches as well
>>>
>>> - branch-2.9
>>> - branch-2.8
>>> - branch-2.7
>>> - branch-2.6
>>>
>>
>> ACK
> 
> Thanks for reporting this Markos, it seems to be a gap in the validation 
> testing we conduct. Are you building with DPDK as a shared lib in this case? 
> I hadn't come across the issue on my own system or in travis to date but it's 
> obviously a problem.
> 

Hi Ian,

Yeah we build against a shared DPDK library. By the looks of it, static
building should be fine though.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Build failures with openvswitch-{2.9.1, 2.8.3} due to missing rte_mempool_ops_get_count DPDK symbol

[Markos Chandras]

On 23/05/18 09:14, Kevin Traynor wrote:
> On 05/23/2018 08:48 AM, Markos Chandras wrote:
>> On 23/05/18 08:41, Markos Chandras wrote:
>>> Hello,
>>> [...]
>>>
>>> This was added in commit 91fccdad72a253a3892dcb3c4453a31833851bb7
>>> ("netdev-dpdk: Free mempool only when no in-use mbufs."). I am not sure
>>> how that ever built since that symbol was never exported by the
>>> rte_mempool DPDK library
>>>
>>
>> I am also adding Kevin Traynor on CC who is the author of this commit.
>>
> 
> hi Markos, sorry for the trouble, I will send a fix this morning,
> 
> thanks,
> Kevin.
> 
Hi Kevin,

No problem. Thank you for working on this. A backport will also be
needed for the following branches as well

- branch-2.9
- branch-2.8
- branch-2.7
- branch-2.6

Maybe worth cutting new releases with the fix to resolve the DPDK build
failures.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Build failures with openvswitch-{2.9.1, 2.8.3} due to missing rte_mempool_ops_get_count DPDK symbol

[Markos Chandras]

On 23/05/18 08:41, Markos Chandras wrote:
> Hello,
> [...]
> 
> This was added in commit 91fccdad72a253a3892dcb3c4453a31833851bb7
> ("netdev-dpdk: Free mempool only when no in-use mbufs."). I am not sure
> how that ever built since that symbol was never exported by the
> rte_mempool DPDK library
> 

I am also adding Kevin Traynor on CC who is the author of this commit.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Build failures with openvswitch-{2.9.1, 2.8.3} due to missing rte_mempool_ops_get_count DPDK symbol

[Markos Chandras]

Hello,

I was trying to update the openSUSE package to 2.9.1 (2.8.3 has the same
issue) but the following build error occurs

[   58s] libtool: link: gcc -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 -Wswitch-bool
-Wlogical-not-parentheses -Wsizeof-array-argument -Wbool-compare
-Wshift-negative-value -Wduplicated-cond -mssse3
-I/usr/local/include/dpdk -I/usr/include/dpdk -D_FILE_OFFSET_BITS=64
-fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2
-fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables
-fstack-clash-protection -o ovn/controller/.libs/ovn-controller
ovn/controller/bfd.o ovn/controller/binding.o ovn/controller/chassis.o
ovn/controller/encaps.o ovn/controller/gchassis.o ovn/controller/lflow.o
ovn/controller/lport.o ovn/controller/ofctrl.o ovn/controller/pinctrl.o
ovn/controller/patch.o ovn/controller/ovn-controller.o
ovn/controller/physical.o  ovn/lib/.libs/libovn.so
lib/.libs/libopenvswitch.so -lssl -lcrypto -lcap-ng -ldpdk -lnuma
-latomic -lpthread -lrt -lm -lpcap -Wl,-rpath -Wl,/usr/lib64
[   58s] lib/.libs/libopenvswitch.so: undefined reference to
`rte_mempool_ops_get_count'

The 'rte_mempool_ops_get_count' symbol seems to be an internal DPDK one
since it doesn't seem to be exported in the map file

http://www.dpdk.org/browse/dpdk/tree/lib/librte_mempool/rte_mempool_version.map

This was added in commit 91fccdad72a253a3892dcb3c4453a31833851bb7
("netdev-dpdk: Free mempool only when no in-use mbufs."). I am not sure
how that ever built since that symbol was never exported by the
rte_mempool DPDK library

Any clues?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: openvswitch-fedora.spec.in: Drop explicit usermod/groupadd deps

[Markos Chandras]

These dependencies have been moved from the %post to the %pre scriptlet
in f624bf23b62a ("rhel: user/group openvswitch does not exist") and are
already provided by the shadow-utils package so we can simply drop
them.

Cc: Alan Pevec <alan.pe...@redhat.com>
Cc: Aaron Conole <acon...@redhat.com>
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 rhel/openvswitch-fedora.spec.in | 4 
 1 file changed, 4 deletions(-)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 3e5cf21e0..6dbf20ce1 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -94,10 +94,6 @@ Requires: openssl hostname iproute module-init-tools
 
 Requires(pre): shadow-utils
 Requires(post): /bin/sed
-%if %{with dpdk}
-Requires(post): /usr/sbin/usermod
-Requires(post): /usr/sbin/groupadd
-%endif
 Requires(post): systemd-units
 Requires(preun): systemd-units
 Requires(postun): systemd-units
-- 
2.16.3

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

Re: [ovs-dev] [PATCH v3] rhel: user/group openvswitch does not exist

[Markos Chandras]

On 19/04/18 16:27, Aaron Conole wrote:
> From: Alan Pevec <alan.pe...@redhat.com>
> 
> Default ownership[1] for config files is failing on an empty system:
>   Running scriptlet: openvswitch-2.9.0-3.fc28.x86_64
> warning: user openvswitch does not exist - using root
> warning: group openvswitch does not exist - using root
> ...
> 
> Required user/group need to be created in %pre as documented in
> Fedora guideline[2]
> 
> [1] 
> https://github.com/openvswitch/ovs/commit/951d79e638ecdb3b1dcd19df1adb2ff91fe61af8
> 
> [2] https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation
> 
> Submitted-at: https://github.com/openvswitch/ovs/pull/223
> Signed-off-by: Alan Pevec <alan.pe...@redhat.com>
> Co-authored-by: Aaron Conole <acon...@redhat.com>
> Signed-off-by: Aaron Conole <acon...@redhat.com>

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg

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

Re: [ovs-dev] [PATCH v2] rhel: user/group openvswitch does not exist

[Markos Chandras]

Hi Aaron,

On 18/04/18 15:51, Aaron Conole wrote:
> v2:
>  * Removed the requires(post) lines
>  * Removed 'exit 0'

I realize that I was the one suggested to drop 'exit 0', but right at
the bottom of
https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation

it seems that 'exit 0' may do more good than bad. Having said that, this
could make it easy to miss useradd/usermod/etc failure so I am not too
sure if we want to do that :)

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: user/group openvswitch does not exist

[Markos Chandras]

On 10/04/18 14:49, Aaron Conole wrote:
> +%pre
> +getent group openvswitch >/dev/null || groupadd -r openvswitch
> +getent passwd openvswitch >/dev/null || \
> +useradd -r -g openvswitch -d / -s /sbin/nologin \
> +-c "Open vSwitch Daemons" openvswitch
> +
> +%if %{with dpdk}
> +getent group hugetlbfs >/dev/null || groupadd hugetlbfs
> +usermod -a -G hugetlbfs openvswitch
> +%endif
> +exit 0

Maybe this 'exit 0' is not actually needed here?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg

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

Re: [ovs-dev] [PATCH] rhel: user/group openvswitch does not exist

[Markos Chandras]

On 10/04/18 14:49, Aaron Conole wrote:
> From: Alan Pevec <alan.pe...@redhat.com>
> 
> Default ownership[1] for config files is failing on an empty system:
>   Running scriptlet: openvswitch-2.9.0-3.fc28.x86_64
> warning: user openvswitch does not exist - using root
> warning: group openvswitch does not exist - using root
> ...
> 
> Required user/group need to be created in %pre as documented in
> Fedora guideline[2]
> 
> [1] 
> https://github.com/openvswitch/ovs/commit/951d79e638ecdb3b1dcd19df1adb2ff91fe61af8
> 
> [2] https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation
> 
> Submitted-at: https://github.com/openvswitch/ovs/pull/223
> Signed-off-by: Alan Pevec <alan.pe...@redhat.com>
> Co-authored-by: Aaron Conole <acon...@redhat.com>
> Signed-off-by: Aaron Conole <acon...@redhat.com>
> ---

Looks good to me.

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg

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

Re: [ovs-dev] [PATCH] rhel: Stop managing the /run/openvswitch directory with systemd.

[Markos Chandras]

On 27/03/18 15:06, Aaron Conole wrote:
> There are a few advantages (and some disadvantages, also).
> 
> One thing that's nice is systemd will clean up the directories when the
> service ends.  I realize that /run is usually tmpfs, but it's nice that
> they don't linger - even if ovs-lib "breaks in the middle" (meaning
> something goes wrong .. though I'm unable to name an instance where I
> observed that).  Actually, I am looking at tmpfiles.d entries for
> managing some of these complicated directory lists (like /dev/hugepages,
> etc).
> 

I agree, tmpfiles.d is probably going to simplify things a bit.

> Another advantage is when we fully hook up with the
> user+group+capabilities (it's on my TODO list) in systemd service
> files.  At that point, it will not be possible for the ovs-lib to create
> the runtime directories.
> 
> Of course, we know the biggest disadvantage - if systemd breaks things,
> they are really broken.
> 
> Does it make sense?
> 

Yes thank you very much

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Stop managing the /run/openvswitch directory with systemd.

[Markos Chandras]

On 27/03/18 14:34, Aaron Conole wrote:
> 
> Systemd has fixed this with commit:
> 
> 30c81ce2cef9 ("pid1: when creating service directories, don't chown existing 
> files")
> 
> Which was caught thanks to some proactive testing:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1508495
> 
> I think we probably don't need this fix, provided downstream versions
> backport that commit.
> 

Hi Aaron,

Thank you for the information. I am curious, do you know why we are
managing the /run/openvswitch directory in the systemd service file
given that ovs-lib already tries to manage it as well?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: Stop managing the /run/openvswitch directory with systemd.

[Markos Chandras]

It appears that new systemd versions (tested with v237) changed the
way RuntimeDirectory option behaves. Upstream commit 3536f49e8fa2
("core: add {State,Cache,Log,Configuration}Directory=") modified the
RuntimeDirectory code to run before every ExecStart* command instead
of running it once per service file when the service is run as 'root'.

This breaks the ovsdb-server because after the chown command was applied,
the RuntimeDirectory code was executed again, effectively wiping the
/run/openvswitch directory and creating it again resulting in the
following problem.

|2|daemon_unix|EMER|/var/run/openvswitch/ovsdb-server.pid.tmp: create 
failed (Permission denied)
Mar 19 16:37:20 susetest ovs-ctl[3045]: ovsdb-server: 
/var/run/openvswitch/ovsdb-server.pid.tmp: create failed (Permission denied)
Mar 19 16:37:20 susetest ovs-ctl[3045]: Starting ovsdb-server ... failed!

The ovs-lib code can already manage that directory for us so we can
remove these entries from the systemd file and let ovs-vsctl do it.

Cc: Aaron Conole <acon...@redhat.com>
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 rhel/usr_lib_systemd_system_ovsdb-server.service | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 234d39355..ce496ec30 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -10,7 +10,6 @@ Type=forking
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
-ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovs-vswitchd --no-monitor --system-id=random \
   --ovs-user=${OVS_USER_ID} \
@@ -19,5 +18,3 @@ 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} \
--no-monitor restart $OPTIONS
-RuntimeDirectory=openvswitch
-RuntimeDirectoryMode=0755
-- 
2.16.2

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

Re: [ovs-dev] [PATCH] m4: Try to use "python" as Python2 interpreter only as last resort

[Markos Chandras]

Hi Timothy,

On 08/03/18 16:15, Timothy Redaelli wrote:
> This patch tries to find Python 2 as "python2", then "python2.7" and finally
> "python".
> 
> This is needed since "/usr/bin/python" is used as Python 3 on some Linux
> distributions (for example on Arch Linux) and on Fedora 28
> "/usr/bin/python" will be deprecated [1]:
> "All scripts shall explicitly use /usr/bin/python2."
> 
> [1] https://fedoraproject.org/wiki/FinalizingFedoraSwitchtoPython3
> 
> Signed-off-by: Timothy Redaelli 
> ---

So if I understand this correctly, in Fedora 28 and Arch Linux,
/usr/bin/python will actually be the python3 interp right? Why not
extend the macro then to also look for python3 and python3.5 python3.4
if no python2 or python exists? In openSUSE tumbleweed, there is no
python2, python2.X or python if you only have python3 installed so you
are only left with /usr/bin/python3 and /usr/bin/python3.X

Would you be able to amend your patch to look for these additional
python strings?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.8 4/4] ovsdb: ovsdb-dot.in: Replace sys.maxint with sys.maxsize for Python3

[Markos Chandras]

There is no sys.maxint anymore on python3. However, sys.maxsize can be
used as an integer larger than any practical list or string index.

Link: https://docs.python.org/3.1/whatsnew/3.0.html#integers
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 43c50dabd..8eea61724 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -15,14 +15,14 @@ def printEdge(tableName, type, baseType, label):
 if type.n_min == 0:
 if type.n_max == 1:
 arity = "?"
-elif type.n_max == sys.maxint:
+elif type.n_max == sys.maxsize:
 arity = "*"
 else:
 arity = "{,%d}" % type.n_max
 elif type.n_min == 1:
 if type.n_max == 1:
 arity = ""
-elif type.n_max == sys.maxint:
+elif type.n_max == sys.maxsize:
 arity = "+"
 else:
 arity = "{1,%d}" % type.n_max
-- 
2.16.2

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

[ovs-dev] [PATCH branch-2.8 2/4] ovsdb: ovsdb-dot.in: Use print function for Python3

[Markos Chandras]

The python2 print statement no longer works in python3 since the
latter uses a print function. As such, replace all instances of
'print' with 'print()'. This fixes the following build problem with
python3

> ovsdb/ovsdb-client.1.tmp
File "./ovsdb/ovsdb-dot.in", line 34
print "\t%s -> %s [%s];" % (
   ^
SyntaxError: invalid syntax

Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index f7b7ab0db..7f846836d 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -31,38 +31,38 @@ def printEdge(tableName, type, baseType, label):
 options['label'] = '"%s%s"' % (label, arity)
 if baseType.ref_type == 'weak':
 options['style'] = 'dotted'
-print "\t%s -> %s [%s];" % (
+print ("\t%s -> %s [%s];" % (
 tableName,
 baseType.ref_table_name,
-', '.join(['%s=%s' % (k,v) for k,v in options.items()]))
+', '.join(['%s=%s' % (k,v) for k,v in options.items()])))
 
 def schemaToDot(schemaFile, arrows):
 schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schemaFile))
 
-print "digraph %s {" % schema.name
-print '\trankdir=LR;'
-print '\tsize="6.5,4";'
-print '\tmargin="0";'
-print "\tnode [shape=box];"
+print ("digraph %s {" % schema.name)
+print ('\trankdir=LR;')
+print ('\tsize="6.5,4";')
+print ('\tmargin="0";')
+print ("\tnode [shape=box];")
 if not arrows:
-print "\tedge [dir=none, arrowhead=none, arrowtail=none];"
+print ("\tedge [dir=none, arrowhead=none, arrowtail=none];")
 for tableName, table in schema.tables.items():
 options = {}
 if table.is_root:
 options['style'] = 'bold'
-print "\t%s [%s];" % (
+print ("\t%s [%s];" % (
 tableName,
-', '.join(['%s=%s' % (k,v) for k,v in options.items()]))
+', '.join(['%s=%s' % (k,v) for k,v in options.items()])))
 for columnName, column in table.columns.items():
 if column.type.value:
 printEdge(tableName, column.type, column.type.key, "%s key" % 
columnName)
 printEdge(tableName, column.type, column.type.value, "%s 
value" % columnName)
 else:
 printEdge(tableName, column.type, column.type.key, columnName)
-print "}";
+print ("}");
 
 def usage():
-print """\
+print ("""\
 %(argv0)s: compiles ovsdb schemas to graphviz format
 Prints a .dot file that "dot" can render to an entity-relationship diagram
 usage: %(argv0)s [OPTIONS] SCHEMA
@@ -72,7 +72,7 @@ The following options are also available:
   --no-arrows omit arrows from diagram
   -h, --help  display this help message
   -V, --version   display version information\
-""" % {'argv0': argv0}
+""" % {'argv0': argv0})
 sys.exit(0)
 
 if __name__ == "__main__":
@@ -92,7 +92,7 @@ if __name__ == "__main__":
 elif key in ['-h', '--help']:
 usage()
 elif key in ['-V', '--version']:
-print "ovsdb-dot (Open vSwitch) @VERSION@"
+print ("ovsdb-dot (Open vSwitch) @VERSION@")
 else:
 sys.exit(0)
 
-- 
2.16.2

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

[ovs-dev] [PATCH branch-2.8 3/4] ovsdb: ovsdb-dot.in: Change exception semantics for Python3

[Markos Chandras]

PEP-3110 changes the semantics for capturing exceptions in Python3
from 'except E,N' to 'except E as N'. This fixes the following problem
when building with python3

SyntaxError: invalid syntax
  File "./ovsdb/ovsdb-dot.in", line 106
except ovs.db.error.Error, e:
 ^
SyntaxError: invalid syntax

Link: https://www.python.org/dev/peps/pep-3110/
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 7f846836d..43c50dabd 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -81,7 +81,7 @@ if __name__ == "__main__":
 options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
   ['no-arrows',
'help', 'version',])
-except getopt.GetoptError, geo:
+except getopt.GetoptError as geo:
 sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
 sys.exit(1)
 
@@ -103,7 +103,7 @@ if __name__ == "__main__":
 
 schemaToDot(args[0], arrows)
 
-except ovs.db.error.Error, e:
+except ovs.db.error.Error as e:
 sys.stderr.write("%s: %s\n" % (argv0, e.msg))
 sys.exit(1)
 
-- 
2.16.2

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

[ovs-dev] [PATCH branch-2.8 1/4] ovsdb: Use items() instead of iteritems() for Python3

[Markos Chandras]

Python3 removed the iteritems() iterator and replaced it with items()
which should also work in Python2. This fixes the following build
problem on Python3:

Traceback (most recent call last):
  File "./ovsdb/ovsdb-idlc.in", line 1436, in 
func(*args[1:])
  File "./ovsdb/ovsdb-idlc.in", line 314, in printCIDLHeader
for columnName, column in sorted(table.columns.iteritems()):
AttributeError: 'dict' object has no attribute 'iteritems'

Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in  | 4 ++--
 ovsdb/ovsdb-idlc.in | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 134ce2269..f7b7ab0db 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -46,14 +46,14 @@ def schemaToDot(schemaFile, arrows):
 print "\tnode [shape=box];"
 if not arrows:
 print "\tedge [dir=none, arrowhead=none, arrowtail=none];"
-for tableName, table in schema.tables.iteritems():
+for tableName, table in schema.tables.items():
 options = {}
 if table.is_root:
 options['style'] = 'bold'
 print "\t%s [%s];" % (
 tableName,
 ', '.join(['%s=%s' % (k,v) for k,v in options.items()]))
-for columnName, column in table.columns.iteritems():
+for columnName, column in table.columns.items():
 if column.type.value:
 printEdge(tableName, column.type, column.type.key, "%s key" % 
columnName)
 printEdge(tableName, column.type, column.type.value, "%s 
value" % columnName)
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index f065ef1c6..7e1c36d1f 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -281,7 +281,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum 
%(s)s_column_id);
 # Table indexes.
 print("struct %(s)s * %(s)s_index_init_row(struct ovsdb_idl *, const 
struct ovsdb_idl_table_class *);" % {'s': structName})
 print
-for columnName, column in sorted(table.columns.iteritems()):
+for columnName, column in sorted(table.columns.items()):
 print('void %(s)s_index_set_%(c)s(const struct %(s)s *,' % {'s': 
structName, 'c': columnName})
 if column.type.is_smap():
 args = ['const struct smap *']
@@ -1030,7 +1030,7 @@ void
 struct %(s)s *
 %(s)s_index_init_row(struct ovsdb_idl *idl, const struct ovsdb_idl_table_class 
*class)
 {""" % {'s': structName, 't': tableName})
-#for columnName, column in sorted(table.columns.iteritems()):
+#for columnName, column in sorted(table.columns.items()):
 #if column.type.is_smap():
 #print "smap_init(>%s);" % columnName
 print("return (struct %(s)s *) ovsdb_idl_index_init_row(idl, 
class);" % {'s': structName, 't': tableName})
@@ -1097,7 +1097,7 @@ struct %(s)s *
 return %(s)s_cast(ovsdb_idl_index_data(CONST_CAST(struct 
ovsdb_idl_index_cursor *, cursor)));
 }""" % { 's' : structName })
 # Indexes Set functions
-for columnName, column in sorted(table.columns.iteritems()):
+for columnName, column in sorted(table.columns.items()):
 type = column.type
 
 comment, members = cMembers(prefix, tableName, columnName,
-- 
2.16.2

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

[ovs-dev] [PATCH branch-2.8 0/4] ovsdb-dot python3 fixes

[Markos Chandras]

Hello,

This patchset is a backport of the following python-3 fixes
https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/342460.html
for branch 2.8

Markos Chandras (4):
  ovsdb: Use items() instead of iteritems() for Python3
  ovsdb: ovsdb-dot.in: Use print function for Python3
  ovsdb: ovsdb-dot.in: Change exception semantics for Python3
  ovsdb: ovsdb-dot.in: Replace sys.maxint with sys.maxsize for Python3

 ovsdb/ovsdb-dot.in  | 40 
 ovsdb/ovsdb-idlc.in |  6 +++---
 2 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.16.2

___
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

[Markos Chandras]

Hello,

On 31/01/18 00:07, Marcos Felipe Schwarz wrote:
> 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##*:}

There is something missing^^ here. Where do you apply the chown command?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.8] netdev-dpdk: replace uint8_t with dpdk_port_t

[Markos Chandras]

Hi Ben,

On 09/01/18 16:56, Ben Pfaff wrote:
>> Sure, I was just concerned was it fixing a compilation issue or such for 
>> you. I've seen it's been applied already and I've given it a quick 
>> validation check without issue so no worries.
> 
> I interpreted your Signed-off-by as a request to apply it, but I think I
> must have misunderstood a backport of a patch already on master.  I
> won't apply it so quickly next time.  I'm learning here too :-)
> 

Oh should I have done something different to make it clear that it was a
backport? I simply took the patch from master, modified the Subject line
to indicate the branch for the backport and added my SoB line. Might be
better to use 'cherry-pick -x' for backports to make it more explicit. I
apologize for any confusion this may have caused

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.8] netdev-dpdk: replace uint8_t with dpdk_port_t

[Markos Chandras]

Hi Ian,

On 09/01/18 16:16, Stokes, Ian wrote:
>> -Original Message-
>> From: Markos Chandras [mailto:mchand...@suse.de]
>> Sent: Tuesday, January 9, 2018 3:55 PM
>> To: d...@openvswitch.org
>> Cc: Kavanagh, Mark B <mark.b.kavan...@intel.com>; Ilya Maximets
>> <i.maxim...@samsung.com>; Stokes, Ian <ian.sto...@intel.com>; Markos
>> Chandras <mchand...@suse.de>
>> Subject: [PATCH branch-2.8] netdev-dpdk: replace uint8_t with dpdk_port_t
>>
>> From: Mark Kavanagh <mark.b.kavan...@intel.com>
>>
>> netdev_dpdk_detach() declares a 'port_id' variable, of type uint8_t.
>> This variable should instead be of type dpdk_port_t.
> 
> Hi Markos, is there a specific reason you require this patch back ported to 
> 2.8?

No particular reason I just thought to point that out since in other
parts of this file dpdk_port_t is used for the port_id, for example in
the netdev_dpdk_process_devargs() and netdev_dpdk_set_config() functions
so maybe worth fixing these inconsistencies.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.8] netdev-dpdk: replace uint8_t with dpdk_port_t

[Markos Chandras]

From: Mark Kavanagh <mark.b.kavan...@intel.com>

netdev_dpdk_detach() declares a 'port_id' variable, of type uint8_t.
This variable should instead be of type dpdk_port_t.

Fixes: bb37956ac ("netdev-dpdk: Use uint8_t for port_id.")
CC: Ilya Maximets <i.maxim...@samsung.com>
Signed-off-by: Mark Kavanagh <mark.b.kavan...@intel.com>
Acked-by: Ilya Maximets <i.maxim...@samsung.com>
Signed-off-by: Ian Stokes <ian.sto...@intel.com>
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 41acb5b62..dc96d7ce3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2521,7 +2521,7 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 {
 int ret;
 char *response;
-uint8_t port_id;
+dpdk_port_t port_id;
 char devname[RTE_ETH_NAME_MAX_LEN];
 struct netdev_dpdk *dev;
 
-- 
2.15.1

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

[ovs-dev] [PATCH 4/4] ovsdb: ovsdb-dot.in: Replace sys.maxint with sys.maxsize for Python3

[Markos Chandras]

There is no sys.maxint anymore on python3. However, sys.maxsize can be
used as an integer larger than any practical list or string index.

Link: https://docs.python.org/3.1/whatsnew/3.0.html#integers
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 43c50dabd..8eea61724 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -15,14 +15,14 @@ def printEdge(tableName, type, baseType, label):
 if type.n_min == 0:
 if type.n_max == 1:
 arity = "?"
-elif type.n_max == sys.maxint:
+elif type.n_max == sys.maxsize:
 arity = "*"
 else:
 arity = "{,%d}" % type.n_max
 elif type.n_min == 1:
 if type.n_max == 1:
 arity = ""
-elif type.n_max == sys.maxint:
+elif type.n_max == sys.maxsize:
 arity = "+"
 else:
 arity = "{1,%d}" % type.n_max
-- 
2.15.1

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

[ovs-dev] [PATCH 3/4] ovsdb: ovsdb-dot.in: Change exception semantics for Python3

[Markos Chandras]

PEP-3110 changes the semantics for capturing exceptions in Python3
from 'except E,N' to 'except E as N'. This fixes the following problem
when building with python3

SyntaxError: invalid syntax
  File "./ovsdb/ovsdb-dot.in", line 106
except ovs.db.error.Error, e:
 ^
SyntaxError: invalid syntax

Link: https://www.python.org/dev/peps/pep-3110/
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 7f846836d..43c50dabd 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -81,7 +81,7 @@ if __name__ == "__main__":
 options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
   ['no-arrows',
'help', 'version',])
-except getopt.GetoptError, geo:
+except getopt.GetoptError as geo:
 sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
 sys.exit(1)
 
@@ -103,7 +103,7 @@ if __name__ == "__main__":
 
 schemaToDot(args[0], arrows)
 
-except ovs.db.error.Error, e:
+except ovs.db.error.Error as e:
 sys.stderr.write("%s: %s\n" % (argv0, e.msg))
 sys.exit(1)
 
-- 
2.15.1

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

[ovs-dev] [PATCH 2/4] ovsdb: ovsdb-dot.in: Use print function for Python3

[Markos Chandras]

The python2 print statement no longer works in python3 since the
latter uses a print function. As such, replace all instances of
'print' with 'print()'. This fixes the following build problem with
python3

> ovsdb/ovsdb-client.1.tmp
File "./ovsdb/ovsdb-dot.in", line 34
print "\t%s -> %s [%s];" % (
   ^
SyntaxError: invalid syntax

Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index f7b7ab0db..7f846836d 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -31,38 +31,38 @@ def printEdge(tableName, type, baseType, label):
 options['label'] = '"%s%s"' % (label, arity)
 if baseType.ref_type == 'weak':
 options['style'] = 'dotted'
-print "\t%s -> %s [%s];" % (
+print ("\t%s -> %s [%s];" % (
 tableName,
 baseType.ref_table_name,
-', '.join(['%s=%s' % (k,v) for k,v in options.items()]))
+', '.join(['%s=%s' % (k,v) for k,v in options.items()])))
 
 def schemaToDot(schemaFile, arrows):
 schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schemaFile))
 
-print "digraph %s {" % schema.name
-print '\trankdir=LR;'
-print '\tsize="6.5,4";'
-print '\tmargin="0";'
-print "\tnode [shape=box];"
+print ("digraph %s {" % schema.name)
+print ('\trankdir=LR;')
+print ('\tsize="6.5,4";')
+print ('\tmargin="0";')
+print ("\tnode [shape=box];")
 if not arrows:
-print "\tedge [dir=none, arrowhead=none, arrowtail=none];"
+print ("\tedge [dir=none, arrowhead=none, arrowtail=none];")
 for tableName, table in schema.tables.items():
 options = {}
 if table.is_root:
 options['style'] = 'bold'
-print "\t%s [%s];" % (
+print ("\t%s [%s];" % (
 tableName,
-', '.join(['%s=%s' % (k,v) for k,v in options.items()]))
+', '.join(['%s=%s' % (k,v) for k,v in options.items()])))
 for columnName, column in table.columns.items():
 if column.type.value:
 printEdge(tableName, column.type, column.type.key, "%s key" % 
columnName)
 printEdge(tableName, column.type, column.type.value, "%s 
value" % columnName)
 else:
 printEdge(tableName, column.type, column.type.key, columnName)
-print "}";
+print ("}");
 
 def usage():
-print """\
+print ("""\
 %(argv0)s: compiles ovsdb schemas to graphviz format
 Prints a .dot file that "dot" can render to an entity-relationship diagram
 usage: %(argv0)s [OPTIONS] SCHEMA
@@ -72,7 +72,7 @@ The following options are also available:
   --no-arrows omit arrows from diagram
   -h, --help  display this help message
   -V, --version   display version information\
-""" % {'argv0': argv0}
+""" % {'argv0': argv0})
 sys.exit(0)
 
 if __name__ == "__main__":
@@ -92,7 +92,7 @@ if __name__ == "__main__":
 elif key in ['-h', '--help']:
 usage()
 elif key in ['-V', '--version']:
-print "ovsdb-dot (Open vSwitch) @VERSION@"
+print ("ovsdb-dot (Open vSwitch) @VERSION@")
 else:
 sys.exit(0)
 
-- 
2.15.1

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

[ovs-dev] [PATCH 1/4] ovsdb: Use items() instead of iteritems() for Python3

[Markos Chandras]

Python3 removed the iteritems() iterator and replaced it with items()
which should also work in Python2. This fixes the following build
problem on Python3:

Traceback (most recent call last):
  File "./ovsdb/ovsdb-idlc.in", line 1436, in 
func(*args[1:])
  File "./ovsdb/ovsdb-idlc.in", line 314, in printCIDLHeader
for columnName, column in sorted(table.columns.iteritems()):
AttributeError: 'dict' object has no attribute 'iteritems'

Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 ovsdb/ovsdb-dot.in  | 4 ++--
 ovsdb/ovsdb-idlc.in | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ovsdb/ovsdb-dot.in b/ovsdb/ovsdb-dot.in
index 134ce2269..f7b7ab0db 100755
--- a/ovsdb/ovsdb-dot.in
+++ b/ovsdb/ovsdb-dot.in
@@ -46,14 +46,14 @@ def schemaToDot(schemaFile, arrows):
 print "\tnode [shape=box];"
 if not arrows:
 print "\tedge [dir=none, arrowhead=none, arrowtail=none];"
-for tableName, table in schema.tables.iteritems():
+for tableName, table in schema.tables.items():
 options = {}
 if table.is_root:
 options['style'] = 'bold'
 print "\t%s [%s];" % (
 tableName,
 ', '.join(['%s=%s' % (k,v) for k,v in options.items()]))
-for columnName, column in table.columns.iteritems():
+for columnName, column in table.columns.items():
 if column.type.value:
 printEdge(tableName, column.type, column.type.key, "%s key" % 
columnName)
 printEdge(tableName, column.type, column.type.value, "%s 
value" % columnName)
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 96420ca0b..2edb9ef54 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -311,7 +311,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum 
%(s)s_column_id);
 # Table indexes.
 print("struct %(s)s * %(s)s_index_init_row(struct ovsdb_idl *, const 
struct ovsdb_idl_table_class *);" % {'s': structName})
 print
-for columnName, column in sorted(table.columns.iteritems()):
+for columnName, column in sorted(table.columns.items()):
 print('void %(s)s_index_set_%(c)s(const struct %(s)s *,' % {'s': 
structName, 'c': columnName})
 if column.type.is_smap():
 args = ['const struct smap *']
@@ -1065,7 +1065,7 @@ void
 struct %(s)s *
 %(s)s_index_init_row(struct ovsdb_idl *idl, const struct ovsdb_idl_table_class 
*class_)
 {""" % {'s': structName, 't': tableName})
-#for columnName, column in sorted(table.columns.iteritems()):
+#for columnName, column in sorted(table.columns.items()):
 #if column.type.is_smap():
 #print "smap_init(>%s);" % columnName
 print("return (struct %(s)s *) ovsdb_idl_index_init_row(idl, 
class_);" % {'s': structName, 't': tableName})
@@ -1132,7 +1132,7 @@ struct %(s)s *
 return %(s)s_cast(ovsdb_idl_index_data(CONST_CAST(struct 
ovsdb_idl_index_cursor *, cursor)));
 }""" % { 's' : structName })
 # Indexes Set functions
-for columnName, column in sorted(table.columns.iteritems()):
+for columnName, column in sorted(table.columns.items()):
 type = column.type
 
 comment, members = cMembers(prefix, tableName, columnName,
-- 
2.15.1

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

[ovs-dev] [PATCH 0/4] ovsdb-dot python3 fixes

[Markos Chandras]

Hello,

This patchset fixes some python3 issues when running the ovsdb-dot
tool with python3.

Markos Chandras (4):
  ovsdb: Use items() instead of iteritems() for Python3
  ovsdb: ovsdb-dot.in: Use print function for Python3
  ovsdb: ovsdb-dot.in: Change exception semantics for Python3
  ovsdb: ovsdb-dot.in: Replace sys.maxint with sys.maxsize for Python3

 ovsdb/ovsdb-dot.in  | 40 
 ovsdb/ovsdb-idlc.in |  6 +++---
 2 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.15.1

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

Re: [ovs-dev] [PATCH] daemon-unix: include missing help information

[Markos Chandras]

On 11/12/17 15:07, Aaron Conole wrote:
> These options have existed for a while, but were not expressed in the
> help information.  Inform the user that these options exist, and give
> some basic help.
> 
> Reported-by: Saravanan KR <skram...@redhat.com>
> Signed-off-by: Aaron Conole <acon...@redhat.com>
> ---
>  lib/daemon-unix.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index 967a28432..adb549c98 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -537,6 +537,8 @@ daemon_usage(void)
>  printf(
>  "\nDaemon options:\n"
>  "  --detachrun in background as daemon\n"
> +"  --monitor   creates a process to monitor this 
> daemon\n"
> +"  --user=username[:group] changes the effective daemon user:group\n"
>  "  --no-chdir  do not chdir to '/'\n"
>  "  --pidfile[=FILE]create pidfile (default: %s/%s.pid)\n"
>  "  --overwrite-pidfile with --pidfile, start even if already "
> 

Good catch

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2.5] ofp-util: Fix memory leaks on error cases in ofputil_decode_group_mod().

[Markos Chandras]

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

Found by libFuzzer.

Reported-by: Bhargava Shastry <bshas...@sec.t-labs.tu-berlin.de>
Signed-off-by: Ben Pfaff <b...@ovn.org>
Acked-by: Justin Pettit <jpet...@ovn.org>
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
Hi Ben,

It seems that this patch applies with some fixups on branch-2.5 so it should be
good for inclusion.
---
 lib/ofp-util.c | 88 ++
 1 file changed, 51 insertions(+), 37 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7b4d62b99..a29c47370 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -8039,6 +8039,7 @@ ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t 
buckets_length,
 if (!ob) {
 VLOG_WARN_RL(_ofmsg_rl, "buckets end with %"PRIuSIZE" leftover 
bytes",
  buckets_length);
+ofputil_bucket_list_destroy(buckets);
 return OFPERR_OFPGMFC_BAD_BUCKET;
 }
 
@@ -8046,11 +8047,13 @@ ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t 
buckets_length,
 if (ob_len < sizeof *ob) {
 VLOG_WARN_RL(_ofmsg_rl, "OpenFlow message bucket length "
  "%"PRIuSIZE" is not valid", ob_len);
+ofputil_bucket_list_destroy(buckets);
 return OFPERR_OFPGMFC_BAD_BUCKET;
 } else if (ob_len > buckets_length) {
 VLOG_WARN_RL(_ofmsg_rl, "OpenFlow message bucket length "
  "%"PRIuSIZE" exceeds remaining buckets data size 
%"PRIuSIZE,
  ob_len, buckets_length);
+ofputil_bucket_list_destroy(buckets);
 return OFPERR_OFPGMFC_BAD_BUCKET;
 }
 buckets_length -= ob_len;
@@ -8769,6 +8772,7 @@ ofputil_pull_ofp11_group_mod(struct ofpbuf *msg, enum 
ofp_version ofp_version,
 && gm->command == OFPGC11_DELETE
 && !list_is_empty(>buckets)) {
 error = OFPERR_OFPGMFC_INVALID_GROUP;
+ofputil_bucket_list_destroy(>buckets);
 }
 
 return error;
@@ -8836,44 +8840,9 @@ ofputil_pull_ofp15_group_mod(struct ofpbuf *msg, enum 
ofp_version ofp_version,
 return error;
 }
 
-/* Converts OpenFlow group mod message 'oh' into an abstract group mod in
- * 'gm'.  Returns 0 if successful, otherwise an OpenFlow error code. */
-enum ofperr
-ofputil_decode_group_mod(const struct ofp_header *oh,
- struct ofputil_group_mod *gm)
+static enum ofperr
+ofputil_check_group_mod(const struct ofputil_group_mod *gm)
 {
-enum ofp_version ofp_version = oh->version;
-struct ofpbuf msg;
-struct ofputil_bucket *bucket;
-enum ofperr err;
-
-ofpbuf_use_const(, oh, ntohs(oh->length));
-ofpraw_pull_assert();
-
-ofputil_init_group_properties(>props);
-
-switch (ofp_version)
-{
-case OFP11_VERSION:
-case OFP12_VERSION:
-case OFP13_VERSION:
-case OFP14_VERSION:
-err = ofputil_pull_ofp11_group_mod(, ofp_version, gm);
-break;
-
-case OFP15_VERSION:
-err = ofputil_pull_ofp15_group_mod(, ofp_version, gm);
-break;
-
-case OFP10_VERSION:
-default:
-OVS_NOT_REACHED();
-}
-
-if (err) {
-return err;
-}
-
 switch (gm->type) {
 case OFPGT11_INDIRECT:
 if (!list_is_singleton(>buckets)) {
@@ -8903,6 +8872,7 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
 return OFPERR_OFPGMFC_BAD_COMMAND;
 }
 
+struct ofputil_bucket *bucket;
 LIST_FOR_EACH (bucket, list_node, >buckets) {
 if (bucket->weight && gm->type != OFPGT11_SELECT) {
 return OFPERR_OFPGMFC_INVALID_GROUP;
@@ -8930,6 +8900,50 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
 return 0;
 }
 
+/* Converts OpenFlow group mod message 'oh' into an abstract group mod in
+ * 'gm'.  Returns 0 if successful, otherwise an OpenFlow error code. */
+enum ofperr
+ofputil_decode_group_mod(const struct ofp_header *oh,
+ struct ofputil_group_mod *gm)
+{
+enum ofp_version ofp_version = oh->version;
+struct ofpbuf msg;
+enum ofperr err;
+
+ofpbuf_use_const(, oh, ntohs(oh->length));
+ofpraw_pull_assert();
+
+ofputil_init_group_properties(>props);
+
+switch (ofp_version)
+{
+case OFP11_VERSION:
+case OFP12_VERSION:
+case OFP13_VERSION:
+case OFP14_VERSION:
+err = ofputil_pull_ofp11_group_mod(, ofp_version, gm);
+break;
+
+case OFP15_VERSION:
+err = ofputil_pull_ofp15_group_mod(, ofp_version, gm);
+break;
+
+case OFP10_VERSION:
+default:
+OVS_NOT_REACHED();
+}
+
+if (err) {
+return err;
+}
+
+err = ofputil_check_group_mod(gm);
+if (err) {
+ofputil_uninit_g

Re: [ovs-dev] [PATCH v4 2/3] ofp-util: Fix memory leaks on error cases in ofputil_decode_group_mod().

[Markos Chandras]

On 22/09/17 22:57, Ben Pfaff wrote:
> On Fri, Sep 22, 2017 at 02:26:36PM -0700, Justin Pettit wrote:
>>
>>> On Sep 21, 2017, at 9:59 AM, Ben Pfaff  wrote:
>>>
>>> Found by libFuzzer.
>>>
>>> Reported-by: Bhargava Shastry 
>>> Signed-off-by: Ben Pfaff 
>>
>> Acked-by: Justin Pettit 
> 
> Thanks, applied to master, backported as far as 2.6.

Hello,

Should this patch be in 2.5 too given it's an LTS release?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: fix log directory permissions

[Markos Chandras]

On 22/09/17 23:47, Flavio Leitner wrote:
> On Fri, 22 Sep 2017 09:44:18 -0400
> Aaron Conole <acon...@redhat.com> wrote:
> 
>> When the logrotate script runs, and Open vSwitch is running as a non-root
>> user, the /var/log/openvswitch directory doesn't have other rx bits set.
>> This means the reopen attempt will fail with "permission denied", even though
>> the default logrotate configuration creates a new log file with the
>> appropriate attributes.
>>
>> This change sets the r/x bits for other on /var/log/messages
> 
> /var/log/openvswitch? :-)
> 
> Reproduced here
> # ovs-appctl -t ovs-vswitchd vlog/reopen 
> Permission denied
> ovs-appctl: ovs-vswitchd: server returned an error
> 
> Acked-by: Flavio Leitner <f...@sysclose.org>
> 

Reviewed-by: Markos Chandras <mchand...@suse.de>

I guess we may want this patch in the 2.8 branch as well :)

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Markos Chandras]

On 08/08/2017 11:01 PM, Timothy M. Redaelli wrote:
> 
> The script is actually "specialized" to how Fedora/RHEL starts
> openvswitch since, I think, only Fedora/RHEL have ovsdb-server and
> ovs-vswitchd as splitted systemd unit files.

For the record, SUSE uses the same approach

> [...]
> 
> Sounds like a good suggestion.
> 
> I'll do another patchset with a commit that adds the --bundle option +
> get_highest_ofp_version in ovs-save and another commit that changes
> ovs-reload script to use it.
> 
> get_highest_ofp_version is used to find if we can use --bundle and to
> make ovs-ofctl work when you have a bridge with only OpenFlow15+
> protocols enabled.
> 
> Any other suggestions?

The rest looks good to me. Thank you

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/4] redhat: dynamically allocate and reference ovs user

[Markos Chandras]

On 07/05/2017 10:16 PM, Aaron Conole wrote:
> Hi Markos,
> [...]
>>
>> I am a bit puzzled about this to be honest... I am wondering if it would
>> be better to do it the other way around. For example, supply a sysconfig
>> file with OVS_USER_ID commented out, but if it's an upgrade, then do the
>> sed magic to switch to root:root so things keep working as before. Would
>> that be better?
> 
> I prefer to do modifications only on install; that's the only time we
> know for sure the exact state of the files being manipulated.  On
> upgrade, I worry that a user can change the contents in such a way that
> the script matches, but does the wrong thing.  Does it make sense, or
> did I misunderstand?
> 

Yeah I agree that it may need some work to ensure that it will always do
the right thing. I don't mind, just wanted to explore the alternative
option.

>>> +
>>> +# In the case of upgrade, this is not needed.
>>> +chown -R openvswitch:openvswitch /etc/openvswitch
>>
>> Should this be part of the systemd file in a ExecStartPre statement
>> instead? Similar to what you do for the /var/run/openvswitch directory.
> 
> I thought about doing that in the systemd script, but it exposes a
> vulnerability.  Assume that I have access to the openvswitch user (for a
> moment).
> 
>   openvswitch /tmp$ gcc -o give_me_a_shell give_me_a_shell.c
>   openvswitch /tmp$ chmod 4755 give_me_a_shell
>   openvswitch /tmp$ cp give_me_a_shell /etc/openvswitch/
>   openvswitch /tmp$ echo 'OVS_USER_ID=root:root' >> \
>  /etc/sysconfig/openvswitch
>   openvswitch /tmp$ # do something that makes the admin restart ovs
>   openvswitch /tmp$ ls -lah /etc/openvswitch/give_me_a_shell
>   srwxr-xr-x.   1 root root 5.5K Jul  5 13:42 give_me_a_shell
>   openvswitch /tmp$ /etc/openvswitch/give_me_a_shell
>   # id
>   uid=0(root) gid=0(root) groups=0(root)
> 
> So, I left it out.  The alternative is to list every file we wish to
> have chmod'ed, but I think that's probably a bit much.

Good point.

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/4] redhat: dynamically allocate and reference ovs user

[Markos Chandras]

Hi Aaron,

On 07/05/2017 08:56 PM, Aaron Conole wrote:
> After this commit, the fedora RPM will create the openvswitch user, from the
> non-static pool, for use as an Open vSwitch daemon user.  This only happens
> on install - not upgrade.  This will be the default user:group
> combination for the openvswitch daemons.
> 
> Signed-off-by: Aaron Conole 
> ---
>  rhel/openvswitch-fedora.spec.in  | 13 +
>  rhel/usr_lib_systemd_system_ovsdb-server.service |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 88d4331..7c805b2 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -92,6 +92,9 @@ Requires: openssl hostname iproute module-init-tools
>  #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3
>  #Requires: kernel >= 3.15.0-0
>  
> +Requires(post): /usr/bin/getent
> +Requires(post): /usr/sbin/useradd
> +Requires(post): /usr/bin/sed
>  Requires(post): systemd-units
>  Requires(preun): systemd-units
>  Requires(postun): systemd-units
> @@ -357,6 +360,16 @@ rm -rf $RPM_BUILD_ROOT
>  %endif
>  
>  %post
> +if [ $1 -eq 1 ]; then
> +getent passwd openvswitch >/dev/null || \
> +useradd -r -d / -s /sbin/nologin -c "Open vSwitch Daemons" 
> openvswitch
> +
> +sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch

I am a bit puzzled about this to be honest... I am wondering if it would
be better to do it the other way around. For example, supply a sysconfig
file with OVS_USER_ID commented out, but if it's an upgrade, then do the
sed magic to switch to root:root so things keep working as before. Would
that be better?

> +
> +# In the case of upgrade, this is not needed.
> +chown -R openvswitch:openvswitch /etc/openvswitch

Should this be part of the systemd file in a ExecStartPre statement
instead? Similar to what you do for the /var/run/openvswitch directory.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 4/6] redhat: allow arbitrary user:group

[Markos Chandras]

On 03/06/17 16:09, Aaron Conole wrote:
> Under rpm based distributions, the only user:group that the rhel daemons run
> as is 'root:root'.  This is fine as a default, but as part of a security
> procedure, users may want to run as an alternate uid/gid.  This commit
> adds an OVS_USER_ID environment variable for systemd, which defaults to
> root:root, but can be overridden by changing the /etc/sysconfig/openvswitch
> environment file.
> 
> Signed-off-by: Aaron Conole <acon...@redhat.com>
> ---

I think it looks reasonable

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/6] redhat: make the rpm aware of the lock file

[Markos Chandras]

On 07/06/17 22:00, Flavio Leitner wrote:
> On Sat, Jun 03, 2017 at 11:09:58AM -0400, Aaron Conole wrote:
>> Currently, the db lockfile will cause the openvswitch directory to
>> linger after uninstall because the rpm database isn't aware that it
>> should be treated as part of the system.  This commit informs the rpmdb
>> properly as a 'ghost' so that when the package is uninstalled, it will
>> be removed automatically.  This means that if no extra files exist in
>> /etc/openvswitch, the whole directory will be removed from /etc/.
>>
>> Signed-off-by: Aaron Conole <acon...@redhat.com>
>> ---
> 
> This looks unrelated to the userspace work as the previous patch,
> other than that it looks good to me.
> 
> Acked-by: Flavio Leitner <f...@sysclose.org>
> 

Reviewed-by: Markos Chandras <mchan...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] lib/automake.mk: remove runtime directories

[Markos Chandras]

Hi Aaron,

On 03/09/2017 03:35 PM, Aaron Conole wrote:
> The Open vSwitch run, log, and DB directories are installed as part of the
> normal `make install` process.  However, this means they are created with
> user and group ownership that may conflict with the desired user.  For
> example, running `make install` as root will install those files as
> root:root, whereas the runtime user desired may be openvswitch:openvswitch.
> 
> Since these directories are automatically created as part of the ovs-ctl
> command, and with the correct user:group permissions, it makes sense to
> delay creation until these directories are actually required.
> 
> Signed-off-by: Aaron Conole <acon...@redhat.com>

It looks reasonable to me. Thanks!

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Makefile: Break the build if .gitignore files are distributed.

[Markos Chandras]

Hi Ben,

On 03/08/2017 12:29 AM, Ben Pfaff wrote:
> This would have found a .gitignore file recently added to the distribution.
> 
> CC: Markos Chandras <mchand...@suse.de>
> CC: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---

Looks reasonable to me. Thank you

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs-ctl: allow passing user:group to daemons

[Markos Chandras]

On 02/21/2017 10:31 PM, Aaron Conole wrote:
> The Open vSwitch daemons allow passing --user user[:group] to allow
> spawning under different user privileges.  ovs-ctl now accepts --ovs-user
> in the same form to pass this argument on, as well as create databases and
> data directories with the appropriate privileges.
> 
> Signed-off-by: Aaron Conole <acon...@redhat.com>

Looks reasonable to me. Thanks!

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel-systemd: Restart openvswitch service if a daemon crashes

[Markos Chandras]

On 02/07/2017 02:49 PM, Eelco Chaudron wrote:
> On 07/02/17 13:56, Markos Chandras wrote:
>> Hi,
>>
>> It looks sensible to me but...
>>
>> On 02/07/2017 01:38 PM, Eelco Chaudron wrote:
>>> Currently if either ovsdb-server or ovs-vswitchd is crashing the
>>> daemon is not restarting leaving the system in faulty state.
>>> This patch will detect the daemon crash and will restart the
>>> openvswitch service.
>> Is there any chance to end up in some sort of infinite 'start' loop here
>> if the ovsdb-server or ovs-vswitchd crashes can't be self-healed by a
>> simple restart?
>>
> In this case systemd will step in and not restart the process. You will
> get an error like this;
> systemd: start request repeated too quickly for openvswitch.service
> 
> //Eelco
> 

OK sounds good to me.

Reviewed-by: Markos Chandras <mchand...@suse.de>

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel-systemd: Restart openvswitch service if a daemon crashes

[Markos Chandras]

Hi,

It looks sensible to me but...

On 02/07/2017 01:38 PM, Eelco Chaudron wrote:
> Currently if either ovsdb-server or ovs-vswitchd is crashing the
> daemon is not restarting leaving the system in faulty state.
> This patch will detect the daemon crash and will restart the
> openvswitch service.
> 
> Here is a (bit to wide) table showing the behavior before and after
> the patch. Note that only the Crash behavior has changed:
> [..]
> ---
> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service 
> b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
> index 01d9cee..39627e9 100644
> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service
> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
> @@ -9,6 +9,7 @@ PartOf=openvswitch.service
>  
>  [Service]
>  Type=forking
> +Restart=on-failure
>  [...]
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
> b/rhel/usr_lib_systemd_system_ovsdb-server.service
> index d3d7408..68deace 100644
> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -7,6 +7,7 @@ PartOf=openvswitch.service
>  
>  [Service]
>  Type=forking
> +Restart=on-failure

Is there any chance to end up in some sort of infinite 'start' loop here
if the ovsdb-server or ovs-vswitchd crashes can't be self-healed by a
simple restart?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] libX: add new release / version info tags

[Markos Chandras]

Hi,

On 01/16/2017 08:06 PM, Aaron Conole wrote:
> This commit uses the $PACKAGE_VERSION automake variable to construct a
> release and version info combination which sets the library name to be:
> 
>libfoo-$(OVS_MAJOR_VERSION).so.$(OVS_MINOR_VERSION).0.$(OVS_MICRO_VERSION)
> 
> where formerly, it was always:
> 
>libfoo.so.1.0.0
> 
> This allows releases of Open vSwitch libraries to reflect which specific
> versions they came with, and sets up a psuedo ABI-versioning scheme.  In
> this fashion, future releases of Open vSwitch could be installed
> alongside older releases, allowing 3rd party utilities linked against
> previous versions to continue to function.

So I am a bit late here but the end result is not very consistent I think

/usr/lib64/libofproto-2.so.7
/usr/lib64/libofproto-2.so.7.0.90
/usr/lib64/libopenvswitch-2.so.7
/usr/lib64/libopenvswitch-2.so.7.0.90
...
/usr/lib64/libofproto.so
/usr/lib64/libopenvswitch.so
/usr/lib64/libovn.so
/usr/lib64/libovsdb.so

Shouldn't the linker names also have the major version in the naming
scheme? Like

/usr/lib64/libofproto-2.so.7
/usr/lib64/libofproto-2.so.7.0.90
/usr/lib64/libopenvswitch-2.so.7
/usr/lib64/libopenvswitch-2.so.7.0.90
...
/usr/lib64/libofproto-2.so
/usr/lib64/libopenvswitch-2.so
/usr/lib64/libovn-2.so
/usr/lib64/libovsdb-2.so

I understand that it could be problematic for existing applications and
that we may have to provide symlinks like

libofproto.so -> libofproto-2.so

etc

but the existing solution feels like that only half of the problem is
solved. Or perhaps this is OK and we want everybody to link against the
latest version when building an app.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] windows: automake.mk: Remove the .gitignore file from distributed files

[Markos Chandras]

Commit d183efc22b2b ("This commit adds the windows installer to the
OVS tree.) added the .gitignore file to the distributed files but this
file shouldn't be part of the distributed archive.

Cc: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com>
Signed-off-by: Markos Chandras <mchand...@suse.de>
---
 windows/automake.mk | 1 -
 1 file changed, 1 deletion(-)

diff --git a/windows/automake.mk b/windows/automake.mk
index db2b61639..4fec99ed9 100644
--- a/windows/automake.mk
+++ b/windows/automake.mk
@@ -38,7 +38,6 @@ windows_installer: all
MSBuild.exe windows/ovs-windows-installer.sln /target:Build 
/property:Configuration="Release" /property:Version="$(PACKAGE_VERSION)"
 
 EXTRA_DIST += \
-   windows/.gitignore \
windows/automake.mk \
windows/README.rst \
windows/ovs-windows-installer.sln \
-- 
2.11.0

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

Re: [ovs-dev] [PATCH] datapath: internal-dev: Disable rtnl_link_ops register under linux < 3.17

[Markos Chandras]

Hi,

On 12/14/2016 03:44 AM, Zhang Dongya wrote:
> From: "fortitude.zhang" <fortitude.zh...@gmail.com>
> 
> Under linux < 3.17, b0ab2fabb5b91da99c189db02e91ae10bc8355c5 is not 
> introduced,
> which allow internal-dev created by openvswitch datapath deleted by using rtnl
> interface, this causes data related to internal-dev not freed and stops 
> datapath
> working correctly.
> 
> Signed-off-by: fortitude.zhang <fortitude.zh...@gmail.com>

I recently encountered this bug and this commit fixed it for me. So...

Tested-by: Markos Chandras <mchand...@suse.de>

It might also worth backporting it to stable branches as well.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-dpdk: Add support for DPDK 16.11

[Markos Chandras]

Hi Ciara,

On 11/24/2016 02:20 PM, Ciara Loftus wrote:
> This commit announces support for DPDK 16.11. Compaitibilty with DPDK
There is a typo in "Compatibility" :)

> v16.07 is not broken yet thanks to only minor code changes being needed
> for the upgrade.
> 
> Signed-off-by: Ciara Loftus 
> ---

Do you have any idea if this patch (and possibly the series from
https://mail.openvswitch.org/pipermail/ovs-dev/2016-October/243394.html)
are going to be backported to the 'branch-2.6'? My gut tells me 'no', so
I suspect that ovs-2.6 will still work with dpdk-16.11 but it will not
make use of all its features right?

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev