[ovs-dev] [PATCH] bridge: Allow flows based on ARP opcode to be installed.

Ben Pfaff blp at nicira.com
Mon Oct 26 18:01:42 UTC 2009


Jesse Gross <jesse at nicira.com> writes:

>  static bool
> -is_bcast_arp_reply(const flow_t *flow, const struct ofpbuf *packet)
> +is_bcast_arp_reply(const flow_t *flow)
>  {
> -    struct arp_eth_header *arp = (struct arp_eth_header *) packet->data;
>      return (flow->dl_type == htons(ETH_TYPE_ARP)
> -            && eth_addr_is_broadcast(flow->dl_dst)
> -            && packet->size >= sizeof(struct arp_eth_header)
> -            && arp->ar_op == ARP_OP_REQUEST);
> +            && flow->nw_proto == ARP_OP_REQUEST
> +            && eth_addr_is_broadcast(flow->dl_dst));
>  }

The function name talks about ARP replies.  The comments talk
about ARP replies.  The code, however (both old and new),
actually tests for an ARP *request*.  I think that this is a bug.

Otherwise, this looks good to me.




More information about the dev mailing list