On Wed, May 27, 2026 at 11:36 AM Ales Musil <[email protected]> wrote: > > > > On Wed, May 27, 2026 at 4:18 PM Mark Michelson <[email protected]> wrote: >> >> Hi Ales, >> >> I did a detailed analysis on this, and the lock-free methods used here >> look sound to me. I tried thinking of odd sequences of events and it >> appeared that in every case, it should not encounter errors. The worst >> thing I could find was a potential spurious wake-up of the pinctrl >> handler thread, but that is not a big deal. >> >> I have a more basic concern though. While this patch lessens the >> chances of the pinctrl_mutex causing delays of buffered packets, it >> does not outright eliminate the problem. In most cases, I believe the >> assumption is that the workflow goes as such: >> * We receive ARP or ND packet and call >> pinctrl_handle_buffered_packets() from the pinctrl_handler thread. >> * pinctrl_handle_buffered_packets() creates the buffered_packets >> struct and adds it to the cmap. The pinctrl_handler thread then >> notifies the main thread. >> * The main thread wakes up and calls buffered_packets_lookup_run(). >> This results in setting the MAC address in the buffered_packets >> structure. The main thread then notifies the pinctrl_handler thread. >> * The pinctrl_handler thread wakes up and calls >> send_mac_binding_buffered_pkts() in order to send the buffered packets >> and remove them from the cmap. >> >> The above workflow is now lock-free with this change. However, other >> parts of both threads still use the pinctrl_mutex. Consider in the >> last step that when the pinctrl_handler thread wakes up, it handles an >> action that uses the pinctrl_mutex (e.g. ACTION_OPCODE_BFD_MSG). Now >> the pinctrl_handler thread may need to wait on that mutex to be >> unlocked by the main thread before it can handle the BFD message. Only >> after handling incoming packets will the pinctrl_handler thread run >> send_mac_binding_buffered_pkts(). The result is the same as it was >> before this patch: the pinctrl_handler thread has to wait for the main >> thread to release the mutex in order for buffered packets to be sent >> out. >> >> If we're fine with reducing the likelihood of a delay in the >> pinctrl_handler thread, then this change works. However, if we want to >> guarantee there will be no delay in the processing of buffered >> packets, then we need some additional code in place to guarantee the >> swift handling of buffered packets. > > > Hi Mark, > > thank you for the review. You are raising a good point > so I'll try to explain why I think it's still a good > idea to proceed with this. So as you said this doesn't > prevent lock contention on its own, but in my opinion > is still a step in the right direction. The reason why > I went with ARP handling is that this is probably the > most common code path we hit in pinctrl packet-in > processing. There is a pretty big chance that the 50 > packets we process in a single pinctrl thread loop > will all be ARPs. In that case there won't be any > contention at all. Of course that would be a perfect > scenario, and if one of those packets requires the > lock it will add latency for all remaining packets in > that batch, but the buffered packet path itself > remains entirely lock-free. > > With that said, the patch itself isn't that complex > and is low risk especially only for the next release. > I also plan to tackle other lock "offenders" in the > future to reduce the lock contention to a minimum. > Removing it completely is probably not worth the > effort as some of the packets are very rare to hit so > the benefit versus the code complexity would be almost > none. Just to give you an idea, the next thing I have > in mind are put_ARP/ND/FDB actions. I'm pretty > confident that together with this patch we will be > looking at solid performance improvement for pinctrl > packet processing. > > Regards, > Ales
Thanks for the explanation, Ales. I agree that in the vast majority of cases, this is going to decrease contention. That, plus the knowledge that you plan to cover other common packet-in scenarios means that I'm happy to accept this patch without any further additions. Acked-by: Mark Michelson <[email protected]> >> >> >> On Tue, May 26, 2026 at 11:17 AM Ales Musil via dev >> <[email protected]> wrote: >> > >> > The packet buffering, which is part of the ARP processing, required >> > a pinctrl lock. As a consequence we could block packet processing >> > when the lock is taken by main thread and possibly drop some traffic >> > that is heading towards pinctrl. Make sure the packet buffering is >> > now handled without any lock, that is done by using cmap instead of >> > hmap to store the objects and all concurrent members are accessed >> > via atomics instead. This should increase the throughput of the >> > packet-in processing for ARP handling. >> > >> > Reported-at: https://redhat.atlassian.net/browse/FDP-3301 >> > Signed-off-by: Ales Musil <[email protected]> >> > --- >> > controller/mac-cache.c | 127 +++++++++++++++++++++++------------------ >> > controller/mac-cache.h | 48 +++++++--------- >> > controller/pinctrl.c | 81 +++++++++++--------------- >> > 3 files changed, 125 insertions(+), 131 deletions(-) >> > >> > diff --git a/controller/mac-cache.c b/controller/mac-cache.c >> > index bdf35eeb7..d345d5c54 100644 >> > --- a/controller/mac-cache.c >> > +++ b/controller/mac-cache.c >> > @@ -59,12 +59,11 @@ static void >> > mac_cache_update_req_delay(struct hmap *thresholds, uint64_t *req_delay); >> > >> > static struct buffered_packets * >> > -buffered_packets_find(struct buffered_packets_ctx *ctx, >> > +buffered_packets_find(struct cmap *bp_map, >> > const struct mac_binding_data *mb_data); >> > >> > static void >> > -buffered_packets_remove(struct buffered_packets_ctx *ctx, >> > - struct buffered_packets *bp); >> > +buffered_packets_free(struct buffered_packets *bp); >> > >> > static void >> > buffered_packets_db_lookup(struct buffered_packets *bp, >> > @@ -550,24 +549,24 @@ bp_packet_data_destroy(struct bp_packet_data *pd) { >> > } >> > >> > struct buffered_packets * >> > -buffered_packets_add(struct buffered_packets_ctx *ctx, >> > - struct mac_binding_data mb_data) { >> > +buffered_packets_add(struct cmap *bp_map, struct mac_binding_data >> > mb_data) { >> > uint32_t hash = mac_binding_data_hash(&mb_data); >> > >> > - struct buffered_packets *bp = buffered_packets_find(ctx, &mb_data); >> > + struct buffered_packets *bp = buffered_packets_find(bp_map, &mb_data); >> > if (!bp) { >> > - if (hmap_count(&ctx->buffered_packets) >= MAX_BUFFERED_PACKETS) { >> > + if (cmap_count(bp_map) >= MAX_BUFFERED_PACKETS) { >> > return NULL; >> > } >> > >> > bp = xmalloc(sizeof *bp); >> > - hmap_insert(&ctx->buffered_packets, &bp->hmap_node, hash); >> > bp->mb_data = mb_data; >> > + atomic_init(&bp->resolved_mac, 0); >> > /* Schedule the freshly added buffered packet to do lookup >> > * immediately. */ >> > bp->lookup_at_ms = 0; >> > bp->queue = VECTOR_CAPACITY_INITIALIZER(struct bp_packet_data, >> > BUFFER_QUEUE_DEPTH); >> > + cmap_insert(bp_map, &bp->cmap_node, hash); >> > } >> > >> > bp->expire_at_ms = time_msec() + BUFFERED_PACKETS_TIMEOUT_MS; >> > @@ -605,25 +604,28 @@ buffered_packets_packet_data_enqueue(struct >> > buffered_packets *bp, >> > vector_push(&bp->queue, &pd); >> > } >> > >> > -void >> > -buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, >> > - const struct hmap *recent_mbs, >> > - struct ovsdb_idl_index *sbrec_pb_by_key, >> > - struct ovsdb_idl_index *sbrec_dp_by_key, >> > - struct ovsdb_idl_index *sbrec_pb_by_name, >> > - struct ovsdb_idl_index *sbrec_mb_by_lport_ip) { >> > +bool >> > +buffered_packets_lookup_run(struct cmap *bp_map, const struct hmap >> > *recent_mbs, >> > + struct ovsdb_idl_index *sbrec_pb_by_key, >> > + struct ovsdb_idl_index *sbrec_dp_by_key, >> > + struct ovsdb_idl_index *sbrec_pb_by_name, >> > + struct ovsdb_idl_index *sbrec_mb_by_lport_ip) >> > { >> > struct ds ip = DS_EMPTY_INITIALIZER; >> > long long now = time_msec(); >> > + bool updated = false; >> > >> > struct buffered_packets *bp; >> > - HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { >> > - struct eth_addr mac = eth_addr_zero; >> > - /* Remove expired buffered packets. */ >> > - if (now > bp->expire_at_ms) { >> > - buffered_packets_remove(ctx, bp); >> > + CMAP_FOR_EACH (bp, cmap_node, bp_map) { >> > + uint64_t mac64; >> > + atomic_read(&bp->resolved_mac, &mac64); >> > + /* MAC for given entry was already resolved, >> > + * no need to resolve it again. */ >> > + if (mac64) { >> > continue; >> > } >> > >> > + struct eth_addr mac = eth_addr_zero; >> > + >> > struct mac_binding *mb = mac_binding_find(recent_mbs, >> > &bp->mb_data); >> > if (mb) { >> > mac = mb->data.mac; >> > @@ -634,13 +636,46 @@ buffered_packets_ctx_run(struct buffered_packets_ctx >> > *ctx, >> > sbrec_mb_by_lport_ip); >> > /* Schedule next lookup even if we found the MAC address, >> > * if the address was found this struct will be deleted >> > anyway. */ >> > + >> > bp->lookup_at_ms = now + BUFFERED_PACKETS_LOOKUP_MS; >> > } >> > >> > - if (eth_addr_is_zero(mac)) { >> > + if (!eth_addr_is_zero(mac)) { >> > + atomic_store(&bp->resolved_mac, eth_addr_to_uint64(mac)); >> > + updated = true; >> > + } >> > + } >> > + >> > + ds_destroy(&ip); >> > + >> > + return updated; >> > +} >> > + >> > +void >> > +buffered_packets_run(struct cmap *bp_map, struct vector *rpd) >> > +{ >> > + long long now = time_msec(); >> > + >> > + struct buffered_packets *bp; >> > + CMAP_FOR_EACH (bp, cmap_node, bp_map) { >> > + uint32_t hash = mac_binding_data_hash(&bp->mb_data); >> > + >> > + /* Remove expired buffered packets. */ >> > + if (now > bp->expire_at_ms) { >> > + cmap_remove(bp_map, &bp->cmap_node, hash); >> > + ovsrcu_postpone(buffered_packets_free, bp); >> > + continue; >> > + } >> > + >> > + uint64_t mac64; >> > + atomic_read(&bp->resolved_mac, &mac64); >> > + if (!mac64) { >> > continue; >> > } >> > >> > + struct eth_addr mac; >> > + eth_addr_from_uint64(mac64, &mac); >> > + >> > struct bp_packet_data *pd; >> > VECTOR_FOR_EACH_PTR (&bp->queue, pd) { >> > struct dp_packet packet; >> > @@ -650,45 +685,25 @@ buffered_packets_ctx_run(struct buffered_packets_ctx >> > *ctx, >> > eth->eth_dst = mac; >> > } >> > >> > - vector_push_array(&ctx->ready_packets_data, >> > - vector_get_array(&bp->queue), >> > + vector_push_array(rpd, vector_get_array(&bp->queue), >> > vector_len(&bp->queue)); >> > vector_clear(&bp->queue); >> > - buffered_packets_remove(ctx, bp); >> > - } >> > - >> > - ds_destroy(&ip); >> > -} >> > - >> > -bool >> > -buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx) { >> > - return !vector_is_empty(&ctx->ready_packets_data); >> > -} >> > - >> > -bool >> > -buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx) { >> > - return !hmap_is_empty(&ctx->buffered_packets); >> > -} >> > >> > -void >> > -buffered_packets_ctx_init(struct buffered_packets_ctx *ctx) { >> > - ctx->ready_packets_data = VECTOR_EMPTY_INITIALIZER(struct >> > bp_packet_data); >> > - hmap_init(&ctx->buffered_packets); >> > + cmap_remove(bp_map, &bp->cmap_node, hash); >> > + ovsrcu_postpone(buffered_packets_free, bp); >> > + } >> > } >> > >> > void >> > -buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx) { >> > - struct bp_packet_data *pd; >> > - VECTOR_FOR_EACH_PTR (&ctx->ready_packets_data, pd) { >> > - bp_packet_data_destroy(pd); >> > - } >> > - vector_destroy(&ctx->ready_packets_data); >> > - >> > +buffered_packets_map_destroy(struct cmap *bp_map) { >> > struct buffered_packets *bp; >> > - HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { >> > - buffered_packets_remove(ctx, bp); >> > + CMAP_FOR_EACH (bp, cmap_node, bp_map) { >> > + cmap_remove(bp_map, &bp->cmap_node, >> > + mac_binding_data_hash(&bp->mb_data)); >> > + ovsrcu_postpone(buffered_packets_free, bp); >> > } >> > - hmap_destroy(&ctx->buffered_packets); >> > + >> > + cmap_destroy(bp_map); >> > } >> > >> > static uint32_t >> > @@ -771,12 +786,12 @@ mac_cache_update_req_delay(struct hmap *thresholds, >> > uint64_t *req_delay) >> > } >> > >> > static struct buffered_packets * >> > -buffered_packets_find(struct buffered_packets_ctx *ctx, >> > +buffered_packets_find(struct cmap *bp_map, >> > const struct mac_binding_data *mb_data) { >> > uint32_t hash = mac_binding_data_hash(mb_data); >> > >> > struct buffered_packets *bp; >> > - HMAP_FOR_EACH_WITH_HASH (bp, hmap_node, hash, &ctx->buffered_packets) >> > { >> > + CMAP_FOR_EACH_WITH_HASH (bp, cmap_node, hash, bp_map) { >> > if (mac_binding_data_equals(&bp->mb_data, mb_data)) { >> > return bp; >> > } >> > @@ -786,14 +801,12 @@ buffered_packets_find(struct buffered_packets_ctx >> > *ctx, >> > } >> > >> > static void >> > -buffered_packets_remove(struct buffered_packets_ctx *ctx, >> > - struct buffered_packets *bp) { >> > +buffered_packets_free(struct buffered_packets *bp) { >> > struct bp_packet_data *pd; >> > VECTOR_FOR_EACH_PTR (&bp->queue, pd) { >> > bp_packet_data_destroy(pd); >> > } >> > >> > - hmap_remove(&ctx->buffered_packets, &bp->hmap_node); >> > vector_destroy(&bp->queue); >> > free(bp); >> > } >> > diff --git a/controller/mac-cache.h b/controller/mac-cache.h >> > index 7edb129d7..365219d33 100644 >> > --- a/controller/mac-cache.h >> > +++ b/controller/mac-cache.h >> > @@ -18,9 +18,9 @@ >> > >> > #include <stdint.h> >> > >> > +#include "cmap.h" >> > #include "dp-packet.h" >> > #include "openvswitch/hmap.h" >> > -#include "openvswitch/hmap.h" >> > #include "openvswitch/list.h" >> > #include "openvswitch/ofpbuf.h" >> > #include "openvswitch/ofp-flow.h" >> > @@ -115,25 +115,25 @@ struct bp_packet_data { >> > }; >> > >> > struct buffered_packets { >> > - struct hmap_node hmap_node; >> > + struct cmap_node cmap_node; >> > >> > - struct mac_binding_data mb_data; >> > + struct mac_binding_data mb_data; /* Immutable after insert. */ >> > >> > - /* Queue of packet_data associated with this struct. */ >> > + /* Queue of packet_data associated with this struct. >> > + * Handler thread only. */ >> > struct vector queue; >> > >> > - /* Timestamp in ms when the buffered packet should expire. */ >> > + /* Timestamp in ms when the buffered packet should expire. >> > + * Handler thread only. */ >> > long long int expire_at_ms; >> > >> > - /* Timestamp in ms when the buffered packet should do full SB >> > lookup.*/ >> > - long long int lookup_at_ms; >> > -}; >> > + /* Resolved MAC address packed as uint64. 0 means unresolved. >> > + * Written by main thread, read by handler thread. */ >> > + atomic_uint64_t resolved_mac; >> > >> > -struct buffered_packets_ctx { >> > - /* Map of all buffered packets waiting for the MAC address. */ >> > - struct hmap buffered_packets; >> > - /* List of packet data that are ready to be sent. */ >> > - struct vector ready_packets_data; >> > + /* Timestamp in ms when the buffered packet should do full SB lookup. >> > + * Main thread only. */ >> > + long long int lookup_at_ms; >> > }; >> > >> > /* Thresholds. */ >> > @@ -211,27 +211,23 @@ void fdb_stats_run(struct vector *stats_vec, >> > uint64_t *req_delay, void *data); >> > void bp_packet_data_destroy(struct bp_packet_data *pd); >> > >> > struct buffered_packets * >> > -buffered_packets_add(struct buffered_packets_ctx *ctx, >> > +buffered_packets_add(struct cmap *bp_map, >> > struct mac_binding_data mb_data); >> > >> > void buffered_packets_packet_data_enqueue(struct buffered_packets *bp, >> > const struct ofputil_packet_in >> > *pin, >> > const struct ofpbuf >> > *continuation); >> > >> > -void buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, >> > - const struct hmap *recent_mbs, >> > - struct ovsdb_idl_index *sbrec_pb_by_key, >> > - struct ovsdb_idl_index *sbrec_dp_by_key, >> > - struct ovsdb_idl_index *sbrec_pb_by_name, >> > - struct ovsdb_idl_index >> > *sbrec_mb_by_lport_ip); >> > - >> > -void buffered_packets_ctx_init(struct buffered_packets_ctx *ctx); >> > - >> > -void buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx); >> > +bool buffered_packets_lookup_run(struct cmap *bp_map, >> > + const struct hmap *recent_mbs, >> > + struct ovsdb_idl_index *sbrec_pb_by_key, >> > + struct ovsdb_idl_index *sbrec_dp_by_key, >> > + struct ovsdb_idl_index *sbrec_pb_by_name, >> > + struct ovsdb_idl_index >> > *sbrec_mb_by_lport_ip); >> > >> > -bool buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx >> > *ctx); >> > +void buffered_packets_run(struct cmap *bp_map, struct vector *rpd); >> > >> > -bool buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx); >> > +void buffered_packets_map_destroy(struct cmap *bp_map); >> > >> > void mac_binding_probe_stats_process_flow_stats( >> > struct vector *stats_vec, >> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> > index 4473ec668..bb8d20e7f 100644 >> > --- a/controller/pinctrl.c >> > +++ b/controller/pinctrl.c >> > @@ -185,16 +185,15 @@ struct pinctrl { >> > static struct pinctrl pinctrl; >> > >> > static bool pinctrl_is_sb_commited(int64_t commit_cfg, int64_t cur_cfg); >> > -static void init_buffered_packets_ctx(void); >> > -static void destroy_buffered_packets_ctx(void); >> > +static void init_buffered_packets_map(void); >> > +static void destroy_buffered_packets_map(void); >> > static void >> > run_buffered_binding(const struct sbrec_mac_binding_table >> > *mac_binding_table, >> > const struct hmap *local_datapaths, >> > struct ovsdb_idl_index *sbrec_port_binding_by_key, >> > struct ovsdb_idl_index >> > *sbrec_datapath_binding_by_key, >> > struct ovsdb_idl_index *sbrec_port_binding_by_name, >> > - struct ovsdb_idl_index >> > *sbrec_mac_binding_by_lport_ip) >> > - OVS_REQUIRES(pinctrl_mutex); >> > + struct ovsdb_idl_index >> > *sbrec_mac_binding_by_lport_ip); >> > >> > static void pinctrl_handle_put_mac_binding(const struct flow *md, >> > const struct flow *headers, >> > @@ -209,8 +208,7 @@ static void run_put_mac_bindings( >> > struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip) >> > OVS_REQUIRES(pinctrl_mutex); >> > static void wait_put_mac_bindings(void); >> > -static void send_mac_binding_buffered_pkts(struct rconn *swconn) >> > - OVS_REQUIRES(pinctrl_mutex); >> > +static void send_mac_binding_buffered_pkts(struct rconn *swconn); >> > >> > static void pinctrl_activation_strategy_handler(const struct match *md); >> > >> > @@ -562,7 +560,7 @@ pinctrl_init(void) >> > init_send_arps_nds(); >> > init_ipv6_ras(); >> > init_ipv6_prefixd(); >> > - init_buffered_packets_ctx(); >> > + init_buffered_packets_map(); >> > init_activated_ports(); >> > init_event_table(); >> > ip_mcast_snoop_init(); >> > @@ -1577,18 +1575,18 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn >> > *ovnsb_idl_txn, >> > } >> > } >> > >> > -static struct buffered_packets_ctx buffered_packets_ctx; >> > +static struct cmap buffered_packets_map; >> > >> > static void >> > -init_buffered_packets_ctx(void) >> > +init_buffered_packets_map(void) >> > { >> > - buffered_packets_ctx_init(&buffered_packets_ctx); >> > + cmap_init(&buffered_packets_map); >> > } >> > >> > static void >> > -destroy_buffered_packets_ctx(void) >> > +destroy_buffered_packets_map(void) >> > { >> > - buffered_packets_ctx_destroy(&buffered_packets_ctx); >> > + buffered_packets_map_destroy(&buffered_packets_map); >> > } >> > >> > /* Called with in the pinctrl_handler thread context. */ >> > @@ -1596,7 +1594,6 @@ static void >> > pinctrl_handle_buffered_packets(const struct ofputil_packet_in *pin, >> > const struct ofpbuf *continuation, >> > bool is_arp) >> > -OVS_REQUIRES(pinctrl_mutex) >> > { >> > const struct match *md = &pin->flow_metadata; >> > struct mac_binding_data mb_data; >> > @@ -1613,7 +1610,7 @@ OVS_REQUIRES(pinctrl_mutex) >> > md->flow.regs[MFF_LOG_OUTPORT - MFF_REG0], >> > ip, eth_addr_zero); >> > >> > - struct buffered_packets *bp = >> > buffered_packets_add(&buffered_packets_ctx, >> > + struct buffered_packets *bp = >> > buffered_packets_add(&buffered_packets_map, >> > mb_data); >> > if (!bp) { >> > COVERAGE_INC(pinctrl_drop_buffered_packets_map); >> > @@ -1643,9 +1640,7 @@ pinctrl_handle_arp(struct rconn *swconn, const >> > struct flow *ip_flow, >> > return; >> > } >> > >> > - ovs_mutex_lock(&pinctrl_mutex); >> > pinctrl_handle_buffered_packets(pin, continuation, true); >> > - ovs_mutex_unlock(&pinctrl_mutex); >> > >> > /* Compose an ARP packet. */ >> > uint64_t packet_stub[128 / 8]; >> > @@ -4081,12 +4076,12 @@ pinctrl_handler(void *arg_) >> > send_arp_nd_run(swconn, &send_arp_nd_time); >> > send_ipv6_ras(swconn, &send_ipv6_ra_time); >> > send_ipv6_prefixd(swconn, &send_prefixd_time); >> > - send_mac_binding_buffered_pkts(swconn); >> > bfd_monitor_send_msg(swconn, &bfd_time); >> > ovs_mutex_unlock(&pinctrl_mutex); >> > } else { >> > lock_failed = true; >> > } >> > + send_mac_binding_buffered_pkts(swconn); >> > send_garp_rarp_run(swconn, &send_garp_rarp_time); >> > ip_mcast_querier_run(swconn, &send_mcast_query_time); >> > } >> > @@ -4226,11 +4221,6 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, >> > sbrec_port_binding_by_key, >> > sbrec_igmp_groups, >> > sbrec_ip_multicast_opts); >> > - run_buffered_binding(mac_binding_table, local_datapaths, >> > - sbrec_port_binding_by_key, >> > - sbrec_datapath_binding_by_key, >> > - sbrec_port_binding_by_name, >> > - sbrec_mac_binding_by_lport_ip); >> > sync_svc_monitors(ovnsb_idl_txn, svc_mon_table, >> > sbrec_port_binding_by_name, >> > chassis); >> > bfd_monitor_run(ovnsb_idl_txn, bfd_table, sbrec_port_binding_by_name, >> > @@ -4241,6 +4231,12 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, >> > run_activated_ports(ovnsb_idl_txn, sbrec_datapath_binding_by_key, >> > sbrec_port_binding_by_key, chassis); >> > ovs_mutex_unlock(&pinctrl_mutex); >> > + >> > + run_buffered_binding(mac_binding_table, local_datapaths, >> > + sbrec_port_binding_by_key, >> > + sbrec_datapath_binding_by_key, >> > + sbrec_port_binding_by_name, >> > + sbrec_mac_binding_by_lport_ip); >> > } >> > >> > /* Table of ipv6_ra_state structures, keyed on logical port name. >> > @@ -4758,7 +4754,7 @@ pinctrl_destroy(void) >> > destroy_send_arps_nds(); >> > destroy_ipv6_ras(); >> > destroy_ipv6_prefixd(); >> > - destroy_buffered_packets_ctx(); >> > + destroy_buffered_packets_map(); >> > destroy_activated_ports(); >> > event_table_destroy(); >> > destroy_put_mac_bindings(); >> > @@ -4843,30 +4839,24 @@ pinctrl_handle_put_mac_binding(const struct flow >> > *md, >> > notify_pinctrl_main(); >> > } >> > >> > -#define READY_PACKETS_VEC_CAPACITY_THRESHOLD 1024 >> > - >> > /* Called with in the pinctrl_handler thread context. */ >> > static void >> > send_mac_binding_buffered_pkts(struct rconn *swconn) >> > - OVS_REQUIRES(pinctrl_mutex) >> > { >> > enum ofp_version version = rconn_get_version(swconn); >> > enum ofputil_protocol proto = >> > ofputil_protocol_from_ofp_version(version); >> > - struct vector *rpd = &buffered_packets_ctx.ready_packets_data; >> > + struct vector rpd = VECTOR_EMPTY_INITIALIZER(struct bp_packet_data); >> > + >> > + buffered_packets_run(&buffered_packets_map, &rpd); >> > >> > struct bp_packet_data *pd; >> > - VECTOR_FOR_EACH_PTR (rpd, pd) { >> > + VECTOR_FOR_EACH_PTR (&rpd, pd) { >> > queue_msg(swconn, ofputil_encode_resume(&pd->pin, >> > pd->continuation, >> > proto)); >> > bp_packet_data_destroy(pd); >> > } >> > >> > - vector_clear(rpd); >> > - if (vector_capacity(rpd) >= READY_PACKETS_VEC_CAPACITY_THRESHOLD) { >> > - VLOG_DBG("The ready_packets_data vector capacity (%"PRIuSIZE") " >> > - "is over threshold.", vector_capacity(rpd)); >> > - vector_shrink_to_fit(rpd); >> > - } >> > + vector_destroy(&rpd); >> > } >> > >> > static void >> > @@ -4935,9 +4925,8 @@ run_buffered_binding(const struct >> > sbrec_mac_binding_table *mac_binding_table, >> > struct ovsdb_idl_index >> > *sbrec_datapath_binding_by_key, >> > struct ovsdb_idl_index *sbrec_port_binding_by_name, >> > struct ovsdb_idl_index >> > *sbrec_mac_binding_by_lport_ip) >> > - OVS_REQUIRES(pinctrl_mutex) >> > { >> > - if (!buffered_packets_ctx_has_packets(&buffered_packets_ctx)) { >> > + if (cmap_is_empty(&buffered_packets_map)) { >> > return; >> > } >> > >> > @@ -4983,18 +4972,16 @@ run_buffered_binding(const struct >> > sbrec_mac_binding_table *mac_binding_table, >> > mac_binding_add(&recent_mbs, mb_data, smb, 0); >> > } >> > >> > - buffered_packets_ctx_run(&buffered_packets_ctx, &recent_mbs, >> > - sbrec_port_binding_by_key, >> > - sbrec_datapath_binding_by_key, >> > - sbrec_port_binding_by_name, >> > - sbrec_mac_binding_by_lport_ip); >> > + if (buffered_packets_lookup_run(&buffered_packets_map, &recent_mbs, >> > + sbrec_port_binding_by_key, >> > + sbrec_datapath_binding_by_key, >> > + sbrec_port_binding_by_name, >> > + sbrec_mac_binding_by_lport_ip)) { >> > + notify_pinctrl_handler(); >> > + } >> > >> > mac_bindings_clear(&recent_mbs); >> > hmap_destroy(&recent_mbs); >> > - >> > - if (buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx)) { >> > - notify_pinctrl_handler(); >> > - } >> > } >> > >> > static void >> > @@ -6438,7 +6425,7 @@ may_inject_pkts(void) >> > !cmap_is_empty(&garp_rarp_get_data()->data) || >> > ipv6_prefixd_should_inject() || >> > !ovs_list_is_empty(&mcast_query_list) || >> > - buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx) >> > || >> > + !cmap_is_empty(&buffered_packets_map) || >> > bfd_monitor_should_inject()); >> > } >> > >> > @@ -6528,9 +6515,7 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const >> > struct flow *ip_flow, >> > return; >> > } >> > >> > - ovs_mutex_lock(&pinctrl_mutex); >> > pinctrl_handle_buffered_packets(pin, continuation, false); >> > - ovs_mutex_unlock(&pinctrl_mutex); >> > >> > uint64_t packet_stub[128 / 8]; >> > struct dp_packet packet; >> > -- >> > 2.54.0 >> > >> > _______________________________________________ >> > dev mailing list >> > [email protected] >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
