Re: [ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
On Tue, Nov 12, 2019 at 1:56 AM Han Zhou wrote:
>
>
>
> On Mon, Nov 11, 2019 at 11:03 AM Dumitru Ceara wrote:
> >
> > On Mon, Nov 11, 2019 at 6:11 PM Han Zhou 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 wrote:
> > > >
> > > > On Sat, Nov 9, 2019 at 8:35 AM Han Zhou wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara
> > > > > 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
> > > > > > Signed-off-by: Dumitru Ceara
> > > > > >
> > > > > > ---
> > > > > > 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=, arp.op == 1" action:
> > > > "outport=MC_FLOOD"
> > > >
> > > > But instead we would hit:
> > > > priority=75 match: "arp.op == 1 && arp.tpa == > > > "outport=" 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?
> >
>
Re: [ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
On Mon, Nov 11, 2019 at 11:03 AM Dumitru Ceara wrote:
>
> On Mon, Nov 11, 2019 at 6:11 PM Han Zhou 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 wrote:
> > >
> > > On Sat, Nov 9, 2019 at 8:35 AM Han Zhou wrote:
> > > >
> > > >
> > > >
> > > > On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara
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
> > > > > Signed-off-by: Dumitru Ceara
> > > > >
> > > > > ---
> > > > > 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=, arp.op == 1" action:
> > > "outport=MC_FLOOD"
> > >
> > > But instead we would hit:
> > > priority=75 match: "arp.op == 1 && arp.tpa == > > "outport=" 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 generate
Re: [ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
On Mon, Nov 11, 2019 at 6:11 PM Han Zhou 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 wrote:
> >
> > On Sat, Nov 9, 2019 at 8:35 AM Han Zhou wrote:
> > >
> > >
> > >
> > > On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara 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
> > > > Signed-off-by: Dumitru Ceara
> > > >
> > > > ---
> > > > 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=, arp.op == 1" action:
> > "outport=MC_FLOOD"
> >
> > But instead we would hit:
> > priority=75 match: "arp.op == 1 && arp.tpa == > "outport=" 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 f
Re: [ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
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)
On Mon, Nov 11, 2019 at 5:32 AM Dumitru Ceara wrote:
>
> On Sat, Nov 9, 2019 at 8:35 AM Han Zhou wrote:
> >
> >
> >
> > On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara 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
> > > Signed-off-by: Dumitru Ceara
> > >
> > > ---
> > > 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=, arp.op == 1" action:
> "outport=MC_FLOOD"
>
> But instead we would hit:
> priority=75 match: "arp.op == 1 && arp.tpa == "outport=" 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?
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?
>
> >
> > 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
Re: [ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
On Sat, Nov 9, 2019 at 8:35 AM Han Zhou wrote:
>
>
>
> On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara 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
> > Signed-off-by: Dumitru Ceara
> >
> > ---
> > 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=, arp.op == 1" action:
"outport=MC_FLOOD"
But instead we would hit:
priority=75 match: "arp.op == 1 && arp.tpa == " and never send flood the packet out on
the second HV.
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.
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
Re: [ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
On Fri, Nov 8, 2019 at 6:38 AM Dumitru Ceara 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 > Signed-off-by: Dumitru Ceara > > --- > 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? 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
[ovs-dev] [PATCH ovn v6] ovn-northd: Limit ARP/ND broadcast domain whenever possible.
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
Signed-off-by: Dumitru Ceara
---
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.
v5: Address Numan's comments: update comments & make autotest more
robust.
v4: Rebase.
v3: Properly deal with VXLAN traffic. Address review comments from
Numan (add autotests). Fix function get_router_load_balancer_ips.
Rebase -> deal with IPv6 NAT too.
v2: Move ARP broadcast domain limiting to table S_SWITCH_IN_L2_LKUP to
address localnet ports too.
---
controller/physical.c| 2 +
include/ovn/logical-fields.h | 4 +
northd/ovn-northd.8.xml | 16 ++
northd/ovn-northd.c | 337 ---
ovn-architecture.7.xml | 19 +++
tests/ovn.at | 307 +--
6 files changed, 591 insertions(+), 94 deletions(-)
diff --git a/controller/physical.c b/controller/physical.c
index 500d419..751dbbf 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -567,6 +567,7 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
if (tag) {
ofpact_put_STRIP_VLAN(ofpacts_p);
+put_load(1, MFF_LOG_FLAGS, MLF_RCV_FROM_VLAN_BIT, 1, ofpacts_p);
}
load_logical_ingress_metadata(localnet_port, &zone_ids, ofpacts_p);
replace_mac = ofpact_put_SET_ETH_SRC(ofpacts_p);
@@ -1124,6 +1125,7 @@ consider_port_binding(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
ofpacts_p);
}
ofpact_put_STRIP_VLAN(ofpacts_p);
+put_load(1, MFF_LOG_FLAGS, MLF_RCV_FROM_VLAN_BIT, 1, ofpacts_p);
}
/* Remember the size with just strip vlan added so far,
diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h
index 9b7c34f..15e0d1e 100644
--- a/include/ovn/logical-fields.h
+++ b/include/ovn/logical-fields.h
@@ -57,6 +57,7 @@ enum mff_log_flags_bits {
MLF_LOCAL_ONLY_BIT = 4,
MLF_NESTED_CONTAINER_BIT = 5,
MLF_LOOKUP_MAC_BIT = 6,
+MLF_RCV_FROM_VLAN_BIT = 7,
};
/* MFF_LOG_FLAGS_REG flag assignments */
@@ -88,6 +89,9 @@ enum mff_log_flags {
/* Indicate that the lookup in the mac binding table was successful. */
MLF_LOOKUP_MAC = (1 << MLF_LOOKUP_MAC_BIT),
+
+/* Indicate that a packet was received on a VLAN backed network. */
+MLF_RCV_FROM_VLAN = (1 << MLF_RCV_FROM_VLAN_BIT),
};
/* OVN logical fields
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 0a33dcd..6fbb3ab 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1005,6 +1005,22 @@ output;
+Priority-80 flows for each port connected to a logical router
+matching self originated GARP/ARP request/ND packets. These packets
+are flooded to the MC_FLOOD which contains all logical
+ports.
+
+
+
+Priority-75 flows for each IP address/VIP/NAT address owned by a
+router port connected to the switch. These flows match ARP requests
+and ND packets for the specific IP addresses. Matched packets are
+forwarded in the L3 domain only to the router that owns the IP
+address and flooded in the L2 domain on all ports except patch
+ports connected to logical routers.
+
+
+
A priority-70 flow that outputs all packets with an Ethernet broadcast
or multicast eth.dst to the MC_FLOOD
multicast group.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 2f0f501..9de3d20 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -210,6 +210,9 @@ enum ovn_stage {
#define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[4]"
