On Tue, Oct 20, 2026 at 1:54 AM Mark Michelson via dev < [email protected]> wrote:
> The "nexthop" column in the northbound database is deprecated in favor > of the "nexthops" column. However, the "nexthops" column only matters if > the policy is a "reroute" policy. Therefore, we can ignore the fact that > a deprecated column has its value set if the policy type does not rely > on nexthops at all. > > Signed-off-by: Mark Michelson <[email protected]> > --- > v1 -> v2: > * Added a NEWS entry about this change. > * Updated ovn-nb documentation so that it does not imply that the > "nexthop" column is used. > --- > NEWS | 5 +++ > northd/en-route-policies.c | 17 +++++----- > ovn-nb.xml | 3 +- > tests/ovn-northd.at | 66 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 81 insertions(+), 10 deletions(-) > > diff --git a/NEWS b/NEWS > index c4ebe5c39..f177b7f32 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,10 @@ > Post v26.09.0 > -------------- > + - Router policies that used the deprecated "nexthop" column used to > + always be skipped and not applied. Now these router policies are > + only skipped if their action is "reroute". Router policies of type > + "allow", "drop", and "jump" will still be accepted, even if the > + "nexthop" column is set on those policies. > - Learn ARP sender hardware address (arp.sha) and IPv6 Neighbor > Discovery link-layer addresses (nd.tll, nd.sll) into the FDB > table in addition to eth.src. This allows VRRP virtual MACs to > diff --git a/northd/en-route-policies.c b/northd/en-route-policies.c > index 9597980ec..2110b8b60 100644 > --- a/northd/en-route-policies.c > +++ b/northd/en-route-policies.c > @@ -176,14 +176,6 @@ build_route_policies(struct ovn_datapath *od, > for (int i = 0; i < od->nbr->n_policies; i++) { > const struct nbrec_logical_router_policy *rule = > od->nbr->policies[i]; > > - if (rule->nexthop && rule->nexthop[0]) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "Logical router: %s, policy uses deprecated" > - " column \"nexthop\", this column is ignored. > Please" > - "use \"nexthops\" column instead.", > od->nbr->name); > - continue; > - } > - > size_t n_valid_nexthops = 0; > char **valid_nexthops = NULL; > uint32_t chain_id = 0; > @@ -224,6 +216,15 @@ build_route_policies(struct ovn_datapath *od, > } > > if (!strcmp(rule->action, "reroute")) { > + if (rule->nexthop && rule->nexthop[0]) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "Logical router: %s, policy uses > deprecated" > + " column \"nexthop\", this policy is > skipped. " > + "Please use \"nexthops\" column instead.", > + od->nbr->name); > + continue; > + } > + > if (rule->output_port && rule->n_nexthops != 1) { > static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > VLOG_WARN_RL(&rl, > diff --git a/ovn-nb.xml b/ovn-nb.xml > index feea50a25..ebcd3e4d1 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -5368,8 +5368,7 @@ or > </li> > > <li> > - <code>reroute</code>: Reroute packet to <ref column="nexthop"/> > or > - <ref column="nexthops"/>. > + <code>reroute</code>: Reroute packet to <ref column="nexthops"/> > </li> > > <li> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 7bd0466c6..1401aecea 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -25135,3 +25135,69 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE > OVN_CLEANUP_NORTHD > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([Router policy selective nexthop deprecation warning]) > +ovn_start > + > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl lrp-add lr0 lr0-p1 00:00:00:00:00:01 172.16.1.1/24 > + > +# Add a policy of each type to this router > +check ovn-nbctl lr-policy-add lr0 100 "ip4.src == 10.0.0.0/24" allow > +check ovn-nbctl lr-policy-add lr0 100 "ip4.src == 20.0.0.0/24" drop > +check ovn-nbctl lr-policy-add lr0 100 "ip4.src == 30.0.0.0/24" reroute > 172.16.1.50 > +check ovn-nbctl lr-policy-add lr0 100 "ip4.src == 40.0.0.0/24" jump > target > +check ovn-nbctl --chain=target lr-policy-add lr0 200 "1" allow > +check ovn-nbctl --wait=sb sync > + > +# Let's double-check that all of the expected flows are present > +AT_CHECK([ovn-sbctl lflow-list lr0 | grep "lr_in_policy[[^_]]" | > ovn_strip_lflows | sort], [0], [dnl > + table=??(lr_in_policy ), priority=0 , match=(1), > action=(reg8[[0..15]] = 0; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 10.0.0.0/24)), action=(reg8[[0..15]] = 0; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 20.0.0.0/24)), action=(drop;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 30.0.0.0/24)), action=(reg0 = 172.16.1.50; reg5 = > 172.16.1.1; eth.src = 00:00:00:00:00:01; outport = "lr0-p1"; flags.loopback > = 1; reg8[[0..15]] = 0; reg9[[9]] = 1; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 40.0.0.0/24)), action=(reg9[[16..31]]=1; > next(pipeline=ingress, table=??);) > + table=??(lr_in_policy ), priority=200 , match=(reg9[[16..31]] == > 1 && (1)), action=(reg8[[0..15]] = 0; next;) > +]) > + > +allow_uuid=$(fetch_column nb:Logical_Router_Policy _uuid priority=100 > action=allow) > +drop_uuid=$(fetch_column nb:Logical_Router_Policy _uuid priority=100 > action=drop) > +jump_uuid=$(fetch_column nb:Logical_Router_Policy _uuid priority=100 > action=jump) > +reroute_uuid=$(fetch_column nb:Logical_Router_Policy _uuid priority=100 > action=reroute) > + > +# For the allow, drop, and jump policies, we'll set the "nexthop" column. > The policies > +# should remain unchanged, and we shouldn't see a warning in the log. > +check ovn-nbctl set Logical_Router_Policy $allow_uuid nexthop=50.0.0.50 > +check ovn-nbctl set Logical_Router_Policy $drop_uuid nexthop=50.0.0.50 > +check ovn-nbctl set Logical_Router_Policy $jump_uuid nexthop=50.0.0.50 > +check ovn-nbctl --wait=sb sync > + > +AT_CHECK([grep -qE "policy uses deprecated" northd/ovn-northd.log], [1]) > + > +AT_CHECK([ovn-sbctl lflow-list lr0 | grep "lr_in_policy[[^_]]" | > ovn_strip_lflows | sort], [0], [dnl > + table=??(lr_in_policy ), priority=0 , match=(1), > action=(reg8[[0..15]] = 0; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 10.0.0.0/24)), action=(reg8[[0..15]] = 0; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 20.0.0.0/24)), action=(drop;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 30.0.0.0/24)), action=(reg0 = 172.16.1.50; reg5 = > 172.16.1.1; eth.src = 00:00:00:00:00:01; outport = "lr0-p1"; flags.loopback > = 1; reg8[[0..15]] = 0; reg9[[9]] = 1; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 40.0.0.0/24)), action=(reg9[[16..31]]=1; > next(pipeline=ingress, table=??);) > + table=??(lr_in_policy ), priority=200 , match=(reg9[[16..31]] == > 1 && (1)), action=(reg8[[0..15]] = 0; next;) > +]) > + > +# Now let's set the "nexthop" column on the reroute policy. This should > result in the > +# warning appearing and the reroute policy not showing up in the lflows. > +check ovn-nbctl set Logical_Router_Policy $reroute_uuid nexthop=50.0.0.50 > +check ovn-nbctl --wait=sb sync > + > +AT_CHECK([grep -qE "policy uses deprecated" northd/ovn-northd.log], [0]) > +AT_CHECK([ovn-sbctl lflow-list lr0 | grep "lr_in_policy[[^_]]" | > ovn_strip_lflows | sort], [0], [dnl > + table=??(lr_in_policy ), priority=0 , match=(1), > action=(reg8[[0..15]] = 0; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 10.0.0.0/24)), action=(reg8[[0..15]] = 0; next;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 20.0.0.0/24)), action=(drop;) > + table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == > 0 && (ip4.src == 40.0.0.0/24)), action=(reg9[[16..31]]=1; > next(pipeline=ingress, table=??);) > + table=??(lr_in_policy ), priority=200 , match=(reg9[[16..31]] == > 1 && (1)), action=(reg8[[0..15]] = 0; next;) > +]) > + > +OVN_CLEANUP_NORTHD > +AT_CLEANUP > +]) > -- > 2.55.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
