[ovs-dev] [PATCH ovn] Learn the mac binding only if required
zhouhan at gmail.com
Thu Sep 12 07:04:01 UTC 2019
On Wed, Sep 11, 2019 at 1:05 PM <nusiddiq at redhat.com> wrote:
> From: Numan Siddique <nusiddiq at redhat.com>
> OVN has the actions - put_arp and put_nd to learn the mac bindings from
> ARP/ND packets. These actions update the Southbound MAC_Binding table.
> These actions translates to controller actions. Whenever pinctrl thread
> receives such packets, it wakes up the main ovn-controller thread.
> If the MAC_Binding table is already upto date, this results
> in unnecessary CPU cyles. There are some security implications as well.
> A rogue VM can flood broadcast ARP request/reply packets and this
> could cause DoS issues. A physical switch may send periodic GARPs
> and these packets hit ovn-controllers.
> This patch solves these problems by learning the mac bindings only if
> required. There is no need to apply the put_arp/put_nd action if the
> Southbound MAC_Binding row is upto date.
> A new action - lookup_arp and lookup_nd is added which looks up the
> IP, MAC pair in the mac_binding table and updates the eth.dst if
> the entry is present, else eth.dst is set to 00:00:00:00:00:00.
> ovn-northd adds 2 new stages - lookup_arp and put_arp before ip_input
> in the router ingress pipeline.
> The logical flows looks something like:
> table=1 (lr_in_lookup_arp), priority=100 , match=(arp),
> action=(xxreg1[0..47] = eth.dst;
> lookup_arp(inport, arp.spa, arp.sha);
> xxreg0[0..47] = eth.dst; eth.dst = xxreg1[0..47]; next;)
> table=1 (lr_in_lookup_arp), priority=0 , match=(1), action=(next;)
> table=2 (lr_in_put_arp ), priority=100 ,
> match=(arp.op == 2 && xxreg0[0..47] == 00:00:00:00:00:00),
> action=(put_arp(inport, arp.spa, arp.sha);)
> table=2 (lr_in_put_arp ), priority=90 , match=(arp.op == 2),
> table=2 (lr_in_put_arp ), priority=0 , match=(1), action=(next;)
> The lflow module of ovn-controller adds OF flows in table 31
> for each mac_binding entry with the match reg0 = ip && eth.src = mac with
> the action - eth.dst = mac
> This patch should also address the issue reported in 'Reported-at'
> Reported-at: https://bugzilla.redhat.com/1729846
> Reported-by: Haidong Li <haili at redhat.com>
> CC: Han ZHou <hzhou8 at ebay.com>
> CC: Dumitru Ceara <dceara at redhat.com>
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
Thanks Numan for optimizing this. First of all, this approach looks good to
me. I haven't finished the review yet, but have a quick question. Why the
action lookup_arp() need to store the mac in eth.dst? I thought it would be
more straightforward if it sets 1 in a register if matched, and 0 if no
match. Is there any limit of OpenFlow for doing that?
More information about the dev