> > > > 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.
IIUC what you mean here, this approach will be helpful just if pod A and pod B are on the same logical switch but they are running on two different hypervisors, right? What if pod A and pod B are on the same logical switch and they are running on the same HV? Regards, Lorenzo > > > 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
