On 4/8/22 13:39, Stokes, Ian wrote:
>> For packets which don't already have a hash calculated,
>> miniflow_hash_5tuple() calculates the hash of a packet
>> using the previously built miniflow.
>>
>> This commit adds IPv4 profile specific hashing which
>> uses fixed offsets into the packet to improve hashing
>> performance.
>>
>> Signed-off-by: Kumar Amber <[email protected]>
>> Signed-off-by: Harry van Haaren <[email protected]>
>> Co-authored-by: Harry van Haaren <[email protected]>
>> Acked-by: Cian Ferriter <[email protected]>
>
> Thanks for the patch Amber.
>
> @Ilya, I know your busy with the stable releases this week,
> but does this revision address you previous concerns?
It seems to address the unaligned memory access, yes. But there
are few style issues like use of the sizeof(type), and casts to
void * seems unnecessary.
I'd like to take a wider look at the set as a whole since I only
glanced over the first patch last time. That will not happen
today, but I hope to do that early next week. Does that sound
good to you?
Best regards, Ilya Maximets.
P.S. Please, don't re-spin the set just for style fixes for now.
>
> Thanks
> Ian
>
>> ---
>> v9:
>> - Use memcpy in place of typecast to fix memory alingment.
>> v8:
>> - Fix comments from cian.
>> v4:
>> - Use pre-defined hash length values.
>> v3:
>> - Fix check-patch sign-offs.
>> ---
>> ---
>> NEWS | 2 +
>> lib/dpif-netdev-extract-avx512.c | 72 ++++++++++++++++++++++++++++++++
>> 2 files changed, 74 insertions(+)
>>
>> diff --git a/NEWS b/NEWS
>> index 8fa57836a..db58be457 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -3,6 +3,8 @@ Post-v2.17.0
>> - OVSDB:
>> * 'relay' service model now supports transaction history, i.e. honors
>> the
>> 'last-txn-id' field in 'monitor_cond_since' requests from clients.
>> + - Userspace datapath:
>> + * Add IPv4 profile based 5tuple hashing optimizations.
>>
>>
>> v2.17.0 - 17 Feb 2022
>> diff --git a/lib/dpif-netdev-extract-avx512.c
>> b/lib/dpif-netdev-extract-avx512.c
>> index c1c1fefb6..1e0d4e31a 100644
>> --- a/lib/dpif-netdev-extract-avx512.c
>> +++ b/lib/dpif-netdev-extract-avx512.c
>> @@ -278,6 +278,10 @@ struct mfex_profile {
>> uint64_t mf_bits[FLOWMAP_UNITS];
>> uint16_t dp_pkt_offs[4];
>> uint16_t dp_pkt_min_size;
>> +
>> + /* Constant data offsets for hashing. */
>> + uint8_t hash_pkt_offs[4];
>> + uint32_t hash_len;
>> };
>>
>> /* Ensure dp_pkt_offs[4] is the correct size as in struct dp_packet. */
>> @@ -327,6 +331,13 @@ enum MFEX_PROFILES {
>> PROFILE_COUNT,
>> };
>>
>> +/* Packet offsets for 5 tuple hash function. */
>> +#define HASH_IPV4 \
>> + 26, 30, 23, 34
>> +
>> +#define HASH_DT1Q_IPV4 \
>> + 30, 34, 27, 38
>> +
>> /* Static const instances of profiles. These are compile-time constants,
>> * and are specialized into individual miniflow-extract functions.
>> * NOTE: Order of the fields is significant, any change in the order must be
>> @@ -347,6 +358,9 @@ static const struct mfex_profile
>> mfex_profiles[PROFILE_COUNT] =
>> 0, UINT16_MAX, 14, 34,
>> },
>> .dp_pkt_min_size = 42,
>> +
>> + .hash_pkt_offs = { HASH_IPV4 },
>> + .hash_len = 72,
>> },
>>
>> [PROFILE_ETH_IPV4_TCP] = {
>> @@ -370,6 +384,9 @@ static const struct mfex_profile
>> mfex_profiles[PROFILE_COUNT] =
>> 0, UINT16_MAX, 14, 34,
>> },
>> .dp_pkt_min_size = 54,
>> +
>> + .hash_pkt_offs = { HASH_IPV4 },
>> + .hash_len = 80,
>> },
>>
>> [PROFILE_ETH_VLAN_IPV4_UDP] = {
>> @@ -389,6 +406,9 @@ static const struct mfex_profile
>> mfex_profiles[PROFILE_COUNT] =
>> 14, UINT16_MAX, 18, 38,
>> },
>> .dp_pkt_min_size = 46,
>> +
>> + .hash_pkt_offs = { HASH_DT1Q_IPV4 },
>> + .hash_len = 80,
>> },
>>
>> [PROFILE_ETH_VLAN_IPV4_TCP] = {
>> @@ -414,6 +434,9 @@ static const struct mfex_profile
>> mfex_profiles[PROFILE_COUNT] =
>> 14, UINT16_MAX, 18, 38,
>> },
>> .dp_pkt_min_size = 58,
>> +
>> + .hash_pkt_offs = { HASH_DT1Q_IPV4 },
>> + .hash_len = 88,
>> },
>> };
>>
>> @@ -467,6 +490,40 @@ mfex_handle_tcp_flags(const struct tcp_header *tcp,
>> uint64_t *block)
>> *block = ctl_u64 << 32;
>> }
>>
>> +static inline void
>> +mfex_5tuple_hash_ipv4(struct dp_packet *packet, const uint8_t *pkt,
>> + struct netdev_flow_key *key,
>> + const uint8_t *pkt_offsets)
>> +{
>> + if (!dp_packet_rss_valid(packet)) {
>> + uint32_t hash = 0;
>> + uint32_t ip_src = 0;
>> + uint32_t ip_dst = 0;
>> + uint32_t ports = 0;
>> + void *ipv4_src = (void *) &pkt[pkt_offsets[0]];
>> + void *ipv4_dst = (void *) &pkt[pkt_offsets[1]];
>> + void *ports_l4 = (void *) &pkt[pkt_offsets[3]];
>> +
>> + memcpy(&ip_src, ipv4_src, sizeof(uint32_t));
>> + memcpy(&ip_dst, ipv4_dst, sizeof(uint32_t));
>> + memcpy(&ports, ports_l4, sizeof(uint32_t));
>> +
>> + /* IPv4 Src and Dst. */
>> + hash = hash_add(hash, ip_src);
>> + hash = hash_add(hash, ip_dst);
>> + /* IPv4 proto. */
>> + hash = hash_add(hash, pkt[pkt_offsets[2]]);
>> + /* L4 ports. */
>> + hash = hash_add(hash, ports);
>> + hash = hash_finish(hash, 42);
>> +
>> + dp_packet_set_rss_hash(packet, hash);
>> + key->hash = hash;
>> + } else {
>> + key->hash = dp_packet_get_rss_hash(packet);
>> + }
>> +}
>> +
>> /* Generic loop to process any mfex profile. This code is specialized into
>> * multiple actual MFEX implementation functions. Its marked ALWAYS_INLINE
>> * to ensure the compiler specializes each instance. The code is marked
>> "hot"
>> @@ -577,6 +634,10 @@ mfex_avx512_process(struct dp_packet_batch
>> *packets,
>> /* Process TCP flags, and store to blocks. */
>> const struct tcp_header *tcp = (void *)&pkt[38];
>> mfex_handle_tcp_flags(tcp, &blocks[7]);
>> +
>> + mfex_5tuple_hash_ipv4(packet, pkt, &keys[i],
>> + profile->hash_pkt_offs);
>> + keys[i].len = profile->hash_len;
>> } break;
>>
>> case PROFILE_ETH_VLAN_IPV4_UDP: {
>> @@ -588,6 +649,10 @@ mfex_avx512_process(struct dp_packet_batch
>> *packets,
>> UDP_HEADER_LEN)) {
>> continue;
>> }
>> +
>> + mfex_5tuple_hash_ipv4(packet, pkt, &keys[i],
>> + profile->hash_pkt_offs);
>> + keys[i].len = profile->hash_len;
>> } break;
>>
>> case PROFILE_ETH_IPV4_TCP: {
>> @@ -602,6 +667,10 @@ mfex_avx512_process(struct dp_packet_batch
>> *packets,
>> TCP_HEADER_LEN)) {
>> continue;
>> }
>> +
>> + mfex_5tuple_hash_ipv4(packet, pkt, &keys[i],
>> + profile->hash_pkt_offs);
>> + keys[i].len = profile->hash_len;
>> } break;
>>
>> case PROFILE_ETH_IPV4_UDP: {
>> @@ -613,6 +682,9 @@ mfex_avx512_process(struct dp_packet_batch
>> *packets,
>> continue;
>> }
>>
>> + mfex_5tuple_hash_ipv4(packet, pkt, &keys[i],
>> + profile->hash_pkt_offs);
>> + keys[i].len = profile->hash_len;
>> } break;
>> default:
>> break;
>> --
>> 2.25.1
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
