[ovs-dev] [PATCH 2/3] Better abstract wildcards for Ethernet destination field.

Ethan Jackson ethan at nicira.com
Tue Jun 7 02:13:15 UTC 2011


flow_wildcard_to_dl_dst_mask() is a tad on the fancy side, but I think
it's fine.

Ethan

On Mon, Jun 6, 2011 at 14:28, Ben Pfaff <blp at nicira.com> wrote:
> I think that this makes nx-match.c a little easier to read.  The new
> functions added here will have more users in an upcoming patch.
> ---
>  lib/classifier.c |   20 ++++++++++++++
>  lib/classifier.h |    2 +
>  lib/flow.c       |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/flow.h       |    5 +++
>  lib/nx-match.c   |   35 +++---------------------
>  5 files changed, 108 insertions(+), 31 deletions(-)
>
> diff --git a/lib/classifier.c b/lib/classifier.c
> index edaceb4..e89a6c8 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -162,6 +162,7 @@ cls_rule_set_dl_src(struct cls_rule *rule, const uint8_t dl_src[ETH_ADDR_LEN])
>     memcpy(rule->flow.dl_src, dl_src, ETH_ADDR_LEN);
>  }
>
> +/* Modifies 'rule' so that the Ethernet address must match 'dl_dst' exactly. */
>  void
>  cls_rule_set_dl_dst(struct cls_rule *rule, const uint8_t dl_dst[ETH_ADDR_LEN])
>  {
> @@ -169,6 +170,25 @@ cls_rule_set_dl_dst(struct cls_rule *rule, const uint8_t dl_dst[ETH_ADDR_LEN])
>     memcpy(rule->flow.dl_dst, dl_dst, ETH_ADDR_LEN);
>  }
>
> +/* Modifies 'rule' so that the Ethernet address must match 'dl_dst' after each
> + * byte is ANDed with the appropriate byte in 'mask'.
> + *
> + * This function will assert-fail if 'mask' is invalid.  Only 'mask' values
> + * accepted by flow_wildcards_is_dl_dst_mask_valid() are allowed. */
> +void
> +cls_rule_set_dl_dst_masked(struct cls_rule *rule,
> +                           const uint8_t dl_dst[ETH_ADDR_LEN],
> +                           const uint8_t mask[ETH_ADDR_LEN])
> +{
> +    flow_wildcards_t *wc = &rule->wc.wildcards;
> +    size_t i;
> +
> +    *wc = flow_wildcards_set_dl_dst_mask(*wc, mask);
> +    for (i = 0; i < ETH_ADDR_LEN; i++) {
> +        rule->flow.dl_dst[i] = dl_dst[i] & mask[i];
> +    }
> +}
> +
>  void
>  cls_rule_set_dl_tci(struct cls_rule *rule, ovs_be16 tci)
>  {
> diff --git a/lib/classifier.h b/lib/classifier.h
> index 08e2c0d..4227c98 100644
> --- a/lib/classifier.h
> +++ b/lib/classifier.h
> @@ -84,6 +84,8 @@ void cls_rule_set_in_port(struct cls_rule *, uint16_t odp_port);
>  void cls_rule_set_dl_type(struct cls_rule *, ovs_be16);
>  void cls_rule_set_dl_src(struct cls_rule *, const uint8_t[6]);
>  void cls_rule_set_dl_dst(struct cls_rule *, const uint8_t[6]);
> +void cls_rule_set_dl_dst_masked(struct cls_rule *, const uint8_t dl_dst[6],
> +                                const uint8_t mask[6]);
>  void cls_rule_set_dl_tci(struct cls_rule *, ovs_be16 tci);
>  void cls_rule_set_dl_tci_masked(struct cls_rule *,
>                                 ovs_be16 tci, ovs_be16 mask);
> diff --git a/lib/flow.c b/lib/flow.c
> index 754c0de..d4afad4 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -16,6 +16,7 @@
>  #include <config.h>
>  #include <sys/types.h>
>  #include "flow.h"
> +#include <assert.h>
>  #include <errno.h>
>  #include <inttypes.h>
>  #include <netinet/in.h>
> @@ -726,6 +727,82 @@ flow_wildcards_set_reg_mask(struct flow_wildcards *wc, int idx, uint32_t mask)
>     wc->reg_masks[idx] = mask;
>  }
>
> +/* Returns the wildcard bitmask for the Ethernet destination address
> + * that 'wc' specifies.  The bitmask has a 0 in each bit that is wildcarded
> + * and a 1 in each bit that must match.  */
> +const uint8_t *
> +flow_wildcards_to_dl_dst_mask(flow_wildcards_t wc)
> +{
> +    static const uint8_t masks[4][ETH_ADDR_LEN] = {
> +        {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
> +        {0xfe, 0xff, 0xff, 0xff, 0xff, 0xff},
> +        {0x01, 0x00, 0x00, 0x00, 0x00, 0x00},
> +        {0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    };
> +
> +    bool wild_mcast = (wc & FWW_ETH_MCAST) != 0;
> +    bool wild_addr = (wc & FWW_DL_DST) != 0;
> +
> +    return masks[wild_mcast + 2 * wild_addr];
> +}
> +
> +/* Returns true if 'mask' is a valid wildcard bitmask for the Ethernet
> + * destination address.  Valid bitmasks are either all-bits-0 or all-bits-1,
> + * except that the multicast bit may differ from the rest of the bits.  So,
> + * there are four possible valid bitmasks:
> + *
> + *  - 00:00:00:00:00:00
> + *  - 01:00:00:00:00:00
> + *  - fe:ff:ff:ff:ff:ff
> + *  - ff:ff:ff:ff:ff:ff
> + *
> + * All other bitmasks are invalid. */
> +bool
> +flow_wildcards_is_dl_dst_mask_valid(const uint8_t mask[ETH_ADDR_LEN])
> +{
> +    switch (mask[0]) {
> +    case 0x00:
> +    case 0x01:
> +        return (mask[1] | mask[2] | mask[3] | mask[4] | mask[5]) == 0x00;
> +
> +    case 0xfe:
> +    case 0xff:
> +        return (mask[1] & mask[2] & mask[3] & mask[4] & mask[5]) == 0xff;
> +
> +    default:
> +        return false;
> +    }
> +}
> +
> +/* Returns 'wc' with the FWW_DL_DST and FWW_ETH_MCAST bits modified
> + * appropriately to match 'mask'.
> + *
> + * This function will assert-fail if 'mask' is invalid.  Only 'mask' values
> + * accepted by flow_wildcards_is_dl_dst_mask_valid() are allowed. */
> +flow_wildcards_t
> +flow_wildcards_set_dl_dst_mask(flow_wildcards_t wc,
> +                               const uint8_t mask[ETH_ADDR_LEN])
> +{
> +    assert(flow_wildcards_is_dl_dst_mask_valid(mask));
> +
> +    switch (mask[0]) {
> +    case 0x00:
> +        return wc | FWW_DL_DST | FWW_ETH_MCAST;
> +
> +    case 0x01:
> +        return (wc | FWW_DL_DST) & ~FWW_ETH_MCAST;
> +
> +    case 0xfe:
> +        return (wc & ~FWW_DL_DST) | FWW_ETH_MCAST;
> +
> +    case 0xff:
> +        return wc & ~(FWW_DL_DST | FWW_ETH_MCAST);
> +
> +    default:
> +        NOT_REACHED();
> +    }
> +}
> +
>  /* Hashes 'flow' based on its L2 through L4 protocol information. */
>  uint32_t
>  flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis)
> diff --git a/lib/flow.h b/lib/flow.h
> index a83987b..2e617b0 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -167,4 +167,9 @@ bool flow_wildcards_equal(const struct flow_wildcards *,
>                           const struct flow_wildcards *);
>  uint32_t flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis);
>
> +const uint8_t *flow_wildcards_to_dl_dst_mask(flow_wildcards_t);
> +bool flow_wildcards_is_dl_dst_mask_valid(const uint8_t[6]);
> +flow_wildcards_t flow_wildcards_set_dl_dst_mask(flow_wildcards_t,
> +                                                const uint8_t mask[6]);
> +
>  #endif /* flow.h */
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 4a41136..f66c655 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -78,16 +78,6 @@ static struct nxm_field nxm_fields[N_NXM_FIELDS] = {
>  /* Hash table of 'nxm_fields'. */
>  static struct hmap all_nxm_fields = HMAP_INITIALIZER(&all_nxm_fields);
>
> -/* Possible masks for NXM_OF_ETH_DST_W. */
> -static const uint8_t eth_all_0s[ETH_ADDR_LEN]
> -    = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
> -static const uint8_t eth_all_1s[ETH_ADDR_LEN]
> -    = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> -static const uint8_t eth_mcast_1[ETH_ADDR_LEN]
> -    = {0x01, 0x00, 0x00, 0x00, 0x00, 0x00};
> -static const uint8_t eth_mcast_0[ETH_ADDR_LEN]
> -    = {0xfe, 0xff, 0xff, 0xff, 0xff, 0xff};
> -
>  static void
>  nxm_init(void)
>  {
> @@ -192,20 +182,8 @@ parse_nxm_entry(struct cls_rule *rule, const struct nxm_field *f,
>         if ((wc->wildcards & (FWW_DL_DST | FWW_ETH_MCAST))
>             != (FWW_DL_DST | FWW_ETH_MCAST)) {
>             return NXM_DUP_TYPE;
> -        } else if (eth_addr_equals(mask, eth_mcast_1)) {
> -            wc->wildcards &= ~FWW_ETH_MCAST;
> -            flow->dl_dst[0] = *(uint8_t *) value & 0x01;
> -            return 0;
> -        } else if (eth_addr_equals(mask, eth_mcast_0)) {
> -            wc->wildcards &= ~FWW_DL_DST;
> -            memcpy(flow->dl_dst, value, ETH_ADDR_LEN);
> -            flow->dl_dst[0] &= 0xfe;
> -            return 0;
> -        } else if (eth_addr_equals(mask, eth_all_0s)) {
> -            return 0;
> -        } else if (eth_addr_equals(mask, eth_all_1s)) {
> -            wc->wildcards &= ~(FWW_DL_DST | FWW_ETH_MCAST);
> -            memcpy(flow->dl_dst, value, ETH_ADDR_LEN);
> +        } else if (flow_wildcards_is_dl_dst_mask_valid(mask)) {
> +            cls_rule_set_dl_dst_masked(rule, value, mask);
>             return 0;
>         } else {
>             return NXM_BAD_MASK;
> @@ -682,15 +660,10 @@ nxm_put_eth_dst(struct ofpbuf *b,
>     switch (wc & (FWW_DL_DST | FWW_ETH_MCAST)) {
>     case FWW_DL_DST | FWW_ETH_MCAST:
>         break;
> -    case FWW_DL_DST:
> -        nxm_put_header(b, NXM_OF_ETH_DST_W);
> -        ofpbuf_put(b, value, ETH_ADDR_LEN);
> -        ofpbuf_put(b, eth_mcast_1, ETH_ADDR_LEN);
> -        break;
> -    case FWW_ETH_MCAST:
> +    default:
>         nxm_put_header(b, NXM_OF_ETH_DST_W);
>         ofpbuf_put(b, value, ETH_ADDR_LEN);
> -        ofpbuf_put(b, eth_mcast_0, ETH_ADDR_LEN);
> +        ofpbuf_put(b, flow_wildcards_to_dl_dst_mask(wc), ETH_ADDR_LEN);
>         break;
>     case 0:
>         nxm_put_eth(b, NXM_OF_ETH_DST, value);
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list