Ramu
High level comment earlier that still applies:
1) garp reply is still being used rather than garp request
garp replies are more likely to be dropped or explicitly filtered so
garp requests are "better"
Do you need to make changes to build_lswitch_flows() to allow the garp
requests out the localnet
ports ?
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>
>
> 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
