[v4,0/6] kni: add API to set link status on kernel interface

  Hi All,

Attached is version 4 of a patchset to add a new API function to
set the link status on kernel interfaces created with the KNI kernel
module.

v4
====
* Rework rte_kni_update_link to only take linkup/linkdown as parameter,
  return previous link state, and remove log messages.

* Update patch to set default carrier state to make default carrier
  state configurable by passing the 'carrier=[on|off]' option to
  the rte_kni kernel module.  This is necessary in order to allow
  applications which use KNI as pure virtual interfaces without
  corresponding physical ethernet port to use the interfaces without
  having to set the carrier state to 'on' via rte_kni_update_link()
  or by writing to /sys/devices/virtual/net/<ifaceX>/carrier.
  Note that the default is 'off'.

* Add command line flag '-m' to examples/kni to continuously monitor
  and update the KNI interface link status according to the link
  status of the corresponding physical ethernet port.


> v3
> ====
> * Use separate function to test rte_kni_update_link() in 'test' app.
> 
> * Separate changes to 'test' app into separate patch to facilitate
>   possible merge with https://patches.dpdk.org/patch/44730/
> 
> * Remove changes to set KNI interfaces to 'up' in example/kni
> 
> v2
> ====
> 
> * Fix bug where "Fixed" and "AutoNeg" were transposed in the link
>   status log message.
> 
> * Add rte_kni_update_link() to rte_kni_version.map
> 
> * Add rte_kni_update_link() tests to kni_autotest
> 
> * Update examples/kni to continuously monitor link status and
>   update the corresponding kernel interface with
>   rte_kni_update_link().
> 
> * Minor improvements to examples/kni: Add log message showing how
>   to show/zero stats.  Improve zeroing statistics.
> 
> Note that checkpatches.sh compains about patch 1/5, but this appears
> to be a bug with check-symbol-change or something.  If I move the
> fragment of the patch modifying rte_kni_version.map to the bottom of
> the patch file, it doesn't complain any more...  I just don't really
> have time to investigate this right now.
  
thanks
dan


Dan Gora (6):
  kni: add API to set link status on kernel interface
  kni: add link status test
  kni: set default carrier state of interface
  examples/kni: monitor and update link status continually
  examples/kni: add log msgs to show and clear stats
  examples/kni: improve zeroing statistics

 examples/kni/Makefile              |  2 +
 examples/kni/main.c                | 92 +++++++++++++++++++++++++++---
 kernel/linux/kni/kni_dev.h         |  3 +
 kernel/linux/kni/kni_misc.c        | 40 +++++++++++++
 kernel/linux/kni/kni_net.c         |  5 ++
 lib/librte_kni/rte_kni.c           | 41 +++++++++++++
 lib/librte_kni/rte_kni.h           | 20 +++++++
 lib/librte_kni/rte_kni_version.map |  6 ++
 test/test/test_kni.c               | 77 +++++++++++++++++++++++++
 9 files changed, 279 insertions(+), 7 deletions(-)
  

On 10/17/2018 2:04 AM, Dan Gora wrote:
> Hi All,
> 
> Attached is version 4 of a patchset to add a new API function to
> set the link status on kernel interfaces created with the KNI kernel
> module.
> 
> v4
> ====
> * Rework rte_kni_update_link to only take linkup/linkdown as parameter,
>   return previous link state, and remove log messages.
> 
> * Update patch to set default carrier state to make default carrier
>   state configurable by passing the 'carrier=[on|off]' option to
>   the rte_kni kernel module.  This is necessary in order to allow
>   applications which use KNI as pure virtual interfaces without
>   corresponding physical ethernet port to use the interfaces without
>   having to set the carrier state to 'on' via rte_kni_update_link()
>   or by writing to /sys/devices/virtual/net/<ifaceX>/carrier.
>   Note that the default is 'off'.
> 
> * Add command line flag '-m' to examples/kni to continuously monitor
>   and update the KNI interface link status according to the link
>   status of the corresponding physical ethernet port.
> 
> 
>> v3
>> ====
>> * Use separate function to test rte_kni_update_link() in 'test' app.
>>
>> * Separate changes to 'test' app into separate patch to facilitate
>>   possible merge with https://patches.dpdk.org/patch/44730/
>>
>> * Remove changes to set KNI interfaces to 'up' in example/kni
>>
>> v2
>> ====
>>
>> * Fix bug where "Fixed" and "AutoNeg" were transposed in the link
>>   status log message.
>>
>> * Add rte_kni_update_link() to rte_kni_version.map
>>
>> * Add rte_kni_update_link() tests to kni_autotest
>>
>> * Update examples/kni to continuously monitor link status and
>>   update the corresponding kernel interface with
>>   rte_kni_update_link().
>>
>> * Minor improvements to examples/kni: Add log message showing how
>>   to show/zero stats.  Improve zeroing statistics.
>>
>> Note that checkpatches.sh compains about patch 1/5, but this appears
>> to be a bug with check-symbol-change or something.  If I move the
>> fragment of the patch modifying rte_kni_version.map to the bottom of
>> the patch file, it doesn't complain any more...  I just don't really
>> have time to investigate this right now.
>   
> thanks
> dan
> 
> 
> Dan Gora (6):
>   kni: add API to set link status on kernel interface
>   kni: add link status test
>   kni: set default carrier state of interface
>   examples/kni: monitor and update link status continually
>   examples/kni: add log msgs to show and clear stats
>   examples/kni: improve zeroing statistics

Hi Dan,

Overall looks good to me, thanks for the updates.

Only it is missing some documentation, I think it is good to document:
1- KNI interface by default carrier will be off
2- New kni module parameter "carrier"
3- New sample application "-m" parameter.

I think 2 & 3 can be documented in "Kernel NIC Interface Sample Application",
unfortunately module parameters seems only documented here.

And 1 in "Kernel NIC Interface", perhaps in "KNI Creation and Deletion" section
as a detail to creation part.

Please embed documentation updates into patches that does the implementation,
instead of separate documentation patch.

And there is an issue on 3/6 to fix.

Thanks,
ferruh