From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0679AA056A; Fri, 6 Mar 2020 17:22:28 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0703B2BA8; Fri, 6 Mar 2020 17:22:28 +0100 (CET) Received: from mail-il1-f196.google.com (mail-il1-f196.google.com [209.85.166.196]) by dpdk.org (Postfix) with ESMTP id 4960023D for ; Fri, 6 Mar 2020 17:22:27 +0100 (CET) Received: by mail-il1-f196.google.com with SMTP id a6so2555477ilc.4 for ; Fri, 06 Mar 2020 08:22:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CVIQrKyzJFIXU2MYrFTa9SqqX62PBViDFBYLmih3npA=; b=EkJY7mCdiqG3BisUt4x7dTXicQzkDn56KkE6w1hkdbJ9T1/2nr4vsrk8ECD/jZzglb shkS9HK+HovSRqh7c3s8egL5KKXGAAl7FXQQGyPm+ebiN3W4BHboashtIQdydEGk2GU5 bLWk6JpD+w6Y7VNvFKEE3WsTUf+78J1NFlyB6s4Wmlmn97sOuu5F4iV9V9L7J43gKuIg 21PE+f0jvGvoJ4YX9l5bOPpvBx945C/bnn/rnQr0IK+ciBjByMRg4wFiEetE9NJPRZTT Z73ZPL+/Qy8LNWL5B3PNxbo/TOwsYs5eR6iGGGo6uKqnqYQ9AYqHFf/OuLhpyKqYnRk4 t5Xg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CVIQrKyzJFIXU2MYrFTa9SqqX62PBViDFBYLmih3npA=; b=Ybfo1H8ftH4wRDZu0YWWunZkgXdHM1olT95wxFDzPiVCmUuKmAUPqKllFP2QP+YjUN 3V4WPlzJpJ/hj4vGz1XiGc1DmGWkamhxU+7qbxxmd07tJ/lSNBzERT/tApSTLqs3NVrr 2LDNTnAMYI2S8ZsWKsT26rRUmvA9KnOYc4Kla2wXmedbTBpU2qtrt4aGPuAOcQb24VnU Xmge+bllBVhgjI5dJdjLspCIEB4uVn/R7MOjwAz4Tx3kHnlclXfOews6528hMzDnLUAg fo4aGmTuJaVGDJaxUP+eNTPwqrtP0HuOKRoBFmgGIsUWoqxj1I7GwYcnDx8RUlO2d+3T 0poA== X-Gm-Message-State: ANhLgQ1hQ9HijvcEITDIvTsi0IQXvbDLnTOl38goTxdUWlJbdraT2S2j SRumO4hPT2J/qsOasMfRpkeOzokBDewt4HzJUpg= X-Google-Smtp-Source: ADFU+vtSA6SYg5M0uCdjh1kzxdevbvYLSkHjLkL+bPEPDtb+pOJVX33miBKJla6bbwTyliDI5tKU4DZYnXyjzNMQz6M= X-Received: by 2002:a92:1906:: with SMTP id 6mr4109873ilz.130.1583511746403; Fri, 06 Mar 2020 08:22:26 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Jerin Jacob Date: Fri, 6 Mar 2020 21:52:10 +0530 Message-ID: To: "Coyle, David" Cc: Shreyansh Jain , Hemant Agrawal , "dev@dpdk.org" , "stephen@networkplumber.org" , "Doherty, Declan" , "Trahe, Fiona" , "Ryan, Brendan" , "O'loingsigh, Mairtin" Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [RFC] Accelerator API to chain packet processing functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list 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 6, 2020 at 8:25 PM Coyle, David wrote: > > > > > > > /** Error Detection Algorithms */ > > > enum rte_rawdev_multi_fn_err_detect_algorithm { > > > RTE_RAWDEV_MULTI_FN_ERR_DETECT_CRC32_ETH, > > > > IMO, It does not make sense to add protocol specific stuff in rawdev > > symbols. > > > > IMO, It is better to have a separate library for CRC and BIP32 acceleration like > > the rte_security library and underneath still it can use rawdev or anydev if > > required. > > [DC] This protocol stuff is only in the rawdev interface definition, which is known only to the application and the rawdev PMDs which will use this interface. > So these defines/enums/structs etc for CRC and BIP are completely opaque to rte_rawdev itself. > > This is how all existing rawdev PMDs interfaces are defined, where the interface is very specific to the job(s) the PMD is implementing. If you see .map file in driver/raw/. None of the drivers are exposing any API with rte_rawdev_*. This addition will be exposing new rte_rawdev_* APIs from driver/rawdev/. That's is not correct. $ find drivers/raw/ -name *.map drivers/raw/skeleton/rte_rawdev_skeleton_version.map drivers/raw/octeontx2_ep/rte_rawdev_octeontx2_ep_version.map drivers/raw/ntb/rte_rawdev_ntb_version.map drivers/raw/dpaa2_qdma/rte_rawdev_dpaa2_qdma_version.map drivers/raw/dpaa2_cmdif/rte_rawdev_dpaa2_cmdif_version.map drivers/raw/ioat/rte_rawdev_ioat_version.map drivers/raw/octeontx2_dma/rte_rawdev_octeontx2_dma_version.map drivers/raw/ifpga/rte_rawdev_ifpga_version.map IMO, Correct thing to do will be, Either of 1) As mentioned below, If you would like to limit the scope only to a new rawdev driver then a) Create a new driver at driver/raw// b) expose the drier specific customer API as rte__...(example: drivers/raw/dpaa2_qdma/rte_rawdev_dpaa2_qdma_version.map 2) If we would like to have public API then create a subsystem like libsecurity to have features. Let the API exposed from lib/... > > Also, these particular defines/enums/structs for CRC and BIP are only for defining xform and op chains containing these particular operations. > The actual code to do the CRC and BIP is already in the AESNI-MB library or DPDK rte_net_crc library, which our aesni_mb and qat rawdev PMDs will call/use > > > > > IMO, Exposing the public API in > > drivers/raw/common/rte_rawdev_multi_fn.h is a shortcut. > > IMO, public API should be in lib/.. > > [DC] To be honest, I tend to agree. I don't like that public APIs are exposed from the drivers directory. > But as I mentioned above, this is how all rawdev PMD interfaces are defined, where the interface definition is within the PMD directory (e.g. drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h) > Our's is slightly different in that we have 2 PMDs which will use the same interface, which is why we have added it in drivers/raw/common > So by keeping our interface under drivers, we are trying to be consistent with all existing rawdev PMDs > > As I mentioned in my previous post though, this could potentially be moved under lib in the future if other PMDs would find it useful See above. Point (1). > > We could possibly rename our interface file to rte_pmd_multi_fn.h to be a bit more consistent with the majority of the existing PMDs and take away the idea for now that this is some kind of extension to the main rte_rawdev API. > But unfortunately there is no full consistency in the rawdev PMD interface filenames (e.g. dpaa2_cmdif uses the "rte_pmd_" prefix - rte_pmd_dpaa2_cmdif.h, octeontx2_dma uses the "_rawdev" suffix - otx2_dpi_rawdev.h) > > > > > Just my 2c.