On Mon, Apr 11, 2016 at 6:29 PM, Ben Pfaff <b...@ovn.org> wrote: > 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> >
Ben, Thanks for your review, > Darrell (CCed) brought up some issues with this idea. I don't know > whether his questions have been satisfactorily answered. > In the next version, I will update as per Darrell's comments. > Is there an equivalent of GARP for IPv6? > I dont know enough IPv6 to answer this, The problem we saw which motivated this fix was in a usecase with IPv4. > 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(). > The code assumes that pinctr_run() would be run often enough to not need to fire timers. But this is not a correct assumption, as there could be pauses between successive runs of pinctl_run(), causing successive garps to be sent at arbitrary intervals. An approach based on timers makes the fix deterministic. I will look into this further in the next version. > 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.) > I will update the next version with careful attention to the coding style for this and the following notes on sizeof, and expressions, > 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); > For this I will use the get_local_datapath() helper. > 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
