[ovs-discuss] [ACLv2 09/19] Add ability for the datapath to match IP address in ARPs

[Ben Pfaff]

Jesse Gross <jesse at nicira.com> writes:

> +struct arp_eth_header {
> +	/* Generic members. */
> +	uint16_t ar_hrd;           /* Hardware type. */
> +	uint16_t ar_pro;           /* Protocol type. */
> +	uint8_t ar_hln;            /* Hardware address length. */
> +	uint8_t ar_pln;            /* Protocol address length. */
> +	uint16_t ar_op;            /* Opcode. */
> +
> +	/* Ethernet+IPv4 specific members. */
> +	uint8_t ar_sha[ETH_ALEN];  /* Sender hardware address. */
> +	uint32_t ar_sip;           /* Sender protocol address. */
> +	uint8_t ar_tha[ETH_ALEN];  /* Target hardware address. */
> +	uint32_t ar_tip;           /* Target protocol address. */
> +} __attribute__((packed));

Probably these should use the kernel-style u8, u16, u32 or
"unsigned char", "unsigned short", "unsigned int".  There are at
least two copies of this structure in the kernel already, in
include/linux/if_arp.h (with the last few members #if 0'd out)
and drivers/net/via-velocity.h (as struct arp_packet).  It would
be nice to copy one of them verbatim, if possible.  Otherwise
we'll get more complaints when we try to push upstream.

> @@ -215,7 +238,15 @@ flow_extract_stats(const flow_t *flow, const struct ofpbuf *packet,
>  void
>  flow_to_match(const flow_t *flow, uint32_t wildcards, struct ofp_match *match)
>  {
> +    /* The datapath supports matching on an ARP's opcode and IP addresses, 
> +     * but OpenFlow does not.  We need to clear the appropriate wildcard 
> +     * bits so that OpenFlow is unaware of our trickery. */
> +    if (flow->dl_type == htons(ETH_TYPE_ARP)) {
> +        wildcards = ~(OFPFW_NW_PROTO | OFPFW_NW_SRC_ALL | OFPFW_NW_DST_ALL) 
> +                    & wildcards;
> +    }

Looks like a great opportunity to use the &= operator :-)

Also, don't we also need to also set nw_proto, nw_src, nw_dst to
0 at the same time?

Should we turn off the TP_* wildcards too?