DPDK patches and discussions
 help / color / mirror / Atom feed
From: Reshma Pattan <reshma.pattan@intel.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] kdp: add kernel data path kernel module
Date: Mon, 8 Feb 2016 17:14:54 +0000	[thread overview]
Message-ID: <56B8CD0E.5050104@intel.com> (raw)
In-Reply-To: <1453912360-18179-2-git-send-email-ferruh.yigit@intel.com>



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.

> +
> +/*
> + * 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?

> +	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?


> 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?


> 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


> 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?

     Thanks,
     Reshma

  reply	other threads:[~2016-02-08 17:15 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 [this message]
2016-02-09 10:53     ` Ferruh Yigit
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=56B8CD0E.5050104@intel.com \
    --to=reshma.pattan@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@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).