[dpdk-dev] [PATCH 1/3] kni: minor opto

[dpdk-dev] [PATCH 1/3] kni: minor opto

[Jay Rolette]

Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it

Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
 lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 1935d32..312f196 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -213,13 +213,12 @@ static int
 kni_thread_single(void *unused)
 {
 	int j;
-	struct kni_dev *dev, *n;
+	struct kni_dev *dev;
 
 	while (!kthread_should_stop()) {
 		down_read(&kni_list_lock);
 		for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
-			list_for_each_entry_safe(dev, n,
-					&kni_list_head, list) {
+			list_for_each_entry(dev, &kni_list_head, list) {
 #ifdef RTE_KNI_VHOST
 				kni_chk_vhost_rx(dev);
 #else
-- 
2.3.2 (Apple Git-55)

[dpdk-dev] [PATCH 2/3] kni: minor opto

[Jay Rolette]

No reason to check out many entries are in kni->rx_q prior to
actually pulling them from the fifo. You can't dequeue more than
are there anyway. Max entries to dequeue is either the max batch
size or however much space is available on kni->free_q (lesser of the two)

Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
 lib/librte_eal/linuxapp/kni/kni_net.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index dd95db5..13ccbb8 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -131,7 +131,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 {
 	unsigned ret;
 	uint32_t len;
-	unsigned i, num, num_rq, num_fq;
+	unsigned i, num, num_fq;
 	struct rte_kni_mbuf *kva;
 	struct rte_kni_mbuf *va[MBUF_BURST_SZ];
 	void * data_kva;
@@ -139,24 +139,19 @@ kni_net_rx_normal(struct kni_dev *kni)
 	struct sk_buff *skb;
 	struct net_device *dev = kni->net_dev;
 
-	/* Get the number of entries in rx_q */
-	num_rq = kni_fifo_count(kni->rx_q);
-
 	/* Get the number of free entries in free_q */
-	num_fq = kni_fifo_free_count(kni->free_q);
-
-	/* Calculate the number of entries to dequeue in rx_q */
-	num = min(num_rq, num_fq);
-	num = min(num, (unsigned)MBUF_BURST_SZ);
-
-	/* Return if no entry in rx_q and no free entry in free_q */
-	if (num == 0)
+	if ((num_fq = kni_fifo_free_count(kni->free_q)) == 0) {
+		/* No room on the free_q, bail out */
 		return;
+	}
+
+	/* Calculate the number of entries to dequeue from rx_q */
+	num = min(num_fq, (unsigned)MBUF_BURST_SZ);
 
 	/* Burst dequeue from rx_q */
 	ret = kni_fifo_get(kni->rx_q, (void **)va, num);
 	if (ret == 0)
-		return; /* Failing should not happen */
+		return;
 
 	/* Transfer received packets to netif */
 	for (i = 0; i < num; i++) {
-- 
2.3.2 (Apple Git-55)

Re: [dpdk-dev] [PATCH 2/3] kni: minor opto

[Jay Rolette]

Some sort of hiccup sending. Not sure why 2/3 didn't come out as expected.
I'll try sending again.

Jay

On Wed, Jun 3, 2015 at 2:07 PM, Jay Rolette <rolette@infiniteio.com> wrote:

> No reason to check out many entries are in kni->rx_q prior to
> actually pulling them from the fifo. You can't dequeue more than
> are there anyway. Max entries to dequeue is either the max batch
> size or however much space is available on kni->free_q (lesser of the two)
>
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> ---
>  lib/librte_eal/linuxapp/kni/kni_net.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index dd95db5..13ccbb8 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -131,7 +131,7 @@ kni_net_rx_normal(struct kni_dev *kni)
>  {
>         unsigned ret;
>         uint32_t len;
> -       unsigned i, num, num_rq, num_fq;
> +       unsigned i, num, num_fq;
>         struct rte_kni_mbuf *kva;
>         struct rte_kni_mbuf *va[MBUF_BURST_SZ];
>         void * data_kva;
> @@ -139,24 +139,19 @@ kni_net_rx_normal(struct kni_dev *kni)
>         struct sk_buff *skb;
>         struct net_device *dev = kni->net_dev;
>
> -       /* Get the number of entries in rx_q */
> -       num_rq = kni_fifo_count(kni->rx_q);
> -
>         /* Get the number of free entries in free_q */
> -       num_fq = kni_fifo_free_count(kni->free_q);
> -
> -       /* Calculate the number of entries to dequeue in rx_q */
> -       num = min(num_rq, num_fq);
> -       num = min(num, (unsigned)MBUF_BURST_SZ);
> -
> -       /* Return if no entry in rx_q and no free entry in free_q */
> -       if (num == 0)
> +       if ((num_fq = kni_fifo_free_count(kni->free_q)) == 0) {
> +               /* No room on the free_q, bail out */
>                 return;
> +       }
> +
> +       /* Calculate the number of entries to dequeue from rx_q */
> +       num = min(num_fq, (unsigned)MBUF_BURST_SZ);
>
>         /* Burst dequeue from rx_q */
>         ret = kni_fifo_get(kni->rx_q, (void **)va, num);
>         if (ret == 0)
> -               return; /* Failing should not happen */
> +               return;
>
>         /* Transfer received packets to netif */
>         for (i = 0; i < num; i++) {
> --
> 2.3.2 (Apple Git-55)
>
>

Re: [dpdk-dev] [PATCH 2/3] kni: minor opto

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette
> Sent: Thursday, June 4, 2015 3:08 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/3] kni: minor opto
> 
> No reason to check out many entries are in kni->rx_q prior to actually pulling
> them from the fifo. You can't dequeue more than are there anyway. Max entries
> to dequeue is either the max batch size or however much space is available on
> kni->free_q (lesser of the two)
> 
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

> ---
>  lib/librte_eal/linuxapp/kni/kni_net.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index dd95db5..13ccbb8 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -131,7 +131,7 @@ kni_net_rx_normal(struct kni_dev *kni)  {
>  	unsigned ret;
>  	uint32_t len;
> -	unsigned i, num, num_rq, num_fq;
> +	unsigned i, num, num_fq;
>  	struct rte_kni_mbuf *kva;
>  	struct rte_kni_mbuf *va[MBUF_BURST_SZ];
>  	void * data_kva;
> @@ -139,24 +139,19 @@ kni_net_rx_normal(struct kni_dev *kni)
>  	struct sk_buff *skb;
>  	struct net_device *dev = kni->net_dev;
> 
> -	/* Get the number of entries in rx_q */
> -	num_rq = kni_fifo_count(kni->rx_q);
> -
>  	/* Get the number of free entries in free_q */
> -	num_fq = kni_fifo_free_count(kni->free_q);
> -
> -	/* Calculate the number of entries to dequeue in rx_q */
> -	num = min(num_rq, num_fq);
> -	num = min(num, (unsigned)MBUF_BURST_SZ);
> -
> -	/* Return if no entry in rx_q and no free entry in free_q */
> -	if (num == 0)
> +	if ((num_fq = kni_fifo_free_count(kni->free_q)) == 0) {
> +		/* No room on the free_q, bail out */
>  		return;
> +	}
> +
> +	/* Calculate the number of entries to dequeue from rx_q */
> +	num = min(num_fq, (unsigned)MBUF_BURST_SZ);
> 
>  	/* Burst dequeue from rx_q */
>  	ret = kni_fifo_get(kni->rx_q, (void **)va, num);
>  	if (ret == 0)
> -		return; /* Failing should not happen */
> +		return;
> 
>  	/* Transfer received packets to netif */
>  	for (i = 0; i < num; i++) {
> --
> 2.3.2 (Apple Git-55)

[dpdk-dev] [PATCH 3/3] kni: rx loop was using the wrong counter

[Jay Rolette]

Loop processing packets dequeued from rx_q was using the number of
packets requested, not how many it actually received.

Variable rename to make code a little more clear

Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
 lib/librte_eal/linuxapp/kni/kni_net.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index 13ccbb8..d37a6b9 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -131,7 +131,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 {
 	unsigned ret;
 	uint32_t len;
-	unsigned i, num, num_fq;
+	unsigned i, num_rx, num_fq;
 	struct rte_kni_mbuf *kva;
 	struct rte_kni_mbuf *va[MBUF_BURST_SZ];
 	void * data_kva;
@@ -146,15 +146,15 @@ kni_net_rx_normal(struct kni_dev *kni)
 	}
 
 	/* Calculate the number of entries to dequeue from rx_q */
-	num = min(num_fq, (unsigned)MBUF_BURST_SZ);
+	num_rx = min(num_fq, (unsigned)MBUF_BURST_SZ);
 
 	/* Burst dequeue from rx_q */
-	ret = kni_fifo_get(kni->rx_q, (void **)va, num);
-	if (ret == 0)
+	num_rx = kni_fifo_get(kni->rx_q, (void **)va, num_rx);
+	if (num_rx == 0)
 		return;
 
 	/* Transfer received packets to netif */
-	for (i = 0; i < num; i++) {
+	for (i = 0; i < num_rx; i++) {
 		kva = (void *)va[i] - kni->mbuf_va + kni->mbuf_kva;
 		len = kva->data_len;
 		data_kva = kva->buf_addr + kva->data_off - kni->mbuf_va
@@ -184,8 +184,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 	}
 
 	/* Burst enqueue mbufs into free_q */
-	ret = kni_fifo_put(kni->free_q, (void **)va, num);
-	if (ret != num)
+	ret = kni_fifo_put(kni->free_q, (void **)va, num_rx);
+	if (ret != num_rx)
 		/* Failing should not happen */
 		KNI_ERR("Fail to enqueue entries into free_q\n");
 }
-- 
2.3.2 (Apple Git-55)

Re: [dpdk-dev] [PATCH 1/3] kni: minor opto

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette
> Sent: Thursday, June 4, 2015 3:08 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/3] kni: minor opto
> 
> Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from
> the list as you iterate over it
> 
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

> ---
>  lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
> b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 1935d32..312f196 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -213,13 +213,12 @@ static int
>  kni_thread_single(void *unused)
>  {
>  	int j;
> -	struct kni_dev *dev, *n;
> +	struct kni_dev *dev;
> 
>  	while (!kthread_should_stop()) {
>  		down_read(&kni_list_lock);
>  		for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
> -			list_for_each_entry_safe(dev, n,
> -					&kni_list_head, list) {
> +			list_for_each_entry(dev, &kni_list_head, list) {
>  #ifdef RTE_KNI_VHOST
>  				kni_chk_vhost_rx(dev);
>  #else
> --
> 2.3.2 (Apple Git-55)

[dpdk-dev] [PATCH 1/3] kni: minor opto

[Jay Rolette]

Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it

Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
 lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 1935d32..312f196 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -213,13 +213,12 @@ static int
 kni_thread_single(void *unused)
 {
 	int j;
-	struct kni_dev *dev, *n;
+	struct kni_dev *dev;
 
 	while (!kthread_should_stop()) {
 		down_read(&kni_list_lock);
 		for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
-			list_for_each_entry_safe(dev, n,
-					&kni_list_head, list) {
+			list_for_each_entry(dev, &kni_list_head, list) {
 #ifdef RTE_KNI_VHOST
 				kni_chk_vhost_rx(dev);
 #else
-- 
2.3.2 (Apple Git-55)

[dpdk-dev] [PATCH 2/3] kni: minor opto

[Jay Rolette]

No reason to check out many entries are in kni->rx_q prior to
actually pulling them from the fifo. You can't dequeue more than
are there anyway. Max entries to dequeue is either the max batch
size or however much space is available on kni->free_q (lesser of the two)

Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
 lib/librte_eal/linuxapp/kni/kni_net.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index dd95db5..13ccbb8 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -131,7 +131,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 {
 	unsigned ret;
 	uint32_t len;
-	unsigned i, num, num_rq, num_fq;
+	unsigned i, num, num_fq;
 	struct rte_kni_mbuf *kva;
 	struct rte_kni_mbuf *va[MBUF_BURST_SZ];
 	void * data_kva;
@@ -139,24 +139,19 @@ kni_net_rx_normal(struct kni_dev *kni)
 	struct sk_buff *skb;
 	struct net_device *dev = kni->net_dev;
 
-	/* Get the number of entries in rx_q */
-	num_rq = kni_fifo_count(kni->rx_q);
-
 	/* Get the number of free entries in free_q */
-	num_fq = kni_fifo_free_count(kni->free_q);
-
-	/* Calculate the number of entries to dequeue in rx_q */
-	num = min(num_rq, num_fq);
-	num = min(num, (unsigned)MBUF_BURST_SZ);
-
-	/* Return if no entry in rx_q and no free entry in free_q */
-	if (num == 0)
+	if ((num_fq = kni_fifo_free_count(kni->free_q)) == 0) {
+		/* No room on the free_q, bail out */
 		return;
+	}
+
+	/* Calculate the number of entries to dequeue from rx_q */
+	num = min(num_fq, (unsigned)MBUF_BURST_SZ);
 
 	/* Burst dequeue from rx_q */
 	ret = kni_fifo_get(kni->rx_q, (void **)va, num);
 	if (ret == 0)
-		return; /* Failing should not happen */
+		return;
 
 	/* Transfer received packets to netif */
 	for (i = 0; i < num; i++) {
-- 
2.3.2 (Apple Git-55)

Re: [dpdk-dev] [PATCH 2/3] kni: minor opto

[Bruce Richardson]

On Wed, Jun 03, 2015 at 02:18:56PM -0500, Jay Rolette wrote:
> No reason to check out many entries are in kni->rx_q prior to
> actually pulling them from the fifo. You can't dequeue more than
> are there anyway. Max entries to dequeue is either the max batch
> size or however much space is available on kni->free_q (lesser of the two)
> 
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>

Again title needs to be made clearer. Otherwise ok.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 2/3] kni: minor opto

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette
> Sent: Thursday, June 4, 2015 3:19 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/3] kni: minor opto
The title is not so descriptive for the changes.
The change itself seems OK for me. Thanks!

- Helin

> 
> No reason to check out many entries are in kni->rx_q prior to actually pulling
> them from the fifo. You can't dequeue more than are there anyway. Max entries
> to dequeue is either the max batch size or however much space is available on
> kni->free_q (lesser of the two)
> 
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> ---
>  lib/librte_eal/linuxapp/kni/kni_net.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index dd95db5..13ccbb8 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -131,7 +131,7 @@ kni_net_rx_normal(struct kni_dev *kni)  {
>  	unsigned ret;
>  	uint32_t len;
> -	unsigned i, num, num_rq, num_fq;
> +	unsigned i, num, num_fq;
>  	struct rte_kni_mbuf *kva;
>  	struct rte_kni_mbuf *va[MBUF_BURST_SZ];
>  	void * data_kva;
> @@ -139,24 +139,19 @@ kni_net_rx_normal(struct kni_dev *kni)
>  	struct sk_buff *skb;
>  	struct net_device *dev = kni->net_dev;
> 
> -	/* Get the number of entries in rx_q */
> -	num_rq = kni_fifo_count(kni->rx_q);
> -
>  	/* Get the number of free entries in free_q */
> -	num_fq = kni_fifo_free_count(kni->free_q);
> -
> -	/* Calculate the number of entries to dequeue in rx_q */
> -	num = min(num_rq, num_fq);
> -	num = min(num, (unsigned)MBUF_BURST_SZ);
> -
> -	/* Return if no entry in rx_q and no free entry in free_q */
> -	if (num == 0)
> +	if ((num_fq = kni_fifo_free_count(kni->free_q)) == 0) {
> +		/* No room on the free_q, bail out */
>  		return;
> +	}
> +
> +	/* Calculate the number of entries to dequeue from rx_q */
> +	num = min(num_fq, (unsigned)MBUF_BURST_SZ);
> 
>  	/* Burst dequeue from rx_q */
>  	ret = kni_fifo_get(kni->rx_q, (void **)va, num);
>  	if (ret == 0)
> -		return; /* Failing should not happen */
> +		return;
> 
>  	/* Transfer received packets to netif */
>  	for (i = 0; i < num; i++) {
> --
> 2.3.2 (Apple Git-55)