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 <dce...@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 ]) -- 2.48.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev