[v3,3/9] net/ionic: update documentation and MAINTAINERS
diff mbox series

Message ID 20201204201646.51746-4-aboyer@pensando.io
State Changes Requested
Delegated to: Ferruh Yigit
Headers show
Series
  • net/ionic: minor updates and documentation
Related show

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andrew Boyer Dec. 4, 2020, 8:16 p.m. UTC
The UNMAINTAINED flag will be removed in a future patch.

Signed-off-by: Andrew Boyer <aboyer@pensando.io>
---
 MAINTAINERS                        |  3 ++-
 doc/guides/nics/features/ionic.ini |  2 ++
 doc/guides/nics/ionic.rst          | 13 +++++++------
 3 files changed, 11 insertions(+), 7 deletions(-)

Comments

Ferruh Yigit Dec. 9, 2020, 12:03 p.m. UTC | #1
On 12/4/2020 8:16 PM, Andrew Boyer wrote:
> The UNMAINTAINED flag will be removed in a future patch.
> 
> Signed-off-by: Andrew Boyer <aboyer@pensando.io>
> ---
>   MAINTAINERS                        |  3 ++-
>   doc/guides/nics/features/ionic.ini |  2 ++
>   doc/guides/nics/ionic.rst          | 13 +++++++------
>   3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eafe9f8c4..6534983c1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>   F: doc/guides/nics/features/pfe.ini
>   
>   Pensando ionic - UNMAINTAINED
> -M: Alfredo Cardigliano <cardigliano@ntop.org>
> +M: Andrew Boyer <aboyer@pensando.io>
> +M: Pensando Drivers <drivers@pensando.io>

Same comment from previous version, please don't add group as maintainer, only 
actual people.

>   F: drivers/net/ionic/
>   F: doc/guides/nics/ionic.rst
>   F: doc/guides/nics/features/ionic.ini
> diff --git a/doc/guides/nics/features/ionic.ini b/doc/guides/nics/features/ionic.ini
> index 083c7bd99..dd29dbed6 100644
> --- a/doc/guides/nics/features/ionic.ini
> +++ b/doc/guides/nics/features/ionic.ini
> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>   Link status          = Y
>   Link status event    = Y
>   Queue start/stop     = Y
> +Lock-free Tx queue   = Y

Are you sure this is supported?
Since it is not advertised as capability, I think this can't be claimed as 
supported, but still even after this is added as capability, can you please 
confirm your device supports multiple core enqueue to same queue without locks?

>   MTU update           = Y
>   Jumbo frame          = Y
>   Scattered Rx         = Y
> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>   RSS hash             = Y
>   RSS key update       = Y
>   RSS reta update      = Y
> +SR-IOV               = Y

Can you please explain what is exactly supported? Like can DPDK drive both PF & VF?

<...>

> @@ -7,15 +7,16 @@ IONIC Driver
>   The ionic driver provides support for Pensando server adapters.
>   It currently supports the below models:
>   
> -- `Naples DSC-25 <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>`_
> -- `Naples DSC-100 <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>`_
> +- DSC-25 dual-port 25G Distributed Services Card
> +- DSC-100 dual-port 100G Distributed Services Card
>   

Same comment from previous version, can you provide link for these devices, it 
is hard to find the devices from the main site.
Andrew Boyer Dec. 9, 2020, 2:36 p.m. UTC | #2
Please respond to my questions this time. I have ~70 more patches to post by December 20.

> On Dec 9, 2020, at 7:03 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 12/4/2020 8:16 PM, Andrew Boyer wrote:
>> The UNMAINTAINED flag will be removed in a future patch.
>> Signed-off-by: Andrew Boyer <aboyer@pensando.io>
>> ---
>>  MAINTAINERS                        |  3 ++-
>>  doc/guides/nics/features/ionic.ini |  2 ++
>>  doc/guides/nics/ionic.rst          | 13 +++++++------
>>  3 files changed, 11 insertions(+), 7 deletions(-)
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index eafe9f8c4..6534983c1 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>>  F: doc/guides/nics/features/pfe.ini
>>    Pensando ionic - UNMAINTAINED
>> -M: Alfredo Cardigliano <cardigliano@ntop.org>
>> +M: Andrew Boyer <aboyer@pensando.io>
>> +M: Pensando Drivers <drivers@pensando.io>
> 
> Same comment from previous version, please don't add group as maintainer, only actual people.

I responded to your original comment about this back in November. Is there an official DPDK policy against doing this? Is it your preference? We would very much prefer to have this in the file as a fallback. As long as there is still at least one person listed, what is the harm?

>>  F: drivers/net/ionic/
>>  F: doc/guides/nics/ionic.rst
>>  F: doc/guides/nics/features/ionic.ini
>> diff --git a/doc/guides/nics/features/ionic.ini b/doc/guides/nics/features/ionic.ini
>> index 083c7bd99..dd29dbed6 100644
>> --- a/doc/guides/nics/features/ionic.ini
>> +++ b/doc/guides/nics/features/ionic.ini
>> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>>  Link status          = Y
>>  Link status event    = Y
>>  Queue start/stop     = Y
>> +Lock-free Tx queue   = Y
> 
> Are you sure this is supported?
> Since it is not advertised as capability, I think this can't be claimed as supported, but still even after this is added as capability, can you please confirm your device supports multiple core enqueue to same queue without locks?

I misunderstood the meaning of this flag, will remove.

>>  MTU update           = Y
>>  Jumbo frame          = Y
>>  Scattered Rx         = Y
>> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>>  RSS hash             = Y
>>  RSS key update       = Y
>>  RSS reta update      = Y
>> +SR-IOV               = Y
> 
> Can you please explain what is exactly supported? Like can DPDK drive both PF & VF?

Yes. The PMD does not distinguish between PFs and VFs.

> 
> <...>
> 
>> @@ -7,15 +7,16 @@ IONIC Driver
>>  The ionic driver provides support for Pensando server adapters.
>>  It currently supports the below models:
>>  -- `Naples DSC-25 <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>>`_
>> -- `Naples DSC-100 <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>>`_
>> +- DSC-25 dual-port 25G Distributed Services Card
>> +- DSC-100 dual-port 100G Distributed Services Card
>>  
> 
> Same comment from previous version, can you provide link for these devices, it is hard to find the devices from the main site.

And my same response from your previous comment. I do not control the website and do not wish to put stale PDF links in this document, which will live forever. The text includes the URL of the page containing links to the PDFs. Why is this not acceptable?

-Andrew
Ferruh Yigit Dec. 9, 2020, 3:24 p.m. UTC | #3
On 12/9/2020 2:36 PM, Andrew Boyer wrote:
> Please respond to my questions this time. I have ~70 more patches to post by 
> December 20.
> 
>> On Dec 9, 2020, at 7:03 AM, Ferruh Yigit <ferruh.yigit@intel.com 
>> <mailto:ferruh.yigit@intel.com>> wrote:
>>
>> On 12/4/2020 8:16 PM, Andrew Boyer wrote:
>>> The UNMAINTAINED flag will be removed in a future patch.
>>> Signed-off-by: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io>>
>>> ---
>>>  MAINTAINERS                        |  3 ++-
>>>  doc/guides/nics/features/ionic.ini |  2 ++
>>>  doc/guides/nics/ionic.rst          | 13 +++++++------
>>>  3 files changed, 11 insertions(+), 7 deletions(-)
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index eafe9f8c4..6534983c1 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>>>  F: doc/guides/nics/features/pfe.ini
>>>    Pensando ionic - UNMAINTAINED
>>> -M: Alfredo Cardigliano <cardigliano@ntop.org <mailto:cardigliano@ntop.org>>
>>> +M: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io>>
>>> +M: Pensando Drivers <drivers@pensando.io <mailto:drivers@pensando.io>>
>>
>> Same comment from previous version, please don't add group as maintainer, only 
>> actual people.
> 
> I responded to your original comment about this back in November. Is there an 
> official DPDK policy against doing this? Is it your preference? We would very 
> much prefer to have this in the file as a fallback. As long as there is still at 
> least one person listed, what is the harm?
> 

There is no official policy against it as far as I know.

The problem with the groups is we don't know who is behind it, it blurs who is 
the owner/responsible of the component. Actual people makes it clear that who is 
responsible.

Why do you prefer to add a group as maintainer?

>>>  F: drivers/net/ionic/
>>>  F: doc/guides/nics/ionic.rst
>>>  F: doc/guides/nics/features/ionic.ini
>>> diff --git a/doc/guides/nics/features/ionic.ini 
>>> b/doc/guides/nics/features/ionic.ini
>>> index 083c7bd99..dd29dbed6 100644
>>> --- a/doc/guides/nics/features/ionic.ini
>>> +++ b/doc/guides/nics/features/ionic.ini
>>> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>>>  Link status          = Y
>>>  Link status event    = Y
>>>  Queue start/stop     = Y
>>> +Lock-free Tx queue   = Y
>>
>> Are you sure this is supported?
>> Since it is not advertised as capability, I think this can't be claimed as 
>> supported, but still even after this is added as capability, can you please 
>> confirm your device supports multiple core enqueue to same queue without locks?
> 
> I misunderstood the meaning of this flag, will remove.
> 
>>>  MTU update           = Y
>>>  Jumbo frame          = Y
>>>  Scattered Rx         = Y
>>> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>>>  RSS hash             = Y
>>>  RSS key update       = Y
>>>  RSS reta update      = Y
>>> +SR-IOV               = Y
>>
>> Can you please explain what is exactly supported? Like can DPDK drive both PF 
>> & VF?
> 
> Yes. The PMD does not distinguish between PFs and VFs.
> 
>>
>> <...>
>>
>>> @@ -7,15 +7,16 @@ IONIC Driver
>>>  The ionic driver provides support for Pensando server adapters.
>>>  It currently supports the below models:
>>>  -- `Naples DSC-25 
>>> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf 
>>> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>>`_
>>> -- `Naples DSC-100 
>>> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf 
>>> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>>`_
>>> +- DSC-25 dual-port 25G Distributed Services Card
>>> +- DSC-100 dual-port 100G Distributed Services Card
>>>
>>
>> Same comment from previous version, can you provide link for these devices, it 
>> is hard to find the devices from the main site.
> 
> And my same response from your previous comment. I do not control the website 
> and do not wish to put stale PDF links in this document, which will live 
> forever. The text includes the URL of the page containing links to the PDFs. Why 
> is this not acceptable?
> 

The request is to put links to the products that you are providing the driver 
for. This is to help people that are already interested your driver and reading 
your driver document, to reach to the product information easily.

The request is NOT to provide pdf etc, just a reference to the product. Don't 
you advertise your product in your official web site? If your product 
information is not visible/hidden, why you are providing the open source drivers 
for it?
Andrew Boyer Dec. 9, 2020, 4:24 p.m. UTC | #4
> On Dec 9, 2020, at 10:24 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 12/9/2020 2:36 PM, Andrew Boyer wrote:
>> Please respond to my questions this time. I have ~70 more patches to post by December 20.
>>> On Dec 9, 2020, at 7:03 AM, Ferruh Yigit <ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com>> wrote:
>>> 
>>> On 12/4/2020 8:16 PM, Andrew Boyer wrote:
>>>> The UNMAINTAINED flag will be removed in a future patch.
>>>> Signed-off-by: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io>>
>>>> ---
>>>>  MAINTAINERS                        |  3 ++-
>>>>  doc/guides/nics/features/ionic.ini |  2 ++
>>>>  doc/guides/nics/ionic.rst          | 13 +++++++------
>>>>  3 files changed, 11 insertions(+), 7 deletions(-)
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index eafe9f8c4..6534983c1 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>>>>  F: doc/guides/nics/features/pfe.ini
>>>>    Pensando ionic - UNMAINTAINED
>>>> -M: Alfredo Cardigliano <cardigliano@ntop.org <mailto:cardigliano@ntop.org>>
>>>> +M: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io>>
>>>> +M: Pensando Drivers <drivers@pensando.io <mailto:drivers@pensando.io>>
>>> 
>>> Same comment from previous version, please don't add group as maintainer, only actual people.
>> I responded to your original comment about this back in November. Is there an official DPDK policy against doing this? Is it your preference? We would very much prefer to have this in the file as a fallback. As long as there is still at least one person listed, what is the harm?
> 
> There is no official policy against it as far as I know.
> 
> The problem with the groups is we don't know who is behind it, it blurs who is the owner/responsible of the component. Actual people makes it clear that who is responsible.
> 
> Why do you prefer to add a group as maintainer?

Because if I am on leave for some reason, one of the other handful of maintainers might be able to help someone with a problem or a question.

If I am listed specifically, doesn’t that make clear “who is the owner/responsible” for ionic PMD? What harm does having drivers@ listed do?

>>>>  F: drivers/net/ionic/
>>>>  F: doc/guides/nics/ionic.rst
>>>>  F: doc/guides/nics/features/ionic.ini
>>>> diff --git a/doc/guides/nics/features/ionic.ini b/doc/guides/nics/features/ionic.ini
>>>> index 083c7bd99..dd29dbed6 100644
>>>> --- a/doc/guides/nics/features/ionic.ini
>>>> +++ b/doc/guides/nics/features/ionic.ini
>>>> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>>>>  Link status          = Y
>>>>  Link status event    = Y
>>>>  Queue start/stop     = Y
>>>> +Lock-free Tx queue   = Y
>>> 
>>> Are you sure this is supported?
>>> Since it is not advertised as capability, I think this can't be claimed as supported, but still even after this is added as capability, can you please confirm your device supports multiple core enqueue to same queue without locks?
>> I misunderstood the meaning of this flag, will remove.
>>>>  MTU update           = Y
>>>>  Jumbo frame          = Y
>>>>  Scattered Rx         = Y
>>>> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>>>>  RSS hash             = Y
>>>>  RSS key update       = Y
>>>>  RSS reta update      = Y
>>>> +SR-IOV               = Y
>>> 
>>> Can you please explain what is exactly supported? Like can DPDK drive both PF & VF?
>> Yes. The PMD does not distinguish between PFs and VFs.
>>> 
>>> <...>
>>> 
>>>> @@ -7,15 +7,16 @@ IONIC Driver
>>>>  The ionic driver provides support for Pensando server adapters.
>>>>  It currently supports the below models:
>>>>  -- `Naples DSC-25 <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>>`_
>>>> -- `Naples DSC-100 <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>>`_
>>>> +- DSC-25 dual-port 25G Distributed Services Card
>>>> +- DSC-100 dual-port 100G Distributed Services Card
>>>> 
>>> 
>>> Same comment from previous version, can you provide link for these devices, it is hard to find the devices from the main site.
>> And my same response from your previous comment. I do not control the website and do not wish to put stale PDF links in this document, which will live forever. The text includes the URL of the page containing links to the PDFs. Why is this not acceptable?
> 
> The request is to put links to the products that you are providing the driver for. This is to help people that are already interested your driver and reading your driver document, to reach to the product information easily.
> 
> The request is NOT to provide pdf etc, just a reference to the product. Don't you advertise your product in your official web site? If your product information is not visible/hidden, why you are providing the open source drivers for it?

Does this line in the doc not satisfy your request?

+The `Documents <https://pensando.io/documents/ <https://pensando.io/documents/>>`_ page contains Product Briefs and other product information.

-Andrew
Ferruh Yigit Dec. 9, 2020, 5:15 p.m. UTC | #5
On 12/9/2020 4:24 PM, Andrew Boyer wrote:
> 
>> On Dec 9, 2020, at 10:24 AM, Ferruh Yigit <ferruh.yigit@intel.com 
>> <mailto:ferruh.yigit@intel.com>> wrote:
>>
>> On 12/9/2020 2:36 PM, Andrew Boyer wrote:
>>> Please respond to my questions this time. I have ~70 more patches to post by 
>>> December 20.
>>>> On Dec 9, 2020, at 7:03 AM, Ferruh Yigit <ferruh.yigit@intel.com 
>>>> <mailto:ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com 
>>>> <mailto:ferruh.yigit@intel.com>>> wrote:
>>>>
>>>> On 12/4/2020 8:16 PM, Andrew Boyer wrote:
>>>>> The UNMAINTAINED flag will be removed in a future patch.
>>>>> Signed-off-by: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io> 
>>>>> <mailto:aboyer@pensando.io <mailto:aboyer@pensando.io>>>
>>>>> ---
>>>>>  MAINTAINERS                        |  3 ++-
>>>>>  doc/guides/nics/features/ionic.ini |  2 ++
>>>>>  doc/guides/nics/ionic.rst          | 13 +++++++------
>>>>>  3 files changed, 11 insertions(+), 7 deletions(-)
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index eafe9f8c4..6534983c1 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>>>>>  F: doc/guides/nics/features/pfe.ini
>>>>>    Pensando ionic - UNMAINTAINED
>>>>> -M: Alfredo Cardigliano <cardigliano@ntop.org <mailto:cardigliano@ntop.org> 
>>>>> <mailto:cardigliano@ntop.org <mailto:cardigliano@ntop.org>>>
>>>>> +M: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io> 
>>>>> <mailto:aboyer@pensando.io <mailto:aboyer@pensando.io>>>
>>>>> +M: Pensando Drivers <drivers@pensando.io <mailto:drivers@pensando.io> 
>>>>> <mailto:drivers@pensando.io <mailto:drivers@pensando.io>>>
>>>>
>>>> Same comment from previous version, please don't add group as maintainer, 
>>>> only actual people.
>>> I responded to your original comment about this back in November. Is there an 
>>> official DPDK policy against doing this? Is it your preference? We would very 
>>> much prefer to have this in the file as a fallback. As long as there is still 
>>> at least one person listed, what is the harm?
>>
>> There is no official policy against it as far as I know.
>>
>> The problem with the groups is we don't know who is behind it, it blurs who is 
>> the owner/responsible of the component. Actual people makes it clear that who 
>> is responsible.
>>
>> Why do you prefer to add a group as maintainer?
> 
> Because if I am on leave for some reason, one of the other handful of 
> maintainers might be able to help someone with a problem or a question.
> 
> If I am listed specifically, doesn’t that make clear “who is the 
> owner/responsible” for ionic PMD? What harm does having drivers@ listed do?
> 

If you want backups, pick and list a few of those handful maintainers and add to 
the list, so we can know who they are.

As already said problem is we don't know who is behind a group, if they are 
reliable or not, or is there really someone or not, what happens if group become 
silent?
You can cc that group in your patches, that is commonly done, so they can be 
part of the development process, but the maintainers file is to define 
responsible people, adding a group is hiding actual responsible people.

>>>>>  F: drivers/net/ionic/
>>>>>  F: doc/guides/nics/ionic.rst
>>>>>  F: doc/guides/nics/features/ionic.ini
>>>>> diff --git a/doc/guides/nics/features/ionic.ini 
>>>>> b/doc/guides/nics/features/ionic.ini
>>>>> index 083c7bd99..dd29dbed6 100644
>>>>> --- a/doc/guides/nics/features/ionic.ini
>>>>> +++ b/doc/guides/nics/features/ionic.ini
>>>>> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>>>>>  Link status          = Y
>>>>>  Link status event    = Y
>>>>>  Queue start/stop     = Y
>>>>> +Lock-free Tx queue   = Y
>>>>
>>>> Are you sure this is supported?
>>>> Since it is not advertised as capability, I think this can't be claimed as 
>>>> supported, but still even after this is added as capability, can you please 
>>>> confirm your device supports multiple core enqueue to same queue without locks?
>>> I misunderstood the meaning of this flag, will remove.
>>>>>  MTU update           = Y
>>>>>  Jumbo frame          = Y
>>>>>  Scattered Rx         = Y
>>>>> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>>>>>  RSS hash             = Y
>>>>>  RSS key update       = Y
>>>>>  RSS reta update      = Y
>>>>> +SR-IOV               = Y
>>>>
>>>> Can you please explain what is exactly supported? Like can DPDK drive both 
>>>> PF & VF?
>>> Yes. The PMD does not distinguish between PFs and VFs.
>>>>
>>>> <...>
>>>>
>>>>> @@ -7,15 +7,16 @@ IONIC Driver
>>>>>  The ionic driver provides support for Pensando server adapters.
>>>>>  It currently supports the below models:
>>>>>  -- `Naples DSC-25 
>>>>> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf 
>>>>> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf> 
>>>>> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf 
>>>>> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>>>`_
>>>>> -- `Naples DSC-100 
>>>>> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf 
>>>>> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf> 
>>>>> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf 
>>>>> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>>>`_
>>>>> +- DSC-25 dual-port 25G Distributed Services Card
>>>>> +- DSC-100 dual-port 100G Distributed Services Card
>>>>>
>>>>
>>>> Same comment from previous version, can you provide link for these devices, 
>>>> it is hard to find the devices from the main site.
>>> And my same response from your previous comment. I do not control the website 
>>> and do not wish to put stale PDF links in this document, which will live 
>>> forever. The text includes the URL of the page containing links to the PDFs. 
>>> Why is this not acceptable?
>>
>> The request is to put links to the products that you are providing the driver 
>> for. This is to help people that are already interested your driver and 
>> reading your driver document, to reach to the product information easily.
>>
>> The request is NOT to provide pdf etc, just a reference to the product. Don't 
>> you advertise your product in your official web site? If your product 
>> information is not visible/hidden, why you are providing the open source 
>> drivers for it?
> 
> Does this line in the doc not satisfy your request?
> 
> +The `Documents <https://pensando.io/documents/ 
> <https://pensando.io/documents/>>`_ page contains Product Briefs and other 
> product information.
> 

No it doesn't, it is not clear which document/product you are referring to.
Why it is hard to provide the link of products that your driver is for?
Andrew Boyer Dec. 9, 2020, 7:05 p.m. UTC | #6
> On Dec 9, 2020, at 12:15 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 12/9/2020 4:24 PM, Andrew Boyer wrote:
>>> On Dec 9, 2020, at 10:24 AM, Ferruh Yigit <ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com>> wrote:
>>> 
>>> On 12/9/2020 2:36 PM, Andrew Boyer wrote:
>>>> Please respond to my questions this time. I have ~70 more patches to post by December 20.
>>>>> On Dec 9, 2020, at 7:03 AM, Ferruh Yigit <ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com>>> wrote:
>>>>> 
>>>>> On 12/4/2020 8:16 PM, Andrew Boyer wrote:
>>>>>> The UNMAINTAINED flag will be removed in a future patch.
>>>>>> Signed-off-by: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io> <mailto:aboyer@pensando.io <mailto:aboyer@pensando.io>>>
>>>>>> ---
>>>>>>  MAINTAINERS                        |  3 ++-
>>>>>>  doc/guides/nics/features/ionic.ini |  2 ++
>>>>>>  doc/guides/nics/ionic.rst          | 13 +++++++------
>>>>>>  3 files changed, 11 insertions(+), 7 deletions(-)
>>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>>> index eafe9f8c4..6534983c1 100644
>>>>>> --- a/MAINTAINERS
>>>>>> +++ b/MAINTAINERS
>>>>>> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>>>>>>  F: doc/guides/nics/features/pfe.ini
>>>>>>    Pensando ionic - UNMAINTAINED
>>>>>> -M: Alfredo Cardigliano <cardigliano@ntop.org <mailto:cardigliano@ntop.org> <mailto:cardigliano@ntop.org <mailto:cardigliano@ntop.org>>>
>>>>>> +M: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io> <mailto:aboyer@pensando.io <mailto:aboyer@pensando.io>>>
>>>>>> +M: Pensando Drivers <drivers@pensando.io <mailto:drivers@pensando.io> <mailto:drivers@pensando.io <mailto:drivers@pensando.io>>>
>>>>> 
>>>>> Same comment from previous version, please don't add group as maintainer, only actual people.
>>>> I responded to your original comment about this back in November. Is there an official DPDK policy against doing this? Is it your preference? We would very much prefer to have this in the file as a fallback. As long as there is still at least one person listed, what is the harm?
>>> 
>>> There is no official policy against it as far as I know.
>>> 
>>> The problem with the groups is we don't know who is behind it, it blurs who is the owner/responsible of the component. Actual people makes it clear that who is responsible.
>>> 
>>> Why do you prefer to add a group as maintainer?
>> Because if I am on leave for some reason, one of the other handful of maintainers might be able to help someone with a problem or a question.
>> If I am listed specifically, doesn’t that make clear “who is the owner/responsible” for ionic PMD? What harm does having drivers@ listed do?
> 
> If you want backups, pick and list a few of those handful maintainers and add to the list, so we can know who they are.
> 
> As already said problem is we don't know who is behind a group, if they are reliable or not, or is there really someone or not, what happens if group become silent?
> You can cc that group in your patches, that is commonly done, so they can be part of the development process, but the maintainers file is to define responsible people, adding a group is hiding actual responsible people.

Fine. Perhaps you could document somewhere that this requirement exists to save future people the trouble.


>>>>>>  F: drivers/net/ionic/
>>>>>>  F: doc/guides/nics/ionic.rst
>>>>>>  F: doc/guides/nics/features/ionic.ini
>>>>>> diff --git a/doc/guides/nics/features/ionic.ini b/doc/guides/nics/features/ionic.ini
>>>>>> index 083c7bd99..dd29dbed6 100644
>>>>>> --- a/doc/guides/nics/features/ionic.ini
>>>>>> +++ b/doc/guides/nics/features/ionic.ini
>>>>>> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>>>>>>  Link status          = Y
>>>>>>  Link status event    = Y
>>>>>>  Queue start/stop     = Y
>>>>>> +Lock-free Tx queue   = Y
>>>>> 
>>>>> Are you sure this is supported?
>>>>> Since it is not advertised as capability, I think this can't be claimed as supported, but still even after this is added as capability, can you please confirm your device supports multiple core enqueue to same queue without locks?
>>>> I misunderstood the meaning of this flag, will remove.
>>>>>>  MTU update           = Y
>>>>>>  Jumbo frame          = Y
>>>>>>  Scattered Rx         = Y
>>>>>> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>>>>>>  RSS hash             = Y
>>>>>>  RSS key update       = Y
>>>>>>  RSS reta update      = Y
>>>>>> +SR-IOV               = Y
>>>>> 
>>>>> Can you please explain what is exactly supported? Like can DPDK drive both PF & VF?
>>>> Yes. The PMD does not distinguish between PFs and VFs.
>>>>> 
>>>>> <...>
>>>>> 
>>>>>> @@ -7,15 +7,16 @@ IONIC Driver
>>>>>>  The ionic driver provides support for Pensando server adapters.
>>>>>>  It currently supports the below models:
>>>>>>  -- `Naples DSC-25 <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>>>`_
>>>>>> -- `Naples DSC-100 <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>>>`_
>>>>>> +- DSC-25 dual-port 25G Distributed Services Card
>>>>>> +- DSC-100 dual-port 100G Distributed Services Card
>>>>>> 
>>>>> 
>>>>> Same comment from previous version, can you provide link for these devices, it is hard to find the devices from the main site.
>>>> And my same response from your previous comment. I do not control the website and do not wish to put stale PDF links in this document, which will live forever. The text includes the URL of the page containing links to the PDFs. Why is this not acceptable?
>>> 
>>> The request is to put links to the products that you are providing the driver for. This is to help people that are already interested your driver and reading your driver document, to reach to the product information easily.
>>> 
>>> The request is NOT to provide pdf etc, just a reference to the product. Don't you advertise your product in your official web site? If your product information is not visible/hidden, why you are providing the open source drivers for it?
>> Does this line in the doc not satisfy your request?
>> +The `Documents <https://pensando.io/documents/ <https://pensando.io/documents/>>`_ page contains Product Briefs and other product information.
> 
> No it doesn't, it is not clear which document/product you are referring to.
> Why it is hard to provide the link of products that your driver is for?
> 

Fine. These links will become stale next time the files are updated, just like the links in the 20.02-20.11 releases are stale.

https://pensando.io/wp-content/uploads/2020/03/Pensando-DSC-25-Product-Brief.pdf
https://pensando.io/wp-content/uploads/2020/03/Pensando-DSC-100-Product-Brief.pdf

-Andrew
Ferruh Yigit Dec. 10, 2020, 9:23 a.m. UTC | #7
On 12/9/2020 7:05 PM, Andrew Boyer wrote:
> 
> 
>> On Dec 9, 2020, at 12:15 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 12/9/2020 4:24 PM, Andrew Boyer wrote:
>>>> On Dec 9, 2020, at 10:24 AM, Ferruh Yigit <ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com>> wrote:
>>>>
>>>> On 12/9/2020 2:36 PM, Andrew Boyer wrote:
>>>>> Please respond to my questions this time. I have ~70 more patches to post by December 20.
>>>>>> On Dec 9, 2020, at 7:03 AM, Ferruh Yigit <ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com> <mailto:ferruh.yigit@intel.com <mailto:ferruh.yigit@intel.com>>> wrote:
>>>>>>
>>>>>> On 12/4/2020 8:16 PM, Andrew Boyer wrote:
>>>>>>> The UNMAINTAINED flag will be removed in a future patch.
>>>>>>> Signed-off-by: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io> <mailto:aboyer@pensando.io <mailto:aboyer@pensando.io>>>
>>>>>>> ---
>>>>>>>   MAINTAINERS                        |  3 ++-
>>>>>>>   doc/guides/nics/features/ionic.ini |  2 ++
>>>>>>>   doc/guides/nics/ionic.rst          | 13 +++++++------
>>>>>>>   3 files changed, 11 insertions(+), 7 deletions(-)
>>>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>>>> index eafe9f8c4..6534983c1 100644
>>>>>>> --- a/MAINTAINERS
>>>>>>> +++ b/MAINTAINERS
>>>>>>> @@ -841,7 +841,8 @@ F: drivers/net/pfe/
>>>>>>>   F: doc/guides/nics/features/pfe.ini
>>>>>>>     Pensando ionic - UNMAINTAINED
>>>>>>> -M: Alfredo Cardigliano <cardigliano@ntop.org <mailto:cardigliano@ntop.org> <mailto:cardigliano@ntop.org <mailto:cardigliano@ntop.org>>>
>>>>>>> +M: Andrew Boyer <aboyer@pensando.io <mailto:aboyer@pensando.io> <mailto:aboyer@pensando.io <mailto:aboyer@pensando.io>>>
>>>>>>> +M: Pensando Drivers <drivers@pensando.io <mailto:drivers@pensando.io> <mailto:drivers@pensando.io <mailto:drivers@pensando.io>>>
>>>>>>
>>>>>> Same comment from previous version, please don't add group as maintainer, only actual people.
>>>>> I responded to your original comment about this back in November. Is there an official DPDK policy against doing this? Is it your preference? We would very much prefer to have this in the file as a fallback. As long as there is still at least one person listed, what is the harm?
>>>>
>>>> There is no official policy against it as far as I know.
>>>>
>>>> The problem with the groups is we don't know who is behind it, it blurs who is the owner/responsible of the component. Actual people makes it clear that who is responsible.
>>>>
>>>> Why do you prefer to add a group as maintainer?
>>> Because if I am on leave for some reason, one of the other handful of maintainers might be able to help someone with a problem or a question.
>>> If I am listed specifically, doesn’t that make clear “who is the owner/responsible” for ionic PMD? What harm does having drivers@ listed do?
>>
>> If you want backups, pick and list a few of those handful maintainers and add to the list, so we can know who they are.
>>
>> As already said problem is we don't know who is behind a group, if they are reliable or not, or is there really someone or not, what happens if group become silent?
>> You can cc that group in your patches, that is commonly done, so they can be part of the development process, but the maintainers file is to define responsible people, adding a group is hiding actual responsible people.
> 
> Fine. Perhaps you could document somewhere that this requirement exists to save future people the trouble.
> 
> 
>>>>>>>   F: drivers/net/ionic/
>>>>>>>   F: doc/guides/nics/ionic.rst
>>>>>>>   F: doc/guides/nics/features/ionic.ini
>>>>>>> diff --git a/doc/guides/nics/features/ionic.ini b/doc/guides/nics/features/ionic.ini
>>>>>>> index 083c7bd99..dd29dbed6 100644
>>>>>>> --- a/doc/guides/nics/features/ionic.ini
>>>>>>> +++ b/doc/guides/nics/features/ionic.ini
>>>>>>> @@ -8,6 +8,7 @@ Speed capabilities   = Y
>>>>>>>   Link status          = Y
>>>>>>>   Link status event    = Y
>>>>>>>   Queue start/stop     = Y
>>>>>>> +Lock-free Tx queue   = Y
>>>>>>
>>>>>> Are you sure this is supported?
>>>>>> Since it is not advertised as capability, I think this can't be claimed as supported, but still even after this is added as capability, can you please confirm your device supports multiple core enqueue to same queue without locks?
>>>>> I misunderstood the meaning of this flag, will remove.
>>>>>>>   MTU update           = Y
>>>>>>>   Jumbo frame          = Y
>>>>>>>   Scattered Rx         = Y
>>>>>>> @@ -19,6 +20,7 @@ Unicast MAC filter   = Y
>>>>>>>   RSS hash             = Y
>>>>>>>   RSS key update       = Y
>>>>>>>   RSS reta update      = Y
>>>>>>> +SR-IOV               = Y
>>>>>>
>>>>>> Can you please explain what is exactly supported? Like can DPDK drive both PF & VF?
>>>>> Yes. The PMD does not distinguish between PFs and VFs.
>>>>>>
>>>>>> <...>
>>>>>>
>>>>>>> @@ -7,15 +7,16 @@ IONIC Driver
>>>>>>>   The ionic driver provides support for Pensando server adapters.
>>>>>>>   It currently supports the below models:
>>>>>>>   -- `Naples DSC-25 <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf> <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>>>`_
>>>>>>> -- `Naples DSC-100 <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf> <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>>>`_
>>>>>>> +- DSC-25 dual-port 25G Distributed Services Card
>>>>>>> +- DSC-100 dual-port 100G Distributed Services Card
>>>>>>>
>>>>>>
>>>>>> Same comment from previous version, can you provide link for these devices, it is hard to find the devices from the main site.
>>>>> And my same response from your previous comment. I do not control the website and do not wish to put stale PDF links in this document, which will live forever. The text includes the URL of the page containing links to the PDFs. Why is this not acceptable?
>>>>
>>>> The request is to put links to the products that you are providing the driver for. This is to help people that are already interested your driver and reading your driver document, to reach to the product information easily.
>>>>
>>>> The request is NOT to provide pdf etc, just a reference to the product. Don't you advertise your product in your official web site? If your product information is not visible/hidden, why you are providing the open source drivers for it?
>>> Does this line in the doc not satisfy your request?
>>> +The `Documents <https://pensando.io/documents/ <https://pensando.io/documents/>>`_ page contains Product Briefs and other product information.
>>
>> No it doesn't, it is not clear which document/product you are referring to.
>> Why it is hard to provide the link of products that your driver is for?
>>
> 
> Fine. These links will become stale next time the files are updated, just like the links in the 20.02-20.11 releases are stale.
> 
> https://pensando.io/wp-content/uploads/2020/03/Pensando-DSC-25-Product-Brief.pdf
> https://pensando.io/wp-content/uploads/2020/03/Pensando-DSC-100-Product-Brief.pdf
> 

If you are sure that they will go stale, that is also not good solution. Why 
they are keep getting stale? Can't you communicate internally in your company 
for permanent link?

Patch
diff mbox series

diff --git a/MAINTAINERS b/MAINTAINERS
index eafe9f8c4..6534983c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -841,7 +841,8 @@  F: drivers/net/pfe/
 F: doc/guides/nics/features/pfe.ini
 
 Pensando ionic - UNMAINTAINED
-M: Alfredo Cardigliano <cardigliano@ntop.org>
+M: Andrew Boyer <aboyer@pensando.io>
+M: Pensando Drivers <drivers@pensando.io>
 F: drivers/net/ionic/
 F: doc/guides/nics/ionic.rst
 F: doc/guides/nics/features/ionic.ini
diff --git a/doc/guides/nics/features/ionic.ini b/doc/guides/nics/features/ionic.ini
index 083c7bd99..dd29dbed6 100644
--- a/doc/guides/nics/features/ionic.ini
+++ b/doc/guides/nics/features/ionic.ini
@@ -8,6 +8,7 @@  Speed capabilities   = Y
 Link status          = Y
 Link status event    = Y
 Queue start/stop     = Y
+Lock-free Tx queue   = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
@@ -19,6 +20,7 @@  Unicast MAC filter   = Y
 RSS hash             = Y
 RSS key update       = Y
 RSS reta update      = Y
+SR-IOV               = Y
 VLAN filter          = Y
 VLAN offload         = Y
 Flow control         = Y
diff --git a/doc/guides/nics/ionic.rst b/doc/guides/nics/ionic.rst
index fd32926bf..0b78cfbac 100644
--- a/doc/guides/nics/ionic.rst
+++ b/doc/guides/nics/ionic.rst
@@ -1,5 +1,5 @@ 
 ..  SPDX-License-Identifier: (BSD-3-Clause OR GPL-2.0)
-    Copyright(c) 2018-2019 Pensando Systems, Inc. All rights reserved.
+    Copyright(c) 2018-2020 Pensando Systems, Inc. All rights reserved.
 
 IONIC Driver
 ============
@@ -7,15 +7,16 @@  IONIC Driver
 The ionic driver provides support for Pensando server adapters.
 It currently supports the below models:
 
-- `Naples DSC-25 <https://pensando.io/assets/documents/Naples-25_ProductBrief_10-2019.pdf>`_
-- `Naples DSC-100 <https://pensando.io/assets/documents/Naples_100_ProductBrief-10-2019.pdf>`_
+- DSC-25 dual-port 25G Distributed Services Card
+- DSC-100 dual-port 100G Distributed Services Card
 
-Please visit https://pensando.io for more information.
+Please visit the Pensando web site at https://pensando.io for more information.
+The `Documents <https://pensando.io/documents/>`_ page contains Product Briefs and other product information.
 
 Identifying the Adapter
 -----------------------
 
-To find if one or more Pensando PCI Ethernet devices are installed
+To determine if one or more Pensando DSC Ethernet devices are installed
 on the host, check for the PCI devices:
 
    .. code-block:: console
@@ -28,6 +29,6 @@  on the host, check for the PCI devices:
 Building DPDK
 -------------
 
-The ionic PMD driver supports UIO and VFIO, please refer to the
+The ionic PMD supports UIO and VFIO. Please refer to the
 :ref:`DPDK documentation that comes with the DPDK suite <linux_gsg>`
 for instructions on how to build DPDK.