| Message ID | 1420485556-12841-1-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Jan 5, 2015 at 1:19 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > This is only compile tested, and there are a few questions in the patch. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c > index 96126a1..571b66e 100644 > --- a/lib/netdev-odp.c > +++ b/lib/netdev-odp.c > @@ -45,7 +45,7 @@ > #include "unaligned.h" > #include "timeval.h" > #include "unixctl.h" > -#include "vlog.h" > +#include "openvswitch/vlog.h" > > VLOG_DEFINE_THIS_MODULE(odp); > > @@ -94,7 +94,7 @@ struct netdev_rxq_odp { > void > free_odp_buf(struct ofpbuf *b) > { > - odph_packet_free(b->odp_pkt); > + odp_packet_free(b->odp_pkt); > odp_buffer_free(b->odp_ofpbuf); > } > > @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) > static int > odp_class_init(void) > { > - void *pool_base; > odp_shm_t shm; > + odp_buffer_pool_param_t params; > int result = 0; > > /* create packet pool */ > shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - pool = odp_buffer_pool_create("packet_pool", pool_base, > - SHM_PKT_POOL_SIZE, > - SHM_PKT_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_PACKET); > + params.buf_size = SHM_PKT_POOL_BUF_SIZE; > + params.buf_align = ODP_CACHE_LINE_SIZE; > + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; > + params.buf_type = ODP_BUFFER_TYPE_PACKET; > + > + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); > > You really shouldn't be attempting to allocate your own SHM here. Instead, delete the odp_shm_reserve() calls and pass ODP_SHM_NULL as the second parameter to odp_buffer_pool_create() and ODP will allocate the memory for you. There is no architected way to determine how large a SHM you need to allocate to successfully do it the other way, so this provision of the API is of somewhat questionable utility. > if (pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: packet pool create failed.\n"); > @@ -159,19 +159,18 @@ odp_class_init(void) > /* create ofpbuf pool */ > shm = odp_shm_reserve("shm_ofpbuf_pool", SHM_OFPBUF_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", pool_base, > - SHM_OFPBUF_POOL_SIZE, > - SHM_OFPBUF_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_RAW); > + params.buf_size = SHM_OFPBUF_POOL_BUF_SIZE; > + params.num_bufs = SHM_OFPBUF_POOL_SIZE / SHM_OFPBUF_POOL_BUF_SIZE; > + params.buf_type = ODP_BUFFER_TYPE_RAW; > + > + ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", shm, ¶ms); > Same comment here. Let ODP allocate the memory. > > if (ofpbuf_pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: ofpbuf pool create failed.\n"); > @@ -182,19 +181,17 @@ odp_class_init(void) > /* create pool for structures */ > shm = odp_shm_reserve("shm_struct_pool", SHM_STRUCT_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - struct_pool = odp_buffer_pool_create("struct_pool", pool_base, > - SHM_STRUCT_POOL_SIZE, > - SHM_STRUCT_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_RAW); > + params.buf_size = SHM_STRUCT_POOL_BUF_SIZE; > + params.num_bufs = SHM_STRUCT_POOL_SIZE / SHM_STRUCT_POOL_BUF_SIZE; > + > + struct_pool = odp_buffer_pool_create("struct_pool", shm, ¶ms); > And again here, same comment. > > if (struct_pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: packet pool create failed.\n"); > @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) > } > > netdev->pkt_pool = pool; > - pkt = odph_packet_alloc(netdev->pkt_pool); > - if (!odph_packet_is_valid(pkt)) { > + pkt = odp_packet_alloc(netdev->pkt_pool, 0); > + if (!odp_packet_is_valid(pkt)) { > out_of_memory(); > goto out_err; > } > It's more efficient to say if (pkt == ODP_PACKET_INVALID) here for this test. > > - netdev->max_frame_len = odph_packet_buf_size(pkt); > + netdev->max_frame_len = odp_buffer_size(odp_packet_to_buffer(pkt)); > Need more context as to what you're trying to determine here. If you want to know what's the largest size packet you can receive that's ODP_CONFIG_PACKET_BUF_LEN_MAX > - odph_packet_free(pkt); > + odp_packet_free(pkt); > > ovs_mutex_init(&netdev->mutex); > > @@ -304,7 +301,7 @@ static int drop_err_pkts(odp_packet_t pkt_tbl[], > unsigned len) > pkt = pkt_tbl[i]; > > if (odp_unlikely(odp_packet_error(pkt))) { > - odph_packet_free(pkt); /* Drop */ > + odp_packet_free(pkt); /* Drop */ > pkt_cnt--; > } else if (odp_unlikely(i != j++)) { > pkt_tbl[j-1] = pkt; > @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct > dpif_packet **pkts, > dropped++; > continue; > } > - pkt = odph_packet_alloc(dev->pkt_pool); > + pkt = odp_packet_alloc(dev->pkt_pool, size); > > - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { > + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { > VLOG_WARN_RL(&rl, "Could not allocate packet"); > dropped += cnt -i; > break; > } > > - odp_packet_init(pkt); > - odp_packet_set_l2_offset(pkt, 0); > + odp_packet_reset(pkt, 0); > + odp_packet_l2_offset_set(pkt, 0); > > - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), size); > - odp_packet_parse(pkt, size, 0); > + memcpy(odp_packet_l2_ptr(pkt, NULL), > ofpbuf_data(&pkts[i]->ofpbuf), > + size); > + /* TODO: is this OK? */ > You shouldn't use memcpy on packets. Instead, to set the length and copy the packet this should be: odp_packet_push_tail(pkt, size); odp_packet_copydata_in(pkt, 0, size, ofpbuf_data(&pkts[i]->ofpbuf)); > + _odp_packet_parse(pkt); > This is an internal API and should not be called by an ODP app. It should be an ODP API but Petri hasn't approved it as such, hence it can't be used. > > odp_pkts[newcnt] = pkt; > newcnt++; > @@ -386,7 +385,7 @@ netdev_odp_send(struct netdev *netdev, struct > dpif_packet **pkts, int cnt, > } else { > for (i = 0; i < cnt; i++) { > odp_pkts[i] = pkts[i]->ofpbuf.odp_pkt; > - odph_packet_free(pkts[i]->ofpbuf.odp_ofpbuf); > + odp_packet_free(pkts[i]->ofpbuf.odp_ofpbuf); > } > pkts_ok = cnt; > } > @@ -396,7 +395,7 @@ netdev_odp_send(struct netdev *netdev, struct > dpif_packet **pkts, int cnt, > ovs_mutex_lock(&dev->mutex); > dev->stats.tx_packets += pkts_ok; > for (i = 0; i < pkts_ok; i++) { > - dev->stats.tx_bytes += odp_packet_get_len(odp_pkts[i]); > + dev->stats.tx_bytes += odp_packet_len(odp_pkts[i]); > } > ovs_mutex_unlock(&dev->mutex); > > @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, struct > dpif_packet **packets, > out_of_memory(); > } > packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); > - ofpbuf_init_odp(&packets[i]->ofpbuf, > odph_packet_buf_size(pkt_tbl[i])); > + ofpbuf_init_odp(&packets[i]->ofpbuf, > + odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); > + /* ^^^ Is > this OK?*/ > I'm not sure what you're trying to do here to say what these calls should be. Can you clarify? > packets[i]->ofpbuf.odp_pkt = pkt_tbl[i]; > packets[i]->ofpbuf.odp_ofpbuf = buf; > - rx_bytes += odp_packet_get_len(pkt_tbl[i]); > + rx_bytes += odp_packet_len(pkt_tbl[i]); > } > > *c = pkts_ok; > diff --git a/lib/netdev-odp.h b/lib/netdev-odp.h > index 9f521da..6162dd4 100644 > --- a/lib/netdev-odp.h > +++ b/lib/netdev-odp.h > @@ -9,7 +9,7 @@ > #include <odp.h> > #include <odph_eth.h> > #include <odph_ip.h> > -#include <odph_packet.h> > +#include <odp_packet.h> > > /* This function is not exported, we need another way to deal with > creating a packet from an ofpbuf */ > diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c > index d216917..764a799 100644 > --- a/lib/ofpbuf.c > +++ b/lib/ofpbuf.c > @@ -154,7 +154,7 @@ ofpbuf_uninit(struct ofpbuf *b) > #endif > } else if (b->source == OFPBUF_ODP) { > #ifdef ODP_NETDEV > - odph_packet_free(b->odp_pkt); > + odp_packet_free(b->odp_pkt); > odp_buffer_free(b->odp_ofpbuf); > #else > ovs_assert(b->source != OFPBUF_ODP); > diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h > index 1c5166f..47cee29 100644 > --- a/lib/ofpbuf.h > +++ b/lib/ofpbuf.h > @@ -429,7 +429,7 @@ static inline void * ofpbuf_data(const struct ofpbuf > *b) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) > - return odp_packet_l2(b->odp_pkt); > + return odp_packet_l2_ptr(b->odp_pkt, NULL); > #endif > > return b->data_; > @@ -439,8 +439,7 @@ static inline void ofpbuf_set_data(struct ofpbuf *b, > void *d) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_set_data\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_data\n"); > } > #endif > > @@ -451,8 +450,7 @@ static inline void * ofpbuf_base(const struct ofpbuf > *b) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_base\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_base\n"); > } > #endif > > @@ -463,8 +461,7 @@ static inline void ofpbuf_set_base(struct ofpbuf *b, > void *d) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_set_base\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_base\n\n"); > } > #endif > > @@ -475,7 +472,7 @@ static inline uint32_t ofpbuf_size(const struct ofpbuf > *b) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) > - return odp_packet_get_len(b->odp_pkt); > + return odp_packet_len(b->odp_pkt); > #endif > > return b->size_; > @@ -485,8 +482,7 @@ static inline void ofpbuf_set_size(struct ofpbuf *b, > uint32_t v) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_set_size\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_size\n"); > } > #endif > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On Mon, Jan 5, 2015 at 9:19 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > This is only compile tested, and there are a few questions in the patch. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c > index 96126a1..571b66e 100644 > --- a/lib/netdev-odp.c > +++ b/lib/netdev-odp.c > @@ -45,7 +45,7 @@ > #include "unaligned.h" > #include "timeval.h" > #include "unixctl.h" > -#include "vlog.h" > +#include "openvswitch/vlog.h" > > VLOG_DEFINE_THIS_MODULE(odp); > > @@ -94,7 +94,7 @@ struct netdev_rxq_odp { > void > free_odp_buf(struct ofpbuf *b) > { > - odph_packet_free(b->odp_pkt); > + odp_packet_free(b->odp_pkt); > odp_buffer_free(b->odp_ofpbuf); > } > > @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) > static int > odp_class_init(void) > { > - void *pool_base; > odp_shm_t shm; > + odp_buffer_pool_param_t params; > int result = 0; > > /* create packet pool */ > shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - pool = odp_buffer_pool_create("packet_pool", pool_base, > - SHM_PKT_POOL_SIZE, > - SHM_PKT_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_PACKET); > + params.buf_size = SHM_PKT_POOL_BUF_SIZE; > + params.buf_align = ODP_CACHE_LINE_SIZE; > + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; > + params.buf_type = ODP_BUFFER_TYPE_PACKET; > + > + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); > > if (pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: packet pool create failed.\n"); > @@ -159,19 +159,18 @@ odp_class_init(void) > /* create ofpbuf pool */ > shm = odp_shm_reserve("shm_ofpbuf_pool", SHM_OFPBUF_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", pool_base, > - SHM_OFPBUF_POOL_SIZE, > - SHM_OFPBUF_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_RAW); > + params.buf_size = SHM_OFPBUF_POOL_BUF_SIZE; > + params.num_bufs = SHM_OFPBUF_POOL_SIZE / SHM_OFPBUF_POOL_BUF_SIZE; > + params.buf_type = ODP_BUFFER_TYPE_RAW; > + > + ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", shm, ¶ms); > > if (ofpbuf_pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: ofpbuf pool create failed.\n"); > @@ -182,19 +181,17 @@ odp_class_init(void) > /* create pool for structures */ > shm = odp_shm_reserve("shm_struct_pool", SHM_STRUCT_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - struct_pool = odp_buffer_pool_create("struct_pool", pool_base, > - SHM_STRUCT_POOL_SIZE, > - SHM_STRUCT_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_RAW); > + params.buf_size = SHM_STRUCT_POOL_BUF_SIZE; > + params.num_bufs = SHM_STRUCT_POOL_SIZE / SHM_STRUCT_POOL_BUF_SIZE; > + > + struct_pool = odp_buffer_pool_create("struct_pool", shm, ¶ms); > > if (struct_pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: packet pool create failed.\n"); > @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) > } > > netdev->pkt_pool = pool; > - pkt = odph_packet_alloc(netdev->pkt_pool); > - if (!odph_packet_is_valid(pkt)) { > + pkt = odp_packet_alloc(netdev->pkt_pool, 0); > + if (!odp_packet_is_valid(pkt)) { > out_of_memory(); > goto out_err; > } > > - netdev->max_frame_len = odph_packet_buf_size(pkt); > + netdev->max_frame_len = odp_buffer_size(odp_packet_to_buffer(pkt)); > > - odph_packet_free(pkt); > + odp_packet_free(pkt); > > ovs_mutex_init(&netdev->mutex); > > @@ -304,7 +301,7 @@ static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len) > pkt = pkt_tbl[i]; > > if (odp_unlikely(odp_packet_error(pkt))) { > - odph_packet_free(pkt); /* Drop */ > + odp_packet_free(pkt); /* Drop */ > pkt_cnt--; > } else if (odp_unlikely(i != j++)) { > pkt_tbl[j-1] = pkt; > @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct dpif_packet **pkts, > dropped++; > continue; > } > - pkt = odph_packet_alloc(dev->pkt_pool); > + pkt = odp_packet_alloc(dev->pkt_pool, size); > > - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { > + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { > VLOG_WARN_RL(&rl, "Could not allocate packet"); > dropped += cnt -i; > break; > } > > - odp_packet_init(pkt); > - odp_packet_set_l2_offset(pkt, 0); > + odp_packet_reset(pkt, 0); > + odp_packet_l2_offset_set(pkt, 0); > > - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), size); > - odp_packet_parse(pkt, size, 0); > + memcpy(odp_packet_l2_ptr(pkt, NULL), ofpbuf_data(&pkts[i]->ofpbuf), > + size); > + /* TODO: is this OK? */ > + _odp_packet_parse(pkt); You actually don't need this call, _odp_packet_parse is called automatically by the implementation on both odp_schedule and odp_pktio_recv. Just remove it, it should be fine, > > odp_pkts[newcnt] = pkt; > newcnt++; > @@ -386,7 +385,7 @@ netdev_odp_send(struct netdev *netdev, struct dpif_packet **pkts, int cnt, > } else { > for (i = 0; i < cnt; i++) { > odp_pkts[i] = pkts[i]->ofpbuf.odp_pkt; > - odph_packet_free(pkts[i]->ofpbuf.odp_ofpbuf); > + odp_packet_free(pkts[i]->ofpbuf.odp_ofpbuf); > } > pkts_ok = cnt; > } > @@ -396,7 +395,7 @@ netdev_odp_send(struct netdev *netdev, struct dpif_packet **pkts, int cnt, > ovs_mutex_lock(&dev->mutex); > dev->stats.tx_packets += pkts_ok; > for (i = 0; i < pkts_ok; i++) { > - dev->stats.tx_bytes += odp_packet_get_len(odp_pkts[i]); > + dev->stats.tx_bytes += odp_packet_len(odp_pkts[i]); > } > ovs_mutex_unlock(&dev->mutex); > > @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, struct dpif_packet **packets, > out_of_memory(); > } > packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); > - ofpbuf_init_odp(&packets[i]->ofpbuf, odph_packet_buf_size(pkt_tbl[i])); > + ofpbuf_init_odp(&packets[i]->ofpbuf, > + odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); > + /* ^^^ Is this OK?*/ > packets[i]->ofpbuf.odp_pkt = pkt_tbl[i]; > packets[i]->ofpbuf.odp_ofpbuf = buf; > - rx_bytes += odp_packet_get_len(pkt_tbl[i]); > + rx_bytes += odp_packet_len(pkt_tbl[i]); > } > > *c = pkts_ok; > diff --git a/lib/netdev-odp.h b/lib/netdev-odp.h > index 9f521da..6162dd4 100644 > --- a/lib/netdev-odp.h > +++ b/lib/netdev-odp.h > @@ -9,7 +9,7 @@ > #include <odp.h> > #include <odph_eth.h> > #include <odph_ip.h> > -#include <odph_packet.h> > +#include <odp_packet.h> > > /* This function is not exported, we need another way to deal with > creating a packet from an ofpbuf */ > diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c > index d216917..764a799 100644 > --- a/lib/ofpbuf.c > +++ b/lib/ofpbuf.c > @@ -154,7 +154,7 @@ ofpbuf_uninit(struct ofpbuf *b) > #endif > } else if (b->source == OFPBUF_ODP) { > #ifdef ODP_NETDEV > - odph_packet_free(b->odp_pkt); > + odp_packet_free(b->odp_pkt); > odp_buffer_free(b->odp_ofpbuf); > #else > ovs_assert(b->source != OFPBUF_ODP); > diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h > index 1c5166f..47cee29 100644 > --- a/lib/ofpbuf.h > +++ b/lib/ofpbuf.h > @@ -429,7 +429,7 @@ static inline void * ofpbuf_data(const struct ofpbuf *b) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) > - return odp_packet_l2(b->odp_pkt); > + return odp_packet_l2_ptr(b->odp_pkt, NULL); > #endif > > return b->data_; > @@ -439,8 +439,7 @@ static inline void ofpbuf_set_data(struct ofpbuf *b, void *d) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_set_data\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_data\n"); > } > #endif > > @@ -451,8 +450,7 @@ static inline void * ofpbuf_base(const struct ofpbuf *b) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_base\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_base\n"); > } > #endif > > @@ -463,8 +461,7 @@ static inline void ofpbuf_set_base(struct ofpbuf *b, void *d) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_set_base\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_base\n\n"); > } > #endif > > @@ -475,7 +472,7 @@ static inline uint32_t ofpbuf_size(const struct ofpbuf *b) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) > - return odp_packet_get_len(b->odp_pkt); > + return odp_packet_len(b->odp_pkt); > #endif > > return b->size_; > @@ -485,8 +482,7 @@ static inline void ofpbuf_set_size(struct ofpbuf *b, uint32_t v) > { > #ifdef ODP_NETDEV > if (b->source == OFPBUF_ODP) { > - ODP_ERR("ODP: Invalid use of ofpbuf_set_size\n"); > - ovs_abort(0, "Invalid function call\n"); > + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_size\n"); > } > #endif > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 05/01/15 20:26, Bill Fischofer wrote: > > > On Mon, Jan 5, 2015 at 1:19 PM, Zoltan Kiss <zoltan.kiss@linaro.org > <mailto:zoltan.kiss@linaro.org>> wrote: > > This is only compile tested, and there are a few questions in the patch. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org > <mailto:zoltan.kiss@linaro.org>> > --- > diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c > index 96126a1..571b66e 100644 > --- a/lib/netdev-odp.c > +++ b/lib/netdev-odp.c > @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) > static int > odp_class_init(void) > { > - void *pool_base; > odp_shm_t shm; > + odp_buffer_pool_param_t params; > int result = 0; > > /* create packet pool */ > shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, > ODP_CACHE_LINE_SIZE, 0); > - pool_base = odp_shm_addr(shm); > > - if (odp_unlikely(pool_base == NULL)) { > + if (odp_unlikely(shm == ODP_SHM_INVALID)) { > VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); > out_of_memory(); > return -1; > } > > - pool = odp_buffer_pool_create("packet_pool", pool_base, > - SHM_PKT_POOL_SIZE, > - SHM_PKT_POOL_BUF_SIZE, > - ODP_CACHE_LINE_SIZE, > - ODP_BUFFER_TYPE_PACKET); > + params.buf_size = SHM_PKT_POOL_BUF_SIZE; > + params.buf_align = ODP_CACHE_LINE_SIZE; > + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; > + params.buf_type = ODP_BUFFER_TYPE_PACKET; > + > + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); > > You really shouldn't be attempting to allocate your own SHM here. > Instead, delete the odp_shm_reserve() calls and pass ODP_SHM_NULL as the > second parameter to odp_buffer_pool_create() and ODP will allocate the > memory for you. There is no architected way to determine how large a > SHM you need to allocate to successfully do it the other way, so this > provision of the API is of somewhat questionable utility. Ok, I'll use it that way. > > > if (struct_pool == ODP_BUFFER_POOL_INVALID) { > VLOG_ERR("Error: packet pool create failed.\n"); > @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) > } > > netdev->pkt_pool = pool; > - pkt = odph_packet_alloc(netdev->pkt_pool); > - if (!odph_packet_is_valid(pkt)) { > + pkt = odp_packet_alloc(netdev->pkt_pool, 0); > + if (!odp_packet_is_valid(pkt)) { > out_of_memory(); > goto out_err; > } > > > It's more efficient to say if (pkt == ODP_PACKET_INVALID) here for this > test. > > > - netdev->max_frame_len = odph_packet_buf_size(pkt); > + netdev->max_frame_len = odp_buffer_size(odp_packet_to_buffer(pkt)); > > > Need more context as to what you're trying to determine here. If you > want to know what's the largest size packet you can receive that's > ODP_CONFIG_PACKET_BUF_LEN_MAX This was written by Ciprian, but as far as I saw the only reason to allocate this packet is to determine the maximum packet size for this particular pool, not absolute maximum. odp_buffer_pool_info is better for this goal. > @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct > dpif_packet **pkts, > dropped++; > continue; > } > - pkt = odph_packet_alloc(dev->pkt_pool); > + pkt = odp_packet_alloc(dev->pkt_pool, size); > > - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { > + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { > VLOG_WARN_RL(&rl, "Could not allocate packet"); > dropped += cnt -i; > break; > } > > - odp_packet_init(pkt); > - odp_packet_set_l2_offset(pkt, 0); > + odp_packet_reset(pkt, 0); > + odp_packet_l2_offset_set(pkt, 0); > > - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), > size); > - odp_packet_parse(pkt, size, 0); > + memcpy(odp_packet_l2_ptr(pkt, NULL), > ofpbuf_data(&pkts[i]->ofpbuf), > + size); > + /* TODO: is this OK? */ > > > You shouldn't use memcpy on packets. Instead, to set the length and > copy the packet this should be: > > odp_packet_push_tail(pkt, size); > odp_packet_copydata_in(pkt, 0, size, > ofpbuf_data(&pkts[i]->ofpbuf)); > > + _odp_packet_parse(pkt); > > > This is an internal API and should not be called by an ODP app. It > should be an ODP API but Petri hasn't approved it as such, hence it > can't be used. If this came in on an ODP interface, then we don't need to parse it as Ciprian wrote in an another mail, because it was parsed during reception. But if it came from another kind of port (I'm not sure that's possible at the moment, but I think we will need that), we need to parse here. > > @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, > struct dpif_packet **packets, > out_of_memory(); > } > packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); > - ofpbuf_init_odp(&packets[i]->ofpbuf, > odph_packet_buf_size(pkt_tbl[i])); > + ofpbuf_init_odp(&packets[i]->ofpbuf, > + > odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); > + /* ^^^ > Is this OK?*/ > > > I'm not sure what you're trying to do here to say what these calls > should be. Can you clarify? I need to determine the allocated buffer size from a odp_packet_t. I don't know if there is a helper for that, I haven't found it, but maybe I missed it. Zoli
On Tue, Jan 6, 2015 at 12:27 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > > > On 05/01/15 20:26, Bill Fischofer wrote: > >> >> >> On Mon, Jan 5, 2015 at 1:19 PM, Zoltan Kiss <zoltan.kiss@linaro.org >> <mailto:zoltan.kiss@linaro.org>> wrote: >> >> This is only compile tested, and there are a few questions in the >> patch. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org >> <mailto:zoltan.kiss@linaro.org>> >> --- >> diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c >> index 96126a1..571b66e 100644 >> --- a/lib/netdev-odp.c >> +++ b/lib/netdev-odp.c >> > > @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) >> static int >> odp_class_init(void) >> { >> - void *pool_base; >> odp_shm_t shm; >> + odp_buffer_pool_param_t params; >> int result = 0; >> >> /* create packet pool */ >> shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, >> ODP_CACHE_LINE_SIZE, 0); >> - pool_base = odp_shm_addr(shm); >> >> - if (odp_unlikely(pool_base == NULL)) { >> + if (odp_unlikely(shm == ODP_SHM_INVALID)) { >> VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); >> out_of_memory(); >> return -1; >> } >> >> - pool = odp_buffer_pool_create("packet_pool", pool_base, >> - SHM_PKT_POOL_SIZE, >> - SHM_PKT_POOL_BUF_SIZE, >> - ODP_CACHE_LINE_SIZE, >> - ODP_BUFFER_TYPE_PACKET); >> + params.buf_size = SHM_PKT_POOL_BUF_SIZE; >> + params.buf_align = ODP_CACHE_LINE_SIZE; >> + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; >> + params.buf_type = ODP_BUFFER_TYPE_PACKET; >> + >> + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); >> >> You really shouldn't be attempting to allocate your own SHM here. >> Instead, delete the odp_shm_reserve() calls and pass ODP_SHM_NULL as the >> second parameter to odp_buffer_pool_create() and ODP will allocate the >> memory for you. There is no architected way to determine how large a >> SHM you need to allocate to successfully do it the other way, so this >> provision of the API is of somewhat questionable utility. >> > > Ok, I'll use it that way. > > >> >> if (struct_pool == ODP_BUFFER_POOL_INVALID) { >> VLOG_ERR("Error: packet pool create failed.\n"); >> @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) >> } >> >> netdev->pkt_pool = pool; >> - pkt = odph_packet_alloc(netdev->pkt_pool); >> - if (!odph_packet_is_valid(pkt)) { >> + pkt = odp_packet_alloc(netdev->pkt_pool, 0); >> + if (!odp_packet_is_valid(pkt)) { >> out_of_memory(); >> goto out_err; >> } >> >> >> It's more efficient to say if (pkt == ODP_PACKET_INVALID) here for this >> test. >> >> >> - netdev->max_frame_len = odph_packet_buf_size(pkt); >> + netdev->max_frame_len = odp_buffer_size(odp_packet_to_ >> buffer(pkt)); >> >> >> Need more context as to what you're trying to determine here. If you >> want to know what's the largest size packet you can receive that's >> ODP_CONFIG_PACKET_BUF_LEN_MAX >> > > This was written by Ciprian, but as far as I saw the only reason to > allocate this packet is to determine the maximum packet size for this > particular pool, not absolute maximum. > odp_buffer_pool_info is better for this goal. > > Yes, odp_buffer_pool_info() will return the buf_size associated with the pool, which will tell you that. Note to Petri: Please note that applications need to know the packet size associated with a buffer pool, not the packet segment size. If you want to separately specify packet segment size on the odp_buffer_pool_param_t, we still need the packet size as well. > > > @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct >> dpif_packet **pkts, >> dropped++; >> continue; >> } >> - pkt = odph_packet_alloc(dev->pkt_pool); >> + pkt = odp_packet_alloc(dev->pkt_pool, size); >> >> - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { >> + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { >> VLOG_WARN_RL(&rl, "Could not allocate packet"); >> dropped += cnt -i; >> break; >> } >> >> - odp_packet_init(pkt); >> - odp_packet_set_l2_offset(pkt, 0); >> + odp_packet_reset(pkt, 0); >> + odp_packet_l2_offset_set(pkt, 0); >> >> - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), >> size); >> - odp_packet_parse(pkt, size, 0); >> + memcpy(odp_packet_l2_ptr(pkt, NULL), >> ofpbuf_data(&pkts[i]->ofpbuf), >> + size); >> + /* TODO: is this OK? */ >> >> >> You shouldn't use memcpy on packets. Instead, to set the length and >> copy the packet this should be: >> > > > >> odp_packet_push_tail(pkt, size); >> odp_packet_copydata_in(pkt, 0, size, >> ofpbuf_data(&pkts[i]->ofpbuf)); >> >> + _odp_packet_parse(pkt); >> >> >> This is an internal API and should not be called by an ODP app. It >> should be an ODP API but Petri hasn't approved it as such, hence it >> can't be used. >> > If this came in on an ODP interface, then we don't need to parse it as > Ciprian wrote in an another mail, because it was parsed during reception. > But if it came from another kind of port (I'm not sure that's possible at > the moment, but I think we will need that), we need to parse here. OK. > > > > >> @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, >> struct dpif_packet **packets, >> out_of_memory(); >> } >> packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); >> - ofpbuf_init_odp(&packets[i]->ofpbuf, >> odph_packet_buf_size(pkt_tbl[i])); >> + ofpbuf_init_odp(&packets[i]->ofpbuf, >> + >> odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); >> + /* ^^^ >> Is this OK?*/ >> >> >> I'm not sure what you're trying to do here to say what these calls >> should be. Can you clarify? >> > I need to determine the allocated buffer size from a odp_packet_t. I don't > know if there is a helper for that, I haven't found it, but maybe I missed > it. > odp_packet_buf_len(pkt) tells you that info. > > Zoli >
On Wed, Jan 7, 2015 at 12:28 AM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Tue, Jan 6, 2015 at 12:27 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: >> >> >> >> On 05/01/15 20:26, Bill Fischofer wrote: >>> >>> >>> >>> On Mon, Jan 5, 2015 at 1:19 PM, Zoltan Kiss <zoltan.kiss@linaro.org >>> <mailto:zoltan.kiss@linaro.org>> wrote: >>> >>> This is only compile tested, and there are a few questions in the >>> patch. >>> >>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org >>> <mailto:zoltan.kiss@linaro.org>> >>> --- >>> diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c >>> index 96126a1..571b66e 100644 >>> --- a/lib/netdev-odp.c >>> +++ b/lib/netdev-odp.c >> >> >>> @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) >>> static int >>> odp_class_init(void) >>> { >>> - void *pool_base; >>> odp_shm_t shm; >>> + odp_buffer_pool_param_t params; >>> int result = 0; >>> >>> /* create packet pool */ >>> shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, >>> ODP_CACHE_LINE_SIZE, 0); >>> - pool_base = odp_shm_addr(shm); >>> >>> - if (odp_unlikely(pool_base == NULL)) { >>> + if (odp_unlikely(shm == ODP_SHM_INVALID)) { >>> VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); >>> out_of_memory(); >>> return -1; >>> } >>> >>> - pool = odp_buffer_pool_create("packet_pool", pool_base, >>> - SHM_PKT_POOL_SIZE, >>> - SHM_PKT_POOL_BUF_SIZE, >>> - ODP_CACHE_LINE_SIZE, >>> - ODP_BUFFER_TYPE_PACKET); >>> + params.buf_size = SHM_PKT_POOL_BUF_SIZE; >>> + params.buf_align = ODP_CACHE_LINE_SIZE; >>> + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; >>> + params.buf_type = ODP_BUFFER_TYPE_PACKET; >>> + >>> + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); >>> >>> You really shouldn't be attempting to allocate your own SHM here. >>> Instead, delete the odp_shm_reserve() calls and pass ODP_SHM_NULL as the >>> second parameter to odp_buffer_pool_create() and ODP will allocate the >>> memory for you. There is no architected way to determine how large a >>> SHM you need to allocate to successfully do it the other way, so this >>> provision of the API is of somewhat questionable utility. >> >> >> Ok, I'll use it that way. >> >>> >>> >>> if (struct_pool == ODP_BUFFER_POOL_INVALID) { >>> VLOG_ERR("Error: packet pool create failed.\n"); >>> @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) >>> } >>> >>> netdev->pkt_pool = pool; >>> - pkt = odph_packet_alloc(netdev->pkt_pool); >>> - if (!odph_packet_is_valid(pkt)) { >>> + pkt = odp_packet_alloc(netdev->pkt_pool, 0); >>> + if (!odp_packet_is_valid(pkt)) { >>> out_of_memory(); >>> goto out_err; >>> } >>> >>> >>> It's more efficient to say if (pkt == ODP_PACKET_INVALID) here for this >>> test. >>> >>> >>> - netdev->max_frame_len = odph_packet_buf_size(pkt); >>> + netdev->max_frame_len = >>> odp_buffer_size(odp_packet_to_buffer(pkt)); >>> >>> >>> Need more context as to what you're trying to determine here. If you >>> want to know what's the largest size packet you can receive that's >>> ODP_CONFIG_PACKET_BUF_LEN_MAX >> >> >> This was written by Ciprian, but as far as I saw the only reason to >> allocate this packet is to determine the maximum packet size for this >> particular pool, not absolute maximum. >> odp_buffer_pool_info is better for this goal. >> > > Yes, odp_buffer_pool_info() will return the buf_size associated with the > pool, which will tell you that. Note to Petri: Please note that > applications need to know the packet size associated with a buffer pool, not > the packet segment size. If you want to separately specify packet segment > size on the odp_buffer_pool_param_t, we still need the packet size as well. > >> >> >> >>> @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct >>> dpif_packet **pkts, >>> dropped++; >>> continue; >>> } >>> - pkt = odph_packet_alloc(dev->pkt_pool); >>> + pkt = odp_packet_alloc(dev->pkt_pool, size); >>> >>> - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { >>> + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { >>> VLOG_WARN_RL(&rl, "Could not allocate packet"); >>> dropped += cnt -i; >>> break; >>> } >>> >>> - odp_packet_init(pkt); >>> - odp_packet_set_l2_offset(pkt, 0); >>> + odp_packet_reset(pkt, 0); >>> + odp_packet_l2_offset_set(pkt, 0); >>> >>> - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), >>> size); >>> - odp_packet_parse(pkt, size, 0); >>> + memcpy(odp_packet_l2_ptr(pkt, NULL), >>> ofpbuf_data(&pkts[i]->ofpbuf), >>> + size); >>> + /* TODO: is this OK? */ >>> >>> >>> You shouldn't use memcpy on packets. Instead, to set the length and >>> copy the packet this should be: >> >> >> >>> >>> odp_packet_push_tail(pkt, size); >>> odp_packet_copydata_in(pkt, 0, size, >>> ofpbuf_data(&pkts[i]->ofpbuf)); >>> >>> + _odp_packet_parse(pkt); >>> >>> >>> This is an internal API and should not be called by an ODP app. It >>> should be an ODP API but Petri hasn't approved it as such, hence it >>> can't be used. >> >> If this came in on an ODP interface, then we don't need to parse it as >> Ciprian wrote in an another mail, because it was parsed during reception. >> But if it came from another kind of port (I'm not sure that's possible at >> the moment, but I think we will need that), we need to parse here. The line in question is inside clone_pkts, which is on the TX path, it's called when it's needed to transmit a packet that is not an ODP buffer (from a tap port for example). I don't remember exactly why I was parsing the packet at that point, but it doesn't seem to make sense to keep it now. The odp-generator for example doesn't parse anything on the TX side. All other things needed seem to have been covered by Bill's comments, can you send version 2 now? Also can you did the deb packages build for, you, Mike told me he had some problems with that. Regular build worked though. > > > OK. > >> >> >> >> >>> >>> @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, >>> struct dpif_packet **packets, >>> out_of_memory(); >>> } >>> packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); >>> - ofpbuf_init_odp(&packets[i]->ofpbuf, >>> odph_packet_buf_size(pkt_tbl[i])); >>> + ofpbuf_init_odp(&packets[i]->ofpbuf, >>> + >>> odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); >>> + /* ^^^ >>> Is this OK?*/ >>> >>> >>> I'm not sure what you're trying to do here to say what these calls >>> should be. Can you clarify? >> >> I need to determine the allocated buffer size from a odp_packet_t. I don't >> know if there is a helper for that, I haven't found it, but maybe I missed >> it. > > > odp_packet_buf_len(pkt) tells you that info. > >> >> >> Zoli > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On Wed, Jan 7, 2015 at 4:02 PM, Ciprian Barbu <ciprian.barbu@linaro.org> wrote: > On Wed, Jan 7, 2015 at 12:28 AM, Bill Fischofer > <bill.fischofer@linaro.org> wrote: >> >> >> On Tue, Jan 6, 2015 at 12:27 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: >>> >>> >>> >>> On 05/01/15 20:26, Bill Fischofer wrote: >>>> >>>> >>>> >>>> On Mon, Jan 5, 2015 at 1:19 PM, Zoltan Kiss <zoltan.kiss@linaro.org >>>> <mailto:zoltan.kiss@linaro.org>> wrote: >>>> >>>> This is only compile tested, and there are a few questions in the >>>> patch. >>>> >>>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org >>>> <mailto:zoltan.kiss@linaro.org>> >>>> --- >>>> diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c >>>> index 96126a1..571b66e 100644 >>>> --- a/lib/netdev-odp.c >>>> +++ b/lib/netdev-odp.c >>> >>> >>>> @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) >>>> static int >>>> odp_class_init(void) >>>> { >>>> - void *pool_base; >>>> odp_shm_t shm; >>>> + odp_buffer_pool_param_t params; >>>> int result = 0; >>>> >>>> /* create packet pool */ >>>> shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, >>>> ODP_CACHE_LINE_SIZE, 0); >>>> - pool_base = odp_shm_addr(shm); >>>> >>>> - if (odp_unlikely(pool_base == NULL)) { >>>> + if (odp_unlikely(shm == ODP_SHM_INVALID)) { >>>> VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); >>>> out_of_memory(); >>>> return -1; >>>> } >>>> >>>> - pool = odp_buffer_pool_create("packet_pool", pool_base, >>>> - SHM_PKT_POOL_SIZE, >>>> - SHM_PKT_POOL_BUF_SIZE, >>>> - ODP_CACHE_LINE_SIZE, >>>> - ODP_BUFFER_TYPE_PACKET); >>>> + params.buf_size = SHM_PKT_POOL_BUF_SIZE; >>>> + params.buf_align = ODP_CACHE_LINE_SIZE; >>>> + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; >>>> + params.buf_type = ODP_BUFFER_TYPE_PACKET; >>>> + >>>> + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); >>>> >>>> You really shouldn't be attempting to allocate your own SHM here. >>>> Instead, delete the odp_shm_reserve() calls and pass ODP_SHM_NULL as the >>>> second parameter to odp_buffer_pool_create() and ODP will allocate the >>>> memory for you. There is no architected way to determine how large a >>>> SHM you need to allocate to successfully do it the other way, so this >>>> provision of the API is of somewhat questionable utility. >>> >>> >>> Ok, I'll use it that way. >>> >>>> >>>> >>>> if (struct_pool == ODP_BUFFER_POOL_INVALID) { >>>> VLOG_ERR("Error: packet pool create failed.\n"); >>>> @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) >>>> } >>>> >>>> netdev->pkt_pool = pool; >>>> - pkt = odph_packet_alloc(netdev->pkt_pool); >>>> - if (!odph_packet_is_valid(pkt)) { >>>> + pkt = odp_packet_alloc(netdev->pkt_pool, 0); >>>> + if (!odp_packet_is_valid(pkt)) { >>>> out_of_memory(); >>>> goto out_err; >>>> } >>>> >>>> >>>> It's more efficient to say if (pkt == ODP_PACKET_INVALID) here for this >>>> test. >>>> >>>> >>>> - netdev->max_frame_len = odph_packet_buf_size(pkt); >>>> + netdev->max_frame_len = >>>> odp_buffer_size(odp_packet_to_buffer(pkt)); >>>> >>>> >>>> Need more context as to what you're trying to determine here. If you >>>> want to know what's the largest size packet you can receive that's >>>> ODP_CONFIG_PACKET_BUF_LEN_MAX >>> >>> >>> This was written by Ciprian, but as far as I saw the only reason to >>> allocate this packet is to determine the maximum packet size for this >>> particular pool, not absolute maximum. >>> odp_buffer_pool_info is better for this goal. >>> >> >> Yes, odp_buffer_pool_info() will return the buf_size associated with the >> pool, which will tell you that. Note to Petri: Please note that >> applications need to know the packet size associated with a buffer pool, not >> the packet segment size. If you want to separately specify packet segment >> size on the odp_buffer_pool_param_t, we still need the packet size as well. >> >>> >>> >>> >>>> @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct >>>> dpif_packet **pkts, >>>> dropped++; >>>> continue; >>>> } >>>> - pkt = odph_packet_alloc(dev->pkt_pool); >>>> + pkt = odp_packet_alloc(dev->pkt_pool, size); >>>> >>>> - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { >>>> + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { >>>> VLOG_WARN_RL(&rl, "Could not allocate packet"); >>>> dropped += cnt -i; >>>> break; >>>> } >>>> >>>> - odp_packet_init(pkt); >>>> - odp_packet_set_l2_offset(pkt, 0); >>>> + odp_packet_reset(pkt, 0); >>>> + odp_packet_l2_offset_set(pkt, 0); >>>> >>>> - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), >>>> size); >>>> - odp_packet_parse(pkt, size, 0); >>>> + memcpy(odp_packet_l2_ptr(pkt, NULL), >>>> ofpbuf_data(&pkts[i]->ofpbuf), >>>> + size); >>>> + /* TODO: is this OK? */ >>>> >>>> >>>> You shouldn't use memcpy on packets. Instead, to set the length and >>>> copy the packet this should be: >>> >>> >>> >>>> >>>> odp_packet_push_tail(pkt, size); >>>> odp_packet_copydata_in(pkt, 0, size, >>>> ofpbuf_data(&pkts[i]->ofpbuf)); >>>> >>>> + _odp_packet_parse(pkt); >>>> >>>> >>>> This is an internal API and should not be called by an ODP app. It >>>> should be an ODP API but Petri hasn't approved it as such, hence it >>>> can't be used. >>> >>> If this came in on an ODP interface, then we don't need to parse it as >>> Ciprian wrote in an another mail, because it was parsed during reception. >>> But if it came from another kind of port (I'm not sure that's possible at >>> the moment, but I think we will need that), we need to parse here. > > The line in question is inside clone_pkts, which is on the TX path, > it's called when it's needed to transmit a packet that is not an ODP > buffer (from a tap port for example). I don't remember exactly why I > was parsing the packet at that point, but it doesn't seem to make > sense to keep it now. The odp-generator for example doesn't parse > anything on the TX side. > > All other things needed seem to have been covered by Bill's comments, > can you send version 2 now? Also can you did the deb packages build > for, you, Mike told me he had some problems with that. Regular build > worked though. So the thing Mike had troubles with, might have been caused by configuring ovs --with-odp=<odp_dir> whereas it is supposed to be the directory where ODP was installed to. I usually configure ODP like this: ./configure --enable-shared=no --prefix=$PWD/install make make install and then point OVS to the install directory. I think I will send a patch with these instructions to make things clear. > >> >> >> OK. >> >>> >>> >>> >>> >>>> >>>> @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, >>>> struct dpif_packet **packets, >>>> out_of_memory(); >>>> } >>>> packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); >>>> - ofpbuf_init_odp(&packets[i]->ofpbuf, >>>> odph_packet_buf_size(pkt_tbl[i])); >>>> + ofpbuf_init_odp(&packets[i]->ofpbuf, >>>> + >>>> odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); >>>> + /* ^^^ >>>> Is this OK?*/ >>>> >>>> >>>> I'm not sure what you're trying to do here to say what these calls >>>> should be. Can you clarify? >>> >>> I need to determine the allocated buffer size from a odp_packet_t. I don't >>> know if there is a helper for that, I haven't found it, but maybe I missed >>> it. >> >> >> odp_packet_buf_len(pkt) tells you that info. >> >>> >>> >>> Zoli >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >>
On 07/01/15 14:02, Ciprian Barbu wrote: >>>> @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct >>>> dpif_packet **pkts, >>>> dropped++; >>>> continue; >>>> } >>>> - pkt = odph_packet_alloc(dev->pkt_pool); >>>> + pkt = odp_packet_alloc(dev->pkt_pool, size); >>>> >>>> - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { >>>> + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { >>>> VLOG_WARN_RL(&rl, "Could not allocate packet"); >>>> dropped += cnt -i; >>>> break; >>>> } >>>> >>>> - odp_packet_init(pkt); >>>> - odp_packet_set_l2_offset(pkt, 0); >>>> + odp_packet_reset(pkt, 0); >>>> + odp_packet_l2_offset_set(pkt, 0); >>>> >>>> - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), >>>> size); >>>> - odp_packet_parse(pkt, size, 0); >>>> + memcpy(odp_packet_l2_ptr(pkt, NULL), >>>> ofpbuf_data(&pkts[i]->ofpbuf), >>>> + size); >>>> + /* TODO: is this OK? */ >>>> >>>> >>>> You shouldn't use memcpy on packets. Instead, to set the length and >>>> copy the packet this should be: >>> >>> >>> >>>> >>>> odp_packet_push_tail(pkt, size); >>>> odp_packet_copydata_in(pkt, 0, size, >>>> ofpbuf_data(&pkts[i]->ofpbuf)); >>>> >>>> + _odp_packet_parse(pkt); >>>> >>>> >>>> This is an internal API and should not be called by an ODP app. It >>>> should be an ODP API but Petri hasn't approved it as such, hence it >>>> can't be used. >>> >>> If this came in on an ODP interface, then we don't need to parse it as >>> Ciprian wrote in an another mail, because it was parsed during reception. >>> But if it came from another kind of port (I'm not sure that's possible at >>> the moment, but I think we will need that), we need to parse here. > > The line in question is inside clone_pkts, which is on the TX path, > it's called when it's needed to transmit a packet that is not an ODP > buffer (from a tap port for example). I don't remember exactly why I > was parsing the packet at that point, but it doesn't seem to make > sense to keep it now. The odp-generator for example doesn't parse > anything on the TX side. > > All other things needed seem to have been covered by Bill's comments, > can you send version 2 now? Also can you did the deb packages build > for, you, Mike told me he had some problems with that. Regular build > worked though. If odp_pktio_send doesn't need those values set by _odp_packet_parse, then it's not necessary indeed. As far as I can see it doesn't use it, Bill, can you confirm it? Zoli
The current odp_pktio_send() uses odp_packet_l2_ptr() which requires that the packet be parsed to have that set. There are a couple of ways around that. I can submit a patch to make that call work whether or not _odp_packet_parse() has been called. On Wed, Jan 7, 2015 at 12:01 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > > > On 07/01/15 14:02, Ciprian Barbu wrote: > >> @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct >>>>> dpif_packet **pkts, >>>>> dropped++; >>>>> continue; >>>>> } >>>>> - pkt = odph_packet_alloc(dev->pkt_pool); >>>>> + pkt = odp_packet_alloc(dev->pkt_pool, size); >>>>> >>>>> - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { >>>>> + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { >>>>> VLOG_WARN_RL(&rl, "Could not allocate packet"); >>>>> dropped += cnt -i; >>>>> break; >>>>> } >>>>> >>>>> - odp_packet_init(pkt); >>>>> - odp_packet_set_l2_offset(pkt, 0); >>>>> + odp_packet_reset(pkt, 0); >>>>> + odp_packet_l2_offset_set(pkt, 0); >>>>> >>>>> - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), >>>>> size); >>>>> - odp_packet_parse(pkt, size, 0); >>>>> + memcpy(odp_packet_l2_ptr(pkt, NULL), >>>>> ofpbuf_data(&pkts[i]->ofpbuf), >>>>> + size); >>>>> + /* TODO: is this OK? */ >>>>> >>>>> >>>>> You shouldn't use memcpy on packets. Instead, to set the length and >>>>> copy the packet this should be: >>>>> >>>> >>>> >>>> >>>> >>>>> odp_packet_push_tail(pkt, size); >>>>> odp_packet_copydata_in(pkt, 0, size, >>>>> ofpbuf_data(&pkts[i]->ofpbuf)); >>>>> >>>>> + _odp_packet_parse(pkt); >>>>> >>>>> >>>>> This is an internal API and should not be called by an ODP app. It >>>>> should be an ODP API but Petri hasn't approved it as such, hence it >>>>> can't be used. >>>>> >>>> >>>> If this came in on an ODP interface, then we don't need to parse it as >>>> Ciprian wrote in an another mail, because it was parsed during >>>> reception. >>>> But if it came from another kind of port (I'm not sure that's possible >>>> at >>>> the moment, but I think we will need that), we need to parse here. >>>> >>> >> The line in question is inside clone_pkts, which is on the TX path, >> it's called when it's needed to transmit a packet that is not an ODP >> buffer (from a tap port for example). I don't remember exactly why I >> was parsing the packet at that point, but it doesn't seem to make >> sense to keep it now. The odp-generator for example doesn't parse >> anything on the TX side. >> >> All other things needed seem to have been covered by Bill's comments, >> can you send version 2 now? Also can you did the deb packages build >> for, you, Mike told me he had some problems with that. Regular build >> worked though. >> > > > If odp_pktio_send doesn't need those values set by _odp_packet_parse, then > it's not necessary indeed. As far as I can see it doesn't use it, Bill, can > you confirm it? > > Zoli >
This code already took care of that by calling odp_packet_l2_offset_set. On 07/01/15 18:13, Bill Fischofer wrote: > The current odp_pktio_send() uses odp_packet_l2_ptr() which requires > that the packet be parsed to have that set. There are a couple of ways > around that. I can submit a patch to make that call work whether or not > _odp_packet_parse() has been called. > > On Wed, Jan 7, 2015 at 12:01 PM, Zoltan Kiss <zoltan.kiss@linaro.org > <mailto:zoltan.kiss@linaro.org>> wrote: > > > > On 07/01/15 14:02, Ciprian Barbu wrote: > > @@ -334,19 +331,21 @@ clone_pkts(struct > netdev_odp *dev, struct > dpif_packet **pkts, > dropped++; > continue; > } > - pkt = odph_packet_alloc(dev->pkt___pool); > + pkt = > odp_packet_alloc(dev->pkt___pool, size); > > - if > (OVS_UNLIKELY(!odph_packet_is___valid(pkt))) { > + if (OVS_UNLIKELY(pkt == > ODP_PACKET_INVALID)) { > VLOG_WARN_RL(&rl, "Could not > allocate packet"); > dropped += cnt -i; > break; > } > > - odp_packet_init(pkt); > - odp_packet_set_l2_offset(pkt, 0); > + odp_packet_reset(pkt, 0); > + odp_packet_l2_offset_set(pkt, 0); > > - memcpy(odp_packet_l2(pkt), > ofpbuf_data(&pkts[i]->ofpbuf), > size); > - odp_packet_parse(pkt, size, 0); > + memcpy(odp_packet_l2_ptr(pkt, NULL), > ofpbuf_data(&pkts[i]->ofpbuf), > + size); > + /* TODO: is this OK? */ > > > You shouldn't use memcpy on packets. Instead, to > set the length and > copy the packet this should be: > > > > > > odp_packet_push_tail(pkt, size); > odp_packet_copydata_in(pkt, 0, size, > ofpbuf_data(&pkts[i]->ofpbuf))__; > > + _odp_packet_parse(pkt); > > > This is an internal API and should not be called by > an ODP app. It > should be an ODP API but Petri hasn't approved it as > such, hence it > can't be used. > > > If this came in on an ODP interface, then we don't need > to parse it as > Ciprian wrote in an another mail, because it was parsed > during reception. > But if it came from another kind of port (I'm not sure > that's possible at > the moment, but I think we will need that), we need to > parse here. > > > The line in question is inside clone_pkts, which is on the TX path, > it's called when it's needed to transmit a packet that is not an ODP > buffer (from a tap port for example). I don't remember exactly why I > was parsing the packet at that point, but it doesn't seem to make > sense to keep it now. The odp-generator for example doesn't parse > anything on the TX side. > > All other things needed seem to have been covered by Bill's > comments, > can you send version 2 now? Also can you did the deb packages build > for, you, Mike told me he had some problems with that. Regular build > worked though. > > > > If odp_pktio_send doesn't need those values set by > _odp_packet_parse, then it's not necessary indeed. As far as I can > see it doesn't use it, Bill, can you confirm it? > > Zoli > >
diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c index 96126a1..571b66e 100644 --- a/lib/netdev-odp.c +++ b/lib/netdev-odp.c @@ -45,7 +45,7 @@ #include "unaligned.h" #include "timeval.h" #include "unixctl.h" -#include "vlog.h" +#include "openvswitch/vlog.h" VLOG_DEFINE_THIS_MODULE(odp); @@ -94,7 +94,7 @@ struct netdev_rxq_odp { void free_odp_buf(struct ofpbuf *b) { - odph_packet_free(b->odp_pkt); + odp_packet_free(b->odp_pkt); odp_buffer_free(b->odp_ofpbuf); } @@ -129,26 +129,26 @@ odp_init(int argc, char *argv[]) static int odp_class_init(void) { - void *pool_base; odp_shm_t shm; + odp_buffer_pool_param_t params; int result = 0; /* create packet pool */ shm = odp_shm_reserve("shm_packet_pool", SHM_PKT_POOL_SIZE, ODP_CACHE_LINE_SIZE, 0); - pool_base = odp_shm_addr(shm); - if (odp_unlikely(pool_base == NULL)) { + if (odp_unlikely(shm == ODP_SHM_INVALID)) { VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); out_of_memory(); return -1; } - pool = odp_buffer_pool_create("packet_pool", pool_base, - SHM_PKT_POOL_SIZE, - SHM_PKT_POOL_BUF_SIZE, - ODP_CACHE_LINE_SIZE, - ODP_BUFFER_TYPE_PACKET); + params.buf_size = SHM_PKT_POOL_BUF_SIZE; + params.buf_align = ODP_CACHE_LINE_SIZE; + params.num_bufs = SHM_PKT_POOL_SIZE / SHM_PKT_POOL_BUF_SIZE; + params.buf_type = ODP_BUFFER_TYPE_PACKET; + + pool = odp_buffer_pool_create("packet_pool", shm, ¶ms); if (pool == ODP_BUFFER_POOL_INVALID) { VLOG_ERR("Error: packet pool create failed.\n"); @@ -159,19 +159,18 @@ odp_class_init(void) /* create ofpbuf pool */ shm = odp_shm_reserve("shm_ofpbuf_pool", SHM_OFPBUF_POOL_SIZE, ODP_CACHE_LINE_SIZE, 0); - pool_base = odp_shm_addr(shm); - if (odp_unlikely(pool_base == NULL)) { + if (odp_unlikely(shm == ODP_SHM_INVALID)) { VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); out_of_memory(); return -1; } - ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", pool_base, - SHM_OFPBUF_POOL_SIZE, - SHM_OFPBUF_POOL_BUF_SIZE, - ODP_CACHE_LINE_SIZE, - ODP_BUFFER_TYPE_RAW); + params.buf_size = SHM_OFPBUF_POOL_BUF_SIZE; + params.num_bufs = SHM_OFPBUF_POOL_SIZE / SHM_OFPBUF_POOL_BUF_SIZE; + params.buf_type = ODP_BUFFER_TYPE_RAW; + + ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", shm, ¶ms); if (ofpbuf_pool == ODP_BUFFER_POOL_INVALID) { VLOG_ERR("Error: ofpbuf pool create failed.\n"); @@ -182,19 +181,17 @@ odp_class_init(void) /* create pool for structures */ shm = odp_shm_reserve("shm_struct_pool", SHM_STRUCT_POOL_SIZE, ODP_CACHE_LINE_SIZE, 0); - pool_base = odp_shm_addr(shm); - if (odp_unlikely(pool_base == NULL)) { + if (odp_unlikely(shm == ODP_SHM_INVALID)) { VLOG_ERR("Error: ODP packet pool mem alloc failed\n"); out_of_memory(); return -1; } - struct_pool = odp_buffer_pool_create("struct_pool", pool_base, - SHM_STRUCT_POOL_SIZE, - SHM_STRUCT_POOL_BUF_SIZE, - ODP_CACHE_LINE_SIZE, - ODP_BUFFER_TYPE_RAW); + params.buf_size = SHM_STRUCT_POOL_BUF_SIZE; + params.num_bufs = SHM_STRUCT_POOL_SIZE / SHM_STRUCT_POOL_BUF_SIZE; + + struct_pool = odp_buffer_pool_create("struct_pool", shm, ¶ms); if (struct_pool == ODP_BUFFER_POOL_INVALID) { VLOG_ERR("Error: packet pool create failed.\n"); @@ -247,15 +244,15 @@ netdev_odp_construct(struct netdev *netdev_) } netdev->pkt_pool = pool; - pkt = odph_packet_alloc(netdev->pkt_pool); - if (!odph_packet_is_valid(pkt)) { + pkt = odp_packet_alloc(netdev->pkt_pool, 0); + if (!odp_packet_is_valid(pkt)) { out_of_memory(); goto out_err; } - netdev->max_frame_len = odph_packet_buf_size(pkt); + netdev->max_frame_len = odp_buffer_size(odp_packet_to_buffer(pkt)); - odph_packet_free(pkt); + odp_packet_free(pkt); ovs_mutex_init(&netdev->mutex); @@ -304,7 +301,7 @@ static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len) pkt = pkt_tbl[i]; if (odp_unlikely(odp_packet_error(pkt))) { - odph_packet_free(pkt); /* Drop */ + odp_packet_free(pkt); /* Drop */ pkt_cnt--; } else if (odp_unlikely(i != j++)) { pkt_tbl[j-1] = pkt; @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct dpif_packet **pkts, dropped++; continue; } - pkt = odph_packet_alloc(dev->pkt_pool); + pkt = odp_packet_alloc(dev->pkt_pool, size); - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) { + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) { VLOG_WARN_RL(&rl, "Could not allocate packet"); dropped += cnt -i; break; } - odp_packet_init(pkt); - odp_packet_set_l2_offset(pkt, 0); + odp_packet_reset(pkt, 0); + odp_packet_l2_offset_set(pkt, 0); - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf), size); - odp_packet_parse(pkt, size, 0); + memcpy(odp_packet_l2_ptr(pkt, NULL), ofpbuf_data(&pkts[i]->ofpbuf), + size); + /* TODO: is this OK? */ + _odp_packet_parse(pkt); odp_pkts[newcnt] = pkt; newcnt++; @@ -386,7 +385,7 @@ netdev_odp_send(struct netdev *netdev, struct dpif_packet **pkts, int cnt, } else { for (i = 0; i < cnt; i++) { odp_pkts[i] = pkts[i]->ofpbuf.odp_pkt; - odph_packet_free(pkts[i]->ofpbuf.odp_ofpbuf); + odp_packet_free(pkts[i]->ofpbuf.odp_ofpbuf); } pkts_ok = cnt; } @@ -396,7 +395,7 @@ netdev_odp_send(struct netdev *netdev, struct dpif_packet **pkts, int cnt, ovs_mutex_lock(&dev->mutex); dev->stats.tx_packets += pkts_ok; for (i = 0; i < pkts_ok; i++) { - dev->stats.tx_bytes += odp_packet_get_len(odp_pkts[i]); + dev->stats.tx_bytes += odp_packet_len(odp_pkts[i]); } ovs_mutex_unlock(&dev->mutex); @@ -649,10 +648,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, struct dpif_packet **packets, out_of_memory(); } packets[i] = (struct dpif_packet*) odp_buffer_addr(buf); - ofpbuf_init_odp(&packets[i]->ofpbuf, odph_packet_buf_size(pkt_tbl[i])); + ofpbuf_init_odp(&packets[i]->ofpbuf, + odp_buffer_size(odp_packet_to_buffer(pkt_tbl[i]))); + /* ^^^ Is this OK?*/ packets[i]->ofpbuf.odp_pkt = pkt_tbl[i]; packets[i]->ofpbuf.odp_ofpbuf = buf; - rx_bytes += odp_packet_get_len(pkt_tbl[i]); + rx_bytes += odp_packet_len(pkt_tbl[i]); } *c = pkts_ok; diff --git a/lib/netdev-odp.h b/lib/netdev-odp.h index 9f521da..6162dd4 100644 --- a/lib/netdev-odp.h +++ b/lib/netdev-odp.h @@ -9,7 +9,7 @@ #include <odp.h> #include <odph_eth.h> #include <odph_ip.h> -#include <odph_packet.h> +#include <odp_packet.h> /* This function is not exported, we need another way to deal with creating a packet from an ofpbuf */ diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c index d216917..764a799 100644 --- a/lib/ofpbuf.c +++ b/lib/ofpbuf.c @@ -154,7 +154,7 @@ ofpbuf_uninit(struct ofpbuf *b) #endif } else if (b->source == OFPBUF_ODP) { #ifdef ODP_NETDEV - odph_packet_free(b->odp_pkt); + odp_packet_free(b->odp_pkt); odp_buffer_free(b->odp_ofpbuf); #else ovs_assert(b->source != OFPBUF_ODP); diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h index 1c5166f..47cee29 100644 --- a/lib/ofpbuf.h +++ b/lib/ofpbuf.h @@ -429,7 +429,7 @@ static inline void * ofpbuf_data(const struct ofpbuf *b) { #ifdef ODP_NETDEV if (b->source == OFPBUF_ODP) - return odp_packet_l2(b->odp_pkt); + return odp_packet_l2_ptr(b->odp_pkt, NULL); #endif return b->data_; @@ -439,8 +439,7 @@ static inline void ofpbuf_set_data(struct ofpbuf *b, void *d) { #ifdef ODP_NETDEV if (b->source == OFPBUF_ODP) { - ODP_ERR("ODP: Invalid use of ofpbuf_set_data\n"); - ovs_abort(0, "Invalid function call\n"); + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_data\n"); } #endif @@ -451,8 +450,7 @@ static inline void * ofpbuf_base(const struct ofpbuf *b) { #ifdef ODP_NETDEV if (b->source == OFPBUF_ODP) { - ODP_ERR("ODP: Invalid use of ofpbuf_base\n"); - ovs_abort(0, "Invalid function call\n"); + ovs_abort(0, "ODP: Invalid use of ofpbuf_base\n"); } #endif @@ -463,8 +461,7 @@ static inline void ofpbuf_set_base(struct ofpbuf *b, void *d) { #ifdef ODP_NETDEV if (b->source == OFPBUF_ODP) { - ODP_ERR("ODP: Invalid use of ofpbuf_set_base\n"); - ovs_abort(0, "Invalid function call\n"); + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_base\n\n"); } #endif @@ -475,7 +472,7 @@ static inline uint32_t ofpbuf_size(const struct ofpbuf *b) { #ifdef ODP_NETDEV if (b->source == OFPBUF_ODP) - return odp_packet_get_len(b->odp_pkt); + return odp_packet_len(b->odp_pkt); #endif return b->size_; @@ -485,8 +482,7 @@ static inline void ofpbuf_set_size(struct ofpbuf *b, uint32_t v) { #ifdef ODP_NETDEV if (b->source == OFPBUF_ODP) { - ODP_ERR("ODP: Invalid use of ofpbuf_set_size\n"); - ovs_abort(0, "Invalid function call\n"); + ovs_abort(0, "ODP: Invalid use of ofpbuf_set_size\n"); } #endif
This is only compile tested, and there are a few questions in the patch. Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> ---