Re: [ovs-dev] [PATCH] Revert "netdev: Fix netdev_open() to adhere to class type if given"

[nickcooper-zhangtonghao]

> On Jul 19, 2017, at 7:32 AM, Justin Pettit  wrote:
> 
>> 
>> On Jul 18, 2017, at 4:15 AM, Eelco Chaudron  wrote:
>> 
>> On 18/07/17 08:28, Justin Pettit wrote:
 On Jul 17, 2017, at 10:06 PM, Numan Siddique  wrote:
 
 On Tue, Jul 18, 2017 at 9:42 AM, Justin Pettit  wrote:
 
> On Jul 17, 2017, at 8:51 PM, Guru Shetty  wrote:
> 
> 
>> On 17 July 2017 at 12:51, Justin Pettit  wrote:
>> 
 Sounds reasonable to me.  Does anyone else have any opinions?  If I don't 
 hear anything by late tomorrow morning (PDT), I'll revert the patch and 
 release 2.7.2.
 
 
 Reverting the patch and a release 2.7.2 sounds fine to me.
>>> Are you okay with that, Eelco?  Unless anyone objects, we'll leave the 
>>> commit on master and look at applying your new patch.
>>> 
>> Sound fine to me.
> 
> Okay, I reverted the patch and pushed to branch-2.7.  I've sent out a series 
> to release 2.7.2:
> 
>   https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335886.html
> 
> If someone can review that, I'll get the release out tonight.
> 
> --Justin

Hi all,
I sent patches, I hope it can help us. Thanks.

http://patchwork.ozlabs.org/patch/790729/
http://patchwork.ozlabs.org/patch/790730/







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

Re: [ovs-dev] [PATCH] rstp: Show root bridge info.

[nickcooper-zhangtonghao]

> On Jul 14, 2017, at 7:05 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> This change makes me nervous about dereferencing a null 'p'.  I think
> that it will not do so, for a root bridge, but it takes some study to
> see that.
> 
> How about this, instead?  It has a small amount of code duplication, but
> it is easier to see that it avoids a null dereference.


Looks good to me. Thanks.

Acked-by: nickcooper-zhangtonghao <n...@opencloud.tech>


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

Re: [ovs-dev] [PATCH] rstp: Show root bridge info.

[nickcooper-zhangtonghao]

Who can help me to review this patch. Thanks.

> On Jun 20, 2017, at 11:04 AM, nickcooper-zhangtonghao <n...@opencloud.tech> 
> wrote:
> 
> When we use the 'ovs-appctl rstp/show', the root bridge
> of rstp is always 'unknown root port'. We don't expect
> that. The reason is that the committer added the check
> for var 'p'. In the rstp, if a bridge is root bridge,
> there is not root port, and we don't use the root port
> 'p', 'rstp/show' in the same case. If we check only rstp
> root port, the root info will not shown any more.
> 
> CC: Ben Pfaff <b...@ovn.org>
> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
> ---
> lib/rstp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/rstp.c b/lib/rstp.c
> index 9280b3a..b30f930 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -1602,7 +1602,7 @@ rstp_print_details(struct ds *ds, const struct rstp 
> *rstp)
> 
> bool is_root = rstp_is_root_bridge__(rstp);
> struct rstp_port *p = rstp_get_root_port__(rstp);
> -if (!p) {
> +if (!is_root && !p) {
> ds_put_cstr(ds, "unknown root port\n");
> return;
> }



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

Re: [ovs-dev] [PATCH v9] netdev-dpdk: Increase pmd thread priority.

[nickcooper-zhangtonghao]

If we fail to set the priority, we should return the err code
and not return 0.


> On Jun 23, 2017, at 4:51 AM, Bhanuprakash Bodireddy 
>  wrote:
> 
> Increase the DPDK pmd thread scheduling priority by lowering the nice
> value. This will advise the kernel scheduler to prioritize pmd thread
> over other processes and will help PMD to provide deterministic
> performance in out-of-the-box deployments.
> 
> This patch sets the nice value of PMD threads to '-20'.
> 
>  $ ps -eLo comm,policy,psr,nice | grep pmd
> 
>   COMMAND  POLICY  PROCESSORNICE
>pmd62 TS3-20
>pmd63 TS0-20
>pmd64 TS1-20
>pmd65 TS2-20
> 
> Signed-off-by: Bhanuprakash Bodireddy  >
> Tested-by: Billy O'Mahony  >
> ---
> v8->v9:
> * Rebase
> 
> v7->v8:
> * Rebase
> * Update the documentation file @Documentation/intro/install/dpdk-advanced.rst
> 
> v6->v7:
> * Remove realtime scheduling policy logic.
> * Increase pmd thread scheduling priority by lowering nice value to -20.
> * Update doc accordingly.
> 
> v5->v6:
> * Prohibit spawning pmd thread on the lowest core in dpdk-lcore-mask if
>  lcore-mask and pmd-mask affinity are identical.
> * Updated Note section in INSTALL.DPDK-ADVANCED doc.
> * Tested below cases to verify system stability with pmd priority patch
> 
> v4->v5:
> * Reword Note section in DPDK-ADVANCED.md
> 
> v3->v4:
> * Document update
> * Use ovs_strerror for reporting errors in lib-numa.c
> 
> v2->v3:
> * Move set_priority() function to lib/ovs-numa.c
> * Apply realtime scheduling policy and priority to pmd thread only if
>  pmd-cpu-mask is passed.
> * Update INSTALL.DPDK-ADVANCED.
> 
> v1->v2:
> * Removed #ifdef and introduced dummy function "pmd_thread_setpriority"
>  in netdev-dpdk.h
> * Rebase
> 
> Documentation/intro/install/dpdk.rst |  8 +++-
> lib/dpif-netdev.c|  4 
> lib/ovs-numa.c   | 21 +
> lib/ovs-numa.h   |  1 +
> 4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/intro/install/dpdk.rst 
> b/Documentation/intro/install/dpdk.rst
> index e83f852..b5c26ba 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -453,7 +453,8 @@ affinitized accordingly.
>   to be affinitized to isolated cores for optimum performance.
> 
>   By setting a bit in the mask, a pmd thread is created and pinned to the
> -  corresponding CPU core. e.g. to run a pmd thread on core 2::
> +  corresponding CPU core with nice value set to -20.
> +  e.g. to run a pmd thread on core 2::
> 
>   $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x4
> 
> @@ -493,6 +494,11 @@ improvements as there will be more total CPU occupancy 
> available::
> 
> NIC port0 <-> OVS <-> VM <-> OVS <-> NIC port 1
> 
> +  .. note::
> +It is recommended that the OVS control thread and pmd thread shouldn't be
> +pinned to the same core i.e 'dpdk-lcore-mask' and 'pmd-cpu-mask' cpu mask
> +settings should be non-overlapping.
> +
> DPDK Physical Port Rx Queues
> 
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index f83b632..6bbd786 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3712,6 +3712,10 @@ pmd_thread_main(void *f_)
> ovs_numa_thread_setaffinity_core(pmd->core_id);
> dpdk_set_lcore_id(pmd->core_id);
> poll_cnt = pmd_load_queues_and_ports(pmd, _list);
> +
> +/* Set pmd thread's nice value to -20 */
> +#define MIN_NICE -20
> +ovs_numa_thread_setpriority(MIN_NICE);
> reload:
> emc_cache_init(>flow_cache);
> 
> diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
> index 98e97cb..a1921b3 100644
> --- a/lib/ovs-numa.c
> +++ b/lib/ovs-numa.c
> @@ -23,6 +23,7 @@
> #include 
> #include 
> #include 
> +#include 
> #include 
> #include 
> #endif /* __linux__ */
> @@ -570,3 +571,23 @@ int ovs_numa_thread_setaffinity_core(unsigned core_id 
> OVS_UNUSED)
> return EOPNOTSUPP;
> #endif /* __linux__ */
> }
> +
> +int
> +ovs_numa_thread_setpriority(int nice OVS_UNUSED)
> +{
> +if (dummy_numa) {
> +return 0;
> +}
> +
> +#ifndef _WIN32
> +int err;
> +err = setpriority(PRIO_PROCESS, 0, nice);
> +if (err) {
> +VLOG_ERR("Thread priority error %s",ovs_strerror(err));
> +}
> +
> +return 0;

return err;

if success, err == 0

> +#else
> +return EOPNOTSUPP;
> +#endif
> +}
> diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h
> index 6946cdc..e132483 100644
> --- a/lib/ovs-numa.h
> +++ b/lib/ovs-numa.h
> @@ -62,6 +62,7 @@ bool ovs_numa_dump_contains_core(const struct ovs_numa_dump 
> *,
> size_t ovs_numa_dump_count(const struct ovs_numa_dump *);
> void ovs_numa_dump_destroy(struct ovs_numa_dump *);
> int 

[ovs-dev] [PATCH] rstp: Show root bridge info.

[nickcooper-zhangtonghao]

When we use the 'ovs-appctl rstp/show', the root bridge
of rstp is always 'unknown root port'. We don't expect
that. The reason is that the committer added the check
for var 'p'. In the rstp, if a bridge is root bridge,
there is not root port, and we don't use the root port
'p', 'rstp/show' in the same case. If we check only rstp
root port, the root info will not shown any more.

CC: Ben Pfaff <b...@ovn.org>
Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rstp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 9280b3a..b30f930 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -1602,7 +1602,7 @@ rstp_print_details(struct ds *ds, const struct rstp *rstp)
 
 bool is_root = rstp_is_root_bridge__(rstp);
 struct rstp_port *p = rstp_get_root_port__(rstp);
-if (!p) {
+if (!is_root && !p) {
 ds_put_cstr(ds, "unknown root port\n");
 return;
 }
-- 
1.8.3.1




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

Re: [ovs-dev] [PATCH v2 02/12] openvswitch.h: Use odp_port_t for port numbers in userspace-only structs.

[nickcooper-zhangtonghao]

Reviewed-by: nickcooper-zhangtonghao <n...@opencloud.tech>

> On Jun 19, 2017, at 7:29 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> Using the correct type reduces the need for type conversions.
> 
> Signed-off-by: Ben Pfaff <b...@ovn.org <mailto:b...@ovn.org>>
> ---
> datapath/linux/compat/include/linux/openvswitch.h | 4 ++--
> lib/dpif-netdev.c | 2 +-
> lib/netdev.c  | 2 +-
> ofproto/ofproto-dpif-sflow.c  | 2 +-
> ofproto/ofproto-dpif-xlate.c  | 4 ++--
> 5 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index 4c88de1d610d..24e51cb311d2 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -714,8 +714,8 @@ struct ovs_action_hash {
>  * this header to build final header according to actual packet parameters.
>  */
> struct ovs_action_push_tnl {
> - uint32_t tnl_port;
> - uint32_t out_port;
> + odp_port_t tnl_port;
> + odp_port_t out_port;
>   uint32_t header_len;
>   uint32_t tnl_type; /* For logging. */
>   uint32_t header[TNL_PUSH_HEADER_SIZE / 4];
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 2b65dc74a269..f97e97ab2931 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4956,7 +4956,7 @@ push_tnl_action(const struct dp_netdev_pmd_thread *pmd,
> 
> data = nl_attr_get(attr);
> 
> -tun_port = pmd_tnl_port_cache_lookup(pmd, u32_to_odp(data->tnl_port));
> +tun_port = pmd_tnl_port_cache_lookup(pmd, data->tnl_port);
> if (!tun_port) {
> err = -EINVAL;
> goto error;
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 001b7b37bb57..765bf4b9ccad 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -831,7 +831,7 @@ netdev_push_header(const struct netdev *netdev,
> struct dp_packet *packet;
> DP_PACKET_BATCH_FOR_EACH (packet, batch) {
> netdev->netdev_class->push_header(packet, data);
> -pkt_metadata_init(>md, u32_to_odp(data->out_port));
> +pkt_metadata_init(>md, data->out_port);
> }
> 
> return 0;
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index d9fddb1564b5..fc665a636853 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -901,7 +901,7 @@ sflow_read_tnl_push_action(const struct nlattr *attr,
> const struct ip_header *ip
> = ALIGNED_CAST(const struct ip_header *, eth + 1);
> 
> -sflow_actions->out_port = u32_to_odp(data->out_port);
> +sflow_actions->out_port = data->out_port;
> 
> /* Ethernet. */
> /* TODO: SFlow does not currently define a MAC-in-MAC
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index e15e3dec3f1c..48c4bad4ac0b 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3211,8 +3211,8 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct 
> xport *xport,
> if (err) {
> return err;
> }
> -tnl_push_data.tnl_port = odp_to_u32(tunnel_odp_port);
> -tnl_push_data.out_port = odp_to_u32(out_dev->odp_port);
> +tnl_push_data.tnl_port = tunnel_odp_port;
> +tnl_push_data.out_port = out_dev->odp_port;
> 
> /* After tunnel header has been added, packet_type of flow and base_flow
>  * need to be set to PT_ETH. */
> -- 
> 2.10.2

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

Re: [ovs-dev] [PATCH v2 01/12] ofp-util: Remove prototype for unimplemented function.

[nickcooper-zhangtonghao]

Reviewed-by: nickcooper-zhangtonghao <n...@opencloud.tech>

> On Jun 19, 2017, at 7:29 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> Signed-off-by: Ben Pfaff <b...@ovn.org <mailto:b...@ovn.org>>
> ---
> include/openvswitch/ofp-util.h | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
> index bbf6ffec5dd3..07723b427ce8 100644
> --- a/include/openvswitch/ofp-util.h
> +++ b/include/openvswitch/ofp-util.h
> @@ -247,8 +247,6 @@ void ofputil_match_to_ofp10_match(const struct match *, 
> struct ofp10_match *);
> enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *, const struct tun_table 
> *,
>  const struct vl_mff_map *, struct match 
> *,
>  uint16_t *padded_match_len);
> -enum ofperr ofputil_pull_ofp11_mask(struct ofpbuf *, struct match *,
> -struct mf_bitmap *bm);
> enum ofperr ofputil_match_from_ofp11_match(const struct ofp11_match *,
>struct match *);
> int ofputil_put_ofp11_match(struct ofpbuf *, const struct match *,
> -- 
> 2.10.2

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

[ovs-dev] [PATCH] doc: Add more details for in_port.

[nickcooper-zhangtonghao]

When I test OvS openflow, I confuse the 'in_port' with
'output:in_port'. I doc the openflow.rst and it may
help others to avoid it.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 Documentation/faq/openflow.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/faq/openflow.rst b/Documentation/faq/openflow.rst
index 7cd6161..1563893 100644
--- a/Documentation/faq/openflow.rst
+++ b/Documentation/faq/openflow.rst
@@ -362,6 +362,10 @@ but OVS drops the packets instead.
 
 $ ovs-ofctl add-flow br0 in_port=2,actions=in_port
 
+But if you use the ``output:in_port``, the packet is not output.::
+
+$ ovs-ofctl add-flow br0 in_port=2,actions=output:in_port
+
 This also works in some circumstances where the flow doesn't match on the
 input port.  For example, if you know that your switch has five ports
 numbered 2 through 6, then the following will send every received packet
-- 
1.8.3.1




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

Re: [ovs-dev] why ovs can't send packets to in_port.

[nickcooper-zhangtonghao]

Thanks.

> On Jun 10, 2017, at 12:44 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> On Fri, Jun 09, 2017 at 08:28:41PM +0800, nickcooper-zhangtonghao wrote:
>>  Why OvS does not support that we output a packet to a port which it is 
>> coming from.
>> In the case, eth1 can receive vlan1 and vlan2 packets and I hope when 
>> receiving packets, 
>> ovs can strip vlan1 packets, push it vlan2 and send it to eth1, and strip 
>> vlan2 packets,
>> push it vlan1 and send it eth1. 
>> 
>> openflow:
>> ovs-ofctl add-flow br-trans 
>> priority=100,in_port=1,dl_vlan=1,actions=strip_vlan,mod_vlan_vid:2,output:in_port
>> ovs-ofctl add-flow br-trans 
>> priority=100,in_port=1,dl_vlan=2,actions=strip_vlan,mod_vlan_vid:1,output:in_port
>> 
>> I guess it is necessary to support that feature. If not, we can doc the 
>> ovs-ofctl manpage for more detail.
> 
> The manpage already mentions this.  The FAQ has lots of detail:

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

Re: [ovs-dev] [PATCH v3 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On Jun 1, 2017, at 7:01 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> On Fri, May 19, 2017 at 12:20:39AM -0700, nickcooper-zhangtonghao wrote:
>> * This patch will be used for next patch. The 'rstp/show' command,
>> which uses the mutex, calls functions which also use the mutex.
>> We should init it as a recursive mutex.
>> 
>> * Because of recursive mutex, this patch remove the OVS_EXCLUDED in
>> list/rstp.[ch] files.
>> 
>> * Some rstp tests of OvS, which run with ovstest, does not run rstp_init
>> in the bridge_init. We should call rstp_init when creating the rstp
>> and stp also do that, otherwise tests fail.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
> 
> I think I'd prefer to add internal versions of functions that don't take
> the locks, instead.  Did you consider that approach?

The v4 patches has been submitted. We use the internal functions to implement 
the ‘rstp/show’. 


http://patchwork.ozlabs.org/patch/769478/ 
<http://patchwork.ozlabs.org/patch/769478/>
http://patchwork.ozlabs.org/patch/769476/ 
<http://patchwork.ozlabs.org/patch/769476/>
http://patchwork.ozlabs.org/patch/769477/ 
<http://patchwork.ozlabs.org/patch/769477/>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v4 1/3] rstp: Add rstp port name for human reading.

[nickcooper-zhangtonghao]

This patch is useful to debug rstp subsystem and log the
port name instead of port number. This patch will also
be used to display rstp info for next patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
Acked-by: Jarno Rajahalme <ja...@ovn.org>
---
 lib/rstp-common.h  |  1 +
 lib/rstp.c | 14 +-
 lib/rstp.h |  3 ++-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index 27e8079..c108232 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -262,6 +262,7 @@ struct rstp_port {
 struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
 struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
 void *aux OVS_GUARDED_BY(rstp_mutex);
+char *port_name;
 struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
 /*
  * MAC status parameters
diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..67e6912 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -751,6 +751,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
uint16_t port_number)
 }
 }
 
+static void
+rstp_port_set_port_name__(struct rstp_port *port, const char *name)
+OVS_REQUIRES(rstp_mutex)
+{
+free(port->port_name);
+port->port_name = xstrdup(name);
+}
+
 /* Converts the link speed to a port path cost [Table 17-3]. */
 uint32_t
 rstp_convert_speed_to_cost(unsigned int speed)
@@ -1164,6 +1172,7 @@ rstp_add_port(struct rstp *rstp)
 rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
 rstp_port_set_port_number__(p, 0);
 p->aux = NULL;
+p->port_name = NULL;
 rstp_initialize_port_defaults__(p);
 VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
  p->port_id);
@@ -1201,6 +1210,7 @@ rstp_port_unref(struct rstp_port *rp)
 ovs_mutex_lock(_mutex);
 rstp = rp->rstp;
 rstp_port_set_state__(rp, RSTP_DISABLED);
+free(rp->port_name);
 hmap_remove(>ports, >node);
 VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
  rp->port_id);
@@ -1439,13 +1449,15 @@ void
 rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
-  bool admin_port_state, bool do_mcheck, void *aux)
+  bool admin_port_state, bool do_mcheck, void *aux,
+  const char *name)
 OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 port->aux = aux;
 rstp_port_set_priority__(port, priority);
 rstp_port_set_port_number__(port, port_num);
+rstp_port_set_port_name__(port, name);
 rstp_port_set_path_cost__(port, path_cost);
 rstp_port_set_admin_edge__(port, is_admin_edge);
 rstp_port_set_auto_edge__(port, is_auto_edge);
diff --git a/lib/rstp.h b/lib/rstp.h
index 4942d59..5213f98 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -227,7 +227,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
 void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
enum rstp_admin_point_to_point_mac_state 
admin_p2p_mac_state,
-   bool admin_port_state, bool do_mcheck, void *aux)
+   bool admin_port_state, bool do_mcheck, void *aux,
+   const char *name)
 OVS_EXCLUDED(rstp_mutex);
 
 enum rstp_state rstp_port_get_state(const struct rstp_port *)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5d247e3..231b5b0 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2761,7 +2761,7 @@ set_rstp_port(struct ofport *ofport_,
 rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
   s->admin_edge_port, s->auto_edge,
   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
-  ofport);
+  ofport, netdev_get_name(ofport->up.netdev));
 update_rstp_port_state(ofport);
 /* Synchronize operational status. */
 rstp_port_set_mac_operational(rp, ofport->may_enable);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v4 3/3] rstp: Add the 'ovs-appctl rstp/show' command.

[nickcooper-zhangtonghao]

The rstp/show command will help users and developers to
get more details about rstp. This patch works together with
the previous patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 NEWS   |   3 +-
 lib/rstp.c | 107 +
 vswitchd/ovs-vswitchd.8.in |  11 -
 3 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index dd39d2a..e68d465 100644
--- a/NEWS
+++ b/NEWS
@@ -36,7 +36,8 @@ Post-v2.7.0
abbreviated to 4 hex digits.
  * "ovn-sbctl lflow-list" can now print OpenFlow flows that correspond
to logical flows.
-   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
+   - Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
+ (see ovs-vswitchd(8)).
- OpenFlow:
  * All features required by OpenFlow 1.4 are now implemented, so
ovs-vswitchd now enables OpenFlow 1.4 by default (in addition to
diff --git a/lib/rstp.c b/lib/rstp.c
index 4455c4a..e6b04f9 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -130,6 +130,8 @@ static rstp_identifier rstp_get_root_id__(const struct rstp 
*rstp)
 OVS_REQUIRES(rstp_mutex);
 static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
  const char *argv[], void *aux);
+static void rstp_unixctl_show(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -248,6 +250,8 @@ rstp_init(void)
 {
 unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
  NULL);
+unixctl_command_register("rstp/show", "[bridge]", 0, 1, rstp_unixctl_show,
+ NULL);
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -1573,3 +1577,106 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock(_mutex);
 }
+
+static void
+rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
+   const uint16_t hello_time, const uint16_t max_age,
+   const uint16_t forward_delay)
+OVS_REQUIRES(rstp_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%"PRIu16"s\n", hello_time);
+ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
+}
+
+static void
+rstp_print_details(struct ds *ds, const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+ds_put_format(ds, " %s \n", rstp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+bool is_root = rstp_is_root_bridge__(rstp);
+struct rstp_port *p = rstp_get_root_port__(rstp);
+
+rstp_identifier bridge_id =
+is_root ? rstp->bridge_identifier : rstp_get_root_id__(rstp);
+uint16_t hello_time =
+is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
+uint16_t max_age =
+is_root ? rstp->bridge_max_age : p->designated_times.max_age;
+uint16_t forward_delay =
+is_root ? rstp->bridge_forward_delay : 
p->designated_times.forward_delay;
+
+rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, forward_delay);
+if (is_root) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n",
+  rstp_get_root_path_cost__(rstp));
+}
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+rstp_bridge_id_details(ds, rstp->bridge_identifier,
+   rstp->bridge_hello_time,
+   rstp->bridge_max_age,
+   rstp->bridge_forward_delay);
+ds_put_cstr(ds, "\n");
+
+ds_put_format(ds, "\t%-11.10s%-11.10s%-11.10s%-9.8s%-8.7s\n",
+  "Interface", "Role", "State", "Cost", "Pri.Nbr");
+ds_put_cstr(ds, "\t-- -- --  ---\n");
+HMAP_FOR_EACH (p, node, >ports) {
+if (p->rstp_state != RSTP_DISABLED) {
+ds_put_format(ds, "\t%-11.10s",
+  p->port_name ? p->port_name : "null");
+ds_put_format(ds, &qu

[ovs-dev] [PATCH v4 2/3] rstp: Add internal functions without locks.

[nickcooper-zhangtonghao]

This patch adds some internal functions which
does not use the locks. This patch is used for
next patch.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rstp.c | 63 +-
 lib/rstp.h |  2 +-
 2 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 67e6912..4455c4a 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -120,6 +120,16 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
bool mcheck)
 OVS_REQUIRES(rstp_mutex);
 static void reinitialize_port__(struct rstp_port *p)
 OVS_REQUIRES(rstp_mutex);
+static bool rstp_is_root_bridge__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static uint32_t rstp_get_root_path_cost__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static struct rstp_port *rstp_get_root_port__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static rstp_identifier rstp_get_root_id__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex);
+static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -208,9 +218,6 @@ rstp_port_get_number(const struct rstp_port *p)
 return number;
 }
 
-static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
- const char *argv[], void *aux);
-
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
@@ -796,6 +803,13 @@ rstp_port_set_path_cost__(struct rstp_port *port, uint32_t 
path_cost)
 }
 
 /* Gets the root path cost. */
+static uint32_t
+rstp_get_root_path_cost__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+return rstp->root_priority.root_path_cost;
+}
+
 uint32_t
 rstp_get_root_path_cost(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
@@ -803,7 +817,7 @@ rstp_get_root_path_cost(const struct rstp *rstp)
 uint32_t cost;
 
 ovs_mutex_lock(_mutex);
-cost = rstp->root_priority.root_path_cost;
+cost = rstp_get_root_path_cost__(rstp);
 ovs_mutex_unlock(_mutex);
 return cost;
 }
@@ -1313,6 +1327,13 @@ rstp_get_designated_id(const struct rstp *rstp)
 }
 
 /* Returns the root bridge id. */
+static rstp_identifier
+rstp_get_root_id__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+return rstp->root_priority.root_bridge_id;
+}
+
 rstp_identifier
 rstp_get_root_id(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
@@ -1320,7 +1341,7 @@ rstp_get_root_id(const struct rstp *rstp)
 rstp_identifier root_id;
 
 ovs_mutex_lock(_mutex);
-root_id = rstp->root_priority.root_bridge_id;
+root_id = rstp_get_root_id__(rstp);
 ovs_mutex_unlock(_mutex);
 
 return root_id;
@@ -1357,6 +1378,14 @@ rstp_get_bridge_port_id(const struct rstp *rstp)
 /* Returns true if the bridge believes to the be root of the spanning tree,
  * false otherwise.
  */
+static bool
+rstp_is_root_bridge__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+return rstp->bridge_identifier ==
+rstp->root_priority.designated_bridge_id;
+}
+
 bool
 rstp_is_root_bridge(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
@@ -1364,8 +1393,7 @@ rstp_is_root_bridge(const struct rstp *rstp)
 bool is_root;
 
 ovs_mutex_lock(_mutex);
-is_root = rstp->bridge_identifier ==
-rstp->root_priority.designated_bridge_id;
+is_root = rstp_is_root_bridge__(rstp);
 ovs_mutex_unlock(_mutex);
 
 return is_root;
@@ -1388,23 +1416,32 @@ rstp_get_designated_root(const struct rstp *rstp)
 /* Returns the port connecting 'rstp' to the root bridge, or a null pointer if
  * there is no such port.
  */
-struct rstp_port *
-rstp_get_root_port(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
+static struct rstp_port *
+rstp_get_root_port__(const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
 {
 struct rstp_port *p;
 
-ovs_mutex_lock(_mutex);
 HMAP_FOR_EACH (p, node, >ports) {
 if (p->port_id == rstp->root_port_id) {
-ovs_mutex_unlock(_mutex);
 return p;
 }
 }
-ovs_mutex_unlock(_mutex);
 return NULL;
 }
 
+struct rstp_port *
+rstp_get_root_port(const struct rstp *rstp)
+OVS_EXCLUDED(rstp_mutex)
+{
+struct rstp_port *p;
+
+ovs_mutex_lock(_mutex);
+p = rstp_get_root_port__(rstp);
+ovs_mutex_unlock(_mutex);
+return p;
+}
+
 /* Returns the state of port 'p'. */
 enum rstp_state
 rstp_port_get_state(const struct rstp_port *p)
diff --git a/lib/rstp.h b/lib/rstp.h
index 5213f98..39a13b5 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -207,7 +207,7 @@ uint16_t rstp_get_designated_port_id(const struct rstp *)
 OVS_EXCLUDED(rstp_mutex);
 uint16_t rstp_get_bridge_port_id(const struct rstp *)
 OVS_EXCLUDED(rstp_mutex);
-struct rstp_port * rstp_get_root_port(struct rstp *)
+struct rstp_port * rstp_get_root_port(const stru

[ovs-dev] [PATCH v3 2/5] rstp: Add rstp port name for human reading.

[nickcooper-zhangtonghao]

This patch is useful to debug rstp subsystem and log the
port name instead of port number. This patch will also
be used to display rstp info for next patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
Acked-by: Jarno Rajahalme <ja...@ovn.org>
---
 lib/rstp-common.h  |  1 +
 lib/rstp.c | 14 +-
 lib/rstp.h |  3 ++-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index 27e8079..c108232 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -262,6 +262,7 @@ struct rstp_port {
 struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
 struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
 void *aux OVS_GUARDED_BY(rstp_mutex);
+char *port_name;
 struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
 /*
  * MAC status parameters
diff --git a/lib/rstp.c b/lib/rstp.c
index 9ad1b0d..48df7ad 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -744,6 +744,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
uint16_t port_number)
 }
 }
 
+static void
+rstp_port_set_port_name__(struct rstp_port *port, const char *name)
+OVS_REQUIRES(rstp_mutex)
+{
+free(port->port_name);
+port->port_name = xstrdup(name);
+}
+
 /* Converts the link speed to a port path cost [Table 17-3]. */
 uint32_t
 rstp_convert_speed_to_cost(unsigned int speed)
@@ -1150,6 +1158,7 @@ rstp_add_port(struct rstp *rstp)
 rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
 rstp_port_set_port_number__(p, 0);
 p->aux = NULL;
+p->port_name = NULL;
 rstp_initialize_port_defaults__(p);
 VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
  p->port_id);
@@ -1185,6 +1194,7 @@ rstp_port_unref(struct rstp_port *rp)
 ovs_mutex_lock(_mutex);
 rstp = rp->rstp;
 rstp_port_set_state__(rp, RSTP_DISABLED);
+free(rp->port_name);
 hmap_remove(>ports, >node);
 VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
  rp->port_id);
@@ -1414,12 +1424,14 @@ void
 rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
-  bool admin_port_state, bool do_mcheck, void *aux)
+  bool admin_port_state, bool do_mcheck, void *aux,
+  const char *name)
 {
 ovs_mutex_lock(_mutex);
 port->aux = aux;
 rstp_port_set_priority__(port, priority);
 rstp_port_set_port_number__(port, port_num);
+rstp_port_set_port_name__(port, name);
 rstp_port_set_path_cost__(port, path_cost);
 rstp_port_set_admin_edge__(port, is_admin_edge);
 rstp_port_set_auto_edge__(port, is_auto_edge);
diff --git a/lib/rstp.h b/lib/rstp.h
index 7a08228..b6f9089 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -194,7 +194,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
 void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
enum rstp_admin_point_to_point_mac_state 
admin_p2p_mac_state,
-   bool admin_port_state, bool do_mcheck, void *aux);
+   bool admin_port_state, bool do_mcheck, void *aux,
+   const char *name);
 
 enum rstp_state rstp_port_get_state(const struct rstp_port *);
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index dc5f004..6d3f1fb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2722,7 +2722,7 @@ set_rstp_port(struct ofport *ofport_,
 rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
   s->admin_edge_port, s->auto_edge,
   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
-  ofport);
+  ofport, netdev_get_name(ofport->up.netdev));
 update_rstp_port_state(ofport);
 /* Synchronize operational status. */
 rstp_port_set_mac_operational(rp, ofport->may_enable);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v3 5/5] stp: Add link-state checking support for stp ports.

[nickcooper-zhangtonghao]

When bridge stp enabled, we can enable the stp ports
despite ports are down. When initializing, this patch checks
link-state of ports and enable or disable them according
to their link-state. This patch also allow user to enable
and disable a port when bridge stp is running. If a stp
port is in disable state, it can forward packets. If its
link is down and this patch sets it to disable, there is
no L2 loop.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 41 -
 tests/stp.at   | 71 +-
 2 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6d3f1fb..098d363 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2574,6 +2574,37 @@ update_stp_port_state(struct ofport_dpif *ofport)
 }
 }
 
+static void
+stp_check_and_update_link_state(struct ofproto_dpif *ofproto)
+{
+struct ofport *ofport_;
+struct ofport_dpif *ofport;
+bool up;
+
+HMAP_FOR_EACH (ofport_, hmap_node, >up.ports) {
+ofport = ofport_dpif_cast(ofport_);
+up = netdev_get_carrier(ofport_->netdev);
+
+if (ofport->stp_port &&
+up != (stp_port_get_state(ofport->stp_port) != STP_DISABLED)) {
+
+VLOG_DBG("bridge: %s, port: %s is %s, %s it", ofproto->up.name,
+ netdev_get_name(ofport->up.netdev),
+ up ? "up" : "down",
+ up ? "enabling" : "disabling");
+
+if (up) {
+stp_port_enable(ofport->stp_port);
+stp_port_set_aux(ofport->stp_port, ofport);
+} else {
+stp_port_disable(ofport->stp_port);
+}
+
+update_stp_port_state(ofport);
+}
+}
+}
+
 /* Configures STP on 'ofport_' using the settings defined in 's'.  The
  * caller is responsible for assigning STP port numbers and ensuring
  * there are no duplicates. */
@@ -2604,7 +2635,12 @@ set_stp_port(struct ofport *ofport_,
 /* Set name before enabling the port so that debugging messages can print
  * the name. */
 stp_port_set_name(sp, netdev_get_name(ofport->up.netdev));
-stp_port_enable(sp);
+
+if (netdev_get_carrier(ofport_->netdev)) {
+stp_port_enable(sp);
+} else {
+stp_port_disable(sp);
+}
 
 stp_port_set_aux(sp, ofport);
 stp_port_set_priority(sp, s->priority);
@@ -2666,6 +2702,9 @@ stp_run(struct ofproto_dpif *ofproto)
 stp_tick(ofproto->stp, MIN(INT_MAX, elapsed));
 ofproto->stp_last_tick = now;
 }
+
+stp_check_and_update_link_state(ofproto);
+
 while (stp_get_changed_port(ofproto->stp, )) {
 struct ofport_dpif *ofport = stp_port_get_aux(sp);
 
diff --git a/tests/stp.at b/tests/stp.at
index bd5d208..e27600e 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -420,6 +420,7 @@ AT_CHECK([ovs-vsctl add-port br1 p8 -- \
set port p8 other_config:stp-enable=false -- \
 ])
 
+ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 AT_CHECK([ovs-ofctl add-flow br0 "in_port=7 icmp actions=1"])
@@ -519,7 +520,7 @@ AT_CHECK([
 set interface p6 type=dummy options:pstream=punix:$OVS_RUNDIR/p6.sock 
ofport_request=6
 ], [0])
 
-
+ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 # give time for STP to move initially
@@ -626,5 +627,73 @@ AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl
  port  VLAN  GROUPAge
 ])
 
+AT_CLEANUP
+
+AT_SETUP([STP - check link-state when stp is running])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl -- \
+set port br0 other_config:stp-enable=false -- \
+set bridge br0 datapath-type=dummy stp_enable=true \
+other-config:hwaddr=aa:66:aa:66:00:00
+], [0])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- \
+set interface p1 type=dummy -- \
+set port p1 other_config:stp-port-num=1
+ovs-vsctl add-port br0 p2 -- \
+set interface p2 type=dummy -- \
+set port p2 other_config:stp-port-num=2
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state up
+ovs-appctl time/stop
+
+# give time for STP to move initially
+for i in $(seq 0 30); do
+ovs-appctl time/warp 1000
+done
+
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+
+# add a stp port
+AT_CHECK([
+ovs-vsctl add-port br0 p3 -- \
+set interface p3 type=dummy -- \
+set port p3 other_config:stp-port-num=3
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state p3 down
+
+# We should not show the p3 because its link-state is down
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+

[ovs-dev] [PATCH v3 4/5] rstp: Increment the rstp port num counter.

[nickcooper-zhangtonghao]

OvS only supports RSTP_MAX_PORTS rstp ports while max port
num of stp is STP_MAX_PORTS. This patch increments the rstp
port num counter, otherwise the counter is 0 and the checking
above will always fail.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 vswitchd/bridge.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 972146e..5a08219 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1485,6 +1485,10 @@ port_configure_rstp(const struct ofproto *ofproto, 
struct port *port,
 port_s->port_num = 0;
 }
 
+/* Increment the port num counter, because we only support
+ * RSTP_MAX_PORTS rstp ports. */
+(*port_num_counter) ++;
+
 config_str = smap_get(>cfg->other_config, "rstp-path-cost");
 if (config_str) {
 port_s->path_cost = strtoul(config_str, NULL, 10);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v3 3/5] rstp: Add the 'ovs-appctl rstp/show' command.

[nickcooper-zhangtonghao]

The rstp/show command will help users and developers to
get more details about rstp. This patch works together with
the previous patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 NEWS   |   3 +-
 lib/rstp.c | 114 +++--
 lib/rstp.h |   2 +-
 vswitchd/ovs-vswitchd.8.in |  11 -
 4 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 7a2b185..4576408 100644
--- a/NEWS
+++ b/NEWS
@@ -28,7 +28,8 @@ Post-v2.7.0
abbreviated to 4 hex digits.
  * "ovn-sbctl lflow-list" can now print OpenFlow flows that correspond
to logical flows.
-   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
+   - Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
+ (see ovs-vswitchd(8)).
- OpenFlow:
  * All features required by OpenFlow 1.4 are now implemented, so
ovs-vswitchd now enables OpenFlow 1.4 by default (in addition to
diff --git a/lib/rstp.c b/lib/rstp.c
index 48df7ad..095e01f 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -120,6 +120,10 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
bool mcheck)
 OVS_REQUIRES(rstp_mutex);
 static void reinitialize_port__(struct rstp_port *p)
 OVS_REQUIRES(rstp_mutex);
+static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
+static void rstp_unixctl_show(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -205,9 +209,6 @@ rstp_port_get_number(const struct rstp_port *p)
 return number;
 }
 
-static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
- const char *argv[], void *aux);
-
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
@@ -240,6 +241,8 @@ rstp_init(void)
 
 unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
  NULL);
+unixctl_command_register("rstp/show", "[bridge]", 0, 1,
+ rstp_unixctl_show, NULL);
 ovsthread_once_done();
 }
 }
@@ -1367,7 +1370,7 @@ rstp_get_designated_root(const struct rstp *rstp)
  * there is no such port.
  */
 struct rstp_port *
-rstp_get_root_port(struct rstp *rstp)
+rstp_get_root_port(const struct rstp *rstp)
 {
 struct rstp_port *p;
 
@@ -1506,3 +1509,106 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock(_mutex);
 }
+
+static void
+rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
+   const uint16_t hello_time, const uint16_t max_age,
+   const uint16_t forward_delay)
+OVS_REQUIRES(rstp_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%"PRIu16"s\n", hello_time);
+ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
+}
+
+static void
+rstp_print_details(struct ds *ds, const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+ds_put_format(ds, " %s \n", rstp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+bool is_root = rstp_is_root_bridge(rstp);
+struct rstp_port *p = rstp_get_root_port(rstp);
+
+rstp_identifier bridge_id =
+is_root ? rstp->bridge_identifier : rstp_get_root_id(rstp);
+uint16_t hello_time =
+is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
+uint16_t max_age =
+is_root ? rstp->bridge_max_age : p->designated_times.max_age;
+uint16_t forward_delay =
+is_root ? rstp->bridge_forward_delay : 
p->designated_times.forward_delay;
+
+rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, forward_delay);
+if (is_root) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n",
+  rstp_get_root_path_cost(rstp));
+}
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+rstp_bridge_id_details(ds, rstp->bridge_identifier,
+   rstp->bridge_hello_time,
+ 

[ovs-dev] [PATCH v3 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

* This patch will be used for next patch. The 'rstp/show' command,
which uses the mutex, calls functions which also use the mutex.
We should init it as a recursive mutex.

* Because of recursive mutex, this patch remove the OVS_EXCLUDED in
list/rstp.[ch] files.

* Some rstp tests of OvS, which run with ovstest, does not run rstp_init
in the bridge_init. We should call rstp_init when creating the rstp
and stp also do that, otherwise tests fail.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rstp.c |  54 ++---
 lib/rstp.h | 131 +
 2 files changed, 56 insertions(+), 129 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..9ad1b0d 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -50,7 +50,7 @@
 
 VLOG_DEFINE_THIS_MODULE(rstp);
 
-struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
+struct ovs_mutex rstp_mutex;
 
 static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
 static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
_rstps__;
@@ -161,7 +161,6 @@ rstp_port_role_name(enum rstp_port_role role)
  * while taking a new reference. */
 struct rstp *
 rstp_ref(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 if (rstp) {
 ovs_refcount_ref(>ref_cnt);
@@ -172,7 +171,6 @@ rstp_ref(struct rstp *rstp)
 /* Frees RSTP struct when reference count reaches zero. */
 void
 rstp_unref(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 if (rstp && ovs_refcount_unref_relaxed(>ref_cnt) == 1) {
 ovs_mutex_lock(_mutex);
@@ -197,7 +195,6 @@ rstp_unref(struct rstp *rstp)
  * port_number). */
 int
 rstp_port_get_number(const struct rstp_port *p)
-OVS_EXCLUDED(rstp_mutex)
 {
 int number;
 
@@ -214,7 +211,6 @@ static void rstp_unixctl_tcn(struct unixctl_conn *, int 
argc,
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 decrease_rstp_port_timers__(rstp);
@@ -225,7 +221,6 @@ rstp_tick_timers(struct rstp *rstp)
 void
 rstp_port_received_bpdu(struct rstp_port *rp, const void *bpdu,
 size_t bpdu_size)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 /* Only process packets on ports that have RSTP enabled. */
@@ -237,10 +232,16 @@ rstp_port_received_bpdu(struct rstp_port *rp, const void 
*bpdu,
 
 void
 rstp_init(void)
-OVS_EXCLUDED(rstp_mutex)
 {
-unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
- NULL);
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+ovs_mutex_init_recursive(_mutex);
+
+unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
+ NULL);
+ovsthread_once_done();
+}
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -249,12 +250,13 @@ rstp_create(const char *name, rstp_identifier 
bridge_address,
 void (*send_bpdu)(struct dp_packet *bpdu, void *port_aux,
   void *rstp_aux),
 void *aux)
-OVS_EXCLUDED(rstp_mutex)
 {
 struct rstp *rstp;
 
 VLOG_DBG("Creating RSTP instance");
 
+rstp_init();
+
 rstp = xzalloc(sizeof *rstp);
 rstp->name = xstrdup(name);
 
@@ -338,7 +340,6 @@ rstp_set_bridge_address__(struct rstp *rstp, 
rstp_identifier bridge_address)
 /* Sets the bridge address. */
 void
 rstp_set_bridge_address(struct rstp *rstp, rstp_identifier bridge_address)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_address__(rstp, bridge_address);
@@ -347,7 +348,6 @@ rstp_set_bridge_address(struct rstp *rstp, rstp_identifier 
bridge_address)
 
 const char *
 rstp_get_name(const struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 char *name;
 
@@ -359,7 +359,6 @@ rstp_get_name(const struct rstp *rstp)
 
 rstp_identifier
 rstp_get_bridge_id(const struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 rstp_identifier bridge_id;
 
@@ -391,7 +390,6 @@ rstp_set_bridge_priority__(struct rstp *rstp, int 
new_priority)
 
 void
 rstp_set_bridge_priority(struct rstp *rstp, int new_priority)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_priority__(rstp, new_priority);
@@ -413,7 +411,6 @@ rstp_set_bridge_ageing_time__(struct rstp *rstp, int 
new_ageing_time)
 
 void
 rstp_set_bridge_ageing_time(struct rstp *rstp, int new_ageing_time)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_ageing_time__(rstp, new_ageing_time);
@@ -509,7 +506,6 @@ rstp_set_bridge_force_protocol_version__(struct rstp *rstp,
 void
 rstp_set_bridge_force_protocol_version(struct rstp *rstp,
 enum rstp_force_protocol_version new_force_prot

Re: [ovs-dev] [PATCH v2 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On May 19, 2017, at 6:47 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> On Wed, May 10, 2017 at 04:15:02AM -0700, nickcooper-zhangtonghao wrote:
>> * This patch will be used for next patch. The 'rstp/show' command,
>> which uses the mutex, calls functions which also use the mutex.
>> We should init it as a recursive mutex.
>> 
>> * Some rstp tests of OvS, which run with ovstest, does not run rstp_init
>> in the bridge_init. We should call rstp_init when creating the rstp
>> and stp also do that, otherwise tests fail.
>> 
>> * This patch remove the rstp_mutex in header file and make it static
>> in c file because we only use it in lib/rstp.c
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
> 
> Thanks for working on the rstp code!
> 
> This patch causes a huge number of build failures with Clang, like this:
> 
>In file included from ../lib/rstp.c:32:
>../lib/rstp.h:135:18: error: use of undeclared identifier 'rstp_mutex'
>../include/openvswitch/compiler.h:136:57: note: expanded from macro 
> 'OVS_EXCLUDED'
> 
> I think that's the only reason that rstp_mutex is global.


Thanks for your review, the v3 patches don’t remove the rstp_mutex from header 
file
and we can build it with gcc and clang. Because of recursive mutex, I remove 
the 
OVS_EXCLUDED in the lib/rstp.[ch] files. Thanks very much.


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

[ovs-dev] [PATCH v2 5/5] stp: Add link-state checking support for stp ports.

[nickcooper-zhangtonghao]

When bridge stp enabled, we can enable the stp ports
despite ports are down. When initializing, this patch checks
link-state of ports and enable or disable them according
to their link-state. This patch also allow user to enable
and disable a port when bridge stp is running. If a stp
port is in disable state, it can forward packets. If its
link is down and this patch sets it to disable, there is
no L2 loop.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 41 -
 tests/stp.at   | 71 +-
 2 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6d3f1fb..098d363 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2574,6 +2574,37 @@ update_stp_port_state(struct ofport_dpif *ofport)
 }
 }
 
+static void
+stp_check_and_update_link_state(struct ofproto_dpif *ofproto)
+{
+struct ofport *ofport_;
+struct ofport_dpif *ofport;
+bool up;
+
+HMAP_FOR_EACH (ofport_, hmap_node, >up.ports) {
+ofport = ofport_dpif_cast(ofport_);
+up = netdev_get_carrier(ofport_->netdev);
+
+if (ofport->stp_port &&
+up != (stp_port_get_state(ofport->stp_port) != STP_DISABLED)) {
+
+VLOG_DBG("bridge: %s, port: %s is %s, %s it", ofproto->up.name,
+ netdev_get_name(ofport->up.netdev),
+ up ? "up" : "down",
+ up ? "enabling" : "disabling");
+
+if (up) {
+stp_port_enable(ofport->stp_port);
+stp_port_set_aux(ofport->stp_port, ofport);
+} else {
+stp_port_disable(ofport->stp_port);
+}
+
+update_stp_port_state(ofport);
+}
+}
+}
+
 /* Configures STP on 'ofport_' using the settings defined in 's'.  The
  * caller is responsible for assigning STP port numbers and ensuring
  * there are no duplicates. */
@@ -2604,7 +2635,12 @@ set_stp_port(struct ofport *ofport_,
 /* Set name before enabling the port so that debugging messages can print
  * the name. */
 stp_port_set_name(sp, netdev_get_name(ofport->up.netdev));
-stp_port_enable(sp);
+
+if (netdev_get_carrier(ofport_->netdev)) {
+stp_port_enable(sp);
+} else {
+stp_port_disable(sp);
+}
 
 stp_port_set_aux(sp, ofport);
 stp_port_set_priority(sp, s->priority);
@@ -2666,6 +2702,9 @@ stp_run(struct ofproto_dpif *ofproto)
 stp_tick(ofproto->stp, MIN(INT_MAX, elapsed));
 ofproto->stp_last_tick = now;
 }
+
+stp_check_and_update_link_state(ofproto);
+
 while (stp_get_changed_port(ofproto->stp, )) {
 struct ofport_dpif *ofport = stp_port_get_aux(sp);
 
diff --git a/tests/stp.at b/tests/stp.at
index bd5d208..e27600e 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -420,6 +420,7 @@ AT_CHECK([ovs-vsctl add-port br1 p8 -- \
set port p8 other_config:stp-enable=false -- \
 ])
 
+ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 AT_CHECK([ovs-ofctl add-flow br0 "in_port=7 icmp actions=1"])
@@ -519,7 +520,7 @@ AT_CHECK([
 set interface p6 type=dummy options:pstream=punix:$OVS_RUNDIR/p6.sock 
ofport_request=6
 ], [0])
 
-
+ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 # give time for STP to move initially
@@ -626,5 +627,73 @@ AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl
  port  VLAN  GROUPAge
 ])
 
+AT_CLEANUP
+
+AT_SETUP([STP - check link-state when stp is running])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl -- \
+set port br0 other_config:stp-enable=false -- \
+set bridge br0 datapath-type=dummy stp_enable=true \
+other-config:hwaddr=aa:66:aa:66:00:00
+], [0])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- \
+set interface p1 type=dummy -- \
+set port p1 other_config:stp-port-num=1
+ovs-vsctl add-port br0 p2 -- \
+set interface p2 type=dummy -- \
+set port p2 other_config:stp-port-num=2
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state up
+ovs-appctl time/stop
+
+# give time for STP to move initially
+for i in $(seq 0 30); do
+ovs-appctl time/warp 1000
+done
+
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+
+# add a stp port
+AT_CHECK([
+ovs-vsctl add-port br0 p3 -- \
+set interface p3 type=dummy -- \
+set port p3 other_config:stp-port-num=3
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state p3 down
+
+# We should not show the p3 because its link-state is down
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+

[ovs-dev] [PATCH v2 3/5] rstp: Add the 'ovs-appctl rstp/show' command.

[nickcooper-zhangtonghao]

The rstp/show command will help users and developers to
get more details about rstp. This patch works together with
the previous patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 NEWS   |   3 +-
 lib/rstp.c | 114 +++--
 lib/rstp.h |   2 +-
 vswitchd/ovs-vswitchd.8.in |  11 -
 4 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 7a2b185..4576408 100644
--- a/NEWS
+++ b/NEWS
@@ -28,7 +28,8 @@ Post-v2.7.0
abbreviated to 4 hex digits.
  * "ovn-sbctl lflow-list" can now print OpenFlow flows that correspond
to logical flows.
-   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
+   - Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
+ (see ovs-vswitchd(8)).
- OpenFlow:
  * All features required by OpenFlow 1.4 are now implemented, so
ovs-vswitchd now enables OpenFlow 1.4 by default (in addition to
diff --git a/lib/rstp.c b/lib/rstp.c
index b942f6e..99ae161 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -120,6 +120,10 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
bool mcheck)
 OVS_REQUIRES(rstp_mutex);
 static void reinitialize_port__(struct rstp_port *p)
 OVS_REQUIRES(rstp_mutex);
+static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
+static void rstp_unixctl_show(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -208,9 +212,6 @@ rstp_port_get_number(const struct rstp_port *p)
 return number;
 }
 
-static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
- const char *argv[], void *aux);
-
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
@@ -246,6 +247,8 @@ rstp_init(void)
 
 unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
  NULL);
+unixctl_command_register("rstp/show", "[bridge]", 0, 1,
+ rstp_unixctl_show, NULL);
 ovsthread_once_done();
 }
 }
@@ -1398,7 +1401,7 @@ rstp_get_designated_root(const struct rstp *rstp)
  * there is no such port.
  */
 struct rstp_port *
-rstp_get_root_port(struct rstp *rstp)
+rstp_get_root_port(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
 {
 struct rstp_port *p;
@@ -1545,3 +1548,106 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock(_mutex);
 }
+
+static void
+rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
+   const uint16_t hello_time, const uint16_t max_age,
+   const uint16_t forward_delay)
+OVS_REQUIRES(rstp_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%"PRIu16"s\n", hello_time);
+ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
+}
+
+static void
+rstp_print_details(struct ds *ds, const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+ds_put_format(ds, " %s \n", rstp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+bool is_root = rstp_is_root_bridge(rstp);
+struct rstp_port *p = rstp_get_root_port(rstp);
+
+rstp_identifier bridge_id =
+is_root ? rstp->bridge_identifier : rstp_get_root_id(rstp);
+uint16_t hello_time =
+is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
+uint16_t max_age =
+is_root ? rstp->bridge_max_age : p->designated_times.max_age;
+uint16_t forward_delay =
+is_root ? rstp->bridge_forward_delay : 
p->designated_times.forward_delay;
+
+rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, forward_delay);
+if (is_root) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n",
+  rstp_get_root_path_cost(rstp));
+}
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+rstp_bridge_id_details(ds, rstp->bridge_identifier,
+   rstp->bridge_hello_time,
+ 

[ovs-dev] [PATCH v2 4/5] rstp: Increment the rstp port num counter.

[nickcooper-zhangtonghao]

OvS only supports RSTP_MAX_PORTS rstp ports while max port
num of stp is STP_MAX_PORTS. This patch increments the rstp
port num counter, otherwise the counter is 0 and the checking
above will always fail.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 vswitchd/bridge.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 31203d1..86b7c11 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1485,6 +1485,10 @@ port_configure_rstp(const struct ofproto *ofproto, 
struct port *port,
 port_s->port_num = 0;
 }
 
+/* Increment the port num counter, because we only support
+ * RSTP_MAX_PORTS rstp ports. */
+(*port_num_counter) ++;
+
 config_str = smap_get(>cfg->other_config, "rstp-path-cost");
 if (config_str) {
 port_s->path_cost = strtoul(config_str, NULL, 10);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v2 2/5] rstp: Add rstp port name for human reading.

[nickcooper-zhangtonghao]

This patch is useful to debug rstp subsystem and log the
port name instead of port number. This patch will also
be used to display rstp info for next patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
Acked-by: Jarno Rajahalme <ja...@ovn.org>
---
 lib/rstp-common.h  |  1 +
 lib/rstp.c | 14 +-
 lib/rstp.h |  3 ++-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index 27e8079..c108232 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -262,6 +262,7 @@ struct rstp_port {
 struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
 struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
 void *aux OVS_GUARDED_BY(rstp_mutex);
+char *port_name;
 struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
 /*
  * MAC status parameters
diff --git a/lib/rstp.c b/lib/rstp.c
index 6f1c1e3..b942f6e 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -760,6 +760,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
uint16_t port_number)
 }
 }
 
+static void
+rstp_port_set_port_name__(struct rstp_port *port, const char *name)
+OVS_REQUIRES(rstp_mutex)
+{
+free(port->port_name);
+port->port_name = xstrdup(name);
+}
+
 /* Converts the link speed to a port path cost [Table 17-3]. */
 uint32_t
 rstp_convert_speed_to_cost(unsigned int speed)
@@ -1173,6 +1181,7 @@ rstp_add_port(struct rstp *rstp)
 rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
 rstp_port_set_port_number__(p, 0);
 p->aux = NULL;
+p->port_name = NULL;
 rstp_initialize_port_defaults__(p);
 VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
  p->port_id);
@@ -1210,6 +1219,7 @@ rstp_port_unref(struct rstp_port *rp)
 ovs_mutex_lock(_mutex);
 rstp = rp->rstp;
 rstp_port_set_state__(rp, RSTP_DISABLED);
+free(rp->port_name);
 hmap_remove(>ports, >node);
 VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
  rp->port_id);
@@ -1448,13 +1458,15 @@ void
 rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
-  bool admin_port_state, bool do_mcheck, void *aux)
+  bool admin_port_state, bool do_mcheck, void *aux,
+  const char *name)
 OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 port->aux = aux;
 rstp_port_set_priority__(port, priority);
 rstp_port_set_port_number__(port, port_num);
+rstp_port_set_port_name__(port, name);
 rstp_port_set_path_cost__(port, path_cost);
 rstp_port_set_admin_edge__(port, is_admin_edge);
 rstp_port_set_auto_edge__(port, is_auto_edge);
diff --git a/lib/rstp.h b/lib/rstp.h
index 78e07fb..fa67e3c 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -221,7 +221,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
 void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
enum rstp_admin_point_to_point_mac_state 
admin_p2p_mac_state,
-   bool admin_port_state, bool do_mcheck, void *aux)
+   bool admin_port_state, bool do_mcheck, void *aux,
+   const char *name)
 OVS_EXCLUDED(rstp_mutex);
 
 enum rstp_state rstp_port_get_state(const struct rstp_port *)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index dc5f004..6d3f1fb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2722,7 +2722,7 @@ set_rstp_port(struct ofport *ofport_,
 rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
   s->admin_edge_port, s->auto_edge,
   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
-  ofport);
+  ofport, netdev_get_name(ofport->up.netdev));
 update_rstp_port_state(ofport);
 /* Synchronize operational status. */
 rstp_port_set_mac_operational(rp, ofport->may_enable);
-- 
1.8.3.1



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

[ovs-dev] [PATCH v2 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

* This patch will be used for next patch. The 'rstp/show' command,
which uses the mutex, calls functions which also use the mutex.
We should init it as a recursive mutex.

* Some rstp tests of OvS, which run with ovstest, does not run rstp_init
in the bridge_init. We should call rstp_init when creating the rstp
and stp also do that, otherwise tests fail.

* This patch remove the rstp_mutex in header file and make it static
in c file because we only use it in lib/rstp.c

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rstp.c | 15 ---
 lib/rstp.h |  6 --
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..6f1c1e3 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -50,7 +50,7 @@
 
 VLOG_DEFINE_THIS_MODULE(rstp);
 
-struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
+static struct ovs_mutex rstp_mutex;
 
 static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
 static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
_rstps__;
@@ -239,8 +239,15 @@ void
 rstp_init(void)
 OVS_EXCLUDED(rstp_mutex)
 {
-unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
- NULL);
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+ovs_mutex_init_recursive(_mutex);
+
+unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
+ NULL);
+ovsthread_once_done();
+}
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
bridge_address,
 
 VLOG_DBG("Creating RSTP instance");
 
+rstp_init();
+
 rstp = xzalloc(sizeof *rstp);
 rstp->name = xstrdup(name);
 
diff --git a/lib/rstp.h b/lib/rstp.h
index 4942d59..78e07fb 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -36,12 +36,6 @@
 #include "compiler.h"
 #include "util.h"
 
-/* Thread Safety: Callers passing in RSTP and RSTP port object
- * pointers must hold a reference to the passed object to ensure that
- * the object does not become stale while it is being accessed. */
-
-extern struct ovs_mutex rstp_mutex;
-
 #define RSTP_MAX_PORTS 4095
 
 struct dp_packet;
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH 6/6] rstp: Add the 'ovs-appctl rstp/show' command.

[nickcooper-zhangtonghao]

> On Apr 21, 2017, at 9:08 AM, Jarno Rajahalme <ja...@ovn.org> wrote:
> 
> I’d like to see one of the existing RSTP test cases modified to use this new 
> feature.
> 
> One more comment below,
> 
>  Jarno
> 
> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao <n...@opencloud.tech> 
>> wrote:
>> 
>> The rstp/show command will help users and developers to
>> get more details about rstp. This patch works together with
>> the previous patches.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
>> ---
>> NEWS   |   4 +-
>> lib/rstp.c | 113 
>> +++--
>> lib/rstp.h |   2 +-
>> vswitchd/ovs-vswitchd.8.in |  11 -
>> 4 files changed, 123 insertions(+), 7 deletions(-)
>> 
>> diff --git a/NEWS b/NEWS
>> index 00c9106..a28b8da 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -15,7 +15,9 @@ 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)).
>> +   - STP/RSTP
>> + * Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
>> +   (see ovs-vswitchd(8)).
>> 
>> v2.7.0 - 21 Feb 2017
>> -
>> diff --git a/lib/rstp.c b/lib/rstp.c
>> index b942f6e..7a4f1ea 100644
>> --- a/lib/rstp.c
>> +++ b/lib/rstp.c
>> @@ -120,6 +120,10 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
>> bool mcheck)
>>OVS_REQUIRES(rstp_mutex);
>> static void reinitialize_port__(struct rstp_port *p)
>>OVS_REQUIRES(rstp_mutex);
>> +static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
>> + const char *argv[], void *aux);
>> +static void rstp_unixctl_show(struct unixctl_conn *, int argc,
>> +  const char *argv[], void *aux);
>> 
>> const char *
>> rstp_state_name(enum rstp_state state)
>> @@ -208,9 +212,6 @@ rstp_port_get_number(const struct rstp_port *p)
>>return number;
>> }
>> 
>> -static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
>> - const char *argv[], void *aux);
>> -
>> /* Decrements the State Machines' timers. */
>> void
>> rstp_tick_timers(struct rstp *rstp)
>> @@ -246,6 +247,8 @@ rstp_init(void)
>> 
>>unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
>> rstp_unixctl_tcn,
>> NULL);
>> +unixctl_command_register("rstp/show", "[bridge]", 0, 1,
>> + rstp_unixctl_show, NULL);
>>ovsthread_once_done();
>>}
>> }
>> @@ -1398,7 +1401,7 @@ rstp_get_designated_root(const struct rstp *rstp)
>> * there is no such port.
>> */
>> struct rstp_port *
>> -rstp_get_root_port(struct rstp *rstp)
>> +rstp_get_root_port(const struct rstp *rstp)
>>OVS_EXCLUDED(rstp_mutex)
>> {
>>struct rstp_port *p;
>> @@ -1545,3 +1548,105 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
>> out:
>>ovs_mutex_unlock(_mutex);
>> }
>> +
>> +static void
>> +rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
>> +   const uint16_t hello_time, const uint16_t max_age,
>> +   const uint16_t forward_delay)
>> +OVS_REQUIRES(rstp_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%"PRIu16"s\n", hello_time);
>> +ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
>> +ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
>> +}
>> +
>> +static void
>> +rstp_print_details(struct ds *ds, const struct rstp *rstp)
>> +OVS_REQUIRES(rstp_mutex)
>> +{
>> +ds_put_format(ds, " %s \n", rstp->name);
>> +ds_put_cstr(ds, "Root ID:\n");
>> +
>> +bool is_root = rstp_is_root_

Re: [ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On Apr 21, 2017, at 8:59 AM, Jarno Rajahalme <ja...@ovn.org 
> <mailto:ja...@ovn.org>> wrote:
> 
>> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>> wrote:
>> 
>> This patch will be used for next patch.
> 
> I don’t see exactly what in the following patch(es) need this. Could you 
> elaborate?

The next patch 6/6 will call some functions which use the mutex and the 
‘rstp/show’ use it agin. 
we should init it as a recursive mutex. 

> 
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
>> ---
>> lib/rstp.c | 15 ---
>> lib/rstp.h |  6 --
>> 2 files changed, 12 insertions(+), 9 deletions(-)
>> 
>> diff --git a/lib/rstp.c b/lib/rstp.c
>> index 907a907..6f1c1e3 100644
>> --- a/lib/rstp.c
>> +++ b/lib/rstp.c
>> @@ -50,7 +50,7 @@
>> 
>> VLOG_DEFINE_THIS_MODULE(rstp);
>> 
>> -struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
>> +static struct ovs_mutex rstp_mutex;
>> 
>> static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
>> static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
>> _rstps__;
>> @@ -239,8 +239,15 @@ void
>> rstp_init(void)
>>OVS_EXCLUDED(rstp_mutex)
>> {
>> -unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
>> - NULL);
>> +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> +
>> +if (ovsthread_once_start()) {
>> +ovs_mutex_init_recursive(_mutex);
>> +
>> +unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
>> rstp_unixctl_tcn,
>> + NULL);
>> +ovsthread_once_done();
>> +}
>> }
>> 
>> /* Creates and returns a new RSTP instance that initially has no ports. */
>> @@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
>> bridge_address,
>> 
>>VLOG_DBG("Creating RSTP instance");
>> 
>> +rstp_init();
>> +
> 
> rstp_init() is already called earlier from the bridge_init(), so I see little 
> point calling it from here. Not having multiple call sites would also remove 
> the need for most of the changes above.

Yes, but some rstp testes, which run with ovstest, will not run rstp_init in 
the bridge_init.

> 
>>rstp = xzalloc(sizeof *rstp);
>>rstp->name = xstrdup(name);
>> 
>> diff --git a/lib/rstp.h b/lib/rstp.h
>> index 4942d59..78e07fb 100644
>> --- a/lib/rstp.h
>> +++ b/lib/rstp.h
>> @@ -36,12 +36,6 @@
>> #include "compiler.h"
>> #include "util.h"
>> 
>> -/* Thread Safety: Callers passing in RSTP and RSTP port object
>> - * pointers must hold a reference to the passed object to ensure that
>> - * the object does not become stale while it is being accessed. */
>> -
>> -extern struct ovs_mutex rstp_mutex;
>> -
> 
> This change, if needed, should be in a separate patch with it’s own commit 
> message.
> 
>  Jarno

Yes, If other patches will be ok, I will put it to a separate patch.




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

Re: [ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On Apr 21, 2017, at 8:59 AM, Jarno Rajahalme <ja...@ovn.org 
> <mailto:ja...@ovn.org>> wrote:
> 
>> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>> wrote:
>> 
>> This patch will be used for next patch.
> 
> I don’t see exactly what in the following patch(es) need this. Could you 
> elaborate?

The next patch 6/6 will call some functions which use the mutex and the 
‘rstp/show’ use it agin. 
we should init it as a recursive mutex. 

> 
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
>> ---
>> lib/rstp.c | 15 ---
>> lib/rstp.h |  6 --
>> 2 files changed, 12 insertions(+), 9 deletions(-)
>> 
>> diff --git a/lib/rstp.c b/lib/rstp.c
>> index 907a907..6f1c1e3 100644
>> --- a/lib/rstp.c
>> +++ b/lib/rstp.c
>> @@ -50,7 +50,7 @@
>> 
>> VLOG_DEFINE_THIS_MODULE(rstp);
>> 
>> -struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
>> +static struct ovs_mutex rstp_mutex;
>> 
>> static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
>> static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
>> _rstps__;
>> @@ -239,8 +239,15 @@ void
>> rstp_init(void)
>>OVS_EXCLUDED(rstp_mutex)
>> {
>> -unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
>> - NULL);
>> +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> +
>> +if (ovsthread_once_start()) {
>> +ovs_mutex_init_recursive(_mutex);
>> +
>> +unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
>> rstp_unixctl_tcn,
>> + NULL);
>> +ovsthread_once_done();
>> +}
>> }
>> 
>> /* Creates and returns a new RSTP instance that initially has no ports. */
>> @@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
>> bridge_address,
>> 
>>VLOG_DBG("Creating RSTP instance");
>> 
>> +rstp_init();
>> +
> 
> rstp_init() is already called earlier from the bridge_init(), so I see little 
> point calling it from here. Not having multiple call sites would also remove 
> the need for most of the changes above.

Yes, but some rstp testes, which run with ovstest, will not run rstp_init in 
the bridge_init.

> 
>>rstp = xzalloc(sizeof *rstp);
>>rstp->name = xstrdup(name);
>> 
>> diff --git a/lib/rstp.h b/lib/rstp.h
>> index 4942d59..78e07fb 100644
>> --- a/lib/rstp.h
>> +++ b/lib/rstp.h
>> @@ -36,12 +36,6 @@
>> #include "compiler.h"
>> #include "util.h"
>> 
>> -/* Thread Safety: Callers passing in RSTP and RSTP port object
>> - * pointers must hold a reference to the passed object to ensure that
>> - * the object does not become stale while it is being accessed. */
>> -
>> -extern struct ovs_mutex rstp_mutex;
>> -
> 
> This change, if needed, should be in a separate patch with it’s own commit 
> message.
> 
>  Jarno

Yes, If other patches will be ok, I will put it to a separate patch.




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

Re: [ovs-dev] [PATCH 2/6] stp: Use OpenFlow port number for stp ports.

[nickcooper-zhangtonghao]

> On Apr 21, 2017, at 8:34 AM, Jarno Rajahalme <ja...@ovn.org> wrote:
> 
>> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>> wrote:
>> 
>> When a bridge stp enabled, we assign sequentially element
>> of stp_port array (in stp struct) to bridge ports. That is
>> ok when no ports are added to bridge. When adding a port
>> to bridge which stp enabled, the ovs-vswitchd will assign
>> stp_port sequentially again. Then the stp_port belonging
>> to one port may belong to other one.
> 
> Could you elaborate the problem with this STP port renumbering?

When we add a port to bridge which stp is enabled, the stp ports are renumbered.

For example:
p0 is mapped to one array element of struct stp_port ports[STP_MAX_PORTS] (e.g. 
ports[0])
p1 is mapped to one array element of struct stp_port ports[STP_MAX_PORTS] (e.g. 
ports[1])

When we add a port p2 to bridge, the p2 may be mapped to ports[0], p0 is mapped 
to ports[1], and  p1 is mapped to ports[2].
But we hope that p2 may be mapped to ports[2], p0 is mapped to ports[0], and  
p1 is mapped to ports[1].
If not, the stp ports of bridge will converge. As a general rule, the state of 
new port should be changed
(e.g goto forwarding state or blocking.) and other stp ports remain the same.


Test shell:

#!/bin/bash
ovs-vsctl add-br br0
ovs-vsctl add-br br1

ovs-appctl vlog/set ofproto_dpif:dbg

ovs-vsctl add-port br0 p1 -- \
set interface p1 type=dummy options:pstream=punix:/tmp/p1.sock 
ofport_request=1

ovs-vsctl add-port br0 p2 -- \
set interface p2 type=dummy options:pstream=punix:/tmp/p2.sock 
ofport_request=2

ovs-vsctl add-port br1 p6 -- \
set interface p6 type=dummy options:stream=unix:/tmp/p1.sock 
ofport_request=6

ovs-vsctl add-port br1 p7 -- \
set interface p7 type=dummy options:stream=unix:/tmp/p2.sock 
ofport_request=7

ovs-ofctl add-flow br0 action=normal
ovs-ofctl add-flow br1 action=normal

ovs-appctl netdev-dummy/set-admin-state up

ovs-vsctl set port br0 other_config:stp-enable=false -- \
set bridge br0 stp_enable=true other-config:hwaddr=aa:66:aa:66:00:00

ovs-vsctl set port br1 other_config:stp-enable=false -- \
set bridge br1 stp_enable=true other-config:hwaddr=aa:66:aa:66:00:01


# ovs-appctl stp/show
<---cut--->
Bridge ID:
stp-priority32768
stp-system-id   aa:66:aa:66:00:01
stp-hello-time  2s
stp-max-age 20s
stp-fwd-delay   15s

Interface  Role   State  Cost  Pri.Nbr
-- -- -- - ---
p6 alternate  blocking   19128.1
p7 root   forwarding 19128.2


# ovs-vsctl add-port br1 p8
# ovs-appctl stp/show
<---cut--->
Bridge ID:
stp-priority32768
stp-system-id   aa:66:aa:66:00:01
stp-hello-time  2s
stp-max-age 20s
stp-fwd-delay   15s

Interface  Role   State  Cost  Pri.Nbr
-- -- -- - ---
p8 alternate  blocking   19128.1
p6 root   forwarding 19128.2
p7 designated listening  19128.3


After adding the p8 port, p8 uses the p6 state, p6 uses the p7 state, and p7 
uses the new state.
The rstp works well without elaboration.

>> This patch uses the
>> OpenFlow port numbers instead of sequence numbers to avoid
>> it.
>> 
> 
> Using OpenFlow port numbers (32-bit) as STP port numbers (8-bit) seems wrong 
> to me. Besides, you can always use the other-config stp-port-num in ovsdb to 
> specify which STP port number you want, if you do not want them to be 
> numbered automatically.

Yes, the max number of stp ports on bridge is STP_MAX_PORTS(255), and this 
should not be problem. Assigning the openflow port num to stp port is simple. 
But we can use other way to assign stp port number. If you have any idea, 
please let me know.









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

Re: [ovs-dev] [PATCH 1/6] rstp/stp: Unref the rstp/stp when bridges destroyed.

[nickcooper-zhangtonghao]

who can help me to review patches ? Thanks very much.

stp:
http://patchwork.ozlabs.org/patch/745880/ 
<http://patchwork.ozlabs.org/patch/745880/>
http://patchwork.ozlabs.org/patch/745881/ 
<http://patchwork.ozlabs.org/patch/745881/>
http://patchwork.ozlabs.org/patch/745884/ 
<http://patchwork.ozlabs.org/patch/745884/>
http://patchwork.ozlabs.org/patch/745882/ 
<http://patchwork.ozlabs.org/patch/745882/>
http://patchwork.ozlabs.org/patch/745883/ 
<http://patchwork.ozlabs.org/patch/745883/>
http://patchwork.ozlabs.org/patch/745885/ 
<http://patchwork.ozlabs.org/patch/745885/>

ofproto:
http://patchwork.ozlabs.org/patch/743155/ 
<http://patchwork.ozlabs.org/patch/743155/>


Thanks.
Nick

> On Apr 1, 2017, at 11:11 AM, nickcooper-zhangtonghao <n...@opencloud.tech> 
> wrote:
> 
> When bridges destroyed, which stp enabled, you can
> still get stp info via the command 'ovs-appctl stp/show'.
> And the rstp is also in the same case. We should unref
> them. The rstp/stp ports have been unregistered via
> 'ofproto_port_unregister' function when ports destroyed.
> We will unref rstp/stp struct in the 'destruct' of
> ofproto-dpif provider.
> 
> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
> <mailto:n...@opencloud.tech>>
> ---
> ofproto/ofproto-dpif.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 523adad..4beacda 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1494,6 +1494,8 @@ destruct(struct ofproto *ofproto_)
> hmap_destroy(>bundles);
> mac_learning_unref(ofproto->ml);
> mcast_snooping_unref(ofproto->ms);
> +stp_unref(ofproto->stp);
> +rstp_unref(ofproto->rstp);
> 
> sset_destroy(>ports);
> sset_destroy(>ghost_ports);
> -- 
> 1.8.3.1

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

[ovs-dev] [PATCH 6/6] rstp: Add the 'ovs-appctl rstp/show' command.

[nickcooper-zhangtonghao]

The rstp/show command will help users and developers to
get more details about rstp. This patch works together with
the previous patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 NEWS   |   4 +-
 lib/rstp.c | 113 +++--
 lib/rstp.h |   2 +-
 vswitchd/ovs-vswitchd.8.in |  11 -
 4 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 00c9106..a28b8da 100644
--- a/NEWS
+++ b/NEWS
@@ -15,7 +15,9 @@ 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)).
+   - STP/RSTP
+ * Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
+   (see ovs-vswitchd(8)).
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/lib/rstp.c b/lib/rstp.c
index b942f6e..7a4f1ea 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -120,6 +120,10 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
bool mcheck)
 OVS_REQUIRES(rstp_mutex);
 static void reinitialize_port__(struct rstp_port *p)
 OVS_REQUIRES(rstp_mutex);
+static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
+ const char *argv[], void *aux);
+static void rstp_unixctl_show(struct unixctl_conn *, int argc,
+  const char *argv[], void *aux);
 
 const char *
 rstp_state_name(enum rstp_state state)
@@ -208,9 +212,6 @@ rstp_port_get_number(const struct rstp_port *p)
 return number;
 }
 
-static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
- const char *argv[], void *aux);
-
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
@@ -246,6 +247,8 @@ rstp_init(void)
 
 unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
  NULL);
+unixctl_command_register("rstp/show", "[bridge]", 0, 1,
+ rstp_unixctl_show, NULL);
 ovsthread_once_done();
 }
 }
@@ -1398,7 +1401,7 @@ rstp_get_designated_root(const struct rstp *rstp)
  * there is no such port.
  */
 struct rstp_port *
-rstp_get_root_port(struct rstp *rstp)
+rstp_get_root_port(const struct rstp *rstp)
 OVS_EXCLUDED(rstp_mutex)
 {
 struct rstp_port *p;
@@ -1545,3 +1548,105 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
 out:
 ovs_mutex_unlock(_mutex);
 }
+
+static void
+rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
+   const uint16_t hello_time, const uint16_t max_age,
+   const uint16_t forward_delay)
+OVS_REQUIRES(rstp_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%"PRIu16"s\n", hello_time);
+ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
+}
+
+static void
+rstp_print_details(struct ds *ds, const struct rstp *rstp)
+OVS_REQUIRES(rstp_mutex)
+{
+ds_put_format(ds, " %s \n", rstp->name);
+ds_put_cstr(ds, "Root ID:\n");
+
+bool is_root = rstp_is_root_bridge(rstp);
+struct rstp_port *p = rstp_get_root_port(rstp);
+
+rstp_identifier bridge_id =
+is_root ? rstp->bridge_identifier : rstp_get_root_id(rstp);
+uint16_t hello_time =
+is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
+uint16_t max_age =
+is_root ? rstp->bridge_max_age : p->designated_times.max_age;
+uint16_t forward_delay =
+is_root ? rstp->bridge_forward_delay : 
p->designated_times.forward_delay;
+
+rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, forward_delay);
+if (is_root) {
+ds_put_cstr(ds, "\tThis bridge is the root\n");
+} else {
+ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
+ds_put_format(ds, "\troot-path-cost\t%u\n",
+  rstp_get_root_path_cost(rstp));
+}
+ds_put_cstr(ds, "\n");
+
+ds_put_cstr(ds, "Bridge ID:\n");
+rstp_bridge_id_details(ds, rstp->bridge_identifier,
+   rstp->bridge_hello_time,
+   rstp->bridge_max_age,
+   rstp->bridge_forward_delay);
+ds_put_cstr(ds, "\n");
+
+d

[ovs-dev] [PATCH 3/6] stp: Add link-state checking support for stp ports.

[nickcooper-zhangtonghao]

When bridge stp enabled, we enable the stp ports despite
ports are down. When initializing, this patch checks
link-state of ports and enable or disable them according
to their link-state. This patch also allow user to enable
and disable a port when bridge stp is running.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 41 +++-
 tests/stp.at   | 73 ++
 2 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 4beacda..f015131 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2488,6 +2488,37 @@ update_stp_port_state(struct ofport_dpif *ofport)
 }
 }
 
+static void
+stp_check_and_update_link_state(struct ofproto_dpif *ofproto)
+{
+struct ofport *ofport_;
+struct ofport_dpif *ofport;
+bool up;
+
+HMAP_FOR_EACH (ofport_, hmap_node, >up.ports) {
+ofport = ofport_dpif_cast(ofport_);
+up = netdev_get_carrier(ofport_->netdev);
+
+if (ofport->stp_port &&
+up != (stp_port_get_state(ofport->stp_port) != STP_DISABLED)) {
+
+VLOG_DBG("bridge: %s, port: %s is %s, %s it", ofproto->up.name,
+ netdev_get_name(ofport->up.netdev),
+ up ? "up" : "down",
+ up ? "enabling" : "disabling");
+
+if (up) {
+stp_port_enable(ofport->stp_port);
+stp_port_set_aux(ofport->stp_port, ofport);
+} else {
+stp_port_disable(ofport->stp_port);
+}
+
+update_stp_port_state(ofport);
+}
+}
+}
+
 /* Configures STP on 'ofport_' using the settings defined in 's'.  The
  * caller is responsible for assigning STP port numbers and ensuring
  * there are no duplicates. */
@@ -2518,7 +2549,12 @@ set_stp_port(struct ofport *ofport_,
 /* Set name before enabling the port so that debugging messages can print
  * the name. */
 stp_port_set_name(sp, netdev_get_name(ofport->up.netdev));
-stp_port_enable(sp);
+
+if (netdev_get_carrier(ofport_->netdev)) {
+stp_port_enable(sp);
+} else {
+stp_port_disable(sp);
+}
 
 stp_port_set_aux(sp, ofport);
 stp_port_set_priority(sp, s->priority);
@@ -2580,6 +2616,9 @@ stp_run(struct ofproto_dpif *ofproto)
 stp_tick(ofproto->stp, MIN(INT_MAX, elapsed));
 ofproto->stp_last_tick = now;
 }
+
+stp_check_and_update_link_state(ofproto);
+
 while (stp_get_changed_port(ofproto->stp, )) {
 struct ofport_dpif *ofport = stp_port_get_aux(sp);
 
diff --git a/tests/stp.at b/tests/stp.at
index 98632a8..de8f971 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -420,6 +420,8 @@ AT_CHECK([ovs-vsctl add-port br1 p8 -- \
set port p8 other_config:stp-enable=false -- \
 ])
 
+ovs-appctl netdev-dummy/set-admin-state up
+
 ovs-appctl time/stop
 
 AT_CHECK([ovs-ofctl add-flow br0 "in_port=7 icmp actions=1"])
@@ -519,6 +521,7 @@ AT_CHECK([
 set interface p6 type=dummy options:pstream=punix:$OVS_RUNDIR/p6.sock 
ofport_request=6
 ], [0])
 
+ovs-appctl netdev-dummy/set-admin-state up
 
 ovs-appctl time/stop
 
@@ -633,6 +636,8 @@ AT_CHECK([
 set interface p2 type=dummy ofport_request=2
 ], [0])
 
+ovs-appctl netdev-dummy/set-admin-state up
+
 ovs-appctl time/stop
 
 # give time for STP to move initially
@@ -653,6 +658,8 @@ AT_CHECK([
 set interface p3 type=dummy ofport_request=3
 ], [0])
 
+ovs-appctl netdev-dummy/set-admin-state p3 up
+
 # The new stp port should be a listening state and other
 # stp ports keep forwarding.
 AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
@@ -676,5 +683,71 @@ AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl
p3 designated listening  19128.3
 ])
 
+AT_CLEANUP
+
+AT_SETUP([STP - check link-state when stp is running])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl -- \
+set port br0 other_config:stp-enable=false -- \
+set bridge br0 datapath-type=dummy stp_enable=true \
+other-config:hwaddr=aa:66:aa:66:00:00
+], [0])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- \
+set interface p1 type=dummy ofport_request=1
+ovs-vsctl add-port br0 p2 -- \
+set interface p2 type=dummy ofport_request=2
+], [0])
+
+ovs-appctl netdev-dummy/set-admin-state up
+
+ovs-appctl time/stop
+
+# give time for STP to move initially
+for i in $(seq 0 30); do
+ovs-appctl time/warp 1000
+done
+
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+
+# add a stp port
+AT_CHECK([
+ovs-vsctl add-port br0 p3 -- \
+

[ovs-dev] [PATCH 5/6] rstp: Add rstp port name for human reading.

[nickcooper-zhangtonghao]

This patch is useful to debug rstp subsystem and log the
port name instead of port number. This patch will also
be used to display rstp info for next patches.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rstp-common.h  |  1 +
 lib/rstp.c | 14 +-
 lib/rstp.h |  3 ++-
 ofproto/ofproto-dpif.c |  2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index 27e8079..c108232 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -262,6 +262,7 @@ struct rstp_port {
 struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
 struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
 void *aux OVS_GUARDED_BY(rstp_mutex);
+char *port_name;
 struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
 /*
  * MAC status parameters
diff --git a/lib/rstp.c b/lib/rstp.c
index 6f1c1e3..b942f6e 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -760,6 +760,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
uint16_t port_number)
 }
 }
 
+static void
+rstp_port_set_port_name__(struct rstp_port *port, const char *name)
+OVS_REQUIRES(rstp_mutex)
+{
+free(port->port_name);
+port->port_name = xstrdup(name);
+}
+
 /* Converts the link speed to a port path cost [Table 17-3]. */
 uint32_t
 rstp_convert_speed_to_cost(unsigned int speed)
@@ -1173,6 +1181,7 @@ rstp_add_port(struct rstp *rstp)
 rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
 rstp_port_set_port_number__(p, 0);
 p->aux = NULL;
+p->port_name = NULL;
 rstp_initialize_port_defaults__(p);
 VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
  p->port_id);
@@ -1210,6 +1219,7 @@ rstp_port_unref(struct rstp_port *rp)
 ovs_mutex_lock(_mutex);
 rstp = rp->rstp;
 rstp_port_set_state__(rp, RSTP_DISABLED);
+free(rp->port_name);
 hmap_remove(>ports, >node);
 VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
  rp->port_id);
@@ -1448,13 +1458,15 @@ void
 rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
-  bool admin_port_state, bool do_mcheck, void *aux)
+  bool admin_port_state, bool do_mcheck, void *aux,
+  const char *name)
 OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 port->aux = aux;
 rstp_port_set_priority__(port, priority);
 rstp_port_set_port_number__(port, port_num);
+rstp_port_set_port_name__(port, name);
 rstp_port_set_path_cost__(port, path_cost);
 rstp_port_set_admin_edge__(port, is_admin_edge);
 rstp_port_set_auto_edge__(port, is_auto_edge);
diff --git a/lib/rstp.h b/lib/rstp.h
index 78e07fb..fa67e3c 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -221,7 +221,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
 void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
enum rstp_admin_point_to_point_mac_state 
admin_p2p_mac_state,
-   bool admin_port_state, bool do_mcheck, void *aux)
+   bool admin_port_state, bool do_mcheck, void *aux,
+   const char *name)
 OVS_EXCLUDED(rstp_mutex);
 
 enum rstp_state rstp_port_get_state(const struct rstp_port *)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f015131..d41d90f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2675,7 +2675,7 @@ set_rstp_port(struct ofport *ofport_,
 rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
   s->admin_edge_port, s->auto_edge,
   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
-  ofport);
+  ofport, netdev_get_name(ofport->up.netdev));
 update_rstp_port_state(ofport);
 /* Synchronize operational status. */
 rstp_port_set_mac_operational(rp, ofport->may_enable);
-- 
1.8.3.1



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

[ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

This patch will be used for next patch.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rstp.c | 15 ---
 lib/rstp.h |  6 --
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..6f1c1e3 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -50,7 +50,7 @@
 
 VLOG_DEFINE_THIS_MODULE(rstp);
 
-struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
+static struct ovs_mutex rstp_mutex;
 
 static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
 static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
_rstps__;
@@ -239,8 +239,15 @@ void
 rstp_init(void)
 OVS_EXCLUDED(rstp_mutex)
 {
-unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
- NULL);
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+ovs_mutex_init_recursive(_mutex);
+
+unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
+ NULL);
+ovsthread_once_done();
+}
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
bridge_address,
 
 VLOG_DBG("Creating RSTP instance");
 
+rstp_init();
+
 rstp = xzalloc(sizeof *rstp);
 rstp->name = xstrdup(name);
 
diff --git a/lib/rstp.h b/lib/rstp.h
index 4942d59..78e07fb 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -36,12 +36,6 @@
 #include "compiler.h"
 #include "util.h"
 
-/* Thread Safety: Callers passing in RSTP and RSTP port object
- * pointers must hold a reference to the passed object to ensure that
- * the object does not become stale while it is being accessed. */
-
-extern struct ovs_mutex rstp_mutex;
-
 #define RSTP_MAX_PORTS 4095
 
 struct dp_packet;
-- 
1.8.3.1



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

[ovs-dev] [PATCH 2/6] stp: Use OpenFlow port number for stp ports.

[nickcooper-zhangtonghao]

When a bridge stp enabled, we assign sequentially element
of stp_port array (in stp struct) to bridge ports. That is
ok when no ports are added to bridge. When adding a port
to bridge which stp enabled, the ovs-vswitchd will assign
stp_port sequentially again. Then the stp_port belonging
to one port may belong to other one. This patch uses the
OpenFlow port numbers instead of sequence numbers to avoid
it.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 tests/stp.at  | 90 ++-
 vswitchd/bridge.c | 10 +--
 2 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/tests/stp.at b/tests/stp.at
index bd5d208..98632a8 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -570,11 +570,6 @@ for i in $(seq 0 35); do
 done
 
 # root bridge sends query packet
-# we don't want to lose that message, so send it twice
-AT_CHECK([ovs-appctl netdev-dummy/receive br0 \
-
'01005E010101000C29A027D181010800451C00014002CBCBAC102201E0010104EEEB'])
-
-ovs-appctl time/warp 1000
 AT_CHECK([ovs-appctl netdev-dummy/receive br0 \
 
'01005E010101000C29A027D181010800451C00014002CBCBAC102201E0010104EEEB'])
 
@@ -589,21 +584,14 @@ OVS_WAIT_UNTIL([ovs-appctl mdb/show br2 | grep 'querier'])
 # del p2 on the br0, the topology will be changed
 AT_CHECK([ovs-vsctl del-port br0 p2])
 
-# give time for STP to synchronize
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
+# We give time for STP to synchronize. The message age of
+# p6 port will time out after 20s. The p5 will left blocked
+# state to forwarding after 30s and then br2 bridge will
+# detect the topology change, flush the fdb and sent tcn BPDU.
+# br1 and br0 will also flush fdb when receiving tcn BPDU.
+for i in $(seq 0 52); do
+ovs-appctl time/warp 1000
+done
 
 # check fdb and mdb
 AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl
@@ -626,5 +614,67 @@ AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl
  port  VLAN  GROUPAge
 ])
 
+AT_CLEANUP
+
+AT_SETUP([STP - check the stp ports num])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl -- \
+set port br0 other_config:stp-enable=false -- \
+set bridge br0 datapath-type=dummy stp_enable=true \
+other-config:hwaddr=aa:66:aa:66:00:00
+], [0])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- \
+set interface p1 type=dummy ofport_request=1
+ovs-vsctl add-port br0 p2 -- \
+set interface p2 type=dummy ofport_request=2
+], [0])
+
+ovs-appctl time/stop
+
+# give time for STP to move initially
+for i in $(seq 0 30); do
+ovs-appctl time/warp 1000
+done
+
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+
+# add a stp port
+AT_CHECK([
+ovs-vsctl add-port br0 p3 -- \
+set interface p3 type=dummy ofport_request=3
+], [0])
+
+# The new stp port should be a listening state and other
+# stp ports keep forwarding.
+AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
+   p1 designated forwarding 19128.1
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl
+   p3 designated listening  19128.3
+])
+
+# delete p1 stp port
+AT_CHECK([ovs-vsctl del-port br0 p1])
+
+# The other stp ports keep original state.
+AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl
+   p2 designated forwarding 19128.2
+])
+AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl
+   p3 designated listening  19128.3
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 867a26d..d683000 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1396,13 +1396,17 @@ port_configure_stp(const struct ofproto *ofproto, 
struct port *port,
 bitmap_set1(port_num_bitmap, port_idx);
 port_s->port_num = port_idx;
 } else {
-if (*port_num_counter >= STP_MAX_PORTS) {
-VLOG_ERR("port %s: too many STP ports, disabling", port->name);
+if (iface->ofp_port > STP_MAX_PORTS) {
+VLOG_ERR("port %s: too many STP ports or OpenFlow port number "
+ "(%d) > STP_MAX_PORTS (%d), disabling", port->name,
+ iface->ofp_port, STP_MAX_PORTS);
+
 port_s->enable = false;
 return;
 }
 
-port_s->port_num = (*port_num_counter)++;
+

[ovs-dev] [PATCH 1/6] rstp/stp: Unref the rstp/stp when bridges destroyed.

[nickcooper-zhangtonghao]

When bridges destroyed, which stp enabled, you can
still get stp info via the command 'ovs-appctl stp/show'.
And the rstp is also in the same case. We should unref
them. The rstp/stp ports have been unregistered via
'ofproto_port_unregister' function when ports destroyed.
We will unref rstp/stp struct in the 'destruct' of
ofproto-dpif provider.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 523adad..4beacda 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1494,6 +1494,8 @@ destruct(struct ofproto *ofproto_)
 hmap_destroy(>bundles);
 mac_learning_unref(ofproto->ml);
 mcast_snooping_unref(ofproto->ms);
+stp_unref(ofproto->stp);
+rstp_unref(ofproto->rstp);
 
 sset_destroy(>ports);
 sset_destroy(>ghost_ports);
-- 
1.8.3.1




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

[ovs-dev] [PATCH] ofproto: Limit log rate when controller disconnected.

[nickcooper-zhangtonghao]

There are a lot of logs when OvS bridges can’t connect
to controllers. The controller may stop or something
causes a disruption in network traffic.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/rconn.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/rconn.c b/lib/rconn.c
index 8a29864..8ba5106 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -455,9 +455,10 @@ reconnect(struct rconn *rc)
 OVS_REQUIRES(rc->mutex)
 {
 int retval;
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
 
 if (rconn_logging_connection_attempts__(rc)) {
-VLOG_INFO("%s: connecting...", rc->name);
+VLOG_INFO_RL(, "%s: connecting...", rc->name);
 }
 rc->n_attempted_connections++;
 retval = vconn_open(rc->target, rc->allowed_versions, rc->dscp,
@@ -466,8 +467,8 @@ reconnect(struct rconn *rc)
 rc->backoff_deadline = time_now() + rc->backoff;
 state_transition(rc, S_CONNECTING);
 } else {
-VLOG_WARN("%s: connection failed (%s)",
-  rc->name, ovs_strerror(retval));
+VLOG_WARN_RL(, "%s: connection failed (%s)",
+ rc->name, ovs_strerror(retval));
 rc->backoff_deadline = TIME_MAX; /* Prevent resetting backoff. */
 disconnect(rc, retval);
 }
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH] tests: Add and improve stp tests.

[nickcooper-zhangtonghao]

Hi, the new patch has been submitted. It fixes the issue on
ubuntu and newer kernel (4.10.4). You can review and tested it.

http://patchwork.ozlabs.org/patch/742517/ 

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

[ovs-dev] [PATCH v2] stp: Fix stp tests and make them more stable.

[nickcooper-zhangtonghao]

The difference between machines may cause the test to fail.
More importantly, when topology is changed or the root brdige
receives the TCN BPDU, the root bridge will start the topology
change timer. We should wait the topology change timer to stop
after 35s (max age 20 + forward delay 15). After 35s, the root
bridge will stop send CONF BPDU with STP_CONFIG_TOPOLOGY_CHANGE
flag and the topology will be stable. During this time, we should
make time warp (in a second) because the hold timer of stp ports
will stop after 1s. Then the root bridge can send quickly topology
change ack (other bridges may send TCN BPDU to root bridge) for
avoiding root brdige to flush fdb and mdb frequently.

This patch has been tested on centos 7.2 (kernel 3.10.0, python
2.7.5 and gcc 4.8.5), ubuntu 16.04 (kernel 4.4.0, python 3.5.2
and gcc 5.4.0) and ubuntu 16.04 (kernel 4.10.4, python 3.5.2 and
gcc 5.4.0). This patch has been tested for 3 hours. This patch
may make the stp tests more stable.

Fixes: 427e9751f300 ("tests: Add and improve stp tests.")
Reported-at: http://paste.ubuntu.com/24215426
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/330032.html
Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 tests/stp.at | 61 ++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/tests/stp.at b/tests/stp.at
index 20f7940..a71cf80 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -377,6 +377,11 @@ grep 'STP state change' | sed '
 s/.*ofproto_dpif|.*|port .*:/port <>:/
 ']])
 
+m4_define([FILTER_STP_TOPOLOGY_LISTENING], [[
+grep 'disabled to listening' | sed '
+  s/.*ofproto_dpif|.*|port .*:/port <>:/
+']])
+
 m4_define([FILTER_STP_TOPOLOGY_FORWARDING], [[
 grep 'learning to forwarding' | sed '
   s/.*ofproto_dpif|.*|port .*:/port <>:/
@@ -427,7 +432,7 @@ AT_CHECK([ovs-ofctl add-flow br1 "in_port=2 icmp 
actions=8"])
 ovs-appctl time/warp 3000
 ovs-appctl time/warp 3000
 
-AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
+AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
 port <>: STP state changed from disabled to listening
 port <>: STP state changed from disabled to listening
 ])
@@ -493,6 +498,7 @@ AT_CHECK([
 ], [0])
 
 AT_CHECK([ovs-appctl vlog/set ofproto_dpif:dbg])
+AT_CHECK([ovs-appctl vlog/set ofproto_dpif_xlate:dbg])
 
 AT_CHECK([ovs-ofctl add-flow br0 action=normal])
 AT_CHECK([ovs-ofctl add-flow br1 action=normal])
@@ -520,7 +526,7 @@ ovs-appctl time/stop
 ovs-appctl time/warp 3000
 ovs-appctl time/warp 3000
 
-AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
+AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
 port <>: STP state changed from disabled to listening
 port <>: STP state changed from disabled to listening
 port <>: STP state changed from disabled to listening
@@ -550,6 +556,57 @@ port <>: STP state changed from learning to forwarding
 port <>: STP state changed from learning to forwarding
 ])
 
+# When topology is changed or the root brdige receives the TCN BPDU, the
+# root bridge will start the topology change timer. We should wait the
+# topology change timer to stop after 35s (max age 20 + forward delay 15).
+# After 35s, the root bridge will stop send CONF BPDU with
+# STP_CONFIG_TOPOLOGY_CHANGE flag and the topology will be stable. More
+# importantly, we should make time warp (in a second) because the hold timer
+# of stp ports will stop after 1s. So the root bridge can send quickly
+# topology change ack (other bridges may send TCN BPDU to root bridge) for
+# avoiding root brdige to flush fdb and mdb frequently.
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+ovs-appctl time/warp 1000
+
 # root bridge sends query packet
 # we don't want to lose that message, so send it twice
 AT_CHECK([ovs-appctl netdev-dummy/receive br0 \
-- 
1.8.3.1

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

[ovs-dev] [PATCH] rstp/stp: Unref the rstp/stp when bridges destroyed.

[nickcooper-zhangtonghao]

When bridges destroyed, which stp enabled, you can
still get stp info via the command 'ovs-appctl stp/show'.
And the rstp is also in the same case. We should unref
them. The rstp/stp ports have been unregistered via
'ofproto_port_unregister' function when ports destroyed.
We will unref rstp/stp struct in the 'destruct' of
ofproto-dpif provider.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 523adad..4beacda 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1494,6 +1494,8 @@ destruct(struct ofproto *ofproto_)
 hmap_destroy(>bundles);
 mac_learning_unref(ofproto->ml);
 mcast_snooping_unref(ofproto->ms);
+stp_unref(ofproto->stp);
+rstp_unref(ofproto->rstp);
 
 sset_destroy(>ports);
 sset_destroy(>ghost_ports);
-- 
1.8.3.1




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

[ovs-dev] [PATCH] stp: Fix stp test.

[nickcooper-zhangtonghao]

The difference between machines may cause the test to fail.
This patch has been tested on centos 7.2 (kernel 3.10.0,
python 2.7.5 and gcc 4.8.5) and ubuntu 16.04 (kernel 4.4.0,
python 3.5.2 and 5.4.0). This patch may make the stp test
more stable.

Fixes: 427e9751f300 ("tests: Add and improve stp tests.")
Reported-at: http://paste.ubuntu.com/24215426
Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 tests/stp.at | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/stp.at b/tests/stp.at
index 20f7940..542e700 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -377,6 +377,11 @@ grep 'STP state change' | sed '
 s/.*ofproto_dpif|.*|port .*:/port <>:/
 ']])
 
+m4_define([FILTER_STP_TOPOLOGY_LISTENING], [[
+grep 'disabled to listening' | sed '
+  s/.*ofproto_dpif|.*|port .*:/port <>:/
+']])
+
 m4_define([FILTER_STP_TOPOLOGY_FORWARDING], [[
 grep 'learning to forwarding' | sed '
   s/.*ofproto_dpif|.*|port .*:/port <>:/
@@ -427,7 +432,7 @@ AT_CHECK([ovs-ofctl add-flow br1 "in_port=2 icmp 
actions=8"])
 ovs-appctl time/warp 3000
 ovs-appctl time/warp 3000
 
-AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
+AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
 port <>: STP state changed from disabled to listening
 port <>: STP state changed from disabled to listening
 ])
@@ -520,7 +525,7 @@ ovs-appctl time/stop
 ovs-appctl time/warp 3000
 ovs-appctl time/warp 3000
 
-AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
+AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
 port <>: STP state changed from disabled to listening
 port <>: STP state changed from disabled to listening
 port <>: STP state changed from disabled to listening
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH] tests: Add and improve stp tests.

[nickcooper-zhangtonghao]

I tested it via command as below for a long time.

while true; do make check TESTSUITEFLAGS='-k stp'; done

But it’s ok. I try to find what might cause this.


Thanks.
Nick

> On Mar 21, 2017, at 5:24 AM, Joe Stringer  wrote:
> 
> Right, Travis seems happy too so most likely it's something to do with
> my setup. I'm open to ideas about what might cause this.

___
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 <n...@opencloud.tech>
Acked-by: Ben Pfaff <b...@ovn.org>
---
 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 <n...@opencloud.tech>
Co-authored-by: Ben Pfaff <b...@ovn.org>
---
 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);
+  

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 <b...@ovn.org> 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 <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
> 
> 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 <b...@ovn.org> 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 <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
> 
> 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] [PATCH 1/4] stp: Change the api for next patch.

[nickcooper-zhangtonghao]

Is there someone can help me to review stp patches?

http://patchwork.ozlabs.org/patch/736157/ 
<http://patchwork.ozlabs.org/patch/736157/>
http://patchwork.ozlabs.org/patch/736158/ 
<http://patchwork.ozlabs.org/patch/736158/>
http://patchwork.ozlabs.org/patch/736159/ 
<http://patchwork.ozlabs.org/patch/736159/>
http://patchwork.ozlabs.org/patch/736229/ 
<http://patchwork.ozlabs.org/patch/736229/>
http://patchwork.ozlabs.org/patch/736548/ 
<http://patchwork.ozlabs.org/patch/736548/>



Thanks.
Nick

> On Mar 7, 2017, at 9:11 PM, nickcooper-zhangtonghao <n...@opencloud.tech> 
> wrote:
> 
> 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 <n...@opencloud.tech 
> <mailto:n...@opencloud.tech>>

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

Re: [ovs-dev] [PATCH] debian; Avoid installing ovs-vswitchd.conf.db manpage as "db" language.

[nickcooper-zhangtonghao]

I have tested it. It is ok now. Simon, what do you think about it?


Tested-by: nickcooper-zhangtonghao <n...@opencloud.tech>

> On Mar 9, 2017, at 2:22 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> Simon, would you mind reviewing this, since it's about the Debian
> packaging?

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

[ovs-dev] [PATCH] debian: Rewrite "ifconfig" to "ip" command.

[nickcooper-zhangtonghao]

Some debian distribution may not contain the ifconfig.
We use the ip command instead of ifconfig in debian/ifupdown.sh

Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329503.html
CC: prochazka <procha...@cortex.cz>
Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 debian/ifupdown.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/debian/ifupdown.sh b/debian/ifupdown.sh
index fe15b71..c917e62 100755
--- a/debian/ifupdown.sh
+++ b/debian/ifupdown.sh
@@ -50,24 +50,24 @@ if [ "${MODE}" = "start" ]; then
 "${IFACE}" ${IF_OVS_OPTIONS} \
 ${OVS_EXTRA+-- $OVS_EXTRA}
 
-ifconfig "${IFACE}" up
+ip link set "${IFACE}" up
 ;;
 OVSIntPort)
 ovs_vsctl -- --may-exist add-port "${IF_OVS_BRIDGE}"\
 "${IFACE}" ${IF_OVS_OPTIONS} -- set Interface "${IFACE}"\
 type=internal ${OVS_EXTRA+-- $OVS_EXTRA}
 
-ifconfig "${IFACE}" up
+ip link set "${IFACE}" up
 ;;
 OVSBond)
 ovs_vsctl -- --fake-iface add-bond "${IF_OVS_BRIDGE}"\
 "${IFACE}" ${IF_OVS_BONDS} ${IF_OVS_OPTIONS} \
 ${OVS_EXTRA+-- $OVS_EXTRA}
 
-ifconfig "${IFACE}" up
+ip link set "${IFACE}" up
 for slave in ${IF_OVS_BONDS}
 do
-ifconfig "${slave}" up
+ip link set "${IFACE}" up
 done
 ;;
 OVSPatchPort)
-- 
1.8.3.1



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

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

[nickcooper-zhangtonghao]

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 <n...@opencloud.tech>
---
 lib/stp.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/stp.c b/lib/stp.c
index d828c64..b9eac1c 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -1065,13 +1065,14 @@ stp_transmit_config(struct stp_port *p) 
OVS_REQUIRES(mutex)
 if (root) {
 config.message_age = htons(0);
 } else {
-config.message_age = htons(stp->root_port->message_age_timer.value
-   + MESSAGE_AGE_INCREMENT);
+config.message_age =
+htons(timer_to_ms(stp->root_port->message_age_timer.value) / 
1000
+  + MESSAGE_AGE_INCREMENT);
 }
-config.max_age = htons(stp->max_age);
-config.hello_time = htons(stp->hello_time);
-config.forward_delay = htons(stp->forward_delay);
-if (ntohs(config.message_age) < stp->max_age) {
+config.max_age = htons(timer_to_ms(stp->max_age) / 1000);
+config.hello_time = htons(timer_to_ms(stp->hello_time) / 1000);
+config.forward_delay = htons(timer_to_ms(stp->forward_delay) / 1000);
+if (ntohs(config.message_age) < timer_to_ms(stp->max_age) / 1000) {
 p->topology_change_ack = false;
 p->config_pending = false;
 VLOG_DBG_RL(_rl, "bridge: %s, port: %s, transmit config bpdu",
@@ -1108,7 +1109,8 @@ stp_record_config_information(struct stp_port *p,
 p->designated_cost = ntohl(config->root_path_cost);
 p->designated_bridge = ntohll(config->bridge_id);
 p->designated_port = ntohs(config->port_id);
-stp_start_timer(>message_age_timer, ntohs(config->message_age));
+stp_start_timer(>message_age_timer,
+ms_to_timer(ntohs(config->message_age) * 1000));
 }
 
 static void
@@ -1116,9 +1118,9 @@ stp_record_config_timeout_values(struct stp *stp,
  const struct stp_config_bpdu  *config)
  OVS_REQUIRES(mutex)
 {
-stp->max_age = ntohs(config->max_age);
-stp->hello_time = ntohs(config->hello_time);
-stp->forward_delay = ntohs(config->forward_delay);
+stp->max_age = ms_to_timer(ntohs(config->max_age) * 1000);
+stp->hello_time = ms_to_timer(ntohs(config->hello_time) * 1000);
+stp->forward_delay = ms_to_timer(ntohs(config->forward_delay) * 1000);
 stp->topology_change = config->flags & STP_CONFIG_TOPOLOGY_CHANGE;
 }
 
-- 
1.8.3.1



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

[ovs-dev] [PATCH 4/4] stp: Don't wait a hello-time before sending BPDU.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/stp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/stp.c b/lib/stp.c
index d90b400..d828c64 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -306,7 +306,7 @@ stp_create(const char *name, stp_identifier bridge_id,
 
 stp_stop_timer(>tcn_timer);
 stp_stop_timer(>topology_change_timer);
-stp_start_timer(>hello_timer, 0);
+stp_start_timer(>hello_timer, stp->hello_time);
 
 stp->send_bpdu = send_bpdu;
 stp->aux = aux;
-- 
1.8.3.1




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

[ovs-dev] [PATCH 3/4] stp: Set BPDU max age exactly.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 vswitchd/bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 2e10013..961ebe6 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1538,7 +1538,7 @@ bridge_configure_stp(struct bridge *br, bool enable_stp)
   STP_DEFAULT_HELLO_TIME);
 
 br_s.max_age = smap_get_ullong(>cfg->other_config, "stp-max-age",
-   STP_DEFAULT_HELLO_TIME / 1000) * 1000;
+   STP_DEFAULT_MAX_AGE / 1000) * 1000;
 br_s.fwd_delay = smap_get_ullong(>cfg->other_config,
  "stp-forward-delay",
  STP_DEFAULT_FWD_DELAY / 1000) * 1000;
-- 
1.8.3.1




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

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

[nickcooper-zhangtonghao]

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 <n...@opencloud.tech>
---
 lib/stp.c | 108 ++
 1 file changed, 108 insertions(+)

diff --git a/lib/stp.c b/lib/stp.c
index 1444dd7..d90b400 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,107 @@ 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)
+{
+struct eth_addr mac;
+const uint64_t mac_bits = (UINT64_C(1) << 48) - 1;
+
+ds_put_format(ds, "\tstp-priority\t%"PRIu32"\n",
+  (uint16_t)(bridge_id >> 48));
+
+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%"PRId32"s\n",
+  timer_to_ms(hello_time) / 1000);
+ds_put_format(ds, "\tstp-max-age\t%"PRId32"s\n",
+  timer_to_ms(max_age) / 1000);
+ds_put_format(ds, "\tstp-fwd-delay\t%"PRId32"s\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%"PRIu32"\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, "%-6"PRId32, p->path_cost);
+ds_put_format(ds, "%"PRIu16".%"PRIu16"\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();
+}
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/4] 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 <n...@opencloud.tech>
---
 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 ecef012..1444dd7 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 1e1b107..526ef27 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2442,10 +2442,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] mcast-snooping: Add and improve mcast-snooping tests.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 tests/mcast-snooping.at | 112 
 1 file changed, 112 insertions(+)

diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at
index c03aba3..90de8b3 100644
--- a/tests/mcast-snooping.at
+++ b/tests/mcast-snooping.at
@@ -29,6 +29,8 @@ dummy@ovs-dummy: hit:0 missed:0
p2 2/2: (dummy)
 ])
 
+ovs-appctl time/stop
+
 # Send IGMPv3 query on p2 with vlan 1725
 # 5c:8a:38:55:25:52 > 01:00:5e:00:00:01, ethertype 802.1Q (0x8100), length 64: 
vlan 1725, p 0, ethertype IPv4,
 # 172.17.25.1 > 224.0.0.1: igmp query v3
@@ -102,3 +104,113 @@ AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([mcast - delete the port mdb when vlan configruation changed])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl set bridge br0 \
+datapath_type=dummy \
+mcast_snooping_enable=true \
+other-config:mcast-snooping-disable-flood-unregistered=false
+], [0])
+
+AT_CHECK([ovs-ofctl add-flow br0 action=normal])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy \
+other-config:hwaddr=aa:55:aa:55:00:01 ofport_request=1 \
+-- add-port br0 p2 \
+-- set Interface p2 type=dummy other-config:hwaddr=aa:55:aa:55:00:02 
ofport_request=2 \
+-- add-port br0 p3 \
+-- set Interface p3 type=dummy other-config:hwaddr=aa:55:aa:55:00:03 
ofport_request=3
+], [0])
+
+ovs-appctl time/stop
+
+# send report packets
+AT_CHECK([
+ovs-appctl netdev-dummy/receive p1  \
+
'01005E010101000C29A027A181010800451C00014002CBAEAC10221EE001010112140CE9E0010101'
+ovs-appctl netdev-dummy/receive p1  \
+
'01005E010101000C29A027A281020800451C00014002CBAEAC10221EE001010112140CE9E0010101'
+], [0])
+
+# send query packets
+AT_CHECK([
+ovs-appctl netdev-dummy/receive p3  \
+   
'01005E010101000C29A027D181010800451C00014002CBCBAC102201E0010104EEEB'
+ovs-appctl netdev-dummy/receive p3  \
+
'01005E010101000C29A027D281020800451C00014002CBCAAC102202E0010104EEEB'
+], [0])
+
+AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
+ port  VLAN  GROUPAge
+1 1  224.1.1.1   0
+1 2  224.1.1.1   0
+3 1  querier   0
+3 2  querier   0
+])
+
+AT_CHECK([ovs-vsctl set port p3 tag=2], [0])
+
+AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
+ port  VLAN  GROUPAge
+1 1  224.1.1.1   0
+1 2  224.1.1.1   0
+])
+
+AT_CLEANUP
+
+AT_SETUP([mcast - delete the port mdb when port destroyed])
+OVS_VSWITCHD_START([])
+
+AT_CHECK([
+ovs-vsctl set bridge br0 \
+datapath_type=dummy \
+mcast_snooping_enable=true \
+other-config:mcast-snooping-disable-flood-unregistered=false
+], [0])
+
+AT_CHECK([ovs-ofctl add-flow br0 action=normal])
+
+AT_CHECK([
+ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy \
+other-config:hwaddr=aa:55:aa:55:00:01 ofport_request=1 \
+-- add-port br0 p2 \
+-- set Interface p2 type=dummy other-config:hwaddr=aa:55:aa:55:00:02 
ofport_request=2 \
+], [0])
+
+# send report packets
+AT_CHECK([
+ovs-appctl netdev-dummy/receive p1  \
+
'01005E010101000C29A027A181010800451C00014002CBAEAC10221EE001010112140CE9E0010101'
+ovs-appctl netdev-dummy/receive p1  \
+
'01005E010101000C29A027A281020800451C00014002CBAEAC10221EE001010112140CE9E0010101'
+], [0])
+
+# send query packets
+AT_CHECK([
+ovs-appctl netdev-dummy/receive p2  \
+   
'01005E010101000C29A027D181010800451C00014002CBCBAC102201E0010104EEEB'
+ovs-appctl netdev-dummy/receive p2  \
+
'01005E010101000C29A027D281020800451C00014002CBCAAC102202E0010104EEEB'
+], [0])
+
+AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
+ port  VLAN  GROUPAge
+1 1  224.1.1.1   0
+1 2  224.1.1.1   0
+2 1  querier   0
+2 2  querier   0
+])
+
+AT_CHECK([ovs-vsctl del-port br0 p2], [0])
+
+AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl
+ port  VLAN  GROUPAge
+1 1  224.1.1.1   0
+1 2  224.1.1.1   0
+])
+
+AT_CLEANUP
-- 
1.8.3.1



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

[ovs-dev] [PATCH v2 1/2] mcast-snooping: Flush ports mdb when VLAN configuration changed.

[nickcooper-zhangtonghao]

If VLAN configuration(e.g. id, mode) change occurs, the IGMP
snooping-learned multicast groups from this port on the VLAN are
deleted. This avoids a MCAST_ENTRY_DEFAULT_IDLE_TIME delay before
mdb is updated again. Hardware switches (e.g. cisco) also do that.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/mcast-snooping.c   | 31 +++
 lib/mcast-snooping.h   |  1 +
 ofproto/ofproto-dpif.c |  1 +
 3 files changed, 33 insertions(+)

diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index f71321d..a3163f5 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -942,3 +942,34 @@ mcast_snooping_wait(struct mcast_snooping *ms)
 mcast_snooping_wait__(ms);
 ovs_rwlock_unlock(>rwlock);
 }
+
+void
+mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port)
+{
+struct mcast_group *g, *next_g;
+struct mcast_mrouter_bundle *m, *next_m;
+
+if (!mcast_snooping_enabled(ms)) {
+return;
+}
+
+ovs_rwlock_wrlock(>rwlock);
+LIST_FOR_EACH_SAFE (g, next_g, group_node, >group_lru) {
+if (mcast_group_delete_bundle(ms, g, port)) {
+ms->need_revalidate = true;
+
+if (!mcast_group_has_bundles(g)) {
+mcast_snooping_flush_group__(ms, g);
+}
+}
+}
+
+LIST_FOR_EACH_SAFE (m, next_m, mrouter_node, >mrouter_lru) {
+if (m->port == port) {
+mcast_snooping_flush_mrouter(m);
+ms->need_revalidate = true;
+}
+}
+
+ovs_rwlock_unlock(>rwlock);
+}
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index af7fb93..f120405 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -214,5 +214,6 @@ bool mcast_snooping_is_membership(ovs_be16 igmp_type);
 /* Flush. */
 void mcast_snooping_mdb_flush(struct mcast_snooping *ms);
 void mcast_snooping_flush(struct mcast_snooping *ms);
+void mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port);
 
 #endif /* mcast-snooping.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 89c7b7f..366b7a2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2948,6 +2948,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
  * everything on this port and force flow revalidation. */
 if (need_flush) {
 bundle_flush_macs(bundle, false);
+mcast_snooping_flush_bundle(ofproto->ms, bundle);
 }
 
 return 0;
-- 
1.8.3.1



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

[ovs-dev] [PATCH v2 2/2] mcast-snooping: Avoid segfault for vswitchd.

[nickcooper-zhangtonghao]

The ports which are attached mrouters or hosts, were destroyed
by users via ovs-vsctl commands. Currently the vswitch will
segfault if users use "ovs-appctl mdb/show" to show mdb info.
This patch avoids a segfault.

ofproto_unixctl_mcast_snooping_show ofproto/ofproto-dpif.c:4781
process_command lib/unixctl.c:313
run_connection lib/unixctl.c:347
unixctl_server_run lib/unixctl.c:400
main vswitchd/ovs-vswitchd.c:112

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 366b7a2..1e1b107 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2756,6 +2756,7 @@ bundle_destroy(struct ofbundle *bundle)
 }
 
 bundle_flush_macs(bundle, true);
+mcast_snooping_flush_bundle(ofproto->ms, bundle);
 hmap_remove(>bundles, >hmap_node);
 free(bundle->name);
 free(bundle->trunks);
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/2] mcast-snooping: Flush ports mdb when VLAN configuration changed.

[nickcooper-zhangtonghao]

If VLAN configuration(e.g. id, mode) change occurs, the IGMP
snooping-learned multicast groups from this port on the VLAN are
deleted. This avoids a MCAST_ENTRY_DEFAULT_IDLE_TIME delay before
mdb is updated again. Hardware switches (e.g. cisco) also do that.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/mcast-snooping.c   | 31 +++
 lib/mcast-snooping.h   |  1 +
 ofproto/ofproto-dpif.c |  1 +
 3 files changed, 33 insertions(+)

diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index f71321d..a3163f5 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -942,3 +942,34 @@ mcast_snooping_wait(struct mcast_snooping *ms)
 mcast_snooping_wait__(ms);
 ovs_rwlock_unlock(>rwlock);
 }
+
+void
+mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port)
+{
+struct mcast_group *g, *next_g;
+struct mcast_mrouter_bundle *m, *next_m;
+
+if (!mcast_snooping_enabled(ms)) {
+return;
+}
+
+ovs_rwlock_wrlock(>rwlock);
+LIST_FOR_EACH_SAFE (g, next_g, group_node, >group_lru) {
+if (mcast_group_delete_bundle(ms, g, port)) {
+ms->need_revalidate = true;
+
+if (!mcast_group_has_bundles(g)) {
+mcast_snooping_flush_group__(ms, g);
+}
+}
+}
+
+LIST_FOR_EACH_SAFE (m, next_m, mrouter_node, >mrouter_lru) {
+if (m->port == port) {
+mcast_snooping_flush_mrouter(m);
+ms->need_revalidate = true;
+}
+}
+
+ovs_rwlock_unlock(>rwlock);
+}
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index af7fb93..f120405 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -214,5 +214,6 @@ bool mcast_snooping_is_membership(ovs_be16 igmp_type);
 /* Flush. */
 void mcast_snooping_mdb_flush(struct mcast_snooping *ms);
 void mcast_snooping_flush(struct mcast_snooping *ms);
+void mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port);
 
 #endif /* mcast-snooping.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 89c7b7f..366b7a2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2948,6 +2948,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
  * everything on this port and force flow revalidation. */
 if (need_flush) {
 bundle_flush_macs(bundle, false);
+mcast_snooping_flush_bundle(ofproto->ms, bundle);
 }
 
 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] mcast-snooping: Avoid segfault for vswitchd.

[nickcooper-zhangtonghao]

The ports which are attached mrouters or hosts, were destroyed
by users via ovs-vsctl commands. Currently the vswitch will
segfault if users use "ovs-appctl mdb/show" to show mdb info.
This patch avoids a segfault.

or

ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 366b7a2..1e1b107 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2756,6 +2756,7 @@ bundle_destroy(struct ofbundle *bundle)
 }
 
 bundle_flush_macs(bundle, true);
+mcast_snooping_flush_bundle(ofproto->ms, bundle);
 hmap_remove(>bundles, >hmap_node);
 free(bundle->name);
 free(bundle->trunks);
-- 
1.8.3.1




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

[ovs-dev] [PATCH 2/3] dpdk: Improve manpage for dpdk memory configuration.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 vswitchd/vswitch.xml | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 782417f..a91be59 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -241,10 +241,6 @@
   regardless of socket. It is recommended that dpdk-socket-mem is used
   instead.
 
-
-  If not specified, the value is 0. Changing this value requires
-  restarting the daemon.
-
   
 
   
 
-  If not specified, the default value is 1024,0. Changing this value
-  requires restarting the daemon.
+  If dpdk-socket-mem and dpdk-alloc-mem are not specified, 
dpdk-socket-mem
+  will be used and the default value is 1024,0. If dpdk-socket-mem and
+  dpdk-alloc-mem are specified at same time, dpdk-socket-mem will be
+  used as default. Changing this value requires restarting the daemon.
 
   
 
-- 
1.8.3.1

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

[ovs-dev] [PATCH 3/3] lacp: Fix formatting typo.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/lacp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index 64c9849..7716387 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -337,7 +337,7 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
 
 pdu = parse_lacp_packet(packet);
 if (!pdu) {
-   slave->count_rx_pdus_bad++;
+slave->count_rx_pdus_bad++;
 VLOG_WARN_RL(, "%s: received an unparsable LACP PDU.", lacp->name);
 goto out;
 }
@@ -555,7 +555,7 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) 
OVS_EXCLUDED(mutex)
 slave->ntt_actor = actor;
 compose_lacp_pdu(, >partner, );
 send_pdu(slave->aux, , sizeof pdu);
-   slave->count_tx_pdus++;
+slave->count_tx_pdus++;
 
 duration = (slave->partner.state & LACP_STATE_TIME
 ? LACP_FAST_TIME_TX
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/3] doc: Fix issues.rst formatting typo.

[nickcooper-zhangtonghao]

The preformatted block is only finished when the text
falls back to the same indentation level as a paragraph
prior to the preformatted block.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 Documentation/faq/issues.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/faq/issues.rst b/Documentation/faq/issues.rst
index 9bb087c..c60336a 100644
--- a/Documentation/faq/issues.rst
+++ b/Documentation/faq/issues.rst
@@ -105,7 +105,7 @@ very high.
 
 - Perhaps you don't actually need eth0 and eth1 to be on the same bridge.
   For example, if you simply want to be able to connect each of them to
-  virtual machines, then you can put each of them on a bridge of its own:
+  virtual machines, then you can put each of them on a bridge of its own::
 
   $ ovs-vsctl add-br br0
   $ ovs-vsctl add-port br0 eth0
@@ -212,6 +212,8 @@ immediately put it back.  For example, consider that p1 is 
a port of
  add-port br0 p1 -- \
  set interface p1 type=internal
 
+Any other type of port gets the same effect.
+
 A: It's an expected behaviour.
 
 If del-port and add-port happen in a single OVSDB transaction as your
-- 
1.8.3.1



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

[ovs-dev] [PATCH] doc: Fix issues.rst formatting typo.

[nickcooper-zhangtonghao]

The preformatted block is only finished when the text
falls back to the same indentation level as a paragraph
prior to the preformatted block.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 Documentation/faq/issues.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/faq/issues.rst b/Documentation/faq/issues.rst
index 9bb087c..c60336a 100644
--- a/Documentation/faq/issues.rst
+++ b/Documentation/faq/issues.rst
@@ -105,7 +105,7 @@ very high.
 
 - Perhaps you don't actually need eth0 and eth1 to be on the same bridge.
   For example, if you simply want to be able to connect each of them to
-  virtual machines, then you can put each of them on a bridge of its own:
+  virtual machines, then you can put each of them on a bridge of its own::
 
   $ ovs-vsctl add-br br0
   $ ovs-vsctl add-port br0 eth0
@@ -212,6 +212,8 @@ immediately put it back.  For example, consider that p1 is 
a port of
  add-port br0 p1 -- \
  set interface p1 type=internal
 
+Any other type of port gets the same effect.
+
 A: It's an expected behaviour.
 
 If del-port and add-port happen in a single OVSDB transaction as your
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH] ofproto/bond: Fix bond/show when all interfaces are disabled

[nickcooper-zhangtonghao]

> On Feb 17, 2017, at 4:49 AM, Andy Zhou  wrote:
> 
> Without this patch, when all slaves are disabled, the 'bond/show'
> command still shows the mac address of last active slave in
> 'active slave mac' output. This patch clears them to zeros.
> 
> Signed-off-by: Andy Zhou >
> ---
> ofproto/bond.c | 12 
> 1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index c138593..260023e 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -488,10 +488,13 @@ bond_find_slave_by_mac(const struct bond *bond, const 
> struct eth_addr mac)
> static void
> bond_active_slave_changed(struct bond *bond)
> {
> -struct eth_addr mac;
> -
> -netdev_get_etheraddr(bond->active_slave->netdev, );
> -bond->active_slave_mac = mac;
> +if (bond->active_slave) {
> +struct eth_addr mac;
> +netdev_get_etheraddr(bond->active_slave->netdev, );
> +bond->active_slave_mac = mac;
> +} else {
> +bond->active_slave_mac = eth_addr_zero;
> +}
> bond->active_slave_changed = true;
> seq_change(connectivity_seq_get());
> }
> @@ -1866,6 +1869,7 @@ bond_choose_active_slave(struct bond *bond)
> bond_active_slave_changed(bond);
> }
> } else if (old_active_slave) {
> +bond_active_slave_changed(bond);
> VLOG_INFO_RL(, "bond %s: all interfaces disabled", bond->name);
> }
> }
> -- 
> 1.9.1


looks good to me.



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

[ovs-dev] [PATCH] ovs-appctl: Print lacp_fallback_ab info in "bond/show".

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 3 +++
 tests/lacp.at  | 9 +
 2 files changed, 12 insertions(+)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2e018aa..de75f87 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -1325,6 +1325,9 @@ bond_print_details(struct ds *ds, const struct bond *bond)
 break;
 }
 
+ds_put_format(ds, "lacp_fallback_ab: %s\n",
+  bond->lacp_fallback_ab ? "true" : "false");
+
 ds_put_cstr(ds, "active slave mac: ");
 ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac));
 slave = bond_find_slave_by_mac(bond, bond->active_slave_mac);
diff --git a/tests/lacp.at b/tests/lacp.at
index 8f78e79..20ec09e 100644
--- a/tests/lacp.at
+++ b/tests/lacp.at
@@ -124,6 +124,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 active slave mac: 00:00:00:00:00:00(none)
 
 slave p1: disabled
@@ -288,6 +289,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 slave p0: enabled
may_enable: true
@@ -302,6 +304,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 slave p2: enabled
may_enable: true
@@ -423,6 +426,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 
 slave p0: disabled
@@ -439,6 +443,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 
 slave p2: disabled
@@ -553,6 +558,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 
 slave p0: disabled
@@ -569,6 +575,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 
 slave p2: disabled
@@ -688,6 +695,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 
 slave p0: enabled
@@ -704,6 +712,7 @@ bond-hash-basis: 0
 updelay: 0 ms
 downdelay: 0 ms
 lacp_status: negotiated
+lacp_fallback_ab: false
 
 
 slave p2: enabled
-- 
1.8.3.1



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

[ovs-dev] [PATCH v2] ofproto/bond: Drop traffic in balance-tcp mode without lacp.

[nickcooper-zhangtonghao]

The balance-tcp mode requires the upstream switch to support 802.3ad
with successful LACP negotiation. When bond ports are configured to
balance-tcp mode without lacp or lacp is disabled, drop the traffic.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2e018aa..e4caf98 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -786,7 +786,11 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 if (!bond->lacp_fallback_ab) {
 goto out;
 }
+break;
 case LACP_DISABLED:
+if (bond->balance == BM_TCP) {
+goto out;
+}
 break;
 }
 
-- 
1.8.3.1




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

Re: [ovs-dev] [PATCH 3/4] ofproto/bond: balance-slb mode fallbacks to active-backup mode.

[nickcooper-zhangtonghao]

> On Feb 15, 2017, at 12:01 PM, Andy Zhou <az...@ovn.org> wrote:
> 
> On Mon, Feb 13, 2017 at 10:52 PM, nickcooper-zhangtonghao
> <n...@opencloud.tech <mailto:n...@opencloud.tech>> wrote:
>> lacp-fallback-ab determines the behavior of OvS bond in LACP mode.
>> If the partner switch does not support LACP, setting this option
>> to true allows OvS to fallback to active-backup. The balance-tcp
>> mode requires lacp. If LACP negotiation fails and
>> other-config:lacp-fallback-ab is true, then active-backup mode is
>> used. But if users configure the bond port to balance-slb and lacp
>> (unsuccessfully), active-backup mode is also used.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
> 
> I think only accepts input from one port may be too restrictive. In case the
> up stream switch choose to transmit on the port that we deicde not to receive,
> then connection will be broken.
> 
> It seems the currently the lacp_fallback_ab only applies to
> transmit(output).  Why should
> it also apply to receive?


When we receive packets, balance-tcp may fallback to active-backup, and 
balance-slb
should also fallback. But I didn’t consider the case above. Maybe, this patch 
is not useful.

Thanks for your review.

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

Re: [ovs-dev] [PATCH 2/4] ofproto/bond: Drop traffic in balance-tcp mode without lacp.

[nickcooper-zhangtonghao]

> On Feb 15, 2017, at 11:51 AM, Andy Zhou <az...@ovn.org> wrote:
> 
> On Mon, Feb 13, 2017 at 10:52 PM, nickcooper-zhangtonghao
> <n...@opencloud.tech <mailto:n...@opencloud.tech>> wrote:
>> The balance-tcp mode requires the upstream switch to support 802.3ad
>> with successful LACP negotiation. When bond ports are configured to
>> balance-tcp mode without lacp or lacp is disabled, drop the traffic.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
> 
> I think the following change is equivalent and may be easier to
> follow, what do you think?
> 
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index 2703bc9..4c2781d 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -787,6 +787,9 @@ bond_check_admissibility(struct bond *bond, const
> void *slave_,
> goto out;
> }
break;
We should add ‘break' here. If not, traffic(e.g lacp) will be dropped.
> case LACP_DISABLED:
> +if (bond->balance == BM_TCP) {
> +goto out;
> +}
> break;
> }

Thanks for your review.


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

[ovs-dev] [PATCH 4/4] doc: Updates bonding.rst because of api changed.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 Documentation/topics/bonding.rst | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/topics/bonding.rst b/Documentation/topics/bonding.rst
index 2f67cbb..461935a 100644
--- a/Documentation/topics/bonding.rst
+++ b/Documentation/topics/bonding.rst
@@ -74,7 +74,7 @@ port for traffic that was destined for that slave (see
 ``bond_enable_slave()``).  It also sends a "gratuitous learning packet",
 specifically a RARP, on the bond port (on the newly chosen slave) for each MAC
 address that the vswitch has learned on a port other than the bond (see
-``bond_send_learning_packets()``), to teach the physical switch that the new
+``bundle_send_learning_packets()``), to teach the physical switch that the new
 slave should be used in place of the one that is now disabled.  (This behavior
 probably makes sense only for a vswitch that has only one port (the bond)
 connected to a physical switch; vswitchd should probably provide a way to
@@ -106,7 +106,7 @@ exception does not match normal ARP replies.  It will match 
the learning
 packets sent on bond fail-over.)
 
 The active slave is simply the first slave to be enabled after the bond is
-created (see ``bond_choose_active_iface()``).  If the active slave is disabled,
+created (see ``bond_choose_active_slave()``).  If the active slave is disabled,
 then a new active slave is chosen among the slaves that remain active.
 Currently due to the way that configuration works, this tends to be the
 remaining slave whose interface name is first alphabetically, but this is by no
@@ -116,7 +116,7 @@ Bond Packet Output
 --
 
 When a packet is sent out a bond port, the bond slave actually used is selected
-based on the packet's source MAC and VLAN tag (see ``choose_output_iface()``).
+based on the packet's source MAC and VLAN tag (see 
``bond_choose_output_slave()``).
 In particular, the source MAC and VLAN tag are hashed into one of 256 values,
 and that value is looked up in a hash table (the "bond hash") kept in the
 ``bond_hash`` member of struct port.  The hash table entry identifies a bond
@@ -124,7 +124,7 @@ slave.  If no bond slave has yet been chosen for that hash 
table entry,
 vswitchd chooses one arbitrarily.
 
 Every 10 seconds, vswitchd rebalances the bond slaves (see
-``bond_rebalance_port()``).  To rebalance, vswitchd examines the statistics for
+``bond_rebalance()``).  To rebalance, vswitchd examines the statistics for
 the number of bytes transmitted by each slave over approximately the past
 minute, with data sent more recently weighted more heavily than data sent less
 recently.  It considers each of the slaves in order from most-loaded to
-- 
1.8.3.1



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

[ovs-dev] [PATCH 3/4] ofproto/bond: balance-slb mode fallbacks to active-backup mode.

[nickcooper-zhangtonghao]

lacp-fallback-ab determines the behavior of OvS bond in LACP mode.
If the partner switch does not support LACP, setting this option
to true allows OvS to fallback to active-backup. The balance-tcp
mode requires lacp. If LACP negotiation fails and
other-config:lacp-fallback-ab is true, then active-backup mode is
used. But if users configure the bond port to balance-slb and lacp
(unsuccessfully), active-backup mode is also used.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2703bc9..56996bf 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -798,6 +798,19 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 }
 
 switch (bond->balance) {
+case BM_SLB:
+/* Drop all packets for which we have learned a different input port,
+ * because we probably sent the packet on one slave and got it back on
+ * the other.  Gratuitous ARP packets are an exception to this rule:
+ * the host has moved to another switch.  The exception to the
+ * exception is if we locked the learning table to avoid reflections on
+ * bond slaves. */
+if (bond->lacp_status == LACP_DISABLED) {
+verdict = BV_DROP_IF_MOVED;
+goto out;
+}
+/* Allows OvS to fallback to BM_AB. */
+
 case BM_TCP:
 /* TCP balanced bonds require successful LACP negotiations. Based on
  * the above check, LACP is off or lacp_fallback_ab is true on this
@@ -821,16 +834,6 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 }
 verdict = BV_ACCEPT;
 goto out;
-
-case BM_SLB:
-/* Drop all packets for which we have learned a different input port,
- * because we probably sent the packet on one slave and got it back on
- * the other.  Gratuitous ARP packets are an exception to this rule:
- * the host has moved to another switch.  The exception to the
- * exception is if we locked the learning table to avoid reflections on
- * bond slaves. */
-verdict = BV_DROP_IF_MOVED;
-goto out;
 }
 
 OVS_NOT_REACHED();
-- 
1.8.3.1



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

[ovs-dev] [PATCH 2/4] ofproto/bond: Drop traffic in balance-tcp mode without lacp.

[nickcooper-zhangtonghao]

The balance-tcp mode requires the upstream switch to support 802.3ad
with successful LACP negotiation. When bond ports are configured to
balance-tcp mode without lacp or lacp is disabled, drop the traffic.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2e018aa..2703bc9 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -799,11 +799,12 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 
 switch (bond->balance) {
 case BM_TCP:
-/* TCP balanced bonds require successful LACP negotiations. Based on 
the
- * above check, LACP is off or lacp_fallback_ab is true on this bond.
- * If lacp_fallback_ab is true fall through to BM_AB case else, we
- * drop all incoming traffic. */
-if (!bond->lacp_fallback_ab) {
+/* TCP balanced bonds require successful LACP negotiations. Based on
+ * the above check, LACP is off or lacp_fallback_ab is true on this
+ * bond. If LACP is in LACP_DISABLED state, drop all incoming traffic.
+ * If LACP is in LACP_CONFIGURED state and lacp_fallback_ab is true
+ * fall through to BM_AB case else, we drop all incoming traffic. */
+if (bond->lacp_status == LACP_DISABLED || !bond->lacp_fallback_ab) {
 goto out;
 }
 
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/4] ofproto/bond: Validate active-slave mac.

[nickcooper-zhangtonghao]

That the mac of active-slave is invalid(e.g. 00:00:00:00:00:00)
is incidental. The reason is described as below.

In the bridge_reconfig():
1. bond devices created in port_configure().
2. the bonded interfaces may be disabled even calling bridge_run__(),
   because the interface link is not ready.

The OvS will run bridge_run__() in next loop. In next loop, the
active-slave may be selected. But OvS the bridge_reconfig() again,
the bond_reconfigure() set active-slave mac zero and flag false.
If using the 'ovs-appctl bond/show bond-name' to check active-slave
mac, you will find the mac is zero and mac in the ovsdb is also zero.

The active_slave_mac and active_slave_changed should be initialized
when created.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 5063b3f..2e018aa 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -242,6 +242,9 @@ bond_create(const struct bond_settings *s, struct 
ofproto_dpif *ofproto)
 ovs_refcount_init(>ref_cnt);
 hmap_init(>pr_rule_ops);
 
+bond->active_slave_mac = eth_addr_zero;
+bond->active_slave_changed = false;
+
 bond_reconfigure(bond, s);
 return bond;
 }
@@ -457,9 +460,6 @@ bond_reconfigure(struct bond *bond, const struct 
bond_settings *s)
 bond_entry_reset(bond);
 }
 
-bond->active_slave_mac = s->active_slave_mac;
-bond->active_slave_changed = false;
-
 ovs_rwlock_unlock();
 return revalidate;
 }
-- 
1.8.3.1



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

[ovs-dev] [PATCH] dpdk: Improve manpage for dpdk memory configuration.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 vswitchd/vswitch.xml | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 146a816..a3beafd 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -241,10 +241,6 @@
   regardless of socket. It is recommended that dpdk-socket-mem is used
   instead.
 
-
-  If not specified, the value is 0. Changing this value requires
-  restarting the daemon.
-
   
 
   
 
-  If not specified, the default value is 1024,0. Changing this value
-  requires restarting the daemon.
+  If dpdk-socket-mem and dpdk-alloc-mem are not specified, 
dpdk-socket-mem
+  will be used and the default value is 1024,0. If dpdk-socket-mem and
+  dpdk-alloc-mem are specified at same time, dpdk-socket-mem will be
+  used as default. Changing this value requires restarting the daemon.
 
   
 
-- 
1.8.3.1



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

[ovs-dev] [PATCH 2/3] ofproto/bond: balance-tcp mode fallbacks to active-backup mode.

[nickcooper-zhangtonghao]

lacp-fallback-ab determines the behavior of OvS bond in LACP mode.
If the partner switch does not support LACP, setting this option
to true allows OvS to fallback to active-backup. The balance-tcp
mode requires lacp. If LACP negotiation fails and
other-config:lacp-fallback-ab is true, then active-backup mode is
used. But if users configure the bond port to balance-slb and lacp
(unsuccessfully), active-backup mode is also used.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 3e512d7..dd29681 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -798,6 +798,19 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 }
 
 switch (bond->balance) {
+case BM_SLB:
+/* Drop all packets for which we have learned a different input port,
+ * because we probably sent the packet on one slave and got it back on
+ * the other.  Gratuitous ARP packets are an exception to this rule:
+ * the host has moved to another switch.  The exception to the
+ * exception is if we locked the learning table to avoid reflections on
+ * bond slaves. */
+if (bond->lacp_status == LACP_DISABLED) {
+verdict = BV_DROP_IF_MOVED;
+goto out;
+}
+/* Allows OvS to fallback to BM_AB. */
+
 case BM_TCP:
 /* TCP balanced bonds require successful LACP negotiations. Based on
  * the above check, LACP is off or lacp_fallback_ab is true on this
@@ -821,16 +834,6 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 }
 verdict = BV_ACCEPT;
 goto out;
-
-case BM_SLB:
-/* Drop all packets for which we have learned a different input port,
- * because we probably sent the packet on one slave and got it back on
- * the other.  Gratuitous ARP packets are an exception to this rule:
- * the host has moved to another switch.  The exception to the
- * exception is if we locked the learning table to avoid reflections on
- * bond slaves. */
-verdict = BV_DROP_IF_MOVED;
-goto out;
 }
 
 OVS_NOT_REACHED();
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/3] ofproto/bond: Drops traffic in balance-tcp mode without lacp.

[nickcooper-zhangtonghao]

The balance-tcp mode requires the upstream switch to support 802.3ad
with successful LACP negotiation. When bond ports are configured to
balance-tcp mode without lacp or lacp is disabled, drop the traffic.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/bond.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 5063b3f..3e512d7 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -799,11 +799,12 @@ bond_check_admissibility(struct bond *bond, const void 
*slave_,
 
 switch (bond->balance) {
 case BM_TCP:
-/* TCP balanced bonds require successful LACP negotiations. Based on 
the
- * above check, LACP is off or lacp_fallback_ab is true on this bond.
- * If lacp_fallback_ab is true fall through to BM_AB case else, we
- * drop all incoming traffic. */
-if (!bond->lacp_fallback_ab) {
+/* TCP balanced bonds require successful LACP negotiations. Based on
+ * the above check, LACP is off or lacp_fallback_ab is true on this
+ * bond. If LACP is in LACP_DISABLED state, drop all incoming traffic.
+ * If LACP is in LACP_CONFIGURED state and lacp_fallback_ab is true
+ * fall through to BM_AB case else, we drop all incoming traffic. */
+if (bond->lacp_status == LACP_DISABLED || !bond->lacp_fallback_ab) {
 goto out;
 }
 
-- 
1.8.3.1



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

[ovs-dev] [PATCH] ofproto: Uses the VLOG_WARN_RL instead of VLOG_WARN.

[nickcooper-zhangtonghao]

There are a lot of logs when OvS bridges, connected to controllers,
can't find the right routes. So we may use the VLOG_WARN_RL instead
of VLOG_WARN to limit the log messages. The netdev-open and
arp-lookingup are in the same case in this function.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/in-band.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 4bb47c0..3d3675e 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -119,9 +119,11 @@ refresh_remote(struct in_band *ib, struct in_band_remote 
*r)
 retval = netdev_get_next_hop(ib->local_netdev, >remote_addr.sin_addr,
  _hop_inaddr, _hop_dev);
 if (retval) {
-VLOG_WARN("%s: cannot find route for controller ("IP_FMT"): %s",
-  ib->ofproto->name, IP_ARGS(r->remote_addr.sin_addr.s_addr),
-  ovs_strerror(retval));
+VLOG_WARN_RL(, "%s: cannot find route for controller "
+ "("IP_FMT"): %s",
+ ib->ofproto->name,
+ IP_ARGS(r->remote_addr.sin_addr.s_addr),
+ ovs_strerror(retval));
 return 1;
 }
 if (!next_hop_inaddr.s_addr) {
-- 
1.8.3.1



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

Re: [ovs-dev] [BUG] upcall handler thread crash

[nickcooper-zhangtonghao]

Hi, what’s the OvS version you tested. I didn’t get the crash with master 
version.
The test script is described as below.

ovs-vsctl add-br br0
ovs-vsctl add-port br0 eth1
for i in `seq 0 1000`;
do
ovs-vsctl add-port br0 eth2
ovs-vsctl del-port br0 eth2
done


Thanks.
Nick

> On Feb 4, 2017, at 7:21 PM, wangyunjian  wrote:
> 
> Recently, write a script add and delete port repeatly, ovs upcall handler 
> thread crash with the following trace.
> In the code bellow, weather the operations of mbridge->mbundles hmap should 
> with a lock to protect content between ovs-vswichd thread and the upcall 
> handler thread:
> 
> static struct mbundle *
> mbundle_lookup(const struct mbridge *mbridge, struct ofbundle *ofbundle)
> {
>struct mbundle *mbundle;
> 
>HMAP_FOR_EACH_IN_BUCKET (mbundle, hmap_node, hash_pointer(ofbundle, 0),
> >mbundles) {
>if (mbundle->ofbundle == ofbundle) {
>return mbundle;
>}
>}
>return NULL;
> }
> 
> Call Trace:
> #0  0x0044d838 in mbundle_lookup (mbridge=0x7fbf68000cc0, 
> ofbundle=0x7fbf7007c3d0) at ofproto/ofproto_dpif_mirror.c:472
> #1  0x0044da15 in mirror_bundle_out (mbridge=, 
> ofbundle=) at ofproto/ofproto_dpif_mirror.c:192
> #2  0x00448658 in xbundle_mirror_out (xbridge=0x7fbf5c6468a0, 
> xbundle=0x7fbf3d48a160) at ofproto/ofproto_dpif_xlate.c:1556
> #3  xlate_normal_flood (ctx=ctx@entry=0x7fbf7729e3d0, 
> in_xbundle=in_xbundle@entry=0x7fbf5c22f870, vlan=vlan@entry=100) at 
> ofproto/ofproto_dpif_xlate.c:2525
> #4  0x00448e7e in xlate_normal (ctx=0x7fbf7729e3d0) at 
> ofproto/ofproto_dpif_xlate.c:2724
> #5 xlate_output_action (ctx=ctx@entry=0x7fbf7729e3d0, port=, 
> max_len=, may_packet_in=may_packet_in@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:4061
> #6 0x00445147 in do_xlate_actions (ofpacts=0x6eb7288, ofpacts_len=16, 
> ctx=ctx@entry=0x7fbf7729e3d0) at ofproto/ofproto_dpif_xlate.c:4616
> #7 0x00446481 in xlate_recursively (rule=0x6eb7100, 
> ctx=0x7fbf7729e3d0) at ofproto/ofproto_dpif_xlate.c:3445
> #8 xlate_table_action (ctx=0x7fbf7729e3d0, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3513
> #9 0x004475a2 in compose_output_action__ 
> (ctx=ctx@entry=0x7fbf7729e3d0, ofp_port=, xr=, 
> check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3206
> #10 0x00447bbf in compose_output_action (xr=, 
> ofp_port=, ctx=0x7fbf7729e3d0) at 
> ofproto/ofproto_dpif_xlate.c:3426
> #11 output_normal (ctx=ctx@entry=0x7fbf7729e3d0, 
> out_xbundle=out_xbundle@entry=0x7fbf5cdf4aa0, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2073
> #12 0x004486ae in xlate_normal_flood (ctx=ctx@entry=0x7fbf7729e3d0, 
> in_xbundle=in_xbundle@entry=0x7fbf5e1d44d0, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2529
> #13 0x00448e7e in xlate_normal (ctx=0x7fbf7729e3d0) at 
> ofproto/ofproto_dpif_xlate.c:2724
> #14 xlate_output_action (ctx=ctx@entry=0x7fbf7729e3d0, port=, 
> max_len=, may_packet_in=may_packet_in@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:4061
> #15 0x00445147 in do_xlate_actions 
> (ofpacts=ofpacts@entry=0x7fbf70005ae8, ofpacts_len=ofpacts_len@entry=8, 
> ctx=ctx@entry=0x7fbf7729e3d0) at ofproto/ofproto_dpif_xlate.c:4616
> #16 0x0044a739 in xlate_actions (xin=xin@entry=0x7fbf7729f570, 
> xout=xout@entry=0x7fbf772c0b18) at ofproto/ofproto_dpif_xlate.c:5509
> #17 0x0043e4b6 in upcall_xlate (wc=0x7fbf772c0b70, 
> odp_actions=0x7fbf772c0b30, upcall=0x7fbf772c0ac0, udpif=0x6e164a0) at 
> ofproto/ofproto_dpif_upcall.c:1082
> #18 process_upcall (udpif=udpif@entry=0x6e164a0, 
> upcall=upcall@entry=0x7fbf772c0ac0, 
> odp_actions=odp_actions@entry=0x7fbf772c0b30, wc=wc@entry=0x7fbf772c0b70) at 
> ofproto/ofproto_dpif_upcall.c:1220
> #19 0x004407d3 in recv_upcalls (handler=0x7fbf58944810, 
> handler=0x7fbf58944810) at ofproto/ofproto_dpif_upcall.c:784
> #20 0x00440cca in udpif_upcall_handler (arg=0x7fbf58944810) at 
> ofproto/ofproto_dpif_upcall.c:701
> #21 0x004c95e4 in ovsthread_wrapper (aux_=) at 
> lib/ovs_thread.c:649
> #22 0x7fbf7aeaedc5 in start_thread () from /usr/lib64/libpthread.so.0
> #23 0x7fbf79a5e71d in clone () from /usr/lib64/libc.so.6
> 
> #0  0x0044d838 in mbundle_lookup (mbridge=0x7fbf68000cc0, 
> ofbundle=0x7fbf7007c3d0)
> 472   if (mbundle->ofbundle == ofbundle) {
> (gdb) p mbundle
> $1 = (struct mbundle *) 0x31

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

[ovs-dev] [PATCH] dpif-netdev: Avoids repeated addition of DP_STAT_LOST.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/dpif-netdev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 08167b5..3901129 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4258,7 +4258,6 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
 ofpbuf_uninit();
 ofpbuf_uninit(_actions);
 fat_rwlock_unlock(>upcall_rwlock);
-dp_netdev_count_packet(pmd, DP_STAT_LOST, lost_cnt);
 } else if (OVS_UNLIKELY(any_miss)) {
 for (i = 0; i < cnt; i++) {
 if (OVS_UNLIKELY(!rules[i])) {
-- 
1.8.3.1




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

[ovs-dev] [PATCH v3] netdev-dummy: Limits the number of tx/rx queues.

[nickcooper-zhangtonghao]

This patch avoids the ovs_rcu to report WARN, caused by blocked
for a long time, when ovs-vswitchd processes a port with many
rx/tx queues. The number of tx/rx queues per port may be appropriate,
because the dpdk uses it as an default max value.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
v3:
* Limits the number of tx/rx queues in set_config().
* Adds the WARN log when exceeds DUMMY_MAX_QUEUES_PER_PORT.
---
 lib/netdev-dummy.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index bdb77e1..4a23cba 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -827,6 +827,8 @@ netdev_dummy_set_in6(struct netdev *netdev_, struct 
in6_addr *in6,
 return 0;
 }
 
+#define DUMMY_MAX_QUEUES_PER_PORT 1024
+
 static int
 netdev_dummy_set_config(struct netdev *netdev_, const struct smap *args)
 {
@@ -870,6 +872,21 @@ netdev_dummy_set_config(struct netdev *netdev_, const 
struct smap *args)
 
 new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
 new_n_txq = MAX(smap_get_int(args, "n_txq", NR_QUEUE), 1);
+
+if (new_n_rxq > DUMMY_MAX_QUEUES_PER_PORT ||
+new_n_txq > DUMMY_MAX_QUEUES_PER_PORT) {
+VLOG_WARN("The one or both of interface %s queues"
+  "(rxq: %d, txq: %d) exceed %d. Sets it %d.\n",
+  netdev->up.name,
+  new_n_rxq,
+  new_n_txq,
+  DUMMY_MAX_QUEUES_PER_PORT,
+  DUMMY_MAX_QUEUES_PER_PORT);
+
+new_n_rxq = MIN(DUMMY_MAX_QUEUES_PER_PORT, new_n_rxq);
+new_n_txq = MIN(DUMMY_MAX_QUEUES_PER_PORT, new_n_txq);
+}
+
 new_numa_id = smap_get_int(args, "numa_id", 0);
 if (new_n_rxq != netdev->requested_n_rxq
 || new_n_txq != netdev->requested_n_txq
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH 2/4] datapath: Limits the number of tx/rx queues for netdev-dummy.

[nickcooper-zhangtonghao]

Thanks, I got it. :)


> On Jan 10, 2017, at 10:11 AM, Daniele Di Proietto <diproiet...@ovn.org> wrote:
> 
> 2017-01-08 20:02 GMT-08:00 nickcooper-zhangtonghao <n...@opencloud.tech 
> <mailto:n...@opencloud.tech>>:
>> Thanks Daniele,
>> Yes, it’s a small improvement. but it is necessary for us. I will check it
>> in
>> set_config(). One question to ask: should we check the tx/rx queue for
>> netdev-dpdk in set_config()?
> 
> I think for DPDK devices ultimately there's no way to check without
> actually setting up the queues, that's why it's done in reconfigure().
> 
> Thanks,
> 
> Daniele

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

Re: [ovs-dev] [PATCH v2] dpdk: Late initialization.

[nickcooper-zhangtonghao]

Thank you for explaining this to me. I got it.

> On Jan 10, 2017, at 10:09 AM, Daniele Di Proietto  
> wrote:
> 
>> 
>> 
>> 
>> hi Daniele,
>> I reviewed this patch. One question to ask: should we check the
>> hugepage mm before calling the rte_eal_init()? improvement on next version?
> 
> How do you suggest to check for hugepage before calling rte_eal_init()?
> 
> I think everybody agrees that in the long term we need to avoid aborting if 
> the initialization fails, but most of that work need to happen in dpdk 
> library.
> 
> If there's a simple check we could do here, I'm fine with including that, if 
> it's something more complicated and needs to be a separate patch, we should 
> probably defer it, since we're on feature freeze now.
> 
> Thanks,
> 
> Daniele

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

Re: [ovs-dev] [PATCH v2] netdev-dummy: Limits the number of tx/rx queues.

[nickcooper-zhangtonghao]

When you set the tx/rx queue number to 1(or lager), ovs-vswitchd takes more 
time to process
a port with so many rx/tx queues and the ovs_rcu will report the WARN. The 1024 
may be appropriate value because the dpdk uses it as default max value. 
However, 2048 is also ok, just a limiter.

Thanks.
Nick

> On Jan 9, 2017, at 11:52 PM, Aaron Conole <acon...@redhat.com> wrote:
> 
>> This patch avoids the ovs_rcu to report WARN, caused by blocked
>> for a long time, when ovs-vswitchd processes a port with many
>> rx/tx queues. The number of tx/rx queues per port may be appropriate,
>> because the dpdk uses it as an default max value.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
>> ---
> 
> I don't understand how this change impacts the ovs_rcu warning, unless
> you find an issue creating 1025 tx/rx queues.  Is that what happened?

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

Re: [ovs-dev] [PATCH v2] dpdk: Late initialization.

[nickcooper-zhangtonghao]

Yes, but this patch looks good to me.


> On Jan 9, 2017, at 11:35 PM, Aaron Conole <acon...@redhat.com> wrote:
> 
> nickcooper-zhangtonghao <n...@opencloud.tech <mailto:n...@opencloud.tech>> 
> writes:
> 
>> hi Daniele,
>> I reviewed this patch. One question to ask: should we check the
>> hugepage mm before calling the rte_eal_init()? improvement on next version?
> 
> Are you concerned for the possible rte_panic() call which could happen
> if the hugepage configuration is not set correctly?

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

[ovs-dev] [PATCH v2] netdev-dummy: Limits the number of tx/rx queues.

[nickcooper-zhangtonghao]

This patch avoids the ovs_rcu to report WARN, caused by blocked
for a long time, when ovs-vswitchd processes a port with many
rx/tx queues. The number of tx/rx queues per port may be appropriate,
because the dpdk uses it as an default max value.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/netdev-dummy.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index bdb77e1..5370404 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -827,6 +827,8 @@ netdev_dummy_set_in6(struct netdev *netdev_, struct 
in6_addr *in6,
 return 0;
 }
 
+#define DUMMY_MAX_QUEUES_PER_PORT 1024
+
 static int
 netdev_dummy_set_config(struct netdev *netdev_, const struct smap *args)
 {
@@ -868,8 +870,11 @@ netdev_dummy_set_config(struct netdev *netdev_, const 
struct smap *args)
 goto exit;
 }
 
-new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
-new_n_txq = MAX(smap_get_int(args, "n_txq", NR_QUEUE), 1);
+new_n_rxq = MIN(DUMMY_MAX_QUEUES_PER_PORT,
+MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1));
+new_n_txq = MIN(DUMMY_MAX_QUEUES_PER_PORT,
+MAX(smap_get_int(args, "n_txq", NR_QUEUE), 1));
+
 new_numa_id = smap_get_int(args, "numa_id", 0);
 if (new_n_rxq != netdev->requested_n_rxq
 || new_n_txq != netdev->requested_n_txq
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH v2] dpdk: Late initialization.

[nickcooper-zhangtonghao]

hi Daniele,
I reviewed this patch. One question to ask: should we check the
hugepage mm before calling the rte_eal_init()? improvement on next version?


Thanks.
Nick

> On Jan 9, 2017, at 11:21 AM, Daniele Di Proietto  
> wrote:
> 
> With this commit, we allow the user to set other_config:dpdk-init=true
> after the process is started.  This makes it easier to start Open
> vSwitch with DPDK using standard init scripts without restarting the
> service.
> 
> This is still far from ideal, because initializing DPDK might still
> abort the process (e.g. if there not enough memory), so the user must
> check the status of the process after setting dpdk-init to true.
> 
> Nonetheless, I think this is an improvement, because it doesn't require
> restarting the whole unit.
> 
> CC: Aaron Conole >
> Signed-off-by: Daniele Di Proietto  >
> ---
> v1->v2: No change, first non-RFC post.
> ---
> lib/dpdk-stub.c |  8 
> lib/dpdk.c  | 30 --
> tests/ofproto-macros.at  |  2 +-
> 3 files changed, 25 insertions(+), 15 deletions(-)

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

Re: [ovs-dev] [PATCH 2/4] datapath: Limits the number of tx/rx queues for netdev-dummy.

[nickcooper-zhangtonghao]

Thanks Daniele,
Yes, it’s a small improvement. but it is necessary for us. I will check it in 
set_config(). One question to ask: should we check the tx/rx queue for 
netdev-dpdk in set_config()?

Now we check it in dpdk_eth_dev_init().

Thanks.



> On Jan 9, 2017, at 11:22 AM, Daniele Di Proietto <diproiet...@ovn.org> wrote:
> 
> 2017-01-08 17:30 GMT-08:00 nickcooper-zhangtonghao <n...@opencloud.tech 
> <mailto:n...@opencloud.tech>>:
>> This patch avoids the ovs_rcu to report WARN, caused by blocked
>> for a long time, when ovs-vswitchd processes a port with many
>> rx/tx queues. The number of tx/rx queues per port may be appropriate,
>> because the dpdk uses it as an default max value.
>> 
>> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech 
>> <mailto:n...@opencloud.tech>>
> 
> I don't think this is a big deal, since netdev-dummy is only used for
> testing, but don't you think it's better to check it in set_config()
> and return an error?
> 
> Also, could you use the prefix netdev-dummy, instead of datapath?
> 
> Thanks,
> 
> Daniele

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

[ovs-dev] [PATCH 4/4] datapath: Uses the OVS_CORE_UNSPEC instead of magic numbers.

[nickcooper-zhangtonghao]

This patch uses OVS_CORE_UNSPEC for the queue unpinned instead
of "-1". More important, the "-1" casted to unsigned int is
equal to NON_PMD_CORE_ID. We make the distinction between them.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/dpif-netdev.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0b73056..99e4d35 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1293,7 +1293,7 @@ port_create(const char *devname, const char *type,
  devname, ovs_strerror(errno));
 goto out_rxq_close;
 }
-port->rxqs[i].core_id = -1;
+port->rxqs[i].core_id = OVS_CORE_UNSPEC;
 n_open_rxqs++;
 }
 
@@ -1517,7 +1517,7 @@ has_pmd_rxq_for_numa(struct dp_netdev *dp, int numa_id)
 for (i = 0; i < port->n_rxq; i++) {
 unsigned core_id = port->rxqs[i].core_id;
 
-if (core_id != -1
+if (core_id != OVS_CORE_UNSPEC
 && ovs_numa_get_numa_id(core_id) == numa_id) {
 return true;
 }
@@ -2704,7 +2704,7 @@ parse_affinity_list(const char *affinity_list, unsigned 
*core_ids, int n_rxq)
 int error = 0;
 
 for (i = 0; i < n_rxq; i++) {
-core_ids[i] = -1;
+core_ids[i] = OVS_CORE_UNSPEC;
 }
 
 if (!affinity_list) {
@@ -3617,7 +3617,7 @@ dp_netdev_add_port_rx_to_pmds(struct dp_netdev *dp,
 
 for (i = 0; i < port->n_rxq; i++) {
 if (pinned) {
-if (port->rxqs[i].core_id == -1) {
+if (port->rxqs[i].core_id == OVS_CORE_UNSPEC) {
 continue;
 }
 pmd = dp_netdev_get_pmd(dp, port->rxqs[i].core_id);
@@ -3631,7 +3631,7 @@ dp_netdev_add_port_rx_to_pmds(struct dp_netdev *dp,
 pmd->isolated = true;
 dp_netdev_pmd_unref(pmd);
 } else {
-if (port->rxqs[i].core_id != -1) {
+if (port->rxqs[i].core_id != OVS_CORE_UNSPEC) {
 continue;
 }
 pmd = dp_netdev_less_loaded_pmd_on_numa(dp, numa_id);
@@ -3760,7 +3760,7 @@ dp_netdev_reset_pmd_threads(struct dp_netdev *dp)
 for (i = 0; i < port->n_rxq; i++) {
 unsigned core_id = port->rxqs[i].core_id;
 
-if (core_id != -1) {
+if (core_id != OVS_CORE_UNSPEC) {
 numa_id = ovs_numa_get_numa_id(core_id);
 hmapx_add(, (void *) numa_id);
 }
-- 
1.8.3.1



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

[ovs-dev] [PATCH 3/4] datapath: Uses the NR_QUEUE instead of magic numbers.

[nickcooper-zhangtonghao]

The NR_QUEUE is defined in "lib/dpif-netdev.h", netdev-dpdk
uses it instead of magic number. netdev-dummy should be
in the same case.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/netdev-dummy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index d75e597..8d9c805 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -868,8 +868,8 @@ netdev_dummy_set_config(struct netdev *netdev_, const 
struct smap *args)
 goto exit;
 }
 
-new_n_rxq = MAX(smap_get_int(args, "n_rxq", 1), 1);
-new_n_txq = MAX(smap_get_int(args, "n_txq", 1), 1);
+new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
+new_n_txq = MAX(smap_get_int(args, "n_txq", NR_QUEUE), 1);
 new_numa_id = smap_get_int(args, "numa_id", 0);
 if (new_n_rxq != netdev->requested_n_rxq
 || new_n_txq != netdev->requested_n_txq
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/4] datapath: Fix formatting typo.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 625f425..376aa4d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -738,7 +738,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 memset(_addr, 0x0, sizeof(eth_addr));
 rte_eth_macaddr_get(dev->port_id, _addr);
-VLOG_INFO_RL(, "Port %d: "ETH_ADDR_FMT"",
+VLOG_INFO_RL(, "Port %d: "ETH_ADDR_FMT,
 dev->port_id, ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes));
 
 memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN);
-- 
1.8.3.1




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

[ovs-dev] [PATCH] openvswitch: FTBFS with dpkg-buildpackage.

[nickcooper-zhangtonghao]

The debian packages are ready. This patch fixes the
bug #831924 reported at debian bug tracking system.
With this patch, openvswitch-2.6.1 will be upload to
the Debian archive.  If we build the packages with
"dpkg-buildpackage --target binary-indep", an error
state arises. debian/rules should be modified so that
the build-indep and binary-indep target generates
the architecture independent packages. If there are
things not be handled properly，let me know.

Reported-at: 
https://people.debian.org/~lucas/logs/2016/07/20/openvswitch_2.5.1~pre+git20160626-2_unstable_archallonly.log
Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 debian/rules | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/debian/rules b/debian/rules
index 4c34b07..12fb94a 100755
--- a/debian/rules
+++ b/debian/rules
@@ -48,7 +48,7 @@ override_dh_auto_clean:
rm -f python/ovs/*.pyc python/ovs/db/*.pyc
dh_auto_clean
 
-override_dh_install:
+override_dh_install-arch:
dh_install
# openvswitch-switch
cp debian/openvswitch-switch.template 
debian/openvswitch-switch/usr/share/openvswitch/switch/default.template
@@ -59,6 +59,9 @@ override_dh_install:
# ovn-central
cp debian/ovn-central.template 
debian/ovn-central/usr/share/ovn/central/default.template
 
+override_dh_install-indep:
+   dh_install
+
# openvswitch-datapath-source
cp debian/rules.modules 
debian/openvswitch-datapath-source/usr/src/modules/openvswitch-datapath/debian/rules
chmod 755 
debian/openvswitch-datapath-source/usr/src/modules/openvswitch-datapath/debian/rules
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH 1/3] datapath: Checks the MTU for netdev-internal ports.

[nickcooper-zhangtonghao]

Thanks Jarno, I will try it.
Thanks.
Nick

> On Dec 13, 2016, at 4:58 AM, Jarno Rajahalme  wrote:
> 
> We should first backport this upstream commit and see if there is anything 
> left to fix. Either way, Linux kernel datapath fixes should be first fixed on 
> the upstream net-next repo:
> 
> - 91572088e3 (“net: use core MTU range checking in core net infra”)
> 
>  Jarno

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

[ovs-dev] [PATCH 1/3] datapath: Checks the MTU for netdev-internal ports.

[nickcooper-zhangtonghao]

We should check the MTU before changing it.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 datapath/vport-internal_dev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 482af37..515fca0 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -89,9 +89,12 @@ static const struct ethtool_ops internal_dev_ethtool_ops = {
.get_link   = ethtool_op_get_link,
 };
 
+#define INTERNAL_MIN_MTU 68/* Min L3 MTU. */
+#define INTERNAL_MAX_MTU 65535 /* Max L3 MTU (arbitrary). */
+
 static int internal_dev_change_mtu(struct net_device *netdev, int new_mtu)
 {
-   if (new_mtu < 68)
+   if (new_mtu < INTERNAL_MIN_MTU || new_mtu > INTERNAL_MAX_MTU)
return -EINVAL;
 
netdev->mtu = new_mtu;
-- 
1.8.3.1




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

Re: [ovs-dev] request for assistance with Debian maintenance

[nickcooper-zhangtonghao]

Hi Ben.
If  there  were not better man for this  job，i may make a contribution to 
openvswitch and do a good job with it . It is a really intersting job and  
probably is  the most worthwhile to  invest more  time developing and  learning 
. I  submited patches for openvswitch and  ovn. However if there is someone who 
is expert in openvswitch and debian system, it will  be beneficial for us.

Thanks.
Nick

> On Dec 7, 2016, at 12:46 AM, Ben Pfaff  wrote:
> 
> I've been doing a bad job as Debian maintainer for Open vSwitch.  I'd
> very much appreciate help from a co-maintainer who can invest more time
> in doing a good job with Debian uploads and bug tracking.
> 
> Thanks,
> 
> Ben.
> ___
> 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 1/2] ovs-sandbox: add '--vswitchd-unforced-dummy' option.

[nickcooper-zhangtonghao]

The ovs-sandbox runs in the "dummy mode" by default.
In this mode of testing, no packets travel across
physical or virtual networks. But sometimes, we may
create veth network devices and add them to ovs bridge
for developing and testing. It's necessary to add an option.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 tutorial/ovs-sandbox | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index 4372da4..4710576 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -69,6 +69,7 @@ built=false
 ovn=false
 ovnsb_schema=
 ovnnb_schema=
+vswitchd_dummy="override"
 
 for option; do
 # This option-parsing mechanism borrowed from a Autoconf-generated
@@ -112,6 +113,7 @@ These options force ovs-sandbox to use an installed Open 
vSwitch:
   --gdb-ovn-northd run ovn-northd under gdb
   --gdb-ovn-controller run ovn-controller under gdb
   --gdb-ovn-controller-vtep run ovn-controller-vtep under gdb
+  --vswitchd-unforced-dummy run ovs-vswitchd with unforced-dummy
   -R, --gdb-runautomatically start running the daemon in gdb
for any daemon set to run under gdb
   -S, --schema=FILEuse FILE as vswitch.ovsschema
@@ -135,6 +137,10 @@ EOF
 srcdir=$optarg
 built=false
 ;;
+--vswitchd-unforced-dummy)
+# Support dummy but don't force its use.
+vswitchd_dummy=
+;;
 -s|--sr*)
 prev=srcdir
 built=false
@@ -361,7 +367,7 @@ run ovs-vsctl --no-wait -- init
 
 # Start ovs-vswitchd.
 rungdb $gdb_vswitchd $gdb_vswitchd_ex ovs-vswitchd --detach --no-chdir 
--pidfile -vconsole:off --log-file \
---enable-dummy=override -vvconn -vnetdev_dummy
+--enable-dummy=$vswitchd_dummy -vvconn -vnetdev_dummy
 
 if $ovn; then
 ovs-vsctl set open . 
external-ids:system-id=56b18105-5706-46ef-80c4-ff20979ab068
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH 1/4] ovs-vswitchd: Avoid segfault for "netdev" datapath.

[nickcooper-zhangtonghao]

Hi Ben and Daniele,

Thanks for your tips. I submitted the patch again. The test has been added.
May I also add “Signed-off-by: Daniele Di Proietto ” ?


Thanks.
Nick

> On Dec 7, 2016, at 7:51 AM, Daniele Di Proietto  wrote:
> 
> Thanks for the patch
> 
>>> ---
>>> lib/dpif-netdev.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index 1400511..8175b7e 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -4380,11 +4380,12 @@ dp_execute_userspace_action(struct 
>>> dp_netdev_pmd_thread *pmd,
>>> const struct nlattr *userdata, long long now)
>>> {
>>> struct dp_packet_batch b;
>>> +struct flow_wildcards wc;
>>> int error;
>>> 
>>> ofpbuf_clear(actions);
>>> 
>>> -error = dp_netdev_upcall(pmd, packet, flow, NULL, ufid,
>>> +error = dp_netdev_upcall(pmd, packet, flow, , ufid,
>>>  DPIF_UC_ACTION, userdata, actions,
>>>  NULL);
>>> if (!error || error == ENOSPC) {
>> 
>> I'm not too familiar with dpif-netdev.  However, there's probably some
>> reason that this wasn't done to start with; for example, it might be a
>> performance optimization of some kind.  Therefore, it's probably best to
>> at least consider fixing whatever function is doing the null
>> dereference.
> 
> I agree, it seems better to handle the case where 'wc' is NULL in upcall_cb().
> process_upcall() already handles it for non-miss upcalls, i.e. when coming 
> from
> an OVS_ACTION_ATTR_USERSPACE datapath action.
> 
> Also, I think we could add a test, like the following:

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

[ovs-dev] [PATCH 2/4] ovs-sandbox: add '-u, --disable-dummy' options.

[nickcooper-zhangtonghao]

The ovs-sandbox runs in the "dummy mode" by default.
In this mode of testing, no packets travel across
physical or virtual networks. But sometimes, we may
create veth network devices and add them to ovs bridge
for developing and testing. It's necessary to add an option.

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 tutorial/ovs-sandbox | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index 4372da4..62f0844 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -61,6 +61,7 @@ gdb_ovn_controller=false
 gdb_ovn_controller_ex=false
 gdb_ovn_controller_vtep=false
 gdb_ovn_controller_vtep_ex=false
+disable_dummy_vswitchd=false
 builddir=
 srcdir=
 schema=
@@ -108,6 +109,7 @@ These options force ovs-sandbox to use a particular OVS 
build:
 These options force ovs-sandbox to use an installed Open vSwitch:
   -i, --installed  use installed Open vSwitch
   -g, --gdb-vswitchd   run ovs-vswitchd under gdb
+  -u, --disable-dummy  run ovs-vswitchd with disable-dummy mode
   -d, --gdb-ovsdb  run ovsdb-server under gdb
   --gdb-ovn-northd run ovn-northd under gdb
   --gdb-ovn-controller run ovn-controller under gdb
@@ -154,6 +156,9 @@ EOF
 gdb_vswitchd=true
 gdb_vswitchd_ex=false
 ;;
+-u|--disable-dummy)
+disable_dummy_vswitchd=true
+;;
 -e|--gdb-ex-v*)
 gdb_vswitchd=true
 gdb_vswitchd_ex=true
@@ -360,8 +365,13 @@ fi
 run ovs-vsctl --no-wait -- init
 
 # Start ovs-vswitchd.
+ovs_vswitchd_dummy_arg="--enable-dummy=override -vnetdev_dummy"
+if $disable_dummy_vswitchd; then
+ovs_vswitchd_dummy_arg=
+fi
+
 rungdb $gdb_vswitchd $gdb_vswitchd_ex ovs-vswitchd --detach --no-chdir 
--pidfile -vconsole:off --log-file \
---enable-dummy=override -vvconn -vnetdev_dummy
+-vvconn $ovs_vswitchd_dummy_arg
 
 if $ovn; then
 ovs-vsctl set open . 
external-ids:system-id=56b18105-5706-46ef-80c4-ff20979ab068
-- 
1.8.3.1



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

[ovs-dev] [PATCH 4/4] ovs-appctl: Add usage content to "upcall/set-flow-limit" command.

[nickcooper-zhangtonghao]

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 ofproto/ofproto-dpif-upcall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 84f1de2..6cb9c2e 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -395,8 +395,8 @@ udpif_init(void)
  upcall_unixctl_disable_ufid, NULL);
 unixctl_command_register("upcall/enable-ufid", "", 0, 0,
  upcall_unixctl_enable_ufid, NULL);
-unixctl_command_register("upcall/set-flow-limit", "", 1, 1,
- upcall_unixctl_set_flow_limit, NULL);
+unixctl_command_register("upcall/set-flow-limit", "flow-limit-number",
+ 1, 1, upcall_unixctl_set_flow_limit, NULL);
 unixctl_command_register("revalidator/wait", "", 0, 0,
  upcall_unixctl_dump_wait, NULL);
 unixctl_command_register("revalidator/purge", "", 0, 0,
-- 
1.8.3.1



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

[ovs-dev] [PATCH 1/4] ovs-vswitchd: Avoid segfault for "netdev" datapath.

[nickcooper-zhangtonghao]

When the datapath, whose type is "netdev", processes packets
in userspce action, it may cause a segmentation fault. In the
dp_execute_userspace_action(), we pass the "wc" argument to
dp_netdev_upcall() using NULL. In the dp_netdev_upcall() call tree,
the "wc" will be used. For example, dp_netdev_upcall() uses the
>masks for debugging, and flow_wildcards_init_for_packet()
uses the  "wc" if we disable megaflow, which is described in
more detail below.

Segmentation fault in flow_wildcards_init_for_packet:

#0  0x00468fe8 flow_wildcards_init_for_packet lib/flow.c:1275
#1  0x00436c0b upcall_cb ofproto/ofproto-dpif-upcall.c:1231
#2  0x0045bd96 dp_netdev_upcall lib/dpif-netdev.c:3857
#3  0x00461bf3 dp_execute_userspace_action lib/dpif-netdev.c:4388
#4  dp_execute_cb lib/dpif-netdev.c:4521
#5  0x00486ae2 odp_execute_actions lib/odp-execute.c:538
#6  0x004607f9 dp_netdev_execute_actions lib/dpif-netdev.c:4627
#7  packet_batch_per_flow_execute lib/dpif-netdev.c:3927
#8  dp_netdev_input__ lib/dpif-netdev.c:4229
#9  0x00460ba8 dp_netdev_input lib/dpif-netdev.c:4238
#10 dp_netdev_process_rxq_port lib/dpif-netdev.c:2873
#11 0x0046126e dpif_netdev_run lib/dpif-netdev.c:3000
#12 0x0042baf5 type_run ofproto/ofproto-dpif.c:504
#13 0x004192bf ofproto_type_run ofproto/ofproto.c:1687
#14 0x00409965 bridge_run__ vswitchd/bridge.c:2875
#15 0x0040f145 bridge_run vswitchd/bridge.c:2938
#16 0x004062e5 main vswitchd/ovs-vswitchd.c:111

Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
---
 lib/dpif-netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 1400511..8175b7e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4380,11 +4380,12 @@ dp_execute_userspace_action(struct dp_netdev_pmd_thread 
*pmd,
 const struct nlattr *userdata, long long now)
 {
 struct dp_packet_batch b;
+struct flow_wildcards wc;
 int error;
 
 ofpbuf_clear(actions);
 
-error = dp_netdev_upcall(pmd, packet, flow, NULL, ufid,
+error = dp_netdev_upcall(pmd, packet, flow, , ufid,
  DPIF_UC_ACTION, userdata, actions,
  NULL);
 if (!error || error == ENOSPC) {
-- 
1.8.3.1



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

Re: [ovs-dev] [PATCH 4/4] ovs: optimize 'ip_parse_port' function.

[nickcooper-zhangtonghao]

Hi Guru Shetty

I folded in the following minor incremental just because ovs_scan_len() is only 
really meant for
situations where the 'n' offset is being incremented over several calls.

Thanks.
Nick

> On Nov 19, 2016, at 2:35 AM, Guru Shetty  wrote:
> 
> Can you tell why one is better than the other?
>  
> ---
>  lib/packets.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/packets.c b/lib/packets.c
> index 990c407..1d2d452 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -436,15 +436,12 @@ char * OVS_WARN_UNUSED_RESULT
>  ip_parse_port(const char *s, ovs_be32 *ip, ovs_be16 *port)
>  {
>  int n = 0;
> -if (!ovs_scan_len(s, , IP_PORT_SCAN_FMT,
> -IP_PORT_SCAN_ARGS(ip, port))) {
> -return xasprintf("%s: invalid IP address or port number", s);
> +if (ovs_scan(s, IP_PORT_SCAN_FMT"%n", IP_PORT_SCAN_ARGS(ip, port), )
> +&& !s[n]) {
> +return NULL;
>  }
> 
> -if (s[n]) {
> -return xasprintf("%s: invalid IP address or port number", s);
> -}
> -return NULL;
> +return xasprintf("%s: invalid IP address or port number", s);
>  }

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

Re: [ovs-dev] [PATCH 1/4] ovn: Support DNAT with port.

[nickcooper-zhangtonghao]

I got it. Thank you explaining this. Can you review the v1 of NAT command,
and other patches ? Thanks very much.

http://patchwork.ozlabs.org/patch/680778/ 

http://patchwork.ozlabs.org/patch/680777/ 

http://patchwork.ozlabs.org/patch/689737/ 


Thanks.
Nick

> On Nov 15, 2016, at 11:58 PM, Guru Shetty  wrote:
> 
> We can do that with LB.

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

Re: [ovs-dev] [PATCH 1/4] ovn: Support DNAT with port.

[nickcooper-zhangtonghao]

Hi Guru Shetty.
LB may support DNAT with port. But in some case, there is only a public ip 
address set on the gateway router, which is connected to multiple VMs. Thus 
this public ip address need to be projected to multiple private ip address. 
They denote a one to many relationship not one to one.

I don't have many public ip, so it is not possible to do VIP:port = 
PrivateIP:port. I hope that we can do VIP:port1 = PrivateIP1:port1， 
VIP:port2 = PrivateIP2:port2 ...

Thanks.
Nick

> On Nov 14, 2016, at 11:56 PM, Guru Shetty  wrote:
> 
> We do support it right now via LB right? i.e. one VIP:port = PrivateIP:Port. 
> With just one target, it acts the same as a DNAT. 

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