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
http://openvswitch.org/mailman/listinfo/dev
