On 8/31/26 10:59 PM, 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,
> v2: Rebase on main
> v3: Address Dumitru's comments. Added a new test case.
>
Thanks for the v3!
> Documentation/ref/ovn-logical-flows.7.rst | 2 +
> northd/northd.c | 8 +-
> tests/ovn-northd.at | 27 +++++++
> tests/ovn.at | 91 +++++++++++++++++++++++
> 4 files changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ref/ovn-logical-flows.7.rst
> b/Documentation/ref/ovn-logical-flows.7.rst
> index 1a9168ac8686..9dc670374965 100644
> --- a/Documentation/ref/ovn-logical-flows.7.rst
> +++ b/Documentation/ref/ovn-logical-flows.7.rst
> @@ -2342,6 +2342,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/northd/northd.c b/northd/northd.c
> index 88e3ece88417..d9434aef8d1e 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -13420,9 +13420,13 @@ 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++) {
> + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> 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);
> }
> }
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 6d191c1a0bc2..16b9afddbad6 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2361,6 +2361,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
> +])
> +
> # 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;)
> diff --git a/tests/ovn.at b/tests/ovn.at
> index a88a077c618a..0096a01e918b 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -14291,6 +14291,97 @@ OVN_CLEANUP([hv1],[hv2])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([/31 router port (RFC 3021)])
> +ovn_start
> +
> +# Logical network:
> +# 2 LS 'sw0' and 'sw1' connected via router R1.
> +# R1 connects to 'sw0' with a /31 point-to-point network (RFC 3021). The
> +# host 'sw0p1' uses 10.0.0.1, which is the address that OVN previously
> +# computed as the /31 "broadcast" and dropped in the L3 admission control.
> +# This test verifies that traffic sourced from the /31 peer is forwarded.
> +
> +check ovn-nbctl lr-add R1
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl ls-add sw1
> +
> +# Connect sw0 to R1 with a /31 network. The router owns 10.0.0.0/31.
> +check ovn-nbctl lrp-add R1 sw0 00:00:00:01:02:03 10.0.0.0/31
> +check ovn-nbctl lsp-add sw0 rp-sw0 -- set Logical_Switch_Port rp-sw0 \
> + type=router options:router-port=sw0 addresses=\"00:00:00:01:02:03\"
> +
> +# Connect sw1 to R1.
> +check ovn-nbctl lrp-add R1 sw1 00:00:00:01:02:04 20.0.0.1/24
> +check ovn-nbctl lsp-add sw1 rp-sw1 -- set Logical_Switch_Port rp-sw1 \
> + type=router options:router-port=sw1 addresses=\"00:00:00:01:02:04\"
> +
> +# Create logical port sw0p1 in sw0. Its IP (10.0.0.1) is the /31 peer of
> +# the router port and equals the address OVN treated as the broadcast.
> +check ovn-nbctl lsp-add sw0 sw0p1 \
> +-- lsp-set-addresses sw0p1 "f0:00:00:01:02:03 10.0.0.1"
> +
> +# Create logical port sw1p1 in sw1.
> +check ovn-nbctl lsp-add sw1 sw1p1 \
> +-- lsp-set-addresses sw1p1 "f0:00:00:01:02:04 20.0.0.2"
> +
> +# Create two hypervisors and OVS ports corresponding to logical ports.
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
Missing 'check'.
> +ovn_attach n1 br-phys 192.168.0.1
> +ovs-vsctl -- add-port br-int hv1-vif1 -- \
> + set interface hv1-vif1 external-ids:iface-id=sw0p1 \
> + options:tx_pcap=hv1/vif1-tx.pcap \
> + options:rxq_pcap=hv1/vif1-rx.pcap \
> + ofport-request=1
Missing 'check'.
> +
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
Missing 'check'.
> +ovn_attach n1 br-phys 192.168.0.2
> +ovs-vsctl -- add-port br-int hv2-vif1 -- \
> + set interface hv2-vif1 external-ids:iface-id=sw1p1 \
> + options:tx_pcap=hv2/vif1-tx.pcap \
> + options:rxq_pcap=hv2/vif1-rx.pcap \
> + ofport-request=1
Missing 'check'.
> +
> +# Pre-populate the hypervisors' ARP tables so that we don't lose any
> +# packets for ARP resolution (native tunneling doesn't queue packets
> +# for ARP resolution).
> +OVN_POPULATE_ARP
> +
> +wait_for_ports_up
> +check ovn-nbctl --wait=hv sync
> +
> +# Send an IP packet from sw0p1 (10.0.0.1, the /31 peer) to sw1p1 (20.0.0.2).
> +# Before the /31 fix this packet was dropped by the L3 admission control,
> +# which treated 10.0.0.1 as the broadcast address of the 10.0.0.0/31 network.
> +src_mac="f00000010203"
> +dst_mac="000000010203"
> +src_ip=`ip_to_hex 10 0 0 1`
> +dst_ip=`ip_to_hex 20 0 0 2`
> +packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000
> +
We should use scapy, e.g:
packet=$(fmt_pkt "Ether(dst='00:00:00:01:02:03', \
src='f0:00:00:01:02:03') / \
IP(src='10.0.0.1', dst='20.0.0.2', ttl=64) / \
UDP(sport=53, dport=4369)")
> +as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
> +
> +# Packet to expect at 'sw1p1'.
> +src_mac="000000010204"
> +dst_mac="f00000010204"
> +src_ip=`ip_to_hex 10 0 0 1`
> +dst_ip=`ip_to_hex 20 0 0 2`
> +echo
> "${dst_mac}${src_mac}08004500001c000000003f110100${src_ip}${dst_ip}0035111100080000"
> > expected
Here too:
packet=$(fmt_pkt "Ether(dst='f0:00:00:01:02:04', \
src='00:00:00:01:02:04') / \
IP(src='10.0.0.1', dst='20.0.0.2', ttl=63) / \
UDP(sport=53, dport=4369)")
echo $packet > expected
> +
> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
> +
> +OVN_CLEANUP([hv1],[hv2])
> +
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD([
> AT_SETUP([2 HVs, 1 lport/HV, localport ports])
> ovn_start
With the minor nits addressed I applied this patch to main, 26.09, 26.03
and 25.09.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
