| Message ID | 1426792426-12507-5-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Zoltan, I don't know what is the code style of OVS but you need to be consistent in the style. In some places you have for (i = 0; in other for (i=0; Thanks, Maxim. On 03/19/15 22:13, Zoltan Kiss 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 <zoltan.kiss@linaro.org> > --- > 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); > + } > + > + /* 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
On Thu, Mar 19, 2015 at 9:13 PM, Zoltan Kiss <zoltan.kiss@linaro.org> 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 <zoltan.kiss@linaro.org> > --- > 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 >
I'll fix that. Unfortunately OVS doesn't have a checkpatch, and its style is slightly different from kernel (e.g. 4 spaces for indentation), which is often confusing. On 20/03/15 15:41, Maxim Uvarov wrote: > Hi Zoltan, > > I don't know what is the code style of OVS but you need to be consistent > in the style. > In some places you have > > for (i = 0; > > in other > for (i=0; > > Thanks, > Maxim. > > > On 03/19/15 22:13, Zoltan Kiss 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 <zoltan.kiss@linaro.org> >> --- >> 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); >> + } >> + >> + /* 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 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 20/03/15 16:30, Ciprian Barbu wrote: >> @@ -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. > I agree with you, I'll try to figure out something better for the first. The second is a workaround for a bug in ODP-DPDK, but I think it doesn't hurt other implementations.
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); + } + + /* 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
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 <zoltan.kiss@linaro.org> --- lib/netdev-odp.c | 67 +++++++++++++++++++++++++++++++------------------------- lib/ofpbuf.c | 1 - 2 files changed, 37 insertions(+), 31 deletions(-)