On Thu, Mar 19, 2015 at 9:13 PM, Zoltan Kiss <[email protected]> wrote:
> Allocating it after every packet receive gives a big performance penalty. So
> move it into the same buffer pool, right after the packet itselg. Note,
> initialization still happens for every packet, that needs to be further
> optimized.
>
> Signed-off-by: Zoltan Kiss <[email protected]>
> ---
> lib/netdev-odp.c | 67
> +++++++++++++++++++++++++++++++-------------------------
> lib/ofpbuf.c | 1 -
> 2 files changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/lib/netdev-odp.c b/lib/netdev-odp.c
> index 0b13f7f..4b13bfe 100644
> --- a/lib/netdev-odp.c
> +++ b/lib/netdev-odp.c
> @@ -58,7 +58,6 @@ VLOG_DEFINE_THIS_MODULE(odp);
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>
> static odp_buffer_pool_t pool;
> -static odp_buffer_pool_t ofpbuf_pool;
> static odp_buffer_pool_t struct_pool;
>
> static int odp_initialized = 0;
> @@ -95,7 +94,6 @@ void
> free_odp_buf(struct ofpbuf *b)
> {
> odp_packet_free(b->odp_pkt);
> - odp_buffer_free(b->odp_ofpbuf);
> }
>
> int
> @@ -130,11 +128,12 @@ static int
> odp_class_init(void)
> {
> odp_buffer_pool_param_t params;
> - int result = 0;
> + int result = 0, i;
> + odp_packet_t* pkts;
>
> - /* create packet pool */
> + /* create packet & ofpbuf pool */
>
> - params.buf_size = SHM_PKT_POOL_BUF_SIZE;
> + params.buf_size = SHM_PKT_POOL_BUF_SIZE + SHM_OFPBUF_POOL_BUF_SIZE;
> params.buf_align = ODP_CACHE_LINE_SIZE;
> params.num_bufs = SHM_PKT_POOL_NUM_BUFS;
> params.buf_type = ODP_BUFFER_TYPE_PACKET;
> @@ -147,19 +146,34 @@ odp_class_init(void)
> }
> odp_buffer_pool_print(pool);
>
> - /* create ofpbuf pool */
> -
> - params.buf_size = SHM_OFPBUF_POOL_BUF_SIZE;
> - params.num_bufs = SHM_OFPBUF_POOL_BUF_SIZE;
> - params.buf_type = ODP_BUFFER_TYPE_RAW;
> -
> - ofpbuf_pool = odp_buffer_pool_create("ofpbuf_pool", ODP_SHM_NULL,
> ¶ms);
> -
> - if (ofpbuf_pool == ODP_BUFFER_POOL_INVALID) {
> - VLOG_ERR("Error: ofpbuf pool create failed.\n");
> + /* Allocate all the packets from the pool and initialize ofpbuf part */
> + pkts = malloc(sizeof(odp_packet_t) * params.num_bufs);
> + for (i=0; i < params.num_bufs; ++i) {
> + struct dpif_packet *packet;
> + char *start;
> + pkts[i] = odp_packet_alloc(pool, 0);
> + if (pkts[i] == ODP_PACKET_INVALID) {
> + VLOG_ERR("Error: packet allocation failed.\n");
> return -1;
> - }
> - odp_buffer_pool_print(ofpbuf_pool);
> + }
> + start = (char *)odp_buffer_addr(odp_packet_to_buffer(pkts[i]));
> + /* TODO: This 256 magic value is needed for ODP-DPDK, when the
> buffer is
> + * allocated by rte_eth_rx_burst the start is at a different place.
> + * Probably due to headroom, this is a workaround here at the
> moment */
> + packet = (struct dpif_packet*) (start + SHM_PKT_POOL_BUF_SIZE - 256);
> + packet->ofpbuf.odp_pkt = pkts[i];
> + /* TODO: odp_packet_alloc reset this to ODP_PACKET_OFFSET_INVALID,
> and
> + * ODP-DPDK doesn't set it later on, which causes a crash in
> + * emc_processing. Workaround this problem here for the moment */
> + odp_packet_l2_offset_set(pkts[i], 0);
I don't agree with this way of handling the odp-dpdk differences, the
demo was run on KS2 as well, not only Intel DPDK. The only reason to
even push these changes is so that people can replicate the demo, but
we can't just ignore the KS2 part.
In odp-ovs we still have the --with-odp-platform configure option, you
could use that for example to differentiate between platforms.
> + }
> +
> + /* Free our packets up */
> + for (i=0; i < params.num_bufs; i++)
> + odp_packet_free(pkts[i]);
> + free(pkts);
> +
> + odp_buffer_pool_print(pool);
>
> /* create pool for structures */
>
> @@ -359,10 +373,8 @@ netdev_odp_send(struct netdev *netdev, int qid
> OVS_UNUSED,
> }
> }
> } else {
> - for (i = 0; i < cnt; i++) {
> + for (i = 0; i < cnt; i++)
> odp_pkts[i] = pkts[i]->ofpbuf.odp_pkt;
> - odp_packet_free(pkts[i]->ofpbuf.odp_ofpbuf);
> - }
> pkts_ok = cnt;
> }
>
> @@ -605,17 +617,12 @@ netdev_odp_rxq_recv(struct netdev_rxq *rxq_, struct
> dpif_packet **packets,
> return EINVAL;
> }
>
> - /* Allocate an ofpbuf for each valid packet */
> + /* Build the array of dpif_packet pointers */
> for (i = 0; i < pkts_ok; i++) {
> - odp_buffer_t buf;
> - buf = odp_buffer_alloc(ofpbuf_pool);
> - if (buf == ODP_BUFFER_INVALID) {
> - out_of_memory();
> - }
> - packets[i] = (struct dpif_packet*) odp_buffer_addr(buf);
> - ofpbuf_init_odp(&packets[i]->ofpbuf, odp_packet_buf_len(pkt_tbl[i]));
> - packets[i]->ofpbuf.odp_pkt = pkt_tbl[i];
> - packets[i]->ofpbuf.odp_ofpbuf = buf;
> + char *start;
> + start = (char*)odp_buffer_addr(odp_packet_to_buffer(pkt_tbl[i]));
> + packets[i] = (struct dpif_packet*) (start + SHM_PKT_POOL_BUF_SIZE);
> + ofpbuf_init_odp(&packets[i]->ofpbuf, 0);
> rx_bytes += odp_packet_len(pkt_tbl[i]);
> }
>
> diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
> index 6f27b47..06a8c1c 100644
> --- a/lib/ofpbuf.c
> +++ b/lib/ofpbuf.c
> @@ -155,7 +155,6 @@ ofpbuf_uninit(struct ofpbuf *b)
> } else if (b->source == OFPBUF_ODP) {
> #ifdef ODP_NETDEV
> odp_packet_free(b->odp_pkt);
> - odp_buffer_free(b->odp_ofpbuf);
> #else
> ovs_assert(b->source != OFPBUF_ODP);
> #endif
> --
> 1.9.1
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
