[ovs-dev] [PATCH] netdev-vport: Checks tunnel status change when route-table is reset.

Ben Pfaff blp at nicira.com
Fri May 2 17:05:06 UTC 2014


On Thu, May 01, 2014 at 04:20:05PM -0700, Alex Wang wrote:
> Commit 3e912ffcbb (netdev: Add 'change_seq' back to netdev.) added per-
> netdev change number for indicating status change.  Future commits used
> this change number to optimize the netdev status update to database.
> However, the work also introduced the bug in the following scenario:
> 
> - assume interface eth0 has address 1.2.3.4, eth1 has adddress 10.0.0.1.
> - assume tunnel port p1 is set with remote_ip=10.0.0.5.
> - after setup, 'ovs-vsctl list interface p1 status' should show the
>   'tunnel_egress_iface="eth1"'.
> - now if the address of eth1 is change to 0 via 'ifconfig eth1 0'.
> - expectedly, after change, 'ovs-vsctl list interface p1 status' should
>   show the 'tunnel_egress_iface="eth0"'
> 
> However, 'tunnel_egress_iface' will not be updated on current master.
> This is in that, the 'netdev-vport' module corresponding to p1 does
> not react to routing related changes.
> 
> To fix the bug, this commit adds a change sequence number in the route-
> table module and makes netdev-vport check the sequence number for
> tunnel status update.
> 
> Bug #1240626
> 
> Signed-off-by: Alex Wang <alexw at nicira.com>

The new version of tunnel_check_status_change__() uses memset(),
memcpy(), and memcmp() to work with strings.  Is there some reason that
one can't use the normal approach for strings, e.g. set byte 0 to '\0'
to make it empty and strcmp() and ovs_strlcpy() to copy?

netdev_vport_route_changed() doesn't appear to use the names in the
shash, so it might make more sense for netdev_get_vports() to just
return an array of pointers to netdevs instead of wasting the extra time
and space on an shash.

There seems to be an assumption that netdev_run() and netdev_wait() are
called single-threaded.  I think this might actually be true now, and
the assumption was there before (more subtly), so it's probably not a
big deal.



More information about the dev mailing list