On Mon, Nov 11, 2019 at 6:11 PM Han Zhou <zhou...@gmail.com> wrote: > > 1. Would there be problem for VLAN backed logical router use case, since a > chassis MAC is used as src MAC to send packets from router ports? > 2. How about checking if tpa == spa to make sure GARP is always flooded? (not > directly supported by OF)
This would've been nice to have in OF :) > > > On Mon, Nov 11, 2019 at 5:32 AM Dumitru Ceara <dce...@redhat.com> wrote: > > > > On Sat, Nov 9, 2019 at 8:35 AM Han Zhou <zhou...@gmail.com> wrote: > > > > > > > > > > > > On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara <dce...@redhat.com> wrote: > > > > > > > > ARP request and ND NS packets for router owned IPs were being > > > > flooded in the complete L2 domain (using the MC_FLOOD multicast group). > > > > However this creates a scaling issue in scenarios where aggregation > > > > logical switches are connected to more logical routers (~350). The > > > > logical pipelines of all routers would have to be executed before the > > > > packet is finally replied to by a single router, the owner of the IP > > > > address. > > > > > > > > This commit limits the broadcast domain by bypassing the L2 Lookup stage > > > > for ARP requests that will be replied by a single router. The packets > > > > are forwarded only to the router port that owns the target IP address. > > > > > > > > IPs that are owned by the routers and for which this fix applies are: > > > > - IP addresses configured on the router ports. > > > > - VIPs. > > > > - NAT IPs. > > > > > > > > This commit also fixes function get_router_load_balancer_ips() which > > > > was incorrectly returning a single address_family even though the > > > > IP set could contain a mix of IPv4 and IPv6 addresses. > > > > > > > > Reported-at: https://bugzilla.redhat.com/1756945 > > > > Reported-by: Anil Venkata <vkomm...@redhat.com> > > > > Signed-off-by: Dumitru Ceara <dce...@redhat.com> > > > > > > > > --- > > > > v6: > > > > - Address Han's comments: > > > > - remove flooding of ARPs targeting OVN owned IP addresses. > > > > - update ovn-architecture documentation. > > > > - rename ARP handling functions. > > > > - Adapt "ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR" autotest to take into > > > > account the new way of forwarding ARPs. > > > > - Also, properly deal with ARP packets on VLAN-backed networks. > > > > > > I am confused by this additional VLAN related change. VLAN is just > > > handled as bridged logical switch where a localnet port is used as > > > *inport*. It seems to me no difference from regular localnet port no > > > matter with or without VLAN tag. When an ARP request is going through the > > > ingress pipeline of the bridged logical switch, the logic of bypassing > > > the flooding added by this patch should just apply, right? It is also > > > very common scenario that the *aggregation switch* for the routers is an > > > external physical network backed by VLAN. I think the purpose of this > > > patch is to make sure scenario scale. Did I misunderstand anything here? > > > > Hi Han, > > > > The reason behind it was that with VLAN backed networks when packets > > move between hypervisors without going through geneve we rerun the > > ingress pipeline. That would mean that the flows I added for self > > originated (G)ARP packets won't be hit for ARP requests originated by > > ovn-controller on a remote hypervisor: > > > > priority=80 match: "inport=<patch-port-to-lr>, arp.op == 1" action: > > "outport=MC_FLOOD" > > > > But instead we would hit: > > priority=75 match: "arp.op == 1 && arp.tpa == <OVN-owned-IP" action: > > "outport=<patch-port-to-lr>" and never send flood the packet out on > > the second HV. > > > > Thanks for the explain. Now I understand the problem. > > > You're right, the way I added the check for all VLAN packets is not OK > > as we fall back to the old behavior too often. However, I'm not sure > > what the best option is. Do you think it's fine if I change the > > priority 80 flow to the following? > > > > priority=80 match: "eth.src={lrp.ea, lr.nat.external_macs} && arp.op > > == 1" action: "outport=MC_FLOOD" > > > > The idea would be to identify self originated ARP requests by matching > > the source mac instead of logical ingress port (as this might not be > > present). I tried it locally and it works fine but we do need to add > > more flows than before. > > > > Would this work when "ovn-chassis-mac-mappings" is configured for VLAN backed > logical routers? We might end up adding chassis unique MACs, too? I think it will work fine because when ovn-chassis-mac-mappings is configured we add an entry in table OFTABLE_PHY_TO_LOG to match on CHASSIS_MAC_TO_ROUTER_MAC_CONJID (comparing eth.src to all ovn-chassis-macs) to rewrite the eth.src address with that of the router port. > > Alternatively, I think we may change the priority 80 flow to match for each > OVN router owned IP: arp.tpa == IP && arp.spa == IP ... Would this ensure OVN > router generated ARPs are flooded? > I considered this too but because a router port can have an "unlimited" number of networks configured I decided to go for MAC to minimize the number of flows. Also, if we match on eth.src we can create a single priority 80 logical flow: priority=80 match: "eth.src={lrp.ea, lr.nat.external_macs} && arp.op == 1" action: "outport=MC_FLOOD" Whereas if we match on each of the IPs we need individual flows: priority=80 match: "arp.tpa == IP && arp.spa == IP && arp.op == 1" action: "outport=MC_FLOOD" In the end they translate to the same number of OF entries but we store less logical flows in the database. Do you think there's anything else missing? Thanks, Dumitru > > > > > > > > In addition, the below macro definition may be renamed to FLAGBIT_..., > > > because it is for the bits of MFF_LOG_FLAGS, which is one of the > > > pre-defined logical fields, instead of for the MFF_LOG_REG0-9 registers. > > > > > > > > +#define REGBIT_NOT_VXLAN "flags[1] == 0" > > > > +#define REGBIT_NOT_VLAN "flags[7] == 0" > > > > + > > > > > > The other part looks good to me. Thanks for simply the patch. > > > > > > Han > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
