| Message ID | 1470861686-15663-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | Accepted |
| Commit | c2d1ed55e871d26cbebae1d0e9080edae9bafe38 |
| Headers | show |
Ping. This urgently needs a review for Monarch release. Testing is simple: example/traffic_mgmt/odp_traffic_mngr sefaults immediately without this patch and passes with it. Thanks. On Wed, Aug 10, 2016 at 3:41 PM, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2458 by only checking > for a proper pktout_queue count if the egress_kind is ODP_TM_EGRESS_PKT_IO. > > This check is also moved before locking and allocating a tm_system struct > to avoid deadlocks and memory leaks that were another side-effect of this > bug. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > Note: This patch should be applied on top of patch > http://patches.opendataplane.org/patch/6801/ > > platform/linux-generic/odp_traffic_mngr.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/platform/linux-generic/odp_traffic_mngr.c > b/platform/linux-generic/odp_traffic_mngr.c > index 37d3323..a5271ed 100644 > --- a/platform/linux-generic/odp_traffic_mngr.c > +++ b/platform/linux-generic/odp_traffic_mngr.c > @@ -2818,6 +2818,13 @@ odp_tm_t odp_tm_create(const char *name, > uint32_t max_tm_queues, max_sorted_lists; > int rc; > > + /* If we are using pktio output (usual case) get the first > associated > + * pktout_queue for this pktio and fail if there isn't one. > + */ > + if (egress->egress_kind == ODP_TM_EGRESS_PKT_IO && > + odp_pktout_queue(egress->pktio, &pktout, 1) != 1) > + return ODP_TM_INVALID; > + > /* Allocate tm_system_t record. */ > odp_ticketlock_lock(&tm_create_lock); > tm_system = tm_system_alloc(); > @@ -2834,9 +2841,6 @@ odp_tm_t odp_tm_create(const char *name, > return ODP_TM_INVALID; > } > > - if (odp_pktout_queue(egress->pktio, &pktout, 1) != 1) > - return ODP_TM_INVALID; > - > tm_system->pktout = pktout; > tm_system->name_tbl_id = name_tbl_id; > max_tm_queues = requirements->max_tm_queues; > -- > 2.7.4 > >
On 10 August 2016 at 16:41, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2458 by only checking > for a proper pktout_queue count if the egress_kind is ODP_TM_EGRESS_PKT_IO. > > This check is also moved before locking and allocating a tm_system struct > to avoid deadlocks and memory leaks that were another side-effect of this > bug. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > Tested-and-reviewed-by: Mike Holmes <mike.holmes@linaro.org> > --- > Note: This patch should be applied on top of patch > http://patches.opendataplane.org/patch/6801/ > > platform/linux-generic/odp_traffic_mngr.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/platform/linux-generic/odp_traffic_mngr.c > b/platform/linux-generic/odp_traffic_mngr.c > index 37d3323..a5271ed 100644 > --- a/platform/linux-generic/odp_traffic_mngr.c > +++ b/platform/linux-generic/odp_traffic_mngr.c > @@ -2818,6 +2818,13 @@ odp_tm_t odp_tm_create(const char *name, > uint32_t max_tm_queues, max_sorted_lists; > int rc; > > + /* If we are using pktio output (usual case) get the first > associated > + * pktout_queue for this pktio and fail if there isn't one. > + */ > + if (egress->egress_kind == ODP_TM_EGRESS_PKT_IO && > + odp_pktout_queue(egress->pktio, &pktout, 1) != 1) > + return ODP_TM_INVALID; > + > /* Allocate tm_system_t record. */ > odp_ticketlock_lock(&tm_create_lock); > tm_system = tm_system_alloc(); > @@ -2834,9 +2841,6 @@ odp_tm_t odp_tm_create(const char *name, > return ODP_TM_INVALID; > } > > - if (odp_pktout_queue(egress->pktio, &pktout, 1) != 1) > - return ODP_TM_INVALID; > - > tm_system->pktout = pktout; > tm_system->name_tbl_id = name_tbl_id; > max_tm_queues = requirements->max_tm_queues; > -- > 2.7.4 > > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
Merged, Maxim. On 08/12/16 21:08, Mike Holmes wrote: > On 10 August 2016 at 16:41, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2458 by only checking >> for a proper pktout_queue count if the egress_kind is ODP_TM_EGRESS_PKT_IO. >> >> This check is also moved before locking and allocating a tm_system struct >> to avoid deadlocks and memory leaks that were another side-effect of this >> bug. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> > Tested-and-reviewed-by: Mike Holmes <mike.holmes@linaro.org> > > > >> --- >> Note: This patch should be applied on top of patch >> http://patches.opendataplane.org/patch/6801/ >> >> platform/linux-generic/odp_traffic_mngr.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/platform/linux-generic/odp_traffic_mngr.c >> b/platform/linux-generic/odp_traffic_mngr.c >> index 37d3323..a5271ed 100644 >> --- a/platform/linux-generic/odp_traffic_mngr.c >> +++ b/platform/linux-generic/odp_traffic_mngr.c >> @@ -2818,6 +2818,13 @@ odp_tm_t odp_tm_create(const char *name, >> uint32_t max_tm_queues, max_sorted_lists; >> int rc; >> >> + /* If we are using pktio output (usual case) get the first >> associated >> + * pktout_queue for this pktio and fail if there isn't one. >> + */ >> + if (egress->egress_kind == ODP_TM_EGRESS_PKT_IO && >> + odp_pktout_queue(egress->pktio, &pktout, 1) != 1) >> + return ODP_TM_INVALID; >> + >> /* Allocate tm_system_t record. */ >> odp_ticketlock_lock(&tm_create_lock); >> tm_system = tm_system_alloc(); >> @@ -2834,9 +2841,6 @@ odp_tm_t odp_tm_create(const char *name, >> return ODP_TM_INVALID; >> } >> >> - if (odp_pktout_queue(egress->pktio, &pktout, 1) != 1) >> - return ODP_TM_INVALID; >> - >> tm_system->pktout = pktout; >> tm_system->name_tbl_id = name_tbl_id; >> max_tm_queues = requirements->max_tm_queues; >> -- >> 2.7.4 >> >> >
diff --git a/platform/linux-generic/odp_traffic_mngr.c b/platform/linux-generic/odp_traffic_mngr.c index 37d3323..a5271ed 100644 --- a/platform/linux-generic/odp_traffic_mngr.c +++ b/platform/linux-generic/odp_traffic_mngr.c @@ -2818,6 +2818,13 @@ odp_tm_t odp_tm_create(const char *name, uint32_t max_tm_queues, max_sorted_lists; int rc; + /* If we are using pktio output (usual case) get the first associated + * pktout_queue for this pktio and fail if there isn't one. + */ + if (egress->egress_kind == ODP_TM_EGRESS_PKT_IO && + odp_pktout_queue(egress->pktio, &pktout, 1) != 1) + return ODP_TM_INVALID; + /* Allocate tm_system_t record. */ odp_ticketlock_lock(&tm_create_lock); tm_system = tm_system_alloc(); @@ -2834,9 +2841,6 @@ odp_tm_t odp_tm_create(const char *name, return ODP_TM_INVALID; } - if (odp_pktout_queue(egress->pktio, &pktout, 1) != 1) - return ODP_TM_INVALID; - tm_system->pktout = pktout; tm_system->name_tbl_id = name_tbl_id; max_tm_queues = requirements->max_tm_queues;
Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2458 by only checking for a proper pktout_queue count if the egress_kind is ODP_TM_EGRESS_PKT_IO. This check is also moved before locking and allocating a tm_system struct to avoid deadlocks and memory leaks that were another side-effect of this bug. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- Note: This patch should be applied on top of patch http://patches.opendataplane.org/patch/6801/ platform/linux-generic/odp_traffic_mngr.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) -- 2.7.4