On Nov 23, Dumitru Ceara wrote:
> 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?
I am fine to just keep a condition that limits the max size of
buffered_packets_map, but if you think that is really not important we can drop
it.
Regards,
Lorenzo
>
> Thanks,
> Dumitru
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
