On Mon, Sep 1, 2025 at 11:56 AM Lorenzo Bianconi < lorenzo.bianc...@redhat.com> wrote:
> Introduce disable_garp_rarp option in the Logical_Router table in order > to disable GARP/RARP announcements by all the peer ports of this logical > router. > > Reported-at: https://issues.redhat.com/browse/FDP-1537 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > --- > Changes in v4: > - Add IP support in en_garp_rarp node for en_sb_datapath_binding dependency > - Improve unit-test > > Changes in v3: > - Trigger garp_rarp_run if datapath sb table change > - Improve unit-test > - cosmetics > --- > Hi Lorenzo, thank you for v4. I have a couple of small comments down below. > NEWS | 2 + > controller/garp_rarp.c | 26 ++++++++- > controller/ovn-controller.c | 32 ++++++++++ > northd/en-datapath-logical-router.c | 5 ++ > ovn-nb.xml | 9 +++ > tests/ovn.at | 91 +++++++++++++++++++++++++++++ > 6 files changed, 163 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 932e173af..f21a7e235 100644 > --- a/NEWS > +++ b/NEWS > @@ -66,6 +66,8 @@ OVN v25.09.0 - xxx xx xxxx > - ovn-sbctl: > * Added --filter option that allows filtering the command output. See > ovn-sbctl(8) for more details. > + - Added disable_garp_rarp option to logical_router table in order to > disable > + GARP/RARP announcements by all the peer ports of this logical router. > > OVN v25.03.0 - 07 Mar 2025 > -------------------------- > diff --git a/controller/garp_rarp.c b/controller/garp_rarp.c > index 8eccf10fe..940c38e13 100644 > --- a/controller/garp_rarp.c > +++ b/controller/garp_rarp.c > @@ -446,6 +446,26 @@ garp_rarp_clear(struct garp_rarp_ctx_in *r_ctx_in) > sset_clear(&r_ctx_in->data->local_lports); > } > > +static bool > +garp_rarp_is_enabled(struct ovsdb_idl_index *sbrec_port_binding_by_name, > + const struct sbrec_port_binding *pb) > +{ > + if (smap_get_bool(&pb->options, "disable_garp_rarp", false)) { > + return false; > + } > + > + /* Check if GARP probing is disabled on the peer logical router. */ > + const struct sbrec_port_binding *peer = lport_get_peer( > + pb, sbrec_port_binding_by_name); > + if (peer && peer->datapath && > nit: Port binding cannot have NULL datapath. > + smap_get_bool(&peer->datapath->external_ids, > + "disable_garp_rarp", false)) { > + return false; > + } > + > + return true; > +} > + > void > garp_rarp_run(struct garp_rarp_ctx_in *r_ctx_in) > { > @@ -478,7 +498,8 @@ garp_rarp_run(struct garp_rarp_ctx_in *r_ctx_in) > SSET_FOR_EACH (iface_id, &localnet_vifs) { > const struct sbrec_port_binding *pb = lport_lookup_by_name( > r_ctx_in->sbrec_port_binding_by_name, iface_id); > - if (pb && !smap_get_bool(&pb->options, "disable_garp_rarp", > false)) { > + if (pb && > + garp_rarp_is_enabled(r_ctx_in->sbrec_port_binding_by_name, > pb)) { > send_garp_rarp_update(r_ctx_in, pb, &nat_addresses); > } > } > @@ -488,7 +509,8 @@ garp_rarp_run(struct garp_rarp_ctx_in *r_ctx_in) > SSET_FOR_EACH (gw_port, &local_l3gw_ports) { > const struct sbrec_port_binding *pb = lport_lookup_by_name( > r_ctx_in->sbrec_port_binding_by_name, gw_port); > - if (pb && !smap_get_bool(&pb->options, "disable_garp_rarp", > false)) { > + if (pb && > + garp_rarp_is_enabled(r_ctx_in->sbrec_port_binding_by_name, > pb)) { > send_garp_rarp_update(r_ctx_in, pb, &nat_addresses); > } > } > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 6396fa898..ab1b53840 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -5678,6 +5678,36 @@ garp_rarp_sb_port_binding_handler(struct > engine_node *node, > return EN_HANDLED_UNCHANGED; > } > > +static enum engine_input_handler_result > +garp_rarp_sb_datapath_binding_handler(struct engine_node *node, > + void *data_ OVS_UNUSED) > +{ > + struct ed_type_runtime_data *rt_data = > + engine_get_input_data("runtime_data", node); > + > + const struct sbrec_datapath_binding_table *dp_binding_table = > + EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); > + const struct sbrec_datapath_binding *dp; > + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_binding_table) { > + if (strcmp(dp->type, "logical-router")) { > + continue; > + } > This check is not needed see below. > + > + struct local_datapath *ld = get_local_datapath( > + &rt_data->local_datapaths, dp->tunnel_key); > + if (!ld) { > We can change the if into " if (!ld || ld->is_switch)". That will also cover the check above. > + continue; > + } > + > + if (sbrec_datapath_binding_is_updated( > + dp, SBREC_DATAPATH_BINDING_COL_EXTERNAL_IDS)) { > + return EN_UNHANDLED; > + } > + } > + > + return EN_HANDLED_UNCHANGED; > +} > + > static enum engine_input_handler_result > garp_rarp_runtime_data_handler(struct engine_node *node, void *data > OVS_UNUSED) > { > @@ -6867,6 +6897,8 @@ main(int argc, char *argv[]) > engine_add_input(&en_garp_rarp, &en_sb_chassis, NULL); > engine_add_input(&en_garp_rarp, &en_sb_port_binding, > garp_rarp_sb_port_binding_handler); > + engine_add_input(&en_garp_rarp, &en_sb_datapath_binding, > + garp_rarp_sb_datapath_binding_handler); > /* The mac_binding data is just used in an index to filter duplicates > when > * inserting data to the southbound. */ > engine_add_input(&en_garp_rarp, &en_sb_mac_binding, > engine_noop_handler); > diff --git a/northd/en-datapath-logical-router.c > b/northd/en-datapath-logical-router.c > index 22687e849..f6f49422f 100644 > --- a/northd/en-datapath-logical-router.c > +++ b/northd/en-datapath-logical-router.c > @@ -83,6 +83,11 @@ gather_external_ids(const struct nbrec_logical_router > *nbr, > "%u", age_threshold); > } > > + bool disable_garp_rarp = smap_get_bool(&nbr->options, > "disable_garp_rarp", > + false); > + smap_add_format(external_ids, "disable_garp_rarp", > + disable_garp_rarp ? "true" : "false"); > + > /* For backwards-compatibility, also store the NB UUID in > * external-ids:logical-router. This is useful if ovn-controller > * has not updated and expects this to be where to find the > diff --git a/ovn-nb.xml b/ovn-nb.xml > index b7b5b5c40..034500e00 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -3333,6 +3333,15 @@ or > routes in <code>ovn-ic</code> daemon. > </p> > </column> > + > + <column name="options" key="disable_garp_rarp" > + type='{"type": "boolean"}'> > + <p> > + If set to <code>true</code>, GARP and RARP announcements are not > + sent by all the VIF peer ports of this logical router. > + The default value is <code>false</code>. > + </p> > + </column> > </group> > <group title="Common Columns"> > <column name="external_ids"> > diff --git a/tests/ovn.at b/tests/ovn.at > index 292ca0dae..88097c928 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -26310,6 +26310,97 @@ OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([Disabling RARP/GARP announcements from Router options]) > +ovn_start > + > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +check ovs-vsctl add-port br-phys snoopvif -- set Interface snoopvif > options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap > +check ovs-vsctl add-port br-int vif1 -- set Interface vif1 > external-ids:iface-id=lsp1 > +check ovs-vsctl add-port br-int vif2 -- set Interface vif2 > external-ids:iface-id=lsp2 > + > +check ovs-vsctl set Open_vSwitch . external-ids:garp-max-timeout-sec=1 > + > +check ovn-nbctl ls-add ls1 > +check ovn-nbctl lsp-add ls1 ln1 > +check ovn-nbctl lsp-set-addresses ln1 unknown > +check ovn-nbctl lsp-set-type ln1 localnet > +check ovn-nbctl lsp-set-options ln1 network_name=phys > +check ovn-nbctl lsp-add ls1 lsp1 > +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:12 192.168.1.2" > +check ovn-nbctl --wait=hv sync > + > +check ovn-nbctl ls-add ls2 > +check ovn-nbctl lsp-add ls2 lsp2 > +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:00:13 10.0.0.2" > +check ovn-nbctl --wait=hv sync > + > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl set Logical_Router lr1 options:disable_garp_rarp="true" > +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:11 192.168.1.1/24 > +check ovn-nbctl lrp-add lr1 lrp2 00:00:00:00:00:14 10.0.0.1/24 > +check ovn-nbctl lsp-add ls1 ls-lrp1 \ > + -- set Logical_Switch_Port ls-lrp1 type=router \ > + options:router-port=lrp1 addresses=\"00:00:00:00:00:11\" > +check ovn-nbctl lsp-add ls2 ls-lrp2 \ > + -- set Logical_Switch_Port ls-lrp2 type=router \ > + options:router-port=lrp2 addresses=\"00:00:00:00:00:14\" > +check ovn-nbctl lsp-set-options ls-lrp1 router-port=lrp1 > nat-addresses="router" > +check ovn-nbctl lr-nat-add lr1 snat 192.168.1.10 10.0.0.0/24 > +check ovn-nbctl lrp-set-gateway-chassis lrp1 hv1 > +check ovn-nbctl --wait=hv sync > + > +wait_for_ports_up > + > +garp_lrp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', > src='00:00:00:00:00:11')/ \ > + ARP(hwsrc='00:00:00:00:00:11', psrc='192.168.1.1', > pdst='192.168.1.1')") > +garp_vif=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', > src='00:00:00:00:00:12')/ \ > + ARP(hwsrc='00:00:00:00:00:12', psrc='192.168.1.2', > pdst='192.168.1.2')") > +garp_nat=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', > src='00:00:00:00:00:11')/ \ > + ARP(hwsrc='00:00:00:00:00:11', psrc='192.168.1.10', > pdst='192.168.1.10')") > +# GARP packet for vif > +echo $garp_vif > expected > +OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected]) > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap > > hv1/snoopvif-tx.packets > +AT_CHECK([grep -q "$garp_lrp" hv1/snoopvif-tx.packets], [1]) > + > +# GARP packet for lrp > +echo $garp_lrp >> expected > +echo $garp_nat >> expected > +check ovn-nbctl --wait=hv set Logical_Router lr1 > options:disable_garp_rarp="false" > +OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected]) > + > +# Check for GW router > +check ovn-nbctl lrp-del-gateway-chassis lrp1 hv1 > +check ovn-nbctl set Logical_Router lr1 options:chassis="hv1" > +check ovn-nbctl set Logical_Router lr1 options:disable_garp_rarp="true" > +check ovn-nbctl --wait=hv sync > + > +sleep_controller hv1 > +reset_pcap_file snoopvif hv1/snoopvif > +wake_up_controller hv1 > + > +echo $garp_vif > expected > +OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected]) > +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap > > hv1/snoopvif-tx.packets > +AT_CHECK([grep -q "$garp_lrp" hv1/snoopvif-tx.packets], [1]) > + > +echo $garp_lrp >> expected > +echo $garp_nat >> expected > +check ovn-nbctl set Logical_Router lr1 options:disable_garp_rarp="false" > +check ovn-nbctl --wait=hv sync > +OVN_CHECK_PACKETS_UNIQ([hv1/snoopvif-tx.pcap], [expected]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([Stateless Floating IP]) > ovn_start > -- > 2.50.1 > > Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
