| Message ID | 20230428094232.528745-5-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | Prevent race in packet buffering | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 4/28/23 11:42, Ales Musil wrote: > Move the refactored packet buffering infrastructure into > mac-learn module. This encapsulates the whole packet buffering > process behind few functions. > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > controller/mac-learn.c | 231 ++++++++++++++++++++++++++++++++++++----- > controller/mac-learn.h | 63 +++++++++-- > controller/pinctrl.c | 198 ++--------------------------------- > 3 files changed, 271 insertions(+), 221 deletions(-) > > diff --git a/controller/mac-learn.c b/controller/mac-learn.c > index ecc3c2acc..a2d3417eb 100644 > --- a/controller/mac-learn.c > +++ b/controller/mac-learn.c > @@ -18,21 +18,34 @@ > #include "mac-learn.h" > > /* OpenvSwitch lib includes. */ > +#include "dp-packet.h" Nit: It's already included by "mac-learn.h". > #include "openvswitch/poll-loop.h" > #include "openvswitch/vlog.h" > #include "lib/packets.h" > -#include "lib/random.h" > #include "lib/smap.h" > #include "lib/timeval.h" > > VLOG_DEFINE_THIS_MODULE(mac_learn); > > -#define MAX_FDB_ENTRIES 1000 > +#define MAX_FDB_ENTRIES 1000 > +#define MAX_BUFFERED_PACKETS 1000 > +#define BUFFER_QUEUE_DEPTH 4 > > +static size_t keys_ip_hash(uint32_t dp_key, uint32_t port_key, > + struct in6_addr *ip); > +static struct mac_binding *mac_binding_find(const struct mac_bindings_map > + *mac_bindings, uint32_t dp_key, > + uint32_t port_key, > + struct in6_addr *ip, size_t hash); > static size_t fdb_entry_hash(uint32_t dp_key, struct eth_addr *); > > static struct fdb_entry *fdb_entry_find(struct hmap *fdbs, uint32_t dp_key, > struct eth_addr *mac, size_t hash); > +static struct buffered_packets * > +buffered_packets_find(struct buffered_packets_ctx *ctx, uint64_t dp_key, > + uint64_t port_key, struct in6_addr *ip, uint32_t hash); > +static void ovn_buffered_packets_remove(struct buffered_packets_ctx *ctx, > + struct buffered_packets *bp); > > /* mac_binding functions. */ > void > @@ -62,7 +75,7 @@ ovn_mac_binding_add(struct mac_bindings_map *mac_bindings, uint32_t dp_key, > uint32_t hash = keys_ip_hash(dp_key, port_key, ip); > > struct mac_binding *mb = > - ovn_mac_binding_find(mac_bindings, dp_key, port_key, ip, hash); > + mac_binding_find(mac_bindings, dp_key, port_key, ip, hash); > size_t max_size = mac_bindings->max_size; > if (!mb) { > if (max_size && hmap_count(&mac_bindings->map) >= max_size) { > @@ -109,29 +122,6 @@ ovn_is_mac_binding_timestamp_past(const struct mac_binding *mb, long long now) > return now >= mb->timestamp; > } > > -size_t > -keys_ip_hash(uint32_t dp_key, uint32_t port_key, struct in6_addr *ip) > -{ > - return hash_bytes(ip, sizeof *ip, hash_2words(dp_key, port_key)); > -} > - > -struct mac_binding * > -ovn_mac_binding_find(const struct mac_bindings_map *mac_bindings, > - uint32_t dp_key, uint32_t port_key, struct in6_addr *ip, > - size_t hash) > -{ > - struct mac_binding *mb; > - > - HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &mac_bindings->map) { > - if (mb->dp_key == dp_key && mb->port_key == port_key && > - IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { > - return mb; > - } > - } > - > - return NULL; > -} > - > /* fdb functions. */ > void > ovn_fdb_init(struct hmap *fdbs) > @@ -179,6 +169,164 @@ ovn_fdb_add(struct hmap *fdbs, uint32_t dp_key, struct eth_addr mac, > > } > > +/* packet buffering functions */ > + > +struct packet_data * > +ovn_packet_data_create(struct ofpbuf ofpacts, > + const struct dp_packet *original_packet) > +{ > + struct packet_data *pd = xmalloc(sizeof *pd); > + > + pd->ofpacts = ofpacts; > + /* clone the packet to send it later with correct L2 address */ > + pd->p = dp_packet_clone_data(dp_packet_data(original_packet), > + dp_packet_size(original_packet)); > + > + return pd; > +} > + > + > +void > +ovn_packet_data_destroy(struct packet_data *pd) > +{ > + dp_packet_delete(pd->p); > + ofpbuf_uninit(&pd->ofpacts); > + free(pd); > +} > + > +struct buffered_packets * > +ovn_buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t dp_key, > + uint64_t port_key, struct in6_addr ip) > +{ > + struct buffered_packets *bp; > + > + uint32_t hash = keys_ip_hash(dp_key, port_key, &ip); > + > + bp = buffered_packets_find(ctx, dp_key, port_key, &ip, hash); > + if (!bp) { > + if (hmap_count(&ctx->buffered_packets) >= MAX_BUFFERED_PACKETS) { > + return NULL; > + } > + > + bp = xmalloc(sizeof *bp); > + hmap_insert(&ctx->buffered_packets, &bp->hmap_node, hash); > + bp->ip = ip; > + bp->dp_key = dp_key; > + bp->port_key = port_key; > + ovs_list_init(&bp->queue); > + } > + > + bp->expire = time_msec() + OVN_BUFFERED_PACKETS_TIMEOUT; > + > + return bp; > +} > + > +void > +ovn_buffered_packets_packet_data_enqueue(struct buffered_packets *bp, > + struct packet_data *pd) > +{ > + if (ovs_list_size(&bp->queue) == BUFFER_QUEUE_DEPTH) { > + struct packet_data *p = CONTAINER_OF(ovs_list_pop_front(&bp->queue), > + struct packet_data, node); > + > + ovn_packet_data_destroy(p); > + } > + ovs_list_push_back(&bp->queue, &pd->node); > +} > + > +void > +ovn_buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, > + const struct mac_bindings_map *recent_mbs) > +{ > + long long now = time_msec(); > + > + struct buffered_packets *bp; > + > + HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { > + /* Remove expired buffered packets. */ > + if (now > bp->expire) { > + ovn_buffered_packets_remove(ctx, bp); > + continue; > + } > + > + uint32_t hash = keys_ip_hash(bp->dp_key, bp->port_key, &bp->ip); > + struct mac_binding *mb = mac_binding_find(recent_mbs, bp->dp_key, > + bp->port_key, &bp->ip, hash); > + > + if (!mb) { > + continue; > + } > + > + struct packet_data *pd; > + LIST_FOR_EACH_POP (pd, node, &bp->queue) { > + struct eth_header *eth = dp_packet_data(pd->p); > + eth->eth_dst = mb->mac; > + > + ovs_list_push_back(&ctx->ready_packets_data, &pd->node); > + } > + > + ovn_buffered_packets_remove(ctx, bp); > + } > +} > + > +bool > +ovn_buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx) > +{ > + return !ovs_list_is_empty(&ctx->ready_packets_data); > +} > + > +bool > +ovn_buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx) > +{ > + return !hmap_is_empty(&ctx->buffered_packets); > +} > + > +void > +ovn_buffered_packets_ctx_init(struct buffered_packets_ctx *ctx) > +{ > + hmap_init(&ctx->buffered_packets); > + ovs_list_init(&ctx->ready_packets_data); > +} > + > +void > +ovn_buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx) > +{ > + struct packet_data *pd; > + LIST_FOR_EACH_POP (pd, node, &ctx->ready_packets_data) { > + ovn_packet_data_destroy(pd); > + } > + > + struct buffered_packets *bp; > + HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { > + ovn_buffered_packets_remove(ctx, bp); > + } > + hmap_destroy(&ctx->buffered_packets); > +} > + > +/* mac_binding related static functions. */ > +static size_t > +keys_ip_hash(uint32_t dp_key, uint32_t port_key, struct in6_addr *ip) > +{ > + return hash_bytes(ip, sizeof *ip, hash_2words(dp_key, port_key)); > +} > + > +static struct mac_binding * > +mac_binding_find(const struct mac_bindings_map *mac_bindings, > + uint32_t dp_key, uint32_t port_key, struct in6_addr *ip, > + size_t hash) > +{ > + struct mac_binding *mb; > + > + HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &mac_bindings->map) { > + if (mb->dp_key == dp_key && mb->port_key == port_key && > + IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { > + return mb; > + } > + } > + > + return NULL; > +} > + > /* fdb related static functions. */ > > static size_t > @@ -201,3 +349,34 @@ fdb_entry_find(struct hmap *fdbs, uint32_t dp_key, > > return NULL; > } > + > +/* packet buffering static functions. */ > +static struct buffered_packets * > +buffered_packets_find(struct buffered_packets_ctx *ctx, uint64_t dp_key, > + uint64_t port_key, struct in6_addr *ip, uint32_t hash) > +{ > + struct buffered_packets *mb; > + > + HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &ctx->buffered_packets) { > + if (mb->dp_key == dp_key && mb->port_key == port_key && > + IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { > + return mb; > + } > + } > + > + return NULL; > +} > + > +static void > +ovn_buffered_packets_remove(struct buffered_packets_ctx *ctx, > + struct buffered_packets *bp) > +{ > + struct packet_data *pd; > + > + LIST_FOR_EACH_POP (pd, node, &bp->queue) { > + ovn_packet_data_destroy(pd); > + } > + > + hmap_remove(&ctx->buffered_packets, &bp->hmap_node); > + free(bp); > +} > diff --git a/controller/mac-learn.h b/controller/mac-learn.h > index 55375cf90..3de57bf4c 100644 > --- a/controller/mac-learn.h > +++ b/controller/mac-learn.h > @@ -19,7 +19,11 @@ > > #include <sys/types.h> > #include <netinet/in.h> > + > +#include "dp-packet.h" > #include "openvswitch/hmap.h" > +#include "openvswitch/list.h" > +#include "openvswitch/ofpbuf.h" > > struct mac_binding { > struct hmap_node hmap_node; /* In a hmap. */ > @@ -57,11 +61,6 @@ struct mac_binding *ovn_mac_binding_add(struct mac_bindings_map *mac_bindings, > struct in6_addr *ip, > struct eth_addr mac, > uint32_t timestamp_offset); > -size_t keys_ip_hash(uint32_t dp_key, uint32_t port_key, struct in6_addr *ip); > -struct mac_binding * > -ovn_mac_binding_find(const struct mac_bindings_map *mac_bindings, > - uint32_t dp_key, uint32_t port_key, struct in6_addr *ip, > - size_t hash); > > > struct fdb_entry { > @@ -80,7 +79,57 @@ void ovn_fdbs_flush(struct hmap *fdbs); > void ovn_fdbs_destroy(struct hmap *fdbs); > > struct fdb_entry *ovn_fdb_add(struct hmap *fdbs, > - uint32_t dp_key, struct eth_addr mac, > - uint32_t port_key); > + uint32_t dp_key, struct eth_addr mac, > + uint32_t port_key); > + > +#define OVN_BUFFERED_PACKETS_TIMEOUT 10000 > + > +struct packet_data { > + struct ovs_list node; > + > + struct ofpbuf ofpacts; > + struct dp_packet *p; > +}; > + > +struct buffered_packets { > + struct hmap_node hmap_node; > + > + struct in6_addr ip; > + uint64_t dp_key; > + uint64_t port_key; > + > + /* Queue of packet_data associated with this struct. */ > + struct ovs_list queue; > + > + /* Timestamp in ms when the buffered packet should expire. */ > + long long int expire; > +}; > + > +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 ovs_list ready_packets_data; > +}; > + > +#define OVN_BUFFERED_PACKETS_TIMEOUT 10000 Re-definition. > + > +struct packet_data * > +ovn_packet_data_create(struct ofpbuf ofpacts, > + const struct dp_packet *original_packet); > +void ovn_packet_data_destroy(struct packet_data *pd); > +struct buffered_packets * > +ovn_buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t dp_key, > + uint64_t port_key, struct in6_addr ip); > +void ovn_buffered_packets_packet_data_enqueue(struct buffered_packets *bp, > + struct packet_data *pd); > +void > +ovn_buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, > + const struct mac_bindings_map *recent_mbs); > +void ovn_buffered_packets_ctx_init(struct buffered_packets_ctx *ctx); > +void ovn_buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx); > +bool > +ovn_buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx); > +bool ovn_buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx); > > #endif /* OVN_MAC_LEARN_H */ > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 9b5147753..b29033703 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -1416,196 +1416,18 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn, > } > } > > -#define BUFFERED_PACKETS_TIMEOUT 10000 > -#define MAX_BUFFERED_PACKETS 1000 > -#define BUFFER_QUEUE_DEPTH 4 > - > -struct packet_data { > - struct ovs_list node; > - > - struct ofpbuf ofpacts; > - struct dp_packet *p; > -}; > - > -struct buffered_packets { > - struct hmap_node hmap_node; > - > - struct in6_addr ip; > - uint64_t dp_key; > - uint64_t port_key; > - > - /* Queue of packet_data associated with this struct. */ > - struct ovs_list queue; > - > - /* Timestamp in ms when the buffered packet should expire. */ > - long long int expire; > -}; > - > -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 ovs_list ready_packets_data; > -}; > - > static struct buffered_packets_ctx buffered_packets_ctx; > > -static struct buffered_packets * > -buffered_packets_find(struct buffered_packets_ctx *ctx, uint64_t dp_key, > - uint64_t port_key, struct in6_addr *ip, uint32_t hash) > -{ > - struct buffered_packets *mb; > - HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &ctx->buffered_packets) { > - if (mb->dp_key == dp_key && mb->port_key == port_key && > - IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { > - return mb; > - } > - } > - > - return NULL; > -} > - > -static struct buffered_packets * > -buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t dp_key, > - uint64_t port_key, struct in6_addr ip) > -{ > - struct buffered_packets *bp; > - > - uint32_t hash = keys_ip_hash(dp_key, port_key, &ip); > - bp = buffered_packets_find(ctx, dp_key, port_key, &ip, hash); > - if (!bp) { > - if (hmap_count(&ctx->buffered_packets) >= MAX_BUFFERED_PACKETS) { > - return NULL; > - } > - > - bp = xmalloc(sizeof *bp); > - hmap_insert(&ctx->buffered_packets, &bp->hmap_node, hash); > - bp->ip = ip; > - bp->dp_key = dp_key; > - bp->port_key = port_key; > - ovs_list_init(&bp->queue); > - } > - > - bp->expire = time_msec() + BUFFERED_PACKETS_TIMEOUT; > - > - return bp; > -} > - > -static struct packet_data * > -packet_data_create(struct ofpbuf ofpacts, > - const struct dp_packet *original_packet) > -{ > - struct packet_data *pd = xmalloc(sizeof *pd); > - > - pd->ofpacts = ofpacts; > - /* clone the packet to send it later with correct L2 address */ > - pd->p = dp_packet_clone_data(dp_packet_data(original_packet), > - dp_packet_size(original_packet)); > - > - return pd; > -} > - > -static void > -packet_data_destroy(struct packet_data *pd) > -{ > - dp_packet_delete(pd->p); > - ofpbuf_uninit(&pd->ofpacts); > - free(pd); > -} > - > -static void > -buffered_packets_packet_data_enqueue(struct buffered_packets *bp, > - struct packet_data *pd) > -{ > - if (ovs_list_size(&bp->queue) == BUFFER_QUEUE_DEPTH) { > - struct packet_data *p = CONTAINER_OF(ovs_list_pop_front(&bp->queue), > - struct packet_data, node); > - packet_data_destroy(p); > - } > - ovs_list_push_back(&bp->queue, &pd->node); > -} > - > - > -static void > -buffered_packets_remove(struct buffered_packets_ctx *ctx, > - struct buffered_packets *bp) > -{ > - struct packet_data *pd; > - LIST_FOR_EACH_POP (pd, node, &bp->queue) { > - packet_data_destroy(pd); > - } > - > - hmap_remove(&ctx->buffered_packets, &bp->hmap_node); > - free(bp); > -} > - > -static void > -buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, > - const struct mac_bindings_map *recent_mac_bindings) > -{ > - long long now = time_msec(); > - > - struct buffered_packets *bp; > - HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { > - /* Remove expired buffered packets. */ > - if (now > bp->expire) { > - buffered_packets_remove(ctx, bp); > - continue; > - } > - > - uint32_t hash = keys_ip_hash(bp->dp_key, bp->port_key, &bp->ip); > - struct mac_binding *mb = ovn_mac_binding_find(recent_mac_bindings, > - bp->dp_key, bp->port_key, > - &bp->ip, hash); > - if (!mb) { > - continue; > - } > - > - struct packet_data *pd; > - LIST_FOR_EACH_POP (pd, node, &bp->queue) { > - struct eth_header *eth = dp_packet_data(pd->p); > - eth->eth_dst = mb->mac; > - > - ovs_list_push_back(&ctx->ready_packets_data, &pd->node); > - } > - > - buffered_packets_remove(ctx, bp); > - } > -} > - > -static bool > -buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx) > -{ > - return !ovs_list_is_empty(&ctx->ready_packets_data); > -} > - > -static bool > -ovn_buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx) > -{ > - return !hmap_is_empty(&ctx->buffered_packets); > -} > - > static void > init_buffered_packets_ctx(void) > { > - hmap_init(&buffered_packets_ctx.buffered_packets); > - ovs_list_init(&buffered_packets_ctx.ready_packets_data); > + ovn_buffered_packets_ctx_init(&buffered_packets_ctx); > } > > static void > destroy_buffered_packets_ctx(void) > { > - struct packet_data *pd; > - LIST_FOR_EACH_POP (pd, node, &buffered_packets_ctx.ready_packets_data) { > - packet_data_destroy(pd); > - } > - > - struct buffered_packets *bp; > - HMAP_FOR_EACH_SAFE (bp, hmap_node, > - &buffered_packets_ctx.buffered_packets) { > - buffered_packets_remove(&buffered_packets_ctx, bp); > - } > - hmap_destroy(&buffered_packets_ctx.buffered_packets); > + ovn_buffered_packets_ctx_destroy(&buffered_packets_ctx); > } > > /* Called with in the pinctrl_handler thread context. */ > @@ -1625,8 +1447,8 @@ OVS_REQUIRES(pinctrl_mutex) > memcpy(&ip, &ip6, sizeof ip); > } > > - struct buffered_packets *bp = buffered_packets_add(&buffered_packets_ctx, > - dp_key, oport_key, ip); > + struct buffered_packets *bp = > + ovn_buffered_packets_add(&buffered_packets_ctx, dp_key, oport_key, ip); > if (!bp) { > COVERAGE_INC(pinctrl_drop_buffered_packets_map); > return; > @@ -1645,8 +1467,8 @@ OVS_REQUIRES(pinctrl_mutex) > resubmit->in_port = OFPP_CONTROLLER; > resubmit->table_id = OFTABLE_REMOTE_OUTPUT; > > - struct packet_data *pd = packet_data_create(ofpacts, pkt_in); > - buffered_packets_packet_data_enqueue(bp, pd); > + struct packet_data *pd = ovn_packet_data_create(ofpacts, pkt_in); > + ovn_buffered_packets_packet_data_enqueue(bp, pd); > > /* There is a chance that the MAC binding was already created. */ > notify_pinctrl_main(); > @@ -4274,7 +4096,7 @@ send_mac_binding_buffered_pkts(struct rconn *swconn) > match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER); > queue_msg(swconn, ofputil_encode_packet_out(&po, proto)); > > - packet_data_destroy(pd); > + ovn_packet_data_destroy(pd); > } > > ovs_list_init(&buffered_packets_ctx.ready_packets_data); > @@ -4487,11 +4309,11 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > pb->tunnel_key, &ip, mac, 0); > } > > - buffered_packets_ctx_run(&buffered_packets_ctx, &recent_mbs); > + ovn_buffered_packets_ctx_run(&buffered_packets_ctx, &recent_mbs); > > ovn_mac_bindings_map_destroy(&recent_mbs); > > - if (buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx)) { > + if (ovn_buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx)) { > notify_pinctrl_handler(); > } > } > @@ -6095,7 +5917,7 @@ may_inject_pkts(void) > !shash_is_empty(&send_garp_rarp_data) || > ipv6_prefixd_should_inject() || > !ovs_list_is_empty(&mcast_query_list) || > - buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx) || > + ovn_buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx) || > bfd_monitor_should_inject()); > } >
diff --git a/controller/mac-learn.c b/controller/mac-learn.c index ecc3c2acc..a2d3417eb 100644 --- a/controller/mac-learn.c +++ b/controller/mac-learn.c @@ -18,21 +18,34 @@ #include "mac-learn.h" /* OpenvSwitch lib includes. */ +#include "dp-packet.h" #include "openvswitch/poll-loop.h" #include "openvswitch/vlog.h" #include "lib/packets.h" -#include "lib/random.h" #include "lib/smap.h" #include "lib/timeval.h" VLOG_DEFINE_THIS_MODULE(mac_learn); -#define MAX_FDB_ENTRIES 1000 +#define MAX_FDB_ENTRIES 1000 +#define MAX_BUFFERED_PACKETS 1000 +#define BUFFER_QUEUE_DEPTH 4 +static size_t keys_ip_hash(uint32_t dp_key, uint32_t port_key, + struct in6_addr *ip); +static struct mac_binding *mac_binding_find(const struct mac_bindings_map + *mac_bindings, uint32_t dp_key, + uint32_t port_key, + struct in6_addr *ip, size_t hash); static size_t fdb_entry_hash(uint32_t dp_key, struct eth_addr *); static struct fdb_entry *fdb_entry_find(struct hmap *fdbs, uint32_t dp_key, struct eth_addr *mac, size_t hash); +static struct buffered_packets * +buffered_packets_find(struct buffered_packets_ctx *ctx, uint64_t dp_key, + uint64_t port_key, struct in6_addr *ip, uint32_t hash); +static void ovn_buffered_packets_remove(struct buffered_packets_ctx *ctx, + struct buffered_packets *bp); /* mac_binding functions. */ void @@ -62,7 +75,7 @@ ovn_mac_binding_add(struct mac_bindings_map *mac_bindings, uint32_t dp_key, uint32_t hash = keys_ip_hash(dp_key, port_key, ip); struct mac_binding *mb = - ovn_mac_binding_find(mac_bindings, dp_key, port_key, ip, hash); + mac_binding_find(mac_bindings, dp_key, port_key, ip, hash); size_t max_size = mac_bindings->max_size; if (!mb) { if (max_size && hmap_count(&mac_bindings->map) >= max_size) { @@ -109,29 +122,6 @@ ovn_is_mac_binding_timestamp_past(const struct mac_binding *mb, long long now) return now >= mb->timestamp; } -size_t -keys_ip_hash(uint32_t dp_key, uint32_t port_key, struct in6_addr *ip) -{ - return hash_bytes(ip, sizeof *ip, hash_2words(dp_key, port_key)); -} - -struct mac_binding * -ovn_mac_binding_find(const struct mac_bindings_map *mac_bindings, - uint32_t dp_key, uint32_t port_key, struct in6_addr *ip, - size_t hash) -{ - struct mac_binding *mb; - - HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &mac_bindings->map) { - if (mb->dp_key == dp_key && mb->port_key == port_key && - IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { - return mb; - } - } - - return NULL; -} - /* fdb functions. */ void ovn_fdb_init(struct hmap *fdbs) @@ -179,6 +169,164 @@ ovn_fdb_add(struct hmap *fdbs, uint32_t dp_key, struct eth_addr mac, } +/* packet buffering functions */ + +struct packet_data * +ovn_packet_data_create(struct ofpbuf ofpacts, + const struct dp_packet *original_packet) +{ + struct packet_data *pd = xmalloc(sizeof *pd); + + pd->ofpacts = ofpacts; + /* clone the packet to send it later with correct L2 address */ + pd->p = dp_packet_clone_data(dp_packet_data(original_packet), + dp_packet_size(original_packet)); + + return pd; +} + + +void +ovn_packet_data_destroy(struct packet_data *pd) +{ + dp_packet_delete(pd->p); + ofpbuf_uninit(&pd->ofpacts); + free(pd); +} + +struct buffered_packets * +ovn_buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t dp_key, + uint64_t port_key, struct in6_addr ip) +{ + struct buffered_packets *bp; + + uint32_t hash = keys_ip_hash(dp_key, port_key, &ip); + + bp = buffered_packets_find(ctx, dp_key, port_key, &ip, hash); + if (!bp) { + if (hmap_count(&ctx->buffered_packets) >= MAX_BUFFERED_PACKETS) { + return NULL; + } + + bp = xmalloc(sizeof *bp); + hmap_insert(&ctx->buffered_packets, &bp->hmap_node, hash); + bp->ip = ip; + bp->dp_key = dp_key; + bp->port_key = port_key; + ovs_list_init(&bp->queue); + } + + bp->expire = time_msec() + OVN_BUFFERED_PACKETS_TIMEOUT; + + return bp; +} + +void +ovn_buffered_packets_packet_data_enqueue(struct buffered_packets *bp, + struct packet_data *pd) +{ + if (ovs_list_size(&bp->queue) == BUFFER_QUEUE_DEPTH) { + struct packet_data *p = CONTAINER_OF(ovs_list_pop_front(&bp->queue), + struct packet_data, node); + + ovn_packet_data_destroy(p); + } + ovs_list_push_back(&bp->queue, &pd->node); +} + +void +ovn_buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, + const struct mac_bindings_map *recent_mbs) +{ + long long now = time_msec(); + + struct buffered_packets *bp; + + HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { + /* Remove expired buffered packets. */ + if (now > bp->expire) { + ovn_buffered_packets_remove(ctx, bp); + continue; + } + + uint32_t hash = keys_ip_hash(bp->dp_key, bp->port_key, &bp->ip); + struct mac_binding *mb = mac_binding_find(recent_mbs, bp->dp_key, + bp->port_key, &bp->ip, hash); + + if (!mb) { + continue; + } + + struct packet_data *pd; + LIST_FOR_EACH_POP (pd, node, &bp->queue) { + struct eth_header *eth = dp_packet_data(pd->p); + eth->eth_dst = mb->mac; + + ovs_list_push_back(&ctx->ready_packets_data, &pd->node); + } + + ovn_buffered_packets_remove(ctx, bp); + } +} + +bool +ovn_buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx) +{ + return !ovs_list_is_empty(&ctx->ready_packets_data); +} + +bool +ovn_buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx) +{ + return !hmap_is_empty(&ctx->buffered_packets); +} + +void +ovn_buffered_packets_ctx_init(struct buffered_packets_ctx *ctx) +{ + hmap_init(&ctx->buffered_packets); + ovs_list_init(&ctx->ready_packets_data); +} + +void +ovn_buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx) +{ + struct packet_data *pd; + LIST_FOR_EACH_POP (pd, node, &ctx->ready_packets_data) { + ovn_packet_data_destroy(pd); + } + + struct buffered_packets *bp; + HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { + ovn_buffered_packets_remove(ctx, bp); + } + hmap_destroy(&ctx->buffered_packets); +} + +/* mac_binding related static functions. */ +static size_t +keys_ip_hash(uint32_t dp_key, uint32_t port_key, struct in6_addr *ip) +{ + return hash_bytes(ip, sizeof *ip, hash_2words(dp_key, port_key)); +} + +static struct mac_binding * +mac_binding_find(const struct mac_bindings_map *mac_bindings, + uint32_t dp_key, uint32_t port_key, struct in6_addr *ip, + size_t hash) +{ + struct mac_binding *mb; + + HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &mac_bindings->map) { + if (mb->dp_key == dp_key && mb->port_key == port_key && + IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { + return mb; + } + } + + return NULL; +} + /* fdb related static functions. */ static size_t @@ -201,3 +349,34 @@ fdb_entry_find(struct hmap *fdbs, uint32_t dp_key, return NULL; } + +/* packet buffering static functions. */ +static struct buffered_packets * +buffered_packets_find(struct buffered_packets_ctx *ctx, uint64_t dp_key, + uint64_t port_key, struct in6_addr *ip, uint32_t hash) +{ + struct buffered_packets *mb; + + HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &ctx->buffered_packets) { + if (mb->dp_key == dp_key && mb->port_key == port_key && + IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { + return mb; + } + } + + return NULL; +} + +static void +ovn_buffered_packets_remove(struct buffered_packets_ctx *ctx, + struct buffered_packets *bp) +{ + struct packet_data *pd; + + LIST_FOR_EACH_POP (pd, node, &bp->queue) { + ovn_packet_data_destroy(pd); + } + + hmap_remove(&ctx->buffered_packets, &bp->hmap_node); + free(bp); +} diff --git a/controller/mac-learn.h b/controller/mac-learn.h index 55375cf90..3de57bf4c 100644 --- a/controller/mac-learn.h +++ b/controller/mac-learn.h @@ -19,7 +19,11 @@ #include <sys/types.h> #include <netinet/in.h> + +#include "dp-packet.h" #include "openvswitch/hmap.h" +#include "openvswitch/list.h" +#include "openvswitch/ofpbuf.h" struct mac_binding { struct hmap_node hmap_node; /* In a hmap. */ @@ -57,11 +61,6 @@ struct mac_binding *ovn_mac_binding_add(struct mac_bindings_map *mac_bindings, struct in6_addr *ip, struct eth_addr mac, uint32_t timestamp_offset); -size_t keys_ip_hash(uint32_t dp_key, uint32_t port_key, struct in6_addr *ip); -struct mac_binding * -ovn_mac_binding_find(const struct mac_bindings_map *mac_bindings, - uint32_t dp_key, uint32_t port_key, struct in6_addr *ip, - size_t hash); struct fdb_entry { @@ -80,7 +79,57 @@ void ovn_fdbs_flush(struct hmap *fdbs); void ovn_fdbs_destroy(struct hmap *fdbs); struct fdb_entry *ovn_fdb_add(struct hmap *fdbs, - uint32_t dp_key, struct eth_addr mac, - uint32_t port_key); + uint32_t dp_key, struct eth_addr mac, + uint32_t port_key); + +#define OVN_BUFFERED_PACKETS_TIMEOUT 10000 + +struct packet_data { + struct ovs_list node; + + struct ofpbuf ofpacts; + struct dp_packet *p; +}; + +struct buffered_packets { + struct hmap_node hmap_node; + + struct in6_addr ip; + uint64_t dp_key; + uint64_t port_key; + + /* Queue of packet_data associated with this struct. */ + struct ovs_list queue; + + /* Timestamp in ms when the buffered packet should expire. */ + long long int expire; +}; + +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 ovs_list ready_packets_data; +}; + +#define OVN_BUFFERED_PACKETS_TIMEOUT 10000 + +struct packet_data * +ovn_packet_data_create(struct ofpbuf ofpacts, + const struct dp_packet *original_packet); +void ovn_packet_data_destroy(struct packet_data *pd); +struct buffered_packets * +ovn_buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t dp_key, + uint64_t port_key, struct in6_addr ip); +void ovn_buffered_packets_packet_data_enqueue(struct buffered_packets *bp, + struct packet_data *pd); +void +ovn_buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, + const struct mac_bindings_map *recent_mbs); +void ovn_buffered_packets_ctx_init(struct buffered_packets_ctx *ctx); +void ovn_buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx); +bool +ovn_buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx); +bool ovn_buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx); #endif /* OVN_MAC_LEARN_H */ diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 9b5147753..b29033703 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -1416,196 +1416,18 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn, } } -#define BUFFERED_PACKETS_TIMEOUT 10000 -#define MAX_BUFFERED_PACKETS 1000 -#define BUFFER_QUEUE_DEPTH 4 - -struct packet_data { - struct ovs_list node; - - struct ofpbuf ofpacts; - struct dp_packet *p; -}; - -struct buffered_packets { - struct hmap_node hmap_node; - - struct in6_addr ip; - uint64_t dp_key; - uint64_t port_key; - - /* Queue of packet_data associated with this struct. */ - struct ovs_list queue; - - /* Timestamp in ms when the buffered packet should expire. */ - long long int expire; -}; - -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 ovs_list ready_packets_data; -}; - static struct buffered_packets_ctx buffered_packets_ctx; -static struct buffered_packets * -buffered_packets_find(struct buffered_packets_ctx *ctx, uint64_t dp_key, - uint64_t port_key, struct in6_addr *ip, uint32_t hash) -{ - struct buffered_packets *mb; - HMAP_FOR_EACH_WITH_HASH (mb, hmap_node, hash, &ctx->buffered_packets) { - if (mb->dp_key == dp_key && mb->port_key == port_key && - IN6_ARE_ADDR_EQUAL(&mb->ip, ip)) { - return mb; - } - } - - return NULL; -} - -static struct buffered_packets * -buffered_packets_add(struct buffered_packets_ctx *ctx, uint64_t dp_key, - uint64_t port_key, struct in6_addr ip) -{ - struct buffered_packets *bp; - - uint32_t hash = keys_ip_hash(dp_key, port_key, &ip); - bp = buffered_packets_find(ctx, dp_key, port_key, &ip, hash); - if (!bp) { - if (hmap_count(&ctx->buffered_packets) >= MAX_BUFFERED_PACKETS) { - return NULL; - } - - bp = xmalloc(sizeof *bp); - hmap_insert(&ctx->buffered_packets, &bp->hmap_node, hash); - bp->ip = ip; - bp->dp_key = dp_key; - bp->port_key = port_key; - ovs_list_init(&bp->queue); - } - - bp->expire = time_msec() + BUFFERED_PACKETS_TIMEOUT; - - return bp; -} - -static struct packet_data * -packet_data_create(struct ofpbuf ofpacts, - const struct dp_packet *original_packet) -{ - struct packet_data *pd = xmalloc(sizeof *pd); - - pd->ofpacts = ofpacts; - /* clone the packet to send it later with correct L2 address */ - pd->p = dp_packet_clone_data(dp_packet_data(original_packet), - dp_packet_size(original_packet)); - - return pd; -} - -static void -packet_data_destroy(struct packet_data *pd) -{ - dp_packet_delete(pd->p); - ofpbuf_uninit(&pd->ofpacts); - free(pd); -} - -static void -buffered_packets_packet_data_enqueue(struct buffered_packets *bp, - struct packet_data *pd) -{ - if (ovs_list_size(&bp->queue) == BUFFER_QUEUE_DEPTH) { - struct packet_data *p = CONTAINER_OF(ovs_list_pop_front(&bp->queue), - struct packet_data, node); - packet_data_destroy(p); - } - ovs_list_push_back(&bp->queue, &pd->node); -} - - -static void -buffered_packets_remove(struct buffered_packets_ctx *ctx, - struct buffered_packets *bp) -{ - struct packet_data *pd; - LIST_FOR_EACH_POP (pd, node, &bp->queue) { - packet_data_destroy(pd); - } - - hmap_remove(&ctx->buffered_packets, &bp->hmap_node); - free(bp); -} - -static void -buffered_packets_ctx_run(struct buffered_packets_ctx *ctx, - const struct mac_bindings_map *recent_mac_bindings) -{ - long long now = time_msec(); - - struct buffered_packets *bp; - HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) { - /* Remove expired buffered packets. */ - if (now > bp->expire) { - buffered_packets_remove(ctx, bp); - continue; - } - - uint32_t hash = keys_ip_hash(bp->dp_key, bp->port_key, &bp->ip); - struct mac_binding *mb = ovn_mac_binding_find(recent_mac_bindings, - bp->dp_key, bp->port_key, - &bp->ip, hash); - if (!mb) { - continue; - } - - struct packet_data *pd; - LIST_FOR_EACH_POP (pd, node, &bp->queue) { - struct eth_header *eth = dp_packet_data(pd->p); - eth->eth_dst = mb->mac; - - ovs_list_push_back(&ctx->ready_packets_data, &pd->node); - } - - buffered_packets_remove(ctx, bp); - } -} - -static bool -buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx) -{ - return !ovs_list_is_empty(&ctx->ready_packets_data); -} - -static bool -ovn_buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx) -{ - return !hmap_is_empty(&ctx->buffered_packets); -} - static void init_buffered_packets_ctx(void) { - hmap_init(&buffered_packets_ctx.buffered_packets); - ovs_list_init(&buffered_packets_ctx.ready_packets_data); + ovn_buffered_packets_ctx_init(&buffered_packets_ctx); } static void destroy_buffered_packets_ctx(void) { - struct packet_data *pd; - LIST_FOR_EACH_POP (pd, node, &buffered_packets_ctx.ready_packets_data) { - packet_data_destroy(pd); - } - - struct buffered_packets *bp; - HMAP_FOR_EACH_SAFE (bp, hmap_node, - &buffered_packets_ctx.buffered_packets) { - buffered_packets_remove(&buffered_packets_ctx, bp); - } - hmap_destroy(&buffered_packets_ctx.buffered_packets); + ovn_buffered_packets_ctx_destroy(&buffered_packets_ctx); } /* Called with in the pinctrl_handler thread context. */ @@ -1625,8 +1447,8 @@ OVS_REQUIRES(pinctrl_mutex) memcpy(&ip, &ip6, sizeof ip); } - struct buffered_packets *bp = buffered_packets_add(&buffered_packets_ctx, - dp_key, oport_key, ip); + struct buffered_packets *bp = + ovn_buffered_packets_add(&buffered_packets_ctx, dp_key, oport_key, ip); if (!bp) { COVERAGE_INC(pinctrl_drop_buffered_packets_map); return; @@ -1645,8 +1467,8 @@ OVS_REQUIRES(pinctrl_mutex) resubmit->in_port = OFPP_CONTROLLER; resubmit->table_id = OFTABLE_REMOTE_OUTPUT; - struct packet_data *pd = packet_data_create(ofpacts, pkt_in); - buffered_packets_packet_data_enqueue(bp, pd); + struct packet_data *pd = ovn_packet_data_create(ofpacts, pkt_in); + ovn_buffered_packets_packet_data_enqueue(bp, pd); /* There is a chance that the MAC binding was already created. */ notify_pinctrl_main(); @@ -4274,7 +4096,7 @@ send_mac_binding_buffered_pkts(struct rconn *swconn) match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER); queue_msg(swconn, ofputil_encode_packet_out(&po, proto)); - packet_data_destroy(pd); + ovn_packet_data_destroy(pd); } ovs_list_init(&buffered_packets_ctx.ready_packets_data); @@ -4487,11 +4309,11 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, pb->tunnel_key, &ip, mac, 0); } - buffered_packets_ctx_run(&buffered_packets_ctx, &recent_mbs); + ovn_buffered_packets_ctx_run(&buffered_packets_ctx, &recent_mbs); ovn_mac_bindings_map_destroy(&recent_mbs); - if (buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx)) { + if (ovn_buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx)) { notify_pinctrl_handler(); } } @@ -6095,7 +5917,7 @@ may_inject_pkts(void) !shash_is_empty(&send_garp_rarp_data) || ipv6_prefixd_should_inject() || !ovs_list_is_empty(&mcast_query_list) || - buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx) || + ovn_buffered_packets_ctx_is_ready_to_send(&buffered_packets_ctx) || bfd_monitor_should_inject()); }
Move the refactored packet buffering infrastructure into mac-learn module. This encapsulates the whole packet buffering process behind few functions. Signed-off-by: Ales Musil <amusil@redhat.com> --- controller/mac-learn.c | 231 ++++++++++++++++++++++++++++++++++++----- controller/mac-learn.h | 63 +++++++++-- controller/pinctrl.c | 198 ++--------------------------------- 3 files changed, 271 insertions(+), 221 deletions(-)