[00/10] kni: Interface detach and link status fixes.

Message ID 20180628224513.18391-1-dg@adax.com (mailing list archive)
Headers
Series kni: Interface detach and link status fixes. |

Message

Dan Gora June 28, 2018, 10:45 p.m. UTC
  Hi All,

The following patches are to fix a problem with detaching a KNI
interface using rte_kni_release and to add a new API function
to allow users to change the link status (up/down, speed, etc)
of the interface in the linux kernel.  In previous versions, it
was impossible to release a KNI interface without waiting for the
timeout in kni_net_process_request to expire if the interface is in
the UP state.  The solution to this issue was to separate the process
of releasing the netdev device from the linux kernel from actually
freeing the KNI interface in the kernel and in the RTE library by
introducing a new API function, rte_kni_free().

The last patch : 'kni: add API to set link status on kernel interface'
adds a new API function to allow the DPDK user to change the link
speed and status reported by the netdev in the linux kernel.

This resolves issues with allowing automatic network configuration
applciations such as NetworkManager to assign addresses and for user
space applications to be able to open sockets on these interfaces,
as some operations rely on the link status being up before they
work properly.

This last patch is included in this series because both new "features"
introduce new ioctls to the rte_kni kernel module, so the order in
which the patches get applied affects which number each new ioctl gets.
I thought it better to bundle them together to try to get them applied
as a series to avoid any issues with this.

Please have a look!

thanks
dan

Dan Gora (10):
  kni: remove unused variables from struct kni_dev
  kni: separate releasing netdev from freeing KNI interface
  kni: don't touch struct kni_dev after freeing
  kni: add rte_kni_free to KNI library
  kni: don't run rte_kni_handle_request after interface release
  kni: increase length of timeout for KNI responses
  kni: update kni test for rte_kni_free
  kni: add rte_kni_free to KNI example app
  kni: add rte_kni_free to KNI vdev driver
  kni: add API to set link status on kernel interface

 drivers/net/kni/rte_eth_kni.c                 |   6 +-
 examples/kni/main.c                           |   4 +-
 kernel/linux/kni/kni_dev.h                    |   5 +-
 kernel/linux/kni/kni_misc.c                   | 156 ++++++++++++++++--
 kernel/linux/kni/kni_net.c                    |   7 +-
 .../eal/include/exec-env/rte_kni_common.h     |  20 +++
 lib/librte_kni/rte_kni.c                      |  75 ++++++++-
 lib/librte_kni/rte_kni.h                      |  51 +++++-
 test/test/test_kni.c                          |  20 +++
 9 files changed, 313 insertions(+), 31 deletions(-)
  

Comments

Dan Gora June 29, 2018, 1:54 a.m. UTC | #1
Hi All,

The following patches are to fix a problem with detaching a KNI
interface using rte_kni_release and to add a new API function
to allow users to change the link status (up/down, speed, etc)
of the interface in the linux kernel.  In previous versions, it
was impossible to release a KNI interface without waiting for the
timeout in kni_net_process_request to expire if the interface is in
the UP state.  The solution to this issue was to separate the process
of releasing the netdev device from the linux kernel from actually
freeing the KNI interface in the kernel and in the RTE library by
introducing a new API function, rte_kni_free().

The last patch : 'kni: add API to set link status on kernel interface'
adds a new API function to allow the DPDK user to change the link
speed and status reported by the netdev in the linux kernel.

This resolves issues with allowing automatic network configuration
applciations such as NetworkManager to assign addresses and for user
space applications to be able to open sockets on these interfaces,
as some operations rely on the link status being up before they
work properly.

This last patch is included in this series because both new "features"
introduce new ioctls to the rte_kni kernel module, so the order in
which the patches get applied affects which number each new ioctl gets.
I thought it better to bundle them together to try to get them applied
as a series to avoid any issues with this.

*v2*

The first time I submitted these patches, they failed with
compilation errors in Patchwork, but they are failing due to
rte_kni_free being a missing symbol, which is only introduced here in
patch 4/10.  I'm not really sure how the automatically compilation
tool works in patchwork.  Does it apply all of the patches in the
series before compilation?  Does it compile after each patch?

I tried my best to break up these patches into small enough pieces
so that they could be reviewed fairly easily, but I'm not sure that
I did a good job.  If there are suggestions on how the changes could
be organized or split better, please let me know.

thanks
dan

v2:
  Re-submitted as a threaded series as per thomas@monjalon.net suggestions.
  cc'd Ferruh on cover letter.
  No code changes.

Dan Gora (10):
  kni: remove unused variables from struct kni_dev
  kni: separate releasing netdev from freeing KNI interface
  kni: don't touch struct kni_dev after freeing
  kni: add rte_kni_free to KNI library
  kni: don't run rte_kni_handle_request after interface release
  kni: increase length of timeout for KNI responses
  kni: update kni test for rte_kni_free
  kni: add rte_kni_free to KNI example app
  kni: add rte_kni_free to KNI vdev driver
  kni: add API to set link status on kernel interface

 drivers/net/kni/rte_eth_kni.c                 |   6 +-
 examples/kni/main.c                           |   4 +-
 kernel/linux/kni/kni_dev.h                    |   5 +-
 kernel/linux/kni/kni_misc.c                   | 156 ++++++++++++++++--
 kernel/linux/kni/kni_net.c                    |   7 +-
 .../eal/include/exec-env/rte_kni_common.h     |  20 +++
 lib/librte_kni/rte_kni.c                      |  75 ++++++++-
 lib/librte_kni/rte_kni.h                      |  51 +++++-
 test/test/test_kni.c                          |  20 +++
 9 files changed, 313 insertions(+), 31 deletions(-)
  
Ferruh Yigit July 20, 2018, 11:36 a.m. UTC | #2
On 6/29/2018 2:54 AM, Dan Gora wrote:
> Hi All,
> 
> The following patches are to fix a problem with detaching a KNI
> interface using rte_kni_release and to add a new API function
> to allow users to change the link status (up/down, speed, etc)
> of the interface in the linux kernel.  In previous versions, it
> was impossible to release a KNI interface without waiting for the
> timeout in kni_net_process_request to expire if the interface is in
> the UP state.  The solution to this issue was to separate the process
> of releasing the netdev device from the linux kernel from actually
> freeing the KNI interface in the kernel and in the RTE library by
> introducing a new API function, rte_kni_free().
> 
> The last patch : 'kni: add API to set link status on kernel interface'
> adds a new API function to allow the DPDK user to change the link
> speed and status reported by the netdev in the linux kernel.
> 
> This resolves issues with allowing automatic network configuration
> applciations such as NetworkManager to assign addresses and for user
> space applications to be able to open sockets on these interfaces,
> as some operations rely on the link status being up before they
> work properly.
> 
> This last patch is included in this series because both new "features"
> introduce new ioctls to the rte_kni kernel module, so the order in
> which the patches get applied affects which number each new ioctl gets.
> I thought it better to bundle them together to try to get them applied
> as a series to avoid any issues with this.
> 
> *v2*
> 
> The first time I submitted these patches, they failed with
> compilation errors in Patchwork, but they are failing due to
> rte_kni_free being a missing symbol, which is only introduced here in
> patch 4/10.  I'm not really sure how the automatically compilation
> tool works in patchwork.  Does it apply all of the patches in the
> series before compilation?  Does it compile after each patch?
> 
> I tried my best to break up these patches into small enough pieces
> so that they could be reviewed fairly easily, but I'm not sure that
> I did a good job.  If there are suggestions on how the changes could
> be organized or split better, please let me know.
> 
> thanks
> dan
> 
> v2:
>   Re-submitted as a threaded series as per thomas@monjalon.net suggestions.
>   cc'd Ferruh on cover letter.
>   No code changes.
> 
> Dan Gora (10):
>   kni: remove unused variables from struct kni_dev
>   kni: separate releasing netdev from freeing KNI interface
>   kni: don't touch struct kni_dev after freeing
>   kni: add rte_kni_free to KNI library
>   kni: don't run rte_kni_handle_request after interface release
>   kni: increase length of timeout for KNI responses
>   kni: update kni test for rte_kni_free
>   kni: add rte_kni_free to KNI example app
>   kni: add rte_kni_free to KNI vdev driver
>   kni: add API to set link status on kernel interface

Hi Dan,

Thank you for the patches, but they were later for this release proposal
deadline and will be considered/reviewed for next release.

Thanks,
ferruh