netdev - Re: [PATCH net-next v3 0/2] 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]

Message-ID: <20220826091147.eechwvoa6eckhuq4@wittgenstein>
Date:   Fri, 26 Aug 2022 11:11:47 +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,
        i.maximets@....org, aconole@...hat.com
Subject: Re: [PATCH net-next v3 0/2] openvswitch: allow specifying ifindex of
 new interfaces

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) <brauner@...nel.org>