Re: [ovs-dev] [PATCH 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[aginwala aginwala]

Sure. I will add ssl usage example with some brief in the ovn-ctl.8.xml and
send v3 for this patch . Does that sound good?

On Mon, Oct 8, 2018 at 5:32 PM Han Zhou  wrote:

>
>
> On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala 
> wrote:
> >
> >
> >
> > On Mon, Oct 8, 2018 at 10:47 AM Han Zhou  wrote:
> >>
> >>
> >>
> >> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala 
> wrote:
> >> >
> >> > Thanks for the review Han. Please find the comments inline below:
> >> > On Thu, Oct 4, 2018 at 9:58 AM Han Zhou  wrote:
> >> >>
> >> >> Thanks Ali, please see my comments below
> >> >>
> >> >> On Fri, Sep 21, 2018 at 5:34 PM  wrote:
> >> >> >
> >> >> >  For OVN DBs to work with SSL in HA, we need to have capability to
> >> >> >  pass ssl certs when starting OVN DBs. Say when starting OVN DBs
> in active
> >> >> >  passive mode, in order for the standby DBs to sync from master
> node, it
> >> >> >  cannot sync because the required ssl certs are not passed when
> standby DBs
> >> >> >  are initialized. Hence, we need to have this option.
> >> >> >
> >> >> > e.g. start nb db with ssl certs as below:
> >> >> > /usr/share/openvswitch/scripts/ovn-ctl
> --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
> >> >> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
> >> >> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
> >> >> > --db-nb-create-insecure-remote=no start_nb_ovsdb
> >> >> >
> >> >> > Certs can be generated based on ovs ssl docs:
> >> >> > http://docs.openvswitch.org/en/latest/howto/ssl/
> >> >> >
> >> >> > Signed-off-by: aginwala 
> >> >> > ---
> >> >> >  ovn/utilities/ovn-ctl | 50
> +++---
> >> >> >  1 file changed, 43 insertions(+), 7 deletions(-)
> >> >> >
> >> >> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> >> >> > index 3ff0df6..4f45f3d 100755
> >> >> > --- a/ovn/utilities/ovn-ctl
> >> >> > +++ b/ovn/utilities/ovn-ctl
> >> >> > @@ -116,6 +116,9 @@ start_ovsdb__() {
> >> >> >  local addr
> >> >> >  local active_conf_file
> >> >> >  local use_remote_in_db
> >> >> > +local ovn_db_ssl_key
> >> >> > +local ovn_db_ssl_cert
> >> >> > +local ovn_db_ssl_cacert
> >> >> >  eval pid=\$DB_${DB}_PID
> >> >> >  eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
> >> >> >  eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> >> >> > @@ -137,6 +140,9 @@ start_ovsdb__() {
> >> >> >  eval addr=\$DB_${DB}_ADDR
> >> >> >  eval active_conf_file=\$ovn${db}_active_conf_file
> >> >> >  eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
> >> >> > +eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
> >> >> > +eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
> >> >> > +eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
> >> >> >
> >> >> >  # Check and eventually start ovsdb-server for DB
> >> >> >  if pidfile_is_running $pid; then
> >> >> > @@ -182,17 +188,32 @@ $cluster_remote_port
> >> >> >
> >> >> >  if test X"$use_remote_in_db" != Xno; then
> >> >> >  set "$@" --remote=db:$schema_name,$table_name,connections
> >> >> > +if test X"$create_insecure_remote" = Xno; then
> >> >> > +set "$@" --remote=pssl:$port:$addr
> >> >> > +elif test X"$create_insecure_remote" = Xyes; then
> >> >> > +set "$@" --remote=ptcp:$port:$addr
> >> >> > +fi
> >> >> Why moving the logic here? This if block only says if the connection
> settings in DB table should be used. Whether insecure mode is allowed was
> supposed to be independent with this condition. Could you explain the
> reason behind the change?
> >> >> >> I moved it because remote=db is needed if ovsdb is running as a
> standalone node or an active ovsdb server node. For standby nodes in case
> of active_passive mode, remote=db will not be there because it uses
> --sync-from. Hope its clear.
> >>
> >> As discussed, $use_remote_in_db and $create_insecure_remote were
> independent. Moving $create_insecure_remote logic here make it useful only
> when $use_remote_in_db is "yes", which is not how it was supposed to be.
> >>
> >> I understand that for standby node, we will set $use_remote_in_db as
> "no". Is this a problem?
> >>
> > >> Just verified and have kept the logic intact as even for ssl, we have
> connection table set for 0.0.0.0 for LB use case and it works fine. Hence,
> have updated it in v2. Please take a look and let me know.
> >>
> >> --sync-from has nothing to do with "remote".
> >> >
> >> >
> >> >>
> >> >> >  fi
> >> >> > -set "$@" --private-key=db:$schema_name,SSL,private_key
> >> >> > -set "$@" --certificate=db:$schema_name,SSL,certificate
> >> >> > -set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
> >> >> > -set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
> >> >> > -set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
> >> >>
> >> >> So it will not use the settings in DB any more? It seems this change
> removed support for "--use-remote-in-db=yes", 

[ovs-dev] Hallo mein Geliebter bitte ich brauche deine Assistentin

[Gertrude Robert via dev]

Hallo mein Geliebter.

Ich grüße dich mit dem Namen unseres Herrn Jesus Christus. Es ist wahr, dass 
dieser Brief als Überraschung zu dir kommen kann. Dennoch bitte ich Sie 
demütig, mir Ihre Aufmerksamkeit zu schenken und mich gut zu hören. Mein Name 
ist Frau Gertrude aus den Vereinigten Staaten von Amerika. Ich bin verheiratet 
mit Herrn Robert Hermann, der im Jahr 2002 mit unserer Botschaft in Deutschland 
gearbeitet hat und auch für 16 Jahre in einer Botschaft in London gearbeitet 
hat, bevor er starb.

Wir waren 25 Jahre ohne Kind verheiratet, bevor er nach kurzer Krankheit starb. 
Seit seinem Tod entschied ich mich aufgrund meines religiösen Glaubens nicht 
wieder zu heiraten. Als mein verstorbener Mann noch am Leben war, hinterlegte 
er die Summe von $ 10.500.000,00 (Zehn Millionen Fünfhunderttausend US-Dollar) 
bei einer Bank hier in Amerika. Zurzeit befindet sich dieses Geld noch in der 
Obhut der Bank. Kürzlich sagte mir mein Doktor, dass ich wegen meiner 
Krebserkrankung für die nächsten vier Monate nicht durchhalten würde.

Nachdem ich meinen Zustand gekannt habe, habe ich beschlossen, dieses Geld an 
Kirchen, Organisationen oder gute Menschen zu spenden, die dieses Geld nutzen 
werden, so wie ich es hier unterrichten werde.

Ich möchte, dass Sie dieses Geld für Kirchen, Wohltätigkeitsorganisationen, 
Waisenhäuser, Witwen und andere Bedürftige verwenden. Ich habe diese 
Entscheidung getroffen, weil ich kein Kind habe, das dieses Geld erbt. Außerdem 
sind die Verwandten meines Mannes mir nicht nahe, da ich ein Krebsproblem habe 
und es ihr Wunsch war, mich tot zu sehen, um seinen Reichtum zu erben, da wir 
kein Kind haben. Diese Leute sind dieses Erbe nicht wert. Aus diesem Grund 
nehme ich die Entscheidung, dich zu kontaktieren und spende diesen Fond an 
dich, damit du ihn für die Wohltätigkeitsarbeit verwenden kannst.
Ich möchte, dass Sie 30% des Gesamtbetrags für sich selbst übernehmen.
Wenn Sie sicher sind, dass Sie diesen Fonds in Übereinstimmung mit den 
Anweisungen verwenden,

Sobald ich Ihre Antwort erhalten habe, werde ich Ihnen den Kontakt der Bank 
hier in den Vereinigten Staaten von Amerika geben, wo dieses Geld hinterlegt 
ist. Ich werde der Bank auch ein Bewilligungsschreiben ausstellen, das Ihnen 
den gegenwärtigen Begünstigten dieses Geldes beweisen wird. Ich möchte auch, 
dass du mich immer in deine Gebete bringst.

Jede Verzögerung in Ihrer Antwort kann mir Raum geben, nach einem anderen guten 
Menschen zu diesem Zweck zu suchen. Bitte versichern Sie mir, dass Sie sich wie 
oben beschrieben verhalten werden.

Danke und verbleibe gesegnet.

Deine Schwester im Herrn,

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

Re: [ovs-dev] [PATCH 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[Han Zhou]

On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala  wrote:
>
>
>
> On Mon, Oct 8, 2018 at 10:47 AM Han Zhou  wrote:
>>
>>
>>
>> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala 
wrote:
>> >
>> > Thanks for the review Han. Please find the comments inline below:
>> > On Thu, Oct 4, 2018 at 9:58 AM Han Zhou  wrote:
>> >>
>> >> Thanks Ali, please see my comments below
>> >>
>> >> On Fri, Sep 21, 2018 at 5:34 PM  wrote:
>> >> >
>> >> >  For OVN DBs to work with SSL in HA, we need to have capability to
>> >> >  pass ssl certs when starting OVN DBs. Say when starting OVN DBs in
active
>> >> >  passive mode, in order for the standby DBs to sync from master
node, it
>> >> >  cannot sync because the required ssl certs are not passed when
standby DBs
>> >> >  are initialized. Hence, we need to have this option.
>> >> >
>> >> > e.g. start nb db with ssl certs as below:
>> >> > /usr/share/openvswitch/scripts/ovn-ctl
--ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
>> >> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
>> >> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
>> >> > --db-nb-create-insecure-remote=no start_nb_ovsdb
>> >> >
>> >> > Certs can be generated based on ovs ssl docs:
>> >> > http://docs.openvswitch.org/en/latest/howto/ssl/
>> >> >
>> >> > Signed-off-by: aginwala 
>> >> > ---
>> >> >  ovn/utilities/ovn-ctl | 50
+++---
>> >> >  1 file changed, 43 insertions(+), 7 deletions(-)
>> >> >
>> >> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
>> >> > index 3ff0df6..4f45f3d 100755
>> >> > --- a/ovn/utilities/ovn-ctl
>> >> > +++ b/ovn/utilities/ovn-ctl
>> >> > @@ -116,6 +116,9 @@ start_ovsdb__() {
>> >> >  local addr
>> >> >  local active_conf_file
>> >> >  local use_remote_in_db
>> >> > +local ovn_db_ssl_key
>> >> > +local ovn_db_ssl_cert
>> >> > +local ovn_db_ssl_cacert
>> >> >  eval pid=\$DB_${DB}_PID
>> >> >  eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
>> >> >  eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
>> >> > @@ -137,6 +140,9 @@ start_ovsdb__() {
>> >> >  eval addr=\$DB_${DB}_ADDR
>> >> >  eval active_conf_file=\$ovn${db}_active_conf_file
>> >> >  eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
>> >> > +eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
>> >> > +eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
>> >> > +eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
>> >> >
>> >> >  # Check and eventually start ovsdb-server for DB
>> >> >  if pidfile_is_running $pid; then
>> >> > @@ -182,17 +188,32 @@ $cluster_remote_port
>> >> >
>> >> >  if test X"$use_remote_in_db" != Xno; then
>> >> >  set "$@" --remote=db:$schema_name,$table_name,connections
>> >> > +if test X"$create_insecure_remote" = Xno; then
>> >> > +set "$@" --remote=pssl:$port:$addr
>> >> > +elif test X"$create_insecure_remote" = Xyes; then
>> >> > +set "$@" --remote=ptcp:$port:$addr
>> >> > +fi
>> >> Why moving the logic here? This if block only says if the connection
settings in DB table should be used. Whether insecure mode is allowed was
supposed to be independent with this condition. Could you explain the
reason behind the change?
>> >> >> I moved it because remote=db is needed if ovsdb is running as a
standalone node or an active ovsdb server node. For standby nodes in case
of active_passive mode, remote=db will not be there because it uses
--sync-from. Hope its clear.
>>
>> As discussed, $use_remote_in_db and $create_insecure_remote were
independent. Moving $create_insecure_remote logic here make it useful only
when $use_remote_in_db is "yes", which is not how it was supposed to be.
>>
>> I understand that for standby node, we will set $use_remote_in_db as
"no". Is this a problem?
>>
> >> Just verified and have kept the logic intact as even for ssl, we have
connection table set for 0.0.0.0 for LB use case and it works fine. Hence,
have updated it in v2. Please take a look and let me know.
>>
>> --sync-from has nothing to do with "remote".
>> >
>> >
>> >>
>> >> >  fi
>> >> > -set "$@" --private-key=db:$schema_name,SSL,private_key
>> >> > -set "$@" --certificate=db:$schema_name,SSL,certificate
>> >> > -set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
>> >> > -set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
>> >> > -set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
>> >>
>> >> So it will not use the settings in DB any more? It seems this change
removed support for "--use-remote-in-db=yes", which is the default behavior
that should be kept. The DB settings should not be used only if
"--use-remote-in-db=no"
>> >
>> > >>  As discussed, this is similar support that we have for
ovn-controller with ssl. If the key is passed from cli, it will use the key
else fall back to default setters for ssl.
>> >>
>> I missed the "else" below. So it does fallback to use DB config when
command line 

Re: [ovs-dev] [PATCH v2] ovn-nbctl: Add basic port group commands.

[Ben Pfaff]

On Mon, Oct 08, 2018 at 04:23:53PM -0400, Mark Michelson wrote:
> This adds the following commands:
> 
> pg-add: Add a new port group, optionally adding switch ports at
> creation.
> pg-set-ports: Sets the logical switch ports on a port group
> pg-del: Remove a port group.
> 
> The main motivation for these commands is that it allows for adding
> logical switch ports by name rather than UUID.
> 
> Signed-off-by: Mark Michelson 
> ---
> v1 -> v2
> Moved tests to tests/ovn-nbctl.at. This revealed issues when listing
> database information in ovn-nbctl daemon mode. These issues are fixed in
> series https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636
> Therefore this patch is dependent on the linked series.

Besides Han's minor comment, would you mind updating the usage message
also?

Thanks,

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

Re: [ovs-dev] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[0-day Robot]

Bleep bloop.  Greetings aginwala aginwala, 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 amgin...@gmail.com  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: aginwala 
Lines checked: 129, Warnings: 1, Errors: 1


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

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

Re: [ovs-dev] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[0-day Robot]

Bleep bloop.  Greetings aginwala aginwala, 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 amgin...@gmail.com  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: aginwala 
Lines checked: 112, Warnings: 1, Errors: 1


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

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

Re: [ovs-dev] [PATCH 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[aginwala aginwala]

On Mon, Oct 8, 2018 at 10:47 AM Han Zhou  wrote:

>
>
> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala 
> wrote:
> >
> > Thanks for the review Han. Please find the comments inline below:
> > On Thu, Oct 4, 2018 at 9:58 AM Han Zhou  wrote:
> >>
> >> Thanks Ali, please see my comments below
> >>
> >> On Fri, Sep 21, 2018 at 5:34 PM  wrote:
> >> >
> >> >  For OVN DBs to work with SSL in HA, we need to have capability to
> >> >  pass ssl certs when starting OVN DBs. Say when starting OVN DBs in
> active
> >> >  passive mode, in order for the standby DBs to sync from master node,
> it
> >> >  cannot sync because the required ssl certs are not passed when
> standby DBs
> >> >  are initialized. Hence, we need to have this option.
> >> >
> >> > e.g. start nb db with ssl certs as below:
> >> > /usr/share/openvswitch/scripts/ovn-ctl
> --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
> >> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
> >> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
> >> > --db-nb-create-insecure-remote=no start_nb_ovsdb
> >> >
> >> > Certs can be generated based on ovs ssl docs:
> >> > http://docs.openvswitch.org/en/latest/howto/ssl/
> >> >
> >> > Signed-off-by: aginwala 
> >> > ---
> >> >  ovn/utilities/ovn-ctl | 50
> +++---
> >> >  1 file changed, 43 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> >> > index 3ff0df6..4f45f3d 100755
> >> > --- a/ovn/utilities/ovn-ctl
> >> > +++ b/ovn/utilities/ovn-ctl
> >> > @@ -116,6 +116,9 @@ start_ovsdb__() {
> >> >  local addr
> >> >  local active_conf_file
> >> >  local use_remote_in_db
> >> > +local ovn_db_ssl_key
> >> > +local ovn_db_ssl_cert
> >> > +local ovn_db_ssl_cacert
> >> >  eval pid=\$DB_${DB}_PID
> >> >  eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
> >> >  eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> >> > @@ -137,6 +140,9 @@ start_ovsdb__() {
> >> >  eval addr=\$DB_${DB}_ADDR
> >> >  eval active_conf_file=\$ovn${db}_active_conf_file
> >> >  eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
> >> > +eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
> >> > +eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
> >> > +eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
> >> >
> >> >  # Check and eventually start ovsdb-server for DB
> >> >  if pidfile_is_running $pid; then
> >> > @@ -182,17 +188,32 @@ $cluster_remote_port
> >> >
> >> >  if test X"$use_remote_in_db" != Xno; then
> >> >  set "$@" --remote=db:$schema_name,$table_name,connections
> >> > +if test X"$create_insecure_remote" = Xno; then
> >> > +set "$@" --remote=pssl:$port:$addr
> >> > +elif test X"$create_insecure_remote" = Xyes; then
> >> > +set "$@" --remote=ptcp:$port:$addr
> >> > +fi
> >> Why moving the logic here? This if block only says if the connection
> settings in DB table should be used. Whether insecure mode is allowed was
> supposed to be independent with this condition. Could you explain the
> reason behind the change?
> >> >> I moved it because remote=db is needed if ovsdb is running as a
> standalone node or an active ovsdb server node. For standby nodes in case
> of active_passive mode, remote=db will not be there because it uses
> --sync-from. Hope its clear.
>
> As discussed, $use_remote_in_db and $create_insecure_remote were
> independent. Moving $create_insecure_remote logic here make it useful only
> when $use_remote_in_db is "yes", which is not how it was supposed to be.
>
> I understand that for standby node, we will set $use_remote_in_db as "no".
> Is this a problem?
>
> >> Just verified and have kept the logic intact as even for ssl, we have
connection table set for 0.0.0.0 for LB use case and it works fine. Hence,
have updated it in v2. Please take a look and let me know.

> --sync-from has nothing to do with "remote".
> >
> >
> >>
> >> >  fi
> >> > -set "$@" --private-key=db:$schema_name,SSL,private_key
> >> > -set "$@" --certificate=db:$schema_name,SSL,certificate
> >> > -set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
> >> > -set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
> >> > -set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
> >>
> >> So it will not use the settings in DB any more? It seems this change
> removed support for "--use-remote-in-db=yes", which is the default behavior
> that should be kept. The DB settings should not be used only if
> "--use-remote-in-db=no"
> >
> > >>  As discussed, this is similar support that we have for
> ovn-controller with ssl. If the key is passed from cli, it will use the key
> else fall back to default setters for ssl.
> >>
> I missed the "else" below. So it does fallback to use DB config when
> command line option is not specified. It looks good for me, but it would be
> better to clarify the behavior in help message of 

Re: [ovs-dev] [PATCH 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[aginwala aginwala]

On Mon, Oct 8, 2018 at 2:17 PM Han Zhou  wrote:

>
>
> On Mon, Oct 8, 2018 at 11:55 AM aginwala aginwala 
> wrote:
> >
> > Yes, that's right.  I will send out v2 in a bit with Han's ack.
> >
> >
> > Regards,
> > Aliasgar
> >
> > On Mon, Oct 8, 2018 at 11:04 AM Ben Pfaff  wrote:
> >>
> >> On Mon, Oct 08, 2018 at 10:58:49AM -0700, Han Zhou wrote:
> >> > On Fri, Oct 5, 2018 at 6:34 PM aginwala aginwala 
> wrote:
> >> > >
> >> > > Thanks for the review Han. Please find the comments inline below:
> >> > >
> >> > > On Thu, Oct 4, 2018 at 10:16 AM Han Zhou  wrote:
> >> > >>
> >> > >> Thanks Ali, please see my comm
> >> > >>
> >> > >> On Fri, Sep 21, 2018 at 5:38 PM  wrote:
> >> > >> >
> >> > >> >  When starting OVN DBs in HA using pacemaker with ssl, we need
> to pass
> >> > ssl
> >> > >> >  certs for starting standby DBs. Hence, we need this change.
> >> > >> >
> >> > >> > Signed-off-by: aginwala 
> >> > >> > ---
> >> > >> >  ovn/utilities/ovndb-servers.ocf | 74
> >> > -
> >> > >> >  1 file changed, 73 insertions(+), 1 deletion(-)
> >> > >> >
> >> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf
> >> > b/ovn/utilities/ovndb-servers.ocf
> >> > >> > index 52141c7..80f81ae 100755
> >> > >> > --- a/ovn/utilities/ovndb-servers.ocf
> >> > >> > +++ b/ovn/utilities/ovndb-servers.ocf
> >> > >> > @@ -10,6 +10,12 @@
> >> > >> >  : ${MANAGE_NORTHD_DEFAULT="no"}
> >> > >> >  : ${INACTIVE_PROBE_DEFAULT="5000"}
> >> > >> >  : ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"}
> >> > >> > +: ${NB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnnb-privkey.pem"}
> >> > >> > +: ${NB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnnb-cert.pem"}
> >> > >> > +: ${NB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
> >> > >> > +: ${SB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnsb-privkey.pem"}
> >> > >> > +: ${SB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnsb-cert.pem"}
> >> > >> > +: ${SB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
> >> > >> >
> >> > >> >  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
> >> > >> >  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type
> crm_config
> >> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server"
> >> > >> > @@ -21,6 +27,13 @@
> >> > SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
> >> > >> >
> >> >
>  SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
> >> > >> >
>  MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
> >> > >> >
> >> >
>  
> INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}}
> >> > >> >
> +NB_PRIVKEY=${OCF_RESKEY_ovn_nb_db_privkey:-${NB_SSL_KEY_DEFAULT}}
> >> > >> > +NB_CERT=${OCF_RESKEY_ovn_nb_db_cert:-${NB_SSL_CERT_DEFAULT}}
> >> > >> >
> +NB_CACERT=${OCF_RESKEY_ovn_nb_db_cacert:-${NB_SSL_CACERT_DEFAULT}}
> >> > >> >
> +SB_PRIVKEY=${OCF_RESKEY_ovn_sb_db_privkey:-${SB_SSL_KEY_DEFAULT}}
> >> > >> > +SB_CERT=${OCF_RESKEY_ovn_sb_db_cert:-${SB_SSL_CERT_DEFAULT}}
> >> > >> >
> +SB_CACERT=${OCF_RESKEY_ovn_sb_db_cacert:-${SB_SSL_CACERT_DEFAULT}}
> >> > >> > +
> >> > >> >
> >> > >> >  # In order for pacemaker to work with LB, we can set
> >> > LISTEN_ON_MASTER_IP_ONLY
> >> > >> >  # to false and pass LB vip IP while creating pcs resource.
> >> > >> > @@ -132,6 +145,54 @@ ovsdb_server_metadata() {
> >> > >> >
> >> > >> >
> >> > >> >
> >> > >> > +  
> >> > >> > +  
> >> > >> > +  OVN NB DB private key absolute path for ssl setup.
> >> > >> > +  
> >> > >> > +  OVN NB DB private key file
> >> > >> > +  
> >> > >> > +  
> >> > >> > +
> >> > >> > +  
> >> > >> > +  
> >> > >> > +  OVN NB DB certificate absolute path for ssl setup.
> >> > >> > +  
> >> > >> > +  OVN NB DB cert file
> >> > >> > +  
> >> > >> > +  
> >> > >> > +
> >> > >> > +  
> >> > >> > +  
> >> > >> > +  OVN NB DB CA certificate absolute path for ssl setup.
> >> > >> > +  
> >> > >> > +  OVN NB DB cacert file
> >> > >> > +  
> >> > >> > +  
> >> > >> > +
> >> > >> > +  
> >> > >> > +  
> >> > >> > +  OVN SB DB private key absolute path for ssl setup.
> >> > >> > +  
> >> > >> > +  OVN SB DB private key file
> >> > >> > +  
> >> > >> > +  
> >> > >> > +
> >> > >> > +  
> >> > >> > +  
> >> > >> > +  OVN SB DB certificate absolute path for ssl setup.
> >> > >> > +  
> >> > >> > +  OVN SB DB cert file
> >> > >> > +  
> >> > >> > +  
> >> > >> > +
> >> > >> > +  
> >> > >> > +  
> >> > >> > +  OVN SB DB CA certificate absolute path for ssl setup.
> >> > >> > +  
> >> > >> > +  OVN SB DB cacert file
> >> > >> > +  
> >> > >> > +  
> >> > >> > +
> >> > >> >
> >> > >> >
> >> > >> >
> >> > >> > @@ -326,6 +387,18 @@ ovsdb_server_start() {
> >> > >> > set $@ --db-sb-addr=${MASTER_IP}
> --db-sb-port=${SB_MASTER_PORT}
> >> > >> >  fi
> >> > >> >
> >> > >> > +if [ "x${NB_MASTER_PROTO}" = xssl ]; then
> >> > >> > +set $@ --db-nb-create-insecure-remote=no
> >> > >> "no" is the default value, so this line is not needed.
> >> > >
> >> > > >> Sure. This makes sense. Will check out the 

[ovs-dev] [PATCH v2 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[amginwal]

When starting OVN DBs in HA using pacemaker with ssl, we need to pass ssl
certs for starting standby DBs. Hence, we need this change.

Signed-off-by: aginwala 
Acked-by: Han Zhou 
---
 ovn/utilities/ovndb-servers.ocf | 72 -
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf
index 52141c7..1031330 100755
--- a/ovn/utilities/ovndb-servers.ocf
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -10,6 +10,12 @@
 : ${MANAGE_NORTHD_DEFAULT="no"}
 : ${INACTIVE_PROBE_DEFAULT="5000"}
 : ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"}
+: ${NB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnnb-privkey.pem"}
+: ${NB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnnb-cert.pem"}
+: ${NB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
+: ${SB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnsb-privkey.pem"}
+: ${SB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnsb-cert.pem"}
+: ${SB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
 
 CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
 CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config --name 
OVN_REPL_INFO -s ovn_ovsdb_master_server"
@@ -21,6 +27,13 @@ 
SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
 SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
 MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
 INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}}
+NB_PRIVKEY=${OCF_RESKEY_ovn_nb_db_privkey:-${NB_SSL_KEY_DEFAULT}}
+NB_CERT=${OCF_RESKEY_ovn_nb_db_cert:-${NB_SSL_CERT_DEFAULT}}
+NB_CACERT=${OCF_RESKEY_ovn_nb_db_cacert:-${NB_SSL_CACERT_DEFAULT}}
+SB_PRIVKEY=${OCF_RESKEY_ovn_sb_db_privkey:-${SB_SSL_KEY_DEFAULT}}
+SB_CERT=${OCF_RESKEY_ovn_sb_db_cert:-${SB_SSL_CERT_DEFAULT}}
+SB_CACERT=${OCF_RESKEY_ovn_sb_db_cacert:-${SB_SSL_CACERT_DEFAULT}}
+
 
 # In order for pacemaker to work with LB, we can set LISTEN_ON_MASTER_IP_ONLY
 # to false and pass LB vip IP while creating pcs resource.
@@ -132,6 +145,54 @@ ovsdb_server_metadata() {
   
   
 
+  
+  
+  OVN NB DB private key absolute path for ssl setup.
+  
+  OVN NB DB private key file
+  
+  
+
+  
+  
+  OVN NB DB certificate absolute path for ssl setup.
+  
+  OVN NB DB cert file
+  
+  
+
+  
+  
+  OVN NB DB CA certificate absolute path for ssl setup.
+  
+  OVN NB DB cacert file
+  
+  
+
+  
+  
+  OVN SB DB private key absolute path for ssl setup.
+  
+  OVN SB DB private key file
+  
+  
+
+  
+  
+  OVN SB DB certificate absolute path for ssl setup.
+  
+  OVN SB DB cert file
+  
+  
+
+  
+  
+  OVN SB DB CA certificate absolute path for ssl setup.
+  
+  OVN SB DB cacert file
+  
+  
+
   
 
   
@@ -326,6 +387,16 @@ ovsdb_server_start() {
set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT}
 fi
 
+if [ "x${NB_MASTER_PROTO}" = xssl ]; then
+set $@ --ovn-nb-db-ssl-key=${NB_PRIVKEY}
+set $@ --ovn-nb-db-ssl-cert=${NB_CERT}
+set $@ --ovn-nb-db-ssl-ca-cert=${NB_CACERT}
+fi
+if [ "x${SB_MASTER_PROTO}" = xssl ]; then
+set $@ --ovn-sb-db-ssl-key=${SB_PRIVKEY}
+set $@ --ovn-sb-db-ssl-cert=${SB_CERT}
+set $@ --ovn-sb-db-ssl-ca-cert=${SB_CACERT}
+fi
 if [ "x${present_master}" = x ]; then
 # No master detected, or the previous master is not among the
 # set starting.
@@ -343,7 +414,6 @@ ovsdb_server_start() {
 set $@ --db-nb-sync-from-addr=${INVALID_IP_ADDRESS} 
--db-sb-sync-from-addr=${INVALID_IP_ADDRESS}
 
 elif [ ${present_master} != ${host_name} ]; then
-# TODO: for using LB vip, need to test for ssl.
 if [ "x${LISTEN_ON_MASTER_IP_ONLY}" = xyes ]; then
 if [ "x${NB_MASTER_PROTO}" = xtcp ]; then
 set $@ --db-nb-create-insecure-remote=yes
-- 
1.9.1

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

[ovs-dev] [PATCH v2 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[amginwal]

For OVN DBs to work with SSL in HA, we need to have capability to pass ssl
certs when starting OVN DBs. Say when starting OVN DBs in active passive mode,
in order for the standby DBs to sync from master node, it cannot sync
because the required ssl certs are not passed when standby DBs are initialized.
Hence, we need to have this option.

e.g. start nb db with ssl certs as below:
/usr/share/openvswitch/scripts/ovn-ctl 
--ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
--ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
--ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
--db-nb-create-insecure-remote=no start_nb_ovsdb

When certs are passed in the command line, it will read certs from the path
mentioned instead of default db configs.

Certs can be generated based on ovs ssl docs:
http://docs.openvswitch.org/en/latest/howto/ssl/

Signed-off-by: aginwala 
---
 ovn/utilities/ovn-ctl | 41 ++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 3ff0df6..d71071a 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -116,6 +116,9 @@ start_ovsdb__() {
 local addr
 local active_conf_file
 local use_remote_in_db
+local ovn_db_ssl_key
+local ovn_db_ssl_cert
+local ovn_db_ssl_cacert
 eval pid=\$DB_${DB}_PID
 eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
 eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
@@ -137,6 +140,9 @@ start_ovsdb__() {
 eval addr=\$DB_${DB}_ADDR
 eval active_conf_file=\$ovn${db}_active_conf_file
 eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
+eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
+eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
+eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
 
 # Check and eventually start ovsdb-server for DB
 if pidfile_is_running $pid; then
@@ -183,9 +189,23 @@ $cluster_remote_port
 if test X"$use_remote_in_db" != Xno; then
 set "$@" --remote=db:$schema_name,$table_name,connections
 fi
-set "$@" --private-key=db:$schema_name,SSL,private_key
-set "$@" --certificate=db:$schema_name,SSL,certificate
-set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
+
+if test X"$ovn_db_ssl_key" != X; then
+set "$@" --private-key=$ovn_db_ssl_key
+else
+set "$@" --private-key=db:$schema_name,SSL,private_key
+fi
+if test X"$ovn_db_ssl_cert" != X; then
+set "$@" --certificate=$ovn_db_ssl_cert
+else
+set "$@" --certificate=db:$schema_name,SSL,certificate
+fi
+if test X"$ovn_db_ssl_cacert" != X; then
+set "$@" --ca-cert=$ovn_db_ssl_cacert
+else
+set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
+fi
+
 set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
 set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
 
@@ -481,6 +501,15 @@ set_defaults () {
 OVN_NORTHD_SB_DB="unix:$DB_SB_SOCK"
 DB_NB_USE_REMOTE_IN_DB="yes"
 DB_SB_USE_REMOTE_IN_DB="yes"
+
+OVN_NB_DB_SSL_KEY=""
+OVN_NB_DB_SSL_CERT=""
+OVN_NB_DB_SSL_CA_CERT=""
+
+OVN_SB_DB_SSL_KEY=""
+OVN_SB_DB_SSL_CERT=""
+OVN_SB_DB_SSL_CA_CERT=""
+
 }
 
 set_option () {
@@ -536,6 +565,12 @@ Options:
   --ovn-controller-ssl-cert=CERT OVN Southbound SSL certificate file
   --ovn-controller-ssl-ca-cert=CERT OVN Southbound SSL CA certificate file
   --ovn-controller-ssl-bootstrap-ca-cert=CERT Bootstrapped OVN Southbound SSL 
CA certificate file
+  --ovn-nb-db-ssl-key=KEY OVN Northbound DB SSL private key file
+  --ovn-nb-db-ssl-cert=CERT OVN Northbound DB SSL certificate file
+  --ovn-nb-db-ssl-ca-cert=CERT OVN Northbound DB SSL CA certificate file
+  --ovn-sb-db-ssl-key=KEY OVN Southbound DB SSL private key file
+  --ovn-sb-db-ssl-cert=CERT OVN Southbound DB SSL certificate file
+  --ovn-sb-db-ssl-ca-cert=CERT OVN Southbound DB SSL CA certificate file
   --ovn-manage-ovsdb=yes|noWhether or not the OVN databases should be
automatically started and stopped along
with ovn-northd. The default is "yes". If
-- 
1.9.1

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

[ovs-dev] [RFC PATCH v1 3/3] windows: Allow add/delete ports via HNS API

[Alin Gabriel Serdean]

On Windows 2016 LTSC(RTM) the Container feature and Hyper-V feature both had
DCOM API to add and delete internal (management) ports on the Hyper-V switch.

Starting from the 1703 release and above, enabling only the Container feature
does not fulfil this requirement anymore.

We need new ways to interact with the host API without the Hyper-V integration.

Unfortunately, there is no C API for it, only golang and .net:
https://github.com/Microsoft/hcsshim (golang)
https://github.com/microsoft/dotnet-computevirtualization (.net)
It is also poorly documented and reserved.

Although not pretty and less performant, this will provide a way to
add/delete and query the new APIs across different versions of Windows,
until new API's or documentation are available.

Signed-off-by: Alin Gabriel Serdean 
---
 lib/wmi.c | 95 ---
 1 file changed, 91 insertions(+), 4 deletions(-)

diff --git a/lib/wmi.c b/lib/wmi.c
index e6dc63cde..14451d591 100644
--- a/lib/wmi.c
+++ b/lib/wmi.c
@@ -203,6 +203,93 @@ wait_for_job(IWbemServices *psvc, wchar_t *job_path)
 return retval;
 }
 
+static boolean
+hns_delete(char *name)
+{
+VLOG_DBG("Trying to delete port via HNS API");
+char buffer[1];
+int count;
+count = snprintf(buffer, 1, "-NoLogo -NoProfile -NonInteractive"
+ "-Command \"try {Delete-OVSHNSInternalPort '%s'}"
+ "catch { exit 1 }; exit 0; \"", name);
+if (count < 0 || count > 1) {
+VLOG_WARN("Could not allocate memory for powershell delete command");
+return false;
+}
+VLOG_DBG("command = %s", buffer);
+DWORD res = 0;
+SHELLEXECUTEINFOA ShExecInfo;
+ShExecInfo.cbSize = sizeof(SHELLEXECUTEINFO);
+ShExecInfo.fMask = SEE_MASK_NOCLOSEPROCESS;
+ShExecInfo.hwnd = NULL;
+ShExecInfo.lpVerb = NULL;
+ShExecInfo.lpFile = "powershell.exe";
+ShExecInfo.lpParameters = buffer;
+ShExecInfo.lpDirectory = NULL;
+ShExecInfo.nShow = SW_HIDE;
+ShExecInfo.hInstApp = NULL;
+ShellExecuteExA();
+WaitForSingleObject(ShExecInfo.hProcess, 5);
+if (GetExitCodeProcess(ShExecInfo.hProcess, )) {
+if (res != 0) {
+VLOG_ERR("Powershell delete command failed with exit code: %d",
+ res);
+CloseHandle(ShExecInfo.hProcess);
+return false;
+}
+} else {
+VLOG_ERR("Failed to get exit code for powershell delete command."
+ "Last Error: %s", ovs_lasterror_to_string());
+CloseHandle(ShExecInfo.hProcess);
+return false;
+}
+CloseHandle(ShExecInfo.hProcess);
+return true;
+}
+
+static boolean
+hns_add(char *name)
+{
+VLOG_WARN("Trying to add port via HNS API");
+char buffer[1];
+int count;
+count = snprintf(buffer, 1, "-NoLogo -NoProfile -NonInteractive"
+ "-Command \"try {Add-OVSHNSInternalPort '%s'}"
+ "catch { exit 1 }; exit 0; \"", name);
+if (count < 0 || count > 1) {
+VLOG_WARN("Could not allocate memory for powershell add command");
+return false;
+}
+VLOG_WARN("command = %s", buffer);
+DWORD res = 0;
+SHELLEXECUTEINFOA ShExecInfo;
+ShExecInfo.cbSize = sizeof(SHELLEXECUTEINFO);
+ShExecInfo.fMask = SEE_MASK_NOCLOSEPROCESS;
+ShExecInfo.hwnd = NULL;
+ShExecInfo.lpVerb = NULL;
+ShExecInfo.lpFile = "powershell.exe";
+ShExecInfo.lpParameters = buffer;
+ShExecInfo.lpDirectory = NULL;
+ShExecInfo.nShow = SW_HIDE;
+ShExecInfo.hInstApp = NULL;
+ShellExecuteExA();
+WaitForSingleObject(ShExecInfo.hProcess, 5);
+if (GetExitCodeProcess(ShExecInfo.hProcess, )) {
+if (res != 0) {
+VLOG_ERR("Powershell add command failed with exit code: %d", res);
+CloseHandle(ShExecInfo.hProcess);
+return false;
+}
+} else {
+VLOG_ERR("Failed to get exit code for powershell add command."
+ "Last Error: %s", ovs_lasterror_to_string());
+CloseHandle(ShExecInfo.hProcess);
+return false;
+}
+CloseHandle(ShExecInfo.hProcess);
+return true;
+}
+
 /* This function will initialize DCOM retrieving the WMI locator's ploc and
  * the context associated to it. */
 static boolean
@@ -390,14 +477,14 @@ delete_wmi_port(char *name)
 
 if (!initialize_wmi(, )) {
 VLOG_WARN("Could not initialize DCOM");
-retval = false;
+retval = hns_delete(name);
 goto error;
 }
 
 if (!connect_set_security(ploc, pcontext, L"Root\\Virtualization\\v2",
   )) {
 VLOG_WARN("Could not connect and set security for virtualization");
-retval = false;
+retval = hns_delete(name);
 goto error;
 }
 
@@ -673,14 +760,14 @@ create_wmi_port(char *name) {
 
 if (!initialize_wmi(, )) {
 VLOG_WARN("Could not initialize DCOM");
-  

[ovs-dev] [RFC PATCH v1 2/3] windows, installer: Add a new module file to the installer

[Alin Gabriel Serdean]

This patch adds the new powershell module HNSHelper.psm1 to the OVS windows
installer.


Signed-off-by: Alin Gabriel Serdean 
---
 windows/automake.mk   | 1 +
 windows/ovs-windows-installer/Product.wxs | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/windows/automake.mk b/windows/automake.mk
index 80dca1467..79ffe4317 100644
--- a/windows/automake.mk
+++ b/windows/automake.mk
@@ -16,6 +16,7 @@ PTHREAD_TEMP_DIR=`echo "$(PTHREAD_LDFLAGS)" | sed 
's|^.\(.*\).$:\1||'`
 windows_installer: all
 #Userspace files needed for the installer
cp -f $(top_srcdir)/datapath-windows/misc/OVS.psm1 
windows/ovs-windows-installer/Services/OVS.psm1
+   cp -f $(top_srcdir)/datapath-windows/misc/HNSHelper.psm1 
windows/ovs-windows-installer/Services/HNSHelper.psm1
cp -f $(top_srcdir)/vswitchd/vswitch.ovsschema 
windows/ovs-windows-installer/Services/vswitch.ovsschema
cp -f $(top_srcdir)/vswitchd/ovs-vswitchd.exe 
windows/ovs-windows-installer/Services/ovs-vswitchd.exe
cp -f $(top_srcdir)/ovsdb/ovsdb-server.exe 
windows/ovs-windows-installer/Services/ovsdb-server.exe
diff --git a/windows/ovs-windows-installer/Product.wxs 
b/windows/ovs-windows-installer/Product.wxs
index ea1bc6896..9742c1773 100644
--- a/windows/ovs-windows-installer/Product.wxs
+++ b/windows/ovs-windows-installer/Product.wxs
@@ -67,6 +67,7 @@
   
   
   
+  
 
 
 
@@ -147,6 +148,7 @@
   
 
   
+  
 
   
 
@@ -156,6 +158,10 @@
 
   
 
+
+
+  
+
   
 
   
-- 
2.16.1.windows.1

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

[ovs-dev] [RFC PATCH v1 1/3] datapath-windows: Introduce HNS OVS calls

[Alin Gabriel Serdean]

We introduce a new powershell module for interacting with HNS (host network 
service):
https://github.com/Microsoft/SDN/blob/master/Kubernetes/windows/hns.psm1

In our repository this shall be named HNSHelper.psm1.

We also add five new commandlets in our OVS powershell module:
Disable-OVSOnHNSNetwork - disable OVS extension on a given HNS network.
Enable-OVSOnHNSNetwork - enable OVS extension on a given HNS network.
Get-OVSEnabledHNSNetworks - return the HNS network on which OVS is enabled.
Add-OVSHNSInternalPort - will add a new internal port with he name: `name`, on
 the HNS network, which has OVS enabled. This port
 shall use the host compartment.
Delete-OVSHNSInternalPort - will delete an internal port with name: `name` on a
HNS network which has OVS enabled.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/automake.mk |   1 +
 datapath-windows/misc/HNSHelper.psm1 | 499 +++
 datapath-windows/misc/OVS.psm1   |  79 +-
 3 files changed, 578 insertions(+), 1 deletion(-)
 create mode 100644 datapath-windows/misc/HNSHelper.psm1

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 3820041f6..d98ab3c60 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -3,6 +3,7 @@ EXTRA_DIST += \
datapath-windows/Package/package.VcxProj.user \
datapath-windows/include/OvsDpInterfaceExt.h \
datapath-windows/include/OvsDpInterfaceCtExt.h \
+   datapath-windows/misc/HNSHelper.psm1 \
datapath-windows/misc/OVS.psm1 \
datapath-windows/misc/install.cmd \
datapath-windows/misc/uninstall.cmd \
diff --git a/datapath-windows/misc/HNSHelper.psm1 
b/datapath-windows/misc/HNSHelper.psm1
new file mode 100644
index 0..ac99889cf
--- /dev/null
+++ b/datapath-windows/misc/HNSHelper.psm1
@@ -0,0 +1,499 @@
+# SDN Sample Scripts  v.1.0
+#
+# Copyright (c) Microsoft Corporation
+#
+# All rights reserved. 
+#
+# MIT License
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the ""Software""), to
+# deal in the Software without restriction, including without limitation the
+# rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+# sell copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED *AS IS*, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+#
+#
+# Global Initialize
+function Get-VmComputeNativeMethods()
+{
+$signature = @'
+ [DllImport("vmcompute.dll")]
+ public static extern void 
HNSCall([MarshalAs(UnmanagedType.LPWStr)] string method, 
[MarshalAs(UnmanagedType.LPWStr)] string path, 
[MarshalAs(UnmanagedType.LPWStr)] string request, 
[MarshalAs(UnmanagedType.LPWStr)] out string response);
+'@
+
+# Compile into runtime type
+try { return [VmCompute.PrivatePInvoke.NativeMethods] }
+catch { return (Add-Type -MemberDefinition $signature -Namespace 
VmCompute.PrivatePInvoke -Name NativeMethods -PassThru) }
+}
+
+#
+# Configuration
+#
+function Get-HnsSwitchExtensions
+{
+param
+(
+[parameter(Mandatory=$true)] [string] $NetworkId
+)
+
+return (Get-HNSNetwork $NetworkId).Extensions
+}
+
+function Set-HnsSwitchExtension
+{
+param
+(
+[parameter(Mandatory=$true)] [string] $NetworkId,
+[parameter(Mandatory=$true)] [string] $ExtensionId,
+[parameter(Mandatory=$true)] [bool]   $state
+)
+
+# { "Extensions": [ { "Id": "...", "IsEnabled": true|false } ] }
+$req = @{
+"Extensions"=@(@{
+"Id"=$ExtensionId;
+"IsEnabled"=$state;
+};)
+}
+Invoke-HNSRequest -Method POST -Type networks -Id $NetworkId -Data 
(ConvertTo-Json $req)
+}
+
+#
+# Activities
+#
+function Get-HNSActivities
+{
+[cmdletbinding()]Param()
+return Invoke-HNSRequest -Type 

Re: [ovs-dev] [PATCH v2] ovn-nbctl: Add basic port group commands.

[Han Zhou]

On Mon, Oct 8, 2018 at 1:24 PM Mark Michelson  wrote:
>
> This adds the following commands:
>
> pg-add: Add a new port group, optionally adding switch ports at
> creation.
> pg-set-ports: Sets the logical switch ports on a port group
> pg-del: Remove a port group.
>
> The main motivation for these commands is that it allows for adding
> logical switch ports by name rather than UUID.
>
> Signed-off-by: Mark Michelson 
> ---
> v1 -> v2
> Moved tests to tests/ovn-nbctl.at. This revealed issues when listing
> database information in ovn-nbctl daemon mode. These issues are fixed in
> series https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636
> Therefore this patch is dependent on the linked series.
> ---
>  ovn/utilities/ovn-nbctl.8.xml | 22 +
>  ovn/utilities/ovn-nbctl.c | 72
+++
>  tests/ovn-nbctl.at| 34 +++-
>  tests/ovn.at  |  1 +
>  4 files changed, 128 insertions(+), 1 deletion(-)
>
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index 5e18fa7c0..14cc02f53 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -881,6 +881,28 @@
>
>  
>
> +Port Group commands
> +
> +
> +  pg-add group [port]...
> +  
> +Creates a new port group in the Port_Group table
named
> +group with optional ports added to the
group.
> +  
> +
> +  pg-set-ports group
port...
> +  
> +Sets ports on the port group named
group. It
> +is an error if group does not exist.
> +  
> +
> +  pg-del group
> +  
> +Deletes port group group. It is an error if
> +group does not exist.
> +  
> +
> +
>  Database Commands
>  These commands query and modify the contents of
ovsdb tables.
>  They are a slight abstraction of the ovsdb interface and
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index bff6d1380..89f0d6213 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -4655,6 +4655,73 @@ cmd_set_ssl(struct ctl_context *ctx)
>  nbrec_nb_global_set_ssl(nb_global, ssl);
>  }
>
> +static char *
> +set_ports_on_pg(struct ctl_context *ctx, const struct nbrec_port_group
*pg,
> +char **new_ports, size_t num_new_ports)
> +{
> +struct nbrec_logical_switch_port **lports;
> +lports = xmalloc(sizeof *lports * num_new_ports);
> +
> +size_t i;
> +char *error = NULL;
> +for (i = 0; i < num_new_ports; i++) {
> +const struct nbrec_logical_switch_port *lsp;
> +error = lsp_by_name_or_uuid(ctx, new_ports[i], true, );
> +if (error) {
> +goto out;
> +}
> +lports[i] = (struct nbrec_logical_switch_port *) lsp;
> +}
> +
> +nbrec_port_group_set_ports(pg, lports, num_new_ports);
> +
> +out:
> +free(lports);
> +return error;
> +}
> +
> +static void
> +cmd_pg_add(struct ctl_context *ctx)
> +{
> +const struct nbrec_port_group *pg;
> +
> +pg = nbrec_port_group_insert(ctx->txn);
> +nbrec_port_group_set_name(pg, ctx->argv[1]);
> +if (ctx->argc > 2) {
> +ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc -
2);
> +}
> +}
> +
> +static void
> +cmd_pg_set_ports(struct ctl_context *ctx)
> +{
> +const struct nbrec_port_group *pg;
> +
> +char *error;
> +error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, );
> +if (error) {
> +ctx->error = error;
> +return;
> +}
> +
> +ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2);
> +}
> +
> +static void
> +cmd_pg_del(struct ctl_context *ctx)
> +{
> +const struct nbrec_port_group *pg;
> +
> +char *error;
> +error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, );
> +if (error) {
> +ctx->error = error;
> +return;
> +}
> +
> +nbrec_port_group_delete(pg);
> +}
> +
>  static const struct ctl_table_class tables[NBREC_N_TABLES] = {
>  [NBREC_TABLE_DHCP_OPTIONS].row_ids
>  = {{_logical_switch_port_col_name, NULL,
> @@ -5123,6 +5190,11 @@ static const struct ctl_command_syntax
nbctl_commands[] = {
>  "PRIVATE-KEY CERTIFICATE CA-CERT [SSL-PROTOS [SSL-CIPHERS]]",
>  pre_cmd_set_ssl, cmd_set_ssl, NULL, "--bootstrap", RW},
>
> +/* Port Group Commands */
> +{"pg-add", 1, INT_MAX, "", NULL, cmd_pg_add, NULL, "", RW },
> +{"pg-set-ports", 2, INT_MAX, "", NULL, cmd_pg_set_ports, NULL, "",
RW },
> +{"pg-del", 1, 1, "", NULL, cmd_pg_del, NULL, "", RW },
> +
>  {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO},
>  };
>
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 3f89874ba..25414b8bd 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -1518,7 +1518,6 @@ AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0],
[dnl
>  "pg0"
>  ])])
>
> -
>  OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
>  dnl This test 

Re: [ovs-dev] [PATCH 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[Han Zhou]

On Mon, Oct 8, 2018 at 11:55 AM aginwala aginwala 
wrote:
>
> Yes, that's right.  I will send out v2 in a bit with Han's ack.
>
>
> Regards,
> Aliasgar
>
> On Mon, Oct 8, 2018 at 11:04 AM Ben Pfaff  wrote:
>>
>> On Mon, Oct 08, 2018 at 10:58:49AM -0700, Han Zhou wrote:
>> > On Fri, Oct 5, 2018 at 6:34 PM aginwala aginwala 
wrote:
>> > >
>> > > Thanks for the review Han. Please find the comments inline below:
>> > >
>> > > On Thu, Oct 4, 2018 at 10:16 AM Han Zhou  wrote:
>> > >>
>> > >> Thanks Ali, please see my comm
>> > >>
>> > >> On Fri, Sep 21, 2018 at 5:38 PM  wrote:
>> > >> >
>> > >> >  When starting OVN DBs in HA using pacemaker with ssl, we need to
pass
>> > ssl
>> > >> >  certs for starting standby DBs. Hence, we need this change.
>> > >> >
>> > >> > Signed-off-by: aginwala 
>> > >> > ---
>> > >> >  ovn/utilities/ovndb-servers.ocf | 74
>> > -
>> > >> >  1 file changed, 73 insertions(+), 1 deletion(-)
>> > >> >
>> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf
>> > b/ovn/utilities/ovndb-servers.ocf
>> > >> > index 52141c7..80f81ae 100755
>> > >> > --- a/ovn/utilities/ovndb-servers.ocf
>> > >> > +++ b/ovn/utilities/ovndb-servers.ocf
>> > >> > @@ -10,6 +10,12 @@
>> > >> >  : ${MANAGE_NORTHD_DEFAULT="no"}
>> > >> >  : ${INACTIVE_PROBE_DEFAULT="5000"}
>> > >> >  : ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"}
>> > >> > +: ${NB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnnb-privkey.pem"}
>> > >> > +: ${NB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnnb-cert.pem"}
>> > >> > +: ${NB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
>> > >> > +: ${SB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnsb-privkey.pem"}
>> > >> > +: ${SB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnsb-cert.pem"}
>> > >> > +: ${SB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
>> > >> >
>> > >> >  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
>> > >> >  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type
crm_config
>> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server"
>> > >> > @@ -21,6 +27,13 @@
>> > SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
>> > >> >
>> >
 SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
>> > >> >
 MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
>> > >> >
>> >
 INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}}
>> > >> > +NB_PRIVKEY=${OCF_RESKEY_ovn_nb_db_privkey:-${NB_SSL_KEY_DEFAULT}}
>> > >> > +NB_CERT=${OCF_RESKEY_ovn_nb_db_cert:-${NB_SSL_CERT_DEFAULT}}
>> > >> >
+NB_CACERT=${OCF_RESKEY_ovn_nb_db_cacert:-${NB_SSL_CACERT_DEFAULT}}
>> > >> > +SB_PRIVKEY=${OCF_RESKEY_ovn_sb_db_privkey:-${SB_SSL_KEY_DEFAULT}}
>> > >> > +SB_CERT=${OCF_RESKEY_ovn_sb_db_cert:-${SB_SSL_CERT_DEFAULT}}
>> > >> >
+SB_CACERT=${OCF_RESKEY_ovn_sb_db_cacert:-${SB_SSL_CACERT_DEFAULT}}
>> > >> > +
>> > >> >
>> > >> >  # In order for pacemaker to work with LB, we can set
>> > LISTEN_ON_MASTER_IP_ONLY
>> > >> >  # to false and pass LB vip IP while creating pcs resource.
>> > >> > @@ -132,6 +145,54 @@ ovsdb_server_metadata() {
>> > >> >
>> > >> >
>> > >> >
>> > >> > +  
>> > >> > +  
>> > >> > +  OVN NB DB private key absolute path for ssl setup.
>> > >> > +  
>> > >> > +  OVN NB DB private key file
>> > >> > +  
>> > >> > +  
>> > >> > +
>> > >> > +  
>> > >> > +  
>> > >> > +  OVN NB DB certificate absolute path for ssl setup.
>> > >> > +  
>> > >> > +  OVN NB DB cert file
>> > >> > +  
>> > >> > +  
>> > >> > +
>> > >> > +  
>> > >> > +  
>> > >> > +  OVN NB DB CA certificate absolute path for ssl setup.
>> > >> > +  
>> > >> > +  OVN NB DB cacert file
>> > >> > +  
>> > >> > +  
>> > >> > +
>> > >> > +  
>> > >> > +  
>> > >> > +  OVN SB DB private key absolute path for ssl setup.
>> > >> > +  
>> > >> > +  OVN SB DB private key file
>> > >> > +  
>> > >> > +  
>> > >> > +
>> > >> > +  
>> > >> > +  
>> > >> > +  OVN SB DB certificate absolute path for ssl setup.
>> > >> > +  
>> > >> > +  OVN SB DB cert file
>> > >> > +  
>> > >> > +  
>> > >> > +
>> > >> > +  
>> > >> > +  
>> > >> > +  OVN SB DB CA certificate absolute path for ssl setup.
>> > >> > +  
>> > >> > +  OVN SB DB cacert file
>> > >> > +  
>> > >> > +  
>> > >> > +
>> > >> >
>> > >> >
>> > >> >
>> > >> > @@ -326,6 +387,18 @@ ovsdb_server_start() {
>> > >> > set $@ --db-sb-addr=${MASTER_IP}
--db-sb-port=${SB_MASTER_PORT}
>> > >> >  fi
>> > >> >
>> > >> > +if [ "x${NB_MASTER_PROTO}" = xssl ]; then
>> > >> > +set $@ --db-nb-create-insecure-remote=no
>> > >> "no" is the default value, so this line is not needed.
>> > >
>> > > >> Sure. This makes sense. Will check out the default behavior and
update
>> > it the revised patch!
>> > >>
>> > >>
>> > >> > +set $@ --ovn-nb-db-ssl-key=${NB_PRIVKEY}
>> > >> > +set $@ --ovn-nb-db-ssl-cert=${NB_CERT}
>> > >> > +set $@ --ovn-nb-db-ssl-ca-cert=${NB_CACERT}
>> > >> This should be needed only for standby which sets
>> > 

Re: [ovs-dev] Issue with OVS lacp-fallback-ab option

[Arun Navasivasakthivelsamy]

Thanks Ben. Let me try it out this week and report back.

On 10/8/18, 1:15 PM, "Ben Pfaff"  wrote:

>I think I see a problem in the implementation of bonding when
>recirculation is available.  Are you able to try out a patch?  If so,
>try the following.  It is not a good way to solve the issue, but it
>should illustrate whether recirculation is the problem.
>
>diff --git a/ofproto/bond.c b/ofproto/bond.c
>index f87cdba7908f..bb6a80411de5 100644
>--- a/ofproto/bond.c
>+++ b/ofproto/bond.c
>@@ -927,7 +928,7 @@ bond_recirculation_account(struct bond *bond)
> static bool
> bond_may_recirc(const struct bond *bond)
> {
>-return bond->balance == BM_TCP && bond->recirc_id;
>+return bond->balance == BM_TCP && bond->recirc_id && false;
> }
> 
> static void
>
>
>On Fri, Oct 05, 2018 at 04:45:21AM +, Arun Navasivasakthivelsamy
>wrote:
>> Also, ofproto/trace is suggesting that the packet will be hashed to a
>>slave (instead of just to the active port) with lacp-fallback-ab option.
>> 
>> This is with active-backup bond mode:
>> 
>> 
>> [root@frankfurter02-4 ~]# ovs-appctl ofproto/trace br0
>>in_port=6,dl_dst=28:99:3a:08:7a:cf
>> 
>> Bridge: br0
>> 
>> Flow: 
>>in_port=6,vlan_tci=0x,dl_src=00:00:00:00:00:00,dl_dst=28:99:3a:08:7a:
>>cf,dl_type=0x
>> 
>> 
>> Rule: table=0 cookie=0 priority=0
>> 
>> OpenFlow actions=NORMAL
>> 
>> forwarding to learned port
>> 
>> 
>> Final flow: unchanged
>> 
>> Megaflow: 
>>recirc_id=0,in_port=6,vlan_tci=0x/0x1fff,dl_src=00:00:00:00:00:00,dl_
>>dst=28:99:3a:08:7a:cf,dl_type=0x
>> 
>> Datapath actions: 5
>> 
>> 
>> This is with balance-tcp with lacp-fallback-ab mode (LACP is not
>>negotiated):
>> 
>> 
>> [root@frankfurter02-4 ~]# ovs-appctl ofproto/trace br0
>>in_port=6,dl_dst=28:99:3a:08:7a:cf
>> 
>> Bridge: br0
>> 
>> Flow: 
>>in_port=6,vlan_tci=0x,dl_src=00:00:00:00:00:00,dl_dst=28:99:3a:08:7a:
>>cf,dl_type=0x
>> 
>> 
>> Rule: table=0 cookie=0 priority=0
>> 
>> OpenFlow actions=NORMAL
>> 
>> forwarding to learned port
>> 
>> 
>> Final flow: unchanged
>> 
>> Megaflow: 
>>recirc_id=0,in_port=6,vlan_tci=0x/0x1fff,dl_src=00:00:00:00:00:00,dl_
>>dst=28:99:3a:08:7a:cf,dl_type=0x
>> 
>> Datapath actions: hash(hash_l4(0)),recirc(0x1)
>> 
>> From: Arunkumar Navasiva
>>mailto:arunkum.nava...@nutanix.com>>
>> Date: Thursday, October 4, 2018 at 4:18 PM
>> To: "ovs-dev@openvswitch.org"
>>mailto:ovs-dev@openvswitch.org>>
>> Subject: Issue with OVS lacp-fallback-ab option
>> 
>> Hello folks,
>> 
>> We¹re seeing an issue with lacp-fallback-ab option on ovs 2.5/2.6/2.8.
>>It looks like when LACP is not enabled on the TOR switch ports, OVS on
>>the centos server is not falling back cleanly to active-backup, and
>>continues to send some portions of the traffic through the backup
>>interface (we¹ve seen this occur with various TOR vendor switches).
>>Please see the attached screenshot which shows that some traffic still
>>hashes to backup interface. We looked at the TOR forwarding table, and
>>MAC addresses of VMs running on this server flaps between the two
>>corresponding TOR switch ports of the bond . I¹m still in the early
>>stages of debugging this, but wanted to reach out to you to see if this
>>is already a known issue? If not, any help on how to debug this further
>>will be helpful.
>> 
>> 
>> Thanks
>> -Arun
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> 
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org
>>_mailman_listinfo_ovs-2Ddev=DwIDaQ=s883GpUCOChKOHiocYtGcg=3XXybm-J2
>>6tdZghk4AB2Q6VwG-xD4UIstn2FwmI-3DQ=yn0Qe0gUppRi3JbdHCjsoa21myUOJTO9tU5M
>>s6ysj7s=iZmArQ2KxmCSrUeNKdCcJndIqugchI9rhK4s9iq4TrA=

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

[ovs-dev] [PATCH v2] ovn-nbctl: Add basic port group commands.

[Mark Michelson]

This adds the following commands:

pg-add: Add a new port group, optionally adding switch ports at
creation.
pg-set-ports: Sets the logical switch ports on a port group
pg-del: Remove a port group.

The main motivation for these commands is that it allows for adding
logical switch ports by name rather than UUID.

Signed-off-by: Mark Michelson 
---
v1 -> v2
Moved tests to tests/ovn-nbctl.at. This revealed issues when listing
database information in ovn-nbctl daemon mode. These issues are fixed in
series https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636
Therefore this patch is dependent on the linked series.
---
 ovn/utilities/ovn-nbctl.8.xml | 22 +
 ovn/utilities/ovn-nbctl.c | 72 +++
 tests/ovn-nbctl.at| 34 +++-
 tests/ovn.at  |  1 +
 4 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 5e18fa7c0..14cc02f53 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -881,6 +881,28 @@
   
 
 
+Port Group commands
+
+
+  pg-add group [port]...
+  
+Creates a new port group in the Port_Group table named
+group with optional ports added to the group.
+  
+
+  pg-set-ports group port...
+  
+Sets ports on the port group named group. It
+is an error if group does not exist.
+  
+
+  pg-del group
+  
+Deletes port group group. It is an error if
+group does not exist.
+  
+
+
 Database Commands
 These commands query and modify the contents of ovsdb 
tables.
 They are a slight abstraction of the ovsdb interface and
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bff6d1380..89f0d6213 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4655,6 +4655,73 @@ cmd_set_ssl(struct ctl_context *ctx)
 nbrec_nb_global_set_ssl(nb_global, ssl);
 }
 
+static char *
+set_ports_on_pg(struct ctl_context *ctx, const struct nbrec_port_group *pg,
+char **new_ports, size_t num_new_ports)
+{
+struct nbrec_logical_switch_port **lports;
+lports = xmalloc(sizeof *lports * num_new_ports);
+
+size_t i;
+char *error = NULL;
+for (i = 0; i < num_new_ports; i++) {
+const struct nbrec_logical_switch_port *lsp;
+error = lsp_by_name_or_uuid(ctx, new_ports[i], true, );
+if (error) {
+goto out;
+}
+lports[i] = (struct nbrec_logical_switch_port *) lsp;
+}
+
+nbrec_port_group_set_ports(pg, lports, num_new_ports);
+
+out:
+free(lports);
+return error;
+}
+
+static void
+cmd_pg_add(struct ctl_context *ctx)
+{
+const struct nbrec_port_group *pg;
+
+pg = nbrec_port_group_insert(ctx->txn);
+nbrec_port_group_set_name(pg, ctx->argv[1]);
+if (ctx->argc > 2) {
+ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2);
+}
+}
+
+static void
+cmd_pg_set_ports(struct ctl_context *ctx)
+{
+const struct nbrec_port_group *pg;
+
+char *error;
+error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, );
+if (error) {
+ctx->error = error;
+return;
+}
+
+ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2);
+}
+
+static void
+cmd_pg_del(struct ctl_context *ctx)
+{
+const struct nbrec_port_group *pg;
+
+char *error;
+error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, );
+if (error) {
+ctx->error = error;
+return;
+}
+
+nbrec_port_group_delete(pg);
+}
+
 static const struct ctl_table_class tables[NBREC_N_TABLES] = {
 [NBREC_TABLE_DHCP_OPTIONS].row_ids
 = {{_logical_switch_port_col_name, NULL,
@@ -5123,6 +5190,11 @@ static const struct ctl_command_syntax nbctl_commands[] 
= {
 "PRIVATE-KEY CERTIFICATE CA-CERT [SSL-PROTOS [SSL-CIPHERS]]",
 pre_cmd_set_ssl, cmd_set_ssl, NULL, "--bootstrap", RW},
 
+/* Port Group Commands */
+{"pg-add", 1, INT_MAX, "", NULL, cmd_pg_add, NULL, "", RW },
+{"pg-set-ports", 2, INT_MAX, "", NULL, cmd_pg_set_ports, NULL, "", RW },
+{"pg-del", 1, 1, "", NULL, cmd_pg_del, NULL, "", RW },
+
 {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO},
 };
 
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 3f89874ba..25414b8bd 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1518,7 +1518,6 @@ AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
 "pg0"
 ])])
 
-
 OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
 dnl This test addresses a specific issue seen when running ovn-nbctl in
 dnl daemon mode. All we have to do is ensure that each time we list database
@@ -1539,3 +1538,36 @@ AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
 AT_CHECK([ovn-nbctl --bare --columns=name list logical_switch sw1], [0], [dnl
 sw1
 ])])
+dnl 

Re: [ovs-dev] Issue with OVS lacp-fallback-ab option

[Ben Pfaff]

I think I see a problem in the implementation of bonding when
recirculation is available.  Are you able to try out a patch?  If so,
try the following.  It is not a good way to solve the issue, but it
should illustrate whether recirculation is the problem.

diff --git a/ofproto/bond.c b/ofproto/bond.c
index f87cdba7908f..bb6a80411de5 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -927,7 +928,7 @@ bond_recirculation_account(struct bond *bond)
 static bool
 bond_may_recirc(const struct bond *bond)
 {
-return bond->balance == BM_TCP && bond->recirc_id;
+return bond->balance == BM_TCP && bond->recirc_id && false;
 }
 
 static void


On Fri, Oct 05, 2018 at 04:45:21AM +, Arun Navasivasakthivelsamy wrote:
> Also, ofproto/trace is suggesting that the packet will be hashed to a slave 
> (instead of just to the active port) with lacp-fallback-ab option.
> 
> This is with active-backup bond mode:
> 
> 
> [root@frankfurter02-4 ~]# ovs-appctl ofproto/trace br0 
> in_port=6,dl_dst=28:99:3a:08:7a:cf
> 
> Bridge: br0
> 
> Flow: 
> in_port=6,vlan_tci=0x,dl_src=00:00:00:00:00:00,dl_dst=28:99:3a:08:7a:cf,dl_type=0x
> 
> 
> Rule: table=0 cookie=0 priority=0
> 
> OpenFlow actions=NORMAL
> 
> forwarding to learned port
> 
> 
> Final flow: unchanged
> 
> Megaflow: 
> recirc_id=0,in_port=6,vlan_tci=0x/0x1fff,dl_src=00:00:00:00:00:00,dl_dst=28:99:3a:08:7a:cf,dl_type=0x
> 
> Datapath actions: 5
> 
> 
> This is with balance-tcp with lacp-fallback-ab mode (LACP is not negotiated):
> 
> 
> [root@frankfurter02-4 ~]# ovs-appctl ofproto/trace br0 
> in_port=6,dl_dst=28:99:3a:08:7a:cf
> 
> Bridge: br0
> 
> Flow: 
> in_port=6,vlan_tci=0x,dl_src=00:00:00:00:00:00,dl_dst=28:99:3a:08:7a:cf,dl_type=0x
> 
> 
> Rule: table=0 cookie=0 priority=0
> 
> OpenFlow actions=NORMAL
> 
> forwarding to learned port
> 
> 
> Final flow: unchanged
> 
> Megaflow: 
> recirc_id=0,in_port=6,vlan_tci=0x/0x1fff,dl_src=00:00:00:00:00:00,dl_dst=28:99:3a:08:7a:cf,dl_type=0x
> 
> Datapath actions: hash(hash_l4(0)),recirc(0x1)
> 
> From: Arunkumar Navasiva 
> mailto:arunkum.nava...@nutanix.com>>
> Date: Thursday, October 4, 2018 at 4:18 PM
> To: "ovs-dev@openvswitch.org" 
> mailto:ovs-dev@openvswitch.org>>
> Subject: Issue with OVS lacp-fallback-ab option
> 
> Hello folks,
> 
> We’re seeing an issue with lacp-fallback-ab option on ovs 2.5/2.6/2.8. It 
> looks like when LACP is not enabled on the TOR switch ports, OVS on the 
> centos server is not falling back cleanly to active-backup, and continues to 
> send some portions of the traffic through the backup interface (we’ve seen 
> this occur with various TOR vendor switches).  Please see the attached 
> screenshot which shows that some traffic still hashes to backup interface. We 
> looked at the TOR forwarding table, and MAC addresses of VMs running on this 
> server flaps between the two corresponding TOR switch ports of the bond . I’m 
> still in the early stages of debugging this, but wanted to reach out to you 
> to see if this is already a known issue? If not, any help on how to debug 
> this further will be helpful.
> 
> 
> Thanks
> -Arun
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/2] Fix ovn-nbctl daemon table printing issues.

[Ben Pfaff]

On Mon, Oct 08, 2018 at 02:49:06PM -0400, Mark Michelson wrote:
> ovn-nbctl when run in daemon mode has two issues:
> 1) An extra newline is prepended to table output
> 2) Table formatting issues are ignored.
> 
> This patch series fixes both issues.
> ---
> v1 -> v2: Added tests on each patch

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

Re: [ovs-dev] [PATCH] flow: Clear ovs_nsh_key's context data when nsh's type can't be handled

[Ben Pfaff]

On Thu, Oct 04, 2018 at 02:23:39PM -0700, Yifeng Sun wrote:
> In the default case when nsh's md_type is not recognized by nsh parser,
> uninitialized data in key->context can sneak into miniflow. This
> patch fixes it.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10519
> Signed-off-by: Yifeng Sun 

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

Re: [ovs-dev] [PATCH 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[aginwala aginwala]

Yes, that's right.  I will send out v2 in a bit with Han's ack.


Regards,
Aliasgar

On Mon, Oct 8, 2018 at 11:04 AM Ben Pfaff  wrote:

> On Mon, Oct 08, 2018 at 10:58:49AM -0700, Han Zhou wrote:
> > On Fri, Oct 5, 2018 at 6:34 PM aginwala aginwala 
> wrote:
> > >
> > > Thanks for the review Han. Please find the comments inline below:
> > >
> > > On Thu, Oct 4, 2018 at 10:16 AM Han Zhou  wrote:
> > >>
> > >> Thanks Ali, please see my comm
> > >>
> > >> On Fri, Sep 21, 2018 at 5:38 PM  wrote:
> > >> >
> > >> >  When starting OVN DBs in HA using pacemaker with ssl, we need to
> pass
> > ssl
> > >> >  certs for starting standby DBs. Hence, we need this change.
> > >> >
> > >> > Signed-off-by: aginwala 
> > >> > ---
> > >> >  ovn/utilities/ovndb-servers.ocf | 74
> > -
> > >> >  1 file changed, 73 insertions(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/ovn/utilities/ovndb-servers.ocf
> > b/ovn/utilities/ovndb-servers.ocf
> > >> > index 52141c7..80f81ae 100755
> > >> > --- a/ovn/utilities/ovndb-servers.ocf
> > >> > +++ b/ovn/utilities/ovndb-servers.ocf
> > >> > @@ -10,6 +10,12 @@
> > >> >  : ${MANAGE_NORTHD_DEFAULT="no"}
> > >> >  : ${INACTIVE_PROBE_DEFAULT="5000"}
> > >> >  : ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"}
> > >> > +: ${NB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnnb-privkey.pem"}
> > >> > +: ${NB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnnb-cert.pem"}
> > >> > +: ${NB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
> > >> > +: ${SB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnsb-privkey.pem"}
> > >> > +: ${SB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnsb-cert.pem"}
> > >> > +: ${SB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
> > >> >
> > >> >  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
> > >> >  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config
> > --name OVN_REPL_INFO -s ovn_ovsdb_master_server"
> > >> > @@ -21,6 +27,13 @@
> > SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
> > >> >
> >
> SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
> > >> >  MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
> > >> >
> >
> INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}}
> > >> > +NB_PRIVKEY=${OCF_RESKEY_ovn_nb_db_privkey:-${NB_SSL_KEY_DEFAULT}}
> > >> > +NB_CERT=${OCF_RESKEY_ovn_nb_db_cert:-${NB_SSL_CERT_DEFAULT}}
> > >> > +NB_CACERT=${OCF_RESKEY_ovn_nb_db_cacert:-${NB_SSL_CACERT_DEFAULT}}
> > >> > +SB_PRIVKEY=${OCF_RESKEY_ovn_sb_db_privkey:-${SB_SSL_KEY_DEFAULT}}
> > >> > +SB_CERT=${OCF_RESKEY_ovn_sb_db_cert:-${SB_SSL_CERT_DEFAULT}}
> > >> > +SB_CACERT=${OCF_RESKEY_ovn_sb_db_cacert:-${SB_SSL_CACERT_DEFAULT}}
> > >> > +
> > >> >
> > >> >  # In order for pacemaker to work with LB, we can set
> > LISTEN_ON_MASTER_IP_ONLY
> > >> >  # to false and pass LB vip IP while creating pcs resource.
> > >> > @@ -132,6 +145,54 @@ ovsdb_server_metadata() {
> > >> >
> > >> >
> > >> >
> > >> > +  
> > >> > +  
> > >> > +  OVN NB DB private key absolute path for ssl setup.
> > >> > +  
> > >> > +  OVN NB DB private key file
> > >> > +  
> > >> > +  
> > >> > +
> > >> > +  
> > >> > +  
> > >> > +  OVN NB DB certificate absolute path for ssl setup.
> > >> > +  
> > >> > +  OVN NB DB cert file
> > >> > +  
> > >> > +  
> > >> > +
> > >> > +  
> > >> > +  
> > >> > +  OVN NB DB CA certificate absolute path for ssl setup.
> > >> > +  
> > >> > +  OVN NB DB cacert file
> > >> > +  
> > >> > +  
> > >> > +
> > >> > +  
> > >> > +  
> > >> > +  OVN SB DB private key absolute path for ssl setup.
> > >> > +  
> > >> > +  OVN SB DB private key file
> > >> > +  
> > >> > +  
> > >> > +
> > >> > +  
> > >> > +  
> > >> > +  OVN SB DB certificate absolute path for ssl setup.
> > >> > +  
> > >> > +  OVN SB DB cert file
> > >> > +  
> > >> > +  
> > >> > +
> > >> > +  
> > >> > +  
> > >> > +  OVN SB DB CA certificate absolute path for ssl setup.
> > >> > +  
> > >> > +  OVN SB DB cacert file
> > >> > +  
> > >> > +  
> > >> > +
> > >> >
> > >> >
> > >> >
> > >> > @@ -326,6 +387,18 @@ ovsdb_server_start() {
> > >> > set $@ --db-sb-addr=${MASTER_IP}
> --db-sb-port=${SB_MASTER_PORT}
> > >> >  fi
> > >> >
> > >> > +if [ "x${NB_MASTER_PROTO}" = xssl ]; then
> > >> > +set $@ --db-nb-create-insecure-remote=no
> > >> "no" is the default value, so this line is not needed.
> > >
> > > >> Sure. This makes sense. Will check out the default behavior and
> update
> > it the revised patch!
> > >>
> > >>
> > >> > +set $@ --ovn-nb-db-ssl-key=${NB_PRIVKEY}
> > >> > +set $@ --ovn-nb-db-ssl-cert=${NB_CERT}
> > >> > +set $@ --ovn-nb-db-ssl-ca-cert=${NB_CACERT}
> > >> This should be needed only for standby which sets
> > --db-sb-use-remote-in-db=no.
> > >
> > > > As discussed, for each of the modes either ssl or tcp, all the nodes
> > should have this option set.
> >
> > Agree. Since this script is for active-standby only, we can 

Re: [ovs-dev] [PATCH] Add basic PG commands.

[Mark Michelson]

On 10/08/2018 11:40 AM, Ben Pfaff wrote:

On Fri, Oct 05, 2018 at 04:05:22PM -0400, Mark Michelson wrote:

Thanks for the feedback Han. Interestingly, when I move the tests from
ovn.at to ovn-nbctl.at, the tests no longer pass. When running in non-daemon
mode, for whatever reason, trying to get the port group's ports from the
database results in an empty return. I can't reproduce this in the sandbox,
so I'll need to dive in and see what's going wrong.

When run in daemon mode, interestingly, it appears that there are a couple
of legitimate bugs I'm running into. First, there's an extra blank line at
the beginning before the output of the database output. Second, it appears
the --bare flag is ignored. I can reproduce both of these in the sandbox.

So I think this is going to result in another patch from me to fix the
daemon mode bugs mentioned above.


I'm going to assume that you'll post a new version of this after the bug
fix patches get in.



Yes, the next version of the patch will be dependent on 
https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636

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

[ovs-dev] [PATCH v2 2/2] ovn-nbctl: Don't parse table-formatting options in nbctl_client

[Mark Michelson]

When ovn-nbctl is running in daemon mode, nbctl_client attempts to parse
table formatting options. The problem is that this then removes the table
formatting options from the array of options passed to the server loop. The
server loop resets the table formatting options to the defaults and then
attempts again to parse table formatting options. Unfortunately, they aren't
present any longer. The result is that tables are always formatted with
the default style.

This patch solves the issue by not parsing the table formatting options
in nbctl_client. Instead, the table formatting options are passed to the
server loop and parsed there instead.

Signed-off-by: Mark Michelson 
---
 ovn/utilities/ovn-nbctl.c | 1 -
 tests/ovn-nbctl.at| 9 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index d65a9ba08..bff6d1380 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -5443,7 +5443,6 @@ nbctl_client(const char *socket_name,
 break;
 
 VLOG_OPTION_HANDLERS
-TABLE_OPTION_HANDLERS(_style)
 
 case OPT_LOCAL:
 default:
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 7d627cdc0..3f89874ba 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1530,3 +1530,12 @@ name: "sw1"
 AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
 name: "sw1"
 ])])
+
+OVN_NBCTL_TEST([ovn_nbctl_table_formatting], [table formatting], [
+dnl This test addresses a specific issue seen when running ovn-nbctl in
+dnl daemon mode. We need to ensure that table formatting options are honored
+dnl when listing database information.
+AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
+AT_CHECK([ovn-nbctl --bare --columns=name list logical_switch sw1], [0], [dnl
+sw1
+])])
-- 
2.14.4

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

[ovs-dev] [PATCH v2 1/2] table: Create method for resetting table formatting.

[Mark Michelson]

Table formatting has a local static integer that is intended to insert
line breaks between tables. This works exactly as intended, as long as
each call to table_format() is done as a single unit within the run of a
process.

When ovn-nbctl is run in daemon mode, it is a long-running process that
makes multiple calls to table_format() throughout its lifetime. After
the first call, this results in an unexpected newline prepended to table
output on each subsequent ovn-nbctl invocation.

The solution is to introduce a function to reset table formatting. This
way, the first time after resetting table formatting, no newline is
prepended.

Signed-off-by: Mark Michelson 
---
 lib/table.c   | 23 +--
 lib/table.h   |  1 +
 ovn/utilities/ovn-nbctl.c |  1 +
 tests/ovn-nbctl.at| 13 +
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/lib/table.c b/lib/table.c
index ab72668c7..48d18b651 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -236,15 +236,18 @@ table_print_timestamp__(const struct table *table, struct 
ds *s)
 }
 }
 
+static bool first_table = true;
+
 static void
 table_print_table__(const struct table *table, const struct table_style *style,
 struct ds *s)
 {
-static int n = 0;
 int *widths;
 size_t x, y;
 
-if (n++ > 0) {
+if (first_table) {
+first_table = false;
+} else {
 ds_put_char(s, '\n');
 }
 
@@ -318,10 +321,11 @@ static void
 table_print_list__(const struct table *table, const struct table_style *style,
struct ds *s)
 {
-static int n = 0;
 size_t x, y;
 
-if (n++ > 0) {
+if (first_table) {
+first_table = false;
+} else {
 ds_put_char(s, '\n');
 }
 
@@ -469,10 +473,11 @@ static void
 table_print_csv__(const struct table *table, const struct table_style *style,
   struct ds *s)
 {
-static int n = 0;
 size_t x, y;
 
-if (n++ > 0) {
+if (first_table) {
+first_table = false;
+} else {
 ds_put_char(s, '\n');
 }
 
@@ -614,6 +619,12 @@ table_format(const struct table *table, const struct 
table_style *style,
 }
 }
 
+void
+table_format_reset(void)
+{
+first_table = true;
+}
+
 /* Outputs 'table' on stdout in the specified 'style'. */
 void
 table_print(const struct table *table, const struct table_style *style)
diff --git a/lib/table.h b/lib/table.h
index 76e65bb70..33263e2a2 100644
--- a/lib/table.h
+++ b/lib/table.h
@@ -133,6 +133,7 @@ void table_parse_cell_format(struct table_style *, const 
char *format);
 void table_print(const struct table *, const struct table_style *);
 void table_format(const struct table *, const struct table_style *,
   struct ds *);
+void table_format_reset(void);
 void table_usage(void);
 
 #endif /* table.h */
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index eabd30308..d65a9ba08 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -5311,6 +5311,7 @@ server_cmd_run(struct unixctl_conn *conn, int argc, const 
char **argv_,
 }
 
 struct ds output = DS_EMPTY_INITIALIZER;
+table_format_reset();
 for (struct ctl_command *c = commands; c < [n_commands]; c++) {
 if (c->table) {
 table_format(c->table, _style, );
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index a599b1bf7..7d627cdc0 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1517,3 +1517,16 @@ AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], 
[ignore])
 AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
 "pg0"
 ])])
+
+
+OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
+dnl This test addresses a specific issue seen when running ovn-nbctl in
+dnl daemon mode. All we have to do is ensure that each time we list database
+dnl information, there is not an extra newline at the beginning of the output.
+AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
+AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
+name: "sw1"
+])
+AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
+name: "sw1"
+])])
-- 
2.14.4

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

[ovs-dev] [PATCH v2 0/2] Fix ovn-nbctl daemon table printing issues.

[Mark Michelson]

ovn-nbctl when run in daemon mode has two issues:
1) An extra newline is prepended to table output
2) Table formatting issues are ignored.

This patch series fixes both issues.
---
v1 -> v2: Added tests on each patch
---
Mark Michelson (2):
  table: Create method for resetting table formatting.
  ovn-nbctl: Don't parse table-formatting options in nbctl_client

 lib/table.c   | 23 +--
 lib/table.h   |  1 +
 ovn/utilities/ovn-nbctl.c |  2 +-
 tests/ovn-nbctl.at| 22 ++
 4 files changed, 41 insertions(+), 7 deletions(-)

-- 
2.14.4

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

Re: [ovs-dev] [PATCH] ovs-save: Parse geneve tlv map correctly.

[Guru Shetty]

On Mon, 8 Oct 2018 at 11:12, Ben Pfaff  wrote:

> On Sun, Oct 07, 2018 at 09:53:15PM -0700, Gurucharan Shetty wrote:
> > We now have an extra space in the o/p of `ovs-ofctl dump-tlv-map`.
> >
> > Fixes: 5a0e4aec1af (treewide: Convert leading tabs to spaces.)
> > Signed-off-by: Gurucharan Shetty 
> > ---
> >  utilities/ovs-save | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/utilities/ovs-save b/utilities/ovs-save
> > index 5c046bb..f85d97a 100755
> > --- a/utilities/ovs-save
> > +++ b/utilities/ovs-save
> > @@ -117,7 +117,7 @@ save_flows () {
> >
> >  printf "%s" "ovs-ofctl add-tlv-map ${bridge} '"
> >  ovs-ofctl dump-tlv-map ${bridge} | \
> > -awk '/^ 0x/ {if (cnt != 0) printf ","; \
> > +awk '/^  0x/ {if (cnt != 0) printf ","; \
>
> It might be better to use /^  *0x/, because that will handle either
> form.  ovs-save is used during upgrades so there's some ambiguity about
> which version of ovs-ofctl is going to be used, and in general accepting
> more flexible input is good.
>
> Acked-by: Ben Pfaff 
>
You are right. I will use your suggestion and merge to master and 2.10


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

Re: [ovs-dev] [PATCH v4] ovn: Support configuring the BFD params for the tunnel interfaces

[Ben Pfaff]

On Sat, Oct 06, 2018 at 08:04:09PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> With this commit the users can override the default values of
> the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
> This can be useful to debug any issues related to BFD (like
> frequent BFD state changes).
> 
> A new column 'options' is added in NB_Global and SB_Global tables
> of OVN_Northbound and OVN_Southbound schemas respectively. CMS
> can define the options 'bfd:min-rx', 'bfd:min-tx',
> 'bfd:decay-min-rx' and 'bfd:mult' in the options column of
> NB_Global table row. ovn-northd copies these options from
> NB_Global to SB_Global. ovn-controller configures these
> options to the tunnel interfaces when enabling BFD.
> 
> Signed-off-by: Numan Siddique 

Thank you for working to make OVN easier to debug and troubleshoot.

Using : in the bfd option names makes it difficult to set them using
ovn-nbctl.  It is necessary to understand how to escape the :, as shown
in the test, e.g.:

ovn-nbctl --wait=hv set NB_Global . options:"bfd\:min-rx"=2000

Do you think it would be a good idea to use - instead of : for this
reason?

When bfd_run() decides to disable bfd on an interface, it looks like it
will still add any user-specified BFD settings.  Is that intentional?

The function interface_set_bfd() is now trivial, so it might make sense
to inline it into the caller.

Thanks,

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

Re: [ovs-dev] [PATCH] ovs-save: Parse geneve tlv map correctly.

[Ben Pfaff]

On Sun, Oct 07, 2018 at 09:53:15PM -0700, Gurucharan Shetty wrote:
> We now have an extra space in the o/p of `ovs-ofctl dump-tlv-map`.
> 
> Fixes: 5a0e4aec1af (treewide: Convert leading tabs to spaces.)
> Signed-off-by: Gurucharan Shetty 
> ---
>  utilities/ovs-save | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utilities/ovs-save b/utilities/ovs-save
> index 5c046bb..f85d97a 100755
> --- a/utilities/ovs-save
> +++ b/utilities/ovs-save
> @@ -117,7 +117,7 @@ save_flows () {
>  
>  printf "%s" "ovs-ofctl add-tlv-map ${bridge} '"
>  ovs-ofctl dump-tlv-map ${bridge} | \
> -awk '/^ 0x/ {if (cnt != 0) printf ","; \
> +awk '/^  0x/ {if (cnt != 0) printf ","; \

It might be better to use /^  *0x/, because that will handle either
form.  ovs-save is used during upgrades so there's some ambiguity about
which version of ovs-ofctl is going to be used, and in general accepting
more flexible input is good.

Acked-by: Ben Pfaff 

Thanks,

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

Re: [ovs-dev] [PATCH] dp-packet.h: move funcs to be within cond block

[Ben Pfaff]

On Mon, Oct 08, 2018 at 05:39:04PM +, Stokes, Ian wrote:
> > For what it's worth, Ian, I'm hoping that you'll take a look at this and
> > decide whether to merge it.
> 
> Sure, thanks for flagging Ben, apologies for the delay, I've been on
> vacation the past 2 weeks but will have time to look at this now.

Oh, I forgot about the vacation, even though you told me about it.
Please forgive me for also re-raising the other issue I emailed about a
few minutes ago.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] expr: Set a limit on the depth of nested parentheses

[Yifeng Sun]

Thanks for the review! I will come up with a new version.

Yifeng

On Mon, Oct 8, 2018 at 11:02 AM Ben Pfaff  wrote:

> On Thu, Oct 04, 2018 at 04:30:10PM -0700, Yifeng Sun wrote:
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714
> > Signed-off-by: Yifeng Sun 
> > Suggested-by: Ben Pfaff 
>
> Thanks for fixing this bug.
>
> In parse_chassis_resident(), in two error cases, paren_depth is
> incremented but never decremented.  Maybe it does not matter because
> these are error cases, but I would prefer to always correctly maintain
> the depth.
>
> It is not necessary to initial paren_depth explicitly in expr_parse(),
> because it will be initialized to 0 by the compiler automatically.  Some
> developers would argue that it's a good idea anyway, which is also a
> fine position to take, but in that case I'd suggest that all of other
> places we initialize an expr_context (I see four others) we should also
> initialize paren_depth explicitly.
>
> Please add a test for this new error message in the "ovn -- expression
> parser" test in tests/ovn.at.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[Ben Pfaff]

On Mon, Oct 08, 2018 at 10:58:49AM -0700, Han Zhou wrote:
> On Fri, Oct 5, 2018 at 6:34 PM aginwala aginwala  wrote:
> >
> > Thanks for the review Han. Please find the comments inline below:
> >
> > On Thu, Oct 4, 2018 at 10:16 AM Han Zhou  wrote:
> >>
> >> Thanks Ali, please see my comm
> >>
> >> On Fri, Sep 21, 2018 at 5:38 PM  wrote:
> >> >
> >> >  When starting OVN DBs in HA using pacemaker with ssl, we need to pass
> ssl
> >> >  certs for starting standby DBs. Hence, we need this change.
> >> >
> >> > Signed-off-by: aginwala 
> >> > ---
> >> >  ovn/utilities/ovndb-servers.ocf | 74
> -
> >> >  1 file changed, 73 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/ovn/utilities/ovndb-servers.ocf
> b/ovn/utilities/ovndb-servers.ocf
> >> > index 52141c7..80f81ae 100755
> >> > --- a/ovn/utilities/ovndb-servers.ocf
> >> > +++ b/ovn/utilities/ovndb-servers.ocf
> >> > @@ -10,6 +10,12 @@
> >> >  : ${MANAGE_NORTHD_DEFAULT="no"}
> >> >  : ${INACTIVE_PROBE_DEFAULT="5000"}
> >> >  : ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"}
> >> > +: ${NB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnnb-privkey.pem"}
> >> > +: ${NB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnnb-cert.pem"}
> >> > +: ${NB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
> >> > +: ${SB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnsb-privkey.pem"}
> >> > +: ${SB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnsb-cert.pem"}
> >> > +: ${SB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
> >> >
> >> >  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
> >> >  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config
> --name OVN_REPL_INFO -s ovn_ovsdb_master_server"
> >> > @@ -21,6 +27,13 @@
> SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
> >> >
>  SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
> >> >  MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
> >> >
>  
> INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}}
> >> > +NB_PRIVKEY=${OCF_RESKEY_ovn_nb_db_privkey:-${NB_SSL_KEY_DEFAULT}}
> >> > +NB_CERT=${OCF_RESKEY_ovn_nb_db_cert:-${NB_SSL_CERT_DEFAULT}}
> >> > +NB_CACERT=${OCF_RESKEY_ovn_nb_db_cacert:-${NB_SSL_CACERT_DEFAULT}}
> >> > +SB_PRIVKEY=${OCF_RESKEY_ovn_sb_db_privkey:-${SB_SSL_KEY_DEFAULT}}
> >> > +SB_CERT=${OCF_RESKEY_ovn_sb_db_cert:-${SB_SSL_CERT_DEFAULT}}
> >> > +SB_CACERT=${OCF_RESKEY_ovn_sb_db_cacert:-${SB_SSL_CACERT_DEFAULT}}
> >> > +
> >> >
> >> >  # In order for pacemaker to work with LB, we can set
> LISTEN_ON_MASTER_IP_ONLY
> >> >  # to false and pass LB vip IP while creating pcs resource.
> >> > @@ -132,6 +145,54 @@ ovsdb_server_metadata() {
> >> >
> >> >
> >> >
> >> > +  
> >> > +  
> >> > +  OVN NB DB private key absolute path for ssl setup.
> >> > +  
> >> > +  OVN NB DB private key file
> >> > +  
> >> > +  
> >> > +
> >> > +  
> >> > +  
> >> > +  OVN NB DB certificate absolute path for ssl setup.
> >> > +  
> >> > +  OVN NB DB cert file
> >> > +  
> >> > +  
> >> > +
> >> > +  
> >> > +  
> >> > +  OVN NB DB CA certificate absolute path for ssl setup.
> >> > +  
> >> > +  OVN NB DB cacert file
> >> > +  
> >> > +  
> >> > +
> >> > +  
> >> > +  
> >> > +  OVN SB DB private key absolute path for ssl setup.
> >> > +  
> >> > +  OVN SB DB private key file
> >> > +  
> >> > +  
> >> > +
> >> > +  
> >> > +  
> >> > +  OVN SB DB certificate absolute path for ssl setup.
> >> > +  
> >> > +  OVN SB DB cert file
> >> > +  
> >> > +  
> >> > +
> >> > +  
> >> > +  
> >> > +  OVN SB DB CA certificate absolute path for ssl setup.
> >> > +  
> >> > +  OVN SB DB cacert file
> >> > +  
> >> > +  
> >> > +
> >> >
> >> >
> >> >
> >> > @@ -326,6 +387,18 @@ ovsdb_server_start() {
> >> > set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT}
> >> >  fi
> >> >
> >> > +if [ "x${NB_MASTER_PROTO}" = xssl ]; then
> >> > +set $@ --db-nb-create-insecure-remote=no
> >> "no" is the default value, so this line is not needed.
> >
> > >> Sure. This makes sense. Will check out the default behavior and update
> it the revised patch!
> >>
> >>
> >> > +set $@ --ovn-nb-db-ssl-key=${NB_PRIVKEY}
> >> > +set $@ --ovn-nb-db-ssl-cert=${NB_CERT}
> >> > +set $@ --ovn-nb-db-ssl-ca-cert=${NB_CACERT}
> >> This should be needed only for standby which sets
> --db-sb-use-remote-in-db=no.
> >
> > > As discussed, for each of the modes either ssl or tcp, all the nodes
> should have this option set.
> 
> Agree. Since this script is for active-standby only, we can assume
> active-standby mode always use command line option instead of DB settings.
> 
> Acked-by: Han Zhou 

I haven't followed the discussion here so I'm going to assume that Ali
will post a v2 with Han's ack.

Thanks,

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

Re: [ovs-dev] [PATCH] expr: Set a limit on the depth of nested parentheses

[Ben Pfaff]

On Thu, Oct 04, 2018 at 04:30:10PM -0700, Yifeng Sun wrote:
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714
> Signed-off-by: Yifeng Sun 
> Suggested-by: Ben Pfaff 

Thanks for fixing this bug.

In parse_chassis_resident(), in two error cases, paren_depth is
incremented but never decremented.  Maybe it does not matter because
these are error cases, but I would prefer to always correctly maintain
the depth.

It is not necessary to initial paren_depth explicitly in expr_parse(),
because it will be initialized to 0 by the compiler automatically.  Some
developers would argue that it's a good idea anyway, which is also a
fine position to take, but in that case I'd suggest that all of other
places we initialize an expr_context (I see four others) we should also
initialize paren_depth explicitly.

Please add a test for this new error message in the "ovn -- expression
parser" test in tests/ovn.at.

Thanks,

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

Re: [ovs-dev] [PATCH 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[Han Zhou]

On Fri, Oct 5, 2018 at 6:34 PM aginwala aginwala  wrote:
>
> Thanks for the review Han. Please find the comments inline below:
>
> On Thu, Oct 4, 2018 at 10:16 AM Han Zhou  wrote:
>>
>> Thanks Ali, please see my comm
>>
>> On Fri, Sep 21, 2018 at 5:38 PM  wrote:
>> >
>> >  When starting OVN DBs in HA using pacemaker with ssl, we need to pass
ssl
>> >  certs for starting standby DBs. Hence, we need this change.
>> >
>> > Signed-off-by: aginwala 
>> > ---
>> >  ovn/utilities/ovndb-servers.ocf | 74
-
>> >  1 file changed, 73 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ovn/utilities/ovndb-servers.ocf
b/ovn/utilities/ovndb-servers.ocf
>> > index 52141c7..80f81ae 100755
>> > --- a/ovn/utilities/ovndb-servers.ocf
>> > +++ b/ovn/utilities/ovndb-servers.ocf
>> > @@ -10,6 +10,12 @@
>> >  : ${MANAGE_NORTHD_DEFAULT="no"}
>> >  : ${INACTIVE_PROBE_DEFAULT="5000"}
>> >  : ${LISTEN_ON_MASTER_IP_ONLY_DEFAULT="yes"}
>> > +: ${NB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnnb-privkey.pem"}
>> > +: ${NB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnnb-cert.pem"}
>> > +: ${NB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
>> > +: ${SB_SSL_KEY_DEFAULT="/etc/openvswitch/ovnsb-privkey.pem"}
>> > +: ${SB_SSL_CERT_DEFAULT="/etc/openvswitch/ovnsb-cert.pem"}
>> > +: ${SB_SSL_CACERT_DEFAULT="/etc/openvswitch/cacert.pem"}
>> >
>> >  CRM_MASTER="${HA_SBIN_DIR}/crm_master -l reboot"
>> >  CRM_ATTR_REPL_INFO="${HA_SBIN_DIR}/crm_attribute --type crm_config
--name OVN_REPL_INFO -s ovn_ovsdb_master_server"
>> > @@ -21,6 +27,13 @@
SB_MASTER_PORT=${OCF_RESKEY_sb_master_port:-${SB_MASTER_PORT_DEFAULT}}
>> >
 SB_MASTER_PROTO=${OCF_RESKEY_sb_master_protocol:-${SB_MASTER_PROTO_DEFAULT}}
>> >  MANAGE_NORTHD=${OCF_RESKEY_manage_northd:-${MANAGE_NORTHD_DEFAULT}}
>> >
 INACTIVE_PROBE=${OCF_RESKEY_inactive_probe_interval:-${INACTIVE_PROBE_DEFAULT}}
>> > +NB_PRIVKEY=${OCF_RESKEY_ovn_nb_db_privkey:-${NB_SSL_KEY_DEFAULT}}
>> > +NB_CERT=${OCF_RESKEY_ovn_nb_db_cert:-${NB_SSL_CERT_DEFAULT}}
>> > +NB_CACERT=${OCF_RESKEY_ovn_nb_db_cacert:-${NB_SSL_CACERT_DEFAULT}}
>> > +SB_PRIVKEY=${OCF_RESKEY_ovn_sb_db_privkey:-${SB_SSL_KEY_DEFAULT}}
>> > +SB_CERT=${OCF_RESKEY_ovn_sb_db_cert:-${SB_SSL_CERT_DEFAULT}}
>> > +SB_CACERT=${OCF_RESKEY_ovn_sb_db_cacert:-${SB_SSL_CACERT_DEFAULT}}
>> > +
>> >
>> >  # In order for pacemaker to work with LB, we can set
LISTEN_ON_MASTER_IP_ONLY
>> >  # to false and pass LB vip IP while creating pcs resource.
>> > @@ -132,6 +145,54 @@ ovsdb_server_metadata() {
>> >
>> >
>> >
>> > +  
>> > +  
>> > +  OVN NB DB private key absolute path for ssl setup.
>> > +  
>> > +  OVN NB DB private key file
>> > +  
>> > +  
>> > +
>> > +  
>> > +  
>> > +  OVN NB DB certificate absolute path for ssl setup.
>> > +  
>> > +  OVN NB DB cert file
>> > +  
>> > +  
>> > +
>> > +  
>> > +  
>> > +  OVN NB DB CA certificate absolute path for ssl setup.
>> > +  
>> > +  OVN NB DB cacert file
>> > +  
>> > +  
>> > +
>> > +  
>> > +  
>> > +  OVN SB DB private key absolute path for ssl setup.
>> > +  
>> > +  OVN SB DB private key file
>> > +  
>> > +  
>> > +
>> > +  
>> > +  
>> > +  OVN SB DB certificate absolute path for ssl setup.
>> > +  
>> > +  OVN SB DB cert file
>> > +  
>> > +  
>> > +
>> > +  
>> > +  
>> > +  OVN SB DB CA certificate absolute path for ssl setup.
>> > +  
>> > +  OVN SB DB cacert file
>> > +  
>> > +  
>> > +
>> >
>> >
>> >
>> > @@ -326,6 +387,18 @@ ovsdb_server_start() {
>> > set $@ --db-sb-addr=${MASTER_IP} --db-sb-port=${SB_MASTER_PORT}
>> >  fi
>> >
>> > +if [ "x${NB_MASTER_PROTO}" = xssl ]; then
>> > +set $@ --db-nb-create-insecure-remote=no
>> "no" is the default value, so this line is not needed.
>
> >> Sure. This makes sense. Will check out the default behavior and update
it the revised patch!
>>
>>
>> > +set $@ --ovn-nb-db-ssl-key=${NB_PRIVKEY}
>> > +set $@ --ovn-nb-db-ssl-cert=${NB_CERT}
>> > +set $@ --ovn-nb-db-ssl-ca-cert=${NB_CACERT}
>> This should be needed only for standby which sets
--db-sb-use-remote-in-db=no.
>
> > As discussed, for each of the modes either ssl or tcp, all the nodes
should have this option set.

Agree. Since this script is for active-standby only, we can assume
active-standby mode always use command line option instead of DB settings.

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

Re: [ovs-dev] [PATCH 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[Han Zhou]

On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala  wrote:
>
> Thanks for the review Han. Please find the comments inline below:
> On Thu, Oct 4, 2018 at 9:58 AM Han Zhou  wrote:
>>
>> Thanks Ali, please see my comments below
>>
>> On Fri, Sep 21, 2018 at 5:34 PM  wrote:
>> >
>> >  For OVN DBs to work with SSL in HA, we need to have capability to
>> >  pass ssl certs when starting OVN DBs. Say when starting OVN DBs in
active
>> >  passive mode, in order for the standby DBs to sync from master node,
it
>> >  cannot sync because the required ssl certs are not passed when
standby DBs
>> >  are initialized. Hence, we need to have this option.
>> >
>> > e.g. start nb db with ssl certs as below:
>> > /usr/share/openvswitch/scripts/ovn-ctl
--ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
>> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
>> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
>> > --db-nb-create-insecure-remote=no start_nb_ovsdb
>> >
>> > Certs can be generated based on ovs ssl docs:
>> > http://docs.openvswitch.org/en/latest/howto/ssl/
>> >
>> > Signed-off-by: aginwala 
>> > ---
>> >  ovn/utilities/ovn-ctl | 50
+++---
>> >  1 file changed, 43 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
>> > index 3ff0df6..4f45f3d 100755
>> > --- a/ovn/utilities/ovn-ctl
>> > +++ b/ovn/utilities/ovn-ctl
>> > @@ -116,6 +116,9 @@ start_ovsdb__() {
>> >  local addr
>> >  local active_conf_file
>> >  local use_remote_in_db
>> > +local ovn_db_ssl_key
>> > +local ovn_db_ssl_cert
>> > +local ovn_db_ssl_cacert
>> >  eval pid=\$DB_${DB}_PID
>> >  eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
>> >  eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
>> > @@ -137,6 +140,9 @@ start_ovsdb__() {
>> >  eval addr=\$DB_${DB}_ADDR
>> >  eval active_conf_file=\$ovn${db}_active_conf_file
>> >  eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
>> > +eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
>> > +eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
>> > +eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
>> >
>> >  # Check and eventually start ovsdb-server for DB
>> >  if pidfile_is_running $pid; then
>> > @@ -182,17 +188,32 @@ $cluster_remote_port
>> >
>> >  if test X"$use_remote_in_db" != Xno; then
>> >  set "$@" --remote=db:$schema_name,$table_name,connections
>> > +if test X"$create_insecure_remote" = Xno; then
>> > +set "$@" --remote=pssl:$port:$addr
>> > +elif test X"$create_insecure_remote" = Xyes; then
>> > +set "$@" --remote=ptcp:$port:$addr
>> > +fi
>> Why moving the logic here? This if block only says if the connection
settings in DB table should be used. Whether insecure mode is allowed was
supposed to be independent with this condition. Could you explain the
reason behind the change?
>> >> I moved it because remote=db is needed if ovsdb is running as a
standalone node or an active ovsdb server node. For standby nodes in case
of active_passive mode, remote=db will not be there because it uses
--sync-from. Hope its clear.

As discussed, $use_remote_in_db and $create_insecure_remote were
independent. Moving $create_insecure_remote logic here make it useful only
when $use_remote_in_db is "yes", which is not how it was supposed to be.

I understand that for standby node, we will set $use_remote_in_db as "no".
Is this a problem?

--sync-from has nothing to do with "remote".
>
>
>>
>> >  fi
>> > -set "$@" --private-key=db:$schema_name,SSL,private_key
>> > -set "$@" --certificate=db:$schema_name,SSL,certificate
>> > -set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
>> > -set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
>> > -set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
>>
>> So it will not use the settings in DB any more? It seems this change
removed support for "--use-remote-in-db=yes", which is the default behavior
that should be kept. The DB settings should not be used only if
"--use-remote-in-db=no"
>
> >>  As discussed, this is similar support that we have for ovn-controller
with ssl. If the key is passed from cli, it will use the key else fall back
to default setters for ssl.
>>
I missed the "else" below. So it does fallback to use DB config when
command line option is not specified. It looks good for me, but it would be
better to clarify the behavior in help message of this tool that command
line options for these SSL related parameters overrides any DB setting.

>> >
>> > -if test X"$create_insecure_remote" = Xyes; then
>> > -set "$@" --remote=ptcp:$port:$addr
>> > +if test X"$ovn_db_ssl_key" != X; then
>> > +set "$@" --private-key=$ovn_db_ssl_key
>> > +else
>> > +set "$@" --private-key=db:$schema_name,SSL,private_key
>> > +fi
>> > +if test X"$ovn_db_ssl_cert" != X; then
>> > +set "$@" 

Re: [ovs-dev] dpif-netdev bug regarding packet matches or ufids?

[Ben Pfaff]

Hi Ian (or others), do you have any thoughts about this?

On Fri, Sep 28, 2018 at 04:24:33PM -0700, Ben Pfaff wrote:
> I have been playing with the OFTest framework off and on over the last
> few months and I believe it's caught an actual bug with its
> DirectVlanPackets test.  I'm appending a script that demonstrates it.
> To see it, run "make sandbox" then execute the script inside it.
> 
> At some level at least, the problem is in dpif-netdev, which during the
> test logs messages like this indicating that a datapath flow can't be
> found and therefore can't be modified:
> 
> WARN|dummy@ovs-dummy: failed to put[modify] (No such file or
> directory) ufid:b2fe59af-26bd-4aa0-960e-fd559dc10c54
> 
> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(1),packet_type(ns=0,id=0),eth(src=00:06:07:08:09:0a,dst=00:01:02:03:04:05),eth_type(0x8100),vlan(vid=1000/0x0,pcp=5/0x0),encap(eth_type(0x0800),ipv4(src=127.0.0.1/0.0.0.0,dst=127.0.0.1/0.0.0.0,proto=0/0,tos=0/0,ttl=64/0,frag=no)),
> 
> actions:userspace(pid=0,controller(reason=7,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535))
> 
> It's possible that the caller is screwing up by providing a flow match
> different from one in the datapath flow table.  I have not looked at
> that question.  But the really strange thing here is that the caller is
> providing a ufid and dpif-netdev is ignoring it.  When I make it honor
> the ufid, like the following, then the test suddenly starts working.
> This seems like a bug to me.
> 
> (The following patch is not actually the real fix because the code
> should still look at the key if no ufid was provided.)
> 
> Any thoughts?
> 
> Thanks,
> 
> Ben.
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index e322f5553fa8..4855cc08f7b7 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3273,7 +3273,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
>  
>  static int
>  flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
> -struct netdev_flow_key *key,
> +struct netdev_flow_key *key OVS_UNUSED,
>  struct match *match,
>  ovs_u128 *ufid,
>  const struct dpif_flow_put *put,
> @@ -3287,7 +3287,7 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
>  }
>  
>  ovs_mutex_lock(>flow_mutex);
> -netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
> +netdev_flow = dp_netdev_pmd_find_flow(pmd, ufid, NULL, 0);
>  if (!netdev_flow) {
>  if (put->flags & DPIF_FP_CREATE) {
>  if (cmap_count(>flow_table) < MAX_FLOWS) {
> 
> 
> 
> --8<--cut here-->8--
> 
> #! /bin/sh -ex
> 
> # Create a bridge and add port p1 on port 1, p2 on port 2
> ovs-appctl vlog/set netdev_dummy dpif
> ovs-vsctl --if-exists del-br br0 \
>-- add-br br0 \
>-- add-port br0 p1 -- set interface p1 ofport_request=1 
> options:tx_pcap=p1-tx.pcap options:rxq_pcap=p1-rx.pcap \
>-- add-port br0 p2 -- set interface p2 ofport_request=2 
> options:tx_pcap=p2-tx.pcap options:rxq_pcap=p2-rx.pcap
> 
> # Add a flow that directs some packets received on p1 to p2 and the
> # rest back out p1.
> #
> # Then inject a packet of the form that should go to p2.  Dump the
> # datapath flow to see that it goes to p2 ("actions:2").  Trace the
> # same packet for good measure, to also see that it should go to p2.
> packet=00010203040500060708090a8100a3e808004514000140007ce77f017f01
> ovs-ofctl add-flow br0 
> priority=1,ip,in_port=1,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,actions=output:2
> ovs-ofctl add-flow br0 priority=0,in_port=1,actions=IN_PORT
> ovs-appctl netdev-dummy/receive p1 $packet
> ovs-appctl ofproto/trace br0 in_port=p1 $packet
> ovs-appctl dpif/dump-flows br0
> 
> # Delete the flows, then add new flows that would not match the same
> # packet as before.
> ovs-ofctl del-flows br0
> ovs-ofctl add-flow br0 
> priority=1,in_port=1,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,dl_type=0x0801,actions=output:2
> ovs-ofctl add-flow br0 priority=0,in_port=1,actions=IN_PORT
> 
> # Give the datapath a few seconds to revalidate.
> sleep 2
> 
> # Now inject the same packet again.  It should go to p1 because it
> # only matches the priority-0 flow, as shown by the trace.  However, instead,
> # the original datapath flow is still there and its counters get incremented.
> ovs-appctl netdev-dummy/receive p1 $packet
> ovs-appctl ofproto/trace br0 in_port=p1 $packet
> ovs-appctl dpif/dump-flows br0
> 
> # Give the datapath some time to expire the flow.
> sleep 12
> 
> # Try again.
> ovs-appctl netdev-dummy/receive p1 $packet
> ovs-appctl ofproto/trace br0 in_port=p1 $packet
> ovs-appctl dpif/dump-flows br0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs-ctl: Add new option to use short hostname.

[Ben Pfaff]

On Thu, Oct 04, 2018 at 01:01:09PM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> Current ovs-ctl forces to set full hostname in external-ids. In
> some situation users may want to set short hostname. For example,
> in OpenStack - OVN integration, Neutron uses the host-id provided
> by Nova, which is usually short hostname, to set "requested-chassis"
> in OVN. The mismatch in hypervisor's external-ids:hostname setting
> causes OVN port binding failure. It can be overridden to short name
> but a openvswitch restart using ovs-ctl would again set it to full
> hostname. This patch ensures in such use cases --no-full-hostname
> can be specified to ovs-ctl to set short hostname instead.
> 
> Signed-off-by: Han Zhou 
> ---
> v1->v2: fix v1, thanks Aaron Conole  for the finding.

Applied to master, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb-idl.c: Increase seqno for change-tracking of table references.

[Ben Pfaff]

On Mon, Oct 08, 2018 at 10:23:55AM -0700, Han Zhou wrote:
> On Mon, Oct 8, 2018 at 8:42 AM Ben Pfaff  wrote:
> >
> > On Fri, Oct 05, 2018 at 12:14:23PM -0700, Han Zhou wrote:
> > > From: Han Zhou 
> > >
> > > This is an enhancement for commit:
> > > 102781c ovsdb-idl: Track changes for table references.
> > >
> > > The seqno change is needed so that the change-tracking helper function
> > > ..._is_new() can work properly.
> > >
> > > Signed-off-by: Han Zhou 
> >
> > Does this fix a bug?  It sounds like it, but the commit message
> > explicitly calls it an "enhancement".
> >
> > Thanks,
> >
> > Ben.
> 
> Without this change, the feature is incomplete, while we assumed it
> complete. So it is exactly a bug fix.
> s/enhancement/fix

Thanks for clarifying.  I updated the commit message and applied this to
master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dp-packet.h: move funcs to be within cond block

[Stokes, Ian]

> For what it's worth, Ian, I'm hoping that you'll take a look at this and
> decide whether to merge it.

Sure, thanks for flagging Ben, apologies for the delay, I've been on vacation 
the past 2 weeks but will have time to look at this now.

Thanks
Ian
> 
> On Tue, Oct 02, 2018 at 10:32:40AM +0100, Lam, Tiago wrote:
> > (Sending this again as yesterday's email seems to have been dropped.)
> >
> > Hi Flavio,
> >
> > Thanks for the patch. Looks good to me, it compiles with both DPDK and
> > without DPDK linked, and the check-system-userspace tests pass.
> >
> > Acked-by: Tiago Lam 
> >
> > On 25/09/2018 22:08, Flavio Leitner wrote:
> > > There is already an ifdef DPDK_NETDEV block, so instead of checking
> > > on each and every function, move them to the right block.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Flavio Leitner 
> > > ---
> > >  lib/dp-packet.h | 260
> > > +++-
> > >  1 file changed, 146 insertions(+), 114 deletions(-)
> > >
> > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h index
> > > ba91e5891..5b4c9c7a3 100644
> > > --- a/lib/dp-packet.h
> > > +++ b/lib/dp-packet.h
> > > @@ -465,113 +465,142 @@ dp_packet_set_allocated(struct dp_packet *b,
> > > uint16_t s)  {
> > >  b->mbuf.buf_len = s;
> > >  }
> > > -#else
> > > -static inline void *
> > > -dp_packet_base(const struct dp_packet *b)
> > > +
> > > +/* Returns the RSS hash of the packet 'p'.  Note that the returned
> > > +value is
> > > + * correct only if 'dp_packet_rss_valid(p)' returns true */ static
> > > +inline uint32_t dp_packet_get_rss_hash(struct dp_packet *p)
> > >  {
> > > -return b->base_;
> > > +return p->mbuf.hash.rss;
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_set_base(struct dp_packet *b, void *d)
> > > +dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
> > >  {
> > > -b->base_ = d;
> > > +p->mbuf.hash.rss = hash;
> > > +p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> > >  }
> > >
> > > -static inline uint32_t
> > > -dp_packet_size(const struct dp_packet *b)
> > > +static inline bool
> > > +dp_packet_rss_valid(struct dp_packet *p)
> > >  {
> > > -return b->size_;
> > > +return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_set_size(struct dp_packet *b, uint32_t v)
> > > +dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
> > >  {
> > > -b->size_ = v;
> > >  }
> > >
> > > -static inline uint16_t
> > > -__packet_data(const struct dp_packet *b)
> > > +static inline void
> > > +dp_packet_mbuf_rss_flag_reset(struct dp_packet *p)
> > >  {
> > > -return b->data_ofs;
> > > +p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> > >  }
> > >
> > > +/* This initialization is needed for packets that do not come
> > > + * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> > > + * The DPDK rte library will still otherwise manage the mbuf.
> > > + * We only need to initialize the mbuf ol_flags. */
> > >  static inline void
> > > -__packet_set_data(struct dp_packet *b, uint16_t v)
> > > +dp_packet_mbuf_init(struct dp_packet *p)
> > >  {
> > > -b->data_ofs = v;
> > > +p->mbuf.ol_flags = 0;
> > >  }
> > >
> > > -static inline uint16_t
> > > -dp_packet_get_allocated(const struct dp_packet *b)
> > > +static inline bool
> > > +dp_packet_ip_checksum_valid(struct dp_packet *p)
> > >  {
> > > -return b->allocated_;
> > > +return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > > +PKT_RX_IP_CKSUM_GOOD;
> > >  }
> > >
> > > -static inline void
> > > -dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> > > +static inline bool
> > > +dp_packet_ip_checksum_bad(struct dp_packet *p)
> > >  {
> > > -b->allocated_ = s;
> > > +return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > > +PKT_RX_IP_CKSUM_BAD;
> > > +}
> > > +
> > > +static inline bool
> > > +dp_packet_l4_checksum_valid(struct dp_packet *p) {
> > > +return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > > +PKT_RX_L4_CKSUM_GOOD;
> > > +}
> > > +
> > > +static inline bool
> > > +dp_packet_l4_checksum_bad(struct dp_packet *p) {
> > > +return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > > +PKT_RX_L4_CKSUM_BAD;
> > >  }
> > > -#endif
> > >
> > >  static inline void
> > > -dp_packet_reset_cutlen(struct dp_packet *b)
> > > +reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
> > >  {
> > > -b->cutlen = 0;
> > > +p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD
> |
> > > +  PKT_RX_IP_CKSUM_GOOD |
> > > + PKT_RX_IP_CKSUM_BAD);
> > >  }
> > >
> > > -static inline uint32_t
> > > -dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> > > +static inline bool
> > > +dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
> > >  {
> > > -if (max_len < ETH_HEADER_LEN) {
> > > -max_len = ETH_HEADER_LEN;
> > > +if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> > > +   

Re: [ovs-dev] [PATCH v3] extend-table: Fix a bug that iterates wrong table

[Ben Pfaff]

On Fri, Oct 05, 2018 at 03:16:50PM -0700, Yifeng Sun wrote:
> This seems to be a copy and paste bug that iterates and frees
> the wrong table. This commit fixes that.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10730
> Co-authored-by: Ben Pfaff 
> Signed-off-by: Yifeng Sun 
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2: Fix email subject by adding [ovs-dev]
> v2->v3: Remove duplicated code by Ben.

Thanks!  Applied to master, backported to 2.10.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb-idl.c: Increase seqno for change-tracking of table references.

[Han Zhou]

On Mon, Oct 8, 2018 at 8:42 AM Ben Pfaff  wrote:
>
> On Fri, Oct 05, 2018 at 12:14:23PM -0700, Han Zhou wrote:
> > From: Han Zhou 
> >
> > This is an enhancement for commit:
> > 102781c ovsdb-idl: Track changes for table references.
> >
> > The seqno change is needed so that the change-tracking helper function
> > ..._is_new() can work properly.
> >
> > Signed-off-by: Han Zhou 
>
> Does this fix a bug?  It sounds like it, but the commit message
> explicitly calls it an "enhancement".
>
> Thanks,
>
> Ben.

Without this change, the feature is incomplete, while we assumed it
complete. So it is exactly a bug fix.
s/enhancement/fix

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

Re: [ovs-dev] [PATCH] odp-util: Fix a use-afer-free bug

[Ben Pfaff]

Thanks, applied to master, backported as far as 2.7.

On Fri, Oct 05, 2018 at 03:43:03PM -0700, Yifeng Sun wrote:
> This patch should also fix the bug reported at
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10802
> 
> On Fri, Oct 5, 2018 at 2:50 PM Yifeng Sun  wrote:
> 
> > After ofpbug_put, actions may have been reallocated and
> > key will point to invalid memory address.
> >
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10796
> > Signed-off-by
> > :
> > Yifeng Sun 
> > ---
> >  lib/odp-util.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index 890c71b7f336..7705bb30ae21 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -2242,13 +2242,14 @@ parse_odp_action(const char *s, const struct simap
> > *port_names,
> >  key->nla_len += size;
> >  ofpbuf_put(actions, mask + 1, size);
> >
> > -/* Add new padding as needed */
> > -ofpbuf_put_zeros(actions, NLA_ALIGN(key->nla_len) -
> > -  key->nla_len);
> > -
> >  /* 'actions' may have been reallocated by ofpbuf_put(). */
> >  nested = ofpbuf_at_assert(actions, start_ofs, sizeof
> > *nested);
> >  nested->nla_type = OVS_ACTION_ATTR_SET_MASKED;
> > +
> > +key = nested + 1;
> > +/* Add new padding as needed */
> > +ofpbuf_put_zeros(actions, NLA_ALIGN(key->nla_len) -
> > +  key->nla_len);
> >  }
> >  }
> >  ofpbuf_uninit();
> > --
> > 2.7.4
> >
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofp-packet: Fix NXT_RESUME with geneve tunnel metadata

[Ben Pfaff]

On Fri, Oct 05, 2018 at 09:19:54AM -0700, Yi-Hung Wei wrote:
> The patch address vswitchd crash when it receives NXT_RESUME with geneve
> tunnel metadata.  The crash is due to segmentation fault with the
> following stack trace, and it is observed only in kernel datapath.
> A test is added to prevent regression.
> 
> Thread 1 "ovs-vswitchd" received signal SIGSEGV, Segmentation fault.
> 0  0x7fcffd0c5412 in tun_metadata_to_geneve__ 
> (flow=flow@entry=0x7ffcb7106680, b=b@entry=0x7ffcb70eb5a8, 
> crit_opt=crit_opt@entry=0x7ffcb70eb287)
>at lib/tun-metadata.c:676
> 1  0x7fcffd0c6858 in tun_metadata_to_geneve_nlattr_flow 
> (b=0x7ffcb70eb5a8, flow=0x7ffcb7106638) at lib/tun-metadata.c:706
> 2  tun_metadata_to_geneve_nlattr (tun=tun@entry=0x7ffcb7106638, 
> flow=flow@entry=0x7ffcb7106638, key=key@entry=0x0, b=b@entry=0x7ffcb70eb5a8)
>at lib/tun-metadata.c:810
> 3  0x7fcffd048464 in tun_key_to_attr (a=a@entry=0x7ffcb70eb5a8, 
> tun_key=tun_key@entry=0x7ffcb7106638, 
> tun_flow_key=tun_flow_key@entry=0x7ffcb7106638,
>key_buf=key_buf@entry=0x0, tnl_type=, tnl_type@entry=0x0) 
> at lib/odp-util.c:2886
> 4  0x7fcffd0551cf in odp_key_from_dp_packet 
> (buf=buf@entry=0x7ffcb70eb5a8, packet=0x7ffcb7106590) at lib/odp-util.c:5909
> 5  0x7fcffd0d7870 in dpif_netlink_encode_execute (buf=0x7ffcb70eb5a8, 
> d_exec=0x7ffcb7106428, dp_ifindex=) at lib/dpif-netlink.c:1873
> 6  dpif_netlink_operate__ (dpif=dpif@entry=0xe65e00, 
> ops=ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif-netlink.c:1959
> 7  0x7fcffd0d842e in dpif_netlink_operate_chunks (n_ops=1, 
> ops=0x7ffcb7106418, dpif=) at lib/dpif-netlink.c:2258
> 8  dpif_netlink_operate (dpif_=0xe65e00, ops=, 
> n_ops=) at lib/dpif-netlink.c:2294
> 9  0x7fcffd014680 in dpif_operate (dpif=, ops= out>, ops@entry=0x7ffcb7106418, n_ops=n_ops@entry=1) at lib/dpif.c:1359
> 10 0x7fcffd014c58 in dpif_execute (dpif=, 
> execute=execute@entry=0x7ffcb71064e0) at lib/dpif.c:1324
> 11 0x7fcffd40d3e6 in nxt_resume (ofproto_=0xe6af50, pin=0x7ffcb7107150) 
> at ofproto/ofproto-dpif.c:4885
> 12 0x7fcffd3f88c3 in handle_nxt_resume (ofconn=ofconn@entry=0xf8c8f0, 
> oh=oh@entry=0xf7ebd0) at ofproto/ofproto.c:3612
> 13 0x7fcffd404a3b in handle_openflow__ (msg=0xeac460, ofconn=0xf8c8f0) at 
> ofproto/ofproto.c:8137
> 14 handle_openflow (ofconn=0xf8c8f0, ofp_msg=0xeac460) at 
> ofproto/ofproto.c:8258
> 15 0x7fcffd3f4653 in ofconn_run (handle_openflow=0x7fcffd4046f0 
> , ofconn=0xf8c8f0) at ofproto/connmgr.c:1432
> 16 connmgr_run (mgr=0xe422f0, 
> handle_openflow=handle_openflow@entry=0x7fcffd4046f0 ) at 
> ofproto/connmgr.c:363
> 17 0x7fcffd3fdc76 in ofproto_run (p=0xe6af50) at ofproto/ofproto.c:1821
> 18 0x0040ca94 in bridge_run__ () at vswitchd/bridge.c:2939
> 19 0x00411d44 in bridge_run () at vswitchd/bridge.c:2997
> 20 0x004094fd in main (argc=12, argv=0x7ffcb71085b8) at 
> vswitchd/ovs-vswitchd.c:119
> 
> VMWare-BZ: #2210216
> Signed-off-by: Yi-Hung Wei 
> ---
> Please backport it to 2.10.

Applied to master and branch-2.10, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] OVN: add buffering support for ip packets

[Ben Pfaff]

On Fri, Oct 05, 2018 at 06:57:24PM +0200, Lorenzo Bianconi wrote:
> Add buffering support for IPv4/IPv6 packets that will be processed
> by arp{}/nd_ns{} action when L2 address is not discovered yet since
> otherwise the packet will be substituted with an ARP/Neighbor
> Solicitation frame and this will result in the lost of the first
> packet of the connection.
> Moreover fix following automatic tests broken by ip-buffering support
> since now original ip packets are transmitted by OVN logical
> router:
> - ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR
> - ovn -- /32 router IP address
> 
> Signed-off-by: Lorenzo Bianconi 

Thanks, applied to master.
___
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

[Ben Pfaff]

On Fri, Oct 05, 2018 at 10:58:35AM +0100, Markos Chandras wrote:
> 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.

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

Re: [ovs-dev] [PATCH v10 00/14] Support multi-segment mbufs

[Ilya Maximets]

Current patch-set breaks a lot of STP unit tests:

## --- ##
## openvswitch 2.10.90 test suite. ##
## --- ##

ofproto-dpif

1149: ofproto-dpif - MPLS handlingFAILED 
(ofproto-dpif.at:2068)
1164: ofproto-dpif - VLAN+MPLS handling   FAILED 
(ofproto-dpif.at:3921)

Spanning Tree Protocol unit tests

2534: STP example from IEEE 802.1D-1998   FAILED (stp.at:18)
2535: STP example from IEEE 802.1D-2004 figures 17.4 and 17.5 FAILED (stp.at:61)
2536: STP example from IEEE 802.1D-2004 figure 17.6   FAILED (stp.at:87)
2537: STP example from IEEE 802.1D-2004 figure 17.7   FAILED (stp.at:116)
2538: STP.io.1.1: Link FailureFAILED (stp.at:155)
2539: STP.io.1.2: Repeated NetworkFAILED (stp.at:181)
2540: STP.io.1.4: Network Initialization  FAILED (stp.at:205)
2541: STP.io.1.5: Topology Change FAILED (stp.at:258)
2545: STP.op.3.3: Root Bridge Selection: Bridge ID Values FAILED (stp.at:337)
2546: STP.op.3.3: Root Bridge Selection: Bridge ID Values FAILED (stp.at:360)

## - ##
## Test results. ##
## - ##

ERROR: All 12 tests were run,
12 failed unexpectedly.
## -- ##
## testsuite.log was created. ##
## -- ##

Please send `tests/testsuite.log' and all information you think might help:

   To: 
   Subject: [openvswitch 2.10.90] testsuite: 1149 1164 2534 2535 2536 2537 2538 
2539 2540 2541 2545 2546 failed

--

Meanwhile, I still have a few concerns about this patch-set
in general:

1. Most of dp_packet APIs are now able to fail.
   But callers doesn't check results in most cases.

2. Many drivers has its own restrictions on the number of segments
   they able to handle in a single packet. For example, igb driver
   supports only up to 8 segments. I think, that we have to call
   tx preparation API and check the result before sending segmented
   packets. It's unclear, what to do with such packets. Will we
   have to have large sized mempool anyway for this case?

2.1 OVS will migrate to DPDK 18.11 as soon as it will be released,
but there are few PMDs that doesn't support segmented packets
at all (octeontx and axgbe, for example). Maybe it'll be better
to work on this patch set on top of ovs/dpdk-latest branch and
include all the required checks for DEV_TX_OFFLOAD_MULTI_SEGS
flag in order to not block upgrade to 18.11 ?

One code specific thing (I did not review the whole code. This just
one thing that I noticed accidentally.):

3. dp_packet_equal should not fail in case of different memory
   layout, but equal data.


Best regards, Ilya Maximets.

On 28.09.2018 19:15, Tiago Lam wrote:
> Overview
> 
> This patchset introduces support for multi-segment mbufs to OvS-DPDK.
> Multi-segment mbufs are typically used when the size of an mbuf is
> insufficient to contain the entirety of a packet's data. Instead, the
> data is split across numerous mbufs, each carrying a portion, or
> 'segment', of the packet data. Mbufs are chained via their 'next'
> attribute (an mbuf pointer).
> 
> The main motivation behind the support for multi-segment mbufs is to
> later introduce TSO (use case i. below) / GRO in OvS-DPDK, which is
> planned to be introduced after this series.
> 
> Use Cases
> =
> i.  Handling oversized (guest-originated) frames, which are marked
> for hardware accelration/offload (TSO, for example).
> 
> Packets which originate from a non-DPDK source may be marked for
> offload; as such, they may be larger than the permitted ingress
> interface's MTU, and may be stored in an oversized dp-packet. In
> order to transmit such packets over a DPDK port, their contents
> must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
> its current implementation, that function only copies data into
> a single mbuf; if the space available in the mbuf is exhausted,
> but not all packet data has been copied, then it is lost.
> Similarly, when cloning a DPDK mbuf, it must be considered
> whether that mbuf contains multiple segments. Both issues are
> resolved within this patchset.
> 
> ii. Handling jumbo frames.
> 
> While OvS already supports jumbo frames, it does so by increasing
> mbuf size, such that the entirety of a jumbo frame may be handled
> in a single mbuf. This is certainly the preferred, and most
> performant approach (and remains the default).
> 
> Enabling multi-segment mbufs
> 
> Multi-segment and single-segment mbufs are mutually exclusive, and the
> user must decide on which approach to adopt on init. The introduction
> of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
> 
> This is a global boolean value, which determines how jumbo frames are
> represented across all 

[ovs-dev] [PATCH] ovs-save: Parse geneve tlv map correctly.

[Gurucharan Shetty]

We now have an extra space in the o/p of `ovs-ofctl dump-tlv-map`.

Fixes: 5a0e4aec1af (treewide: Convert leading tabs to spaces.)
Signed-off-by: Gurucharan Shetty 
---
 utilities/ovs-save | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utilities/ovs-save b/utilities/ovs-save
index 5c046bb..f85d97a 100755
--- a/utilities/ovs-save
+++ b/utilities/ovs-save
@@ -117,7 +117,7 @@ save_flows () {
 
 printf "%s" "ovs-ofctl add-tlv-map ${bridge} '"
 ovs-ofctl dump-tlv-map ${bridge} | \
-awk '/^ 0x/ {if (cnt != 0) printf ","; \
+awk '/^  0x/ {if (cnt != 0) printf ","; \
  cnt++;printf "{class="$1",type="$2",len="$3"}->"$4}'
 echo "'"
 
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] ovsdb-idl.c: Increase seqno for change-tracking of table references.

[Ben Pfaff]

On Fri, Oct 05, 2018 at 12:14:23PM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> This is an enhancement for commit:
> 102781c ovsdb-idl: Track changes for table references.
> 
> The seqno change is needed so that the change-tracking helper function
> ..._is_new() can work properly.
> 
> Signed-off-by: Han Zhou 

Does this fix a bug?  It sounds like it, but the commit message
explicitly calls it an "enhancement".

Thanks,

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

Re: [ovs-dev] [PATCH] Add basic PG commands.

[Ben Pfaff]

On Fri, Oct 05, 2018 at 04:05:22PM -0400, Mark Michelson wrote:
> Thanks for the feedback Han. Interestingly, when I move the tests from
> ovn.at to ovn-nbctl.at, the tests no longer pass. When running in non-daemon
> mode, for whatever reason, trying to get the port group's ports from the
> database results in an empty return. I can't reproduce this in the sandbox,
> so I'll need to dive in and see what's going wrong.
> 
> When run in daemon mode, interestingly, it appears that there are a couple
> of legitimate bugs I'm running into. First, there's an extra blank line at
> the beginning before the output of the database output. Second, it appears
> the --bare flag is ignored. I can reproduce both of these in the sandbox.
> 
> So I think this is going to result in another patch from me to fix the
> daemon mode bugs mentioned above.

I'm going to assume that you'll post a new version of this after the bug
fix patches get in.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] table: Create method for resetting table formatting.

[Ben Pfaff]

On Fri, Oct 05, 2018 at 06:40:33PM -0400, Mark Michelson wrote:
> Table formatting has a local static integer that is intended to insert
> line breaks between tables. This works exactly as intended, as long as
> each call to table_format() is done as a single unit within the run of a
> process.
> 
> When ovn-nbctl is run in daemon mode, it is a long-running process that
> makes multiple calls to table_format() throughout its lifetime. After
> the first call, this results in an unexpected newline prepended to table
> output on each subsequent ovn-nbctl invocation.
> 
> The solution is to introduce a function to reset table formatting. This
> way, the first time after resetting table formatting, no newline is
> prepended.
> 
> Signed-off-by: Mark Michelson 

This seems like it could use a test, too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] ovn-nbctl: Don't parse table-formatting options in nbctl_client

[Ben Pfaff]

On Fri, Oct 05, 2018 at 06:40:34PM -0400, Mark Michelson wrote:
> When ovn-nbctl is running in daemon mode, nbctl_client attempts to parse
> table formatting options. The problem is that this then removes the table
> formatting options from the array of options passed to the server loop. The
> server loop resets the table formatting options to the defaults and then
> attempts again to parse table formatting options. Unfortunately, they aren't
> present any longer. The result is that tables are always formatted with
> the default style.
> 
> This patch solves the issue by not parsing the table formatting options
> in nbctl_client. Instead, the table formatting options are passed to the
> server loop and parsed there instead.
> 
> Signed-off-by: Mark Michelson 

This is a pretty subtle bug.  Would you mind adding a test to prevent
future regression?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink: fix null pointer

[Ben Pfaff]

On Mon, Oct 08, 2018 at 03:15:44PM +, Matteo Croce wrote:
> On Mon, Oct 8, 2018 at 3:11 PM Guru Shetty  wrote:
> >
> >
> >
> > On Sat, 6 Oct 2018 at 09:20, Matteo Croce  wrote:
> >>
> >> In dpif_netlink_port_add__(), socksp could be NULL, because
> >> vport_socksp_to_pids() would allocate a new array and return a single
> >> zero element.
> >> Following vport_socksp_to_pids() removal, a NULL pointer can happen when
> >> dpif_netlink_port_add__() is called and dpif->handlers is 0.
> >>
> >> Restore the old behaviour of using a zero pid when dpif->handlers is 0.
> >>
> >> Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink 
> >> sockets")
> >> Reported-by: Flavio Leitner 
> >> Reported-by: Guru Shetty 
> >> Signed-off-by: Matteo Croce 
> >> ---
> >
> >
> > Not a review of the code. But I can confirm that the patch does fix the 
> > segmentation fault that I was facing.
> >
> >>
> >>  lib/dpif-netlink.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> >> index 21315033c..310bc947d 100644
> >> --- a/lib/dpif-netlink.c
> >> +++ b/lib/dpif-netlink.c
> >> @@ -712,7 +712,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, 
> >> const char *name,
> >>  struct dpif_netlink_vport request, reply;
> >>  struct ofpbuf *buf;
> >>  struct nl_sock *socksp = NULL;
> >> -uint32_t upcall_pids;
> >> +uint32_t upcall_pids = 0;
> >>  int error = 0;
> >>
> >>  if (dpif->handlers) {
> >> @@ -728,7 +728,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, 
> >> const char *name,
> >>  request.name = name;
> >>
> >>  request.port_no = *port_nop;
> >> -upcall_pids = nl_sock_pid(socksp);
> >> +if (socksp)
> >> +upcall_pids = nl_sock_pid(socksp);
> >>  request.n_upcall_pids = 1;
> >>  request.upcall_pids = _pids;
> >>
> >> --
> >> 2.17.1
> >>
> 
> Ok thanks. I'me sending a v2 with the checkpatch.py warning found by
> Aaron's bot fixed

No need, I fixed it myself and applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] dpif-netlink: fix null pointer

[Matteo Croce]

In dpif_netlink_port_add__(), socksp could be NULL, because
vport_socksp_to_pids() would allocate a new array and return a single
zero element.
Following vport_socksp_to_pids() removal, a NULL pointer can happen when
dpif_netlink_port_add__() is called and dpif->handlers is 0.

Restore the old behaviour of using a zero pid when dpif->handlers is 0.

Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink sockets")
Reported-by: Flavio Leitner 
Reported-by: Guru Shetty 
Tested-by: Guru Shetty 
Signed-off-by: Matteo Croce 
---
v2: fix checkpatch.py error about coding style

 lib/dpif-netlink.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 21315033c..ac3d2edeb 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -712,7 +712,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
char *name,
 struct dpif_netlink_vport request, reply;
 struct ofpbuf *buf;
 struct nl_sock *socksp = NULL;
-uint32_t upcall_pids;
+uint32_t upcall_pids = 0;
 int error = 0;
 
 if (dpif->handlers) {
@@ -728,7 +728,9 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
char *name,
 request.name = name;
 
 request.port_no = *port_nop;
-upcall_pids = nl_sock_pid(socksp);
+if (socksp) {
+upcall_pids = nl_sock_pid(socksp);
+}
 request.n_upcall_pids = 1;
 request.upcall_pids = _pids;
 
-- 
2.17.1

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

Re: [ovs-dev] [PATCH] dpif-netlink: fix null pointer

[Matteo Croce]

On Mon, Oct 8, 2018 at 3:11 PM Guru Shetty  wrote:
>
>
>
> On Sat, 6 Oct 2018 at 09:20, Matteo Croce  wrote:
>>
>> In dpif_netlink_port_add__(), socksp could be NULL, because
>> vport_socksp_to_pids() would allocate a new array and return a single
>> zero element.
>> Following vport_socksp_to_pids() removal, a NULL pointer can happen when
>> dpif_netlink_port_add__() is called and dpif->handlers is 0.
>>
>> Restore the old behaviour of using a zero pid when dpif->handlers is 0.
>>
>> Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink sockets")
>> Reported-by: Flavio Leitner 
>> Reported-by: Guru Shetty 
>> Signed-off-by: Matteo Croce 
>> ---
>
>
> Not a review of the code. But I can confirm that the patch does fix the 
> segmentation fault that I was facing.
>
>>
>>  lib/dpif-netlink.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>> index 21315033c..310bc947d 100644
>> --- a/lib/dpif-netlink.c
>> +++ b/lib/dpif-netlink.c
>> @@ -712,7 +712,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
>> char *name,
>>  struct dpif_netlink_vport request, reply;
>>  struct ofpbuf *buf;
>>  struct nl_sock *socksp = NULL;
>> -uint32_t upcall_pids;
>> +uint32_t upcall_pids = 0;
>>  int error = 0;
>>
>>  if (dpif->handlers) {
>> @@ -728,7 +728,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const 
>> char *name,
>>  request.name = name;
>>
>>  request.port_no = *port_nop;
>> -upcall_pids = nl_sock_pid(socksp);
>> +if (socksp)
>> +upcall_pids = nl_sock_pid(socksp);
>>  request.n_upcall_pids = 1;
>>  request.upcall_pids = _pids;
>>
>> --
>> 2.17.1
>>

Ok thanks. I'me sending a v2 with the checkpatch.py warning found by
Aaron's bot fixed

-- 
Matteo Croce
per aspera ad upstream
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink: fix null pointer

[Guru Shetty]

On Sat, 6 Oct 2018 at 09:20, Matteo Croce  wrote:

> In dpif_netlink_port_add__(), socksp could be NULL, because
> vport_socksp_to_pids() would allocate a new array and return a single
> zero element.
> Following vport_socksp_to_pids() removal, a NULL pointer can happen when
> dpif_netlink_port_add__() is called and dpif->handlers is 0.
>
> Restore the old behaviour of using a zero pid when dpif->handlers is 0.
>
> Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink
> sockets")
> Reported-by: Flavio Leitner 
> Reported-by: Guru Shetty 
> Signed-off-by: Matteo Croce 
> ---
>

Not a review of the code. But I can confirm that the patch does fix the
segmentation fault that I was facing.


>  lib/dpif-netlink.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 21315033c..310bc947d 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -712,7 +712,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif,
> const char *name,
>  struct dpif_netlink_vport request, reply;
>  struct ofpbuf *buf;
>  struct nl_sock *socksp = NULL;
> -uint32_t upcall_pids;
> +uint32_t upcall_pids = 0;
>  int error = 0;
>
>  if (dpif->handlers) {
> @@ -728,7 +728,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif,
> const char *name,
>  request.name = name;
>
>  request.port_no = *port_nop;
> -upcall_pids = nl_sock_pid(socksp);
> +if (socksp)
> +upcall_pids = nl_sock_pid(socksp);
>  request.n_upcall_pids = 1;
>  request.upcall_pids = _pids;
>
> --
> 2.17.1
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] ovn-nbctl: Don't parse table-formatting options in nbctl_client

[Numan Siddique]

On Sat, Oct 6, 2018 at 4:11 AM Mark Michelson  wrote:

> When ovn-nbctl is running in daemon mode, nbctl_client attempts to parse
> table formatting options. The problem is that this then removes the table
> formatting options from the array of options passed to the server loop. The
> server loop resets the table formatting options to the defaults and then
> attempts again to parse table formatting options. Unfortunately, they
> aren't
> present any longer. The result is that tables are always formatted with
> the default style.
>
> This patch solves the issue by not parsing the table formatting options
> in nbctl_client. Instead, the table formatting options are passed to the
> server loop and parsed there instead.
>
> Signed-off-by: Mark Michelson 
>

Acked-by: Numan Siddique 



> ---
>  ovn/utilities/ovn-nbctl.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index d65a9ba08..bff6d1380 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -5443,7 +5443,6 @@ nbctl_client(const char *socket_name,
>  break;
>
>  VLOG_OPTION_HANDLERS
> -TABLE_OPTION_HANDLERS(_style)
>
>  case OPT_LOCAL:
>  default:
> --
> 2.14.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] table: Create method for resetting table formatting.

[Numan Siddique]

On Sat, Oct 6, 2018 at 4:11 AM Mark Michelson  wrote:

> Table formatting has a local static integer that is intended to insert
> line breaks between tables. This works exactly as intended, as long as
> each call to table_format() is done as a single unit within the run of a
> process.
>
> When ovn-nbctl is run in daemon mode, it is a long-running process that
> makes multiple calls to table_format() throughout its lifetime. After
> the first call, this results in an unexpected newline prepended to table
> output on each subsequent ovn-nbctl invocation.
>
> The solution is to introduce a function to reset table formatting. This
> way, the first time after resetting table formatting, no newline is
> prepended.
>
> Signed-off-by: Mark Michelson 
>

Acked-by: Numan Siddique 


> ---
>  lib/table.c   | 23 +--
>  lib/table.h   |  1 +
>  ovn/utilities/ovn-nbctl.c |  1 +
>  3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/lib/table.c b/lib/table.c
> index ab72668c7..48d18b651 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -236,15 +236,18 @@ table_print_timestamp__(const struct table *table,
> struct ds *s)
>  }
>  }
>
> +static bool first_table = true;
> +
>  static void
>  table_print_table__(const struct table *table, const struct table_style
> *style,
>  struct ds *s)
>  {
> -static int n = 0;
>  int *widths;
>  size_t x, y;
>
> -if (n++ > 0) {
> +if (first_table) {
> +first_table = false;
> +} else {
>  ds_put_char(s, '\n');
>  }
>
> @@ -318,10 +321,11 @@ static void
>  table_print_list__(const struct table *table, const struct table_style
> *style,
> struct ds *s)
>  {
> -static int n = 0;
>  size_t x, y;
>
> -if (n++ > 0) {
> +if (first_table) {
> +first_table = false;
> +} else {
>  ds_put_char(s, '\n');
>  }
>
> @@ -469,10 +473,11 @@ static void
>  table_print_csv__(const struct table *table, const struct table_style
> *style,
>struct ds *s)
>  {
> -static int n = 0;
>  size_t x, y;
>
> -if (n++ > 0) {
> +if (first_table) {
> +first_table = false;
> +} else {
>  ds_put_char(s, '\n');
>  }
>
> @@ -614,6 +619,12 @@ table_format(const struct table *table, const struct
> table_style *style,
>  }
>  }
>
> +void
> +table_format_reset(void)
> +{
> +first_table = true;
> +}
> +
>  /* Outputs 'table' on stdout in the specified 'style'. */
>  void
>  table_print(const struct table *table, const struct table_style *style)
> diff --git a/lib/table.h b/lib/table.h
> index 76e65bb70..33263e2a2 100644
> --- a/lib/table.h
> +++ b/lib/table.h
> @@ -133,6 +133,7 @@ void table_parse_cell_format(struct table_style *,
> const char *format);
>  void table_print(const struct table *, const struct table_style *);
>  void table_format(const struct table *, const struct table_style *,
>struct ds *);
> +void table_format_reset(void);
>  void table_usage(void);
>
>  #endif /* table.h */
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index eabd30308..d65a9ba08 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -5311,6 +5311,7 @@ server_cmd_run(struct unixctl_conn *conn, int argc,
> const char **argv_,
>  }
>
>  struct ds output = DS_EMPTY_INITIALIZER;
> +table_format_reset();
>  for (struct ctl_command *c = commands; c < [n_commands];
> c++) {
>  if (c->table) {
>  table_format(c->table, _style, );
> --
> 2.14.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ossfuzz: Speed up flow extract fuzzing by

[bshastry]

From: Bhargava Shastry 

Accepts fixes suggested by 0-day robot.

Refactor miniflow tests out of flow_extract_target.c into
 a  new target called miniflow_target.c

The biggest motivation for this massive (7-10x) increase in fuzzing
speed. Prior to the refactoring, we were doing roughly 900 executions
per second on flow_extract_target. Now, we are doing roughly 6000
executions per second on the flow_extract_target and roughly 9000
executions per second on the new miniflow_target.

Moving forward, creating micro fuzz targets that are really fast is a
better strategy. Since all these micro targets can be scheduled in
parallel by oss-fuzz, the test throughput increases by a non-trivial
amount.

Signed-off-by: Bhargava Shastry 
---
 tests/oss-fuzz/automake.mk|  12 +-
 tests/oss-fuzz/config/miniflow_target.options |   3 +
 tests/oss-fuzz/flow_extract_target.c  | 367 ++
 tests/oss-fuzz/miniflow_target.c  | 366 +
 4 files changed, 410 insertions(+), 338 deletions(-)
 create mode 100644 tests/oss-fuzz/config/miniflow_target.options
 create mode 100644 tests/oss-fuzz/miniflow_target.c

diff --git a/tests/oss-fuzz/automake.mk b/tests/oss-fuzz/automake.mk
index 4fbdb4c2b..2c506be3d 100644
--- a/tests/oss-fuzz/automake.mk
+++ b/tests/oss-fuzz/automake.mk
@@ -3,7 +3,8 @@ OSS_FUZZ_TARGETS = \
tests/oss-fuzz/json_parser_target \
tests/oss-fuzz/ofp_print_target \
tests/oss-fuzz/expr_parse_target \
-   tests/oss-fuzz/odp_target
+   tests/oss-fuzz/odp_target \
+   tests/oss-fuzz/miniflow_target
 EXTRA_PROGRAMS += $(OSS_FUZZ_TARGETS)
 oss-fuzz-targets: $(OSS_FUZZ_TARGETS)
 
@@ -38,12 +39,19 @@ tests_oss_fuzz_odp_target_SOURCES = \
 tests_oss_fuzz_odp_target_LDADD = lib/libopenvswitch.la
 tests_oss_fuzz_odp_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++
 
+tests_oss_fuzz_miniflow_target_SOURCES = \
+tests/oss-fuzz/miniflow_target.c \
+tests/oss-fuzz/fuzzer.h
+tests_oss_fuzz_miniflow_target_LDADD = lib/libopenvswitch.la
+tests_oss_fuzz_miniflow_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++
+
 EXTRA_DIST += \
tests/oss-fuzz/config/flow_extract_target.options \
tests/oss-fuzz/config/json_parser_target.options \
tests/oss-fuzz/config/ofp_print_target.options \
tests/oss-fuzz/config/expr_parse_target.options \
tests/oss-fuzz/config/odp_target.options \
+   tests/oss-fuzz/config/miniflow_target.options \
tests/oss-fuzz/config/ovs.dict \
tests/oss-fuzz/config/expr.dict \
-   tests/oss-fuzz/config/odp.dict
+   tests/oss-fuzz/config/odp.dict
\ No newline at end of file
diff --git a/tests/oss-fuzz/config/miniflow_target.options 
b/tests/oss-fuzz/config/miniflow_target.options
new file mode 100644
index 0..4849f282d
--- /dev/null
+++ b/tests/oss-fuzz/config/miniflow_target.options
@@ -0,0 +1,3 @@
+[libfuzzer]
+dict = ovs.dict
+close_fd_mask = 3
\ No newline at end of file
diff --git a/tests/oss-fuzz/flow_extract_target.c 
b/tests/oss-fuzz/flow_extract_target.c
index 3ad1a9628..9e82675e3 100644
--- a/tests/oss-fuzz/flow_extract_target.c
+++ b/tests/oss-fuzz/flow_extract_target.c
@@ -10,338 +10,36 @@
 #include "classifier-private.h"
 
 static void
-shuffle_u32s(uint32_t *p, size_t n)
+test_flow_hash(const struct flow *flow)
 {
-for (; n > 1; n--, p++) {
-uint32_t *q = [random_range(n)];
-uint32_t tmp = *p;
-*p = *q;
-*q = tmp;
-}
-}
-
-/* Returns a copy of 'src'.  The caller must eventually free the returned
- * miniflow with free(). */
-static struct miniflow *
-miniflow_clone__(const struct miniflow *src)
-{
-struct miniflow *dst;
-size_t data_size;
-
-data_size = miniflow_alloc(, 1, src);
-miniflow_clone(dst, src, data_size / sizeof(uint64_t));
-return dst;
-}
-
-/* Returns a hash value for 'flow', given 'basis'. */
-static inline uint32_t
-miniflow_hash__(const struct miniflow *flow, uint32_t basis)
-{
-const uint64_t *p = miniflow_get_values(flow);
-size_t n_values = miniflow_n_values(flow);
-struct flowmap hash_map = FLOWMAP_EMPTY_INITIALIZER;
-uint32_t hash = basis;
-size_t idx;
-
-FLOWMAP_FOR_EACH_INDEX (idx, flow->map) {
-uint64_t value = *p++;
-
-if (value) {
-hash = hash_add64(hash, value);
-flowmap_set(_map, idx, 1);
-}
-}
-map_t map;
-FLOWMAP_FOR_EACH_MAP (map, hash_map) {
-hash = hash_add64(hash, map);
-}
-
-return hash_finish(hash, n_values);
-}
-
-static uint32_t
-random_value(void)
-{
-static const uint32_t values_[] =
-{ 0x, 0x, 0x, 0x8000,
-  0x0001, 0xface, 0x00d00d1e, 0xdeadbeef };
-
-return values_[random_range(ARRAY_SIZE(values_))];
-}
-
-static bool
-choose(unsigned int n, unsigned int *idxp)
-{
-if (*idxp < n) {
-return true;
-} else {
-*idxp -= n;
-return false;
-}
-}