> Here's an incremental which fixes a couple of deadlocks.
Ah getting tired, this doesn't actually fix deadlocks as the mutex is
recursive. Still, I think it's better style to annotate these things,
so I've folded it in.
Ethan
>
> ---
> ofproto/ofproto-dpif-sflow.c | 41 ++++++++++++++++++++++++++---------------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index fc9a836..1176b30 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -275,10 +275,9 @@ success:
> return true;
> }
>
> -void
> -dpif_sflow_clear(struct dpif_sflow *ds)
> +static void
> +dpif_sflow_clear__(struct dpif_sflow *ds) OVS_REQ_WRLOCK(mutex)
> {
> - ovs_mutex_lock(&mutex);
> if (ds->sflow_agent) {
> sfl_agent_release(ds->sflow_agent);
> ds->sflow_agent = NULL;
> @@ -290,11 +289,18 @@ dpif_sflow_clear(struct dpif_sflow *ds)
>
> /* Turn off sampling to save CPU cycles. */
> ds->probability = 0;
> +}
> +
> +void
> +dpif_sflow_clear(struct dpif_sflow *ds) OVS_LOCKS_EXCLUDED(mutex)
> +{
> + ovs_mutex_lock(&mutex);
> + dpif_sflow_clear__(ds);
> ovs_mutex_unlock(&mutex);
> }
>
> bool
> -dpif_sflow_is_enabled(const struct dpif_sflow *ds)
> +dpif_sflow_is_enabled(const struct dpif_sflow *ds) OVS_LOCKS_EXCLUDED(mutex)
> {
> bool enabled;
>
> @@ -346,7 +352,7 @@ dpif_sflow_ref(const struct dpif_sflow *ds_)
> * a value of %UINT32_MAX samples all packets and intermediate values sample
> * intermediate fractions of packets. */
> uint32_t
> -dpif_sflow_get_probability(const struct dpif_sflow *ds)
> +dpif_sflow_get_probability(const struct dpif_sflow *ds)
> OVS_LOCKS_EXCLUDED(mutex)
> {
> uint32_t probability;
> ovs_mutex_lock(&mutex);
> @@ -356,7 +362,7 @@ dpif_sflow_get_probability(const struct dpif_sflow *ds)
> }
>
> void
> -dpif_sflow_unref(struct dpif_sflow *ds)
> +dpif_sflow_unref(struct dpif_sflow *ds) OVS_LOCKS_EXCLUDED(mutex)
> {
> int orig;
>
> @@ -381,6 +387,7 @@ dpif_sflow_unref(struct dpif_sflow *ds)
>
> static void
> dpif_sflow_add_poller(struct dpif_sflow *ds, struct dpif_sflow_port *dsp)
> + OVS_REQ_WRLOCK(mutex)
> {
> SFLPoller *poller = sfl_agent_addPoller(ds->sflow_agent, &dsp->dsi, ds,
> sflow_agent_get_counters);
> @@ -391,7 +398,7 @@ dpif_sflow_add_poller(struct dpif_sflow *ds, struct
> dpif_sflow_port *dsp)
>
> void
> dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport,
> - odp_port_t odp_port)
> + odp_port_t odp_port) OVS_LOCKS_EXCLUDED(mutex)
> {
> struct dpif_sflow_port *dsp;
> int ifindex;
> @@ -436,6 +443,7 @@ dpif_sflow_del_port__(struct dpif_sflow *ds, struct
> dpif_sflow_port *dsp)
>
> void
> dpif_sflow_del_port(struct dpif_sflow *ds, odp_port_t odp_port)
> + OVS_LOCKS_EXCLUDED(mutex)
> {
> struct dpif_sflow_port *dsp;
>
> @@ -450,6 +458,7 @@ dpif_sflow_del_port(struct dpif_sflow *ds, odp_port_t
> odp_port)
> void
> dpif_sflow_set_options(struct dpif_sflow *ds,
> const struct ofproto_sflow_options *options)
> + OVS_LOCKS_EXCLUDED(mutex)
> {
> struct dpif_sflow_port *dsp;
> bool options_changed;
> @@ -464,7 +473,7 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
> if (sset_is_empty(&options->targets) || !options->sampling_rate) {
> /* No point in doing any work if there are no targets or nothing to
> * sample. */
> - dpif_sflow_clear(ds);
> + dpif_sflow_clear__(ds);
> goto out;
> }
>
> @@ -482,7 +491,7 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
> if (ds->collectors == NULL) {
> VLOG_WARN_RL(&rl, "no collectors could be initialized, "
> "sFlow disabled");
> - dpif_sflow_clear(ds);
> + dpif_sflow_clear__(ds);
> goto out;
> }
> }
> @@ -491,7 +500,7 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
> if (!sflow_choose_agent_address(options->agent_device,
> &options->targets,
> options->control_ip, &agentIP)) {
> - dpif_sflow_clear(ds);
> + dpif_sflow_clear__(ds);
> goto out;
> }
>
> @@ -543,13 +552,14 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
> dpif_sflow_add_poller(ds, dsp);
> }
>
> +
> out:
> ovs_mutex_unlock(&mutex);
> }
>
> int
> dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *ds,
> - odp_port_t odp_port)
> + odp_port_t odp_port) OVS_LOCKS_EXCLUDED(mutex)
> {
> struct dpif_sflow_port *dsp;
> int ret;
> @@ -565,6 +575,7 @@ void
> dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
> const struct flow *flow, odp_port_t odp_in_port,
> const union user_action_cookie *cookie)
> + OVS_LOCKS_EXCLUDED(mutex)
> {
> SFL_FLOW_SAMPLE_TYPE fs;
> SFLFlow_sample_element hdrElem;
> @@ -632,10 +643,10 @@ out:
> }
>
> void
> -dpif_sflow_run(struct dpif_sflow *ds)
> +dpif_sflow_run(struct dpif_sflow *ds) OVS_LOCKS_EXCLUDED(mutex)
> {
> ovs_mutex_lock(&mutex);
> - if (dpif_sflow_is_enabled(ds)) {
> + if (ds->collectors != NULL) {
> time_t now = time_now();
> route_table_run();
> if (now >= ds->next_tick) {
> @@ -647,10 +658,10 @@ dpif_sflow_run(struct dpif_sflow *ds)
> }
>
> void
> -dpif_sflow_wait(struct dpif_sflow *ds)
> +dpif_sflow_wait(struct dpif_sflow *ds) OVS_LOCKS_EXCLUDED(mutex)
> {
> ovs_mutex_lock(&mutex);
> - if (dpif_sflow_is_enabled(ds)) {
> + if (ds->collectors != NULL) {
> poll_timer_wait_until(ds->next_tick * 1000LL);
> }
> ovs_mutex_unlock(&mutex);
> --
> 1.7.9.5
>
X-CudaMail-Whitelist-To: dev@openvswitch.org
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
