On Thu, Jun 1, 2017 at 11:16 AM, Ben Pfaff <[email protected]> wrote:
> On Thu, Jun 01, 2017 at 11:13:48AM -0700, Greg Rose wrote:
>> 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.
>
> You might have omitted the attachment.

Oops - I resent with the attachment but forgot to add back ovs-dev.
I'll get this right eventually...

Thanks,

- Greg
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to