On 3/14/24 05:51, Han Zhou wrote: > > Thanks Tao for fixing this. I think the title can be more generic because > this problem and fix applies to all tunnel types rather than just VXLAN. > > On Tue, Mar 12, 2024 at 7:04 AM Tao Liu <[email protected] > <mailto:[email protected]>> wrote: >> >> Reproduce: >> ovs-vsctl add-port br-int p0 \ >> -- set interface p0 type=vxlan options:remote_ip=10.10.10.1 >> >> sleep 2 >> >> ovs-vsctl --if-exists del-port p0 \ >> -- add-port br-int p1 \ >> -- set interface p1 type=vxlan options:remote_ip=10.10.10.1 >> ovs-vsctl: Error detected while setting up 'p1': could not add >> network device p1 to ofproto (File exists). >> >> vswitchd log: >> bridge|INFO|bridge br-int: added interface p0 on port 1106 >> bridge|INFO|bridge br-int: deleted interface p0 on port 1106 >> tunnel|WARN|p1: attempting to add tunnel port with same config as port >> 'p0' (::->10.10.10.1, key=0, legacy_l2, dp port=122) >> ofproto|WARN|br-int: could not add port p1 (File exists) >> bridge|WARN|could not add network device p1 to ofproto (File exists) >> >> CallTrace: >> bridge_reconfigure >> bridge_del_ports >> port_destroy >> iface_destroy__ >> netdev_remove <------ netdev p0 removed >> bridge_delete_or_reconfigure_ports >> OFPROTO_PORT_FOR_EACH >> ofproto_port_dump_next >> port_dump_next >> port_query_by_name <------ netdev_shash do not contain p0 >> ofproto_port_del <------ p0 do not del in ofproto >> bridge_add_ports >> bridge_add_ports__ >> iface_create >> iface_do_create >> ofproto_port_add <------ p1 add failed >> >> Fixes: fe83f81df977 ("netdev: Remove netdev from global shash when the user >> is changing interface configuration.") >> Signed-off-by: Tao Liu <[email protected] <mailto:[email protected]>> >> --- >> ofproto/ofproto-dpif.c | 13 +++++++++---- >> tests/tunnel.at <http://tunnel.at> | 12 ++++++++++++ >> 2 files changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index f59d69c4d..0cac3050d 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -3905,14 +3905,19 @@ port_query_by_name(const struct ofproto *ofproto_, >> const char *devname, >> >> if (sset_contains(&ofproto->ghost_ports, devname)) { >> const char *type = netdev_get_type_from_name(devname); >> + const struct ofport *ofport = >> + shash_find_data(&ofproto->up.port_by_name, devname); >> + if (!type && ofport && ofport->netdev) { >> + type = netdev_get_type(ofport->netdev); >> + } >> >> /* We may be called before ofproto->up.port_by_name is populated >> with >> * the appropriate ofport. For this reason, we must get the name >> and >> - * type from the netdev layer directly. */ >> + * type from the netdev layer directly. >> + * When a port deleted, the corresponding netdev is also removed >> from >> + * netdev_shash. netdev_get_type_from_name returns NULL in such >> case. >> + * We should try to get type from ofport->netdev. */ > > nit: this comment is better to be moved to the above where we are trying to > get the type from ofport. > > Otherwise looks good to me: > Acked-by: Han Zhou <[email protected] <mailto:[email protected]>> > Tested-by: Han Zhou <[email protected] <mailto:[email protected]>>
Thanks, Tao and Han! I fixed the nits and applied the change. Also backported down to 2.17. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
