[ovs-dev] [PATCH] OVN: Fix learning of neighbors from ARP requests.

[Dumitru Ceara]

On Tue, Jul 23, 2019 at 4:26 PM Han Zhou <zhouhan at gmail.com> wrote:
>
>
>
> On Tue, Jul 23, 2019 at 4:05 AM Dumitru Ceara <dceara at redhat.com> wrote:
> >
> > Add a restriction on the target protocol address to match the configured
> > subnet. All other ARP packets are invalid in this context.
> >
> > Reported-at: https://bugzilla.redhat.com/1729846
> > Reported-by: Haidong Li <haili at redhat.com>
> > CC: Han Zhou <zhouhan at gmail.com>
> > Fixes: b068454082f5 ("ovn-northd: Support learning neighbor from ARP request.")
> > Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> > ---
> >  ovn/northd/ovn-northd.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index eb6c47c..29fc726 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -6326,9 +6326,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> >              ds_clear(&match);
> >              ds_put_format(&match,
> > -                          "inport == %s && arp.spa == %s/%u && arp.op == 1",
> > +                          "inport == %s && arp.spa == %s/%u && "
> > +                          "arp.tpa == %s/%u && arp.op == 1",
> >                            op->json_key,
> >                            op->lrp_networks.ipv4_addrs[i].network_s,
> > +                          op->lrp_networks.ipv4_addrs[i].plen,
> > +                          op->lrp_networks.ipv4_addrs[i].network_s,
> >                            op->lrp_networks.ipv4_addrs[i].plen);
> >              if (op->od->l3dgw_port && op == op->od->l3dgw_port
> >                  && op->od->l3redirect_port) {
> > --
> > 1.8.3.1
> >
>
> Thanks for the fix. It looks good to me! Just to remind that for the CPU problem reported in the bug report, this patch may not completely fix it. For example, instead of using 0.0.0.0 as tpa, the test client can still flood ARP requests using IP in the subnet. It can also flood ARP response packets. ARP rate limiting would be needed to solve the problem.

Hi Han,

Thanks for reviewing!

I guess there are two cases:
1. valid ARP packets for IPs in the subnet which should reach the
controller and are currently not rate limited
2. "invalid" ARP packets for IPs outside the subnet: this patch fixes
the ARP requests but I need to fix the ARP replies. I'll send a v2 to
address that part too.

For point 1 above I think the problem is common for all control
packets that need to go to controller (e.g., IGMP too). We currently
don't rate limit anything and we should have a control plane
protection policy in place but i guess it's a bit outside the scope of
this fix.

Regards,
Dumitru

>
> Acked-by: Han Zhou <hzhou8 at ebay.com>