<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
<meta name="generator" content="pandoc" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=yes" />
<style>
html {
line-height: 1.2;
font-family: serif;
font-size: 0.9em;
color: black;
background-color: white;
}
body {
margin: 0;
margin-right: auto;
max-width: 36em;
padding: 1em;
hyphens: auto;
overflow-wrap: break-word;
text-rendering: optimizeLegibility;
font-kerning: normal;
}
@media print {
body {
background-color: transparent;
color: black;
font-size: 11pt;
}
p, h2, h3 {
orphans: 3;
widows: 3;
}
h2, h3, h4 {
page-break-after: avoid;
}
}
p {
margin: 1em 0;
}
a {
color: black;
}
a:visited {
color: black;
}
img {
max-width: 100%;
}
h1, h2, h3, h4, h5, h6 {
margin-top: 1.4em;
}
h5, h6 {
font-size: 1em;
font-style: italic;
}
h6 {
font-weight: normal;
}
ol, ul {
padding-left: 1.7em;
margin-top: 1em;
}
li > ol, li > ul {
margin-top: 0;
}
blockquote {
margin: 0.5em;
padding-left: 0.5em;
border-left: 2px solid #e6e6e6;
color: #444;
}
code {
font-family: 'Lucida Console', monospace;
font-size: 95%;
margin: 0;
}
pre {
margin: 1em 0;
overflow: auto;
max-width: unset;
width: fit-content;
}
pre code {
padding: 0;
overflow: visible;
overflow-wrap: normal;
max-width: unset;
white-space: pre-wrap;
}
pre code span {
white-space: pre;
}
.sourceCode {
background-color: transparent;
overflow: visible;
}
code.diff span.kw,
code.diff span.dt {
font-weight: bold;
}
code.diff span.va {
background-color: rgba(192, 255, 192, 64);
color: rgb(0, 64, 0);
}
code.diff span.st {
background-color: rgba(255, 192, 192, 64);
color: rgb(64, 0, 0);
}
pre.diff {
background-color: rgb(240, 240, 240);
padding: 0.4em;
border: 1pt solid grey;
}
hr {
background-color: black;
border: none;
height: 1px;
margin: 1em 0;
}
table {
margin: 1em 0;
border-collapse: collapse;
width: 100%;
overflow-x: auto;
display: block;
font-variant-numeric: lining-nums tabular-nums;
}
table caption {
margin-bottom: 0.75em;
}
tbody {
margin-top: 0.5em;
border-top: 1px solid black;
border-bottom: 1px solid black;
}
th {
border-top: 1px solid black;
padding: 0.25em 0.5em 0.25em 0.5em;
}
td {
padding: 0.125em 0.5em 0.25em 0.5em;
}
header {
margin-bottom: 4em;
text-align: center;
}
code{white-space: pre-wrap;}
span.smallcaps{font-variant: small-caps;}
span.underline{text-decoration: underline;}
div.column{display: inline-block; vertical-align: top; width: 50%;}
div.hanging-indent{margin-left: 1.5em; text-indent: -1.5em;}
ul.task-list{list-style: none;}
q { quotes: "„" "”" "»" "«"; }
.display.math{display: block; text-align: center; margin: 0.5rem auto;}
</style>
</head>
<body>
<p>Hello Evann!</p>
<p>Thank you for the patch! I have checked it thoroughly and it looks
right.</p>
<blockquote>
<p>Earlier this week, while building a BGP lab I’ve discovered a weird
behavior with aspa_check() that was rejecting routes that should be
correct. It seems to be the same bug as
https://trubka.network.cz/pipermail/bird-users/2026-February/018611.html.</p>
<p>I discover that the previous implementation of the verification
algorithm was not fully compliant with the current internet draft
(draft-ietf-sidrops-aspa-verification-24) and could incorrectly filter
routes when the up-ramp and down-ramp apex were apart by exactly one
hop. In practice, this caused legitimate routes to be dropped, such as
routes involving Tier-1 peering while having an AS0 ASPA record.</p>
<p>[…]</p>
<p>Given this issue, I tried to implement a patch. […]</p>
</blockquote>
<p>I have split your patch, and accepted the part where the test cases
are added. After some back and forth, I decided to fix the function in a
different way, keeping the logic of apex indices. I’m not totally
opposing rewriting the function completely, but we have some more plans
with that function (e.g. giving the up-ramp and down-ramp path chunks
back to the user), and the apex indices approach is more suitable for
that.</p>
<p>You may see my fix in the branch <code>355-aspa-minimal-fix</code>,
where I also added some additional commits, most notably the commit
documenting how the <code>aspa_check()</code> function is designed and
why. Please note that the branch may get rebased before merging into
master.</p>
<p>Side note: I have had a long beef with the authors of the draft about
the wording and overall structure, and I’m very happy that you happened
to implement this by the draft in a way which is not totally
confusing.</p>
<p>I have pushed your rewrite to the branch
<code>355-aspa-downstream-fix-evann-dreumont</code>, so that it doesn’t
get lost if we happen to change our minds later, and also to prove that
the rewrites of the draft were for good.</p>
<blockquote>
<p>To improve unit test coverage, I have added the missing test cases
from the official list
(https://raw.githubusercontent.com/ksriram25/IETF/main/ASPA_path_verification_examples.pdf).
They all passed successfully.</p>
</blockquote>
<p>You could have been much harsher on me for not implementing part of
my own test cases back into BIRD, tbh.</p>
<blockquote>
<p>However, one of the existing custom unit tests did not pass. In this
test, the path consists of two ASes: 65544, 65541. AS65544 declare an
AS0 ASPA and AS65541 declare AS65545 as a provider. The test expects the
route to be invalid. However, I believe this expectation is
incorrect.</p>
</blockquote>
<p>It was indeed incorrect, and again, it was me sending this (or
similar) testcase to Sriram with an incorrect outcome, and after
somebody pointed out that it’s incorrect, I didn’t follow up and fix
that back in BIRD.</p>
<p>Last but not least, sorry for me being silent. There has been an
awful lot on my table recently, including fast-coding a CoAP endpoint
for upcoming BIRD API, replicating and fixing some more crashes in BIRD
3, doing a lot of code reviews, and some crazy stuff better for a pub
than a public mailing-list.</p>
<p>With that, thank you again for your patch, checks, effort and
everything. Have a great day and happy routing!</p>
<p>–<br />
Maria Matejka (she/her) | BIRD Team Leader | CZ.NIC, z.s.p.o.</p>
</body>
</html>