| Message ID | 1433863353-22658-1-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | New |
| Headers | show |
Thanks for spotting that. I hadn't realized that odp_crypto was making those changes, even though it notes that it isn't needed for the API. That looks to be a somewhat ugly solution and I'd like to take a look to see if there isn't a cleaner way to deal with this other than this sort of hack. On Tue, Jun 9, 2015 at 10:22 AM, Christophe Milard < christophe.milard@linaro.org> wrote: > There seems to be an issue with the handling of completion event buffers: > These are drawn from the same pool as the packet event buffers, > but are re-typed as completion events. > (see platform/linux-generic/odp_crypto.c:428) > However, when these are freed and returned to the pool, > the buffer type was not restored to packet event buffers: > The code that allocates packet event buffers from the pool did not update > the buffer type either, relying on all buffers in the pool already being > initialized to be packet event buffers > > This patch resets the buffer type at release time. > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > --- > platform/linux-generic/odp_pool.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/platform/linux-generic/odp_pool.c > b/platform/linux-generic/odp_pool.c > index 35e79a0..6e65926 100644 > --- a/platform/linux-generic/odp_pool.c > +++ b/platform/linux-generic/odp_pool.c > @@ -560,6 +560,9 @@ void odp_buffer_free(odp_buffer_t buf) > odp_buffer_hdr_t *buf_hdr = odp_buf_to_hdr(buf); > pool_entry_t *pool = odp_buf_to_pool(buf_hdr); > > + /* Reset packet type in case it was "casted" (e.g. crypto > completions)*/ > + buf_hdr->type = pool->s.params.type; > + > if (odp_unlikely(pool->s.low_wm_assert)) > ret_buf(&pool->s, buf_hdr); > else > -- > 1.9.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
I posted patch http://patches.opendataplane.org/patch/1822/ as an alternative to this one. The change of the underlying buffer type seems wrong. If an implementation wished to define a new buffer type it should be managed as a separate pool type, but in this case I don't believe that's warranted, so it's best to keep these completion events as "virtual buffer types" similar to the generic 'event' type itself. On Tue, Jun 9, 2015 at 9:37 PM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Thanks for spotting that. I hadn't realized that odp_crypto was making > those changes, even though it notes that it isn't needed for the API. That > looks to be a somewhat ugly solution and I'd like to take a look to see if > there isn't a cleaner way to deal with this other than this sort of hack. > > On Tue, Jun 9, 2015 at 10:22 AM, Christophe Milard < > christophe.milard@linaro.org> wrote: > >> There seems to be an issue with the handling of completion event buffers: >> These are drawn from the same pool as the packet event buffers, >> but are re-typed as completion events. >> (see platform/linux-generic/odp_crypto.c:428) >> However, when these are freed and returned to the pool, >> the buffer type was not restored to packet event buffers: >> The code that allocates packet event buffers from the pool did not update >> the buffer type either, relying on all buffers in the pool already being >> initialized to be packet event buffers >> >> This patch resets the buffer type at release time. >> >> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >> --- >> platform/linux-generic/odp_pool.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/platform/linux-generic/odp_pool.c >> b/platform/linux-generic/odp_pool.c >> index 35e79a0..6e65926 100644 >> --- a/platform/linux-generic/odp_pool.c >> +++ b/platform/linux-generic/odp_pool.c >> @@ -560,6 +560,9 @@ void odp_buffer_free(odp_buffer_t buf) >> odp_buffer_hdr_t *buf_hdr = odp_buf_to_hdr(buf); >> pool_entry_t *pool = odp_buf_to_pool(buf_hdr); >> >> + /* Reset packet type in case it was "casted" (e.g. crypto >> completions)*/ >> + buf_hdr->type = pool->s.params.type; >> + >> if (odp_unlikely(pool->s.low_wm_assert)) >> ret_buf(&pool->s, buf_hdr); >> else >> -- >> 1.9.1 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c index 35e79a0..6e65926 100644 --- a/platform/linux-generic/odp_pool.c +++ b/platform/linux-generic/odp_pool.c @@ -560,6 +560,9 @@ void odp_buffer_free(odp_buffer_t buf) odp_buffer_hdr_t *buf_hdr = odp_buf_to_hdr(buf); pool_entry_t *pool = odp_buf_to_pool(buf_hdr); + /* Reset packet type in case it was "casted" (e.g. crypto completions)*/ + buf_hdr->type = pool->s.params.type; + if (odp_unlikely(pool->s.low_wm_assert)) ret_buf(&pool->s, buf_hdr); else
There seems to be an issue with the handling of completion event buffers: These are drawn from the same pool as the packet event buffers, but are re-typed as completion events. (see platform/linux-generic/odp_crypto.c:428) However, when these are freed and returned to the pool, the buffer type was not restored to packet event buffers: The code that allocates packet event buffers from the pool did not update the buffer type either, relying on all buffers in the pool already being initialized to be packet event buffers This patch resets the buffer type at release time. Signed-off-by: Christophe Milard <christophe.milard@linaro.org> --- platform/linux-generic/odp_pool.c | 3 +++ 1 file changed, 3 insertions(+)