>
> @ -2697,19 +2845,28 @@ fast_path_processing(struct dp_netdev_pmd_thread
>> *pmd,
>> ? &put_actions
>> : &actions;
>>
>> - ovs_mutex_lock(&dp->flow_mutex);
>> - /* XXX: There's a brief race where this flow could have
>> already
>> - * been installed since we last did the flow lookup. This
>> could be
>> - * solved by moving the mutex lock outside the loop, but
>> that's an
>> - * awful long time to be locking everyone out of making flow
>> - * installs. If we move to a per-core classifier, it would
>> be
>> - * reasonable. */
>> - if (OVS_LIKELY(error != ENOSPC)
>> - && !dp_netdev_lookup_flow(dp, mfs[i])) {
>> - dp_netdev_flow_add(dp, &match, ofpbuf_data(add_actions),
>> - ofpbuf_size(add_actions));
>> + if (OVS_LIKELY(error != ENOSPC)) {
>> + keys[i].hash = netdev_flow_key_hash(&keys[i]);
>> +
>> + /* XXX: There's a race window where a flow covering this
>> packet
>> + * could have already been installed since we last did
>> the flow
>> + * lookup before upcall. This could be solved by moving
>> the
>> + * mutex lock outside the loop, but that's an awful long
>> time
>> + * to be locking everyone out of making flow installs.
>> If we
>> + * move to a per-core classifier, it would be
>> reasonable. */
>> + ovs_mutex_lock(&dp->flow_mutex);
>> + netdev_flow = dp_netdev_lookup_flow(dp, &keys[i]);
>> + if (OVS_LIKELY(!netdev_flow)) {
>> + netdev_flow = dp_netdev_flow_add(dp, &match,
>> +
>> ofpbuf_data(add_actions),
>> +
>> ofpbuf_size(add_actions));
>> + }
>> + ovs_mutex_unlock(&dp->flow_mutex);
>> +
>> + /* EMC uses different hash. */
>> + keys[i].hash = dpif_packet_get_dp_hash(packets[i]);
>> + emc_insert(flow_cache, &keys[i], netdev_flow);
>>
>
>
> Could we just assign the 'rules[i] = &netdev_flow->cr;' and install them
> in the
> end of function.
>
> Also seems to me that if we do not do 'rules[i] = &netdev_flow->cr;', the
> pkt
> will not be queued in 'dp_netdev_queue_batches()' at the end of function.
>
Sorry, I missed this just above the modified code:
/* We can't allow the packet batching in the next loop to
execute
* the actions. Otherwise, if there are any slow path actions,
* we'll send the packet up twice. */
dp_netdev_execute_actions(pmd, &packets[i], 1, true,
ofpbuf_data(&actions),
ofpbuf_size(&actions));
So, I'm fine with this change.
Besides, here is another issue:
the output of function 'netdev_flow_key_hash()' seems not got used,
for dpcls_lookup(), we calculate the hash inside the function,
for emc_lookup(), we use 'dpif_netdev_packet_get_dp_hash()'
for flow_table(), we use the 'flow_hash(&flow->flow, 0)'
Could you take a look?
Thanks,
Alex Wang,
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
