Pravin, This patch did not work by itself, I applied the following incremental (some of which are purely cosmetically, though) to make this pass the unit tests. Also, please find some further comments below.
With these: Acked-by: Jarno Rajahalme <[email protected]> commit 502f8c94c11e1994b5f4a6e73aa865ee29642b14 Author: Jarno Rajahalme <[email protected]> Date: Thu Oct 9 10:14:09 2014 -0700 Fixups. --- lib/dpif-netdev.c | 4 +++- lib/dpif.c | 1 + lib/odp-execute.c | 5 +---- ofproto/ofproto-dpif-xlate.c | 2 -- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index ba217c5..40ed0f3 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1860,6 +1860,7 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) } packet.ofpbuf = *execute->packet; + packet.md = execute->md; pp = &packet; /* Tries finding the 'pmd'. If NULL is returned, that means @@ -1885,6 +1886,7 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) * reallocate the ofpbuf memory. We need to pass those changes to the * caller */ *execute->packet = packet.ofpbuf; + execute->md = packet.md; return 0; } @@ -2922,7 +2924,7 @@ dp_netdev_execute_actions(struct dp_netdev_pmd_thread *pmd, bool may_steal, const struct nlattr *actions, size_t actions_len) { - struct dp_netdev_execute_aux aux = {pmd}; + struct dp_netdev_execute_aux aux = { pmd }; odp_execute_actions(&aux, packets, cnt, may_steal, actions, actions_len, dp_execute_cb); diff --git a/lib/dpif.c b/lib/dpif.c index 34d8c13..d088f68 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1074,6 +1074,7 @@ dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute) * reallocate the ofpbuf memory. We need to pass those changes to the * caller */ *execute->packet = packet.ofpbuf; + execute->md = packet.md; return aux.error; } diff --git a/lib/odp-execute.c b/lib/odp-execute.c index b179cf2..230e6e1 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -441,10 +441,7 @@ odp_execute_actions(void *dp, struct dpif_packet **packets, int cnt, bool steal, uint32_t hash; for (i = 0; i < cnt; i++) { - struct ofpbuf *buf = &packets[i]->ofpbuf; - struct pkt_metadata *md = &packets[i]->md; - - flow_extract(buf, md, &flow); + flow_extract(&packets[i]->ofpbuf, &packets[i]->md, &flow); hash = flow_hash_5tuple(&flow, hash_act->hash_basis); /* We also store the hash value with each packet */ diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index cc14b31..07a1f29 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3015,7 +3015,6 @@ execute_controller_action(struct xlate_ctx *ctx, int len, { struct ofproto_packet_in *pin; struct dpif_packet *packet; - struct pkt_metadata md = PKT_METADATA_INITIALIZER(0); ctx->xout->slow |= SLOW_CONTROLLER; if (!ctx->xin->packet) { @@ -3029,7 +3028,6 @@ execute_controller_action(struct xlate_ctx *ctx, int len, &ctx->xout->wc, ctx->xbridge->masked_set_action); - packet->md = md; odp_execute_actions(NULL, &packet, 1, false, ofpbuf_data(ctx->xout->odp_actions), ofpbuf_size(ctx->xout->odp_actions), NULL); On Oct 3, 2014, at 8:23 PM, Pravin B Shelar <[email protected]> wrote: > Today dpif-netdev has single metadat for given batch, since one > batch belongs to one port, but soon packets fro single tunnel ports > can belong to different ports, so we need to have per packet metadata. > > Signed-off-by: Pravin B Shelar <[email protected]> > --- > lib/dpif-netdev.c | 68 ++++++++++++++++++++------------------------ > lib/dpif.c | 7 +++-- > lib/odp-execute.c | 28 ++++++++---------- > lib/odp-execute.h | 3 +- > lib/packet-dpif.c | 3 ++ > lib/packet-dpif.h | 1 + > ofproto/ofproto-dpif-xlate.c | 3 +- > 7 files changed, 54 insertions(+), 59 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 9c61a21..a7c9c92 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -391,12 +391,12 @@ static int dpif_netdev_open(const struct dpif_class *, > const char *name, > bool create, struct dpif **); > static void dp_netdev_execute_actions(struct dp_netdev_pmd_thread *pmd, > struct dpif_packet **, int c, > - bool may_steal, struct pkt_metadata *, > + bool may_steal, > const struct nlattr *actions, > size_t actions_len); > static void dp_netdev_input(struct dp_netdev_pmd_thread *, > - struct dpif_packet **, int cnt, > - struct pkt_metadata *); > + struct dpif_packet **, int cnt); > + > static void dp_netdev_disable_upcall(struct dp_netdev *); > static void dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, > struct dp_netdev *dp, int index, > @@ -1860,7 +1860,6 @@ dpif_netdev_execute(struct dpif *dpif, struct > dpif_execute *execute) > struct dp_netdev *dp = get_dp_netdev(dpif); > struct dp_netdev_pmd_thread *pmd; > struct dpif_packet packet, *pp; > - struct pkt_metadata *md = &execute->md; > > if (ofpbuf_size(execute->packet) < ETH_HEADER_LEN || > ofpbuf_size(execute->packet) > UINT16_MAX) { > @@ -1883,7 +1882,7 @@ dpif_netdev_execute(struct dpif *dpif, struct > dpif_execute *execute) > if (pmd->core_id == NON_PMD_CORE_ID) { > ovs_mutex_lock(&dp->non_pmd_mutex); > } > - dp_netdev_execute_actions(pmd, &pp, 1, false, md, execute->actions, > + dp_netdev_execute_actions(pmd, &pp, 1, false, execute->actions, > execute->actions_len); > if (pmd->core_id == NON_PMD_CORE_ID) { > ovs_mutex_unlock(&dp->non_pmd_mutex); > @@ -2044,10 +2043,15 @@ dp_netdev_process_rxq_port(struct > dp_netdev_pmd_thread *pmd, > > error = netdev_rxq_recv(rxq, packets, &cnt); > if (!error) { > - struct pkt_metadata md = PKT_METADATA_INITIALIZER(port->port_no); > + int i; > > *recirc_depth_get() = 0; > - dp_netdev_input(pmd, packets, cnt, &md); > + > + /* XXX: initialize md in netdev implementation. */ Please do :-) > + for (i = 0; i < cnt; i++) { > + packets[i]->md = PKT_METADATA_INITIALIZER(port->port_no); > + } > + dp_netdev_input(pmd, packets, cnt); > } else if (error != EAGAIN && error != EOPNOTSUPP) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > @@ -2493,7 +2497,6 @@ struct packet_batch { > struct dp_netdev_flow *flow; > > struct dpif_packet *packets[NETDEV_MAX_RX_BATCH]; > - struct pkt_metadata md; > }; > (snip) > diff --git a/lib/packet-dpif.c b/lib/packet-dpif.c > index 3a912e1..5b6e1dc 100644 > --- a/lib/packet-dpif.c > +++ b/lib/packet-dpif.c > @@ -27,6 +27,7 @@ dpif_packet_new_with_headroom(size_t size, size_t headroom) > > ofpbuf_init(b, size + headroom); > ofpbuf_reserve(b, headroom); > + memset(&p->md, 0, sizeof p->md); > > return p; > } > @@ -38,6 +39,7 @@ dpif_packet_clone_from_ofpbuf(const struct ofpbuf *b) > size_t headroom = ofpbuf_headroom(b); > > ofpbuf_init(&p->ofpbuf, ofpbuf_size(b) + headroom); > + memset(&p->md, 0, sizeof p->md); > ofpbuf_reserve(&p->ofpbuf, headroom); > > ofpbuf_put(&p->ofpbuf, ofpbuf_data(b), ofpbuf_size(b)); > @@ -61,6 +63,7 @@ dpif_packet_clone(struct dpif_packet *p) > struct dpif_packet *newp; > > newp = dpif_packet_clone_from_ofpbuf(&p->ofpbuf); > + memcpy(&newp->md, &p->md, sizeof p->md); > > dpif_packet_set_dp_hash(newp, dpif_packet_get_dp_hash(p)); > > diff --git a/lib/packet-dpif.h b/lib/packet-dpif.h > index 89f048e..1a5efb6 100644 > --- a/lib/packet-dpif.h > +++ b/lib/packet-dpif.h > @@ -30,6 +30,7 @@ struct dpif_packet { > #ifndef DPDK_NETDEV > uint32_t dp_hash; /* Packet hash. */ > #endif > + struct pkt_metadata md; > }; Now that the full metadata is in struct dpif_packet, we should remore the conditional dp_hash member and use the md.dp_hash instead. It would also be good to use struct dpif_packet in struct execute directly. That way we would avoid copying the ofpbuf and md around in both dpif_execute_with_help() and dpif_netdev_execute(). > > struct dpif_packet *dpif_packet_new_with_headroom(size_t size, > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 1d46456..880824d 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -3029,7 +3029,8 @@ execute_controller_action(struct xlate_ctx *ctx, int > len, > &ctx->xout->wc, > ctx->xbridge->masked_set_action); > > - odp_execute_actions(NULL, &packet, 1, false, &md, > + packet->md = md; > + odp_execute_actions(NULL, &packet, 1, false, > ofpbuf_data(ctx->xout->odp_actions), > ofpbuf_size(ctx->xout->odp_actions), NULL); > > -- > 1.9.3 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
