[dpdk-dev] nfp: add doc about supported features

Message ID 1480666653-35544-1-git-send-email-alejandro.lucero@netronome.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Alejandro Lucero Dec. 2, 2016, 8:17 a.m. UTC
  Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 doc/guides/nics/features/nfp.ini | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
  

Comments

John McNamara Dec. 2, 2016, 10:56 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alejandro Lucero
> Sent: Friday, December 2, 2016 8:18 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] nfp: add doc about supported features
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Acked-by: John McNamara <john.mcnamara@intel.com>
  
Ferruh Yigit Dec. 6, 2016, 12:08 p.m. UTC | #2
On 12/2/2016 8:17 AM, Alejandro Lucero wrote:
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> ---
>  doc/guides/nics/features/nfp.ini | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/doc/guides/nics/features/nfp.ini b/doc/guides/nics/features/nfp.ini
> index d967151..dd78233 100644
> --- a/doc/guides/nics/features/nfp.ini
> +++ b/doc/guides/nics/features/nfp.ini
> @@ -4,3 +4,28 @@
>  ; Refer to default.ini for the full list of available PMD features.
>  ;
>  [Features]
> +SR-IOV               = Y
> +Link status          = Y
> +Link status event    = Y
> +Rx interrupt         = N

Hi John,

Do we support 'N'? Since this is the first 'N' in the table.

I guess not having a value implies feature is not supported.

> +Queue start/stop     = Y
> +MTU update           = Y
> +Jumbo frame          = Y
> +Scattered Rx         = N
> +TSO                  = N
> +RSS hash             = Y
> +RSS key update       = Y
> +RSS reta update      = Y
> +Flow control         = Y
> +CRC offload          = N
> +VLAN offload         = Y
> +L3 checksum offload  = Y
> +L4 checksum offload  = Y
> +Promiscuous mode     = Y
> +Basic stats          = Y
> +Stats per queue      = Y
> +Linux UIO            = Y
> +Linux VFIO           = Y
> +x86-32               = N
> +x86-64               = Y
> +Usage doc            = Y
>
  
John McNamara Dec. 6, 2016, 12:46 p.m. UTC | #3
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, December 6, 2016 12:08 PM
> To: Alejandro Lucero <alejandro.lucero@netronome.com>; dev@dpdk.org;
> Mcnamara, John <john.mcnamara@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] nfp: add doc about supported features
> 
> On 12/2/2016 8:17 AM, Alejandro Lucero wrote:
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > ---
> >  doc/guides/nics/features/nfp.ini | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/doc/guides/nics/features/nfp.ini
> b/doc/guides/nics/features/nfp.ini
> > index d967151..dd78233 100644
> > --- a/doc/guides/nics/features/nfp.ini
> > +++ b/doc/guides/nics/features/nfp.ini
> > @@ -4,3 +4,28 @@
> >  ; Refer to default.ini for the full list of available PMD features.
> >  ;
> >  [Features]
> > +SR-IOV               = Y
> > +Link status          = Y
> > +Link status event    = Y
> > +Rx interrupt         = N
> 
> Hi John,
> 
> Do we support 'N'? Since this is the first 'N' in the table.
> 
> I guess not having a value implies feature is not supported.

N or any other single letter is supported.

The nice thing about it is that it is explicitly NO rather than implicitly NO. However, if all the nics filled in the NOes then the table might be hard to read.

So, for consistency it is probably best to just let the unsupported features be blank.
  
Alejandro Lucero Dec. 6, 2016, 2:51 p.m. UTC | #4
Then I guess I should send another version of this patch.

On Tue, Dec 6, 2016 at 12:46 PM, Mcnamara, John <john.mcnamara@intel.com>
wrote:

> > -----Original Message-----
> > From: Yigit, Ferruh
> > Sent: Tuesday, December 6, 2016 12:08 PM
> > To: Alejandro Lucero <alejandro.lucero@netronome.com>; dev@dpdk.org;
> > Mcnamara, John <john.mcnamara@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] nfp: add doc about supported features
> >
> > On 12/2/2016 8:17 AM, Alejandro Lucero wrote:
> > > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> > > ---
> > >  doc/guides/nics/features/nfp.ini | 25 +++++++++++++++++++++++++
> > >  1 file changed, 25 insertions(+)
> > >
> > > diff --git a/doc/guides/nics/features/nfp.ini
> > b/doc/guides/nics/features/nfp.ini
> > > index d967151..dd78233 100644
> > > --- a/doc/guides/nics/features/nfp.ini
> > > +++ b/doc/guides/nics/features/nfp.ini
> > > @@ -4,3 +4,28 @@
> > >  ; Refer to default.ini for the full list of available PMD features.
> > >  ;
> > >  [Features]
> > > +SR-IOV               = Y
> > > +Link status          = Y
> > > +Link status event    = Y
> > > +Rx interrupt         = N
> >
> > Hi John,
> >
> > Do we support 'N'? Since this is the first 'N' in the table.
> >
> > I guess not having a value implies feature is not supported.
>
> N or any other single letter is supported.
>
> The nice thing about it is that it is explicitly NO rather than implicitly
> NO. However, if all the nics filled in the NOes then the table might be
> hard to read.
>
> So, for consistency it is probably best to just let the unsupported
> features be blank.
>
>
>
  
Ferruh Yigit Dec. 6, 2016, 4:01 p.m. UTC | #5
On 12/6/2016 2:51 PM, Alejandro Lucero wrote:
> Then I guess I should send another version of this patch.

I can also update the patch while applying, if you agree with following
content (simply just all =N removed):

 [Features]
+SR-IOV               = Y
+Link status          = Y
+Link status event    = Y
+Queue start/stop     = Y
+MTU update           = Y
+Jumbo frame          = Y
+RSS hash             = Y
+RSS key update       = Y
+RSS reta update      = Y
+Flow control         = Y
+VLAN offload         = Y
+L3 checksum offload  = Y
+L4 checksum offload  = Y
+Promiscuous mode     = Y
+Basic stats          = Y
+Stats per queue      = Y
+Linux UIO            = Y
+Linux VFIO           = Y
+x86-64               = Y
+Usage doc            = Y
  
Alejandro Lucero Dec. 9, 2016, 10 a.m. UTC | #6
That's fine.

Thank you

On Tue, Dec 6, 2016 at 4:01 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 12/6/2016 2:51 PM, Alejandro Lucero wrote:
> > Then I guess I should send another version of this patch.
>
> I can also update the patch while applying, if you agree with following
> content (simply just all =N removed):
>
>  [Features]
> +SR-IOV               = Y
> +Link status          = Y
> +Link status event    = Y
> +Queue start/stop     = Y
> +MTU update           = Y
> +Jumbo frame          = Y
> +RSS hash             = Y
> +RSS key update       = Y
> +RSS reta update      = Y
> +Flow control         = Y
> +VLAN offload         = Y
> +L3 checksum offload  = Y
> +L4 checksum offload  = Y
> +Promiscuous mode     = Y
> +Basic stats          = Y
> +Stats per queue      = Y
> +Linux UIO            = Y
> +Linux VFIO           = Y
> +x86-64               = Y
> +Usage doc            = Y
>
>
>
  
Ferruh Yigit Dec. 9, 2016, 11:36 a.m. UTC | #7
On 12/9/2016 10:00 AM, Alejandro Lucero wrote:
> That's fine.

Applied to dpdk-next-net/master, thanks.

> 
> Thank you
> 
> On Tue, Dec 6, 2016 at 4:01 PM, Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 12/6/2016 2:51 PM, Alejandro Lucero wrote:
>     > Then I guess I should send another version of this patch.
> 
>     I can also update the patch while applying, if you agree with following
>     content (simply just all =N removed):
> 
>      [Features]
>     +SR-IOV               = Y
>     +Link status          = Y
>     +Link status event    = Y
>     +Queue start/stop     = Y
>     +MTU update           = Y
>     +Jumbo frame          = Y
>     +RSS hash             = Y
>     +RSS key update       = Y
>     +RSS reta update      = Y
>     +Flow control         = Y
>     +VLAN offload         = Y
>     +L3 checksum offload  = Y
>     +L4 checksum offload  = Y
>     +Promiscuous mode     = Y
>     +Basic stats          = Y
>     +Stats per queue      = Y
>     +Linux UIO            = Y
>     +Linux VFIO           = Y
>     +x86-64               = Y
>     +Usage doc            = Y
> 
> 
>
  
Alejandro Lucero Dec. 16, 2016, 5:29 p.m. UTC | #8
Hi,

One question about this patch. I will send another patch soon which will
require to modify the file created by this patch. So, should I use the
dpdk-next for sending the new patch or the dpdk stable branch? I understand
that using the latter will imply some integration later, but I really do
not know if I should facilitate things using dpdk-next in this case.

By the way, it is not just about this specific patch, because I have other
almost ready which I want to push before the 16.02 deadline.


On Fri, Dec 9, 2016 at 11:36 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 12/9/2016 10:00 AM, Alejandro Lucero wrote:
> > That's fine.
>
> Applied to dpdk-next-net/master, thanks.
>
> >
> > Thank you
> >
> > On Tue, Dec 6, 2016 at 4:01 PM, Ferruh Yigit <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> >     On 12/6/2016 2:51 PM, Alejandro Lucero wrote:
> >     > Then I guess I should send another version of this patch.
> >
> >     I can also update the patch while applying, if you agree with
> following
> >     content (simply just all =N removed):
> >
> >      [Features]
> >     +SR-IOV               = Y
> >     +Link status          = Y
> >     +Link status event    = Y
> >     +Queue start/stop     = Y
> >     +MTU update           = Y
> >     +Jumbo frame          = Y
> >     +RSS hash             = Y
> >     +RSS key update       = Y
> >     +RSS reta update      = Y
> >     +Flow control         = Y
> >     +VLAN offload         = Y
> >     +L3 checksum offload  = Y
> >     +L4 checksum offload  = Y
> >     +Promiscuous mode     = Y
> >     +Basic stats          = Y
> >     +Stats per queue      = Y
> >     +Linux UIO            = Y
> >     +Linux VFIO           = Y
> >     +x86-64               = Y
> >     +Usage doc            = Y
> >
> >
> >
>
>
  
Ferruh Yigit Dec. 19, 2016, 11:45 a.m. UTC | #9
On 12/16/2016 5:29 PM, Alejandro Lucero wrote:
> Hi,
> 
> One question about this patch. I will send another patch soon which will
> require to modify the file created by this patch. So, should I use the
> dpdk-next for sending the new patch or the dpdk stable branch? 

I guess by "dpdk stable branch" you mean dpdk main repo, because we also
have stable sub-tree which is something else.

> I
> understand that using the latter will imply some integration later, but
> I really do not know if I should facilitate things using dpdk-next in
> this case. 

If the patch is driver patch, please send to the next-net sub-tree, as a
PMD maintainer, I expect majority of your patches should target next-net.

If patch just touches the documentation of the driver, you can send it
to the main tree, but both next-net sub-tree and main tree are OK since
PMD documentation is not heavily modified, integration will be (mostly)
easy.

Specific to the this document (feature.ini), since this is directly
correlated with PMD source code, to update this file, you need to update
the source code. And it is better to update this document in next-net as
part of the patchset that updates the PMD code.

> 
> By the way, it is not just about this specific patch, because I have
> other almost ready which I want to push before the 16.02 deadline.

Please push 17.02 patches as soon as possible, although there is
technically still some time for the integration deadline, practically
there is less because of holidays in between ...

> 
> 
<...>
  
Alejandro Lucero Dec. 19, 2016, 11:58 a.m. UTC | #10
On Mon, Dec 19, 2016 at 11:45 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 12/16/2016 5:29 PM, Alejandro Lucero wrote:
> > Hi,
> >
> > One question about this patch. I will send another patch soon which will
> > require to modify the file created by this patch. So, should I use the
> > dpdk-next for sending the new patch or the dpdk stable branch?
>
> I guess by "dpdk stable branch" you mean dpdk main repo, because we also
> have stable sub-tree which is something else.
>
>
Yes. That's what I meant.


> > I
> > understand that using the latter will imply some integration later, but
> > I really do not know if I should facilitate things using dpdk-next in
> > this case.
>
> If the patch is driver patch, please send to the next-net sub-tree, as a
> PMD maintainer, I expect majority of your patches should target next-net.
>
>
OK


> If patch just touches the documentation of the driver, you can send it
> to the main tree, but both next-net sub-tree and main tree are OK since
> PMD documentation is not heavily modified, integration will be (mostly)
> easy.
>
> Specific to the this document (feature.ini), since this is directly
> correlated with PMD source code, to update this file, you need to update
> the source code. And it is better to update this document in next-net as
> part of the patchset that updates the PMD code.
>
>
Fine.


> >
> > By the way, it is not just about this specific patch, because I have
> > other almost ready which I want to push before the 16.02 deadline.
>
> Please push 17.02 patches as soon as possible, although there is
> technically still some time for the integration deadline, practically
> there is less because of holidays in between ...
>
>
I'm working on this. I have been busy doing other more priority things but
this is now top priority for me.

Thanks


> >
> >
> <...>
>
  

Patch

diff --git a/doc/guides/nics/features/nfp.ini b/doc/guides/nics/features/nfp.ini
index d967151..dd78233 100644
--- a/doc/guides/nics/features/nfp.ini
+++ b/doc/guides/nics/features/nfp.ini
@@ -4,3 +4,28 @@ 
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
+SR-IOV               = Y
+Link status          = Y
+Link status event    = Y
+Rx interrupt         = N
+Queue start/stop     = Y
+MTU update           = Y
+Jumbo frame          = Y
+Scattered Rx         = N
+TSO                  = N
+RSS hash             = Y
+RSS key update       = Y
+RSS reta update      = Y
+Flow control         = Y
+CRC offload          = N
+VLAN offload         = Y
+L3 checksum offload  = Y
+L4 checksum offload  = Y
+Promiscuous mode     = Y
+Basic stats          = Y
+Stats per queue      = Y
+Linux UIO            = Y
+Linux VFIO           = Y
+x86-32               = N
+x86-64               = Y
+Usage doc            = Y