DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Tomasz Duszynski <tdu@semihalf.com>, dev@dpdk.org
Cc: mw@semihalf.com, dima@marvell.com, nsamsono@marvell.com,
	Jianbo.liu@linaro.org, Jacek Siuda <jck@semihalf.com>
Subject: Re: [dpdk-dev] [PATCH v3 2/4] net/mrvl: add mrvl net pmd driver
Date: Wed, 4 Oct 2017 01:28:47 +0100	[thread overview]
Message-ID: <eb31547c-ac28-3723-9e7d-89aa653f2720@intel.com> (raw)
In-Reply-To: <1507031500-11473-3-git-send-email-tdu@semihalf.com>

On 10/3/2017 12:51 PM, Tomasz Duszynski wrote:
> Add support for the Marvell PPv2 (Packet Processor v2) 1/10 Gbps adapter.
> Driver is based on external, publicly available, light-weight Marvell
> MUSDK library that provides access to network packet processor.
> 
> Driver comes with support for the following features:
> 
> * Speed capabilities
> * Link status
> * Queue start/stop
> * MTU update
> * Jumbo frame
> * Promiscuous mode
> * Allmulticast mode
> * Unicast MAC filter
> * Multicast MAC filter
> * RSS hash
> * VLAN filter
> * CRC offload
> * L3 checksum offload
> * L4 checksum offload
> * Packet type parsing
> * Basic stats
> * Stats per queue
> 
> Driver was engineered cooperatively by Semihalf and Marvell teams.
> 
> Semihalf:
> Jacek Siuda <jck@semihalf.com>
> Tomasz Duszynski <tdu@semihalf.com>
> 
> Marvell:
> Dmitri Epshtein <dima@marvell.com>
> Natalie Samsonov <nsamsono@marvell.com>
> 
> Signed-off-by: Jacek Siuda <jck@semihalf.com>
> Signed-off-by: Tomasz Duszynski <tdu@semihalf.com>

<...>

> +++ b/config/common_base
> @@ -262,6 +262,13 @@ CONFIG_RTE_LIBRTE_NFP_PMD=n
>  CONFIG_RTE_LIBRTE_NFP_DEBUG=n
> 
>  #
> +# Compile Marvell PMD driver
> +#
> +CONFIG_RTE_LIBRTE_MRVL_PMD=n
> +CONFIG_RTE_LIBRTE_MRVL_DEBUG=n
> +CONFIG_RTE_MRVL_MUSDK_DMA_MEMSIZE=41943040

Is dma memsize needs to be a configuration option?

<...>

> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +ifneq ($(MAKECMDGOALS),clean)
> +ifneq ($(MAKECMDGOALS),config)
> +ifeq ($(LIBMUSDK_PATH),)
> +$(error "Please define LIBMUSDK_PATH environment variable")

Not sure how to resolve this dependency.
What do you think adding this as configuration option?

Or DPDK just adds the -lmusdk external dependency and while compiling
for marvel EXTRA_LDFLAGS parameter should be pass with
"-L$(LIBMUSDK_PATH)" and this can be documented in marvel doc. What do
you think?

> +endif
> +ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),n)
> +$(error "RTE_LIBRTE_CFGFILE must be enabled in configuration!")

This can be also handled in drivers/net/Makefile, it can be possible to
add check there for LIBRTE_CFGFILE dependency.

> +endif
> +endif
> +endif
> +
> +# library name
> +LIB = librte_pmd_mrvl.a
> +
> +# library version
> +LIBABIVER := 1
> +
> +# versioning export map
> +EXPORT_MAP := rte_pmd_mrvl_version.map
> +
> +# external library dependencies
> +CFLAGS += -I$(LIBMUSDK_PATH)/include
> +CFLAGS += -DMVCONF_ARCH_DMA_ADDR_T_64BIT
> +CFLAGS += -DCONF_PP2_BPOOL_COOKIE_SIZE=32
> +CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -O3
> +LDLIBS += -L$(LIBMUSDK_PATH)/lib

This can be LDFLAGS instead of LDLIBS

> +LDLIBS += -lmusdk
> +
> +# library source files
> +SRCS-$(CONFIG_RTE_LIBRTE_MRVL_PMD) += mrvl_ethdev.c
> +SRCS-$(CONFIG_RTE_LIBRTE_MRVL_PMD) += mrvl_qos.c
> +
> +# library dependencies
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_MRVL_PMD) += lib/librte_cfgfile

These variables no more used, you can drop this. drivers/net/Makefile
used for this, you are already updating that file, librte_cfgfile needs
to be added there.

> +
> +include $(RTE_SDK)/mk/rte.lib.mk

<...>

> +/*
> + * To use buffer harvesting based on loopback port shadow queue structure
> + * was introduced for buffers information bookkeeping.
> + *
> + * Before sending the packet, related buffer information (pp2_buff_inf) is
> + * stored in shadow queue. After packet is transmitted no longer used
> + * packet buffer is released back to it's original hardware pool,
> + * on condition it originated from interface.
> + * In case it  was generated by application itself i.e: mbuf->port field is
> + * 0xff then its released to software mempool.

You already explained here but can you please give more details why
shadow queue needed?

> + */
> +struct mrvl_shadow_txq {
> +	int head;           /* write index - used when sending buffers */
> +	int tail;           /* read index - used when releasing buffers */
> +	u16 size;           /* queue occupied size */
> +	u16 num_to_release; /* number of buffers sent, that can be released */
> +	struct buff_release_entry ent[MRVL_PP2_TX_SHADOWQ_SIZE]; /* q entries */
> +};
> +
> +struct mrvl_rxq {
> +	struct mrvl_priv *priv;
> +	struct rte_mempool *mp;
> +	int queue_id;
> +	int port_id;
> +	int cksum_enabled;
> +	uint64_t bytes_recv;
> +	uint64_t drop_mac;
> +};
> +
> +struct mrvl_txq {
> +	struct mrvl_priv *priv;
> +	int queue_id;
> +	int port_id;
> +	uint64_t bytes_sent;
> +};
> +

<...>

> +static int
> +mrvl_dev_start(struct rte_eth_dev *dev)
> +{
> +	struct mrvl_priv *priv = dev->data->dev_private;
> +	char match[MRVL_MATCH_LEN];
> +	int ret;
> +
> +	snprintf(match, sizeof(match), "ppio-%d:%d",
> +		 priv->pp_id, priv->ppio_id);
> +	priv->ppio_params.match = match;

Why this match is used, just a reminder that match is only valid for the
scope of this function, after this function it will be invalid.

<...>

> +
> +	if (rte_spinlock_trylock(&q->priv->lock) == 1) {

Why getting lock in Rx data path?

> +		num = mrvl_get_bpool_size(bpool->pp2_id, bpool->id);
> +
> +		if (unlikely(num <= q->priv->bpool_min_size ||
> +			     (!rx_done && num < q->priv->bpool_init_size))) {
> +			ret = mrvl_fill_bpool(q, MRVL_BURST_SIZE);
> +			if (ret)
> +				RTE_LOG(ERR, PMD, "Failed to fill bpool\n");
> +		} else if (unlikely(num > q->priv->bpool_max_size)) {
> +			int i;
> +			int pkt_to_remove = num - q->priv->bpool_init_size;
> +			struct rte_mbuf *mbuf;
> +			struct pp2_buff_inf buff;
> +
> +			RTE_LOG(DEBUG, PMD,
> +				"\nport-%d:%d: bpool %d oversize - remove %d buffers (pool size: %d -> %d)\n",
> +				bpool->pp2_id, q->priv->ppio->port_id,
> +				bpool->id, pkt_to_remove, num,
> +				q->priv->bpool_init_size);
> +
> +			for (i = 0; i < pkt_to_remove; i++) {
> +				pp2_bpool_get_buff(hifs[core_id], bpool, &buff);
> +				mbuf = (struct rte_mbuf *)
> +					(cookie_addr_high | buff.cookie);
> +				rte_pktmbuf_free(mbuf);
> +			}
> +			mrvl_port_bpool_size
> +				[bpool->pp2_id][bpool->id][core_id] -=
> +								pkt_to_remove;
> +		}
> +		rte_spinlock_unlock(&q->priv->lock);
> +	}
> +
> +	return rx_done;
> +}

<...>

> +	cfgnum = rte_kvargs_count(kvlist, MRVL_CFG_ARG);
> +	if (cfgnum > 1) {
> +		RTE_LOG(ERR, PMD, "Cannot handle more than one config file!\n");
> +		goto out_free_kvlist;
> +	} else if (cfgnum == 1) {
> +		rte_kvargs_process(kvlist, MRVL_CFG_ARG,
> +				   mrvl_get_qoscfg, &mrvl_qos_cfg);

Is the expected format/contect of the config file documented? How one
can know how to create a config file?

> +	}
> +
> +	/*
> +	 * ret == -EEXIST is correct, it means DMA
> +	 * has been already initialized (by another PMD).
> +	 */
> +	ret = mv_sys_dma_mem_init(RTE_MRVL_MUSDK_DMA_MEMSIZE);
> +	if (ret < 0 && ret != -EEXIST)
> +		goto out_free_kvlist;
> +
> +	ret = mrvl_init_pp2();
> +	if (ret) {
> +		RTE_LOG(ERR, PMD, "Failed to init PP!\n");
> +		goto out_deinit_dma;
> +	}
> +
> +	ret = mrvl_init_hifs();
> +	if (ret)
> +		goto out_deinit_hifs;
> +
> +	for (i = 0; i < ifnum; i++) {
> +		RTE_LOG(INFO, PMD, "Creating %s\n", ifnames[i]);
> +		ret = mrvl_eth_dev_create(vdev, ifnames[i]);

So you are supporting multiple ethdev devices created by single vdev
device, by providing multiple "iface" argument in device args.

This will cause eal create single virtual device but driver create
multiple ethdev devices. I don't see direct problem with this but lets
think about it.
This can be problem if you want to provide ethdev specific device
arguments. Perhaps that is why you need to provide a config file ?

It can be an option to define each ethdev with:
"--vdev net_mrvl0,iface=xx0,config=yy0 --vdev
net_mrlv1,iface=xx1,config=yy1 ..."

This may remove your dependecy to librte_cfgfile.

> +		if (ret)
> +			goto out_cleanup;
> +	}
> +
> +	rte_kvargs_free(kvlist);
> +
> +	memset(mrvl_port_bpool_size, 0, sizeof(mrvl_port_bpool_size));
> +
> +	mrvl_lcore_first = RTE_MAX_LCORE;
> +	mrvl_lcore_last = 0;
> +
> +	RTE_LCORE_FOREACH(core_id) {
> +		mrvl_set_first_last_cores(core_id);

This sets limits of core_id. Why you need to know this in PMD level?

<...>

  parent reply	other threads:[~2017-10-04  0:28 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-26  9:39 [dpdk-dev] [PATCH 0/8] add net/crypto mrvl pmd drivers Tomasz Duszynski
2017-09-26  9:39 ` [dpdk-dev] [PATCH 1/8] app: link the whole rte_cfgfile library Tomasz Duszynski
2017-09-26 14:31   ` Bruce Richardson
2017-09-27  7:36     ` Tomasz Duszynski
2017-09-27 12:01       ` Bruce Richardson
2017-09-26  9:39 ` [dpdk-dev] [PATCH 2/8] net/mrvl: add mrvl net pmd driver Tomasz Duszynski
2017-09-26  9:40 ` [dpdk-dev] [PATCH 3/8] doc: add mrvl net pmd documentation Tomasz Duszynski
2017-09-26  9:40 ` [dpdk-dev] [PATCH 4/8] maintainers: add maintainers for the mrvl net pmd Tomasz Duszynski
2017-09-26  9:40 ` [dpdk-dev] [PATCH 5/8] crypto/mrvl: add mrvl crypto pmd driver Tomasz Duszynski
2017-09-26  9:40 ` [dpdk-dev] [PATCH 6/8] doc: add mrvl crypto pmd documentation Tomasz Duszynski
2017-09-26  9:40 ` [dpdk-dev] [PATCH 7/8] maintainers: add maintainers for the mrvl crypto pmd Tomasz Duszynski
2017-09-26  9:40 ` [dpdk-dev] [PATCH 8/8] test: add mrvl crypto pmd unit tests Tomasz Duszynski
2017-09-27  9:40 ` [dpdk-dev] [PATCH 0/8] add net/crypto mrvl pmd drivers De Lara Guarch, Pablo
2017-09-27 10:59   ` Tomasz Duszynski
2017-09-28 10:22 ` [dpdk-dev] [PATCH v2 0/4] add net mrvl pmd driver Tomasz Duszynski
2017-09-28 10:22   ` [dpdk-dev] [PATCH v2 1/4] app: link the whole rte_cfgfile library Tomasz Duszynski
2017-10-03 11:51     ` [dpdk-dev] [PATCH v3 0/4] add net mrvl pmd driver Tomasz Duszynski
2017-10-03 11:51       ` [dpdk-dev] [PATCH v3 1/4] app: link the whole rte_cfgfile library Tomasz Duszynski
2017-10-03 11:51       ` [dpdk-dev] [PATCH v3 2/4] net/mrvl: add mrvl net pmd driver Tomasz Duszynski
2017-10-04  0:24         ` Ferruh Yigit
2017-10-04  8:59           ` Tomasz Duszynski
2017-10-04 16:59             ` Ferruh Yigit
2017-10-05  8:43               ` Tomasz Duszynski
2017-10-05 17:29                 ` Ferruh Yigit
2017-10-06  6:41                   ` Tomasz Duszynski
2017-10-10 21:25                 ` Thomas Monjalon
2017-10-04  0:28         ` Ferruh Yigit [this message]
2017-10-04 13:19           ` Tomasz Duszynski
2017-10-05 17:37             ` Ferruh Yigit
2017-10-03 11:51       ` [dpdk-dev] [PATCH v3 3/4] doc: add mrvl net pmd documentation Tomasz Duszynski
2017-10-04  0:29         ` Ferruh Yigit
2017-10-04  7:53           ` Tomasz Duszynski
2017-10-03 11:51       ` [dpdk-dev] [PATCH v3 4/4] maintainers: add maintainers for the mrvl net pmd Tomasz Duszynski
2017-10-09 15:00       ` [dpdk-dev] [PATCH v4 00/16] add net mrvl pmd driver Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 01/16] app: link the whole rte_cfgfile library Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 02/16] net/mrvl: add mrvl net pmd driver skeleton Tomasz Duszynski
2017-10-11 13:38           ` Thomas Monjalon
2017-10-12  6:51             ` Tomasz Duszynski
2017-10-12  7:59               ` Vincent JARDIN
2017-10-12 12:14                 ` Jacek Siuda
2017-10-12 13:57                   ` Vincent JARDIN
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 03/16] net/mrvl: add rx/tx support Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 04/16] net/mrvl: add link update Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 05/16] net/mrvl: add link speed capabilities Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 06/16] net/mrvl: add support for updating mtu Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 07/16] net/mrvl: add jumbo frame support Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 08/16] net/mrvl: add support for promiscuous and allmulticast modes Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 09/16] net/mrvl: add support for mac filtering Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 10/16] net/mrvl: add rss hashing support Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 11/16] net/mrvl: add support for vlan filtering Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 12/16] net/mrvl: add crc, l3 and l4 offloads support Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 13/16] net/mrvl: add packet type parsing support Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 14/16] net/mrvl: add basic stats support Tomasz Duszynski
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 15/16] maintainers: add maintainers for the mrvl net pmd Tomasz Duszynski
2017-10-09 19:02           ` Ferruh Yigit
2017-10-09 19:09             ` Marcin Wojtas
2017-10-09 19:12               ` Ferruh Yigit
2017-10-10  5:47             ` Tomasz Duszynski
2017-10-10  3:20           ` Jianbo Liu
2017-10-10  4:38             ` Ferruh Yigit
2017-10-09 15:00         ` [dpdk-dev] [PATCH v4 16/16] doc: add mrvl net pmd documentation Tomasz Duszynski
2017-10-09 20:54           ` Ferruh Yigit
2017-10-10  5:51             ` Tomasz Duszynski
2017-10-11 14:27           ` Thomas Monjalon
2017-10-12  2:01             ` Thomas Monjalon
2017-10-09 20:59         ` [dpdk-dev] [PATCH v4 00/16] add net mrvl pmd driver Ferruh Yigit
2017-10-10  0:25           ` Ferruh Yigit
2017-10-10  7:07             ` Tomasz Duszynski
2017-10-10  5:55           ` Tomasz Duszynski
2017-10-12  1:51           ` Ferruh Yigit
2017-10-12  2:37             ` [dpdk-dev] [PATCH] doc: add build steps to mrvl NIC guide Thomas Monjalon
2017-10-12  6:28               ` Tomasz Duszynski
2017-10-12  8:02                 ` Thomas Monjalon
2017-10-12  6:07             ` [dpdk-dev] [PATCH v4 00/16] add net mrvl pmd driver Tomasz Duszynski
2017-09-28 10:22   ` [dpdk-dev] [PATCH v2 2/4] net/mrvl: add mrvl net " Tomasz Duszynski
2017-09-29 15:38     ` Stephen Hemminger
2017-10-02 11:08       ` Bruce Richardson
2017-10-03  6:33         ` Tomasz Duszynski
2017-10-03  6:23       ` Tomasz Duszynski
2017-09-28 10:22   ` [dpdk-dev] [PATCH v2 3/4] doc: add mrvl net pmd documentation Tomasz Duszynski
2017-09-28 10:22   ` [dpdk-dev] [PATCH v2 4/4] maintainers: add maintainers for the mrvl net pmd Tomasz Duszynski
2017-09-28 11:06   ` [dpdk-dev] [PATCH v2 0/4] add net mrvl pmd driver Ferruh Yigit
2017-09-28 11:40     ` Amit Tomer
2017-09-28 12:01       ` Marcin Wojtas
2017-09-28 10:23 ` [dpdk-dev] [PATCH v2 0/4] add crypto " Tomasz Duszynski
2017-09-28 10:23   ` [dpdk-dev] [PATCH v2 1/4] crypto/mrvl: add mrvl crypto " Tomasz Duszynski
2017-10-05 15:01     ` De Lara Guarch, Pablo
2017-10-06  7:24       ` Tomasz Duszynski
2017-10-05 15:47     ` Bruce Richardson
2017-10-06  7:05       ` Tomasz Duszynski
2017-09-28 10:23   ` [dpdk-dev] [PATCH v2 2/4] doc: add mrvl crypto pmd documentation Tomasz Duszynski
2017-10-05 14:45     ` De Lara Guarch, Pablo
2017-10-06  8:06       ` Tomasz Duszynski
2017-09-28 10:23   ` [dpdk-dev] [PATCH v2 3/4] maintainers: add maintainers for the mrvl crypto pmd Tomasz Duszynski
2017-09-28 10:23   ` [dpdk-dev] [PATCH v2 4/4] test: add mrvl crypto pmd unit tests Tomasz Duszynski
2017-10-07 20:28   ` [dpdk-dev] [PATCH v3 0/4] add crypto mrvl pmd driver Tomasz Duszynski
2017-10-07 20:28     ` [dpdk-dev] [PATCH v3 1/4] crypto/mrvl: add mrvl crypto " Tomasz Duszynski
2017-10-10 10:16       ` De Lara Guarch, Pablo
2017-10-10 10:25         ` Tomasz Duszynski
2017-10-07 20:28     ` [dpdk-dev] [PATCH v3 2/4] doc: add mrvl crypto pmd documentation Tomasz Duszynski
2017-10-07 20:28     ` [dpdk-dev] [PATCH v3 3/4] maintainers: add maintainers for the mrvl crypto pmd Tomasz Duszynski
2017-10-07 20:28     ` [dpdk-dev] [PATCH v3 4/4] test: add mrvl crypto pmd unit tests Tomasz Duszynski
2017-10-10 10:44       ` De Lara Guarch, Pablo
2017-10-10 10:57         ` Tomasz Duszynski
2017-10-10 12:17     ` [dpdk-dev] [PATCH v4 0/4] add crypto mrvl pmd driver Tomasz Duszynski
2017-10-10 12:17       ` [dpdk-dev] [PATCH v4 1/4] crypto/mrvl: add mrvl crypto " Tomasz Duszynski
2017-10-10 12:17       ` [dpdk-dev] [PATCH v4 2/4] doc: add mrvl crypto pmd documentation Tomasz Duszynski
2017-10-10 12:17       ` [dpdk-dev] [PATCH v4 3/4] maintainers: add maintainers for the mrvl crypto pmd Tomasz Duszynski
2017-10-10 12:17       ` [dpdk-dev] [PATCH v4 4/4] test: add mrvl crypto pmd unit tests Tomasz Duszynski
2017-10-12 12:11       ` [dpdk-dev] [PATCH v4 0/4] add crypto mrvl pmd driver De Lara Guarch, Pablo

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=eb31547c-ac28-3723-9e7d-89aa653f2720@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=Jianbo.liu@linaro.org \
    --cc=dev@dpdk.org \
    --cc=dima@marvell.com \
    --cc=jck@semihalf.com \
    --cc=mw@semihalf.com \
    --cc=nsamsono@marvell.com \
    --cc=tdu@semihalf.com \
    /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).