On 18 July 2016 at 05:22, Chandra S Vejendla <csvej...@us.ibm.com> wrote:
> When router ip is used as SNAT IP, traffic destined to router
> ip should not be dropped
>
Thank you for the fix. You will need to add your Signed-off-by. Can you
also add a "Fixes:" tag in commit message. Since this is a regression, I
wonder whether we should add a simple unit test that looks at the generated
logical flows to make sure that there is no "drop" for the SNAT IP address.
I also wonder, whether we should disable ICMP response to these SNAT IP
addresses from the router? Don't you see issues with that? I presume you
will have a situation where the ICMP response flow added by the router will
override the SNAT flow and the router will respond to ICMP instead of the
logical port.
> ---
> ovn/northd/ovn-northd.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 7ce509d..78c3a7d 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2399,11 +2399,16 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> ds_put_cstr(&match, "ip4.dst == {");
> bool has_drop_ips = false;
> for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> + bool nat_ip_is_router_ip = false;
> for (int j = 0; j < n_nat_ips; j++) {
> if (op->lrp_networks.ipv4_addrs[i].addr == nat_ips[j]) {
> - continue;
> + nat_ip_is_router_ip = true;
> + break;
> }
> }
> + if (nat_ip_is_router_ip) {
> + continue;
> + }
> ds_put_format(&match, "%s, ",
> op->lrp_networks.ipv4_addrs[i].addr_s);
> has_drop_ips = true;
> --
> 2.6.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
