On 31/10/2022 12:39, Eelco Chaudron wrote:
>
>
> 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;
ok.
>
>> + }
>> +
>> + 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) {
ok
>
>> ovs_mutex_unlock(&prios_lock);
>> return 0;
>
> nit: return TC_RESERVED_PRIORITY_NONE
how about just return prio ? as the comparison is
against TC_RESERVED_PRIORITY_NONE only.
>
>> }
>>
>> 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
