[ovs-dev] [PATCH ovn v2] ovn-northd: Drop IP packets destined to router owned IPs (after NAT).

Dumitru Ceara dceara at redhat.com
Tue Sep 8 11:24:31 UTC 2020


On 9/8/20 12:58 PM, Numan Siddique wrote:
> On Tue, Sep 8, 2020 at 2:13 PM Dumitru Ceara <dceara at redhat.com> wrote:
>>
>> OVN was dropping IP packets destined to IPs owned by logical routers but
>> only if those IPs are not used for SNAT rules. However, if a packet
>> doesn't match an existing NAT session and its destination is still a
>> router owned IP, it can be safely dropped. Otherwise it will trigger an
>> unnecessary packet-in in stage lr_in_arp_request.
>>
>> To achieve that we add flows that drop traffic to router owned IPs in
>> table lr_in_arp_resolve.
>>
>> Reported-by: Tim Rozet <trozet at redhat.com>
>> Reported-at: https://bugzilla.redhat.com/1876174
>> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> 
> 
> 
> Hi Dumitru,
> 
> Thanks for the fix.
> 
> I have few comments.
> 
> Suppose we have the below lr and NAT entries
> 
> router 392b19fe-adf9-4b3d-bf43-040d12240e54 (lr0)
>     port lr0-sw0
>         mac: "00:00:00:00:ff:01"
>         networks: ["10.0.0.1/24"]
>     port lr0-sw1
>         mac: "00:00:00:00:ff:02"
>         networks: ["20.0.0.1/24"]
>     port lr0-public
>         mac: "00:00:20:20:12:13"
>         networks: ["172.168.0.100/24"]
>         gateway chassis: [chassis-1]
>     nat 4c26f3f0-0ae0-4c18-9a1e-9d2751389270
>         external ip: "172.168.0.110"
>         logical ip: "10.0.0.3"
>         type: "dnat_and_snat"
>     nat b9296dc8-ac3a-427a-a97a-a94bf16214b2
>         external ip: "172.168.0.120"
>         logical ip: "20.0.0.3"
>         type: "dnat_and_snat"
>     nat da28129c-4e81-4e20-865a-768bd88e5a26
>         external ip: "172.168.0.100"
>         logical ip: "10.0.0.0/24"
>         type: "snat"
> 
> I see the below lflows added in lr_in_ip_input to drop the pkts for
> router port IPs
> 
> table=3 (lr_in_ip_input     ), priority=60   , match=(ip4.dst ==
> {10.0.0.1} || ip6.dst == {fe80::200:ff:fe00:ff01}), action=(drop;)
> table=3 (lr_in_ip_input     ), priority=60   , match=(ip4.dst ==
> {20.0.0.1} || ip6.dst == {fe80::200:ff:fe00:ff02}), action=(drop;)
> 
> So I think there is no need to add the lflows in lr_in_arp_resolve to
> drop for 10.0.0.1 and 20.0.0.1 again.
> 
> I think it's sufficient to add lflows in lr_in_arp_resolve to drop the
> packet only for packets destined to the router ips which have NAT
> entries.
> 

Hi Numan,

Thanks for the review.

You're right we could try to optimize a bit the number of flows.

However, I didn't want to duplicate the code that builds snat_ips [0]
and I thought it complicates the already long build_lrouter_flows()
function if I add flows to stage LR_IN_ARP_RESOLVE in the same place
where we add flows for LR_IN_IP_INPUT.

Also, the number of IP addresses per router port is usually low so the
number of redundant logical flows in lr_in_arp_resolve would be low too.

If you really have a strong preference about this, I can try to change
it though.

Regards,
Dumitru

[0] https://github.com/ovn-org/ovn/blob/master/northd/ovn-northd.c#L9015



More information about the dev mailing list