DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Coyle, David" <david.coyle@intel.com>
To: Jerin Jacob <jerinjacobk@gmail.com>,
	Shreyansh Jain <shreyansh.jain@nxp.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"Doherty, Declan" <declan.doherty@intel.com>,
	"Trahe, Fiona" <fiona.trahe@intel.com>,
	"Ryan, Brendan" <brendan.ryan@intel.com>,
	"O'loingsigh, Mairtin" <mairtin.oloingsigh@intel.com>
Subject: Re: [dpdk-dev] [RFC] Accelerator API to chain packet processing functions
Date: Fri, 6 Mar 2020 14:55:35 +0000	[thread overview]
Message-ID: <SN6PR11MB3086D1222D0CFF64C0A32F09E3E30@SN6PR11MB3086.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CALBAE1OhVDP8fQGoP69eoVS9OHV_rz+MtTAsWydNNnoiVZ6a0g@mail.gmail.com>

> >
> > /** 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.

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

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.

  reply	other threads:[~2020-03-06 14:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-05 16:44 Coyle, David
2020-03-06  9:06 ` Jerin Jacob
2020-03-06 14:55   ` Coyle, David [this message]
2020-03-06 16:22     ` Jerin Jacob
2020-03-13 18:00       ` Coyle, David
2020-03-13 18:03         ` Jerin Jacob
  -- strict thread matches above, loose matches on Subject: below --
2020-02-04 14:45 David Coyle
2020-02-04 19:52 ` Jerin Jacob
2020-02-06 10:04   ` Coyle, David
2020-02-06 10:54     ` Jerin Jacob
2020-02-06 16:31       ` Coyle, David
2020-02-06 17:13         ` Jerin Jacob
2020-02-07 12:38           ` Coyle, David
2020-02-07 14:18             ` Jerin Jacob
2020-02-07 20:34               ` Stephen Hemminger
2020-02-08  7:22                 ` Jerin Jacob
2020-03-05 17:01                   ` Coyle, David
2020-03-06  8:43                     ` Jerin Jacob
2020-02-13 11:50               ` Doherty, Declan
2020-02-18  5:15                 ` Jerin Jacob
2020-02-13 11:44           ` Doherty, Declan
2020-02-18  5:30             ` Jerin Jacob
2020-02-13 11:31       ` Doherty, Declan
2020-02-18  5:12         ` Jerin Jacob

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN6PR11MB3086D1222D0CFF64C0A32F09E3E30@SN6PR11MB3086.namprd11.prod.outlook.com \
    --to=david.coyle@intel.com \
    --cc=brendan.ryan@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinjacobk@gmail.com \
    --cc=mairtin.oloingsigh@intel.com \
    --cc=shreyansh.jain@nxp.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).