On Thu, Jul 9, 2026 at 12:01 AM Dumitru Ceara via dev <
[email protected]> wrote:
>
> On 7/9/26 8:19 AM, Han Zhou wrote:
> > A /31 point-to-point link has no broadcast address; both addresses in
> > the range are usable hosts. However, ovn-northd computed a traditional
> > subnet broadcast for the router port and included it in the priority-100
> > lr_in_ip_input L3 admission control drop flow (ip4.src == {...}). On a
> > /31, that "broadcast" address is the peer, so all traffic originating
> > from the /31 peer was dropped.
> >
> > Skip the broadcast address in the admission control drop set for /31
> > networks so that /31 router ports work as expected.
> >
> > Fixes: 936b640f4934 ("ovn: Implement basic logical L3 routing.")
> > Assisted-by: Claude Opus 4.8, Cursor
> > Signed-off-by: Han Zhou <[email protected]>
> > ---
>
> Hi Han,
>
> Thanks for the patch!
Thanks Dumitru for the review. Sorry for the late response.
>
> > v1 -> v2: rebase on main
> >
> > Documentation/ref/ovn-logical-flows.7.rst | 2 ++
> > NEWS | 4 ++++
> > northd/northd.c | 6 ++++-
> > tests/ovn-northd.at | 27 +++++++++++++++++++++++
> > 4 files changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ref/ovn-logical-flows.7.rst
b/Documentation/ref/ovn-logical-flows.7.rst
> > index 3d9936414f25..61a486a98301 100644
> > --- a/Documentation/ref/ovn-logical-flows.7.rst
> > +++ b/Documentation/ref/ovn-logical-flows.7.rst
> > @@ -2273,6 +2273,8 @@ contains the following flows to implement very
basic IP host functionality.
> > ``REGBIT_EGRESS_LOOPBACK``.
> >
> > - ``ip4.src`` is the broadcast address of any IP network known to
the router.
> > + Point-to-point (``/31``, RFC 3021) networks have no broadcast
address and
> > + are excluded, so that traffic from a ``/31`` peer is not dropped.
> >
> > - A priority-100 flow parses DHCPv6 replies from IPv6 prefix
delegation routers
> > (``udp.src == 547 && udp.dst == 546``). The ``handle_dhcpv6_reply``
is used to
> > diff --git a/NEWS b/NEWS
> > index d1a6ad27fea8..360c32d4b1f5 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -66,6 +66,10 @@ Post v26.03.0
> > (type 11) and Parameter Problem (type 12) - generated by an
external
> > router are un-NATed correctly. This makes Path MTU discovery and
> > traceroute work through stateless NAT.
> > + - Logical router ports configured with a /31 (RFC 3021
point-to-point)
> > + network are now supported. A /31 has no broadcast address, so
the L3
> > + admission control no longer drops traffic from the /31 peer as a
> > + broadcast source.
>
> I'd argue that this is a bug fix and we wouldn't need a news item.
>
Agree.
> >
> > OVN v26.03.0 - xxx xx xxxx
> > --------------------------
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 62270893ab41..8f75fd860c5b 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -13146,7 +13146,11 @@ op_put_v4_networks(struct ds *ds, const struct
ovn_port *op, bool add_bcast)
> > ds_put_cstr(ds, "{");
> > for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
>
> While at it, can we also replace "int i" with "size_t i" here?
Done.
>
> > ds_put_format(ds, "%s, ",
op->lrp_networks.ipv4_addrs[i].addr_s);
> > - if (add_bcast) {
> > + /* A /31 point-to-point link (RFC 3021) has no broadcast
address:
> > + * both addresses in the range are usable hosts. Including the
> > + * computed "broadcast" here would drop legitimate traffic
from the
> > + * /31 peer, so skip it for /31 networks. */
> > + if (add_bcast && op->lrp_networks.ipv4_addrs[i].plen != 31) {
> > ds_put_format(ds, "%s, ",
op->lrp_networks.ipv4_addrs[i].bcast_s);
> > }
>
> It's a bit weird but it seems that indeed we don't need any ipv6
> changes, is that correct?
Correct. IPv6 doesn't have this problem because it has no broadcast concept.
>
> > }
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index f82902c61da5..420328e116d9 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2476,6 +2476,33 @@ OVN_CLEANUP_NORTHD
> > AT_CLEANUP
> > ])
> >
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([router LRP /31 L3 admission control])
> > +ovn_start
> > +
> > +# A /31 point-to-point link (RFC 3021) has no broadcast address; both
> > +# addresses of the /31 are usable hosts. The L3 admission control
flow must
> > +# therefore not drop the computed "broadcast" address, otherwise
traffic from
> > +# the /31 peer is dropped.
> > +check ovn-nbctl lr-add lr
> > +check ovn-nbctl lrp-add lr lrp31 00:00:00:00:00:01 10.0.0.0/31
> > +check ovn-nbctl lrp-add lr lrp24 00:00:00:00:00:03 10.0.2.1/24
> > +
> > +check ovn-nbctl --wait=sb sync
> > +ovn-sbctl dump-flows lr > lrflows
> > +AT_CAPTURE_FILE([lrflows])
> > +
> > +# The /31 peer (10.0.0.1) must not appear as a dropped broadcast
source;
> > +# only the wider /24 network keeps its broadcast address.
> > +AT_CHECK([grep "lr_in_ip_input" lrflows | grep "priority=100" | grep
"reg9" | ovn_strip_lflows], [0], [dnl
> > + table=??(lr_in_ip_input ), priority=100 , match=(ip4.src ==
{10.0.0.0} && reg9[[0]] == 0), action=(drop;)
> > + table=??(lr_in_ip_input ), priority=100 , match=(ip4.src ==
{10.0.2.1, 10.0.2.255} && reg9[[0]] == 0), action=(drop;)
> > +])
> > +
> > +OVN_CLEANUP_NORTHD
> > +AT_CLEANUP
> > +])
> > +
>
> The pipeline is complex so this specific flow check might not be enough
> to ensure we don't break the feature in the end.
>
> Would it be possible to add a small test in ovn.at that actually injects
> a packet that traverses a /31 router port link?
Agree.
Please check v3:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
Best,
Han
>
> > # This test case tests that when a logical switch has load balancers
associated
> > # (with VIPs configured), the below logical flow is added by
ovn-northd.
> > # table=ls_out_pre_lb, priority=100, match=(ip), action=(reg0[[0]] =
1; next;)
>
> Thanks,
> Dumitru
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
