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.k...@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,
&params);
-
-    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

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to