On Sun, Apr 03, 2016 at 09:49:04PM -0400, Ramu Ramamurthy wrote:
> In some usecases such as VM migration or when VMs reuse IP addresses,
> VMs become unreachable externally because
> external switches/routers on localnet
> have stale port-mac or arp caches. The problem resolves
> after some time when the caches ageout which could be
> minutes for port-mac bindings or hours for arp caches.
>
> To fix this, send some gratuitous ARPs when a logical
> port on a localnet datapath gets added. Such gratuitous arps
> help on a best-effort basis to update the mac-port bindings and arp-caches
> of external switches and routers on the localnet.
>
> Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1545897
> Reported-by: Kyle Mestery <mest...@mestery.com>
> Signed-off-by: Ramu Ramamurthy <ramu.ramamur...@us.ibm.com>
Darrell (CCed) brought up some issues with this idea. I don't know
whether his questions have been satisfactorily answered.
Is there an equivalent of GARP for IPv6?
The GARP code manages and acts based on timers, but I don't see anything
that calls, e.g., poll_timer_wait_until(), to ensure that ovn-controller
wakes up when it's time to send a GARP. This call would ordinarily be
made by adding another call to pinctrl_wait().
Please be careful about coding style. The first thing that jumps out at
me here is comment style. CodingStyle.md says:
Comments should be written as full sentences that start with a
capital letter and end with a period. Put two spaces between
sentences.
(Comments that aren't on lines of their own don't need to be sentences
but still should be capitalized and end with a period.)
The following is unsafe if there is no such datapath, because
CONTAINER_OF does not treat a null pointer specially:
struct local_datapath *ld =
CONTAINER_OF(hmap_first_with_hash(local_datapaths,
pb->datapath->tunnel_key),
struct local_datapath, hmap_node);
Please write sizeof *garp here instead of sizeof(struct garp_data):
/* the mac address must be valid */
struct garp_data *garp = xmalloc(sizeof(struct garp_data));
as requested in CodingStyle:
The "sizeof" operator is unique among C operators in that it accepts
two very different kinds of operands: an expression or a type. In
general, prefer to specify an expression, e.g. "int *x =
xmalloc(sizeof *x);". When the operand of sizeof is an expression,
there is no need to parenthesize that operand, and please don't.
Please put spaces around + and * as recommended by CodingStyle, e.g. here:
+ garp->announce_time = time_msec()+1000;
and here:
+ garp->announce_time = current_time + garp->backoff*1000;
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
