| Message ID | 20250402104605.720034-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] northd: Fix network_id computation for IPv6 LRP networks. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 4/2/25 6:46 AM, Dumitru Ceara wrote: > If a router port has more than 16 networks configured we should set the > flags.network_id to 0 for network whose index is greater than 16. > > This issue has been reported by coverity. Trimmed report: > > *** CID 459645: Code maintainability issues (UNUSED_VALUE) > /northd/northd.c: 15062 in build_lrouter_network_id_flows() > 15056 "flags.network_id = 0.", op->json_key, > 15057 OVN_MAX_NETWORK_ID + 1, > 15058 op->lrp_networks.ipv6_addrs[i].addr_s, > 15059 op->lrp_networks.ipv6_addrs[i].plen); > 15060 network_id = 0; > 15061 } else { > >>> CID 459645: Code maintainability issues (UNUSED_VALUE) > >>> Assigning value from "i" to "network_id" here, but that stored > >> value is overwritten before it can be used. > 15062 network_id = i; > 15063 } > 15064 > 15065 ds_clear(match); > 15066 ds_clear(actions); > 15067 > > Fixes: af6e83707568 ("northd: Use next-hop network for SNAT when lb_force_snat_ip=router_ip.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > northd/northd.c | 3 ++- > tests/ovn-northd.at | 35 ++++++++++++++++++++++++++++++++--- > 2 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 471d9969bf..00bb738b3c 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -15070,7 +15070,8 @@ build_lrouter_network_id_flows( > op->lrp_networks.ipv6_addrs[i].addr_s, > op->lrp_networks.ipv6_addrs[i].plen); > > - ds_put_format(actions, "flags.network_id = %"PRIuSIZE"; next;", i); > + ds_put_format(actions, "flags.network_id = %"PRIuSIZE"; next;", > + network_id); > > ovn_lflow_add(lflows, op->od, S_ROUTER_IN_NETWORK_ID, 110, > ds_cstr(match), ds_cstr(actions), lflow_ref); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 95993e454c..8652e5574b 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -16781,7 +16781,9 @@ check ovn-nbctl lrp-add lr lrp-server 02:00:00:00:00:03 1.1.2.1/24 7.7.7.1/24 \ > 8.8.8.1/24 1.2.1.1/24 1.2.2.1/24 3.3.3.1/24 4.4.4.1/24 5.5.5.1/24 \ > 6.6.6.1/24 6.2.1.1/24 6.3.1.1/24 6.4.1.1/24 6.5.1.1/24 6.6.1.1/24 \ > 6.7.1.1/24 6.8.1.1/24 6.9.1.1/24 7.2.1.1/24 \ > - ff01::02 ff01::03 ff01::06 > + ff01::02 ff01::03 ff01::04 ff01::05 ff01::06 ff01::07 \ > + ff01::08 ff01::09 ff01::0a ff01::0b ff01::0c ff01::0d \ > + ff01::0e ff01::0f ff01::a1 ff01::a2 ff01::a3 > check ovn-nbctl ls-add ls-client > check ovn-nbctl ls-add ls-server > check ovn-nbctl lsp-add ls-client lsp-client-router > @@ -16827,7 +16829,21 @@ AT_CHECK([grep -E flags.network_id lrflows | ovn_strip_lflows], [0], [dnl > table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip4 && reg0 == 8.8.8.1/24), action=(flags.network_id = 0; next;) > table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::2/128), action=(flags.network_id = 0; next;) > table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::3/128), action=(flags.network_id = 1; next;) > - table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::6/128), action=(flags.network_id = 2; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::4/128), action=(flags.network_id = 2; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::5/128), action=(flags.network_id = 3; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::6/128), action=(flags.network_id = 4; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::7/128), action=(flags.network_id = 5; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::8/128), action=(flags.network_id = 6; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::9/128), action=(flags.network_id = 7; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a/128), action=(flags.network_id = 8; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a1/128), action=(flags.network_id = 14; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a2/128), action=(flags.network_id = 15; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a3/128), action=(flags.network_id = 0; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::b/128), action=(flags.network_id = 9; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::c/128), action=(flags.network_id = 10; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::d/128), action=(flags.network_id = 11; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::e/128), action=(flags.network_id = 12; next;) > + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::f/128), action=(flags.network_id = 13; next;) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 0 && ip4 && outport == "lrp-client"), action=(ct_snat(1.1.1.1);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 0 && ip4 && outport == "lrp-server"), action=(ct_snat(1.1.2.1);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 0 && ip6 && outport == "lrp-client"), action=(ct_snat(ff01::1);) > @@ -16835,20 +16851,33 @@ AT_CHECK([grep -E flags.network_id lrflows | ovn_strip_lflows], [0], [dnl > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 1 && ip4 && outport == "lrp-server"), action=(ct_snat(1.2.1.1);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 1 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::3);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 10 && ip4 && outport == "lrp-server"), action=(ct_snat(6.6.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 10 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::c);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 11 && ip4 && outport == "lrp-server"), action=(ct_snat(6.6.6.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 11 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::d);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 12 && ip4 && outport == "lrp-server"), action=(ct_snat(6.7.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 12 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::e);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 13 && ip4 && outport == "lrp-server"), action=(ct_snat(6.8.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 13 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::f);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 14 && ip4 && outport == "lrp-server"), action=(ct_snat(6.9.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 14 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::a1);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 15 && ip4 && outport == "lrp-server"), action=(ct_snat(7.2.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 15 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::a2);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 2 && ip4 && outport == "lrp-server"), action=(ct_snat(1.2.2.1);) > - table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 2 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::6);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 2 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::4);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 3 && ip4 && outport == "lrp-server"), action=(ct_snat(3.3.3.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 3 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::5);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 4 && ip4 && outport == "lrp-server"), action=(ct_snat(4.4.4.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 4 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::6);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 5 && ip4 && outport == "lrp-server"), action=(ct_snat(5.5.5.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 5 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::7);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 6 && ip4 && outport == "lrp-server"), action=(ct_snat(6.2.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 6 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::8);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 7 && ip4 && outport == "lrp-server"), action=(ct_snat(6.3.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 7 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::9);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 8 && ip4 && outport == "lrp-server"), action=(ct_snat(6.4.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 8 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::a);) > table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 9 && ip4 && outport == "lrp-server"), action=(ct_snat(6.5.1.1);) > + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 9 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::b);) > ]) > AT_CLEANUP > ]) This corrects the IPv6 behavior and makes it consistent with that of IPv4. Looks good to me. Thanks! Acked-by: Rosemarie O'Riorden <rosemarie@redhat.com>
On 4/2/25 4:22 PM, Rosemarie O'Riorden wrote: > On 4/2/25 6:46 AM, Dumitru Ceara wrote: >> If a router port has more than 16 networks configured we should set the >> flags.network_id to 0 for network whose index is greater than 16. >> >> This issue has been reported by coverity. Trimmed report: >> >> *** CID 459645: Code maintainability issues (UNUSED_VALUE) >> /northd/northd.c: 15062 in build_lrouter_network_id_flows() >> 15056 "flags.network_id = 0.", op->json_key, >> 15057 OVN_MAX_NETWORK_ID + 1, >> 15058 op->lrp_networks.ipv6_addrs[i].addr_s, >> 15059 op->lrp_networks.ipv6_addrs[i].plen); >> 15060 network_id = 0; >> 15061 } else { >> >>> CID 459645: Code maintainability issues (UNUSED_VALUE) >> >>> Assigning value from "i" to "network_id" here, but that stored >> >> value is overwritten before it can be used. >> 15062 network_id = i; >> 15063 } >> 15064 >> 15065 ds_clear(match); >> 15066 ds_clear(actions); >> 15067 >> >> Fixes: af6e83707568 ("northd: Use next-hop network for SNAT when lb_force_snat_ip=router_ip.") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- > > This corrects the IPv6 behavior and makes it consistent with that of > IPv4. Looks good to me. Thanks! > > Acked-by: Rosemarie O'Riorden <rosemarie@redhat.com> > Hi Rosemarie, Thanks for the review! Applied to main, 25.03, 24.09 and 24.03. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 471d9969bf..00bb738b3c 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -15070,7 +15070,8 @@ build_lrouter_network_id_flows( op->lrp_networks.ipv6_addrs[i].addr_s, op->lrp_networks.ipv6_addrs[i].plen); - ds_put_format(actions, "flags.network_id = %"PRIuSIZE"; next;", i); + ds_put_format(actions, "flags.network_id = %"PRIuSIZE"; next;", + network_id); ovn_lflow_add(lflows, op->od, S_ROUTER_IN_NETWORK_ID, 110, ds_cstr(match), ds_cstr(actions), lflow_ref); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 95993e454c..8652e5574b 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -16781,7 +16781,9 @@ check ovn-nbctl lrp-add lr lrp-server 02:00:00:00:00:03 1.1.2.1/24 7.7.7.1/24 \ 8.8.8.1/24 1.2.1.1/24 1.2.2.1/24 3.3.3.1/24 4.4.4.1/24 5.5.5.1/24 \ 6.6.6.1/24 6.2.1.1/24 6.3.1.1/24 6.4.1.1/24 6.5.1.1/24 6.6.1.1/24 \ 6.7.1.1/24 6.8.1.1/24 6.9.1.1/24 7.2.1.1/24 \ - ff01::02 ff01::03 ff01::06 + ff01::02 ff01::03 ff01::04 ff01::05 ff01::06 ff01::07 \ + ff01::08 ff01::09 ff01::0a ff01::0b ff01::0c ff01::0d \ + ff01::0e ff01::0f ff01::a1 ff01::a2 ff01::a3 check ovn-nbctl ls-add ls-client check ovn-nbctl ls-add ls-server check ovn-nbctl lsp-add ls-client lsp-client-router @@ -16827,7 +16829,21 @@ AT_CHECK([grep -E flags.network_id lrflows | ovn_strip_lflows], [0], [dnl table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip4 && reg0 == 8.8.8.1/24), action=(flags.network_id = 0; next;) table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::2/128), action=(flags.network_id = 0; next;) table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::3/128), action=(flags.network_id = 1; next;) - table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::6/128), action=(flags.network_id = 2; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::4/128), action=(flags.network_id = 2; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::5/128), action=(flags.network_id = 3; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::6/128), action=(flags.network_id = 4; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::7/128), action=(flags.network_id = 5; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::8/128), action=(flags.network_id = 6; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::9/128), action=(flags.network_id = 7; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a/128), action=(flags.network_id = 8; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a1/128), action=(flags.network_id = 14; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a2/128), action=(flags.network_id = 15; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::a3/128), action=(flags.network_id = 0; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::b/128), action=(flags.network_id = 9; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::c/128), action=(flags.network_id = 10; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::d/128), action=(flags.network_id = 11; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::e/128), action=(flags.network_id = 12; next;) + table=??(lr_in_network_id ), priority=110 , match=(outport == "lrp-server" && ip6 && xxreg0 == ff01::f/128), action=(flags.network_id = 13; next;) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 0 && ip4 && outport == "lrp-client"), action=(ct_snat(1.1.1.1);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 0 && ip4 && outport == "lrp-server"), action=(ct_snat(1.1.2.1);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 0 && ip6 && outport == "lrp-client"), action=(ct_snat(ff01::1);) @@ -16835,20 +16851,33 @@ AT_CHECK([grep -E flags.network_id lrflows | ovn_strip_lflows], [0], [dnl table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 1 && ip4 && outport == "lrp-server"), action=(ct_snat(1.2.1.1);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 1 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::3);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 10 && ip4 && outport == "lrp-server"), action=(ct_snat(6.6.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 10 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::c);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 11 && ip4 && outport == "lrp-server"), action=(ct_snat(6.6.6.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 11 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::d);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 12 && ip4 && outport == "lrp-server"), action=(ct_snat(6.7.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 12 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::e);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 13 && ip4 && outport == "lrp-server"), action=(ct_snat(6.8.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 13 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::f);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 14 && ip4 && outport == "lrp-server"), action=(ct_snat(6.9.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 14 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::a1);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 15 && ip4 && outport == "lrp-server"), action=(ct_snat(7.2.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 15 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::a2);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 2 && ip4 && outport == "lrp-server"), action=(ct_snat(1.2.2.1);) - table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 2 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::6);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 2 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::4);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 3 && ip4 && outport == "lrp-server"), action=(ct_snat(3.3.3.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 3 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::5);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 4 && ip4 && outport == "lrp-server"), action=(ct_snat(4.4.4.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 4 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::6);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 5 && ip4 && outport == "lrp-server"), action=(ct_snat(5.5.5.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 5 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::7);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 6 && ip4 && outport == "lrp-server"), action=(ct_snat(6.2.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 6 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::8);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 7 && ip4 && outport == "lrp-server"), action=(ct_snat(6.3.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 7 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::9);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 8 && ip4 && outport == "lrp-server"), action=(ct_snat(6.4.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 8 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::a);) table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 9 && ip4 && outport == "lrp-server"), action=(ct_snat(6.5.1.1);) + table=??(lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && flags.network_id == 9 && ip6 && outport == "lrp-server"), action=(ct_snat(ff01::b);) ]) AT_CLEANUP ])
If a router port has more than 16 networks configured we should set the flags.network_id to 0 for network whose index is greater than 16. This issue has been reported by coverity. Trimmed report: *** CID 459645: Code maintainability issues (UNUSED_VALUE) /northd/northd.c: 15062 in build_lrouter_network_id_flows() 15056 "flags.network_id = 0.", op->json_key, 15057 OVN_MAX_NETWORK_ID + 1, 15058 op->lrp_networks.ipv6_addrs[i].addr_s, 15059 op->lrp_networks.ipv6_addrs[i].plen); 15060 network_id = 0; 15061 } else { >>> CID 459645: Code maintainability issues (UNUSED_VALUE) >>> Assigning value from "i" to "network_id" here, but that stored >> value is overwritten before it can be used. 15062 network_id = i; 15063 } 15064 15065 ds_clear(match); 15066 ds_clear(actions); 15067 Fixes: af6e83707568 ("northd: Use next-hop network for SNAT when lb_force_snat_ip=router_ip.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- northd/northd.c | 3 ++- tests/ovn-northd.at | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 4 deletions(-)