Oh, also I'm getting these errors when compiling with clang's thread saftey analysis.
../lib/dpif-netdev.c:2115:12: error: reading variable 'head' requires locking any mutex [-Werror,-Wthread-safety-analysis] if (q->head - q->tail < MAX_QUEUE_LEN) { ^ ../lib/dpif-netdev.c:2115:22: error: reading variable 'tail' requires locking any mutex [-Werror,-Wthread-safety-analysis] if (q->head - q->tail < MAX_QUEUE_LEN) { ^ ../lib/dpif-netdev.c:2116:53: error: writing variable 'head' requires locking any mutex exclusively [-Werror,-Wthread-safety-analysis] struct dp_netdev_upcall *u = &q->upcalls[q->head++ & QUEUE_MASK]; On Tue, Jun 3, 2014 at 6:05 PM, Ethan Jackson <et...@nicira.com> wrote: > Couple of questions. > > First, this patch adds a bit of code which extends beyond 79 characters. > > dp_netdev_input() does this rather interesting stuff with two keys > that mfk switches between. This is definitely not obvious when first > looking at it. Minimally it needs a comment explaining why. That > said I don't totally understand it, why can't we ruse the same key > after each batch is executed? If we can't, why is using only two keys > sufficient? This seems a bit odd. > > Actually on that note, do we need miniflowkey at all? Could it just > be embedded at the end of batch_pkt_execute? > > After this patch, both the hash and recirc and set actions have the > wrong behavior. I'd rather they abort() the switch than silently > misbehave. Looking at this now, I think this is a rather deep issue > that we need to think through. Let's discuss it tomorrow. > > Ethan > > On Tue, Jun 3, 2014 at 5:10 PM, Daniele Di Proietto > <ddiproie...@vmware.com> wrote: >> This change in dpif-netdev allows faster packet processing for devices which >> implement batching (netdev-dpdk currently). >> >> Signed-off-by: Daniele Di Proietto <ddiproie...@vmware.com> >> --- >> lib/dpif-netdev.c | 240 >> ++++++++++++++++++++++++++++++------------- >> lib/dpif.c | 7 +- >> lib/odp-execute.c | 49 ++++++--- >> lib/odp-execute.h | 4 +- >> ofproto/ofproto-dpif-xlate.c | 2 +- >> 5 files changed, 211 insertions(+), 91 deletions(-) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index e0e5ad8..fba1d65 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -331,18 +331,18 @@ static void dp_netdev_destroy_all_queues(struct >> dp_netdev *dp) >> OVS_REQ_WRLOCK(dp->queue_rwlock); >> static int dpif_netdev_open(const struct dpif_class *, const char *name, >> bool create, struct dpif **); >> -static int dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *, >> - int queue_no, int type, >> +static int dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf >> **, >> + int cnt, int queue_no, int type, >> const struct miniflow *, >> const struct nlattr *userdata); >> static void dp_netdev_execute_actions(struct dp_netdev *dp, >> const struct miniflow *, >> - struct ofpbuf *, bool may_steal, >> + struct ofpbuf **, int cnt, bool >> may_steal, >> struct pkt_metadata *, >> const struct nlattr *actions, >> size_t actions_len); >> -static void dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf >> *packet, >> - struct pkt_metadata *); >> +static void dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf >> **packets, >> + int cnt, odp_port_t port_no); >> >> static void dp_netdev_set_pmd_threads(struct dp_netdev *, int n); >> >> @@ -1518,7 +1518,6 @@ static int >> dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) >> { >> struct dp_netdev *dp = get_dp_netdev(dpif); >> - struct pkt_metadata *md = &execute->md; >> struct { >> struct miniflow flow; >> uint32_t buf[FLOW_U32S]; >> @@ -1531,9 +1530,9 @@ dpif_netdev_execute(struct dpif *dpif, struct >> dpif_execute *execute) >> >> /* Extract flow key. */ >> miniflow_initialize(&key.flow, key.buf); >> - miniflow_extract(execute->packet, md, &key.flow); >> + miniflow_extract(execute->packet, &execute->md, &key.flow); >> >> - dp_netdev_execute_actions(dp, &key.flow, execute->packet, false, md, >> + dp_netdev_execute_actions(dp, &key.flow, &execute->packet, 1, false, >> &execute->md, >> execute->actions, execute->actions_len); >> >> return 0; >> @@ -1747,17 +1746,12 @@ dp_netdev_process_rxq_port(struct dp_netdev *dp, >> struct dp_netdev_port *port, >> struct netdev_rxq *rxq) >> { >> - struct ofpbuf *packet[NETDEV_MAX_RX_BATCH]; >> - int error, c; >> + struct ofpbuf *packets[NETDEV_MAX_RX_BATCH]; >> + int error, cnt; >> >> - error = netdev_rxq_recv(rxq, packet, &c); >> + error = netdev_rxq_recv(rxq, packets, &cnt); >> if (!error) { >> - struct pkt_metadata md = PKT_METADATA_INITIALIZER(port->port_no); >> - int i; >> - >> - for (i = 0; i < c; i++) { >> - dp_netdev_port_input(dp, packet[i], &md); >> - } >> + dp_netdev_port_input(dp, packets, cnt, port->port_no); >> } else if (error != EAGAIN && error != EOPNOTSUPP) { >> static struct vlog_rate_limit rl >> = VLOG_RATE_LIMIT_INIT(1, 5); >> @@ -1954,7 +1948,7 @@ dp_netdev_flow_stats_new_cb(void) >> >> static void >> dp_netdev_flow_used(struct dp_netdev_flow *netdev_flow, >> - const struct ofpbuf *packet, >> + int cnt, int size, >> const struct miniflow *key) >> { >> uint16_t tcp_flags = miniflow_get_tcp_flags(key); >> @@ -1966,8 +1960,8 @@ dp_netdev_flow_used(struct dp_netdev_flow *netdev_flow, >> >> ovs_mutex_lock(&bucket->mutex); >> bucket->used = MAX(now, bucket->used); >> - bucket->packet_count++; >> - bucket->byte_count += ofpbuf_size(packet); >> + bucket->packet_count += cnt; >> + bucket->byte_count += size; >> bucket->tcp_flags |= tcp_flags; >> ovs_mutex_unlock(&bucket->mutex); >> } >> @@ -1981,74 +1975,143 @@ dp_netdev_stats_new_cb(void) >> } >> >> static void >> -dp_netdev_count_packet(struct dp_netdev *dp, enum dp_stat_type type) >> +dp_netdev_count_packet(struct dp_netdev *dp, enum dp_stat_type type, int >> cnt) >> { >> struct dp_netdev_stats *bucket; >> >> bucket = ovsthread_stats_bucket_get(&dp->stats, dp_netdev_stats_new_cb); >> ovs_mutex_lock(&bucket->mutex); >> - bucket->n[type]++; >> + bucket->n[type] += cnt; >> ovs_mutex_unlock(&bucket->mutex); >> } >> >> +struct batch_pkt_execute { >> + unsigned int packet_count; >> + unsigned int byte_count; >> + >> + struct dp_netdev_flow *flow; >> + const struct miniflow *mf; >> + >> + struct ofpbuf *packets[NETDEV_MAX_RX_BATCH]; >> + struct pkt_metadata md; >> +}; >> + >> +static inline void >> +packet_batch_init(struct batch_pkt_execute *batch, struct dp_netdev_flow >> *flow, >> + struct ofpbuf *packet, >> + struct pkt_metadata *md, const struct miniflow *mf) >> +{ >> + batch->flow = flow; >> + batch->mf = mf; >> + batch->md = *md; >> + batch->packets[0] = packet; >> + >> + /* no need to update time and tcp_flags, its same in given input batch. >> */ >> + batch->packet_count = 1; >> + batch->byte_count = ofpbuf_size(packet); >> +} >> + >> +static inline void >> +packet_batch_update(struct batch_pkt_execute *batch, struct ofpbuf *packet) >> +{ >> + batch->packets[batch->packet_count++] = packet; >> + batch->byte_count += ofpbuf_size(packet); >> +} >> + >> +static inline void >> +packet_batch_execute(struct batch_pkt_execute *batch, struct dp_netdev *dp) >> +{ >> + struct dp_netdev_actions *actions; >> + struct dp_netdev_flow *flow = batch->flow; >> + >> + dp_netdev_flow_used(batch->flow, batch->packet_count, >> batch->byte_count, batch->mf); >> + >> + actions = ovsrcu_get(struct dp_netdev_actions *, &flow->actions); >> + >> + dp_netdev_execute_actions(dp, batch->mf, batch->packets, >> + batch->packet_count, true, >> + &batch->md, actions->actions, actions->size); >> + >> + dp_netdev_count_packet(dp, DP_STAT_HIT, batch->packet_count); >> +} >> + >> static void >> -dp_netdev_input(struct dp_netdev *dp, struct ofpbuf *packet, >> +dp_netdev_input(struct dp_netdev *dp, struct ofpbuf **packets, int cnt, >> struct pkt_metadata *md) >> { >> - struct dp_netdev_flow *netdev_flow; >> - struct { >> + struct batch_pkt_execute batch; >> + >> + struct miniflowkey{ >> struct miniflow flow; >> uint32_t buf[FLOW_U32S]; >> - } key; >> + } keys[2]; >> >> - if (ofpbuf_size(packet) < ETH_HEADER_LEN) { >> - ofpbuf_delete(packet); >> - return; >> + struct miniflowkey *mfk = &keys[0]; >> + >> + int i; >> + >> + batch.flow = NULL; >> + batch.mf = NULL; >> + >> + for (i = 0; i < ARRAY_SIZE(keys); i++) { >> + miniflow_initialize(&keys[i].flow, keys[i].buf); >> } >> - miniflow_initialize(&key.flow, key.buf); >> - miniflow_extract(packet, md, &key.flow); >> >> - netdev_flow = dp_netdev_lookup_flow(dp, &key.flow); >> - if (netdev_flow) { >> - struct dp_netdev_actions *actions; >> + for (i = 0; i < cnt; i++) { >> + struct dp_netdev_flow *netdev_flow; >> + >> + if (ofpbuf_size(packets[i]) < ETH_HEADER_LEN) { >> + ofpbuf_delete(packets[i]); >> + continue; >> + } >> + >> + miniflow_extract(packets[i], md, &mfk->flow); >> + >> + netdev_flow = dp_netdev_lookup_flow(dp, &mfk->flow); >> >> - dp_netdev_flow_used(netdev_flow, packet, &key.flow); >> + if (netdev_flow) { >> + if (!batch.flow) { >> + packet_batch_init(&batch, netdev_flow, packets[i], md, >> &mfk->flow); >> + mfk = (batch.mf == &keys[0].flow) ? &keys[1] : &keys[0]; >> + } else if (batch.flow == netdev_flow) { >> + packet_batch_update(&batch, packets[i]); >> + } else { >> + packet_batch_execute(&batch, dp); >> + packet_batch_init(&batch, netdev_flow, packets[i], md, >> &mfk->flow); >> + mfk = (batch.mf == &keys[0].flow) ? &keys[1] : &keys[0]; >> + } >> + } else if (dp->handler_queues) { >> + dp_netdev_count_packet(dp, DP_STAT_MISS, 1); >> + dp_netdev_output_userspace(dp, &packets[i], 1, >> + miniflow_hash_5tuple(&mfk->flow, 0) >> + % dp->n_handlers, >> + DPIF_UC_MISS, &mfk->flow, NULL); >> + ofpbuf_delete(packets[i]); >> + } >> + } >> >> - actions = dp_netdev_flow_get_actions(netdev_flow); >> - dp_netdev_execute_actions(dp, &key.flow, packet, true, md, >> - actions->actions, actions->size); >> - dp_netdev_count_packet(dp, DP_STAT_HIT); >> - } else if (dp->handler_queues) { >> - dp_netdev_count_packet(dp, DP_STAT_MISS); >> - dp_netdev_output_userspace(dp, packet, >> - miniflow_hash_5tuple(&key.flow, 0) >> - % dp->n_handlers, >> - DPIF_UC_MISS, &key.flow, NULL); >> - ofpbuf_delete(packet); >> + if (batch.flow) { >> + packet_batch_execute(&batch, dp); >> } >> } >> >> static void >> -dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf *packet, >> - struct pkt_metadata *md) >> +dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf **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, packet, md); >> + dp_netdev_input(dp, packets, cnt, &md); >> } >> >> static int >> -dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet, >> - int queue_no, int type, const struct miniflow >> *key, >> - const struct nlattr *userdata) >> +dp_netdev_queue_userspace_packet(struct dp_netdev_queue *q, >> + struct ofpbuf *packet, int type, >> + const struct miniflow *key, >> + const struct nlattr *userdata) >> { >> - struct dp_netdev_queue *q; >> - int error; >> - >> - fat_rwlock_rdlock(&dp->queue_rwlock); >> - q = &dp->handler_queues[queue_no]; >> - ovs_mutex_lock(&q->mutex); >> if (q->head - q->tail < MAX_QUEUE_LEN) { >> struct dp_netdev_upcall *u = &q->upcalls[q->head++ & QUEUE_MASK]; >> struct dpif_upcall *upcall = &u->upcall; >> @@ -2075,19 +2138,42 @@ dp_netdev_output_userspace(struct dp_netdev *dp, >> struct ofpbuf *packet, >> /* Put userdata. */ >> if (userdata) { >> upcall->userdata = ofpbuf_put(buf, userdata, >> - NLA_ALIGN(userdata->nla_len)); >> + NLA_ALIGN(userdata->nla_len)); >> } >> >> ofpbuf_set_data(&upcall->packet, >> - ofpbuf_put(buf, ofpbuf_data(packet), >> ofpbuf_size(packet))); >> + ofpbuf_put(buf, ofpbuf_data(packet), >> + ofpbuf_size(packet))); >> ofpbuf_set_size(&upcall->packet, ofpbuf_size(packet)); >> >> seq_change(q->seq); >> >> - error = 0; >> + return 0; >> } else { >> - dp_netdev_count_packet(dp, DP_STAT_LOST); >> - error = ENOBUFS; >> + return ENOBUFS; >> + } >> + >> +} >> + >> +static int >> +dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf **packets, >> int cnt, >> + int queue_no, int type, const struct miniflow >> *key, >> + const struct nlattr *userdata) >> +{ >> + struct dp_netdev_queue *q; >> + int error; >> + int i; >> + >> + fat_rwlock_rdlock(&dp->queue_rwlock); >> + q = &dp->handler_queues[queue_no]; >> + ovs_mutex_lock(&q->mutex); >> + for (i = 0; i < cnt; i++) { >> + struct ofpbuf *packet = packets[i]; >> + >> + error = dp_netdev_queue_userspace_packet(q, packet, type, key, >> userdata); >> + if (error == ENOBUFS) { >> + dp_netdev_count_packet(dp, DP_STAT_LOST, 1); >> + } >> } >> ovs_mutex_unlock(&q->mutex); >> fat_rwlock_unlock(&dp->queue_rwlock); >> @@ -2101,7 +2187,7 @@ struct dp_netdev_execute_aux { >> }; >> >> static void >> -dp_execute_cb(void *aux_, struct ofpbuf *packet, >> +dp_execute_cb(void *aux_, struct ofpbuf **packets, int cnt, >> struct pkt_metadata *md, >> const struct nlattr *a, bool may_steal) >> OVS_NO_THREAD_SAFETY_ANALYSIS >> @@ -2110,12 +2196,13 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet, >> int type = nl_attr_type(a); >> struct dp_netdev_port *p; >> uint32_t *depth = recirc_depth_get(); >> + int i; >> >> switch ((enum ovs_action_attr)type) { >> case OVS_ACTION_ATTR_OUTPUT: >> p = dp_netdev_lookup_port(aux->dp, u32_to_odp(nl_attr_get_u32(a))); >> if (p) { >> - netdev_send(p->netdev, &packet, 1, may_steal); >> + netdev_send(p->netdev, packets, cnt, may_steal); >> } >> break; >> >> @@ -2124,14 +2211,16 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet, >> >> userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA); >> >> - dp_netdev_output_userspace(aux->dp, packet, >> + dp_netdev_output_userspace(aux->dp, packets, cnt, >> miniflow_hash_5tuple(aux->key, 0) >> % aux->dp->n_handlers, >> DPIF_UC_ACTION, aux->key, >> userdata); >> >> if (may_steal) { >> - ofpbuf_delete(packet); >> + for (i = 0; i < cnt; i++) { >> + ofpbuf_delete(packets[i]); >> + } >> } >> break; >> } >> @@ -2144,6 +2233,7 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet, >> if (hash_act->hash_alg == OVS_HASH_ALG_L4) { >> /* Hash need not be symmetric, nor does it need to include >> * L2 fields. */ >> + /* TODO: hash the miniflow, but a batch could have different >> miniflows */ >> hash = miniflow_hash_5tuple(aux->key, hash_act->hash_basis); >> if (!hash) { >> hash = 1; /* 0 is not valid */ >> @@ -2155,19 +2245,25 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet, >> } >> >> md->dp_hash = hash; >> + >> break; >> } >> >> case OVS_ACTION_ATTR_RECIRC: >> if (*depth < MAX_RECIRC_DEPTH) { >> + struct ofpbuf *recirc_packets[cnt]; >> struct pkt_metadata recirc_md = *md; >> - struct ofpbuf *recirc_packet; >> >> - recirc_packet = may_steal ? packet : ofpbuf_clone(packet); >> + if (!may_steal) { >> + for (i = 0; i < cnt; i++) >> + recirc_packets[i] = ofpbuf_clone(packets[i]); >> + } >> + >> recirc_md.recirc_id = nl_attr_get_u32(a); >> >> (*depth)++; >> - dp_netdev_input(aux->dp, recirc_packet, &recirc_md); >> + dp_netdev_input(aux->dp, may_steal ? packets : recirc_packets, >> + cnt, &recirc_md); >> (*depth)--; >> >> break; >> @@ -2190,13 +2286,13 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet, >> >> static void >> dp_netdev_execute_actions(struct dp_netdev *dp, const struct miniflow *key, >> - struct ofpbuf *packet, bool may_steal, >> + struct ofpbuf **packets, int cnt, bool may_steal, >> struct pkt_metadata *md, >> const struct nlattr *actions, size_t actions_len) >> { >> struct dp_netdev_execute_aux aux = {dp, key}; >> >> - odp_execute_actions(&aux, packet, may_steal, md, >> + odp_execute_actions(&aux, packets, cnt, may_steal, md, >> actions, actions_len, dp_execute_cb); >> } >> >> diff --git a/lib/dpif.c b/lib/dpif.c >> index 84dba28..852fcd6 100644 >> --- a/lib/dpif.c >> +++ b/lib/dpif.c >> @@ -1058,13 +1058,16 @@ struct dpif_execute_helper_aux { >> /* This is called for actions that need the context of the datapath to be >> * meaningful. */ >> static void >> -dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet, >> +dpif_execute_helper_cb(void *aux_, struct ofpbuf **packets, int cnt, >> struct pkt_metadata *md, >> const struct nlattr *action, bool may_steal >> OVS_UNUSED) >> { >> struct dpif_execute_helper_aux *aux = aux_; >> struct dpif_execute execute; >> int type = nl_attr_type(action); >> + struct ofpbuf * packet = packets[0]; >> + >> + ovs_assert(cnt == 1); >> >> switch ((enum ovs_action_attr)type) { >> case OVS_ACTION_ATTR_OUTPUT: >> @@ -1103,7 +1106,7 @@ dpif_execute_with_help(struct dpif *dpif, struct >> dpif_execute *execute) >> >> COVERAGE_INC(dpif_execute_with_help); >> >> - odp_execute_actions(&aux, execute->packet, false, &execute->md, >> + odp_execute_actions(&aux, &execute->packet, 1, false, &execute->md, >> execute->actions, execute->actions_len, >> dpif_execute_helper_cb); >> return aux.error; >> diff --git a/lib/odp-execute.c b/lib/odp-execute.c >> index 12ad679..db67150 100644 >> --- a/lib/odp-execute.c >> +++ b/lib/odp-execute.c >> @@ -151,7 +151,7 @@ odp_execute_set_action(struct ofpbuf *packet, const >> struct nlattr *a, >> } >> >> static void >> -odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal, >> +odp_execute_actions__(void *dp, struct ofpbuf **packets, int cnt, bool >> steal, >> struct pkt_metadata *, >> const struct nlattr *actions, size_t actions_len, >> odp_execute_cb dp_execute_action, bool more_actions); >> @@ -186,13 +186,13 @@ odp_execute_sample(void *dp, struct ofpbuf *packet, >> bool steal, >> } >> } >> >> - odp_execute_actions__(dp, packet, steal, md, nl_attr_get(subactions), >> + odp_execute_actions__(dp, &packet, 1, steal, md, >> nl_attr_get(subactions), >> nl_attr_get_size(subactions), dp_execute_action, >> more_actions); >> } >> >> static void >> -odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal, >> +odp_execute_actions__(void *dp, struct ofpbuf **packets, int cnt, bool >> steal, >> struct pkt_metadata *md, >> const struct nlattr *actions, size_t actions_len, >> odp_execute_cb dp_execute_action, bool more_actions) >> @@ -200,6 +200,8 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, >> bool steal, >> const struct nlattr *a; >> unsigned int left; >> >> + int i; >> + >> NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) { >> int type = nl_attr_type(a); >> >> @@ -215,37 +217,52 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, >> bool steal, >> bool may_steal = steal && (!more_actions >> && left <= NLA_ALIGN(a->nla_len) >> && type != >> OVS_ACTION_ATTR_RECIRC); >> - dp_execute_action(dp, packet, md, a, may_steal); >> + dp_execute_action(dp, packets, cnt, md, a, may_steal); >> } >> break; >> >> case OVS_ACTION_ATTR_PUSH_VLAN: { >> const struct ovs_action_push_vlan *vlan = nl_attr_get(a); >> - eth_push_vlan(packet, htons(ETH_TYPE_VLAN), vlan->vlan_tci); >> + >> + for (i = 0; i < cnt; i++) { >> + eth_push_vlan(packets[i], htons(ETH_TYPE_VLAN), >> vlan->vlan_tci); >> + } >> break; >> } >> >> case OVS_ACTION_ATTR_POP_VLAN: >> - eth_pop_vlan(packet); >> + for (i = 0; i < cnt; i++) { >> + eth_pop_vlan(packets[i]); >> + } >> break; >> >> case OVS_ACTION_ATTR_PUSH_MPLS: { >> const struct ovs_action_push_mpls *mpls = nl_attr_get(a); >> - push_mpls(packet, mpls->mpls_ethertype, mpls->mpls_lse); >> + for (i = 0; i < cnt; i++) { >> + push_mpls(packets[i], mpls->mpls_ethertype, mpls->mpls_lse); >> + } >> break; >> } >> >> case OVS_ACTION_ATTR_POP_MPLS: >> - pop_mpls(packet, nl_attr_get_be16(a)); >> + for (i = 0; i < cnt; i++) { >> + pop_mpls(packets[i], nl_attr_get_be16(a)); >> + } >> break; >> >> case OVS_ACTION_ATTR_SET: >> - odp_execute_set_action(packet, nl_attr_get(a), md); >> + for (i = 0; i < cnt; i++) { >> + /* TODO: if setting metadata could do it only once */ >> + odp_execute_set_action(packets[i], nl_attr_get(a), md); >> + } >> break; >> >> case OVS_ACTION_ATTR_SAMPLE: >> - odp_execute_sample(dp, packet, steal, md, a, dp_execute_action, >> - more_actions || left > >> NLA_ALIGN(a->nla_len)); >> + for (i = 0; i < cnt; i++) { >> + odp_execute_sample(dp, packets[i], steal, md, a, >> + dp_execute_action, >> + more_actions || left > >> NLA_ALIGN(a->nla_len)); >> + } >> break; >> >> case OVS_ACTION_ATTR_UNSPEC: >> @@ -256,16 +273,20 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, >> bool steal, >> } >> >> void >> -odp_execute_actions(void *dp, struct ofpbuf *packet, bool steal, >> +odp_execute_actions(void *dp, struct ofpbuf **packets, int cnt, bool steal, >> struct pkt_metadata *md, >> const struct nlattr *actions, size_t actions_len, >> odp_execute_cb dp_execute_action) >> { >> - odp_execute_actions__(dp, packet, steal, md, actions, actions_len, >> + odp_execute_actions__(dp, packets, cnt, steal, md, actions, actions_len, >> dp_execute_action, false); >> >> if (!actions_len && steal) { >> /* Drop action. */ >> - ofpbuf_delete(packet); >> + int i = 0; >> + >> + for (i = 0; i < cnt; i++) { >> + ofpbuf_delete(packets[i]); >> + } >> } >> } >> diff --git a/lib/odp-execute.h b/lib/odp-execute.h >> index 91f0c51..006deda 100644 >> --- a/lib/odp-execute.h >> +++ b/lib/odp-execute.h >> @@ -27,7 +27,7 @@ struct nlattr; >> struct ofpbuf; >> struct pkt_metadata; >> >> -typedef void (*odp_execute_cb)(void *dp, struct ofpbuf *packet, >> +typedef void (*odp_execute_cb)(void *dp, struct ofpbuf **packets, int cnt, >> struct pkt_metadata *, >> const struct nlattr *action, bool may_steal); >> >> @@ -35,7 +35,7 @@ typedef void (*odp_execute_cb)(void *dp, struct ofpbuf >> *packet, >> * to 'dp_execute_action', if non-NULL. Currently this is called only for >> * actions OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_USERSPACE so >> * 'dp_execute_action' needs to handle only these. */ >> -void odp_execute_actions(void *dp, struct ofpbuf *packet, bool steal, >> +void odp_execute_actions(void *dp, struct ofpbuf **packets, int cnt, bool >> steal, >> struct pkt_metadata *, >> const struct nlattr *actions, size_t actions_len, >> odp_execute_cb dp_execute_action); >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >> index 068d54f..875c9e8 100644 >> --- a/ofproto/ofproto-dpif-xlate.c >> +++ b/ofproto/ofproto-dpif-xlate.c >> @@ -2655,7 +2655,7 @@ execute_controller_action(struct xlate_ctx *ctx, int >> len, >> &ctx->xout->odp_actions, >> &ctx->xout->wc); >> >> - odp_execute_actions(NULL, packet, false, &md, >> + odp_execute_actions(NULL, &packet, 1, false, &md, >> ofpbuf_data(&ctx->xout->odp_actions), >> ofpbuf_size(&ctx->xout->odp_actions), NULL); >> >> -- >> 2.0.0.rc2 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev