[ovs-dev] [PATCH 3/3] dpif-netdev: batch packet processing

Daniele Di Proietto ddiproietto at vmware.com
Wed Jun 4 16:35:20 UTC 2014


On Jun 3, 2014, at 6:16 PM, Ethan Jackson <ethan at nicira.com> wrote:

> 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];
> 

Sorry about that.
dp_netdev_queue_userspace_packet needs OVS_REQUIRES(q->mutex).
Fixed

> On Tue, Jun 3, 2014 at 6:05 PM, Ethan Jackson <ethan at 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
>> <ddiproietto at 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 <ddiproietto at 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 at openvswitch.org
>>> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=%2BuUpsuGwwfi9HFr1TEJlAI1sqsI0GfY7oIfTjVrCqis%3D%0A&s=553b15fa799dd0c5f8d53692866f1ffa8ba4e64f58d65629e470b60533d5a666

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140604/3c2a296e/attachment-0005.html>


More information about the dev mailing list