Re: [dpdk-dev] [PATCH] eventdev: negate maintenance capability flag

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, Nov 10, 2021 at 3:37 PM Mattias Rönnblom
<mattias.ronnblom@ericsson.com> wrote:
>
> On 2021-11-10 10:55, Van Haaren, Harry wrote:
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> Sent: Wednesday, November 10, 2021 9:29 AM
> >> To: jerinj@marvell.com; Jerin Jacob <jerinjacobk@gmail.com>; Sunil Kumar Kori
> >> <skori@marvell.com>
> >> Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> >> Hemant Agrawal <hemant.agrawal@nxp.com>; Nipun Gupta
> >> <nipun.gupta@nxp.com>; Van Haaren, Harry <harry.van.haaren@intel.com>;
> >> Thomas Monjalon <thomas@monjalon.net>; David Marchand
> >> <david.marchand@redhat.com>; Gujjar, Abhinandan S
> >> <abhinandan.gujjar@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> >> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Yigit, Ferruh
> >> <ferruh.yigit@intel.com>; Akhil Goyal <gakhil@marvell.com>; mattias.ronnblom
> >> <mattias.ronnblom@ericsson.com>
> >> Subject: [PATCH] eventdev: negate maintenance capability flag
> >>
> >> Replace RTE_EVENT_DEV_CAP_REQUIRES_MAINT, which signaled the need
> >> for the application to call rte_event_maintain(), with
> >> RTE_EVENT_DEV_CAP_MAINTENANCE_FREE, which does the opposite (i.e.,
> >> signifies that the event device does not require maintenance).
> >>
> >> This approach is more in line with how other eventdev hardware and/or
> >> software limitations are handled in the Eventdev API.
> >>
> >> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> > Generally patch looks fine to me, but on testing I got a strange warning from the
> > compiler on the OPDL driver:
> >
> > In file included from ../drivers/event/opdl/opdl_evdev.h:8,
> >                   from ../drivers/event/opdl/opdl_evdev.c:15:
> > ../drivers/event/opdl/opdl_evdev.c: In function ‘opdl_info_get’:
> > ../lib/eventdev/rte_eventdev.h:302:44: warning: conversion from ‘long long unsigned int’ to ‘un
> > signed char’ changes value from ‘1024’ to ‘0’ [-Woverflow]
> >    302 | #define RTE_EVENT_DEV_CAP_MAINTENANCE_FREE (1ULL << 10)
> >        |                                            ^
> > ../drivers/event/opdl/opdl_evdev.c:379:34: note: in expansion of macro ‘RTE_EVENT_DEV_CAP_MAINT
> > ENANCE_FREE’
> >    379 |                                  RTE_EVENT_DEV_CAP_MAINTENANCE_FREE,
> >        |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Investigating, RTE_EVENT_DEV_CAP_* are assigned to struct rte_event_dev_info::event_dev_cap,
> > which is a uint32_t. The error suggests that the variable being assigned to is a "unsigned char"?
> >
> > <snip>
> >
> >>   static void
> >> diff --git a/drivers/event/opdl/opdl_evdev.c b/drivers/event/opdl/opdl_evdev.c
> >> index 5007e9a7bf..787ee4713d 100644
> >> --- a/drivers/event/opdl/opdl_evdev.c
> >> +++ b/drivers/event/opdl/opdl_evdev.c
> >> @@ -376,6 +376,7 @@ opdl_info_get(struct rte_eventdev *dev, struct
> >> rte_event_dev_info *info)
> >>              .max_num_events = OPDL_INFLIGHT_EVENTS_TOTAL,
> >>              .event_dev_cap = RTE_EVENT_DEV_CAP_BURST_MODE |
> >>                               RTE_EVENT_DEV_CAP_CARRY_FLOW_ID,
> >> +                             RTE_EVENT_DEV_CAP_MAINTENANCE_FREE,
> >>      };
> > Aha! Replace:
> >>                               RTE_EVENT_DEV_CAP_CARRY_FLOW_ID,
> > with:
> >>                               RTE_EVENT_DEV_CAP_CARRY_FLOW_ID |
> > The "," character was causing the _FREE flag to not be combined with the previous
> > flags field, instead being its own (unnamed?) assignment to the struct rte_event_dev_info.
> >
> > With that , to | change, all looks good to me. -Harry
> >
> >
> I sent a v2. Thanks!

v2 has doc build error. Please send v3

http://mails.dpdk.org/archives/test-report/2021-November/241649.html
>