Re: [PATCH net-next 0/1] openvswitch: allow specifying ifindex of new interfaces

[Christian Brauner <brauner@kernel.org>]

On Wed, Aug 17, 2022 at 03:49:08PM +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

Ah right, I remember that. :)

> 
> 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.
> 
> 
> As a broader solution, another generic approach is possible: modify
> __dev_change_net_namespace() to allow changing ifindexes within the same
> netns. Yet we will still need to ignore NETIF_F_NETNS_LOCAL and I am not
> sure that all its users are ready for ifindex change.

I think that might become confusing. We already have issues - even with
the tracking infrastucture - to keep track of ifindex changes when a
network device is moved between network namespaces multiple times. So
I'd rather not make it possible to change the ifindex at will within the
same network namespace though I understand the appeal for CRIU.

> This will be indeed better for CRIU so we won't need to change every SDN
> module to be able to checkpoint/restore it. And probably avoid some bloat.
> What do you think of this?
> 
> Andrey Zhadchenko (1):
>   openvswitch: allow specifying ifindex of new interfaces
> 
>  include/uapi/linux/openvswitch.h     |  4 ++++
>  net/openvswitch/datapath.c           | 16 ++++++++++++++--
>  net/openvswitch/vport-internal_dev.c |  1 +
>  net/openvswitch/vport.h              |  2 ++
>  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> -- 
> 2.31.1
>