[net-next,v3,0/2] openvswitch: allow specifying ifindex of new interfaces

Message

Andrey Zhadchenko Aug. 25, 2022, 2:04 a.m. UTC

Hi!

CRIU currently do not support checkpoint/restore of OVS configurations, but
there was several requests for it. For example,
https://github.com/lxc/lxc/issues/2909

The main problem is ifindexes of newly created interfaces. We realy need to
preserve them after restore. Current openvswitch API does not allow to
specify ifindex. Most of the time we can just create an interface via
generic netlink requests and plug it into ovs but datapaths (generally any
OVS_VPORT_TYPE_INTERNAL) can only be created via openvswitch requests which
do not support selecting ifindex.

This patch allows to do so.
For new datapaths I decided to use dp_infindex in header as infindex
because it control ifindex for other requests too.
For internal vports I reused OVS_VPORT_ATTR_IFINDEX.

The only concern I have is that previously dp_ifindex was not used for
OVS_DP_VMD_NEW requests and some software may not set it to zero. However
we have been running this patch at Virtuozzo for 2 years and have not
encountered this problem. Not sure if it is worth to add new
ovs_datapath_attr instead.

v2:
Added two more patches.

Add OVS_DP_ATTR_PER_CPU_PIDS to dumps as suggested by Ilya Maximets.
Without it we won't be able to checkpoint/restore new openvswitch
configurations which use OVS_DP_F_DISPATCH_UPCALL_PER_CPU flag.

Found and fixed memory leak on datapath creation error path.

v3:
Sent memleak fix separately to net.
Improved patches according to the reviews:
 - Added new OVS_DP_ATTR_IFINDEX instead of using ovs_header->dp_ifindex
 - Pre-allocated bigger reply message for upcall pids
 - Some small fixes

Andrey Zhadchenko (2):
  openvswitch: allow specifying ifindex of new interfaces
  openvswitch: add OVS_DP_ATTR_PER_CPU_PIDS to get requests

 include/uapi/linux/openvswitch.h     |  3 +++
 net/openvswitch/datapath.c           | 21 ++++++++++++++++++---
 net/openvswitch/vport-internal_dev.c |  1 +
 net/openvswitch/vport.h              |  2 ++
 4 files changed, 24 insertions(+), 3 deletions(-)

Comments

Christian Brauner Aug. 26, 2022, 9:11 a.m. UTC | #1

On Thu, Aug 25, 2022 at 05:04:48AM +0300, Andrey Zhadchenko wrote:
> Hi!
> 
> CRIU currently do not support checkpoint/restore of OVS configurations, but
> there was several requests for it. For example,
> https://github.com/lxc/lxc/issues/2909
> 
> The main problem is ifindexes of newly created interfaces. We realy need to
> preserve them after restore. Current openvswitch API does not allow to
> specify ifindex. Most of the time we can just create an interface via
> generic netlink requests and plug it into ovs but datapaths (generally any
> OVS_VPORT_TYPE_INTERNAL) can only be created via openvswitch requests which
> do not support selecting ifindex.
> 
> This patch allows to do so.
> For new datapaths I decided to use dp_infindex in header as infindex
> because it control ifindex for other requests too.
> For internal vports I reused OVS_VPORT_ATTR_IFINDEX.
> 
> The only concern I have is that previously dp_ifindex was not used for
> OVS_DP_VMD_NEW requests and some software may not set it to zero. However
> we have been running this patch at Virtuozzo for 2 years and have not
> encountered this problem. Not sure if it is worth to add new
> ovs_datapath_attr instead.
> 
> v2:
> Added two more patches.
> 
> Add OVS_DP_ATTR_PER_CPU_PIDS to dumps as suggested by Ilya Maximets.
> Without it we won't be able to checkpoint/restore new openvswitch
> configurations which use OVS_DP_F_DISPATCH_UPCALL_PER_CPU flag.
> 
> Found and fixed memory leak on datapath creation error path.
> 
> v3:
> Sent memleak fix separately to net.
> Improved patches according to the reviews:
>  - Added new OVS_DP_ATTR_IFINDEX instead of using ovs_header->dp_ifindex
>  - Pre-allocated bigger reply message for upcall pids
>  - Some small fixes

Seems good,
Acked-by: Christian Brauner (Microsoft) <[e-mail]>

[e-mail] Aug. 27, 2022, 2:50 a.m. UTC | #2

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <[e-mail]>:

On Thu, 25 Aug 2022 05:04:48 +0300 you wrote:
> Hi!
> 
> CRIU currently do not support checkpoint/restore of OVS configurations, but
> there was several requests for it. For example,
> https://github.com/lxc/lxc/issues/2909
> 
> The main problem is ifindexes of newly created interfaces. We realy need to
> preserve them after restore. Current openvswitch API does not allow to
> specify ifindex. Most of the time we can just create an interface via
> generic netlink requests and plug it into ovs but datapaths (generally any
> OVS_VPORT_TYPE_INTERNAL) can only be created via openvswitch requests which
> do not support selecting ifindex.
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/2] openvswitch: allow specifying ifindex of new interfaces
    https://git.kernel.org/netdev/net-next/c/54c4ef34c4b6
  - [net-next,v3,2/2] openvswitch: add OVS_DP_ATTR_PER_CPU_PIDS to get requests
    https://git.kernel.org/netdev/net-next/c/347541e299d5

You are awesome, thank you!