[ovs-dev] [PATCH 2/2] lib: Show tunnel egress interface in ovsdb
Ben Pfaff
blp at nicira.com
Wed Dec 29 17:44:34 UTC 2010
On Tue, Dec 28, 2010 at 05:49:48PM -0800, Ethan Jackson wrote:
> This commit parses rtnetlink address notifications from the
> kernel in order to display the egress interface of tunnels in the
> database.
>
> Bug #4103.
This looks good, I have only a few comments.
In struct addr_node, ifa_addr deserves a comment since it is an IP
address stored in host byte order, which is not what readers expect.
In netdev_vport_addr_reset(), nl_dump_done() should be called before
destroying the socket, not after, since it potentially references the
socket. It won't in this particular case, because all of the messages
have been drained, but it really makes me nervous to do it in the
"wrong" order.
I infer from netdev_vport_addr_change() that when an address changes the
kernel first issues a DELADDR for the old address and then a NEWADDR for
the new address. Does it really do that? I would have guessed from
what happens elsewhere that it just issues a NEWADDR for the new
address, but I haven't actually looked at kernel code here.
Does the kernel issue a DELADDR when a network device is deleted? (Will
we "leak" that memory until the next netdev_vport_addr_reset()?)
It looks like this only considers addresses assigned to network devices.
Is that sufficient? Should we be looking at the routing table
(e.g. RTM_*ROUTE) instead or in addition?
It looks to me like netdev_vport_addr_init() and
netdev_vport_addr_destroy() are being called on every creation or
destruction of a vport netdev. That doesn't seem right.
Use nl_attr_get_be32() instead of nl_attr_get_u32() to retrieve
IFA_ADDRESS from the netlink message.
More information about the dev
mailing list