From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 82F2D426AE; Tue, 3 Oct 2023 19:32:26 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 16F6B402A2; Tue, 3 Oct 2023 19:32:26 +0200 (CEST) Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by mails.dpdk.org (Postfix) with ESMTP id A4FD840262; Tue, 3 Oct 2023 19:32:25 +0200 (CEST) Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-4196ae80fc3so7905001cf.0; Tue, 03 Oct 2023 10:32:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696354345; x=1696959145; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=OE24gqGm5CJ889LKtRx1oNsH+46h2Vl/V1VIIa9ySl0=; b=MWILwRPNtrfsY8beNN0qCJTJDMT8ehlO0U8hWManos+yMxEPnFA+Y+qvQDd2+HPKUU 3fU+mqC5qdpDKgnPak4TbRceA/5K6ogNOJsdDNe85ndWXCO7u/NcOGNi7tfMHqg17caH l8uappCglkeWXGXx9jdtRt4L/5g4ZavT3PeIE+RI0FYFcbI5ZgYG1bC+v++SkIy8gnNk XSQ3rZXHg5OGWflOANVKaX10uiuKXmrod7k52XCoM6h8h+dQkJW3hlGO/uBgHqQYOimN fcYsCDi62xFR/hlw3X/AkP4K23aZz50pPthxz9qGb3eXqq3IJMaVyjbnpOevChQmArmU frdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696354345; x=1696959145; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OE24gqGm5CJ889LKtRx1oNsH+46h2Vl/V1VIIa9ySl0=; b=et/l3NnRabOgjEX26aaA1H9/1UXEr20lTjZ75oFmnMWByHGWDjBzqTRkEbtxTmr0P0 yt0HIJZZHBb9VveMYm6ksAYzzk/TlQX/3GFLx/9HXpaPvVQk5wQTz9Ytm59BT3Ry+JYl 2ax+aC/OvJoxauoU7eiKlMpEJnSzTFoxNptzEJ6z9QxQ4isM3Ju6dOedj5dmpj5w3RWY AzGKglQDPBe+1eeEMSUIdq+tI2M3nmCR21lrkF1x5WK9tqNrJElTnuUMO9f/sQ/dglHP Tak2tUVQcN+t86zUYaGXxqg2M4wNuk1bzjrIrBXZilHFDhrrvwnXt78LVdkxkqUAtkw1 bPzg== X-Gm-Message-State: AOJu0YyZkGuVjAT1QQof0kZVaszz4StbAdYFDUyvLxiPjFDsIgoDnI75 ORpHHBUyj7Jqhdxf42XmacT18llTFAjlyZlxt6M= X-Google-Smtp-Source: AGHT+IFJUSLWdZn1iilKRlOFFn6dTDwZ6JWRwFe/GpBBFBilANGersFBN77SYL9LO8sbFQPeABzlBkv7m6ay0NRO27c= X-Received: by 2002:a05:622a:3c9:b0:40f:f0e0:a008 with SMTP id k9-20020a05622a03c900b0040ff0e0a008mr129405qtx.53.1696354344821; Tue, 03 Oct 2023 10:32:24 -0700 (PDT) MIME-Version: 1.0 References: <20230904130313.327809-2-mattias.ronnblom@ericsson.com> <20230922073825.351453-1-mattias.ronnblom@ericsson.com> <20230922073825.351453-2-mattias.ronnblom@ericsson.com> <00d5595f-11a2-089f-f563-3d1d75df27df@lysator.liu.se> In-Reply-To: From: Jerin Jacob Date: Tue, 3 Oct 2023 23:01:58 +0530 Message-ID: Subject: Re: [PATCH v4 1/3] lib: introduce dispatcher library To: Bruce Richardson Cc: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , dev@dpdk.org, Jerin Jacob , techboard@dpdk.org, harry.van.haaren@intel.com, Peter Nilsson , Heng Wang , Naga Harish K S V , Pavan Nikhilesh , Gujjar Abhinandan S , Erik Gabriel Carrillo , Shijith Thotton , Hemant Agrawal , Sachin Saxena , Liang Ma , Peter Mccarthy , Zhirun Yan Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Sep 27, 2023 at 1:43=E2=80=AFPM Bruce Richardson wrote: > > On Tue, Sep 26, 2023 at 11:58:37PM +0530, Jerin Jacob wrote: > > On Mon, Sep 25, 2023 at 12:41=E2=80=AFPM Mattias R=C3=B6nnblom wrote: > > > > > > On 2023-09-22 09:38, Mattias R=C3=B6nnblom wrote: > > > > > > > > > > > > > +int > > > > +rte_dispatcher_create(uint8_t id, uint8_t event_dev_id) > > > > +{ > > > > > > > > > There are two changes I'm considering: > > > > > > 1) Removing the "id" to identify the dispatcher, replacing it with an > > > forward-declared rte_dispatcher struct pointer. > > > > > > struct rte_dispatcher; > > > > > > struct rte_dispatcher * > > > rte_dispatcher_create(uint8_t event_dev_id); > > > > > > > > > The original reason for using an integer id to identify a dispatcher = is > > > to make it look like everything else in Eventdev. I find this pattern= a > > > little awkward to use - in particular the fact the id is > > > application-allocated (and thus require coordination between differen= t > > > part of the application in case multiple instances are used). > > > > > > 2) Adding a flags field to the create function "for future use". But > > > since the API is experimental, there may not be that much need to > > > attempt to be future-proof? > > > > > > Any thoughts are appreciated. > > > > IMO, better to have rte_dispatcher_create(struct > > rte_dispatch_create_params *params) > > for better future proofing with specific > > rte_dispatch_crearte_params_init() API(No need to add reserved fields > > in rte_dispatch_create_params now, may need only for before removing > > experimental status) > > > > Just 2c. > > > > I don't like using structs in those cases, I'd much rather have a flags > parameter, as flags can be checked for explicit zeros for future proofing= , > while a struct cannot be checked for extra space on the end for future > fields added. For lib/dispatcher library, I have don't have specific preference. So anything is fine for me. However, I thought of understanding your rationale for arguments vs structure(Looks like more of vi vs emac discussion) for _my understanding_. In my view, # Use flags for setting up to express specific behavior, not as inputting a lot of input parameters. # Do we need to check extra space if struct have reserved fields and having init() functions for filling default > > Furthermore, if we need to add new parameters to the create function, I > actually believe it is better to add them as explicit parameters rather > than new fields to the struct. Struct fields can be missed by a user just > recompiling, while new function parameters will be flagged by the compile= r I would see this as on the positive side, when - Same code base needs to support multiple DPDK versions. - A lot of times, API consumer may need only _default_ values. Like local_cache value in mempool_create API. So struct with _init() get required values in easy way. My views are based mostly used existing rte_mempool_create() APIs. For some reason, I don't like this scheme. struct rte_mempool * rte_mempool_create(const char *name, unsigned n, unsigned elt_size, unsigned cache_size, unsigned private_data_size, rte_mempool_ctor_t *mp_init, void *mp_init_arg, rte_mempool_obj_cb_t *obj_init, void *obj_init_arg, int socket_id, unsigned flags); > to make the user aware of the change. [There would be no change for ABI > compatibility as function versioning would be usable in both cases] Yes. But need to too much template code via VERSION_SYMBOL where structure scheme does not need. > > /Bruce