From: Jacek Siuda <jck@semihalf.com>
To: Vincent JARDIN <vincent.jardin@6wind.com>
Cc: Tomasz Duszynski <tdu@semihalf.com>,
Thomas Monjalon <thomas@monjalon.net>,
dev@dpdk.org, Marcin Wojtas <mw@semihalf.com>,
Dmitri Epshtein <dima@marvell.com>,
Natalie Samsonov <nsamsono@marvell.com>,
Jianbo.liu@linaro.org
Subject: Re: [dpdk-dev] [PATCH v4 02/16] net/mrvl: add mrvl net pmd driver skeleton
Date: Thu, 12 Oct 2017 14:14:14 +0200 [thread overview]
Message-ID: <CAD_GOWaNReULvdUUQZxKtcLc_qE31SKn+VvymKm+X8z_n9kaqQ@mail.gmail.com> (raw)
In-Reply-To: <6bfe47de-c5b4-2b01-7991-2ac48913b2c3@6wind.com>
Hi,
The problem is, that both drivers are totally separate entities, linked
into separate libs, but we need to use only one setting in both of them.
So, in order to provide any runtime (or even compile-time) verification, we
would need to create a third entity just to handle a simple assertion - a
bit overkill. At the time the code was written, common configuration option
seemed the most logical. If your long-term goal is to get rid of build-time
configurations, then it puts everything in a different perspective.
What we can do, is to add a run-time driver option and oblige user (in
documentation) to set the option only for the first mrvl vdev. If the
driver doesn't see the option set, it won't initialize DMA. This, plus
meaningful error handling/logs should take care of any potential
mishandling by user that we were afraid of. I hope that would be more
acceptable.
Best Regards,
Jacek Siuda.
2017-10-12 9:59 GMT+02:00 Vincent JARDIN <vincent.jardin@6wind.com>:
> +1 with Thomas, see below,
>
> Le 12/10/2017 à 08:51, Tomasz Duszynski a écrit :
>
>> What is MUSDK_DMA_MEMSIZE?
>>> If the value cannot change, it must be a constant in the code.
>>> If it can change, it should be a run-time driver option.
>>>
>> It's up to the user what MUSDK_DMA_MEMSIZE is going to be. Currently it's
>> set to value that should work it all cases.
>>
>> Except that, MUSDK_DMA_MEMSIZE is used as synchronization point for net
>> and crypto (on condition they are used together i.e ipsec-secgw).
>>
>> Suppose we have two different MUSDK_DMA_MEMSIZE defined for net/crypto
>> then
>> dma memsize allocated will depend on driver probing sequence which might
>> confuse user.
>>
> It does not make sense,
> + /*
> + * 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
>
> int mv_sys_dma_mem_init(u64 size)
> {
> struct sys_dma *i_sys_dma;
> int err;
>
> if (sys_dma) {
> pr_err("Dma object already exits.\n");
> return -EEXIST;
> }
>
> So, I do not understand why you cannot add some checks into the drivers to
> assert that users must have set the same value for both when calling:
> ret = mv_sys_dma_mem_init(my_best_size);
> maybe, you need to fix and improve musdk first to avoid DPDK from getting
> such compilation issues.
>
> best regards,
> Vincent
>
next prev parent reply other threads:[~2017-10-12 12:14 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
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 [this message]
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=CAD_GOWaNReULvdUUQZxKtcLc_qE31SKn+VvymKm+X8z_n9kaqQ@mail.gmail.com \
--to=jck@semihalf.com \
--cc=Jianbo.liu@linaro.org \
--cc=dev@dpdk.org \
--cc=dima@marvell.com \
--cc=mw@semihalf.com \
--cc=nsamsono@marvell.com \
--cc=tdu@semihalf.com \
--cc=thomas@monjalon.net \
--cc=vincent.jardin@6wind.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).