On Sat, Sep 27, 2025 at 12:20 AM Dumitru Ceara <[email protected]> wrote:
> An upcoming patch will take advantage of this routine for creating > OpenFlow rules for ARP/ND entries that were learned locally through > EVPN. > > Signed-off-by: Dumitru Ceara <[email protected]> > --- > Hi Dumitru, thank you for the patch. I have two small nits below. > controller/automake.mk | 2 + > controller/lflow.c | 154 +++++++++++---------------------------- > controller/neighbor-of.c | 111 ++++++++++++++++++++++++++++ > controller/neighbor-of.h | 45 ++++++++++++ > 4 files changed, 200 insertions(+), 112 deletions(-) > create mode 100644 controller/neighbor-of.c > create mode 100644 controller/neighbor-of.h > > diff --git a/controller/automake.mk b/controller/automake.mk > index d53d932c12..b3c6293fa9 100644 > --- a/controller/automake.mk > +++ b/controller/automake.mk > @@ -36,6 +36,8 @@ controller_ovn_controller_SOURCES = \ > controller/ofctrl-seqno.h \ > controller/neighbor.c \ > controller/neighbor.h \ > + controller/neighbor-of.c \ > + controller/neighbor-of.h \ > controller/pinctrl.c \ > controller/pinctrl.h \ > controller/patch.c \ > diff --git a/controller/lflow.c b/controller/lflow.c > index 197adaae95..e84fb2486c 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -35,6 +35,7 @@ > #include "lib/ovn-sb-idl.h" > #include "lib/extend-table.h" > #include "lib/uuidset.h" > +#include "neighbor-of.h" > #include "packets.h" > #include "physical.h" > #include "simap.h" > @@ -1288,12 +1289,12 @@ consider_logical_flow(const struct > sbrec_logical_flow *lflow, > } > > static void > -consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name, > - const struct hmap *local_datapaths, > - const struct sbrec_mac_binding *b, > - const struct sbrec_static_mac_binding *smb, > - struct ovn_desired_flow_table *flow_table, > - uint16_t priority) > +consider_neighbor_flow__(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > + const struct hmap *local_datapaths, > + const struct sbrec_mac_binding *b, > + const struct sbrec_static_mac_binding *smb, > + struct ovn_desired_flow_table *flow_table, > + enum neigh_of_rule_prio priority) > { > if (!b && !smb) { > return; > @@ -1302,6 +1303,7 @@ consider_neighbor_flow(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > char *logical_port = b ? b->logical_port : smb->logical_port; > char *ip = b ? b->ip : smb->ip; > char *mac = b ? b->mac : smb->mac; > + bool needs_usage_tracking = !!b; > > const struct sbrec_port_binding *pb > = lport_lookup_by_name(sbrec_port_binding_by_name, logical_port); > @@ -1317,97 +1319,16 @@ consider_neighbor_flow(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > return; > } > > - struct match get_arp_match = MATCH_CATCHALL_INITIALIZER; > - struct match lookup_arp_match = MATCH_CATCHALL_INITIALIZER; > - struct match mb_cache_use_match = MATCH_CATCHALL_INITIALIZER; > - struct match lookup_arp_for_stats_match = MATCH_CATCHALL_INITIALIZER; > - > - match_set_dl_src(&lookup_arp_match, mac_addr); > - match_set_metadata(&lookup_arp_match, > htonll(pb->datapath->tunnel_key)); > - match_set_reg(&lookup_arp_match, MFF_LOG_INPORT - MFF_REG0, > - pb->tunnel_key); > - > - if (strchr(ip, '.')) { > - ovs_be32 ip_addr; > - if (!ip_parse(ip, &ip_addr)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad 'ip' %s", ip); > - return; > - } > - > - match_set_reg(&get_arp_match, 0, ntohl(ip_addr)); > - > - match_set_dl_type(&lookup_arp_match, htons(ETH_TYPE_ARP)); > - lookup_arp_for_stats_match = lookup_arp_match; > - > - match_set_reg(&lookup_arp_match, 0, ntohl(ip_addr)); > - > - match_set_dl_type(&mb_cache_use_match, htons(ETH_TYPE_IP)); > - match_set_nw_src(&mb_cache_use_match, ip_addr); > - > - match_set_arp_opcode_masked(&lookup_arp_for_stats_match, 2, 0xff); > - match_set_arp_spa_masked(&lookup_arp_for_stats_match, ip_addr, > - htonl(0xffffffff)); > - } else { > - struct in6_addr ip6; > - if (!ipv6_parse(ip, &ip6)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad 'ip' %s", ip); > - return; > - } > - ovs_be128 value; > - memcpy(&value, &ip6, sizeof(value)); > - match_set_xxreg(&get_arp_match, 1, ntoh128(value)); > - > - match_set_dl_type(&lookup_arp_match, htons(ETH_TYPE_IPV6)); > - match_set_nw_proto(&lookup_arp_match, 58); > - match_set_icmp_code(&lookup_arp_match, 0); > - > - match_set_xxreg(&lookup_arp_match, 0, ntoh128(value)); > - > - match_set_dl_type(&mb_cache_use_match, htons(ETH_TYPE_IPV6)); > - match_set_ipv6_src(&mb_cache_use_match, &ip6); > - } > - > - match_set_metadata(&get_arp_match, htonll(pb->datapath->tunnel_key)); > - match_set_reg(&get_arp_match, MFF_LOG_OUTPORT - MFF_REG0, > pb->tunnel_key); > - > - match_set_dl_src(&mb_cache_use_match, mac_addr); > - match_set_reg(&mb_cache_use_match, MFF_LOG_INPORT - MFF_REG0, > - pb->tunnel_key); > - match_set_metadata(&mb_cache_use_match, > htonll(pb->datapath->tunnel_key)); > - > - uint64_t stub[1024 / 8]; > - struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub); > - put_load_bytes(mac_addr.ea, sizeof mac_addr.ea, MFF_ETH_DST, 0, 48, > - &ofpacts); > - put_load(1, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1, &ofpacts); > - ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, priority, > - b ? b->header_.uuid.parts[0] : > smb->header_.uuid.parts[0], > - &get_arp_match, &ofpacts, > - b ? &b->header_.uuid : &smb->header_.uuid); > - > - ofpbuf_clear(&ofpacts); > - put_load(1, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1, &ofpacts); > - ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, priority, > - b ? b->header_.uuid.parts[0] : > smb->header_.uuid.parts[0], > - &lookup_arp_match, &ofpacts, > - b ? &b->header_.uuid : &smb->header_.uuid); > - > - if (b) { > - ofpbuf_clear(&ofpacts); > - if (strchr(ip, '.')) { > - ofctrl_add_flow(flow_table, OFTABLE_MAC_CACHE_USE, priority, > - b->header_.uuid.parts[0], > - &lookup_arp_for_stats_match, > - &ofpacts, &b->header_.uuid); > - } > - ofctrl_add_flow(flow_table, OFTABLE_MAC_CACHE_USE, priority, > - b->header_.uuid.parts[0], &mb_cache_use_match, > - &ofpacts, &b->header_.uuid); > + struct in6_addr ip_addr; > + if (!ip46_parse(ip, &ip_addr)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad 'ip' %s", ip); > + return; > } > > - ofpbuf_uninit(&ofpacts); > + consider_neighbor_flow(pb, b ? &b->header_.uuid : &smb->header_.uuid, > + &ip_addr, mac_addr, flow_table, priority, > + needs_usage_tracking); > } > > /* Adds an OpenFlow flow to flow tables for each MAC binding in the OVN > @@ -1422,16 +1343,19 @@ add_neighbor_flows(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > /* Add flows for learnt MAC bindings */ > const struct sbrec_mac_binding *b; > SBREC_MAC_BINDING_TABLE_FOR_EACH (b, mac_binding_table) { > - consider_neighbor_flow(sbrec_port_binding_by_name, > local_datapaths, > - b, NULL, flow_table, 100); > + consider_neighbor_flow__(sbrec_port_binding_by_name, > local_datapaths, > + b, NULL, flow_table, > + NEIGH_OF_DYNAMIC_MAC_BINDING_PRIO); > } > > /* Add flows for statically configured MAC bindings */ > const struct sbrec_static_mac_binding *smb; > SBREC_STATIC_MAC_BINDING_TABLE_FOR_EACH (smb, smb_table) { > - consider_neighbor_flow(sbrec_port_binding_by_name, > local_datapaths, > - NULL, smb, flow_table, > - smb->override_dynamic_mac ? 150 : 50); > + consider_neighbor_flow__(sbrec_port_binding_by_name, > local_datapaths, > + NULL, smb, flow_table, > + smb->override_dynamic_mac > + ? NEIGH_OF_STATIC_MAC_BINDING_HIGH_PRIO > + : NEIGH_OF_STATIC_MAC_BINDING_LOW_PRIO); > } > } > > @@ -1960,8 +1884,9 @@ lflow_handle_changed_mac_bindings( > } > VLOG_DBG("handle new mac_binding "UUID_FMT, > UUID_ARGS(&mb->header_.uuid)); > - consider_neighbor_flow(sbrec_port_binding_by_name, > local_datapaths, > - mb, NULL, flow_table, 100); > + consider_neighbor_flow__(sbrec_port_binding_by_name, > + local_datapaths, mb, NULL, > flow_table, > + NEIGH_OF_DYNAMIC_MAC_BINDING_PRIO); > } > } > } > @@ -1988,9 +1913,11 @@ lflow_handle_changed_static_mac_bindings( > } > VLOG_DBG("handle new static_mac_binding "UUID_FMT, > UUID_ARGS(&smb->header_.uuid)); > - consider_neighbor_flow(sbrec_port_binding_by_name, > local_datapaths, > - NULL, smb, flow_table, > - smb->override_dynamic_mac ? 150 : 50); > + consider_neighbor_flow__(sbrec_port_binding_by_name, > + local_datapaths, NULL, smb, > flow_table, > + smb->override_dynamic_mac > + ? > NEIGH_OF_STATIC_MAC_BINDING_HIGH_PRIO > + : > NEIGH_OF_STATIC_MAC_BINDING_LOW_PRIO); > } > } > } > @@ -2193,9 +2120,10 @@ lflow_add_flows_for_datapath(const struct > sbrec_datapath_binding *dp, > const struct sbrec_mac_binding *mb; > SBREC_MAC_BINDING_FOR_EACH_EQUAL ( > mb, mb_index_row, l_ctx_in->sbrec_mac_binding_by_datapath) { > - consider_neighbor_flow(l_ctx_in->sbrec_port_binding_by_name, > - l_ctx_in->local_datapaths, > - mb, NULL, l_ctx_out->flow_table, 100); > + consider_neighbor_flow__(l_ctx_in->sbrec_port_binding_by_name, > + l_ctx_in->local_datapaths, > + mb, NULL, l_ctx_out->flow_table, > + NEIGH_OF_DYNAMIC_MAC_BINDING_PRIO); > } > sbrec_mac_binding_index_destroy_row(mb_index_row); > > @@ -2206,10 +2134,12 @@ lflow_add_flows_for_datapath(const struct > sbrec_datapath_binding *dp, > const struct sbrec_static_mac_binding *smb; > SBREC_STATIC_MAC_BINDING_FOR_EACH_EQUAL ( > smb, smb_index_row, > l_ctx_in->sbrec_static_mac_binding_by_datapath) { > - consider_neighbor_flow(l_ctx_in->sbrec_port_binding_by_name, > - l_ctx_in->local_datapaths, > - NULL, smb, l_ctx_out->flow_table, > - smb->override_dynamic_mac ? 150 : 50); > + consider_neighbor_flow__(l_ctx_in->sbrec_port_binding_by_name, > + l_ctx_in->local_datapaths, > + NULL, smb, l_ctx_out->flow_table, > + smb->override_dynamic_mac > + ? NEIGH_OF_STATIC_MAC_BINDING_HIGH_PRIO > + : NEIGH_OF_STATIC_MAC_BINDING_LOW_PRIO); > } > sbrec_static_mac_binding_index_destroy_row(smb_index_row); > > diff --git a/controller/neighbor-of.c b/controller/neighbor-of.c > new file mode 100644 > index 0000000000..ea9d9881ad > --- /dev/null > +++ b/controller/neighbor-of.c > @@ -0,0 +1,111 @@ > +/* Copyright (c) 2015, 2016, 2017 Nicira, Inc. > nit: I think we shouldn't carry the copyright to a new file, but I don't have a strong preference. + * Copyright (c) 2025, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > + > +#include "lflow.h" > +#include "neighbor-of.h" > +#include "openvswitch/match.h" > +#include "ovn/logical-fields.h" > + > +void > +consider_neighbor_flow(const struct sbrec_port_binding *pb, > + const struct uuid *neighbor_uuid, > + const struct in6_addr *ip, struct eth_addr mac, > + struct ovn_desired_flow_table *flow_table, > + enum neigh_of_rule_prio priority, > + bool needs_usage_tracking) > +{ > + struct match get_arp_match = MATCH_CATCHALL_INITIALIZER; > + struct match lookup_arp_match = MATCH_CATCHALL_INITIALIZER; > + struct match mb_cache_use_match = MATCH_CATCHALL_INITIALIZER; > + struct match lookup_arp_for_stats_match = MATCH_CATCHALL_INITIALIZER; > + > + match_set_dl_src(&lookup_arp_match, mac); > + match_set_metadata(&lookup_arp_match, > htonll(pb->datapath->tunnel_key)); > + match_set_reg(&lookup_arp_match, MFF_LOG_INPORT - MFF_REG0, > + pb->tunnel_key); > + > + if (IN6_IS_ADDR_V4MAPPED(ip)) { > + ovs_be32 ip_addr = in6_addr_get_mapped_ipv4(ip); > + match_set_reg(&get_arp_match, 0, ntohl(ip_addr)); > + > + match_set_dl_type(&lookup_arp_match, htons(ETH_TYPE_ARP)); > + lookup_arp_for_stats_match = lookup_arp_match; > + > + match_set_reg(&lookup_arp_match, 0, ntohl(ip_addr)); > + > + match_set_dl_type(&mb_cache_use_match, htons(ETH_TYPE_IP)); > + match_set_nw_src(&mb_cache_use_match, ip_addr); > + > + match_set_arp_opcode_masked(&lookup_arp_for_stats_match, 2, 0xff); > + match_set_arp_spa_masked(&lookup_arp_for_stats_match, ip_addr, > + htonl(0xffffffff)); > + } else { > + ovs_be128 value; > + memcpy(&value, ip, sizeof(value)); > + match_set_xxreg(&get_arp_match, 1, ntoh128(value)); > + > + match_set_dl_type(&lookup_arp_match, htons(ETH_TYPE_IPV6)); > + match_set_nw_proto(&lookup_arp_match, 58); > + match_set_icmp_code(&lookup_arp_match, 0); > + > + match_set_xxreg(&lookup_arp_match, 0, ntoh128(value)); > + > + match_set_dl_type(&mb_cache_use_match, htons(ETH_TYPE_IPV6)); > + match_set_ipv6_src(&mb_cache_use_match, ip); > + } > + > + match_set_metadata(&get_arp_match, htonll(pb->datapath->tunnel_key)); > + match_set_reg(&get_arp_match, MFF_LOG_OUTPORT - MFF_REG0, > pb->tunnel_key); > + > + match_set_dl_src(&mb_cache_use_match, mac); > + match_set_reg(&mb_cache_use_match, MFF_LOG_INPORT - MFF_REG0, > + pb->tunnel_key); > + match_set_metadata(&mb_cache_use_match, > htonll(pb->datapath->tunnel_key)); > + > + uint64_t stub[1024 / 8]; > + struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub); > + put_load_bytes(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, > + &ofpacts); > + put_load(1, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1, &ofpacts); > + ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, priority, > + neighbor_uuid->parts[0], > + &get_arp_match, &ofpacts, > + neighbor_uuid); > + > + ofpbuf_clear(&ofpacts); > + put_load(1, MFF_LOG_FLAGS, MLF_LOOKUP_MAC_BIT, 1, &ofpacts); > + ofctrl_add_flow(flow_table, OFTABLE_MAC_LOOKUP, priority, > + neighbor_uuid->parts[0], > + &lookup_arp_match, &ofpacts, > + neighbor_uuid); > + > + if (needs_usage_tracking) { > + ofpbuf_clear(&ofpacts); > + if (IN6_IS_ADDR_V4MAPPED(ip)) { > + ofctrl_add_flow(flow_table, OFTABLE_MAC_CACHE_USE, priority, > + neighbor_uuid->parts[0], > + &lookup_arp_for_stats_match, > + &ofpacts, neighbor_uuid); > + } > + ofctrl_add_flow(flow_table, OFTABLE_MAC_CACHE_USE, priority, > + neighbor_uuid->parts[0], &mb_cache_use_match, > + &ofpacts, neighbor_uuid); > + } > + > + ofpbuf_uninit(&ofpacts); > +} > diff --git a/controller/neighbor-of.h b/controller/neighbor-of.h > new file mode 100644 > index 0000000000..874200e8be > --- /dev/null > +++ b/controller/neighbor-of.h > @@ -0,0 +1,45 @@ > +/* Copyright (c) 2015, 2016, 2017 Nicira, Inc. > nit: Same here. > + * Copyright (c) 2025, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#ifndef NEIGHBOR_OF_H > +#define NEIGHBOR_OF_H 1 > + > +#include "ovn-sb-idl.h" > +#include "openvswitch/uuid.h" > +#include "ofctrl.h" > + > +/* Priorities of ovn-controller generated flows for various types of MAC > + * Bindings in different situations. Valid preference orders, based on > + * the SB.Static_MAC_Binding.override_dynamic_mac value are: > + * > + * - static-mac-binding < dynamic-mac-binding > + * - dynamic-mac-binding < static-mac-binding > + */ > +enum neigh_of_rule_prio { > + NEIGH_OF_STATIC_MAC_BINDING_LOW_PRIO = 50, > + NEIGH_OF_DYNAMIC_MAC_BINDING_PRIO = 100, > + NEIGH_OF_STATIC_MAC_BINDING_HIGH_PRIO = 150, > +}; > + > +void > +consider_neighbor_flow(const struct sbrec_port_binding *, > + const struct uuid *neighbor_uuid, > + const struct in6_addr *, struct eth_addr, > + struct ovn_desired_flow_table *, > + enum neigh_of_rule_prio priority, > + bool needs_usage_tracking); > + > +#endif /* NEIGHBOR_OF_H */ > -- > 2.51.0 > > Regardless of the nit: Acked-by: Ales Musil <[email protected]> Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
