Re: [PATCH 5/7] network: NetworkManager script to monitor/resolve conflicts with new interfaces

On 8/7/24 1:32 PM, Daniel P. Berrangé wrote: > On Wed, Aug 07, 2024 at 01:16:01PM -0400, Laine Stump wrote: > > + > > +import libvirt > > +import sys > > +import os > > +import libxml2 > > +from ipaddress import ip_network > > + > > +# This script should be installed in > > +# /usr/lib/NetworkManager/dispatcher.d/50-libvirt-check-nets. It will be > > +# called by NetworkManager every time a network interface is taken up > > +# or down. When a new network comes up, it checks the libvirt virtual > > +# networks to see if their IP address(es) (including any static > > +# routes) are in conflict with the IP address(es) (or static routes) > > +# of the newly added interface. If so, the libvirt network is > > +# disabled. It is assumed that the user will notice that their guests > > +# no longer have network connectvity (and/or the message logged by > > +# this script), see that the network has been disabled, and then > > +# realize the conflict when they try to restart it. > > +# > > +# set checkDefaultOnly=False to check *all* active libvirt networks > > +# for conflicts with the new interface. Set to True to check only the > > +# libvirt default network (since most networks other than the default > > +# network are added post-install at a time when all of the hosts other > > +# networks are already active, it may be overkill to check all of the > > +# libvirt networks for conflict here (and instead just add more > > +# needless overheard to bringing up a new host interface). > > +# > > +checkDefaultOnly = False > > + > > +# NB: since this file is installed in /usr/lib, it really shouldn't be > > +# modified by the user, but instead should be copied to > > +# /etc/NetworkManager/dispatcher.d, where it will override the copy in > > +# /usr/lib. Even that isn't a proper solution though - if we're going > > +# to actually have this config knob, perhaps we should check for it in > > +# the environment, and if someone wants to modify it they can put a > > +# short script in /etc that exports and environment variable and then > > +# calls this script? Just thinking out loud here. > > + > > + > > +def checkconflict(conn, netname, hostnets, hostif): > > ...snip complex code... > > It occurrs to me that this python code is entirely duplicating > logic that already exists in virtnetworkd that checks for > conflicts. Possibly not *exactly* what's in virtnetworkd, since I'm not certain if NM includes all static routes in the list it puts in the environment variables (while they *would* show up in the system routing table that our network driver uses). Of course that sounds like another reason to prefer re-using the existing code!

> This makes me want to figure out a way to just re-use the existing > code rather than having to write it again. > > Could we just send a SIGUSR2 to virtnetworkd as a way to trigger it > to re-validate all running networks ? That would be easy to trigger > from non-network manager setups too. That sounds simpler (and probably quicker), and if we could do it that I would prefer it ("only works with NetworkManager" is #1 on my list of 'issues' above after all :-)), but what would we do in the case that virtnetworkd (or libvirtd) wasn't currently running? Run some virsh command to trigger virtnetworkd to start, and then send the SIGUSR2?

(this does > But then how about ignoring network manager entirely and going right > to the source, ie the kernel. Does it send out either udev or netlink > events (probably the latter), when NICs have their link status set > online/offline ? If so, virtnetworkd could just listen to the kernel > events and "do the right thing", regardless of what tool is managing > the NICs. I imagine *something* is sent out, and it's definitely worth me investigating. But wouldn't virtnetworkd have to be running all of the time in order to use something like that?