> > @ -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