Commit 613c3e4839 for frr
commit 613c3e4839163504fd3e56ec36cff8c6681c3030
Author: Qipeng Ding <dingqipeng@hotmail.com>
Date: Wed Oct 14 17:32:55 2026 +0800
bgpd: isolate classic NLRI next-hop length from MP context
A mixed UPDATE can give classic IPv4 NLRI the MP IPv6 next hop.
Clear mp_nexthop_len only while parsing classic NLRI, then restore
it before processing the MP section. This avoids a copied attribute
view and leaves cleanup ownership unchanged.
Retain the native two-cycle announce/withdraw regression, including
separate next hops, downstream acceptance, stable sessions and PIDs.
Fixes: #23601
Signed-off-by: Qipeng Ding <dingqipeng@hotmail.com>
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index abf0f08f1c..9f61901ae3 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -2396,6 +2396,7 @@ static int bgp_update_receive(struct peer_connection *connection, bgp_size_t siz
uint8_t *end;
struct stream *s;
struct attr attr;
+ uint8_t mp_nexthop_len;
bgp_size_t attribute_len;
bgp_size_t update_len;
bgp_size_t withdraw_len;
@@ -2577,9 +2578,14 @@ static int bgp_update_receive(struct peer_connection *connection, bgp_size_t siz
switch (i) {
case NLRI_UPDATE:
+ /* Classic IPv4 NLRI uses NEXT_HOP, not the MP_REACH_NLRI next hop. */
+ mp_nexthop_len = attr.mp_nexthop_len;
+ attr.mp_nexthop_len = 0;
+ nlri_ret = bgp_nlri_parse(peer, NLRI_ATTR_ARG, &nlris[i], 0);
+ attr.mp_nexthop_len = mp_nexthop_len;
+ break;
case NLRI_MP_UPDATE:
- nlri_ret = bgp_nlri_parse(peer, NLRI_ATTR_ARG,
- &nlris[i], 0);
+ nlri_ret = bgp_nlri_parse(peer, NLRI_ATTR_ARG, &nlris[i], 0);
break;
case NLRI_WITHDRAW:
case NLRI_MP_WITHDRAW:
diff --git a/tests/topotests/bgp_classic_mp_nexthop/r1/frr.conf b/tests/topotests/bgp_classic_mp_nexthop/r1/frr.conf
new file mode 100644
index 0000000000..f84f8056b2
--- /dev/null
+++ b/tests/topotests/bgp_classic_mp_nexthop/r1/frr.conf
@@ -0,0 +1,20 @@
+interface r1-eth0
+ ip address 192.0.2.1/24
+ ipv6 address 2001:db8:1::1/64
+!
+router bgp 65010
+ bgp router-id 192.0.2.1
+ no bgp ebgp-requires-policy
+ neighbor 192.0.2.13 remote-as 65020
+ neighbor 192.0.2.13 timers connect 1
+ neighbor 192.0.2.30 remote-as 65030
+ address-family ipv4 unicast
+ neighbor 192.0.2.13 activate
+ neighbor 192.0.2.30 activate
+ neighbor 192.0.2.30 next-hop-self
+ exit-address-family
+ address-family ipv6 unicast
+ neighbor 192.0.2.13 activate
+ neighbor 192.0.2.30 activate
+ neighbor 192.0.2.30 next-hop-self
+ exit-address-family
diff --git a/tests/topotests/bgp_classic_mp_nexthop/r2/frr.conf b/tests/topotests/bgp_classic_mp_nexthop/r2/frr.conf
new file mode 100644
index 0000000000..a04aea5791
--- /dev/null
+++ b/tests/topotests/bgp_classic_mp_nexthop/r2/frr.conf
@@ -0,0 +1,3 @@
+interface r2-eth0
+ ip address 192.0.2.13/24
+ ipv6 address 2001:db8:1::13/64
diff --git a/tests/topotests/bgp_classic_mp_nexthop/r3/frr.conf b/tests/topotests/bgp_classic_mp_nexthop/r3/frr.conf
new file mode 100644
index 0000000000..9ba5007dce
--- /dev/null
+++ b/tests/topotests/bgp_classic_mp_nexthop/r3/frr.conf
@@ -0,0 +1,14 @@
+interface r3-eth0
+ ip address 192.0.2.30/24
+ ipv6 address 2001:db8:1::30/64
+!
+router bgp 65030
+ bgp router-id 192.0.2.30
+ no bgp ebgp-requires-policy
+ neighbor 192.0.2.1 remote-as 65010
+ address-family ipv4 unicast
+ neighbor 192.0.2.1 activate
+ exit-address-family
+ address-family ipv6 unicast
+ neighbor 192.0.2.1 activate
+ exit-address-family
diff --git a/tests/topotests/bgp_classic_mp_nexthop/test_bgp_classic_mp_nexthop.py b/tests/topotests/bgp_classic_mp_nexthop/test_bgp_classic_mp_nexthop.py
new file mode 100644
index 0000000000..8f72debdf0
--- /dev/null
+++ b/tests/topotests/bgp_classic_mp_nexthop/test_bgp_classic_mp_nexthop.py
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: ISC
+# Copyright (c) 2026 Qipeng Ding
+"""Classic IPv4 and MP IPv6 NLRI must keep their own next-hop contexts."""
+
+import json
+import os
+import subprocess
+import time
+
+import pytest
+from lib.topogen import Topogen, get_topogen
+
+pytestmark = [pytest.mark.bgpd]
+CWD = os.path.dirname(os.path.realpath(__file__))
+V4, V6 = "198.51.100.61/32", "2001:db8:2361::/64"
+
+
+def build_topo(tgen):
+ switch = tgen.add_switch("s1")
+ for name in ["r1", "r2", "r3"]:
+ switch.add_link(tgen.add_router(name))
+
+
+def setup_module(mod):
+ tgen = Topogen(build_topo, mod.__name__)
+ tgen.start_topology()
+ for name, router in tgen.routers().items():
+ router.cmd(
+ "sysctl -qw net.ipv6.conf.all.disable_ipv6=0 net.ipv6.conf.default.disable_ipv6=0"
+ )
+ router.load_frr_config(os.path.join(CWD, name, "frr.conf"))
+ tgen.start_router()
+
+
+def teardown_module(_mod):
+ get_topogen().stop_topology()
+
+
+def wait_for(predicate, timeout=20):
+ deadline = time.monotonic() + timeout
+ while time.monotonic() < deadline:
+ value = predicate()
+ if value:
+ return value
+ time.sleep(0.1)
+ raise AssertionError("required native BGP observation missing")
+
+
+def test_mixed_nlri_context(tmp_path):
+ tgen = get_topogen()
+ assert not tgen.routers_have_failure(), tgen.errors
+ r1, r2, r3 = [tgen.gears[name] for name in ["r1", "r2", "r3"]]
+ pids = {
+ name: tgen.gears[name].cmd("cat /var/run/frr/bgpd.pid").strip()
+ for name in ["r1", "r3"]
+ }
+
+ def routes(router, family):
+ result = router.vtysh_cmd(
+ "show bgp {} unicast json".format(family), isjson=True
+ )
+ assert "routes" in result, result
+ return result["routes"]
+
+ def present(router):
+ return V4 in routes(router, "ipv4") and V6 in routes(router, "ipv6")
+
+ def absent(router):
+ return V4 not in routes(router, "ipv4") and V6 not in routes(router, "ipv6")
+
+ def accepted():
+ if not present(r3):
+ return False
+ return all(
+ len(routes(r3, family)[prefix]) == 1
+ and routes(r3, family)[prefix][0].get("valid") is True
+ for family, prefix in [("ipv4", V4), ("ipv6", V6)]
+ )
+
+ def drops():
+ return {
+ address: r1.vtysh_cmd(
+ "show bgp neighbor {} json".format(address), isjson=True
+ )[address]["connectionsDropped"]
+ for address in ["192.0.2.13", "192.0.2.30"]
+ }
+
+ peer_log = tmp_path / "peer.log"
+ snapshots = []
+ with peer_log.open("w") as peer_output:
+ peer = None
+ try:
+ peer = r2.popen(
+ ["python3", os.path.join(CWD, "valid_peer.py")],
+ stdin=subprocess.PIPE,
+ stdout=peer_output,
+ stderr=peer_output,
+ )
+ wait_for(lambda: "ESTABLISHED" in peer_log.read_text())
+ wait_for(
+ lambda: r1.vtysh_cmd("show bgp neighbor 192.0.2.30 json", isjson=True)[
+ "192.0.2.30"
+ ]["bgpState"]
+ == "Established"
+ )
+ assert absent(r1) and absent(r3)
+ initial_drops = drops()
+ for cycle in range(2):
+ peer.stdin.write("announce\n")
+ peer.stdin.flush()
+ wait_for(lambda: present(r1))
+ local4, local6 = routes(r1, "ipv4")[V4], routes(r1, "ipv6")[V6]
+ snapshots.append(
+ {"cycle": cycle, "local_v4": local4, "local_v6": local6}
+ )
+ (tmp_path / "observations.json").write_text(
+ json.dumps(snapshots, indent=2)
+ )
+ assert len(local4) == len(local6) == 1
+ assert (
+ local4[0]["nexthops"][0]["ip"] == "192.0.2.13"
+ ), "classic NLRI used the MP next-hop context: {}".format(local4)
+ assert local6[0]["nexthops"][0]["ip"] == "2001:db8:1::13", local6
+ wait_for(accepted)
+ for family, prefix in [("ipv4", V4), ("ipv6", V6)]:
+ observed = routes(r3, family)[prefix]
+ assert len(observed) == 1 and observed[0]["valid"] is True, observed
+ assert observed[0]["path"] == "65010 65020", observed
+ peer.stdin.write("withdraw\n")
+ peer.stdin.flush()
+ wait_for(lambda: absent(r1) and absent(r3))
+ assert drops() == initial_drops
+ for name in ["r1", "r3"]:
+ assert (
+ tgen.gears[name].cmd("cat /var/run/frr/bgpd.pid").strip()
+ == pids[name]
+ )
+ finally:
+ if peer is not None and peer.poll() is None:
+ try:
+ peer.stdin.write("stop\n")
+ peer.stdin.flush()
+ peer.wait(timeout=10)
+ except (BrokenPipeError, OSError, subprocess.TimeoutExpired):
+ peer.terminate()
+ try:
+ peer.wait(timeout=3)
+ except subprocess.TimeoutExpired:
+ peer.kill()
+ peer.wait(timeout=3)
+ assert peer is not None and peer.returncode == 0
diff --git a/tests/topotests/bgp_classic_mp_nexthop/valid_peer.py b/tests/topotests/bgp_classic_mp_nexthop/valid_peer.py
new file mode 100644
index 0000000000..0aaac8a8c9
--- /dev/null
+++ b/tests/topotests/bgp_classic_mp_nexthop/valid_peer.py
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: ISC
+# Copyright (c) 2026 Qipeng Ding
+"""A fixed local BGP peer advertising two valid, distinct AFI/SAFI prefixes."""
+
+import ipaddress
+import select
+import socket
+import struct
+import sys
+import time
+
+
+def message(kind, body=b""):
+ return b"\xff" * 16 + struct.pack("!HB", 19 + len(body), kind) + body
+
+
+def attribute(flags, kind, body):
+ return bytes([flags, kind, len(body)]) + body
+
+
+def nlri(network):
+ prefix = ipaddress.ip_network(network)
+ return (
+ bytes([prefix.prefixlen])
+ + prefix.network_address.packed[: (prefix.prefixlen + 7) // 8]
+ )
+
+
+def read_message(connection):
+ def read(size):
+ result = b""
+ while len(result) < size:
+ value = connection.recv(size - len(result))
+ if not value:
+ raise EOFError("FRR closed the configured peer")
+ result += value
+ return result
+
+ header = read(19)
+ assert header[:16] == b"\xff" * 16
+ length, kind = struct.unpack("!HB", header[16:])
+ assert 19 <= length <= 4096
+ return kind, read(length - 19)
+
+
+def announce():
+ mp = (
+ struct.pack("!HBB", 2, 1, 16)
+ + socket.inet_pton(socket.AF_INET6, "2001:db8:1::13")
+ + b"\0"
+ + nlri("2001:db8:2361::/64")
+ )
+ attrs = (
+ attribute(64, 1, b"\0")
+ + attribute(64, 2, struct.pack("!BBH", 2, 1, 65020))
+ + attribute(64, 3, socket.inet_aton("192.0.2.13"))
+ + attribute(128, 14, mp)
+ )
+ return message(
+ 2, struct.pack("!HH", 0, len(attrs)) + attrs + nlri("198.51.100.61/32")
+ )
+
+
+def withdraw():
+ classic = nlri("198.51.100.61/32")
+ attrs = attribute(128, 15, struct.pack("!HB", 2, 1) + nlri("2001:db8:2361::/64"))
+ return message(
+ 2,
+ struct.pack("!H", len(classic))
+ + classic
+ + struct.pack("!H", len(attrs))
+ + attrs,
+ )
+
+
+with socket.socket() as listener:
+ listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ listener.bind(("192.0.2.13", 179))
+ listener.listen(1)
+ print("LISTENING", flush=True)
+ connection, address = listener.accept()
+ assert address[0] == "192.0.2.1"
+ with connection:
+ connection.settimeout(5)
+ caps = b"".join(
+ bytes([1, 4]) + struct.pack("!HBB", afi, 0, 1) for afi in [1, 2]
+ )
+ options = bytes([2, len(caps)]) + caps
+ connection.sendall(
+ message(
+ 1,
+ struct.pack(
+ "!BHH4sB",
+ 4,
+ 65020,
+ 30,
+ socket.inet_aton("192.0.2.13"),
+ len(options),
+ )
+ + options,
+ )
+ )
+ kind, _ = read_message(connection)
+ assert kind == 1
+ connection.sendall(message(4))
+ while True:
+ kind, _ = read_message(connection)
+ assert kind != 3
+ if kind == 4:
+ break
+ print("ESTABLISHED", flush=True)
+ last = time.monotonic()
+ while True:
+ ready, _, _ = select.select([connection, sys.stdin], [], [], 0.2)
+ if connection in ready:
+ kind, _ = read_message(connection)
+ assert kind != 3
+ if sys.stdin in ready:
+ command = sys.stdin.readline().strip()
+ if command in ["", "stop"]:
+ break
+ assert command in ["announce", "withdraw"]
+ connection.sendall(announce() if command == "announce" else withdraw())
+ print(command.upper(), flush=True)
+ if time.monotonic() - last > 5:
+ connection.sendall(message(4))
+ last = time.monotonic()