[dpdk-dev] [PATCH v3] net/kni: add KNI PMD
Hi Yong,
Thank you for the review.
On 11/3/2016 1:24 AM, Yong Wang wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Monday, October 10, 2016 6:20 AM
>> To: dev at dpdk.org
>> Cc: Ferruh Yigit
>> Subject: [dpdk-dev] [PATCH v3] net/kni: add KNI PMD
>>
>> Add KNI PMD which wraps librte_kni for ease of use.
>>
>> KNI PMD can be used as any regular PMD to send / receive packets to the
>> Linux networking stack.
>>
>> Signed-off-by: Ferruh Yigit
>> ---
>>
>> v3:
>> * rebase on top of latest master
>>
>> v2:
>> * updated driver name eth_kni -> net_kni
>> ---
<...>
>> CONFIG_RTE_LIBRTE_KNI=n
>> +CONFIG_RTE_LIBRTE_PMD_KNI=n
>
> Nit: change this to CONFIG_RTE_LIBRTE_KNI_PMD instead to be consistent with
> all other pmds.
There is an inconsistency between virtual and physical PMD config options.
Physical ones: xxx_PMD=
*IXGBE_PMD, *I40E_PMD, *ENA_PMD, ...
Virtual ones: PMD_xxx=
*PMD_RING, *PMD_PCAP, *PMD_NULL, ...
So I am consistent with inconsistency J
<...>
>> +#define DRV_NAME net_kni
>
> The name generated this way is not consistent with other vdevs. Why not
> simply assign "KNI PMD" to drv_name?
Right, it is not consistent but intentionaly.
With macro RTE_PMD_REGISTER_VDEV(net_kni, xxx), rte_driver.name set to
"net_kni"
and if you set drivername to "KNI PMD", pmd will report driver name as
"KNI PMD"
so there will be two different driver names, I tried to unify them to a
single name.
And some physical drivers already does same thing.
<...>
>> +static uint16_t
>> +eth_kni_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>> +{
>> +struct pmd_queue *kni_q = q;
>> +struct rte_kni *kni = kni_q->internals->kni;
>> +uint16_t nb_pkts;
>> +
>> +nb_pkts = rte_kni_rx_burst(kni, bufs, nb_bufs);
>> +
>> +kni_q->rx.pkts += nb_pkts;
>> +kni_q->rx.err_pkts += nb_bufs - nb_pkts;
>> +
>> +return nb_pkts;
>> +}
>> +
>
> I don't think it's safe to do receive from two queues concurrently on two
> cores sharing the same underlying KNI device due to the current limitation of
> KNI user-space queues not being multi-thread safe.
You are right, above code is not safe.
It is possible to create a KNI interface per queue, but I don't see any
advantage of this against creating a new virtual KNI port.
So I will limit to single queue.
> Is the proposed plan to have the application layer implement
synchronization logic?
> If that's the case, it needs to be clearly documented and depending on
the implementation, measurable overhead will be incurred.
> Otherwise (only single-queue supported), could you check queue number
if the application tries to configure multi-queue?
>
<...>
>> +static struct rte_eth_dev *
>> +eth_kni_create(const char *name, unsigned int numa_node)
>> +{
>> +struct pmd_internals *internals = NULL;
>> +struct rte_eth_dev_data *data;
>> +struct rte_eth_dev *eth_dev;
>> +uint16_t nb_rx_queues = 1;
>> +uint16_t nb_tx_queues = 1;
>
> Since these two values are always 1 here, I think they could be removed.
I will remove them.
Thanks,
ferruh
[dpdk-dev] [PATCH v3] net/kni: add KNI PMD
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ferruh Yigit > Sent: Monday, October 10, 2016 6:20 AM > To: dev at dpdk.org > Cc: Ferruh Yigit > Subject: [dpdk-dev] [PATCH v3] net/kni: add KNI PMD > > Add KNI PMD which wraps librte_kni for ease of use. > > KNI PMD can be used as any regular PMD to send / receive packets to the > Linux networking stack. > > Signed-off-by: Ferruh Yigit > --- > > v3: > * rebase on top of latest master > > v2: > * updated driver name eth_kni -> net_kni > --- > config/common_base | 1 + > config/common_linuxapp | 1 + > drivers/net/Makefile| 1 + > drivers/net/kni/Makefile| 63 + > drivers/net/kni/rte_eth_kni.c | 463 > > drivers/net/kni/rte_pmd_kni_version.map | 4 + > mk/rte.app.mk | 10 +- > 7 files changed, 538 insertions(+), 5 deletions(-) > create mode 100644 drivers/net/kni/Makefile > create mode 100644 drivers/net/kni/rte_eth_kni.c > create mode 100644 drivers/net/kni/rte_pmd_kni_version.map > > diff --git a/config/common_base b/config/common_base > index f5d2eff..03b93c7 100644 > --- a/config/common_base > +++ b/config/common_base > @@ -543,6 +543,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n > # Compile librte_kni > # > CONFIG_RTE_LIBRTE_KNI=n > +CONFIG_RTE_LIBRTE_PMD_KNI=n Nit: change this to CONFIG_RTE_LIBRTE_KNI_PMD instead to be consistent with all other pmds. > CONFIG_RTE_KNI_KMOD=n > CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > CONFIG_RTE_KNI_KO_DEBUG=n > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 2483dfa..2ecd510 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y > CONFIG_RTE_EAL_VFIO=y > CONFIG_RTE_KNI_KMOD=y > CONFIG_RTE_LIBRTE_KNI=y > +CONFIG_RTE_LIBRTE_PMD_KNI=y > CONFIG_RTE_LIBRTE_VHOST=y > CONFIG_RTE_LIBRTE_PMD_VHOST=y > CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > index bc93230..c4771cd 100644 > --- a/drivers/net/Makefile > +++ b/drivers/net/Makefile > @@ -41,6 +41,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic > DIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k > DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e > DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe > +DIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += kni > DIRS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4 > DIRS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5 > DIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += mpipe > diff --git a/drivers/net/kni/Makefile b/drivers/net/kni/Makefile > new file mode 100644 > index 000..0b7cf91 > --- /dev/null > +++ b/drivers/net/kni/Makefile > @@ -0,0 +1,63 @@ > +# BSD LICENSE > +# > +# Copyright(c) 2016 Intel Corporation. All rights reserved. > +# > +# Redistribution and use in source and binary forms, with or without > +# modification, are permitted provided that the following conditions > +# are met: > +# > +# * Redistributions of source code must retain the above copyright > +# notice, this list of conditions and the following disclaimer. > +# * Redistributions in binary form must reproduce the above copyright > +# notice, this list of conditions and the following disclaimer in > +# the documentation and/or other materials provided with the > +# distribution. > +# * Neither the name of Intel Corporation nor the names of its > +# contributors may be used to endorse or promote products derived > +# from this software without specific prior written permission. > +# > +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS > +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT > NOT > +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND > FITNESS FOR > +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > COPYRIGHT > +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > INCIDENTAL, > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT > NOT > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS > OF USE, > +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED > AND ON ANY > +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR > TORT > +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > THE USE > +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > + > +include $(RTE_SDK)/mk/rte.vars.mk > + > +# > +# library name > +# > +LIB = librte_pmd_kni.a > + > +CFLAGS += -O3
[dpdk-dev] [PATCH v3] net/kni: add KNI PMD
Add KNI PMD which wraps librte_kni for ease of use. KNI PMD can be used as any regular PMD to send / receive packets to the Linux networking stack. Signed-off-by: Ferruh Yigit --- v3: * rebase on top of latest master v2: * updated driver name eth_kni -> net_kni --- config/common_base | 1 + config/common_linuxapp | 1 + drivers/net/Makefile| 1 + drivers/net/kni/Makefile| 63 + drivers/net/kni/rte_eth_kni.c | 463 drivers/net/kni/rte_pmd_kni_version.map | 4 + mk/rte.app.mk | 10 +- 7 files changed, 538 insertions(+), 5 deletions(-) create mode 100644 drivers/net/kni/Makefile create mode 100644 drivers/net/kni/rte_eth_kni.c create mode 100644 drivers/net/kni/rte_pmd_kni_version.map diff --git a/config/common_base b/config/common_base index f5d2eff..03b93c7 100644 --- a/config/common_base +++ b/config/common_base @@ -543,6 +543,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n # Compile librte_kni # CONFIG_RTE_LIBRTE_KNI=n +CONFIG_RTE_LIBRTE_PMD_KNI=n CONFIG_RTE_KNI_KMOD=n CONFIG_RTE_KNI_PREEMPT_DEFAULT=y CONFIG_RTE_KNI_KO_DEBUG=n diff --git a/config/common_linuxapp b/config/common_linuxapp index 2483dfa..2ecd510 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y CONFIG_RTE_EAL_VFIO=y CONFIG_RTE_KNI_KMOD=y CONFIG_RTE_LIBRTE_KNI=y +CONFIG_RTE_LIBRTE_PMD_KNI=y CONFIG_RTE_LIBRTE_VHOST=y CONFIG_RTE_LIBRTE_PMD_VHOST=y CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y diff --git a/drivers/net/Makefile b/drivers/net/Makefile index bc93230..c4771cd 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -41,6 +41,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic DIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe +DIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += kni DIRS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4 DIRS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5 DIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += mpipe diff --git a/drivers/net/kni/Makefile b/drivers/net/kni/Makefile new file mode 100644 index 000..0b7cf91 --- /dev/null +++ b/drivers/net/kni/Makefile @@ -0,0 +1,63 @@ +# BSD LICENSE +# +# Copyright(c) 2016 Intel Corporation. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in +# the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Intel Corporation nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +include $(RTE_SDK)/mk/rte.vars.mk + +# +# library name +# +LIB = librte_pmd_kni.a + +CFLAGS += -O3 +CFLAGS += $(WERROR_FLAGS) +LDLIBS += -lpthread + +EXPORT_MAP := rte_pmd_kni_version.map + +LIBABIVER := 1 + +# +# all source are stored in SRCS-y +# +SRCS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += rte_eth_kni.c + +# +# Export include files +# +SYMLINK-y-include += + +# this lib depends upon: +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_eal +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_ether +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_kni +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_mbuf +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_mempool + +include $(RTE_SDK)/mk/rte.lib.mk diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c new file mode 100644 index 000..ce9e758 --- /dev/null +++ b/drivers/net/kni/rte_eth_kni.c @@ -0,0 +1,463 @@ +/*- + * BSD LICENSE + * + * Copyright(c) 2016 Intel Corporation. All rights reserved. + * All rights reserved. + * + * Redistribution and use in source and
