On 27 Oct 2022, at 13:58, Roi Dayan wrote:
> From: Paul Blakey <[email protected]>
>
> Currently ethertype to prio hmap is static and the first ethertype
> being used gets a lower priority. Usually there is an arp request
> before the ip traffic and the arp ethertype gets a lower tc priority
> while the ip traffic proto gets a higher priority.
> In this case ip traffic will go through more hops in tc and HW.
> Instead, reserve lower priorities for ip ethertypes.
Changes look good to me, only one little nit, but this is just personal
preference.
I did not do any specific testing other than the make check tests.
Acked-by: Eelco Chaudron <[email protected]>
> Signed-off-by: Paul Blakey <[email protected]>
> Reviewed-by: Roi Dayan <[email protected]>
> ---
>
> Notes:
> v2:
> - don't reverse ip prio if multi mask per prio is not supported.
>
> lib/netdev-offload-tc.c | 33 +++++++++++++++++++++++++++------
> lib/tc.h | 2 ++
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index f6f90a741fde..b392bbf441f4 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -325,6 +325,28 @@ struct prio_map_data {
> uint16_t prio;
> };
>
> +static uint16_t
> +get_next_available_prio(ovs_be16 protocol)
> +{
> + static uint16_t last_prio = TC_RESERVED_PRIORITY_MAX;
> +
> + if (multi_mask_per_prio) {
> + if (protocol == htons(ETH_P_IP)) {
> + return TC_RESERVED_PRIORITY_IPV4;
> + } else if (protocol == htons(ETH_P_IPV6)) {
> + return TC_RESERVED_PRIORITY_IPV6;
> + }
> + }
> +
> + /* last_prio can overflow if there will be many different kinds of
> + * flows which shouldn't happen organically. */
> + if (last_prio == UINT16_MAX) {
> + return 0;
nit: return TC_RESERVED_PRIORITY_NONE;
> + }
> +
> + return ++last_prio;
> +}
> +
> /* Get free prio for tc flower
> * If prio is already allocated for mask/eth_type combination then return it.
> * If not assign new prio.
> @@ -336,11 +358,11 @@ 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 uint16_t last_prio = TC_RESERVED_PRIORITY_MAX;
> size_t key_len = sizeof(struct tc_flower_key);
> size_t hash = hash_int((OVS_FORCE uint32_t) flower->key.eth_type, 0);
> struct prio_map_data *data;
> struct prio_map_data *new_data;
> + uint16_t prio;
>
> if (!multi_mask_per_prio) {
> hash = hash_bytes(&flower->mask, key_len, hash);
> @@ -359,21 +381,20 @@ get_prio_for_tc_flower(struct tc_flower *flower)
> }
> }
>
> - if (last_prio == UINT16_MAX) {
> - /* last_prio can overflow if there will be many different kinds of
> - * flows which shouldn't happen organically. */
> + prio = get_next_available_prio(flower->key.eth_type);
> + if (!prio) {
nit: if (prio == TC_RESERVED_PRIORITY_NONE) {
> ovs_mutex_unlock(&prios_lock);
> return 0;
nit: return TC_RESERVED_PRIORITY_NONE
> }
>
> new_data = xzalloc(sizeof *new_data);
> memcpy(&new_data->mask, &flower->mask, key_len);
> - new_data->prio = ++last_prio;
> + new_data->prio = prio;
> new_data->protocol = flower->key.eth_type;
> hmap_insert(&prios, &new_data->node, hash);
> ovs_mutex_unlock(&prios_lock);
>
> - return new_data->prio;
> + return prio;
> }
>
> static uint32_t
> diff --git a/lib/tc.h b/lib/tc.h
> index 2e64ad372592..12753c16d405 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -49,6 +49,8 @@
> enum tc_flower_reserved_prio {
> TC_RESERVED_PRIORITY_NONE,
> TC_RESERVED_PRIORITY_POLICE,
> + TC_RESERVED_PRIORITY_IPV4,
> + TC_RESERVED_PRIORITY_IPV6,
> __TC_RESERVED_PRIORITY_MAX
> };
> #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1)
> --
> 2.38.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
