[ovs-dev] [classifier-opt 16/28] flow: Use bit-mask for in_port match, instead of FWW_* flag.

Ethan Jackson ethan at nicira.com
Tue Jul 31 00:44:05 UTC 2012


Looks good, thanks.

Ethan

On Fri, Jul 20, 2012 at 4:25 PM, Ben Pfaff <blp at nicira.com> wrote:
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/classifier.c        |    8 ++---
>  lib/flow.c              |   27 ++++++--------
>  lib/flow.h              |   27 ++------------
>  lib/meta-flow.c         |   90 ++++++++++++++++++++++-------------------------
>  lib/meta-flow.h         |    1 -
>  lib/nx-match.c          |    3 +-
>  lib/ofp-util.c          |    9 ++---
>  tests/test-classifier.c |   55 +++++++++++++---------------
>  8 files changed, 90 insertions(+), 130 deletions(-)
>
> diff --git a/lib/classifier.c b/lib/classifier.c
> index 07a7c4e..47ba49a 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -149,7 +149,7 @@ cls_rule_set_tun_id_masked(struct cls_rule *rule,
>  void
>  cls_rule_set_in_port(struct cls_rule *rule, uint16_t ofp_port)
>  {
> -    rule->wc.wildcards &= ~FWW_IN_PORT;
> +    rule->wc.in_port_mask = UINT16_MAX;
>      rule->flow.in_port = ofp_port;
>  }
>
> @@ -575,7 +575,6 @@ cls_rule_format(const struct cls_rule *rule, struct ds *s)
>  {
>      const struct flow_wildcards *wc = &rule->wc;
>      size_t start_len = s->length;
> -    flow_wildcards_t w = wc->wildcards;
>      const struct flow *f = &rule->flow;
>      bool skip_type = false;
>      bool skip_proto = false;
> @@ -663,7 +662,7 @@ cls_rule_format(const struct cls_rule *rule, struct ds *s)
>                        ntohll(f->metadata), ntohll(wc->metadata_mask));
>          break;
>      }
> -    if (!(w & FWW_IN_PORT)) {
> +    if (wc->in_port_mask) {
>          ds_put_format(s, "in_port=%"PRIu16",", f->in_port);
>      }
>      if (wc->vlan_tci_mask) {
> @@ -1263,7 +1262,6 @@ static bool
>  flow_equal_except(const struct flow *a, const struct flow *b,
>                    const struct flow_wildcards *wildcards)
>  {
> -    const flow_wildcards_t wc = wildcards->wildcards;
>      int i;
>
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
> @@ -1278,7 +1276,7 @@ flow_equal_except(const struct flow *a, const struct flow *b,
>              && !((a->metadata ^ b->metadata) & wildcards->metadata_mask)
>              && !((a->nw_src ^ b->nw_src) & wildcards->nw_src_mask)
>              && !((a->nw_dst ^ b->nw_dst) & wildcards->nw_dst_mask)
> -            && (wc & FWW_IN_PORT || a->in_port == b->in_port)
> +            && !((a->in_port ^ b->in_port) & wildcards->in_port_mask)
>              && !((a->vlan_tci ^ b->vlan_tci) & wildcards->vlan_tci_mask)
>              && !((a->dl_type ^ b->dl_type) & wildcards->dl_type_mask)
>              && !((a->tp_src ^ b->tp_src) & wildcards->tp_src_mask)
> diff --git a/lib/flow.c b/lib/flow.c
> index bd92e87..e8c12a7 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -441,7 +441,6 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority, ovs_be64 tun_id,
>  void
>  flow_zero_wildcards(struct flow *flow, const struct flow_wildcards *wildcards)
>  {
> -    const flow_wildcards_t wc = wildcards->wildcards;
>      int i;
>
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
> @@ -453,9 +452,7 @@ flow_zero_wildcards(struct flow *flow, const struct flow_wildcards *wildcards)
>      flow->metadata &= wildcards->metadata_mask;
>      flow->nw_src &= wildcards->nw_src_mask;
>      flow->nw_dst &= wildcards->nw_dst_mask;
> -    if (wc & FWW_IN_PORT) {
> -        flow->in_port = 0;
> -    }
> +    flow->in_port &= wildcards->in_port_mask;
>      flow->vlan_tci &= wildcards->vlan_tci_mask;
>      flow->dl_type &= wildcards->dl_type_mask;
>      flow->tp_src &= wildcards->tp_src_mask;
> @@ -578,7 +575,6 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
>  {
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
> -    wc->wildcards = FWW_ALL;
>      wc->tun_id_mask = htonll(0);
>      wc->nw_src_mask = htonl(0);
>      wc->nw_dst_mask = htonl(0);
> @@ -588,6 +584,7 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
>      wc->nd_target_mask = in6addr_any;
>      memset(wc->reg_masks, 0, sizeof wc->reg_masks);
>      wc->metadata_mask = htonll(0);
> +    wc->in_port_mask = 0;
>      wc->vlan_tci_mask = htons(0);
>      wc->nw_frag_mask = 0;
>      wc->dl_type_mask = htons(0);
> @@ -610,7 +607,6 @@ flow_wildcards_init_exact(struct flow_wildcards *wc)
>  {
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
> -    wc->wildcards = 0;
>      wc->tun_id_mask = htonll(UINT64_MAX);
>      wc->nw_src_mask = htonl(UINT32_MAX);
>      wc->nw_dst_mask = htonl(UINT32_MAX);
> @@ -620,6 +616,7 @@ flow_wildcards_init_exact(struct flow_wildcards *wc)
>      wc->nd_target_mask = in6addr_exact;
>      memset(wc->reg_masks, 0xff, sizeof wc->reg_masks);
>      wc->metadata_mask = htonll(UINT64_MAX);
> +    wc->in_port_mask = UINT16_MAX;
>      wc->vlan_tci_mask = htons(UINT16_MAX);
>      wc->nw_frag_mask = UINT8_MAX;
>      wc->dl_type_mask = htons(UINT16_MAX);
> @@ -644,12 +641,12 @@ flow_wildcards_is_exact(const struct flow_wildcards *wc)
>
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
> -    if (wc->wildcards
> -        || wc->tun_id_mask != htonll(UINT64_MAX)
> +    if (wc->tun_id_mask != htonll(UINT64_MAX)
>          || wc->nw_src_mask != htonl(UINT32_MAX)
>          || wc->nw_dst_mask != htonl(UINT32_MAX)
>          || wc->tp_src_mask != htons(UINT16_MAX)
>          || wc->tp_dst_mask != htons(UINT16_MAX)
> +        || wc->in_port_mask != UINT16_MAX
>          || wc->vlan_tci_mask != htons(UINT16_MAX)
>          || wc->metadata_mask != htonll(UINT64_MAX)
>          || wc->dl_type_mask != htons(UINT16_MAX)
> @@ -686,12 +683,12 @@ flow_wildcards_is_catchall(const struct flow_wildcards *wc)
>
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
> -    if (wc->wildcards != FWW_ALL
> -        || wc->tun_id_mask != htonll(0)
> +    if (wc->tun_id_mask != htonll(0)
>          || wc->nw_src_mask != htonl(0)
>          || wc->nw_dst_mask != htonl(0)
>          || wc->tp_src_mask != htons(0)
>          || wc->tp_dst_mask != htons(0)
> +        || wc->in_port_mask != 0
>          || wc->vlan_tci_mask != htons(0)
>          || wc->metadata_mask != htonll(0)
>          || wc->dl_type_mask != htons(0)
> @@ -731,7 +728,6 @@ flow_wildcards_combine(struct flow_wildcards *dst,
>
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
> -    dst->wildcards = src1->wildcards | src2->wildcards;
>      dst->tun_id_mask = src1->tun_id_mask & src2->tun_id_mask;
>      dst->nw_src_mask = src1->nw_src_mask & src2->nw_src_mask;
>      dst->nw_dst_mask = src1->nw_dst_mask & src2->nw_dst_mask;
> @@ -746,6 +742,7 @@ flow_wildcards_combine(struct flow_wildcards *dst,
>          dst->reg_masks[i] = src1->reg_masks[i] & src2->reg_masks[i];
>      }
>      dst->metadata_mask = src1->metadata_mask & src2->metadata_mask;
> +    dst->in_port_mask = src1->in_port_mask & src2->in_port_mask;
>      dst->vlan_tci_mask = src1->vlan_tci_mask & src2->vlan_tci_mask;
>      dst->dl_type_mask = src1->dl_type_mask & src2->dl_type_mask;
>      dst->tp_src_mask = src1->tp_src_mask & src2->tp_src_mask;
> @@ -781,10 +778,10 @@ flow_wildcards_equal(const struct flow_wildcards *a,
>
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
> -    if (a->wildcards != b->wildcards
> -        || a->tun_id_mask != b->tun_id_mask
> +    if (a->tun_id_mask != b->tun_id_mask
>          || a->nw_src_mask != b->nw_src_mask
>          || a->nw_dst_mask != b->nw_dst_mask
> +        || a->in_port_mask != b->in_port_mask
>          || a->vlan_tci_mask != b->vlan_tci_mask
>          || a->metadata_mask != b->metadata_mask
>          || a->dl_type_mask != b->dl_type_mask
> @@ -867,11 +864,11 @@ flow_wildcards_has_extra(const struct flow_wildcards *a,
>          return true;
>      }
>
> -    return (a->wildcards & ~b->wildcards
> -            || (a->tun_id_mask & b->tun_id_mask) != b->tun_id_mask
> +    return ((a->tun_id_mask & b->tun_id_mask) != b->tun_id_mask
>              || (a->nw_src_mask & b->nw_src_mask) != b->nw_src_mask
>              || (a->nw_dst_mask & b->nw_dst_mask) != b->nw_dst_mask
>              || (a->ipv6_label_mask & b->ipv6_label_mask) != b->ipv6_label_mask
> +            || (a->in_port_mask & b->in_port_mask) != b->in_port_mask
>              || (a->vlan_tci_mask & b->vlan_tci_mask) != b->vlan_tci_mask
>              || (a->metadata_mask & b->metadata_mask) != b->metadata_mask
>              || (a->dl_type_mask & b->dl_type_mask) != b->dl_type_mask
> diff --git a/lib/flow.h b/lib/flow.h
> index d5abe61..7a40475 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -140,28 +140,10 @@ flow_hash(const struct flow *flow, uint32_t basis)
>      return hash_bytes(flow, FLOW_SIG_SIZE, basis);
>  }
>
> -/* Open vSwitch flow wildcard bits.
> - *
> - * These are used only internally to Open vSwitch, in the 'wildcards' member of
> - * struct flow_wildcards.  They never appear in the wire protocol in this
> - * form. */
> -
> -typedef unsigned int OVS_BITWISE flow_wildcards_t;
> -
> -#define FWW_IN_PORT     ((OVS_FORCE flow_wildcards_t) (1 << 0))
> -#define FWW_ALL         ((OVS_FORCE flow_wildcards_t) (((1 << 1)) - 1))
> -
> -/* Remember to update FLOW_WC_SEQ when adding or removing FWW_*. */
> -BUILD_ASSERT_DECL(FWW_ALL == ((1 << 1) - 1) && FLOW_WC_SEQ == 17);
> -
> -/* Information on wildcards for a flow, as a supplement to "struct flow".
> - *
> - * Note that the meaning of 1-bits in 'wildcards' is opposite that of 1-bits in
> - * the rest of the members. */
> +/* Information on wildcards for a flow, as a supplement to "struct flow". */
>  struct flow_wildcards {
>      ovs_be64 tun_id_mask;       /* 1-bit in each significant tun_id bit. */
>      ovs_be64 metadata_mask;     /* 1-bit in each significant metadata bit. */
> -    flow_wildcards_t wildcards; /* 1-bit in each FWW_* wildcarded field. */
>      uint32_t reg_masks[FLOW_N_REGS]; /* 1-bit in each significant regs bit. */
>      ovs_be32 nw_src_mask;       /* 1-bit in each significant nw_src bit. */
>      ovs_be32 nw_dst_mask;       /* 1-bit in each significant nw_dst bit. */
> @@ -170,6 +152,7 @@ struct flow_wildcards {
>      struct in6_addr nd_target_mask; /* 1-bit in each significant
>                                         nd_target bit. */
>      ovs_be32 ipv6_label_mask;   /* 1 bit in each significant ipv6_label bit. */
> +    uint16_t in_port_mask;      /* 1-bit in each significant in_port bit. */
>      ovs_be16 vlan_tci_mask;     /* 1-bit in each significant vlan_tci bit. */
>      ovs_be16 dl_type_mask;      /* 1-bit in each significant dl_type bit. */
>      ovs_be16 tp_src_mask;       /* 1-bit in each significant tp_src bit. */
> @@ -182,7 +165,7 @@ struct flow_wildcards {
>      uint8_t arp_tha_mask[6];    /* 1-bit in each significant dl_dst bit. */
>      uint8_t nw_tos_mask;        /* 1-bit in each significant nw_tos bit. */
>      uint8_t nw_ttl_mask;        /* 1-bit in each significant nw_ttl bit. */
> -    uint8_t zeros[4];           /* Padding field set to zero. */
> +    uint8_t zeros[6];           /* Padding field set to zero. */
>  };
>
>  /* Remember to update FLOW_WC_SEQ when updating struct flow_wildcards. */
> @@ -208,10 +191,6 @@ 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]);
>  uint32_t flow_hash_fields(const struct flow *, enum nx_hash_fields,
>                            uint16_t basis);
>  const char *flow_hash_fields_to_str(enum nx_hash_fields);
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index cc4ae24..243681c 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -49,7 +49,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_TUN_ID, "tun_id", NULL,
>          MF_FIELD_SIZES(be64),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_HEXADECIMAL,
>          MFP_NONE,
>          true,
> @@ -58,7 +58,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_METADATA, "metadata", NULL,
>          MF_FIELD_SIZES(be64),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_HEXADECIMAL,
>          MFP_NONE,
>          true,
> @@ -67,7 +67,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IN_PORT, "in_port", NULL,
>          MF_FIELD_SIZES(be16),
> -        MFM_NONE, FWW_IN_PORT,
> +        MFM_NONE,
>          MFS_OFP_PORT,
>          MFP_NONE,
>          false,
> @@ -79,7 +79,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {                                           \
>          MFF_REG##IDX, "reg" #IDX, NULL,         \
>          MF_FIELD_SIZES(be32),                   \
> -        MFM_FULLY, 0,                           \
> +        MFM_FULLY,                              \
>          MFS_HEXADECIMAL,                        \
>          MFP_NONE,                               \
>          true,                                   \
> @@ -121,7 +121,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_ETH_SRC, "eth_src", "dl_src",
>          MF_FIELD_SIZES(mac),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_ETHERNET,
>          MFP_NONE,
>          true,
> @@ -130,7 +130,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ETH_DST, "eth_dst", "dl_dst",
>          MF_FIELD_SIZES(mac),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_ETHERNET,
>          MFP_NONE,
>          true,
> @@ -139,7 +139,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ETH_TYPE, "eth_type", "dl_type",
>          MF_FIELD_SIZES(be16),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_HEXADECIMAL,
>          MFP_NONE,
>          false,
> @@ -150,7 +150,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_VLAN_TCI, "vlan_tci", NULL,
>          MF_FIELD_SIZES(be16),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_HEXADECIMAL,
>          MFP_NONE,
>          true,
> @@ -159,7 +159,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_VLAN_VID, "dl_vlan", NULL,
>          sizeof(ovs_be16), 12,
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_NONE,
>          true,
> @@ -168,7 +168,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_VLAN_PCP, "dl_vlan_pcp", NULL,
>          1, 3,
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_NONE,
>          true,
> @@ -183,7 +183,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_IPV4_SRC, "ip_src", "nw_src",
>          MF_FIELD_SIZES(be32),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV4,
>          MFP_IPV4,
>          true,
> @@ -192,7 +192,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IPV4_DST, "ip_dst", "nw_dst",
>          MF_FIELD_SIZES(be32),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV4,
>          MFP_IPV4,
>          true,
> @@ -203,7 +203,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_IPV6_SRC, "ipv6_src", NULL,
>          MF_FIELD_SIZES(ipv6),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV6,
>          MFP_IPV6,
>          true,
> @@ -212,7 +212,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IPV6_DST, "ipv6_dst", NULL,
>          MF_FIELD_SIZES(ipv6),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV6,
>          MFP_IPV6,
>          true,
> @@ -222,7 +222,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_IPV6_LABEL, "ipv6_label", NULL,
>          4, 20,
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_HEXADECIMAL,
>          MFP_IPV6,
>          false,
> @@ -233,7 +233,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_IP_PROTO, "nw_proto", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_IP_ANY,
>          false,
> @@ -242,7 +242,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IP_DSCP, "nw_tos", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_IP_ANY,
>          true,
> @@ -251,7 +251,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IP_ECN, "nw_ecn", NULL,
>          1, 2,
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_IP_ANY,
>          true,
> @@ -260,7 +260,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IP_TTL, "nw_ttl", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_IP_ANY,
>          true,
> @@ -269,7 +269,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_IP_FRAG, "ip_frag", NULL,
>          1, 2,
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_FRAG,
>          MFP_IP_ANY,
>          false,
> @@ -280,7 +280,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_ARP_OP, "arp_op", NULL,
>          MF_FIELD_SIZES(be16),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_ARP,
>          false,
> @@ -289,7 +289,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ARP_SPA, "arp_spa", NULL,
>          MF_FIELD_SIZES(be32),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV4,
>          MFP_ARP,
>          false,
> @@ -298,7 +298,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ARP_TPA, "arp_tpa", NULL,
>          MF_FIELD_SIZES(be32),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV4,
>          MFP_ARP,
>          false,
> @@ -307,7 +307,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ARP_SHA, "arp_sha", NULL,
>          MF_FIELD_SIZES(mac),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_ETHERNET,
>          MFP_ARP,
>          false,
> @@ -316,7 +316,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ARP_THA, "arp_tha", NULL,
>          MF_FIELD_SIZES(mac),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_ETHERNET,
>          MFP_ARP,
>          false,
> @@ -331,7 +331,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_TCP_SRC, "tcp_src", "tp_src",
>          MF_FIELD_SIZES(be16),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_DECIMAL,
>          MFP_TCP,
>          true,
> @@ -340,7 +340,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_TCP_DST, "tcp_dst", "tp_dst",
>          MF_FIELD_SIZES(be16),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_DECIMAL,
>          MFP_TCP,
>          true,
> @@ -351,7 +351,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_UDP_SRC, "udp_src", NULL,
>          MF_FIELD_SIZES(be16),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_DECIMAL,
>          MFP_UDP,
>          true,
> @@ -360,7 +360,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_UDP_DST, "udp_dst", NULL,
>          MF_FIELD_SIZES(be16),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_DECIMAL,
>          MFP_UDP,
>          true,
> @@ -371,7 +371,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_ICMPV4_TYPE, "icmp_type", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_ICMPV4,
>          false,
> @@ -380,7 +380,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ICMPV4_CODE, "icmp_code", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_ICMPV4,
>          false,
> @@ -391,7 +391,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_ICMPV6_TYPE, "icmpv6_type", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_ICMPV6,
>          false,
> @@ -400,7 +400,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ICMPV6_CODE, "icmpv6_code", NULL,
>          MF_FIELD_SIZES(u8),
> -        MFM_NONE, 0,
> +        MFM_NONE,
>          MFS_DECIMAL,
>          MFP_ICMPV6,
>          false,
> @@ -415,7 +415,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      {
>          MFF_ND_TARGET, "nd_target", NULL,
>          MF_FIELD_SIZES(ipv6),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_IPV6,
>          MFP_ND,
>          false,
> @@ -424,7 +424,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ND_SLL, "nd_sll", NULL,
>          MF_FIELD_SIZES(mac),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_ETHERNET,
>          MFP_ND_SOLICIT,
>          false,
> @@ -433,7 +433,7 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
>      }, {
>          MFF_ND_TLL, "nd_tll", NULL,
>          MF_FIELD_SIZES(mac),
> -        MFM_FULLY, 0,
> +        MFM_FULLY,
>          MFS_ETHERNET,
>          MFP_ND_ADVERT,
>          false,
> @@ -555,15 +555,12 @@ bool
>  mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
>  {
>      switch (mf->id) {
> -    case MFF_IN_PORT:
> -        assert(mf->fww_bit != 0);
> -        return (wc->wildcards & mf->fww_bit) != 0;
> -
>      case MFF_TUN_ID:
>          return !wc->tun_id_mask;
>      case MFF_METADATA:
>          return !wc->metadata_mask;
> -
> +    case MFF_IN_PORT:
> +        return !wc->in_port_mask;
>      CASE_MFF_REGS:
>          return !wc->reg_masks[mf->id - MFF_REG0];
>
> @@ -652,18 +649,15 @@ mf_get_mask(const struct mf_field *mf, const struct flow_wildcards *wc,
>              union mf_value *mask)
>  {
>      switch (mf->id) {
> -    case MFF_IN_PORT:
> -        assert(mf->fww_bit != 0);
> -        memset(mask, wc->wildcards & mf->fww_bit ? 0x00 : 0xff, mf->n_bytes);
> -        break;
> -
>      case MFF_TUN_ID:
>          mask->be64 = wc->tun_id_mask;
>          break;
>      case MFF_METADATA:
>          mask->be64 = wc->metadata_mask;
>          break;
> -
> +    case MFF_IN_PORT:
> +        mask->be16 = htons(wc->in_port_mask);
> +        break;
>      CASE_MFF_REGS:
>          mask->be32 = htonl(wc->reg_masks[mf->id - MFF_REG0]);
>          break;
> @@ -1366,8 +1360,8 @@ mf_set_wild(const struct mf_field *mf, struct cls_rule *rule)
>          cls_rule_set_metadata_masked(rule, htonll(0), htonll(0));
>
>      case MFF_IN_PORT:
> -        rule->wc.wildcards |= FWW_IN_PORT;
>          rule->flow.in_port = 0;
> +        rule->wc.in_port_mask = 0;
>          break;
>
>      CASE_MFF_REGS:
> diff --git a/lib/meta-flow.h b/lib/meta-flow.h
> index 1fecc15..977c08d 100644
> --- a/lib/meta-flow.h
> +++ b/lib/meta-flow.h
> @@ -215,7 +215,6 @@ struct mf_field {
>
>      /* Properties. */
>      enum mf_maskable maskable;
> -    flow_wildcards_t fww_bit;   /* Either 0 or exactly one FWW_* bit. */
>      enum mf_string string;
>      enum mf_prereqs prereqs;
>      bool writable;              /* May be written by actions? */
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 16cac92..ba27aef 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -486,7 +486,6 @@ int
>  nx_put_match(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
>               ovs_be64 cookie, ovs_be64 cookie_mask)
>  {
> -    const flow_wildcards_t wc = cr->wc.wildcards;
>      const struct flow *flow = &cr->flow;
>      const size_t start_len = b->size;
>      int match_len;
> @@ -495,7 +494,7 @@ nx_put_match(struct ofpbuf *b, bool oxm, const struct cls_rule *cr,
>      BUILD_ASSERT_DECL(FLOW_WC_SEQ == 17);
>
>      /* Metadata. */
> -    if (!(wc & FWW_IN_PORT)) {
> +    if (cr->wc.in_port_mask) {
>          uint16_t in_port = flow->in_port;
>          if (oxm) {
>              nxm_put_32(b, OXM_OF_IN_PORT, ofputil_port_to_ofp11(in_port));
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index d87573c..665b0da 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -88,9 +88,8 @@ ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
>      /* Initialize most of rule->wc. */
>      flow_wildcards_init_catchall(wc);
>
> -    wc->wildcards = 0;
> -    if (ofpfw & OFPFW10_IN_PORT) {
> -        wc->wildcards |= FWW_IN_PORT;
> +    if (!(ofpfw & OFPFW10_IN_PORT)) {
> +        wc->in_port_mask = UINT16_MAX;
>      }
>
>      if (!(ofpfw & OFPFW10_NW_TOS)) {
> @@ -189,7 +188,7 @@ ofputil_cls_rule_to_ofp10_match(const struct cls_rule *rule,
>
>      /* Figure out most OpenFlow wildcards. */
>      ofpfw = 0;
> -    if (wc->wildcards & FWW_IN_PORT) {
> +    if (!wc->in_port_mask) {
>          ofpfw |= OFPFW10_IN_PORT;
>      }
>      if (!wc->dl_type_mask) {
> @@ -430,7 +429,7 @@ ofputil_cls_rule_to_ofp11_match(const struct cls_rule *rule,
>      match->omh.type = htons(OFPMT_STANDARD);
>      match->omh.length = htons(OFPMT11_STANDARD_LENGTH);
>
> -    if (rule->wc.wildcards & FWW_IN_PORT) {
> +    if (!rule->wc.in_port_mask) {
>          wc |= OFPFW11_IN_PORT;
>      } else {
>          match->in_port = ofputil_port_to_ofp11(rule->flow.in_port);
> diff --git a/tests/test-classifier.c b/tests/test-classifier.c
> index 97e8f3f..baa4cc1 100644
> --- a/tests/test-classifier.c
> +++ b/tests/test-classifier.c
> @@ -41,28 +41,28 @@
>
>  /* Fields in a rule. */
>  #define CLS_FIELDS                                                  \
> -    /*                                    struct flow  all-caps */  \
> -    /*        FWW_* bit(s)                member name  name     */  \
> -    /*        --------------------------  -----------  -------- */  \
> -    CLS_FIELD(0,                          tun_id,      TUN_ID)      \
> -    CLS_FIELD(0,                          metadata,    METADATA)    \
> -    CLS_FIELD(0,                          nw_src,      NW_SRC)      \
> -    CLS_FIELD(0,                          nw_dst,      NW_DST)      \
> -    CLS_FIELD(FWW_IN_PORT,                in_port,     IN_PORT)     \
> -    CLS_FIELD(0,                          vlan_tci,    VLAN_TCI)    \
> -    CLS_FIELD(0,                          dl_type,     DL_TYPE)     \
> -    CLS_FIELD(0,                          tp_src,      TP_SRC)      \
> -    CLS_FIELD(0,                          tp_dst,      TP_DST)      \
> -    CLS_FIELD(0,                          dl_src,      DL_SRC)      \
> -    CLS_FIELD(0,                          dl_dst,      DL_DST)      \
> -    CLS_FIELD(0,                          nw_proto,    NW_PROTO)    \
> -    CLS_FIELD(0,                          nw_tos,      NW_DSCP)
> +    /*        struct flow  all-caps */  \
> +    /*        member name  name     */  \
> +    /*        -----------  -------- */  \
> +    CLS_FIELD(tun_id,      TUN_ID)      \
> +    CLS_FIELD(metadata,    METADATA)    \
> +    CLS_FIELD(nw_src,      NW_SRC)      \
> +    CLS_FIELD(nw_dst,      NW_DST)      \
> +    CLS_FIELD(in_port,     IN_PORT)     \
> +    CLS_FIELD(vlan_tci,    VLAN_TCI)    \
> +    CLS_FIELD(dl_type,     DL_TYPE)     \
> +    CLS_FIELD(tp_src,      TP_SRC)      \
> +    CLS_FIELD(tp_dst,      TP_DST)      \
> +    CLS_FIELD(dl_src,      DL_SRC)      \
> +    CLS_FIELD(dl_dst,      DL_DST)      \
> +    CLS_FIELD(nw_proto,    NW_PROTO)    \
> +    CLS_FIELD(nw_tos,      NW_DSCP)
>
>  /* Field indexes.
>   *
>   * (These are also indexed into struct classifier's 'tables' array.) */
>  enum {
> -#define CLS_FIELD(WILDCARDS, MEMBER, NAME) CLS_F_IDX_##NAME,
> +#define CLS_FIELD(MEMBER, NAME) CLS_F_IDX_##NAME,
>      CLS_FIELDS
>  #undef CLS_FIELD
>      CLS_N_FIELDS
> @@ -72,15 +72,13 @@ enum {
>  struct cls_field {
>      int ofs;                    /* Offset in struct flow. */
>      int len;                    /* Length in bytes. */
> -    flow_wildcards_t wildcards; /* FWW_* bit or bits for this field. */
>      const char *name;           /* Name (for debugging). */
>  };
>
>  static const struct cls_field cls_fields[CLS_N_FIELDS] = {
> -#define CLS_FIELD(WILDCARDS, MEMBER, NAME)      \
> +#define CLS_FIELD(MEMBER, NAME)                 \
>      { offsetof(struct flow, MEMBER),            \
>        sizeof ((struct flow *)0)->MEMBER,        \
> -      WILDCARDS,                                \
>        #NAME },
>      CLS_FIELDS
>  #undef CLS_FIELD
> @@ -187,15 +185,9 @@ match(const struct cls_rule *wild, const struct flow *fixed)
>      int f_idx;
>
>      for (f_idx = 0; f_idx < CLS_N_FIELDS; f_idx++) {
> -        const struct cls_field *f = &cls_fields[f_idx];
>          bool eq;
>
> -        if (f->wildcards) {
> -            void *wild_field = (char *) &wild->flow + f->ofs;
> -            void *fixed_field = (char *) fixed + f->ofs;
> -            eq = ((wild->wc.wildcards & f->wildcards) == f->wildcards
> -                  || !memcmp(wild_field, fixed_field, f->len));
> -        } else if (f_idx == CLS_F_IDX_NW_SRC) {
> +        if (f_idx == CLS_F_IDX_NW_SRC) {
>              eq = !((fixed->nw_src ^ wild->flow.nw_src) & wild->wc.nw_src_mask);
>          } else if (f_idx == CLS_F_IDX_NW_DST) {
>              eq = !((fixed->nw_dst ^ wild->flow.nw_dst) & wild->wc.nw_dst_mask);
> @@ -226,6 +218,9 @@ match(const struct cls_rule *wild, const struct flow *fixed)
>          } else if (f_idx == CLS_F_IDX_DL_TYPE) {
>              eq = !((fixed->dl_type ^ wild->flow.dl_type)
>                     & wild->wc.dl_type_mask);
> +        } else if (f_idx == CLS_F_IDX_IN_PORT) {
> +            eq = !((fixed->in_port ^ wild->flow.in_port)
> +                   & wild->wc.in_port_mask);
>          } else {
>              NOT_REACHED();
>          }
> @@ -486,9 +481,7 @@ make_rule(int wc_fields, unsigned int priority, int value_pat)
>          memcpy((char *) &rule->cls_rule.flow + f->ofs,
>                 values[f_idx][value_idx], f->len);
>
> -        if (f->wildcards) {
> -            rule->cls_rule.wc.wildcards &= ~f->wildcards;
> -        } else if (f_idx == CLS_F_IDX_NW_SRC) {
> +        if (f_idx == CLS_F_IDX_NW_SRC) {
>              rule->cls_rule.wc.nw_src_mask = htonl(UINT32_MAX);
>          } else if (f_idx == CLS_F_IDX_NW_DST) {
>              rule->cls_rule.wc.nw_dst_mask = htonl(UINT32_MAX);
> @@ -512,6 +505,8 @@ make_rule(int wc_fields, unsigned int priority, int value_pat)
>              rule->cls_rule.wc.nw_proto_mask = UINT8_MAX;
>          } else if (f_idx == CLS_F_IDX_DL_TYPE) {
>              rule->cls_rule.wc.dl_type_mask = htons(UINT16_MAX);
> +        } else if (f_idx == CLS_F_IDX_IN_PORT) {
> +            rule->cls_rule.wc.in_port_mask = UINT16_MAX;
>          } else {
>              NOT_REACHED();
>          }
> --
> 1.7.2.5
>



More information about the dev mailing list