[dpdk-dev] [PATCH 1/2] rte_kni: Fix documentation.

[dpdk-dev] [PATCH 1/2] rte_kni: Fix documentation.

[Alex Wang]

From: Alex Wang <alex@awakenetworks.com>

The 'mbufs' alloc/free descriptions for 'rte_kni_tx_burst()'
and 'rte_kni_rx_burst()' should be inverted.

Signed-off-by: Alex Wang <alex@awakenetworks.com>
---
 lib/librte_kni/rte_kni.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index ef9faa9..25fa45e 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -161,8 +161,8 @@ extern int rte_kni_handle_request(struct rte_kni *kni);
 /**
  * Retrieve a burst of packets from a KNI interface. The retrieved packets are
  * stored in rte_mbuf structures whose pointers are supplied in the array of
- * mbufs, and the maximum number is indicated by num. It handles the freeing of
- * the mbufs in the free queue of KNI interface.
+ * mbufs, and the maximum number is indicated by num. It handles allocating
+ * the mbufs for KNI interface alloc queue.
  *
  * @param kni
  *  The KNI interface context.
@@ -180,8 +180,8 @@ extern unsigned rte_kni_rx_burst(struct rte_kni *kni,
 /**
  * Send a burst of packets to a KNI interface. The packets to be sent out are
  * stored in rte_mbuf structures whose pointers are supplied in the array of
- * mbufs, and the maximum number is indicated by num. It handles allocating
- * the mbufs for KNI interface alloc queue.
+ * mbufs, and the maximum number is indicated by num. It handles the freeing of
+ * the mbufs in the free queue of KNI interface.
  *
  * @param kni
  *  The KNI interface context.
-- 
2.1.4

[dpdk-dev] [PATCH 2/2] rte_kni: Add documentation for the mempool capacity.

[Alex Wang]

From: Alex Wang <alex@awakenetworks.com>

Function like 'rte_kni_rx_burst()' keeps allocating
'MAX_MBUF_BURST_NUM' mbufs to kni fifo queue unless the
queue's capacity ('KNI_FIFO_COUNT_MAX') is reached.  So,
if the mempool is under-provisioned, user may run into
"Out of Memory" logs from KNI code.  This commit documents
the need to provision mempool capacity of couple thousand
elements for each KNI interface.

Signed-off-by: Alex Wang <alex@awakenetworks.com>
---
 lib/librte_kni/rte_kni.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 25fa45e..05d2d39 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -113,6 +113,9 @@ extern void rte_kni_init(unsigned int max_kni_ifaces);
  * The rte_kni_alloc shall not be called before rte_kni_init() has been
  * called. rte_kni_alloc is thread safe.
  *
+ * The mempool should have capacity of couple thousand elements for each
+ * KNI interface allocated.
+ *
  * @param pktmbuf_pool
  *  The mempool for allocting mbufs for packets.
  * @param conf
-- 
2.1.4

Re: [dpdk-dev] [PATCH 2/2] rte_kni: Add documentation for the mempool capacity.

[Ferruh Yigit]

On 5/14/2016 7:22 PM, Alex Wang wrote:
> From: Alex Wang <alex@awakenetworks.com>
> 
> Function like 'rte_kni_rx_burst()' keeps allocating
> 'MAX_MBUF_BURST_NUM' mbufs to kni fifo queue unless the
> queue's capacity ('KNI_FIFO_COUNT_MAX') is reached.  So,
> if the mempool is under-provisioned, user may run into
> "Out of Memory" logs from KNI code.  This commit documents
> the need to provision mempool capacity of couple thousand
> elements for each KNI interface.
> 
> Signed-off-by: Alex Wang <alex@awakenetworks.com>
> ---
>  lib/librte_kni/rte_kni.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
> index 25fa45e..05d2d39 100644
> --- a/lib/librte_kni/rte_kni.h
> +++ b/lib/librte_kni/rte_kni.h
> @@ -113,6 +113,9 @@ extern void rte_kni_init(unsigned int max_kni_ifaces);
>   * The rte_kni_alloc shall not be called before rte_kni_init() has been
>   * called. rte_kni_alloc is thread safe.
>   *
> + * The mempool should have capacity of couple thousand elements for each
> + * KNI interface allocated.
> + *
>   * @param pktmbuf_pool
>   *  The mempool for allocting mbufs for packets.
>   * @param conf
> 
Hi Alex,

Same comments are valid for patch subject.

How about saying "more than 2xKNI_FIFO_COUNT_MAX" instead of saying
"couple thousand elements"?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH 1/2] rte_kni: Fix documentation.

[Ferruh Yigit]

On 5/14/2016 7:22 PM, Alex Wang wrote:
> From: Alex Wang <alex@awakenetworks.com>
> 
> The 'mbufs' alloc/free descriptions for 'rte_kni_tx_burst()'
> and 'rte_kni_rx_burst()' should be inverted.
> 
> Signed-off-by: Alex Wang <alex@awakenetworks.com>
> ---
>  lib/librte_kni/rte_kni.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
> index ef9faa9..25fa45e 100644
> --- a/lib/librte_kni/rte_kni.h
> +++ b/lib/librte_kni/rte_kni.h
> @@ -161,8 +161,8 @@ extern int rte_kni_handle_request(struct rte_kni *kni);
>  /**
>   * Retrieve a burst of packets from a KNI interface. The retrieved packets are
>   * stored in rte_mbuf structures whose pointers are supplied in the array of
> - * mbufs, and the maximum number is indicated by num. It handles the freeing of
> - * the mbufs in the free queue of KNI interface.
> + * mbufs, and the maximum number is indicated by num. It handles allocating
> + * the mbufs for KNI interface alloc queue.
>   *
>   * @param kni
>   *  The KNI interface context.
> @@ -180,8 +180,8 @@ extern unsigned rte_kni_rx_burst(struct rte_kni *kni,
>  /**
>   * Send a burst of packets to a KNI interface. The packets to be sent out are
>   * stored in rte_mbuf structures whose pointers are supplied in the array of
> - * mbufs, and the maximum number is indicated by num. It handles allocating
> - * the mbufs for KNI interface alloc queue.
> + * mbufs, and the maximum number is indicated by num. It handles the freeing of
> + * the mbufs in the free queue of KNI interface.
>   *
>   * @param kni
>   *  The KNI interface context.
> 

Hi Alex,

Can you please update the patch subject,
- replace "rte_kni" tag with a "kni",
- after space start with lowercase,
- remove the "." at the end of the sentences,
like:
"kni: fix documentation"
(these are defined in
http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line)

Also can you please add a "Fixes" line, more details on:
http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body

Although this information converted into documentation, this is not
really the documentation, and the patch title gives little information,
if possible can you please add more information while keeping it around
50 chars limit.

finally, patch content is OK.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH 1/2] rte_kni: Fix documentation.

[ALeX Wang]

Sorry for the delayed reply, just sent V2~

On 18 May 2016 at 03:33, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 5/14/2016 7:22 PM, Alex Wang wrote:
> > From: Alex Wang <alex@awakenetworks.com>
> >
> > The 'mbufs' alloc/free descriptions for 'rte_kni_tx_burst()'
> > and 'rte_kni_rx_burst()' should be inverted.
> >
> > Signed-off-by: Alex Wang <alex@awakenetworks.com>
> > ---
> >  lib/librte_kni/rte_kni.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
> > index ef9faa9..25fa45e 100644
> > --- a/lib/librte_kni/rte_kni.h
> > +++ b/lib/librte_kni/rte_kni.h
> > @@ -161,8 +161,8 @@ extern int rte_kni_handle_request(struct rte_kni
> *kni);
> >  /**
> >   * Retrieve a burst of packets from a KNI interface. The retrieved
> packets are
> >   * stored in rte_mbuf structures whose pointers are supplied in the
> array of
> > - * mbufs, and the maximum number is indicated by num. It handles the
> freeing of
> > - * the mbufs in the free queue of KNI interface.
> > + * mbufs, and the maximum number is indicated by num. It handles
> allocating
> > + * the mbufs for KNI interface alloc queue.
> >   *
> >   * @param kni
> >   *  The KNI interface context.
> > @@ -180,8 +180,8 @@ extern unsigned rte_kni_rx_burst(struct rte_kni *kni,
> >  /**
> >   * Send a burst of packets to a KNI interface. The packets to be sent
> out are
> >   * stored in rte_mbuf structures whose pointers are supplied in the
> array of
> > - * mbufs, and the maximum number is indicated by num. It handles
> allocating
> > - * the mbufs for KNI interface alloc queue.
> > + * mbufs, and the maximum number is indicated by num. It handles the
> freeing of
> > + * the mbufs in the free queue of KNI interface.
> >   *
> >   * @param kni
> >   *  The KNI interface context.
> >
>
> Hi Alex,
>
> Can you please update the patch subject,
> - replace "rte_kni" tag with a "kni",
> - after space start with lowercase,
> - remove the "." at the end of the sentences,
> like:
> "kni: fix documentation"
> (these are defined in
>
> http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line
> )
>
> Also can you please add a "Fixes" line, more details on:
> http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body
>
> Although this information converted into documentation, this is not
> really the documentation, and the patch title gives little information,
> if possible can you please add more information while keeping it around
> 50 chars limit.
>
> finally, patch content is OK.
>
> Thanks,
> ferruh
>
>


-- 
Alex Wang,
Open vSwitch developer