On 11/23/22 15:26, Lorenzo Bianconi wrote:
>>>>> /* Called with in the pinctrl_handler thread context. */
>>>>> static int
>>>>> pinctrl_handle_buffered_packets(struct dp_packet *pkt_in,
>>>>> const struct match *md, bool is_arp)
>>>>> OVS_REQUIRES(pinctrl_mutex)
>>>>> {
>>>>> - struct buffered_packets *bp;
>>>>> - struct dp_packet *clone;
>>>>> - struct in6_addr addr;
>>>>> -
>>>>> - if (is_arp) {
>>>>> - addr = in6_addr_mapped_ipv4(htonl(md->flow.regs[0]));
>>>>> - } else {
>>>>> - ovs_be128 ip6 = hton128(flow_get_xxreg(&md->flow, 0));
>>>>> - memcpy(&addr, &ip6, sizeof addr);
>>>>> - }
>>>>> -
>>>>> - uint32_t hash = hash_bytes(&addr, sizeof addr, 0);
>>>>> - bp = pinctrl_find_buffered_packets(&addr, hash);
>>>>> + uint64_t dp_key = ntohll(md->flow.metadata);
>>>>> + uint64_t oport_key = md->flow.regs[MFF_LOG_OUTPORT - MFF_REG0];
>>>>> + uint32_t hash = pinctrl_buffer_apcket_hash(dp_key, oport_key);
>>>>> + struct buffered_packets *bp
>>>>> + = pinctrl_find_buffered_packets(dp_key, oport_key, hash);
>>>>> if (!bp) {
>>>>> if (hmap_count(&buffered_packets_map) >= 1000) {
>> Before your patch we would hit this hard coded limit if there were 1000
>> next hops for which we were bufferring packets.
>>
>> Now, with your change, we will hit his limit only if there are 1000
>> ports for which we are bufferring packets. This seems very unlikely and
>> I wonder if this will ever happen in practice. Do we need to change its
>> value? Also, let's add a define for it somewhere to make it a bit less
>> "magic".
> ack. I would prefer to be a bit more conservative and keep the condition.
> What is the right value to use up to you? 😄
>
Well, if you want to keep the exact same condition you'd have to count
how many individual next hops we're buffering packets for. Is that an
option?
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
