[ovs-dev] [PATCH] vswitchd: Treat gratuitous ARP requests like gratuitous ARP replies.

[Ian Campbell]

On Wed, 2010-05-26 at 21:12 +0100, Ben Pfaff wrote:
> vswitchd has long used a gratuitous ARP reply as an indication that a VM
> has migrated, because Citrix-patch Linux DomUs send such packets out when
> they complete migration.

They aren't really Citrix patches but more a xen.org thing. I often call
them "traditional-Xen" or "classic-Xen" to distinguish them from the
pvops based stuff which is now available in the kernel.org trees.

>   Relatively recently, however, we realized that
> upstream Linux does not do this.  Ian Campbell submitted a patch to the
> Linux network maintainer, Dave Miller, to fix this, but Dave did not accept
> it in its original form.  Instead, he required the form of the packet to
> change to a gratuitous ARP request.

The upstream kernel already sends a gratuitous ARP request (or would if
it weren't for a separate issue, patched separately) and Dave only
rejected my suggestion to turn it into a gratuitous ARP reply.
Gratuitous ARP request and replies are roughly equivalent at the L2
layer wrt switch MAC tables etc but I think Dave was concerned that the
gratuitous replies would also have additional impact on e.g. ARP tables
of other devices on the network (I think this is what he meant by
spamming) so I think he was correct to reject the patch.

The patch for the separate issue mentioned above was also initially
rejected but after discussion we have agreed a different approach. I
have already submitted a new patch with that.

> +/* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
> + * migration.  Older Citrix-patched Linux DomU used gratuitous ARP replies to
> + * indicate this; newer upstream kernels use gratuitous ARP requests. */
>  static bool
> -is_bcast_arp_reply(const flow_t *flow)
> +is_gratuitous_arp(const flow_t *flow)
>  {
>      return (flow->dl_type == htons(ETH_TYPE_ARP)
> -            && flow->nw_proto == ARP_OP_REPLY
> -            && eth_addr_is_broadcast(flow->dl_dst));
> +            && eth_addr_is_broadcast(flow->dl_dst)
> +            && (flow->nw_proto == ARP_OP_REPLY
> +                || (flow->nw_proto == ARP_OP_REQUEST
> +                    && flow->nw_src == flow->nw_dst)));
>  }

If I understand correctly flow->nw_{src,dst} are
flow->dl_type/flow->nw_proto specific fields filed in by flow_extract()
and for ARP types they contain the sender and target protocol addresses.

Assuming that is right this change looks correct to me.

Ian.