From: Ed Czeck <ed.czeck@atomicrules.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org, John Miller <john.miller@atomicrules.com>,
Shepard Siegel <shepard.siegel@atomicrules.com>,
Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v5 1/7] net/ark: PMD for Atomic Rules Arkville driver stub
Date: Tue, 28 Mar 2017 18:38:39 -0400 [thread overview]
Message-ID: <CALZ3GugYJKVBN5ywBcZ3E=LxJk=qf6wvCiVqZOL5CwMVD-3Ajw@mail.gmail.com> (raw)
In-Reply-To: <b8a75b12-7033-e6ee-b85d-6e99487e247f@intel.com>
On Tue, Mar 28, 2017 at 10:34 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:
>
> On 3/23/2017 10:59 PM, Ed Czeck wrote:
> > Enable Arkville on supported configurations
> > Add overview documentation
> > Minimum driver support for valid compile
> > Arkville PMD is not supported on ARM or PowerPC at this time
> >
> > v5:
> > * Address comments from Ferruh Yigit <ferruh.yigit@intel.com>
> > * Added documentation on driver args
> > * Makefile fixes
> > * Safe argument processing
> > * vdev args to dev args
> >
> > v4:
> > * Address issues report from review
> > * Add internal comments on driver arg
> > * provide a bare-bones dev init to avoid compiler warnings
> >
> > v3:
> > * Split large patch into several smaller ones
> >
> > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> > Signed-off-by: John Miller <john.miller@atomicrules.com>
>
> <...>
>
> > +Device Parameters
> > +-------------------
> > +
> > +The ARK PMD supports a series of parameters that are used for packet
routing
> > +and for internal packet generation and packet checking. This section
describes
> > +the supported parameters. These features are primarily used for
> > +diagnostics, testing, and performance verification. The nominal use
> > +of Arkville does not require any configuration using these parameters.
> > +
> > +"Pkt_dir"
> > +
> > +The Packet Director controls connectivity between the Packet Generator,
> > +Packet Checker, UDM, DDM, and external ingress and egress interfaces
for
> > +diagnostic purposes. It includes an internal loopback path from the
DDM to the UDM.
>
> What are UDM and DDM are?
These are clarified in the doc, mostly by removing reference to them.
>
>
> > +
> > +NOTE: Packets from the packet generator to the UDM are all directed to
UDM RX
> > +queue 0. Packets looped back from the DDM to the UDM are directed to
the same
> > +queue number they originated from.
>
> So a packet generator is part of this PMD.
>
>
> Isn't it overkill for PMD to have packet generator feauture. Is it
> really needs to be part of PMD? Can be an option to use external packet
> generators?
The packet generator is part of the arkville hardware. The configuration
file is needed to control it behaviors. Its really not part of the PMD,
just a control hook in the PMD to have access into the FPGA hardware.
>
> > +
>
> > +The packet generator parameter takes a file as its argument. The file
contains configuration
> > +parameters used internally for regression testing and are not intended
to be published at this
> > +level.
>
> If these config options are not planned to use by public, should the
> stripped from public version of the PMD? Is there a benefit to keep them
> in dpdk repo?
Internally, we have had this discussion. The features/control are very
useful for diagnostic and isolating Arkville hardware from customer logic.
Our solution is to have a small set of features as device arguments. For
the nominal case these will not be used. However, if there is a special
case, we would like to have the option available to access them without
dropping in a different PMD.
>
>
> > +
> > +/* Internal prototypes */
> > +static int eth_ark_check_args(struct ark_adapter *ark, const char
*params);
> > +static int eth_ark_dev_init(struct rte_eth_dev *dev);
> > +static int eth_ark_dev_uninit(struct rte_eth_dev *eth_dev);
> > +static int eth_ark_dev_configure(struct rte_eth_dev *dev);
> > +static void eth_ark_dev_info_get(struct rte_eth_dev *dev,
> > + struct rte_eth_dev_info *dev_info);
>
> This may be personal, but I am for reordering functions and removing
> static function declarations, its your call.
I'm leaving the order as is. I like to think that there might be a
balance between static prototype declarations and reading the code.
>
>
> > +
> > +#define ARK_DEV_TO_PCI(eth_dev) \
> > + RTE_DEV_TO_PCI((eth_dev)->device)
>
> This macro can go to header file, ark_ethdev.h perhaps?
Moved to .h file
>
>
> <...>
>
> > +
> > + if (pci_dev->device.devargs)
> > + eth_ark_check_args(ark, pci_dev->device.devargs->args);
> > + else
> > + PMD_DRV_LOG(INFO, "No Device args found\n");
> > +
> > +
> > + ark->bar0 = (uint8_t *)pci_dev->mem_resource[0].addr;
> > + ark->a_bar = (uint8_t *)pci_dev->mem_resource[2].addr;
> > +
> > + dev->dev_ops = &ark_eth_dev_ops;
> > +
> > + /* We process our args last as they require everything to be
setup */
> > + if (pci_dev->device.devargs)
> > + eth_ark_check_args(ark, pci_dev->device.devargs->args);
> > + else
> > + PMD_DRV_LOG(INFO, "No Device args found\n");
>
> This is duplicate, please check ~10 lines above.
Duplicate code removed.
>
>
> Should check the return value of the function, is it OK to continue if
> invalid devargs provided by user?
Return values are now checked.
>
>
> > +
> > + return ret;
> > +}
> > +
> > +static int
> > +eth_ark_dev_uninit(struct rte_eth_dev *dev)
> > +{
> > + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > + return 0;
>
> Should primary process check also required in eth_ark_dev_init() ?
See comments in patch 7 for a explanation.
>
>
> > +
> > + if (rte_kvargs_process(kvlist,
> > + ARK_PKTDIR_ARG,
> > + &process_pktdir_arg,
> > + ark) != 0) {
> > + PMD_DRV_LOG(ERR, "Unable to parse arg %s\n",
ARK_PKTDIR_ARG);
>
> No error returned for this case?
Error are properly handled in this function and it caller.
>
>
> > +RTE_PMD_REGISTER_PCI(eth_ark, rte_ark_pmd.pci_drv);
>
> Net device names updated, eth_xxx -> net_xxx. This should be net_ark.
>
Updated to net_ark
Thanks,
Ed.
next prev parent reply other threads:[~2017-03-28 22:39 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-23 1:03 [dpdk-dev] [PATCH v4 " Ed Czeck
2017-03-23 1:03 ` [dpdk-dev] [PATCH v4 2/7] net/ark: HW API part 1 of 3 Ed Czeck
2017-03-23 11:38 ` Ferruh Yigit
2017-03-23 20:33 ` Ed Czeck
2017-03-29 1:05 ` [dpdk-dev] [PATCH v6 2/7] net/ark: Provide API for hardware modules mpu, rqp, and pktdir Ed Czeck
2017-03-29 21:32 ` [dpdk-dev] [PATCH v7 2/7] net/ark: provide API for hardware modules mpu rqp " Ed Czeck
2017-04-04 19:50 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-03-23 1:03 ` [dpdk-dev] [PATCH v4 3/7] net/ark: HW API part 2 of 3 Ed Czeck
2017-03-29 1:05 ` [dpdk-dev] [PATCH v6 3/7] net/ark: Provide API for hardware modules udm and ddm Ed Czeck
2017-03-29 21:33 ` [dpdk-dev] [PATCH v7 3/7] net/ark: provide " Ed Czeck
2017-04-04 19:50 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-03-23 1:03 ` [dpdk-dev] [PATCH v4 4/7] net/ark: HW API part 3 of 3 Ed Czeck
2017-03-29 1:06 ` [dpdk-dev] [PATCH v6 4/7] net/ark: Provide API for hardware modules pktchkr and pktgen Ed Czeck
2017-03-29 21:34 ` [dpdk-dev] [PATCH v7 4/7] net/ark: provide " Ed Czeck
2017-04-04 19:50 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-03-23 1:03 ` [dpdk-dev] [PATCH v4 5/7] net/ark: Packet TX support initial version Ed Czeck
2017-03-23 12:14 ` Ferruh Yigit
2017-03-23 21:44 ` Ed Czeck
2017-03-29 1:06 ` [dpdk-dev] [PATCH v6 " Ed Czeck
2017-03-29 21:34 ` [dpdk-dev] [PATCH v7 5/7] net/ark: packet Tx " Ed Czeck
2017-04-04 19:51 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-03-23 1:03 ` [dpdk-dev] [PATCH v4 6/7] net/ark: Packet RX " Ed Czeck
2017-03-23 12:14 ` Ferruh Yigit
2017-03-23 21:51 ` Ed Czeck
2017-03-29 1:06 ` [dpdk-dev] [PATCH v6 " Ed Czeck
2017-03-29 21:35 ` [dpdk-dev] [PATCH v7 6/7] net/ark: packet Rx " Ed Czeck
2017-04-04 19:51 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-03-23 1:03 ` [dpdk-dev] [PATCH v4 7/7] net/ark: Arkville PMD component integration Ed Czeck
2017-03-23 12:13 ` Ferruh Yigit
2017-03-23 22:19 ` Ed Czeck
2017-03-28 12:34 ` Ferruh Yigit
2017-03-29 1:07 ` [dpdk-dev] [PATCH v6 " Ed Czeck
2017-03-29 9:54 ` Ferruh Yigit
2017-03-29 21:35 ` [dpdk-dev] [PATCH v7 7/7] net/ark: arkville " Ed Czeck
2017-04-04 19:51 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-03-23 11:36 ` [dpdk-dev] [PATCH v4 1/7] net/ark: PMD for Atomic Rules Arkville driver stub Ferruh Yigit
2017-03-23 13:08 ` Ferruh Yigit
2017-03-23 19:46 ` Ed Czeck
2017-03-28 12:58 ` Ferruh Yigit
2017-03-28 21:11 ` Ed Czeck
2017-03-29 9:42 ` Ferruh Yigit
2017-03-23 22:59 ` [dpdk-dev] [PATCH v5 " Ed Czeck
2017-03-23 23:00 ` [dpdk-dev] [PATCH v5 2/7] net/ark: provide api to hardware module mpu, rqp, and pktdir Ed Czeck
2017-03-28 14:35 ` Ferruh Yigit
2017-03-28 20:14 ` Ed Czeck
2017-03-23 23:00 ` [dpdk-dev] [PATCH v5 3/7] net/ark: provide API hardware module udm and ddm Ed Czeck
2017-03-23 23:00 ` [dpdk-dev] [PATCH v5 4/7] net/ark: prrovide api for hardware module pktchkr and pktgen Ed Czeck
2017-03-23 23:00 ` [dpdk-dev] [PATCH v5 5/7] net/ark: Packet TX support initial version Ed Czeck
2017-03-23 23:01 ` [dpdk-dev] [PATCH v5 6/7] net/ark: Packet RX " Ed Czeck
2017-03-28 14:36 ` Ferruh Yigit
2017-03-28 21:59 ` Ed Czeck
2017-03-23 23:01 ` [dpdk-dev] [PATCH v5 7/7] net/ark: Arkville PMD component integration Ed Czeck
2017-03-28 14:38 ` Ferruh Yigit
2017-03-28 15:00 ` Adrien Mazarguil
2017-03-28 22:42 ` Ed Czeck
2017-03-28 14:34 ` [dpdk-dev] [PATCH v5 1/7] net/ark: PMD for Atomic Rules Arkville driver stub Ferruh Yigit
2017-03-28 22:38 ` Ed Czeck [this message]
2017-03-28 14:41 ` Ferruh Yigit
2017-03-29 1:04 ` [dpdk-dev] [PATCH v6 " Ed Czeck
2017-03-29 21:32 ` [dpdk-dev] [PATCH v7 1/7] net/ark: stub PMD for Atomic Rules Arkville Ed Czeck
2017-03-31 14:51 ` Ferruh Yigit
2017-03-31 15:09 ` Shepard Siegel
2017-04-04 20:58 ` Ed Czeck
2017-04-04 19:50 ` [dpdk-dev] [PATCH v8 " Ed Czeck
2017-04-07 10:54 ` Ferruh Yigit
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='CALZ3GugYJKVBN5ywBcZ3E=LxJk=qf6wvCiVqZOL5CwMVD-3Ajw@mail.gmail.com' \
--to=ed.czeck@atomicrules.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=john.miller@atomicrules.com \
--cc=shepard.siegel@atomicrules.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).