Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't save pkt_mark in compose_output_action__().

[Ben Pfaff]

On Sat, Mar 18, 2017 at 01:15:53PM +0500, Valentine Sinitsyn wrote:
> Hi,
> 
> On 17.03.2017 22:55, Ben Pfaff wrote:
> >Previously, this function could modify the pkt_mark field as part of IPsec
> >integration.  It no longer does that, so there's no longer any need for it
> >to save and restore pkt_mark, and this commit removes that.
> Does it mean that now there is no way to send a bit of information across a
> pair of patch ports, that is, mark a packet on bridge A and check the mark
> on bridge B?

This change doesn't have anything to do with that.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Philanthropic Gesture

[Susan Klatten]

You have been chosen to benefit from Susanne Hanna Ursula Klatten Philanthropic 
Donation. Contact her through her Personal Email: 
susanneklattenchar...@gmail.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] sphinx on os x on travis?

[Lance Richardson]

- Original Message -
> From: "Ben Pfaff" 
> To: d...@openvswitch.org
> Cc: "Lance Richardson" 
> Sent: Friday, March 17, 2017 7:09:51 PM
> Subject: sphinx on os x on travis?
> 
> I'm seeing failures on Travis that boil down to this:
> 
> 
> /home/travis/build/blp/ovs-reviews/Documentation/intro/install/documentation.rst:34:
> ERROR: Error in "note" directive:
> invalid option block.
> 
> .. note::
>   These instructions provide information on building the documentation
>   locally.
>   For information on writing documentation, refer to
>   :doc:`/internals/contributing/documentation-style`
> 
> e.g. in https://travis-ci.org/blp/ovs-reviews/jobs/212266894.
> 
> Some web searches suggest this might be due to an old python-sphinx
> version.  Do anyone know how to make travis use a newer version?
> 
> Thanks,
> 
> Ben.
> 

Hi Ben,

After taking another look, it seems this has nothing to do with os x
(the same failure is occurring on Linux builds as well). These failures
appear to have started with this commit, which lowered the minimum
sphinx version to 1.1:

https://github.com/openvswitch/ovs/commit/59cf52e6d3307d5889335893fc941fe55cd3ed99

The failures (on both Linux and os x) are preceded by:

Running Sphinx v1.1.3

Based on the above, I think the issue is that sphinx 1.1 isn't actually 
sufficient
to build the documentation.

   Lance

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

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't save pkt_mark in compose_output_action__().

[Valentine Sinitsyn]

Hi,

On 17.03.2017 22:55, Ben Pfaff wrote:

Previously, this function could modify the pkt_mark field as part of IPsec
integration.  It no longer does that, so there's no longer any need for it
to save and restore pkt_mark, and this commit removes that.
Does it mean that now there is no way to send a bit of information 
across a pair of patch ports, that is, mark a packet on bridge A and 
check the mark on bridge B?


Thanks,
Valentine



CC: Ansis Atteka 
Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-xlate.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1a82b8d569be..9fe778a32857 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3265,7 +3265,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
 struct flow *flow = >xin->flow;
 struct flow_tnl flow_tnl;
 union flow_vlan_hdr flow_vlans[FLOW_MAX_VLAN_HEADERS];
-uint32_t flow_pkt_mark;
 uint8_t flow_nw_tos;
 odp_port_t out_port, odp_port;
 bool tnl_push_pop_send = false;
@@ -3460,7 +3459,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
 }

 memcpy(flow_vlans, flow->vlans, sizeof flow_vlans);
-flow_pkt_mark = flow->pkt_mark;
 flow_nw_tos = flow->nw_tos;

 if (count_skb_priorities(xport)) {
@@ -3588,7 +3586,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
  out:
 /* Restore flow */
 memcpy(flow->vlans, flow_vlans, sizeof flow->vlans);
-flow->pkt_mark = flow_pkt_mark;
 flow->nw_tos = flow_nw_tos;
 }



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

[ovs-dev] [PATCH 1/2] stp: Change the api for next patch.

[nickcooper-zhangtonghao]

This patch changes the stp_port_get_role and removes
the stp_port_get_id, because stp/show has locked the
mutex before calling the stp_port_get_role, and
stp_port_get_id will not be used.

Signed-off-by: nickcooper-zhangtonghao 
Acked-by: Ben Pfaff 
---
 lib/stp.c  | 29 +
 lib/stp.h  |  4 ++--
 ofproto/ofproto-dpif.c |  4 +---
 3 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/lib/stp.c b/lib/stp.c
index 2ffe5c2..b7ddc39 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -857,18 +857,6 @@ stp_port_no(const struct stp_port *p)
 return index;
 }
 
-/* Returns the port ID for 'p'. */
-int
-stp_port_get_id(const struct stp_port *p)
-{
-int port_id;
-
-ovs_mutex_lock();
-port_id = p->port_id;
-ovs_mutex_unlock();
-return port_id;
-}
-
 /* Returns the state of port 'p'. */
 enum stp_state
 stp_port_get_state(const struct stp_port *p)
@@ -882,13 +870,12 @@ stp_port_get_state(const struct stp_port *p)
 }
 
 /* Returns the role of port 'p'. */
-enum stp_role
-stp_port_get_role(const struct stp_port *p)
+static enum stp_role
+stp_port_get_role(const struct stp_port *p) OVS_REQUIRES(mutex)
 {
 struct stp_port *root_port;
 enum stp_role role;
 
-ovs_mutex_lock();
 root_port = p->stp->root_port;
 if (root_port && root_port->port_id == p->port_id) {
 role = STP_ROLE_ROOT;
@@ -899,7 +886,6 @@ stp_port_get_role(const struct stp_port *p)
 } else {
 role = STP_ROLE_ALTERNATE;
 }
-ovs_mutex_unlock();
 return role;
 }
 
@@ -915,6 +901,17 @@ stp_port_get_counts(const struct stp_port *p,
 ovs_mutex_unlock();
 }
 
+void
+stp_port_get_status(const struct stp_port *p,
+int *port_id, enum stp_state *state, enum stp_role *role)
+{
+ovs_mutex_lock();
+*port_id = p->port_id;
+*state = p->state;
+*role = stp_port_get_role(p);
+ovs_mutex_unlock();
+}
+
 /* Disables STP on port 'p'. */
 void
 stp_port_disable(struct stp_port *p)
diff --git a/lib/stp.h b/lib/stp.h
index 9f945ad..c64089a 100644
--- a/lib/stp.h
+++ b/lib/stp.h
@@ -145,11 +145,11 @@ void stp_port_set_name(struct stp_port *, const char *);
 void stp_port_set_aux(struct stp_port *, void *);
 void *stp_port_get_aux(struct stp_port *);
 int stp_port_no(const struct stp_port *);
-int stp_port_get_id(const struct stp_port *);
 enum stp_state stp_port_get_state(const struct stp_port *);
-enum stp_role stp_port_get_role(const struct stp_port *);
 void stp_port_get_counts(const struct stp_port *,
  int *tx_count, int *rx_count, int *error_count);
+void stp_port_get_status(const struct stp_port *p,
+ int *port_id, enum stp_state *state, enum stp_role 
*role);
 void stp_port_enable(struct stp_port *);
 void stp_port_disable(struct stp_port *);
 void stp_port_set_priority(struct stp_port *, uint8_t new_priority);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6611c23..523adad 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2541,10 +2541,8 @@ get_stp_port_status(struct ofport *ofport_,
 }
 
 s->enabled = true;
-s->port_id = stp_port_get_id(sp);
-s->state = stp_port_get_state(sp);
+stp_port_get_status(sp, >port_id, >state, >role);
 s->sec_in_state = (time_msec() - ofport->stp_state_entered) / 1000;
-s->role = stp_port_get_role(sp);
 
 return 0;
 }
-- 
1.8.3.1



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

[ovs-dev] [PATCH 2/2] stp: Add the 'ovs-appctl stp/show' command.

[nickcooper-zhangtonghao]

The stp/show command will help users and developers to
get more details about stp. This patch works together with
the previous patch "stp: Change the api for next patch."

Signed-off-by: nickcooper-zhangtonghao 
Co-authored-by: Ben Pfaff 
---
 NEWS   |  1 +
 lib/stp.c  | 98 ++
 vswitchd/ovs-vswitchd.8.in |  4 ++
 3 files changed, 103 insertions(+)

diff --git a/NEWS b/NEWS
index e2e456a..00c9106 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@ Post-v2.7.0
  "dot1q-tunnel" port VLAN mode.
- OVN:
  * Make the DHCPv4 router setting optional.
+   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/lib/stp.c b/lib/stp.c
index b7ddc39..952d3ce 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -236,6 +236,8 @@ static void stp_send_bpdu(struct stp_port *, const void *, 
size_t)
 OVS_REQUIRES(mutex);
 static void stp_unixctl_tcn(struct unixctl_conn *, int argc,
 const char *argv[], void *aux);
+static void stp_unixctl_show(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
 
 void
 stp_init(void)
@@ -251,6 +253,8 @@ stp_init(void)
 
 unixctl_command_register("stp/tcn", "[bridge]", 0, 1, stp_unixctl_tcn,
  NULL);
+unixctl_command_register("stp/show", "[bridge]", 0, 1,
+ stp_unixctl_show, NULL);
 ovsthread_once_done();
 }
 }
@@ -1634,3 +1638,97 @@ stp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock();
 }
+
+static void
+stp_bridge_id_details(struct ds *ds, const stp_identifier bridge_id,
+  const int hello_time, const int max_age,
+  const int forward_delay)
+OVS_REQUIRES(mutex)
+{
+uint16_t priority = bridge_id >> 48;
+ds_put_format(ds, "\tstp-priority\t%"PRIu16"\n", priority);
+
+struct eth_addr mac;
+const uint64_t mac_bits = (UINT64_C(1) << 48) - 1;
+eth_addr_from_uint64(bridge_id & mac_bits, );
+ds_put_format(ds, "\tstp-system-id\t"ETH_ADDR_FMT"\n", ETH_ADDR_ARGS(mac));
+ds_put_format(ds, "\tstp-hello-time\t%ds\n",
+  timer_to_ms(hello_time) / 1000);
+ds_put_format(ds, "\tstp-max-age\t%ds\n", timer_to_ms(max_age) / 1000);
+ds_put_format(ds, "\tstp-fwd-delay\t%ds\n",
+  timer_to_ms(forward_delay) / 1000);
+}
+
+static void
+stp_print_details(struct ds *ds, const struct stp *stp)
+OVS_REQUIRES(mutex)
+{
+const uint16_t port_no_bits = (UINT16_C(1) << 8) - 1;
+
+ds_put_format(ds, " %s \n", stp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+stp_bridge_id_details(ds, stp->designated_root, stp->bridge_hello_time,
+  stp->bridge_max_age, stp->bridge_forward_delay);
+
+if (stp_is_root_bridge(stp)) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", stp->root_port->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n", stp->root_path_cost);
+}
+
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+stp_bridge_id_details(ds, stp->bridge_id, stp->hello_time,
+  stp->max_age, stp->forward_delay);
+
+ds_put_cstr(ds, "\n");
+
+const struct stp_port *p;
+ds_put_format(ds, "\t%-11.10s%-11.10s%-11.10s%-6.5s%-8.7s\n",
+  "Interface", "Role", "State", "Cost", "Pri.Nbr");
+ds_put_cstr(ds, "\t-- -- -- - ---\n");
+FOR_EACH_ENABLED_PORT (p, stp) {
+ds_put_format(ds, "\t%-11.10s", p->port_name);
+ds_put_format(ds, "%-11.10s", stp_role_name(stp_port_get_role(p)));
+ds_put_format(ds, "%-11.10s", stp_state_name(p->state));
+ds_put_format(ds, "%-6d", p->path_cost);
+ds_put_format(ds, "%d.%d\n", p->port_id >> 8,
+  p->port_id & port_no_bits);
+}
+
+ds_put_cstr(ds, "\n");
+}
+
+static void
+stp_unixctl_show(struct unixctl_conn *conn, int argc,
+ const char *argv[], void *aux OVS_UNUSED)
+{
+struct ds ds = DS_EMPTY_INITIALIZER;
+
+ovs_mutex_lock();
+if (argc > 1) {
+struct stp *stp = stp_find(argv[1]);
+
+if (!stp) {
+unixctl_command_reply_error(conn, "no such stp object");
+goto out;
+}
+
+stp_print_details(, stp);
+} else {
+struct stp *stp;
+
+LIST_FOR_EACH (stp, node, all_stps) {
+stp_print_details(, stp);
+}
+}
+
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+
+out:
+ovs_mutex_unlock();
+}
diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index fc19f3b..8496aa6 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -140,6 +140,10 @@ Forces a topology 

Re: [ovs-dev] [PATCH] stp: Set time of BPDU packet.

[nickcooper-zhangtonghao]

I was wrong.

Thanks.
Nick

> On Mar 18, 2017, at 6:56 AM, Ben Pfaff  wrote:
> 
> On Wed, Mar 08, 2017 at 03:53:10AM -0800, nickcooper-zhangtonghao wrote:
>> The OvS BPDU packets have not right format. The STP works
>> well in OvS bridges when stp enabled, because they set the
>> time(e.g. max age, hello time and forward delay) in ticks.
>> But they should be sec. If so,  OvS STP can work well with
>> other type of bridge using STP.
>> 
>> Signed-off-by: nickcooper-zhangtonghao > >
> 
> Timer values in spanning tree BPDUs are in 1/256 of a second, not in
> seconds.  Are you sure there's a bug here?

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

Re: [ovs-dev] [PATCH 2/4] stp: Add the stp/show command.

[nickcooper-zhangtonghao]

Thanks very much, I submitted v2 and added the “Co-authored-by".


Nick

> On Mar 18, 2017, at 2:37 AM, Ben Pfaff  wrote:
> 
> On Tue, Mar 07, 2017 at 05:11:28AM -0800, nickcooper-zhangtonghao wrote:
>> The stp/show command will help users and developers get
>> more details about stp. This patch works together with
>> the previous patch "stp: Change the api for next patch."
>> 
>> Signed-off-by: nickcooper-zhangtonghao > >
> 
> Thanks for working on this!
> 
> I noticed that a lot of the printf format specifiers seem to be randomly
> chosen.  I'm appending a patch that fixes these and improves style in a
> couple of ways.
> 
> This patch should also add documentation for the new commands in the
> ovs-vswitchd manpage and probably add a NEWS item too.  Will you send a
> revision?
> 
> Thanks,
> 
> Ben.

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

Re: [ovs-dev] Reproducing ovn-scale-test results

[Valentine Sinitsyn]

Hi Han,

On 17.03.2017 23:36, Han Zhou wrote:


On Fri, Mar 17, 2017 at 2:50 AM, Valentine Sinitsyn
> wrote:

Did you restart controllers or ovn-northd after running full tests

before binding more ports. It would be interesting to learn how long
does it take warm up IDL in controllers/northd in your setup.



No need to restart. It will cool down when test ends. On top of the
scale, we can just test creating & binding 500 lports and then tear down
the 500 lports before next test, which takes much less time than the
full run from empty.



Moreover, in current ovs-scale-test code, the step "wait 100 lport up"
is updated utilizing a new feature (wait for HVs to catch up) that was


Are you referring to ovn-nbctl --wait-until, or something else? If you

were not using it for the talk, what exactly does "create + bind" mean
on the graph?



--wait-until has been used always, to wait lport state become "up" in
NB, which means port binding is reflected in NB. The new change [1] was
using the new feature of ovn-nbctl: "sync --wait=hv" which will wait
until the port binding to be processed on all HVs. This feature was not
there yet when we had the talk, and no reasonable alternative to achieve
the same.
Got you know. This may affect the test negatovely indeed, as OVSDB seems 
to scale poorly with the number of clients, as our tests suggest.


Thanks you again.

Valentine


[1]
https://github.com/openvswitch/ovn-scale-test/commit/0ece1038de45f05f461b45162b21a8bde2793010

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