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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 17 Aug 2022 15:09:45 +0200
From:   Christian Brauner <brauner@...nel.org>
To:     Andrey Zhadchenko <andrey.zhadchenko@...tuozzo.com>
Cc:     netdev@...r.kernel.org, dev@...nvswitch.org, pshelar@....org,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, ptikhomirov@...tuozzo.com,
        alexander.mikhalitsyn@...tuozzo.com, avagin@...gle.com
Subject: Re: [PATCH net-next 0/1] openvswitch: allow specifying ifindex of
 new interfaces

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
>