On the right track, mostly nits at this point.
I'd prefer we simply copied the hw_addr around instead of maintaining
a pointer to the same data in multiple threads. It's only 6 bytes, so
it shouldn't be too expensive.
Let's rename ofproto_dpif_monitor_mport_update() =>
ofproto_dpif_monitor_port_update(). The rest of the code shouldn't
need to know anything about mports.
In xlate_ofport_set() could you just get the hw_addr from the netdev
instead of passing it as another argument?
We could ditch the update_monitor bool pretty easily by doing
something like the following:
if (xport->bfd != bfd || xport->cfm != cfm) {
bfd_unref()
cfm_unref()
xport->bfd = bfd
xport->cfm = cfm
port_update()
}
Less variables means less wrong variables.
I think it'd be cleaner if the stats mutex change was pulled into it's
own separate patch before this one and the previous.
I don't totally get the unit test changes. Could you explain them?
Is there some simpler way to achieve what you're going for? Why are
they needed?
Thanks,
Ethan
On Fri, Sep 27, 2013 at 3:40 PM, Alex Wang <al...@nicira.com> wrote:
> This commit adds a new module ofproto-dpif-monitor in ofproto
> directory. This module is in charge of executing the periodic
> functions of monitoring code (e.g. bfd and cfm).
>
> Signed-off-by: Alex Wang <al...@nicira.com>
> ---
>
> v1 -> v2:
> - ditch the monitor struct in the monitor module.
> - add monitor_rwlock in the monitor module.
> - use pointer to ofport_dpif struct as hash.
> - add the bfd/cfm reference to mport.
> - make ofproto.stats update thread safe in ofproto-dpif.c.
>
> ---
> ofproto/automake.mk | 2 +
> ofproto/ofproto-dpif-monitor.c | 201
> ++++++++++++++++++++++++++++++++++++++++
> ofproto/ofproto-dpif-monitor.h | 33 +++++++
> ofproto/ofproto-dpif-xlate.c | 20 +++-
> ofproto/ofproto-dpif-xlate.h | 5 +-
> ofproto/ofproto-dpif.c | 66 +++----------
> tests/bfd.at | 23 +++--
> 7 files changed, 286 insertions(+), 64 deletions(-)
> create mode 100644 ofproto/ofproto-dpif-monitor.c
> create mode 100644 ofproto/ofproto-dpif-monitor.h
>
> diff --git a/ofproto/automake.mk b/ofproto/automake.mk
> index 47ca1b8..432f083 100644
> --- a/ofproto/automake.mk
> +++ b/ofproto/automake.mk
> @@ -28,6 +28,8 @@ ofproto_libofproto_a_SOURCES = \
> ofproto/ofproto-dpif-ipfix.h \
> ofproto/ofproto-dpif-mirror.c \
> ofproto/ofproto-dpif-mirror.h \
> + ofproto/ofproto-dpif-monitor.c \
> + ofproto/ofproto-dpif-monitor.h \
> ofproto/ofproto-dpif-sflow.c \
> ofproto/ofproto-dpif-sflow.h \
> ofproto/ofproto-dpif-upcall.c \
> diff --git a/ofproto/ofproto-dpif-monitor.c b/ofproto/ofproto-dpif-monitor.c
> new file mode 100644
> index 0000000..97c6e40
> --- /dev/null
> +++ b/ofproto/ofproto-dpif-monitor.c
> @@ -0,0 +1,201 @@
> +/*
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "ofproto-dpif-monitor.h"
> +
> +#include "bfd.h"
> +#include "cfm.h"
> +#include "hash.h"
> +#include "hmap.h"
> +#include "ofpbuf.h"
> +#include "ofproto-dpif.h"
> +#include "util.h"
> +#include "vlog.h"
> +
> +/* Monitored port. It contains references to ofport, bfd, cfm structs. */
> +struct mport {
> + struct hmap_node hmap_node; /* In monitor's hmap. */
> + const struct ofport_dpif *ofport; /* The corresponding ofport. */
> +
> + struct cfm *cfm; /* Reference to cfm. */
> + struct bfd *bfd; /* Reference to bfd. */
> + uint8_t *hw_addr; /* Hardware address. */
> +};
> +
> +/* hmap that contains all port monitors. */
> +static struct hmap monitor_hmap = HMAP_INITIALIZER(&monitor_hmap);
> +
> +static struct ovs_rwlock monitor_rwlock = OVS_RWLOCK_INITIALIZER;
> +
> +static void mport_register(const struct ofport_dpif *, struct bfd *,
> + struct cfm *, uint8_t *)
> + OVS_REQ_WRLOCK(monitor_rwlock);
> +static void mport_unregister(const struct ofport_dpif *)
> + OVS_REQ_WRLOCK(monitor_rwlock);
> +static void mport_update(struct mport *, struct bfd *, struct cfm *, uint8_t
> *)
> + OVS_REQ_WRLOCK(monitor_rwlock);
> +static struct mport *mport_find(const struct ofport_dpif *)
> + OVS_REQ_WRLOCK(monitor_rwlock);
> +
> +/* Tries finding and returning the 'mport' from the monitor's hash map.
> + * If there is no such 'mport', returns NULL. */
> +static struct mport *
> +mport_find(const struct ofport_dpif *ofport) OVS_REQ_WRLOCK(monitor_rwlock)
> +{
> + struct mport *node;
> +
> + HMAP_FOR_EACH_WITH_HASH (node, hmap_node, hash_pointer(ofport, 0),
> + &monitor_hmap) {
> + if (node->ofport == ofport) {
> + return node;
> + }
> + }
> + return NULL;
> +}
> +
> +/* Creates a new mport and inserts it into monitor_hmap, if it doesn't exist.
> + * Otherwise, just updates its fields. */
> +static void
> +mport_register(const struct ofport_dpif *ofport, struct bfd *bfd,
> + struct cfm *cfm, uint8_t *hw_addr)
> + OVS_REQ_WRLOCK(monitor_rwlock)
> +{
> + struct mport *mport = mport_find(ofport);
> +
> + if (!mport) {
> + mport = xzalloc(sizeof *mport);
> + mport->ofport = ofport;
> + hmap_insert(&monitor_hmap, &mport->hmap_node, hash_pointer(ofport,
> 0));
> + }
> + mport_update(mport, bfd, cfm, hw_addr);
> +}
> +
> +/* Removes mport from monitor_hmap and frees it. */
> +static void
> +mport_unregister(const struct ofport_dpif *ofport)
> + OVS_REQ_WRLOCK(monitor_rwlock)
> +{
> + struct mport *mport = mport_find(ofport);
> +
> + if (mport) {
> + mport_update(mport, NULL, NULL, NULL);
> + hmap_remove(&monitor_hmap, &mport->hmap_node);
> + free(mport);
> + }
> +}
> +
> +/* Updates the fields of an existing mport struct. */
> +static void
> +mport_update(struct mport *mport, struct bfd *bfd, struct cfm *cfm,
> + uint8_t *hw_addr) OVS_REQ_WRLOCK(monitor_rwlock)
> +{
> + ovs_assert(mport);
> +
> + if (mport->cfm != cfm) {
> + cfm_unref(mport->cfm);
> + mport->cfm = cfm_ref(cfm);
> + }
> + if (mport->bfd != bfd) {
> + bfd_unref(mport->bfd);
> + mport->bfd = bfd_ref(bfd);
> + }
> + if (mport->hw_addr != hw_addr) {
> + mport->hw_addr = hw_addr;
> + }
> +}
> +
> +
> +/* Creates the mport in monitor module if either bfd or cfm
> + * is configured. Otherwise, deletes the mport. */
> +void
> +ofproto_dpif_monitor_mport_update(const struct ofport_dpif *ofport,
> + struct bfd *bfd, struct cfm *cfm,
> + uint8_t *hw_addr)
> +{
> + ovs_rwlock_wrlock(&monitor_rwlock);
> + if (!cfm && !bfd) {
> + mport_unregister(ofport);
> + } else {
> + mport_register(ofport, bfd, cfm, hw_addr);
> + }
> + ovs_rwlock_unlock(&monitor_rwlock);
> +}
> +
> +/* Checks the sending of control packets on all mports. Sends the control
> + * packets if needed. */
> +void
> +ofproto_dpif_monitor_run_fast(void)
> +{
> + struct mport *mport;
> +
> + ovs_rwlock_rdlock(&monitor_rwlock);
> + HMAP_FOR_EACH (mport, hmap_node, &monitor_hmap) {
> + if (mport->cfm && cfm_should_send_ccm(mport->cfm)) {
> + struct ofpbuf packet;
> +
> + ofpbuf_init(&packet, 0);
> + cfm_compose_ccm(mport->cfm, &packet, mport->hw_addr);
> + ofproto_dpif_send_packet(mport->ofport, &packet);
> + ofpbuf_uninit(&packet);
> + }
> + if (mport->bfd && bfd_should_send_packet(mport->bfd)) {
> + struct ofpbuf packet;
> +
> + ofpbuf_init(&packet, 0);
> + bfd_put_packet(mport->bfd, &packet, mport->hw_addr);
> + ofproto_dpif_send_packet(mport->ofport, &packet);
> + ofpbuf_uninit(&packet);
> + }
> + }
> + ovs_rwlock_unlock(&monitor_rwlock);
> +}
> +
> +/* Executes bfd_run(), cfm_run() on all mports. */
> +void
> +ofproto_dpif_monitor_run(void)
> +{
> + struct mport *mport;
> +
> + ovs_rwlock_rdlock(&monitor_rwlock);
> + HMAP_FOR_EACH (mport, hmap_node, &monitor_hmap) {
> + if (mport->cfm) {
> + cfm_run(mport->cfm);
> + }
> + if (mport->bfd) {
> + bfd_run(mport->bfd);
> + }
> + }
> + ovs_rwlock_unlock(&monitor_rwlock);
> +}
> +
> +/* Executes the bfd_wait() and cfm_wait() functions on all mports. */
> +void
> +ofproto_dpif_monitor_wait(void)
> +{
> + struct mport *mport;
> +
> + ovs_rwlock_rdlock(&monitor_rwlock);
> + HMAP_FOR_EACH (mport, hmap_node, &monitor_hmap) {
> + if (mport->cfm) {
> + cfm_wait(mport->cfm);
> + }
> + if (mport->bfd) {
> + bfd_wait(mport->bfd);
> + }
> + }
> + ovs_rwlock_unlock(&monitor_rwlock);
> +}
> diff --git a/ofproto/ofproto-dpif-monitor.h b/ofproto/ofproto-dpif-monitor.h
> new file mode 100644
> index 0000000..5a03883
> --- /dev/null
> +++ b/ofproto/ofproto-dpif-monitor.h
> @@ -0,0 +1,33 @@
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License. */
> +
> +#ifndef OFPROTO_DPIF_MONITOR_H
> +#define OFPROTO_DPIF_MONITOR_H 1
> +
> +#include <stdint.h>
> +
> +#include "compiler.h"
> +
> +struct bfd;
> +struct cfm;
> +struct ofport_dpif;
> +
> +void ofproto_dpif_monitor_run(void);
> +void ofproto_dpif_monitor_run_fast(void);
> +void ofproto_dpif_monitor_wait(void);
> +
> +void ofproto_dpif_monitor_mport_update(const struct ofport_dpif *,
> + struct bfd *, struct cfm *,
> + uint8_t *);
> +#endif /* ofproto-dpif-monitor.h */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 020588f..3bd9d9e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -42,6 +42,7 @@
> #include "ofp-actions.h"
> #include "ofproto/ofproto-dpif-ipfix.h"
> #include "ofproto/ofproto-dpif-mirror.h"
> +#include "ofproto/ofproto-dpif-monitor.h"
> #include "ofproto/ofproto-dpif-sflow.h"
> #include "ofproto/ofproto-dpif.h"
> #include "ofproto/ofproto-provider.h"
> @@ -131,6 +132,8 @@ struct xport {
>
> struct cfm *cfm; /* CFM handle or null. */
> struct bfd *bfd; /* BFD handle or null. */
> +
> + uint8_t *hw_addr; /* Hardware address. */
> };
>
> struct xlate_ctx {
> @@ -374,10 +377,11 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct
> ofbundle *ofbundle,
> struct ofport_dpif *peer, int stp_port_no,
> const struct ofproto_port_queue *qdscp_list, size_t n_qdscp,
> enum ofputil_port_config config, bool is_tunnel,
> - bool may_enable)
> + uint8_t *hw_addr, bool may_enable)
> {
> struct xport *xport = xport_lookup(ofport);
> size_t i;
> + bool update_monitor = false;
>
> if (!xport) {
> xport = xzalloc(sizeof *xport);
> @@ -407,11 +411,18 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct
> ofbundle *ofbundle,
> if (xport->cfm != cfm) {
> cfm_unref(xport->cfm);
> xport->cfm = cfm_ref(cfm);
> + update_monitor = true;
> }
>
> if (xport->bfd != bfd) {
> bfd_unref(xport->bfd);
> xport->bfd = bfd_ref(bfd);
> + update_monitor = true;
> + }
> +
> + if (xport->hw_addr != hw_addr) {
> + xport->hw_addr = hw_addr;
> + update_monitor = true;
> }
>
> if (xport->peer) {
> @@ -430,6 +441,11 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct
> ofbundle *ofbundle,
> list_insert(&xport->xbundle->xports, &xport->bundle_node);
> }
>
> + /* Updates the monitor hmap. */
> + if (update_monitor) {
> + ofproto_dpif_monitor_mport_update(xport->ofport, xport->bfd,
> + xport->cfm, xport->hw_addr);
> + }
> clear_skb_priorities(xport);
> for (i = 0; i < n_qdscp; i++) {
> struct skb_priority_to_dscp *pdscp;
> @@ -475,6 +491,8 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
> netdev_close(xport->netdev);
> cfm_unref(xport->cfm);
> bfd_unref(xport->bfd);
> + /* Remove xport from monitor hmap. */
> + ofproto_dpif_monitor_mport_update(xport->ofport, NULL, NULL, NULL);
> free(xport);
> }
>
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
> index 9e7a853..89ea5ed 100644
> --- a/ofproto/ofproto-dpif-xlate.h
> +++ b/ofproto/ofproto-dpif-xlate.h
> @@ -15,6 +15,8 @@
> #ifndef OFPROTO_DPIF_XLATE_H
> #define OFPROTO_DPIF_XLATE_H 1
>
> +#include <stdint.h>
> +
> #include "flow.h"
> #include "meta-flow.h"
> #include "odp-util.h"
> @@ -137,7 +139,8 @@ void xlate_ofport_set(struct ofproto_dpif *, struct
> ofbundle *,
> const struct bfd *, struct ofport_dpif *peer,
> int stp_port_no, const struct ofproto_port_queue
> *qdscp,
> size_t n_qdscp, enum ofputil_port_config, bool
> is_tunnel,
> - bool may_enable) OVS_REQ_WRLOCK(xlate_rwlock);
> + uint8_t *hw_addr, bool may_enable)
> + OVS_REQ_WRLOCK(xlate_rwlock);
> void xlate_ofport_remove(struct ofport_dpif *) OVS_REQ_WRLOCK(xlate_rwlock);
>
> int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 42f3d98..b90815a 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -52,6 +52,7 @@
> #include "ofproto-dpif-governor.h"
> #include "ofproto-dpif-ipfix.h"
> #include "ofproto-dpif-mirror.h"
> +#include "ofproto-dpif-monitor.h"
> #include "ofproto-dpif-sflow.h"
> #include "ofproto-dpif-upcall.h"
> #include "ofproto-dpif-xlate.h"
> @@ -360,8 +361,6 @@ ofport_dpif_cast(const struct ofport *ofport)
> }
>
> static void port_run(struct ofport_dpif *);
> -static void port_run_fast(struct ofport_dpif *);
> -static void port_wait(struct ofport_dpif *);
> static int set_bfd(struct ofport *, const struct smap *);
> static int set_cfm(struct ofport *, const struct cfm_settings *);
> static void ofport_update_peer(struct ofport_dpif *);
> @@ -484,6 +483,7 @@ struct ofproto_dpif {
> struct classifier facets; /* Contains 'struct facet's. */
> long long int consistency_rl;
>
> + struct ovs_mutex stats_mutex;
> struct netdev_stats stats; /* To account packets generated and consumed
> in
> * userspace. */
>
> @@ -817,7 +817,7 @@ type_run(const char *type)
> ofport->bfd, ofport->peer, stp_port,
> ofport->qdscp, ofport->n_qdscp,
> ofport->up.pp.config, ofport->is_tunnel,
> - ofport->may_enable);
> + ofport->up.pp.hw_addr, ofport->may_enable);
> }
> ovs_rwlock_unlock(&xlate_rwlock);
>
> @@ -1252,6 +1252,7 @@ construct(struct ofproto *ofproto_)
> ofproto->ml = mac_learning_create(MAC_ENTRY_DEFAULT_IDLE_TIME);
> ofproto->mbridge = mbridge_create();
> ofproto->has_bonded_bundles = false;
> + ovs_mutex_init(&ofproto->stats_mutex);
> ovs_mutex_init(&ofproto->vsp_mutex);
>
> classifier_init(&ofproto->facets);
> @@ -1439,6 +1440,7 @@ destruct(struct ofproto *ofproto_)
> sset_destroy(&ofproto->ghost_ports);
> sset_destroy(&ofproto->port_poll_set);
>
> + ovs_mutex_destroy(&ofproto->stats_mutex);
> ovs_mutex_destroy(&ofproto->vsp_mutex);
>
> close_dpif_backer(ofproto->backer);
> @@ -1449,7 +1451,6 @@ run_fast(struct ofproto *ofproto_)
> {
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> struct ofputil_packet_in *pin, *next_pin;
> - struct ofport_dpif *ofport;
> struct list pins;
>
> /* Do not perform any periodic activity required by 'ofproto' while
> @@ -1466,10 +1467,7 @@ run_fast(struct ofproto *ofproto_)
> free(pin);
> }
>
> - HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
> - port_run_fast(ofport);
> - }
> -
> + ofproto_dpif_monitor_run_fast();
> return 0;
> }
>
> @@ -1511,6 +1509,9 @@ run(struct ofproto *ofproto_)
> dpif_ipfix_run(ofproto->ipfix);
> }
>
> + ofproto_dpif_monitor_run_fast();
> + ofproto_dpif_monitor_run();
> +
> HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
> port_run(ofport);
> }
> @@ -1555,7 +1556,6 @@ static void
> wait(struct ofproto *ofproto_)
> {
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> - struct ofport_dpif *ofport;
> struct ofbundle *bundle;
>
> if (ofproto_get_flow_restore_wait()) {
> @@ -1568,9 +1568,7 @@ wait(struct ofproto *ofproto_)
> if (ofproto->ipfix) {
> dpif_ipfix_wait(ofproto->ipfix);
> }
> - HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
> - port_wait(ofport);
> - }
> + ofproto_dpif_monitor_wait();
> HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
> bundle_wait(bundle);
> }
> @@ -1977,7 +1975,6 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
> if (ofport->bfd != old) {
> ofproto->backer->need_revalidate = REV_RECONFIGURE;
> }
> -
> return 0;
> }
>
> @@ -2863,28 +2860,6 @@ ofport_update_peer(struct ofport_dpif *ofport)
> }
>
> static void
> -port_run_fast(struct ofport_dpif *ofport)
> -{
> - if (ofport->cfm && cfm_should_send_ccm(ofport->cfm)) {
> - struct ofpbuf packet;
> -
> - ofpbuf_init(&packet, 0);
> - cfm_compose_ccm(ofport->cfm, &packet, ofport->up.pp.hw_addr);
> - ofproto_dpif_send_packet(ofport, &packet);
> - ofpbuf_uninit(&packet);
> - }
> -
> - if (ofport->bfd && bfd_should_send_packet(ofport->bfd)) {
> - struct ofpbuf packet;
> -
> - ofpbuf_init(&packet, 0);
> - bfd_put_packet(ofport->bfd, &packet, ofport->up.pp.hw_addr);
> - ofproto_dpif_send_packet(ofport, &packet);
> - ofpbuf_uninit(&packet);
> - }
> -}
> -
> -static void
> port_run(struct ofport_dpif *ofport)
> {
> long long int carrier_seq = netdev_get_carrier_resets(ofport->up.netdev);
> @@ -2895,12 +2870,9 @@ port_run(struct ofport_dpif *ofport)
>
> ofport->carrier_seq = carrier_seq;
>
> - port_run_fast(ofport);
> -
> if (ofport->cfm) {
> int cfm_opup = cfm_get_opup(ofport->cfm);
>
> - cfm_run(ofport->cfm);
> cfm_enable = !cfm_get_fault(ofport->cfm);
>
> if (cfm_opup >= 0) {
> @@ -2909,7 +2881,6 @@ port_run(struct ofport_dpif *ofport)
> }
>
> if (ofport->bfd) {
> - bfd_run(ofport->bfd);
> bfd_enable = bfd_forwarding(ofport->bfd);
> }
>
> @@ -2932,18 +2903,6 @@ port_run(struct ofport_dpif *ofport)
> ofport->may_enable = enable;
> }
>
> -static void
> -port_wait(struct ofport_dpif *ofport)
> -{
> - if (ofport->cfm) {
> - cfm_wait(ofport->cfm);
> - }
> -
> - if (ofport->bfd) {
> - bfd_wait(ofport->bfd);
> - }
> -}
> -
> static int
> port_query_by_name(const struct ofproto *ofproto_, const char *devname,
> struct ofproto_port *ofproto_port)
> @@ -3057,6 +3016,7 @@ port_get_stats(const struct ofport *ofport_, struct
> netdev_stats *stats)
> if (!error && ofport_->ofp_port == OFPP_LOCAL) {
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
>
> + ovs_mutex_lock(&ofproto->stats_mutex);
> /* ofproto->stats.tx_packets represents packets that we created
> * internally and sent to some port (e.g. packets sent with
> * send_packet()). Account for them as if they had come from
> @@ -3081,6 +3041,7 @@ port_get_stats(const struct ofport *ofport_, struct
> netdev_stats *stats)
> if (stats->tx_bytes != UINT64_MAX) {
> stats->tx_bytes += ofproto->stats.rx_bytes;
> }
> + ovs_mutex_unlock(&ofproto->stats_mutex);
> }
>
> return error;
> @@ -4951,10 +4912,11 @@ ofproto_dpif_send_packet(const struct ofport_dpif
> *ofport, struct ofpbuf *packet
> error = xlate_send_packet(ofport, packet);
>
> if (!error) {
> + ovs_mutex_lock(&ofproto->stats_mutex);
> ofproto->stats.tx_packets++;
> ofproto->stats.tx_bytes += packet->size;
> + ovs_mutex_unlock(&ofproto->stats_mutex);
> }
> -
> return error;
> }
>
> diff --git a/tests/bfd.at b/tests/bfd.at
> index 0b2b7cc..cc6755c 100644
> --- a/tests/bfd.at
> +++ b/tests/bfd.at
> @@ -260,10 +260,16 @@ OVS_VSWITCHD_START([add-br br1 -- set bridge br1
> datapath-type=dummy -- \
>
> ovs-appctl time/stop
>
> -# wait for local session state to go from down to up.
> -for i in `seq 0 1`; do ovs-appctl time/warp 500; done
> -BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none],
> [init], [No Diagnostic])
> -
> +# check which port sends the first bfd control packet.
> +if [ ovs-appctl bfd/show p0 | grep "Remote Session State: init" ]
> +then
> +# if p0 sends first, it should have gone up already.
> + BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none],
> [init], [No Diagnostic])
> +else
> +# if p1 sends first, wait 1000ms for p0 to go up.
> + BFD_CHECK([p0], [false], [false], [none], [init], [No Diagnostic],
> [none], [down], [No Diagnostic])
> + for i in `seq 0 1`; do ovs-appctl time/warp 500; done
> +fi
>
> # Test-1 BFD decay: decay to decay_min_rx
> # bfd:decay_min_rx is set to 3000ms after the local state of p0 goes up,
> @@ -450,7 +456,7 @@ done
> AT_CHECK([ovs-vsctl set Interface p0 bfd:decay_min_rx=3000 -- set interface
> p1 bfd:min_tx=5000])
> # there will be poll sequences from both sides. and it is hard to determine
> the
> # order. so just skip 10000ms and check the RX/TX. at that time, p0 should
> be in decay already.
> -for i in `seq 0 19`; do echo $i; ovs-appctl bfd/show; ovs-appctl time/warp
> 500; done
> +for i in `seq 0 19`; do ovs-appctl time/warp 500; done
> BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none],
> [up], [No Diagnostic])
> BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none],
> [up], [No Diagnostic])
> BFD_CHECK_TX([p0], [500ms], [300ms], [5000ms])
> @@ -493,15 +499,12 @@ BFD_CHECK_RX([p0], [300ms], [300ms], [1ms])
>
> # resume the bfd on p1. the bfd should not go to decay mode direclty.
> AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=true])
> -for i in `seq 0 1`; do ovs-appctl time/warp 500; done
> -BFD_CHECK([p0], [true], [false], [none], [up], [Control Detection Time
> Expired], [none], [up], [No Diagnostic])
> +for i in `seq 0 3`; do ovs-appctl time/warp 500; done
> BFD_CHECK_TX([p0], [500ms], [300ms], [500ms])
> BFD_CHECK_RX([p0], [500ms], [300ms], [500ms])
>
> -# since the decay_min_rx is still 3000ms, so after 3000ms, there should be
> the decay and poll sequence.
> +# since the decay_min_rx is still 3000ms, so after 3000ms, there should be
> the decay.
> for i in `seq 0 5`; do ovs-appctl time/warp 500; done
> -BFD_CHECK([p0], [true], [false], [none], [up], [Control Detection Time
> Expired], [final], [up], [No Diagnostic])
> -BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [poll],
> [up], [Control Detection Time Expired])
> BFD_CHECK_TX([p0], [500ms], [300ms], [500ms])
> BFD_CHECK_RX([p0], [3000ms], [3000ms], [500ms])
> # End of Test-8
> ################################################################
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
