[ovs-dev] [ovs-discuss] [ACLv2 10/19] bridge: Allow flows based on ARP opcode to be installed.

Jesse Gross jesse at nicira.com
Fri Sep 4 22:48:52 UTC 2009



Ben Pfaff wrote:
> Jesse Gross <jesse at nicira.com> writes:
>
>   
>> Since we can now distinguish between flows with different ARP opcodes
>> in the kernel, allow these flows to be installed.
>>     
>
> This looks correct but I think that it is incomplete.  Some
> additional code can also be simplified (untested):
>   

Thanks, this code looks good so I added it.

> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ea0641e..c7f0ecb 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1837,13 +1837,11 @@ compose_actions(struct bridge *br, const flow_t *flow, uint16_t vlan,
>  }
>  
>  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));
>  }
>  
>  /* If the composed actions may be applied to any packet in the given 'flow',
> @@ -1957,17 +1955,9 @@ process_flow(struct bridge *br, const flow_t *flow,
>               * an exception to this rule: the host has moved to another
>               * switch. */
>              int src_idx = mac_learning_lookup(br->ml, flow->dl_src, vlan);
> -            if (src_idx != -1 && src_idx != in_port->port_idx) {
> -                if (packet) {
> -                    if (!is_bcast_arp_reply(flow, packet)) {
> -                        goto done;
> -                    }
> -                } else {
> -                    /* No way to know whether it's an ARP reply, because the
> -                     * flow entry doesn't include enough information and we
> -                     * don't have a packet.  Punt. */
> -                    return false;
> -                }
> +            if (src_idx != -1 && src_idx != in_port->port_idx
> +                && !is_bcast_arp_reply(flow)) {
> +                goto done;
>              }
>          }
>      }
>   




More information about the dev mailing list