[ovs-dev] [PATCH ovn v2] OVN: Fix learning of neighbors from ARP/ND packets.

Han Zhou zhouhan at gmail.com
Sun Aug 4 23:40:58 UTC 2019


On Mon, Jul 29, 2019 at 2:30 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> Add a restriction on the target protocol addresses to match the
> configured subnets. All other ARP/ND packets are invalid in this
> context.
>
> One exception is for ARP replies that are received for route next-hops
> that are only reachable via a port but can't be directly resolved
> through route lookups. Such support was introduced by commit:
>
> 6b785fd8fe29 ("ovn-util: Allow /32 IP addresses for router ports.")
>
> Reported-at: https://bugzilla.redhat.com/1729846
> Reported-by: Haidong Li <haili at redhat.com>
> CC: Han Zhou <zhouhan at gmail.com>
> CC: Guru Shetty <guru at ovn.org>
> Fixes: b068454082f5 ("ovn-northd: Support learning neighbor from ARP
request.")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>

Hi Dumitru,

Sorry for my slow response, and thanks a lot for revising the patch for a
bigger scope of validations. However, the exception of /32 address makes me
thinking more about this patch. If ARP replies is allowed from any nexthop
for a LR port with /32, at least ARP request for GARP purpose should also
be allowed. But before asking for a v3, I'd hold on to rethink the purpose
of this patch.

The nexthop specific flows are now from static routes. What if OVN support
dynamical routing protocols in the future? Of course we can then take those
dynamic nexthops into allowed peers. But then I am thinking what is the
real benefit of all these restrictions? Why can't we just have simpler
logic to handle all these situations without validation? I think the major
benefit of the validation is to avoid handling the noise ARP/NDs when
multiple subnets shares same L2, but most cases it is really not a big
deal, right? For the CPU problem caused by ARP flooding as mentioned by the
bug report, it is a real problem, but this patch seems not really helpful
for that, because the attacker can just trigger the same CPU problem with
*valid* packets. So I am not sure if the benefit of the change is worth the
complexity it introduced. Please share your thought and correct me if I
missed something.

Thanks,
Han


More information about the dev mailing list