Until mow, this macro has blindly read the passed-in type's size, but that's unnecessarily risky. This commit changes it to verify that the passed-in type is the same size as the field and, on GCC and Clang, that the types are compatible. It also adds a version that does not check, for the one case where (currently) we deliberately read the wrong size, and updates a few uses to use more precise field names.
Signed-off-by: Ben Pfaff <[email protected]> --- lib/classifier.c | 8 ++++---- lib/dpif-netdev.c | 3 ++- lib/flow.c | 2 +- lib/flow.h | 17 ++++++++++++++++- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index ef7e3de0728c..cea9053b6f67 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -495,8 +495,8 @@ classifier_count(const struct classifier *cls) static inline ovs_be32 minimatch_get_ports(const struct minimatch *match) { /* Could optimize to use the same map if needed for fast path. */ - return MINIFLOW_GET_BE32(match->flow, tp_src) - & MINIFLOW_GET_BE32(&match->mask->masks, tp_src); + return (miniflow_get_ports(match->flow) + & miniflow_get_ports(&match->mask->masks)); } /* Inserts 'rule' into 'cls' in 'version'. Until 'rule' is removed from 'cls', @@ -1501,7 +1501,7 @@ insert_subtable(struct classifier *cls, const struct minimask *mask) /* Ports trie. */ ovsrcu_set_hidden(&subtable->ports_trie, NULL); *CONST_CAST(int *, &subtable->ports_mask_len) - = 32 - ctz32(ntohl(MINIFLOW_GET_BE32(&mask->masks, tp_src))); + = 32 - ctz32(ntohl(miniflow_get_ports(&mask->masks))); /* List of rules. */ rculist_init(&subtable->rules_list); @@ -1696,7 +1696,7 @@ find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, unsigned int mbits; ovs_be32 value, plens, mask; - mask = MINIFLOW_GET_BE32(&subtable->mask.masks, tp_src); + mask = miniflow_get_ports(&subtable->mask.masks); value = ((OVS_FORCE ovs_be32 *)flow)[TP_PORTS_OFS32] & mask; mbits = trie_lookup_value(&subtable->ports_trie, &value, &plens, 32); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index b07fc6b8b327..be31fd0922b0 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -2200,7 +2200,8 @@ dp_netdev_pmd_lookup_flow(struct dp_netdev_pmd_thread *pmd, { struct dpcls *cls; struct dpcls_rule *rule; - odp_port_t in_port = u32_to_odp(MINIFLOW_GET_U32(&key->mf, in_port)); + odp_port_t in_port = u32_to_odp(MINIFLOW_GET_U32(&key->mf, + in_port.odp_port)); struct dp_netdev_flow *netdev_flow = NULL; cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port); diff --git a/lib/flow.c b/lib/flow.c index 38ff29c8cd14..09b66b81858f 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -2007,7 +2007,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, uint32_t basis) } /* Add both ports at once. */ - hash = hash_add(hash, MINIFLOW_GET_U32(flow, tp_src)); + hash = hash_add(hash, (OVS_FORCE uint32_t) miniflow_get_ports(flow)); } out: return hash_finish(hash, 42); diff --git a/lib/flow.h b/lib/flow.h index 770a07a62778..3331e2068ed8 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -684,6 +684,14 @@ miniflow_get__(const struct miniflow *mf, size_t idx) /* Get the value of the struct flow 'FIELD' as up to 8 byte wide integer type * 'TYPE' from miniflow 'MF'. */ #define MINIFLOW_GET_TYPE(MF, TYPE, FIELD) \ + (BUILD_ASSERT(sizeof(TYPE) == sizeof(((struct flow *)0)->FIELD)), \ + BUILD_ASSERT_GCCONLY(__builtin_types_compatible_p(TYPE, typeof(((struct flow *)0)->FIELD))), \ + MINIFLOW_GET_TYPE__(MF, TYPE, FIELD)) + +/* Like MINIFLOW_GET_TYPE, but without checking that TYPE is the correct width + * for FIELD. (This is useful for deliberately reading adjacent fields in one + * go.) */ +#define MINIFLOW_GET_TYPE__(MF, TYPE, FIELD) \ (MINIFLOW_IN_MAP(MF, FLOW_U64_OFFSET(FIELD)) \ ? ((OVS_FORCE const TYPE *)miniflow_get__(MF, FLOW_U64_OFFSET(FIELD))) \ [FLOW_U64_OFFREM(FIELD) / sizeof(TYPE)] \ @@ -806,7 +814,7 @@ miniflow_get_vid(const struct miniflow *flow, size_t n) { if (n < FLOW_MAX_VLAN_HEADERS) { union flow_vlan_hdr hdr = { - .qtag = MINIFLOW_GET_BE32(flow, vlans[n]) + .qtag = MINIFLOW_GET_BE32(flow, vlans[n].qtag) }; return vlan_tci_to_vid(hdr.tci); } @@ -849,6 +857,13 @@ miniflow_get_metadata(const struct miniflow *flow) return MINIFLOW_GET_BE64(flow, metadata); } + +/* Returns the 'tp_src' and 'tp_dst' fields together as one piece of data. */ +static inline ovs_be32 +miniflow_get_ports(const struct miniflow *flow) +{ + return MINIFLOW_GET_TYPE__(flow, ovs_be32, tp_src); +} /* Returns the mask for the OpenFlow 1.1+ "metadata" field in 'mask'. * * The return value is all-1-bits if 'mask' matches on the whole value of the -- 2.16.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
