> 2022年7月14日 下午4:32，Lorenzo Bianconi <[email protected]> 写道： > >>> >>> On 7/13/22 18:07, Lorenzo Bianconi wrote: >>>>> On 6/17/22 00:31, Lorenzo Bianconi wrote: >>>>>> When using VLAN backed networks and OVN routers leveraging the >>>>>> 'ovn-chassis-mac-mappings' option, the eth.src field is replaced by the >>>>>> chassis mac address in order to not expose the router mac address from >>>>>> different nodes and confuse the TOR switch. However doing so the TOR >>>>>> switch is not able to learn the port/mac bindings for routed E/W traffic >>>>>> and it is force to always flood it. Fix this issue adding the capability >>>>>> to send GARP traffic for logical switch ports if the corresponding >>>>>> logical >>>>>> switch has the ovn-lsp-garp parameter set to true in the option column. >>>>>> More into about the issue can be found here [0]. >>>>>> >>>>>> [0] >>>>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html >>>>>> >>>>>> <https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html> >>>>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087779 >>>>>> <https://bugzilla.redhat.com/show_bug.cgi?id=2087779> >>>>>> >>>>>> Signed-off-by: Lorenzo Bianconi <[email protected] >>>>>> <mailto:[email protected]>> >>>>>> --- >>>>> >>>>> Hi Lorenzo, >>>> >>>> Hi Dumitru, >>>> >>>> Thanks for reviewing it :) >>>> >>>>> >>>>> I have a few concerns with this approach: >>>>> >>>>> a. The CMS will have to set this option for all VMs on all logical >>>>> switches which will enable periodic GARPs for all of them all the time. >>>>> That seems like quite a lot of broadcast traffic in the fabric. >>>>> >>>>> b. There's no guarantee that the GARPs are sent in time to prevent the >>>>> FDB timeouts on the ToR switches. At best we could make the interval >>>>> configurable but I don't think this is way better either. >>>>> >>>>> c. This is not really introduced by your patch but we will be causing >>>>> this more often now. With the topology: >>>>> >>>>> (HV1) VM1 -- LS1 --- LR -- LS2 -- VM2 (HV2) >>>>> (VLAN-backed network) >>>>> >>>>> HV1 configured with chassis mac mapping HV1-MAC >>>>> HV2 configured with chassis mac mapping HV2-MAC >>>>> >>>>> We're leaking MAC addresses from LS1's broadcast domain (VM1-MAC) and >>>>> from LS2's broadcast domain (VM2-MAC) into the fabric. I'm not sure >>>>> that's OK. >>>>> >>>>> I think a proper solution is to change how we run the logical pipelines >>>>> in case of vlan-backed networks. We currently have an assymetry: >>>>> >>>>> For packets flowing from VM1 to VM2 we execute: >>>>> - on HV1: LS1-ingress, LS1-egress, LR-ingress, LR-egress, LS2-ingress >>>>> - on HV2: LS2-egress >>>>> >>>>> For packets flowing from VM2 to VM1 we execute: >>>>> - on HV2: LS2-ingress, LS2-egress, LR-ingress, LR-egress, LS1-ingress >>>>> - on HV1: LS1-egress >>>>> >>>>> What if we change this to: >>>>> VM1 -> VM2: >>>>> - on HV1: LS1-ingress, LS1-egress, LR-ingress >>>>> - on HV2: LR-egress, LS2-ingress, LS2-egress >>>>> >>>>> VM2 -> VM1: >>>>> - on HV2: LS2-ingress, LS2-egress, LR-ingress >>>>> - on HV2: LR-egress, LS1-ingress, LS1-egress >>>> >>>> I do not know why the current architecture is done this way (any >>>> suggestions??). >>>> I guess the approach you suggested should work. Are we introducing any >>>> backward >>>> compatibility issue? >>> >>> We would probably create a compatibility issue. :) >>> >>> I don't know if this approach would even work, I was just trying to >>> imagine how traffic from within OVN should be seen on the fabric in this >>> case. >>> >>> Maybe we need to think some more about other options. >>> >>> Regards, >>> Dumitru >> Hi Dumitru and Lorenzo, > > Hi Shy Liu, > >> in my opinion, the flood issue maybe caused by the implemention that >> logical_switch always be an arp-proxy for lsp which has known address, >> but this implemention is not applicable to vlan backed network. Right ? >> >> I have an solution: >> Maybe we can add a option named arp-proxy for logical_switch >> that always be false if network type is vlan, then disable the >> logical_switch response the arp for the lsp which belongs to >> the vlan backed network. >> >> So that this solution can let the destination MAC address >> always learnt by TOR for vlan backed network. > > For e/w routed traffic from pod A to pod B, pod A will send an arp request > for the router ip address. Who is going to reply to this request in this way? Hi Lorenzo, sorry that I had not describe it clearly. We can disable arp response for the lsp which type is not router port nor localport. So the logical_switch still responses the arp request for lrp.
> If ovn replies to the arp request from pod A but let the request go through > the router pipeline I think even the other hypervisor will reply to the arp > request (please remember the logical router is distributed in ovn) and so we > will confuse the TOR switch since we will see the router mac address flapping > between two ports. Do you agree or am I missing something? We can use 'ovn-chassis-mac-mappings’ option to avoid the problem that the lrp's mac address flap. Right ? > > Regards, > Lorenzo > >> >> But the lrp might learn the destination MAC address and put_arp, >> so this operation also should been disabled. >> >> This is not useful for logical_switch which is external net. >> >> >> Reference: >> [0] https://mail.openvswitch.org/pipermail/ovs-discuss/2022-May/051837.html >> <https://mail.openvswitch.org/pipermail/ovs-discuss/2022-May/051837.html>. >>> >>>> >>>> Regards, >>>> Lorenzo >>>> >>>>> >>>>> Would this we ensure that we always only use ovn-chassis-mac-mappings on >>>>> the VLAN network and avoid flooding on the ToR? >>>>> >>>>> Regards, >>>>> Dumitru >>>>> >>>>>> controller/pinctrl.c | 85 +++++++++++++++++++++++++++++--------------- >>>>>> northd/northd.c | 29 +++++++++++++++ >>>>>> 2 files changed, 85 insertions(+), 29 deletions(-) >>>>>> >>>>>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >>>>>> index 9a1a0faa1..eb5739bfc 100644 >>>>>> --- a/controller/pinctrl.c >>>>>> +++ b/controller/pinctrl.c >>>>>> @@ -4533,7 +4533,8 @@ send_garp_rarp(struct rconn *swconn, struct >>>>>> garp_rarp_data *garp_rarp, >>>>>> garp_rarp->backoff *= 2; >>>>>> garp_rarp->announce_time = current_time + garp_rarp->backoff * >>>>>> 1000; >>>>>> } else { >>>>>> - garp_rarp->announce_time = LLONG_MAX; >>>>>> + /* Default timeout is 180s. */ >>>>>> + garp_rarp->announce_time = current_time + 180 * 1000; >>>>>> } >>>>>> return garp_rarp->announce_time; >>>>>> } >>>>>> @@ -5510,14 +5511,15 @@ ip_mcast_querier_wait(long long int query_time) >>>>>> >>>>>> /* Get localnet vifs, local l3gw ports and ofport for localnet patch >>>>>> ports. */ >>>>>> static void >>>>>> -get_localnet_vifs_l3gwports( >>>>>> +get_local_vifs_l3gwports( >>>>>> struct ovsdb_idl_index *sbrec_port_binding_by_datapath, >>>>>> struct ovsdb_idl_index *sbrec_port_binding_by_name, >>>>>> const struct ovsrec_bridge *br_int, >>>>>> const struct sbrec_chassis *chassis, >>>>>> const struct hmap *local_datapaths, >>>>>> struct sset *localnet_vifs, >>>>>> - struct sset *local_l3gw_ports) >>>>>> + struct sset *local_l3gw_ports, >>>>>> + struct sset *local_vifs) >>>>>> { >>>>>> for (int i = 0; i < br_int->n_ports; i++) { >>>>>> const struct ovsrec_port *port_rec = br_int->ports[i]; >>>>>> @@ -5574,7 +5576,8 @@ get_localnet_vifs_l3gwports( >>>>>> /* Get l3gw ports. Consider port bindings with type "l3gateway" >>>>>> * that connect to gateway routers (if local), and consider port >>>>>> * bindings of type "patch" since they might connect to >>>>>> - * distributed gateway ports with NAT addresses. */ >>>>>> + * distributed gateway ports with NAT addresses. >>>>>> + * Get LSP ports if requested by CMS. */ >>>>>> >>>>>> sbrec_port_binding_index_set_datapath(target, ld->datapath); >>>>>> SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target, >>>>>> @@ -5583,6 +5586,11 @@ get_localnet_vifs_l3gwports( >>>>>> || !strcmp(pb->type, "patch")) { >>>>>> sset_add(local_l3gw_ports, pb->logical_port); >>>>>> } >>>>>> + /* GARP packets for lsp ports. */ >>>>>> + if (pb->chassis == chassis && >>>>>> + smap_get_bool(&pb->options, "ovn-lsp-garp", false)) { >>>>>> + sset_add(local_vifs, pb->logical_port); >>>>>> + } >>>>>> } >>>>>> } >>>>>> sbrec_port_binding_index_destroy_row(target); >>>>>> @@ -5761,6 +5769,26 @@ send_garp_rarp_run(struct rconn *swconn, long >>>>>> long int *send_garp_rarp_time) >>>>>> } >>>>>> } >>>>>> >>>>>> +static void >>>>>> +send_garp_rarp_update_for_pb_set( >>>>>> + struct ovsdb_idl_txn *ovnsb_idl_txn, >>>>>> + struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip, >>>>>> + struct ovsdb_idl_index *sbrec_port_binding_by_name, >>>>>> + struct sset *vif_set, const struct hmap *local_datapaths, >>>>>> + struct shash *nat_addresses) >>>>>> +{ >>>>>> + const char *iface_id; >>>>>> + SSET_FOR_EACH (iface_id, vif_set) { >>>>>> + const struct sbrec_port_binding *pb = lport_lookup_by_name( >>>>>> + sbrec_port_binding_by_name, iface_id); >>>>>> + if (pb) { >>>>>> + send_garp_rarp_update(ovnsb_idl_txn, >>>>>> + sbrec_mac_binding_by_lport_ip, >>>>>> + local_datapaths, pb, nat_addresses); >>>>>> + } >>>>>> + } >>>>>> +} >>>>>> + >>>>>> /* Called by pinctrl_run(). Runs with in the main ovn-controller >>>>>> * thread context. */ >>>>>> static void >>>>>> @@ -5776,15 +5804,17 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn >>>>>> *ovnsb_idl_txn, >>>>>> { >>>>>> struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs); >>>>>> struct sset local_l3gw_ports = SSET_INITIALIZER(&local_l3gw_ports); >>>>>> + struct sset local_vifs = SSET_INITIALIZER(&local_vifs); >>>>>> struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys); >>>>>> struct shash nat_addresses; >>>>>> >>>>>> shash_init(&nat_addresses); >>>>>> >>>>>> - get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath, >>>>>> - sbrec_port_binding_by_name, >>>>>> - br_int, chassis, local_datapaths, >>>>>> - &localnet_vifs, &local_l3gw_ports); >>>>>> + get_local_vifs_l3gwports(sbrec_port_binding_by_datapath, >>>>>> + sbrec_port_binding_by_name, >>>>>> + br_int, chassis, local_datapaths, >>>>>> + &localnet_vifs, &local_l3gw_ports, >>>>>> + &local_vifs); >>>>>> >>>>>> get_nat_addresses_and_keys(sbrec_port_binding_by_name, >>>>>> &nat_ip_keys, &local_l3gw_ports, >>>>>> @@ -5795,36 +5825,33 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn >>>>>> *ovnsb_idl_txn, >>>>>> struct shash_node *iter; >>>>>> SHASH_FOR_EACH_SAFE (iter, &send_garp_rarp_data) { >>>>>> if (!sset_contains(&localnet_vifs, iter->name) && >>>>>> - !sset_contains(&nat_ip_keys, iter->name)) { >>>>>> + !sset_contains(&nat_ip_keys, iter->name) && >>>>>> + !sset_contains(&local_vifs, iter->name)) { >>>>>> send_garp_rarp_delete(iter->name); >>>>>> } >>>>>> } >>>>>> >>>>>> /* Update send_garp_rarp_data. */ >>>>>> - const char *iface_id; >>>>>> - SSET_FOR_EACH (iface_id, &localnet_vifs) { >>>>>> - const struct sbrec_port_binding *pb = lport_lookup_by_name( >>>>>> - sbrec_port_binding_by_name, iface_id); >>>>>> - if (pb) { >>>>>> - send_garp_rarp_update(ovnsb_idl_txn, >>>>>> - sbrec_mac_binding_by_lport_ip, >>>>>> - local_datapaths, pb, &nat_addresses); >>>>>> - } >>>>>> - } >>>>>> - >>>>>> + send_garp_rarp_update_for_pb_set(ovnsb_idl_txn, >>>>>> + sbrec_mac_binding_by_lport_ip, >>>>>> + sbrec_port_binding_by_name, >>>>>> + &localnet_vifs, local_datapaths, >>>>>> + &nat_addresses); >>>>>> + send_garp_rarp_update_for_pb_set(ovnsb_idl_txn, >>>>>> + sbrec_mac_binding_by_lport_ip, >>>>>> + sbrec_port_binding_by_name, >>>>>> + &local_vifs, local_datapaths, >>>>>> + &nat_addresses); >>>>>> /* Update send_garp_rarp_data for nat-addresses. */ >>>>>> - const char *gw_port; >>>>>> - SSET_FOR_EACH (gw_port, &local_l3gw_ports) { >>>>>> - const struct sbrec_port_binding *pb >>>>>> - = lport_lookup_by_name(sbrec_port_binding_by_name, gw_port); >>>>>> - if (pb) { >>>>>> - send_garp_rarp_update(ovnsb_idl_txn, >>>>>> sbrec_mac_binding_by_lport_ip, >>>>>> - local_datapaths, pb, &nat_addresses); >>>>>> - } >>>>>> - } >>>>>> + send_garp_rarp_update_for_pb_set(ovnsb_idl_txn, >>>>>> + sbrec_mac_binding_by_lport_ip, >>>>>> + sbrec_port_binding_by_name, >>>>>> + &local_l3gw_ports, local_datapaths, >>>>>> + &nat_addresses); >>>>>> >>>>>> /* pinctrl_handler thread will send the GARPs. */ >>>>>> >>>>>> + sset_destroy(&local_vifs); >>>>>> sset_destroy(&localnet_vifs); >>>>>> sset_destroy(&local_l3gw_ports); >>>>>> >>>>>> diff --git a/northd/northd.c b/northd/northd.c >>>>>> index 0d6ebccde..9a4a880a7 100644 >>>>>> --- a/northd/northd.c >>>>>> +++ b/northd/northd.c >>>>>> @@ -6446,6 +6446,34 @@ ovn_update_ipv6_options(struct hmap *ports) >>>>>> } >>>>>> } >>>>>> >>>>>> +static void >>>>>> +ovn_update_lsp_garp_options(struct hmap *ports) >>>>>> +{ >>>>>> + struct ovn_port *op; >>>>>> + HMAP_FOR_EACH (op, key_node, ports) { >>>>>> + if (!op->nbsp) { >>>>>> + continue; >>>>>> + } >>>>>> + if (op->nbsp->type[0] || op->nbsp->parent_name) { >>>>>> + continue; >>>>>> + } >>>>>> + >>>>>> + struct ovn_datapath *od = op->od; >>>>>> + if (!od || !od->nbs) { >>>>>> + continue; >>>>>> + } >>>>>> + >>>>>> + struct smap options; >>>>>> + smap_clone(&options, &op->sb->options); >>>>>> + >>>>>> + bool ovn_lsp_garp = smap_get_bool(&od->nbs->other_config, >>>>>> + "ovn-lsp-garp", false); >>>>>> + smap_add(&options, "ovn-lsp-garp", ovn_lsp_garp ? "true" : >>>>>> "false"); >>>>>> + sbrec_port_binding_set_options(op->sb, &options); >>>>>> + smap_destroy(&options); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> static void >>>>>> build_port_group_lswitches(struct northd_input *input_data, >>>>>> struct hmap *pgs, >>>>>> @@ -15381,6 +15409,7 @@ ovnnb_db_run(struct northd_input *input_data, >>>>>> stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec()); >>>>>> ovn_update_ipv6_options(&data->ports); >>>>>> ovn_update_ipv6_prefix(&data->ports); >>>>>> + ovn_update_lsp_garp_options(&data->ports); >>>>>> >>>>>> sync_lbs(input_data, ovnsb_txn, &data->datapaths, &data->lbs); >>>>>> sync_address_sets(input_data, ovnsb_txn, &data->datapaths); >>>>> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
