[ovs-dev] [PATCH ovs V3 13/25] netdev-tc-offloads: Add flower mask to priority map
Marcelo Ricardo Leitner
mleitner at redhat.com
Fri Feb 17 21:15:19 UTC 2017
On Wed, Feb 08, 2017 at 05:29:26PM +0200, Roi Dayan wrote:
> From: Paul Blakey <paulb at mellanox.com>
>
> Flower classifer requires a different priority per mask,
> so we hash the mask and generate a new priority for
> each new mask used.
>
> Signed-off-by: Paul Blakey <paulb at mellanox.com>
> Reviewed-by: Roi Dayan <roid at mellanox.com>
> ---
> lib/netdev-tc-offloads.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index f88e7ce..48e452a 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -176,6 +176,44 @@ add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
> return replace;
> }
>
> +struct prio_map_data {
> + struct hmap_node node;
> + struct tc_flower_key mask;
> + ovs_be16 protocol;
> + uint16_t prio;
^^^^^^^^
> +};
> +
> +static uint16_t
^^^^^^^^
> +get_prio_for_tc_flower(struct tc_flower *flower)
> +{
> + static struct hmap prios = HMAP_INITIALIZER(&prios);
> + static struct ovs_mutex prios_lock = OVS_MUTEX_INITIALIZER;
> + static int last_prio = 0;
^^^
> + size_t key_len = sizeof(struct tc_flower_key);
> + size_t hash = hash_bytes(&flower->mask, key_len,
> + (OVS_FORCE uint32_t) flower->key.eth_type);
> + struct prio_map_data *data;
> + struct prio_map_data *new_data;
> +
> + ovs_mutex_lock(&prios_lock);
> + HMAP_FOR_EACH_WITH_HASH(data, node, hash, &prios) {
> + if (!memcmp(&flower->mask, &data->mask, key_len)
> + && data->protocol == flower->key.eth_type) {
> + ovs_mutex_unlock(&prios_lock);
> + return data->prio;
> + }
> + }
> +
> + new_data = xzalloc(sizeof *new_data);
> + memcpy(&new_data->mask, &flower->mask, key_len);
> + new_data->prio = ++last_prio;
^^^^^^^^^^^^^^^^^^
I don't think this type difference is an issue, but it would be nice if
it's avoided.
More importantly, last_prio will overflow sooner or later here, and I
don't see any check on whether the new value is actually free to use. As
you explained on the other email, if there is a mask with another
protocol already using this priority, flower will reject it.
Considering the useful range is only 16 bit wide, this may happen quite
often I'm afraid.
Marcelo
> + new_data->protocol = flower->key.eth_type;
> + hmap_insert(&prios, &new_data->node, hash);
> + ovs_mutex_unlock(&prios_lock);
> +
> + return new_data->prio;
> +}
> +
> int
> netdev_tc_flow_flush(struct netdev *netdev)
> {
> --
> 2.7.4
>
More information about the dev
mailing list