Thanks for the fix, Jarno
Acked-by: Daniele Di Proietto <ddiproie...@vmware.com>
On 9/9/14, 8:20 AM, "Jarno Rajahalme" <jrajaha...@nicira.com> wrote:
>Here¹s the updated description:
>
> lib/dpif-netdev: Make emc_mutex recursive.
>
> dpif_netdev_execute may be called while doing upcall processing.
> Since the context of the input port is not tracked upto this point, we
> use the shared dp->emc_cache for packet execution, where the emc_cache
> is needed for recirculation.
>
> While recursive mutexes can make thread safety analysis hard, for now
> we change emc_mutex to be recursive. Forthcoming new unit tests will
> fail with the current non-recursive mutex. Later improvements may
> remove the need for this recursion.
>
> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
>On Sep 8, 2014, at 4:06 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
>> Forgot to update the description,
>>
>> Jarno
>>
>>> On Sep 8, 2014, at 3:37 PM, Jarno Rajahalme <jrajaha...@nicira.com>
>>>wrote:
>>>
>>> dpif_netdev_execute may be called while doing upcall processing.
>>> Since the context of the input port is not tracked upto this point, we
>>> used the chared dp->emc_cache for packet execution. Execution needs
>>> to be able to pass the cache to recirculation code.
>>>
>>> Typically the pmd threads use their own emc_cache, so there is little
>>> value in using the shared emc_cache while processing recirculation
>>> during packet execution. Also, whenever the shared emc_cache was
>>> already used while doing the upcall, the emc_mutex is already held,
>>> and would be locked recursively in dpif_netdev_execute(). Rather than
>>> changing the mutex to a recursive type, it seems more proper to not
>>> use any emc_cache in dpif_netdev_execute.
>>>
>>> Forthcoming new unit tests will fail with the current lock recursion.
>>>
>>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
>>> ---
>>> lib/dpif-netdev.c | 32 ++++++++++----------------------
>>> 1 file changed, 10 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index 409c9bf..072ed5d 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -380,10 +380,9 @@ static void dp_netdev_execute_actions(struct
>>>dp_netdev *dp,
>>> struct emc_cache *flow_cache,
>>> const struct nlattr *actions,
>>> size_t actions_len);
>>> -static void dp_netdev_port_input(struct dp_netdev *dp,
>>> - struct emc_cache *flow_cache,
>>> - struct dpif_packet **packets, int
>>>cnt,
>>> - odp_port_t port_no);
>>> +static void dp_netdev_input(struct dp_netdev *, struct emc_cache *,
>>> + struct dpif_packet **, int cnt,
>>> + struct pkt_metadata *);
>>>
>>> static void dp_netdev_set_pmd_threads(struct dp_netdev *, int n);
>>> static void dp_netdev_disable_upcall(struct dp_netdev *);
>>> @@ -560,7 +559,7 @@ create_dp_netdev(const char *name, const struct
>>>dpif_class *class,
>>> return error;
>>> }
>>>
>>> - ovs_mutex_init(&dp->emc_mutex);
>>> + ovs_mutex_init_recursive(&dp->emc_mutex);
>>> emc_cache_init(&dp->flow_cache);
>>>
>>> *dpp = dp;
>>> @@ -1799,14 +1798,15 @@ dp_netdev_process_rxq_port(struct dp_netdev
>>>*dp,
>>>
>>> error = netdev_rxq_recv(rxq, packets, &cnt);
>>> if (!error) {
>>> - dp_netdev_port_input(dp, flow_cache, packets, cnt,
>>>port->port_no);
>>> + struct pkt_metadata md =
>>>PKT_METADATA_INITIALIZER(port->port_no);
>>> +
>>> + *recirc_depth_get() = 0;
>>> + dp_netdev_input(dp, flow_cache, packets, cnt, &md);
>>> } else if (error != EAGAIN && error != EOPNOTSUPP) {
>>> - static struct vlog_rate_limit rl
>>> - = VLOG_RATE_LIMIT_INIT(1, 5);
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>>>
>>> VLOG_ERR_RL(&rl, "error receiving data from %s: %s",
>>> - netdev_get_name(port->netdev),
>>> - ovs_strerror(error));
>>> + netdev_get_name(port->netdev),
>>>ovs_strerror(error));
>>> }
>>> }
>>>
>>> @@ -2404,18 +2404,6 @@ dp_netdev_input(struct dp_netdev *dp, struct
>>>emc_cache *flow_cache,
>>> }
>>> }
>>>
>>> -
>>> -static void
>>> -dp_netdev_port_input(struct dp_netdev *dp, struct emc_cache
>>>*flow_cache,
>>> - struct dpif_packet **packets, int cnt,
>>>odp_port_t port_no)
>>> -{
>>> - uint32_t *recirc_depth = recirc_depth_get();
>>> - struct pkt_metadata md = PKT_METADATA_INITIALIZER(port_no);
>>> -
>>> - *recirc_depth = 0;
>>> - dp_netdev_input(dp, flow_cache, packets, cnt, &md);
>>> -}
>>> -
>>> struct dp_netdev_execute_aux {
>>> struct dp_netdev *dp;
>>> struct emc_cache *flow_cache;
>>> --
>>> 1.7.10.4
>>>
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/
>listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2BU
>6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=g8unPLon1K%2B6qsS4Sp6oh6F9JGTL6Ri5XeffqMa
>cdro%3D%0A&s=de80161979e8dfa55c8b2a84e444daa06e427505ed38c52826e832f52cf70
>113
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
