| Message ID | 20220831130635.146270-1-odivlad@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] northd: don't add drop lflow if LB VIP matches LRP IP | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Please, add this tag before applying the patch: Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-August/052021.html Regards, Vladislav Odintsov > On 31 Aug 2022, at 16:06, Vladislav Odintsov <odivlad@gmail.com> wrote: > > If it is needed to create Load Balancer within LR with VIP, which matches > any of LR's LRP IP, there is no need to create SNAT entry. Now such > traffic destined to LRP IP is not dropped. > > With this patch a drop lflow with match=(ipX.dst == {IP}) is not added to > lr_in_ip_input stage if LRP IP matches associated with this LR LB VIP. > > Tests are added as well. > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > --- > NEWS | 3 ++ > northd/northd.c | 10 ++++-- > tests/ovn-northd.at | 86 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 96 insertions(+), 3 deletions(-) > > diff --git a/NEWS b/NEWS > index 0f12b6abf..98dc17dd3 100644 > --- a/NEWS > +++ b/NEWS > @@ -18,6 +18,9 @@ Post v22.06.0 > - Added MAC binding aging mechanism, that is disabled by default. > It can be enabled per logical router with option > "mac_binding_age_threshold". > + - If it is needed to create Load Balancer within LR with VIP, which matches > + any of LR's LRP IP, there is no need to create SNAT entry. Now such > + traffic destined to LRP IP is not dropped. > > OVN v22.06.0 - 03 Jun 2022 > -------------------------- > diff --git a/northd/northd.c b/northd/northd.c > index 7e2681865..338091728 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -10664,7 +10664,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, > const char *ip = op->lrp_networks.ipv4_addrs[i].addr_s; > > bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); > - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); > + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v4, ip); > + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || > + router_ip_in_lb_ips)); > > if (drop_router_ip) { > ds_put_format(&match_ips, "%s, ", ip); > @@ -10690,7 +10692,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, > const char *ip = op->lrp_networks.ipv6_addrs[i].addr_s; > > bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); > - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); > + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v6, ip); > + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || > + router_ip_in_lb_ips)); > > if (drop_router_ip) { > ds_put_format(&match_ips, "%s, ", ip); > @@ -12865,7 +12869,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, > * also a SNAT IP. Those are dropped later, in stage > * "lr_in_arp_resolve", if unSNAT was unsuccessful. > * > - * If op->pd->lb_force_snat_router_ip is true, it means the IP of the > + * If op->od->lb_force_snat_router_ip is true, it means the IP of the > * router port is also SNAT IP. > * > * Priority 60. > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 157f9f60c..a60b3b0a9 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -1499,6 +1499,92 @@ AT_CHECK([grep "lr_in_unsnat" sbflows | sort], [0], [dnl > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([LRP same IP as VIP or SNAT]) > +ovn_start > + > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl lrp-add lr0 lr0-public 00:00:00:00:00:10 192.168.0.1/24 2000::1/64 > +check ovn-nbctl --wait=sb lrp-add lr0 lr0-join 00:00:00:00:00:20 10.10.0.1/24 192.168.1.1/24 > + > +ovn-sbctl dump-flows lr0 > sbflows > +AT_CAPTURE_FILE([sbflows]) > + > +# There should be drop lflows for all IPs of both LRPs > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > +]) > + > +# create SNAT with external IP equal to LRP's IP > +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 192.168.0.1 10.10.0.0/24 > + > +ovn-sbctl dump-flows lr0 > sbflows > +AT_CAPTURE_FILE([sbflows]) > + > +# There should be no drop lflow for 192.168.0.1 > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > +]) > + > +check ovn-nbctl lr-nat-del lr0 > + > +# create SNAT with external IPv6 equal to LRP's IPv6 > +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 2000::1 2aaa::/64 > +ovn-nbctl show lr0 > + > +ovn-sbctl dump-flows lr0 > sbflows > +AT_CAPTURE_FILE([sbflows]) > + > +# There should be no drop lflow for 2000::1 > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > +]) > + > +check ovn-nbctl lr-nat-del lr0 > + > +# create LB with VIP equal to LRP's IP and assign it to LR > +check ovn-nbctl lb-add lb1 "192.168.1.1:8080" "10.0.0.4:8080" > +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 > + > +ovn-sbctl dump-flows lr0 > sbflows > +AT_CAPTURE_FILE([sbflows]) > + > +# There should be no drop lflow for 192.168.1.1 > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > +]) > + > +check ovn-nbctl lb-del lb1 > + > +# create LB with VIP equal to LRP's IPv6 and assign it to LR > +check ovn-nbctl lb-add lb1 [[2000::1]]:8080 [[2aaa::10]]:8080 > +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 > + > +ovn-sbctl dump-flows lr0 > sbflows > +AT_CAPTURE_FILE([sbflows]) > + > +# There should be no drop lflow for 2000::1 > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > +]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([DNAT force snat IP]) > ovn_start > -- > 2.36.1 >
On Wed, Aug 31, 2022 at 9:17 AM Vladislav Odintsov <odivlad@gmail.com> wrote: > > Please, add this tag before applying the patch: > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-August/052021.html I'm sorry. I missed adding this tag before applying the patch. I also backported to branch-22.09 as it can be considered a bug fix. Thanks Numan > > Regards, > Vladislav Odintsov > > > On 31 Aug 2022, at 16:06, Vladislav Odintsov <odivlad@gmail.com> wrote: > > > > If it is needed to create Load Balancer within LR with VIP, which matches > > any of LR's LRP IP, there is no need to create SNAT entry. Now such > > traffic destined to LRP IP is not dropped. > > > > With this patch a drop lflow with match=(ipX.dst == {IP}) is not added to > > lr_in_ip_input stage if LRP IP matches associated with this LR LB VIP. > > > > Tests are added as well. > > > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > > --- > > NEWS | 3 ++ > > northd/northd.c | 10 ++++-- > > tests/ovn-northd.at | 86 +++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 96 insertions(+), 3 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 0f12b6abf..98dc17dd3 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -18,6 +18,9 @@ Post v22.06.0 > > - Added MAC binding aging mechanism, that is disabled by default. > > It can be enabled per logical router with option > > "mac_binding_age_threshold". > > + - If it is needed to create Load Balancer within LR with VIP, which matches > > + any of LR's LRP IP, there is no need to create SNAT entry. Now such > > + traffic destined to LRP IP is not dropped. > > > > OVN v22.06.0 - 03 Jun 2022 > > -------------------------- > > diff --git a/northd/northd.c b/northd/northd.c > > index 7e2681865..338091728 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -10664,7 +10664,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, > > const char *ip = op->lrp_networks.ipv4_addrs[i].addr_s; > > > > bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); > > - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); > > + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v4, ip); > > + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || > > + router_ip_in_lb_ips)); > > > > if (drop_router_ip) { > > ds_put_format(&match_ips, "%s, ", ip); > > @@ -10690,7 +10692,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, > > const char *ip = op->lrp_networks.ipv6_addrs[i].addr_s; > > > > bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); > > - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); > > + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v6, ip); > > + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || > > + router_ip_in_lb_ips)); > > > > if (drop_router_ip) { > > ds_put_format(&match_ips, "%s, ", ip); > > @@ -12865,7 +12869,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, > > * also a SNAT IP. Those are dropped later, in stage > > * "lr_in_arp_resolve", if unSNAT was unsuccessful. > > * > > - * If op->pd->lb_force_snat_router_ip is true, it means the IP of the > > + * If op->od->lb_force_snat_router_ip is true, it means the IP of the > > * router port is also SNAT IP. > > * > > * Priority 60. > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 157f9f60c..a60b3b0a9 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -1499,6 +1499,92 @@ AT_CHECK([grep "lr_in_unsnat" sbflows | sort], [0], [dnl > > AT_CLEANUP > > ]) > > > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([LRP same IP as VIP or SNAT]) > > +ovn_start > > + > > +check ovn-nbctl lr-add lr0 > > +check ovn-nbctl lrp-add lr0 lr0-public 00:00:00:00:00:10 192.168.0.1/24 2000::1/64 > > +check ovn-nbctl --wait=sb lrp-add lr0 lr0-join 00:00:00:00:00:20 10.10.0.1/24 192.168.1.1/24 > > + > > +ovn-sbctl dump-flows lr0 > sbflows > > +AT_CAPTURE_FILE([sbflows]) > > + > > +# There should be drop lflows for all IPs of both LRPs > > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > > +]) > > + > > +# create SNAT with external IP equal to LRP's IP > > +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 192.168.0.1 10.10.0.0/24 > > + > > +ovn-sbctl dump-flows lr0 > sbflows > > +AT_CAPTURE_FILE([sbflows]) > > + > > +# There should be no drop lflow for 192.168.0.1 > > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > > +]) > > + > > +check ovn-nbctl lr-nat-del lr0 > > + > > +# create SNAT with external IPv6 equal to LRP's IPv6 > > +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 2000::1 2aaa::/64 > > +ovn-nbctl show lr0 > > + > > +ovn-sbctl dump-flows lr0 > sbflows > > +AT_CAPTURE_FILE([sbflows]) > > + > > +# There should be no drop lflow for 2000::1 > > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > > +]) > > + > > +check ovn-nbctl lr-nat-del lr0 > > + > > +# create LB with VIP equal to LRP's IP and assign it to LR > > +check ovn-nbctl lb-add lb1 "192.168.1.1:8080" "10.0.0.4:8080" > > +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 > > + > > +ovn-sbctl dump-flows lr0 > sbflows > > +AT_CAPTURE_FILE([sbflows]) > > + > > +# There should be no drop lflow for 192.168.1.1 > > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > > +]) > > + > > +check ovn-nbctl lb-del lb1 > > + > > +# create LB with VIP equal to LRP's IPv6 and assign it to LR > > +check ovn-nbctl lb-add lb1 [[2000::1]]:8080 [[2aaa::10]]:8080 > > +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 > > + > > +ovn-sbctl dump-flows lr0 > sbflows > > +AT_CAPTURE_FILE([sbflows]) > > + > > +# There should be no drop lflow for 2000::1 > > +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) > > + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) > > +]) > > + > > +AT_CLEANUP > > +]) > > + > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([DNAT force snat IP]) > > ovn_start > > -- > > 2.36.1 > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks Numan. Regards, Vladislav Odintsov > On 6 Sep 2022, at 22:44, Numan Siddique <numans@ovn.org> wrote: > > On Wed, Aug 31, 2022 at 9:17 AM Vladislav Odintsov <odivlad@gmail.com> wrote: >> >> Please, add this tag before applying the patch: >> >> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-August/052021.html > > I'm sorry. I missed adding this tag before applying the patch. > > I also backported to branch-22.09 as it can be considered a bug fix. > > Thanks > Numan > >> >> Regards, >> Vladislav Odintsov >> >>> On 31 Aug 2022, at 16:06, Vladislav Odintsov <odivlad@gmail.com> wrote: >>> >>> If it is needed to create Load Balancer within LR with VIP, which matches >>> any of LR's LRP IP, there is no need to create SNAT entry. Now such >>> traffic destined to LRP IP is not dropped. >>> >>> With this patch a drop lflow with match=(ipX.dst == {IP}) is not added to >>> lr_in_ip_input stage if LRP IP matches associated with this LR LB VIP. >>> >>> Tests are added as well. >>> >>> Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> >>> --- >>> NEWS | 3 ++ >>> northd/northd.c | 10 ++++-- >>> tests/ovn-northd.at | 86 +++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 96 insertions(+), 3 deletions(-) >>> >>> diff --git a/NEWS b/NEWS >>> index 0f12b6abf..98dc17dd3 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -18,6 +18,9 @@ Post v22.06.0 >>> - Added MAC binding aging mechanism, that is disabled by default. >>> It can be enabled per logical router with option >>> "mac_binding_age_threshold". >>> + - If it is needed to create Load Balancer within LR with VIP, which matches >>> + any of LR's LRP IP, there is no need to create SNAT entry. Now such >>> + traffic destined to LRP IP is not dropped. >>> >>> OVN v22.06.0 - 03 Jun 2022 >>> -------------------------- >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 7e2681865..338091728 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -10664,7 +10664,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, >>> const char *ip = op->lrp_networks.ipv4_addrs[i].addr_s; >>> >>> bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); >>> - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); >>> + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v4, ip); >>> + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || >>> + router_ip_in_lb_ips)); >>> >>> if (drop_router_ip) { >>> ds_put_format(&match_ips, "%s, ", ip); >>> @@ -10690,7 +10692,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, >>> const char *ip = op->lrp_networks.ipv6_addrs[i].addr_s; >>> >>> bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); >>> - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); >>> + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v6, ip); >>> + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || >>> + router_ip_in_lb_ips)); >>> >>> if (drop_router_ip) { >>> ds_put_format(&match_ips, "%s, ", ip); >>> @@ -12865,7 +12869,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, >>> * also a SNAT IP. Those are dropped later, in stage >>> * "lr_in_arp_resolve", if unSNAT was unsuccessful. >>> * >>> - * If op->pd->lb_force_snat_router_ip is true, it means the IP of the >>> + * If op->od->lb_force_snat_router_ip is true, it means the IP of the >>> * router port is also SNAT IP. >>> * >>> * Priority 60. >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >>> index 157f9f60c..a60b3b0a9 100644 >>> --- a/tests/ovn-northd.at >>> +++ b/tests/ovn-northd.at >>> @@ -1499,6 +1499,92 @@ AT_CHECK([grep "lr_in_unsnat" sbflows | sort], [0], [dnl >>> AT_CLEANUP >>> ]) >>> >>> +OVN_FOR_EACH_NORTHD([ >>> +AT_SETUP([LRP same IP as VIP or SNAT]) >>> +ovn_start >>> + >>> +check ovn-nbctl lr-add lr0 >>> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:00:00:00:10 192.168.0.1/24 2000::1/64 >>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-join 00:00:00:00:00:20 10.10.0.1/24 192.168.1.1/24 >>> + >>> +ovn-sbctl dump-flows lr0 > sbflows >>> +AT_CAPTURE_FILE([sbflows]) >>> + >>> +# There should be drop lflows for all IPs of both LRPs >>> +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) >>> +]) >>> + >>> +# create SNAT with external IP equal to LRP's IP >>> +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 192.168.0.1 10.10.0.0/24 >>> + >>> +ovn-sbctl dump-flows lr0 > sbflows >>> +AT_CAPTURE_FILE([sbflows]) >>> + >>> +# There should be no drop lflow for 192.168.0.1 >>> +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) >>> +]) >>> + >>> +check ovn-nbctl lr-nat-del lr0 >>> + >>> +# create SNAT with external IPv6 equal to LRP's IPv6 >>> +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 2000::1 2aaa::/64 >>> +ovn-nbctl show lr0 >>> + >>> +ovn-sbctl dump-flows lr0 > sbflows >>> +AT_CAPTURE_FILE([sbflows]) >>> + >>> +# There should be no drop lflow for 2000::1 >>> +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) >>> +]) >>> + >>> +check ovn-nbctl lr-nat-del lr0 >>> + >>> +# create LB with VIP equal to LRP's IP and assign it to LR >>> +check ovn-nbctl lb-add lb1 "192.168.1.1:8080" "10.0.0.4:8080" >>> +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 >>> + >>> +ovn-sbctl dump-flows lr0 > sbflows >>> +AT_CAPTURE_FILE([sbflows]) >>> + >>> +# There should be no drop lflow for 192.168.1.1 >>> +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) >>> +]) >>> + >>> +check ovn-nbctl lb-del lb1 >>> + >>> +# create LB with VIP equal to LRP's IPv6 and assign it to LR >>> +check ovn-nbctl lb-add lb1 [[2000::1]]:8080 [[2aaa::10]]:8080 >>> +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 >>> + >>> +ovn-sbctl dump-flows lr0 > sbflows >>> +AT_CAPTURE_FILE([sbflows]) >>> + >>> +# There should be no drop lflow for 2000::1 >>> +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) >>> + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) >>> +]) >>> + >>> +AT_CLEANUP >>> +]) >>> + >>> OVN_FOR_EACH_NORTHD([ >>> AT_SETUP([DNAT force snat IP]) >>> ovn_start >>> -- >>> 2.36.1 >>> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/NEWS b/NEWS index 0f12b6abf..98dc17dd3 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,9 @@ Post v22.06.0 - Added MAC binding aging mechanism, that is disabled by default. It can be enabled per logical router with option "mac_binding_age_threshold". + - If it is needed to create Load Balancer within LR with VIP, which matches + any of LR's LRP IP, there is no need to create SNAT entry. Now such + traffic destined to LRP IP is not dropped. OVN v22.06.0 - 03 Jun 2022 -------------------------- diff --git a/northd/northd.c b/northd/northd.c index 7e2681865..338091728 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -10664,7 +10664,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, const char *ip = op->lrp_networks.ipv4_addrs[i].addr_s; bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v4, ip); + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || + router_ip_in_lb_ips)); if (drop_router_ip) { ds_put_format(&match_ips, "%s, ", ip); @@ -10690,7 +10692,9 @@ build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, const char *ip = op->lrp_networks.ipv6_addrs[i].addr_s; bool router_ip_in_snat_ips = !!shash_find(&op->od->snat_ips, ip); - bool drop_router_ip = (drop_snat_ip == router_ip_in_snat_ips); + bool router_ip_in_lb_ips = !!sset_find(&op->od->lb_ips_v6, ip); + bool drop_router_ip = (drop_snat_ip == (router_ip_in_snat_ips || + router_ip_in_lb_ips)); if (drop_router_ip) { ds_put_format(&match_ips, "%s, ", ip); @@ -12865,7 +12869,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, * also a SNAT IP. Those are dropped later, in stage * "lr_in_arp_resolve", if unSNAT was unsuccessful. * - * If op->pd->lb_force_snat_router_ip is true, it means the IP of the + * If op->od->lb_force_snat_router_ip is true, it means the IP of the * router port is also SNAT IP. * * Priority 60. diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 157f9f60c..a60b3b0a9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1499,6 +1499,92 @@ AT_CHECK([grep "lr_in_unsnat" sbflows | sort], [0], [dnl AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([LRP same IP as VIP or SNAT]) +ovn_start + +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-public 00:00:00:00:00:10 192.168.0.1/24 2000::1/64 +check ovn-nbctl --wait=sb lrp-add lr0 lr0-join 00:00:00:00:00:20 10.10.0.1/24 192.168.1.1/24 + +ovn-sbctl dump-flows lr0 > sbflows +AT_CAPTURE_FILE([sbflows]) + +# There should be drop lflows for all IPs of both LRPs +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) +]) + +# create SNAT with external IP equal to LRP's IP +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 192.168.0.1 10.10.0.0/24 + +ovn-sbctl dump-flows lr0 > sbflows +AT_CAPTURE_FILE([sbflows]) + +# There should be no drop lflow for 192.168.0.1 +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) +]) + +check ovn-nbctl lr-nat-del lr0 + +# create SNAT with external IPv6 equal to LRP's IPv6 +check ovn-nbctl --wait=sb lr-nat-add lr0 snat 2000::1 2aaa::/64 +ovn-nbctl show lr0 + +ovn-sbctl dump-flows lr0 > sbflows +AT_CAPTURE_FILE([sbflows]) + +# There should be no drop lflow for 2000::1 +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) +]) + +check ovn-nbctl lr-nat-del lr0 + +# create LB with VIP equal to LRP's IP and assign it to LR +check ovn-nbctl lb-add lb1 "192.168.1.1:8080" "10.0.0.4:8080" +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 + +ovn-sbctl dump-flows lr0 > sbflows +AT_CAPTURE_FILE([sbflows]) + +# There should be no drop lflow for 192.168.1.1 +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {2000::1, fe80::200:ff:fe00:10}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) +]) + +check ovn-nbctl lb-del lb1 + +# create LB with VIP equal to LRP's IPv6 and assign it to LR +check ovn-nbctl lb-add lb1 [[2000::1]]:8080 [[2aaa::10]]:8080 +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 + +ovn-sbctl dump-flows lr0 > sbflows +AT_CAPTURE_FILE([sbflows]) + +# There should be no drop lflow for 2000::1 +AT_CHECK([grep "lr_in_ip_input" sbflows | grep 'ip.\.dst == {' | grep drop | sed 's/table=../table=??/g' | sort], [0], [dnl + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.10.0.1, 192.168.1.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {192.168.0.1}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:10}), action=(drop;) + table=??(lr_in_ip_input ), priority=60 , match=(ip6.dst == {fe80::200:ff:fe00:20}), action=(drop;) +]) + +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([DNAT force snat IP]) ovn_start
If it is needed to create Load Balancer within LR with VIP, which matches any of LR's LRP IP, there is no need to create SNAT entry. Now such traffic destined to LRP IP is not dropped. With this patch a drop lflow with match=(ipX.dst == {IP}) is not added to lr_in_ip_input stage if LRP IP matches associated with this LR LB VIP. Tests are added as well. Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> --- NEWS | 3 ++ northd/northd.c | 10 ++++-- tests/ovn-northd.at | 86 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-)