From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v5 02/20] event/sw: add new software-only eventdev driver Date: Sat, 25 Mar 2017 11:54:24 +0530 Message-ID: <20170325062423.g44dcf46eweeypwx@localhost.localdomain> References: <489175012-101439-1-git-send-email-harry.van.haaren@intel.com> <1490374395-149320-1-git-send-email-harry.van.haaren@intel.com> <1490374395-149320-3-git-send-email-harry.van.haaren@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, Bruce Richardson To: Harry van Haaren Return-path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0042.outbound.protection.outlook.com [104.47.38.42]) by dpdk.org (Postfix) with ESMTP id CF619D267 for ; Sat, 25 Mar 2017 07:24:44 +0100 (CET) Content-Disposition: inline In-Reply-To: <1490374395-149320-3-git-send-email-harry.van.haaren@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Mar 24, 2017 at 04:52:57PM +0000, Harry van Haaren wrote: > From: Bruce Richardson > > This adds the minimal changes to allow a SW eventdev implementation to > be compiled, linked and created at run time. The eventdev does nothing, > but can be created via vdev on commandline, e.g. > > sudo ./x86_64-native-linuxapp-gcc/app/test --vdev=event_sw0 > ... > PMD: Creating eventdev sw device event_sw0, numa_node=0, sched_quanta=128 > RTE>> > > Signed-off-by: Bruce Richardson > Signed-off-by: Harry van Haaren > --- > config/common_base | 6 + > drivers/event/Makefile | 1 + > drivers/event/sw/Makefile | 66 ++++++++++ > drivers/event/sw/rte_pmd_evdev_sw_version.map | 3 + > drivers/event/sw/sw_evdev.c | 177 ++++++++++++++++++++++++++ [snip] > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > + > +#include > + > +#include > +#include > +#include > +#include > + > +#include "sw_evdev.h" > + > +#define EVENTDEV_NAME_SW_PMD event_sw > +#define NUMA_NODE_ARG "numa_node" > +#define SCHED_QUANTA_ARG "sched_quanta" > +#define CREDIT_QUANTA_ARG "credit_quanta" > + > +static int > +assign_numa_node(const char *key __rte_unused, const char *value, void *opaque) > +{ > + int *socket_id = opaque; > + *socket_id = atoi(value); > + if (*socket_id > RTE_MAX_NUMA_NODES) Shouldn't be ">= RTE_MAX_NUMA_NODES" check, as numa_id is from 0 to RTE_MAX_NUMA_NODES - 1 > + return -1; > + return 0; > +} > + > +static int > +set_sched_quanta(const char *key __rte_unused, const char *value, void *opaque) > +{ > + int *quanta = opaque; > + *quanta = atoi(value); > + if (*quanta < 0 || *quanta > 4096) Is quanta == 4096 valid? or It is only from 0 to 4095? I think, it is nice to set max value as #define value in sw_evdev.h > + return -1; > + return 0; > +} > + > +static int > +set_credit_quanta(const char *key __rte_unused, const char *value, void *opaque) > +{ > + int *credit = opaque; > + *credit = atoi(value); > + if (*credit < 0 || *credit > 128) Same as above comment. > + return -1; > + return 0; > +} > + > +static int > +sw_probe(const char *name, const char *params) > +{ > + static const struct rte_eventdev_ops evdev_sw_ops = { > + }; > + > + static const char *const args[] = { > + NUMA_NODE_ARG, > + SCHED_QUANTA_ARG, > + CREDIT_QUANTA_ARG, > + NULL > + }; > + struct rte_eventdev *dev; > + struct sw_evdev *sw; > + int socket_id = rte_socket_id(); > + int sched_quanta = SW_DEFAULT_SCHED_QUANTA; > + int credit_quanta = SW_DEFAULT_CREDIT_QUANTA; > + > + if (params != NULL && params[0] != '\0') { > + struct rte_kvargs *kvlist = rte_kvargs_parse(params, args); > + > + if (!kvlist) { > + SW_LOG_INFO( > + "Ignoring unsupported parameters when creating device '%s'\n", > + name); > + } else { > + int ret = rte_kvargs_process(kvlist, NUMA_NODE_ARG, > + assign_numa_node, &socket_id); > + if (ret != 0) { > + SW_LOG_ERR( > + "%s: Error parsing numa node parameter", > + name); > + rte_kvargs_free(kvlist); > + return ret; > + } > + > + ret = rte_kvargs_process(kvlist, SCHED_QUANTA_ARG, > + set_sched_quanta, &sched_quanta); > + if (ret != 0) { > + SW_LOG_ERR( > + "%s: Error parsing sched quanta parameter", > + name); > + rte_kvargs_free(kvlist); > + return ret; > + } > + > + ret = rte_kvargs_process(kvlist, CREDIT_QUANTA_ARG, > + set_credit_quanta, &credit_quanta); > + if (ret != 0) { > + SW_LOG_ERR( > + "%s: Error parsing credit quanta parameter", > + name); > + rte_kvargs_free(kvlist); > + return ret; > + } > + > + rte_kvargs_free(kvlist); > + } > + } > + > + SW_LOG_INFO( An extra line here may be not required here. > + "Creating eventdev sw device %s, numa_node=%d, sched_quanta=%d, credit_quanta=%d\n", > + name, socket_id, sched_quanta, credit_quanta); > + > + dev = rte_event_pmd_vdev_init(name, > + sizeof(struct sw_evdev), socket_id); > + if (dev == NULL) { > + SW_LOG_ERR("eventdev vdev init() failed"); > + return -EFAULT; > + } > + dev->dev_ops = &evdev_sw_ops; > + > + sw = dev->data->dev_private; > + sw->data = dev->data; > + > + /* copy values passed from vdev command line to instance */ > + sw->credit_update_quanta = credit_quanta; > + sw->sched_quanta = sched_quanta; > + > + return 0; > +} > + > +static int > +sw_remove(const char *name) > +{ > + if (name == NULL) > + return -EINVAL; > + > + SW_LOG_INFO("Closing eventdev sw device %s\n", name); > + > + return rte_event_pmd_vdev_uninit(name); > +} > + > +static struct rte_vdev_driver evdev_sw_pmd_drv = { > + .probe = sw_probe, > + .remove = sw_remove > +}; > + > +RTE_PMD_REGISTER_VDEV(EVENTDEV_NAME_SW_PMD, evdev_sw_pmd_drv); > +RTE_PMD_REGISTER_PARAM_STRING(event_sw, NUMA_NODE_ARG "= " > + SCHED_QUANTA_ARG "=" CREDIT_QUANTA_ARG "="); With suggested changes, Acked-by: Jerin Jacob