Message ID | 1496194243-11124-1-git-send-email-fukaige@huawei.com |
---|---|
State | Changes Requested |
Headers | show |
On Wed, May 31, 2017 at 09:30:43AM +0800, fukaige wrote: > Start a virtual machine with its backend tap device attached to a brought up linux bridge. > If we delete the linux bridge when vm is still running, we'll get the following error when > trying to create a ovs bridge with the same name. > > The reason is that ovs-router subsystem add the linux bridge into netdev_shash, but does > not remove it when the bridge is deleted in the situation. When the bridge is deleted, ovs > will receive a RTM_DELLINK msg, take this chance to remove the bridge in netdev_shash. > > ovs-vsctl: Error detected while setting up 'br-eth'. See ovs-vswitchd log for details. > > ovs-vswitchd log: > 2017-05-11T03:45:25.293Z|00026|ofproto_dpif|INFO|system@ovs-system: Datapath supports recirculation > 2017-05-11T03:45:25.293Z|00027|ofproto_dpif|INFO|system@ovs-system: MPLS label stack length probed as 1 > 2017-05-11T03:45:25.293Z|00028|ofproto_dpif|INFO|system@ovs-system: Datapath supports unique flow ids > 2017-05-11T03:45:25.293Z|00029|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_state > 2017-05-11T03:45:25.293Z|00030|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_zone > 2017-05-11T03:45:25.293Z|00031|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_mark > 2017-05-11T03:45:25.293Z|00032|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_label > 2017-05-11T03:45:25.364Z|00001|ofproto_dpif_upcall(handler226)|INFO|received packet on unassociated datapath port 0 > 2017-05-11T03:45:25.368Z|00033|netdev_linux|WARN|ethtool command ETHTOOL_GFLAGS on network device br-eth failed: No such device > 2017-05-11T03:45:25.368Z|00034|dpif|WARN|system@ovs-system: failed to add br-eth as port: No such device > 2017-05-11T03:45:25.368Z|00035|bridge|INFO|bridge br-eth: using datapath ID 00002a51cf9f2841 > 2017-05-11T03:45:25.368Z|00036|connmgr|INFO|br-eth: added service controller "punix:/var/run/openvswitch/br-eth.mgmt" > > Change-Id: Ib5ead59bc91453f83549da89937c0d3607e0385e > Signed-off-by: fukaige <fukaige@huawei.com> Thanks for identifying the problem and coming up with a fix. The fix looks to me like it works at the wrong level of abstraction. netdev-linux and tnl-port-map are not directly related, and I believe that the same problem could arise with other kinds of netdevs and tnl-port-map. So, I think that it would be better if tnl-ports itself registered a callback upon device destruction, for example via a call to if_notifier_create() from tnl_port_map_init(). Thanks, Ben.
Thanks for review, I will consider your suggestion and push v2 patch as soon as possible. Thanks fukaige > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Friday, June 09, 2017 4:35 AM > To: fukaige > Cc: dev@openvswitch.org > Subject: Re: [PATCH v1] netdev-linux: Remove device in netdev_shash when > deleted > > On Wed, May 31, 2017 at 09:30:43AM +0800, fukaige wrote: > > Start a virtual machine with its backend tap device attached to a brought up > linux bridge. > > If we delete the linux bridge when vm is still running, we'll get the > > following error when trying to create a ovs bridge with the same name. > > > > The reason is that ovs-router subsystem add the linux bridge into > > netdev_shash, but does not remove it when the bridge is deleted in the > > situation. When the bridge is deleted, ovs will receive a RTM_DELLINK msg, > take this chance to remove the bridge in netdev_shash. > > > > ovs-vsctl: Error detected while setting up 'br-eth'. See ovs-vswitchd log for > details. > > > > ovs-vswitchd log: > > 2017-05-11T03:45:25.293Z|00026|ofproto_dpif|INFO|system@ovs-system: > > Datapath supports recirculation > > 2017-05-11T03:45:25.293Z|00027|ofproto_dpif|INFO|system@ovs-system: > > MPLS label stack length probed as 1 > > 2017-05-11T03:45:25.293Z|00028|ofproto_dpif|INFO|system@ovs-system: > > Datapath supports unique flow ids > > 2017-05-11T03:45:25.293Z|00029|ofproto_dpif|INFO|system@ovs-system: > > Datapath supports ct_state > > 2017-05-11T03:45:25.293Z|00030|ofproto_dpif|INFO|system@ovs-system: > > Datapath supports ct_zone > > 2017-05-11T03:45:25.293Z|00031|ofproto_dpif|INFO|system@ovs-system: > > Datapath supports ct_mark > > 2017-05-11T03:45:25.293Z|00032|ofproto_dpif|INFO|system@ovs-system: > > Datapath supports ct_label > > 2017-05-11T03:45:25.364Z|00001|ofproto_dpif_upcall(handler226)|INFO|re > > ceived packet on unassociated datapath port 0 > > 2017-05-11T03:45:25.368Z|00033|netdev_linux|WARN|ethtool command > > ETHTOOL_GFLAGS on network device br-eth failed: No such device > > 2017-05-11T03:45:25.368Z|00034|dpif|WARN|system@ovs-system: failed > to > > add br-eth as port: No such device > > 2017-05-11T03:45:25.368Z|00035|bridge|INFO|bridge br-eth: using > > datapath ID 00002a51cf9f2841 > > 2017-05-11T03:45:25.368Z|00036|connmgr|INFO|br-eth: added service > controller "punix:/var/run/openvswitch/br-eth.mgmt" > > > > Change-Id: Ib5ead59bc91453f83549da89937c0d3607e0385e > > Signed-off-by: fukaige <fukaige@huawei.com> > > Thanks for identifying the problem and coming up with a fix. > > The fix looks to me like it works at the wrong level of abstraction. > netdev-linux and tnl-port-map are not directly related, and I believe that the > same problem could arise with other kinds of netdevs and tnl-port-map. So, I > think that it would be better if tnl-ports itself registered a callback upon device > destruction, for example via a call to > if_notifier_create() from tnl_port_map_init(). > > Thanks, > > Ben.
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 1e03c75..fae78e5 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -73,6 +73,7 @@ #include "timer.h" #include "unaligned.h" #include "openvswitch/vlog.h" +#include "tnl-ports.h" VLOG_DEFINE_THIS_MODULE(netdev_linux); @@ -749,6 +750,7 @@ netdev_linux_update(struct netdev_linux *dev, dev->get_ifindex_error = 0; } else { netdev_linux_changed(dev, change->ifi_flags, 0); + tnl_port_map_delete_ipdev(dev->up.name); } } else if (rtnetlink_type_is_rtnlgrp_addr(change->nlmsg_type)) { /* Invalidates in4, in6. */
Start a virtual machine with its backend tap device attached to a brought up linux bridge. If we delete the linux bridge when vm is still running, we'll get the following error when trying to create a ovs bridge with the same name. The reason is that ovs-router subsystem add the linux bridge into netdev_shash, but does not remove it when the bridge is deleted in the situation. When the bridge is deleted, ovs will receive a RTM_DELLINK msg, take this chance to remove the bridge in netdev_shash. ovs-vsctl: Error detected while setting up 'br-eth'. See ovs-vswitchd log for details. ovs-vswitchd log: 2017-05-11T03:45:25.293Z|00026|ofproto_dpif|INFO|system@ovs-system: Datapath supports recirculation 2017-05-11T03:45:25.293Z|00027|ofproto_dpif|INFO|system@ovs-system: MPLS label stack length probed as 1 2017-05-11T03:45:25.293Z|00028|ofproto_dpif|INFO|system@ovs-system: Datapath supports unique flow ids 2017-05-11T03:45:25.293Z|00029|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_state 2017-05-11T03:45:25.293Z|00030|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_zone 2017-05-11T03:45:25.293Z|00031|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_mark 2017-05-11T03:45:25.293Z|00032|ofproto_dpif|INFO|system@ovs-system: Datapath supports ct_label 2017-05-11T03:45:25.364Z|00001|ofproto_dpif_upcall(handler226)|INFO|received packet on unassociated datapath port 0 2017-05-11T03:45:25.368Z|00033|netdev_linux|WARN|ethtool command ETHTOOL_GFLAGS on network device br-eth failed: No such device 2017-05-11T03:45:25.368Z|00034|dpif|WARN|system@ovs-system: failed to add br-eth as port: No such device 2017-05-11T03:45:25.368Z|00035|bridge|INFO|bridge br-eth: using datapath ID 00002a51cf9f2841 2017-05-11T03:45:25.368Z|00036|connmgr|INFO|br-eth: added service controller "punix:/var/run/openvswitch/br-eth.mgmt" Change-Id: Ib5ead59bc91453f83549da89937c0d3607e0385e Signed-off-by: fukaige <fukaige@huawei.com> --- lib/netdev-linux.c | 2 ++ 1 file changed, 2 insertions(+)