On Thu, 2017-06-01 at 14:38 +0200, Eelco Chaudron wrote: > When trying to configure a system port as type=internal it could start > an infinite port creation loop. When this happens you will see the > following log messages: > > 2017-06-01T09:00:17.900Z|02813|dpif|WARN|system@ovs-system: failed to add > ve01_1 as port: File exists > 2017-06-01T09:00:17.900Z|02814|bridge|WARN|could not add network device > ve01_1 to ofproto (File exists) > 2017-06-01T09:00:17.907Z|02815|bridge|INFO|bridge bzb: added interface ve01_1 > on port 2 > 2017-06-01T09:00:17.909Z|02816|bridge|INFO|bridge bzb: deleted interface > ve01_1 on port 2 > 2017-06-01T09:00:17.914Z|02817|dpif|WARN|system@ovs-system: failed to add > ve01_1 as port: File exists > 2017-06-01T09:00:17.914Z|02818|bridge|WARN|could not add network device > ve01_1 to ofproto (File exists) > 2017-06-01T09:00:17.921Z|02819|bridge|INFO|bridge bzb: added interface ve01_1 > on port 3 > 2017-06-01T09:00:17.923Z|02820|bridge|INFO|bridge bzb: deleted interface > ve01_1 on port 3 > 2017-06-01T09:00:17.929Z|02821|dpif|WARN|system@ovs-system: failed to add > ve01_1 as port: File exists > 2017-06-01T09:00:17.929Z|02822|bridge|WARN|could not add network device > ve01_1 to ofproto (File exists) > 2017-06-01T09:00:17.936Z|02823|bridge|INFO|bridge bzb: added interface ve01_1 > on port 4 > ... > ... > > This is how to replicate it: > > ip link add name ve01_1 type veth peer name ve01_2 > ovs-vsctl add-br bzb > ovs-vsctl add-port bzb ve01_1 > ovs-vsctl set interface ve01_1 type=internal > ip link set dev ve01_1 up > ip link set dev ve01_2 up > > When changing the type to internal, the async configuration logic get > triggered and because the type has changed it will delete the > interface and the ofproto port. Next it will call iface_do_create() to > re-create the interface as internal. Because we just deleted the > interface netdev_open() will try to recreate it as internal. > > However this will fail with EEXIST as a system interface already > exists withe the name. > > Up till here all is fine... > > Now some ipv6 route change comes along for the ve01_1 interface, and > the route infrastructure will call netdev_open(). This will create the > interface of type system. > > Next the configuration verify process gets triggered due to > if_notifier_changed() being true. We now retry the above, but because > the interface exists (although in the system class) it will use it, > and create the interface successfully. > > This triggers another if notification, causing yet another config > update, and because the system != internal reconfiguration happens and > it start from the top... > > So the fix as presented below is causing netdev_open() only to return > the existing device for the class type requested (if the type is > specified). > > Signed-off-by: Eelco Chaudron <[email protected]> > --- > lib/netdev.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/netdev.c b/lib/netdev.c > index 26c4136..ca3192a 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -412,7 +412,11 @@ netdev_open(const char *name, const char *type, struct > netdev **netdevp) > error = EAFNOSUPPORT; > } > } else { > - error = 0; > + if (type && type[0] && strcmp(type, netdev->netdev_class->type)) { > + error = EEXIST; > + } else { > + error = 0; > + } > } > > if (!error) {
I tested this patch and it does fix the problem but how about the patch that I've attached to this reply instead? It seems a bit cleaner. Thanks, - Greg
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
