From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Reshma Pattan <reshma.pattan@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] kdp: add kernel data path kernel module
Date: Tue, 9 Feb 2016 10:53:08 +0000 [thread overview]
Message-ID: <20160209105308.GA30820@sivlogin002.ir.intel.com> (raw)
In-Reply-To: <56B8CD0E.5050104@intel.com>
On Mon, Feb 08, 2016 at 05:14:54PM +0000, Reshma Pattan wrote:
Hi Reshma,
>
>
> On 1/27/2016 4:32 PM, Ferruh Yigit wrote:
>> This kernel module is based on KNI module, but this one is stripped
>> version of it and only for data messages, no control functionality
>> provided.
>>
>> FIFO implementation of the KNI is kept exact same, but ethtool related
>> code removed and virtual network management related code simplified.
>>
>> This module contains kernel support to create network devices and
>> this module has a simple driver for virtual network device, the driver
>> simply puts/gets packets to/from FIFO instead of real hardware.
>>
>> FIFO is created owned by userspace application, which is for this case
>> KDP PMD.
>>
>> In long term this patch intends to replace the KNI and KNI will be
>> depreciated.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kdp_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kdp_common.h
>> new file mode 100644
>> index 0000000..0c77f58
>> --- /dev/null
>> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kdp_common.h
>>
>> +/**
>> + * KDP name is part of memzone name.
>> + */
>> +#define RTE_KDP_NAMESIZE 32
>> +
>> +#ifndef RTE_CACHE_LINE_SIZE
>> +#define RTE_CACHE_LINE_SIZE 64 /**< Cache line size. */
>> +#endif
>
> Jerin Jacob has patch for cleaning of MACRO RTE_CACHE_LINE_SIZE and having
> CONFIG_RTE_CACHE_LINE_SIZE
>
> in config file. You may need to remove this,once those changes are available
> in code.
>
Thanks, when that patch applied, I can rebase code.
>> +
>> +/*
>> + * The kernel image of the rte_mbuf struct, with only the relevant fields.
>> + * Padding is necessary to assure the offsets of these fields
>> + */
>> +struct rte_kdp_mbuf {
>> + void *buf_addr __attribute__((__aligned__(RTE_CACHE_LINE_SIZE)));
>> + char pad0[10];
>> +
>> + /**< Start address of data in segment buffer. */
>> + uint16_t data_off;
>> + char pad1[4];
>> + uint64_t ol_flags; /**< Offload features. */
>
> You are not using ol_flags down in the code. Should this be removed?
>
Can't remove, this struct should match with rte_mbuf
>> + char pad2[4];
>> +
>> + /**< Total pkt len: sum of all segment data_len. */
>> + uint32_t pkt_len;
>> +
>> + /**< Amount of data in segment buffer. */
>> + uint16_t data_len;
>> +
>> + /* fields on second cache line */
>> + char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_SIZE)));
>> + void *pool;
>> + void *next;
>> +};
>> +
>
> Does all structures should have "__rte_cache_aligned" in their declarations?
> Like other DPDK structs?
>
This is kernel module. Doesn't know about userspace library macros.
>
>> diff --git a/lib/librte_eal/linuxapp/kdp/kdp_dev.h b/lib/librte_eal/linuxapp/kdp/kdp_dev.h
>> new file mode 100644
>> index 0000000..52952b4
>> --- /dev/null
>> +++ b/lib/librte_eal/linuxapp/kdp/kdp_dev.h
>>
>> +
>> +#define KDP_ERR(args...) printk(KERN_DEBUG "KDP: Error: " args)
>> +#define KDP_PRINT(args...) printk(KERN_DEBUG "KDP: " args)
>> +
>> +#ifdef RTE_KDP_KO_DEBUG
>> +#define KDP_DBG(args...) printk(KERN_DEBUG "KDP: " args)
>
> Is it good to haveKERN_DEBUG "KDP:Debug: " like Errors?
>
I think extra "Debug" prefix is not required here.
>
>> diff --git a/lib/librte_eal/linuxapp/kdp/kdp_fifo.h b/lib/librte_eal/linuxapp/kdp/kdp_fifo.h
>> new file mode 100644
>> index 0000000..a5fe080
>> --- /dev/null
>> +++ b/lib/librte_eal/linuxapp/kdp/kdp_fifo.h
>>
>> +/**
>> + * Adds num elements into the fifo. Return the number actually written
>> + */
>> +static inline unsigned
>> +kdp_fifo_put(struct rte_kdp_fifo *fifo, void **data, unsigned num)
>> +{
>> + unsigned i = 0;
>> + unsigned fifo_write = fifo->write;
>> + unsigned fifo_read = fifo->read;
>> + unsigned new_write = fifo_write;
>> +
>> + for (i = 0; i < num; i++) {
>> + new_write = (new_write + 1) & (fifo->len - 1);
>> +
>> + if (new_write == fifo_read)
>> + break;
>> + fifo->buffer[fifo_write] = data[i];
>> + fifo_write = new_write;
>> + }
>> + fifo->write = fifo_write;
>> +
>> + return i;
>> +}
>
> you can add header for all function declarations inside header file with
> below format. Same for other header files and functions.
>
> *@Description
>
> *@params
>
> *@Return value
>
This is private header.
>
>> diff --git a/lib/librte_eal/linuxapp/kdp/kdp_misc.c b/lib/librte_eal/linuxapp/kdp/kdp_misc.c
>> new file mode 100644
>> index 0000000..d97d1c0
>> --- /dev/null
>> +++ b/lib/librte_eal/linuxapp/kdp/kdp_misc.c
>> +static int
>> +kdp_compat_ioctl(struct inode *inode, unsigned int ioctl_num,
>> + unsigned long ioctl_param)
>> +{
>> + /* 32 bits app on 64 bits OS to be supported later */
>> + KDP_PRINT("Not implemented.\n");
>
> Should this be warning/ERR instead of PRINT?
>
>> diff --git a/lib/librte_eal/linuxapp/kdp/kdp_net.c b/lib/librte_eal/linuxapp/kdp/kdp_net.c
>> new file mode 100644
>> index 0000000..5c669f5
>> --- /dev/null
>> +++ b/lib/librte_eal/linuxapp/kdp/kdp_net.c
>> +
>> +static void
>> +kdp_net_set_rx_mode(struct net_device *dev)
>> +{
>> +}
>
> Empty function body?
>
Yes, this is part of net_device_ops, and required to fake multicast support.
Regards,
ferruh
next prev parent reply other threads:[~2016-02-09 10:53 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-27 16:32 [dpdk-dev] [PATCH 0/2] slow data path communication between DPDK port and Linux Ferruh Yigit
2016-01-27 16:32 ` [dpdk-dev] [PATCH 1/2] kdp: add kernel data path kernel module Ferruh Yigit
2016-02-08 17:14 ` Reshma Pattan
2016-02-09 10:53 ` Ferruh Yigit [this message]
2016-01-27 16:32 ` [dpdk-dev] [PATCH 2/2] kdp: add virtual PMD for kernel slow data path communication Ferruh Yigit
2016-01-28 8:16 ` Xu, Qian Q
2016-01-29 16:04 ` Yigit, Ferruh
2016-02-09 17:33 ` Reshma Pattan
2016-02-09 17:51 ` Ferruh Yigit
2016-02-19 5:05 ` [dpdk-dev] [PATCH v2 0/2] slow data path communication between DPDK port and Linux Ferruh Yigit
2016-02-19 5:05 ` [dpdk-dev] [PATCH v2 1/2] kdp: add kernel data path kernel module Ferruh Yigit
2016-02-19 5:05 ` [dpdk-dev] [PATCH v2 2/2] kdp: add virtual PMD for kernel slow data path communication Ferruh Yigit
2016-03-09 11:17 ` [dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux Ferruh Yigit
2016-03-09 11:17 ` [dpdk-dev] [PATCH v3 1/2] kdp: add kernel data path kernel module Ferruh Yigit
2016-03-09 11:17 ` [dpdk-dev] [PATCH v3 2/2] kdp: add virtual PMD for kernel slow data path communication Ferruh Yigit
2016-03-14 15:32 ` [dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux Ferruh Yigit
2016-03-16 7:26 ` Panu Matilainen
2016-03-16 8:19 ` Ferruh Yigit
2016-03-16 8:22 ` Panu Matilainen
2016-03-16 10:26 ` Ferruh Yigit
2016-03-16 10:45 ` Thomas Monjalon
2016-03-16 11:07 ` Mcnamara, John
2016-03-16 11:13 ` Ferruh Yigit
2016-03-16 13:23 ` Panu Matilainen
2016-03-16 13:15 ` Panu Matilainen
2016-03-16 13:58 ` Thomas Monjalon
2016-03-16 15:03 ` Panu Matilainen
2016-03-16 15:15 ` Thomas Monjalon
2016-03-16 11:07 ` Bruce Richardson
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=20160209105308.GA30820@sivlogin002.ir.intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=reshma.pattan@intel.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).