DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Ed Czeck <ed.czeck@atomicrules.com>, dev@dpdk.org
Cc: john.miller@atomicrules.com, shepard.siegel@atomicrules.com,
	stephen@networkplumber.org,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v5 7/7] net/ark: Arkville PMD component integration
Date: Tue, 28 Mar 2017 15:38:13 +0100	[thread overview]
Message-ID: <c70bfd96-29f7-4f20-8804-4a70b0fc4507@intel.com> (raw)
In-Reply-To: <3d68039422963c92ef6f05ad4bd9b0b3497eb7bd.1490309515.git.ed.czeck@atomicrules.com>

On 3/23/2017 11:01 PM, Ed Czeck wrote:
> * Flesh out device configuration
> * Add links dev_ops
> * allow dynamic extension loading
> 
> Signed-off-by: Shepard Siegel <shepard.siegel@atomicrules.com>
> Signed-off-by: John Miller <john.miller@atomicrules.com>
> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>

<...>

> +[Features]
> +Queue start/stop     = Y
> +Jumbo frame          = Y
> +Scattered Rx         = Y
> +Basic stats          = Y
> +Stats per queue      = Y

> +FW version           = Y

FW version is not required, as far as I can see, it requires
fw_version_get eth_dev_ops implemented.

<...>

> +	/* We are a single function multi-port device. */
> +	const unsigned int numa_node = rte_socket_id();
> +	struct ether_addr adr;
> +
> +	ret = ark_config_device(dev);
>  	dev->dev_ops = &ark_eth_dev_ops;
>  
> +	dev->data->mac_addrs = rte_zmalloc("ark", ETHER_ADDR_LEN, 0);
> +	if (!dev->data->mac_addrs) {
> +		PMD_DRV_LOG(ERR,
> +			    "Failed to allocated memory for storing mac address"
> +			    );
> +	}
> +	ether_addr_copy((struct ether_addr *)&adr, &dev->data->mac_addrs[0]);

"adr" has random value at this point, right? Why to copy it?


> +	/*
> +	 * We will create additional devices based on the number of requested
> +	 * ports
> +	 */
> +	int pc = 1;
> +	int p;

I am aware some people prefer the declaring variables close to context,
which is good idea, but if I remember correct, there was a patchset,
from Adrien, to make DPDK C99 compatible, will this break it?

> +
> +	if (ark->user_ext.dev_get_port_count) {
> +		pc = ark->user_ext.dev_get_port_count(dev, ark->user_data);
> +		ark->num_ports = pc;
> +	} else {
> +		ark->num_ports = 1;

Because pc has default value of "1", this if statement can be simplified.

> +	}
> +	for (p = 0; p < pc; p++) {
> +		struct ark_port *port;
> +
> +		port = &ark->port[p];
> +		struct rte_eth_dev_data *data = NULL;
> +
> +		port->id = p;
> +
> +		char name[RTE_ETH_NAME_MAX_LEN];
> +
> +		snprintf(name, sizeof(name), "arketh%d",
> +			 dev->data->port_id + p);
> +
> +		if (p == 0) {
> +			/* First port is already allocated by DPDK */
> +			port->eth_dev = ark->eth_dev;
> +			continue;
> +		}
> +
> +		/* reserve an ethdev entry */
> +		port->eth_dev = rte_eth_dev_allocate(name);
> +		if (!port->eth_dev) {
> +			PMD_DRV_LOG(ERR,
> +				    "Could not allocate eth_dev for port %d\n",
> +				    p);
> +			goto error;
> +		}
> +
> +		data = rte_zmalloc_socket(name, sizeof(*data), 0, numa_node);
> +		if (!data) {
> +			PMD_DRV_LOG(ERR,
> +				    "Could not allocate eth_dev for port %d\n",
> +				    p);
> +			goto error;
> +		}
> +		data->port_id = ark->eth_dev->data->port_id + p;

"ark->eth_dev->data->port_id" is port_id of the first physical ARK
device, and it looks like each device may have multiple ports and "p" is
the port_id within same device.

>From DPDK point of view, port_id is a global value incremented one by
each eth port, so port_id is a unique value, why adding these two values?

> +		port->eth_dev->data = data;

Why overwriting existing data value?

> +		port->eth_dev->device = &pci_dev->device;
> +		port->eth_dev->data->dev_private = ark;
> +		port->eth_dev->driver = ark->eth_dev->driver;
> +		port->eth_dev->dev_ops = ark->eth_dev->dev_ops;
> +		port->eth_dev->tx_pkt_burst = ark->eth_dev->tx_pkt_burst;
> +		port->eth_dev->rx_pkt_burst = ark->eth_dev->rx_pkt_burst;
> +
> +		rte_eth_copy_pci_info(port->eth_dev, pci_dev);
> +
> +		port->eth_dev->data->mac_addrs =
> +			rte_zmalloc(name, ETHER_ADDR_LEN, 0);
> +		if (!port->eth_dev->data->mac_addrs) {
> +			PMD_DRV_LOG(ERR,
> +				    "Memory allocation for MAC failed!"
> +				    " Exiting.\n");
> +			goto error;
> +		}
> +		ether_addr_copy(&adr,
> +				&port->eth_dev->data->mac_addrs[0]);
> +
> +		if (ark->user_ext.dev_init)
> +			ark->user_data =
> +				ark->user_ext.dev_init(dev, ark->a_bar, p);
> +	}

<...>

>  static int
>  eth_ark_dev_uninit(struct rte_eth_dev *dev)
>  {
> +	struct ark_adapter *ark =
> +		(struct ark_adapter *)dev->data->dev_private;
> +
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>  		return 0;
>  
> +	if (ark->user_ext.dev_uninit)
> +		ark->user_ext.dev_uninit(dev, ark->user_data);
> +
> +	ark_pktgen_uninit(ark->pg);
> +	ark_pktchkr_uninit(ark->pc);
> +
>  	dev->dev_ops = NULL;
>  	dev->rx_pkt_burst = NULL;
>  	dev->tx_pkt_burst = NULL;
> +	if (dev->data->mac_addrs)
> +		rte_free(dev->data->mac_addrs);
> +	if (dev->data)
> +		rte_free(dev->data);
> +

Shouldn't uninit go thorough all ports ("for (p = 0; p < pc; p++) ") and
uninit them all?

>  	return 0;
>  }
>  

<...>

> +/*
> + * The following functions are optional and are directly mapped
> + * from the DPDK PMD ops structure.
> + * Each function if implemented is called after the ARK PMD
> + * implementation executes.
> + */
> +int dev_configure(struct rte_eth_dev *dev, void *user_data);
> +int dev_start(struct rte_eth_dev *dev, void *user_data);
> +void dev_stop(struct rte_eth_dev *dev, void *user_data);
> +void dev_close(struct rte_eth_dev *dev, void *user_data);
> +int link_update(struct rte_eth_dev *dev, int wait_to_complete,
> +	void *user_data);
> +int dev_set_link_up(struct rte_eth_dev *dev, void *user_data);
> +int dev_set_link_down(struct rte_eth_dev *dev, void *user_data);
> +void stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
> +	void *user_data);
> +void stats_reset(struct rte_eth_dev *dev, void *user_data);
> +void mac_addr_add(struct rte_eth_dev *dev,
> +	struct ether_addr *macadr,
> +				  uint32_t index,
> +				  uint32_t pool,
> +				  void *user_data);
> +void mac_addr_remove(struct rte_eth_dev *dev, uint32_t index, void *user_data);
> +void mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
> +	void *user_data);

Where these functions are implemented? Do we need these declerations?

  reply	other threads:[~2017-03-28 14:38 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-23  1:03 [dpdk-dev] [PATCH v4 1/7] net/ark: PMD for Atomic Rules Arkville driver stub 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 [this message]
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
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=c70bfd96-29f7-4f20-8804-4a70b0fc4507@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ed.czeck@atomicrules.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).