<div dir="ltr">Hi Ondrej,<div><br></div><div>Thanks for the response. Yes, adding this limited attribute as 'gw_mpls' with a note in the documentation makes sense to me. </div><div><br></div><div>Cheers,</div><div>Trisha</div><div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><span style="color:rgb(136,136,136)">--</span><br style="color:rgb(136,136,136)"><div dir="ltr" style="color:rgb(136,136,136)"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div style="margin:0px;padding:0px;color:rgb(51,51,51);font-family:Arial,sans-serif;font-size:14px;line-height:20px"><img src="https://www.fastly.com/img/sig.png"><br></div><div style="margin:0px;padding:0px;color:rgb(51,51,51);font-family:Arial,sans-serif;font-size:14px;line-height:20px"><b>Trisha Biswas</b> | Sr. Software Engineer, Network Systems</div><div style="margin:0px;padding:0px;color:rgb(51,51,51);font-family:Arial,sans-serif;font-size:14px;line-height:20px"><a href="http://fastly.com/" rel="nofollow" style="color:rgb(59,115,175)" target="_blank">fastly.com</a> | <a href="https://twitter.com/fastly" rel="nofollow" style="color:rgb(59,115,175)" target="_blank">@fastly</a> | <a href="http://www.linkedin.com/company/fastly" rel="nofollow" style="color:rgb(59,115,175)" target="_blank">LinkedIn</a></div></div></div></div></div></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 11, 2021 at 8:10 AM Ondrej Zajicek <<a href="mailto:santiago@crfreenet.org">santiago@crfreenet.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, May 07, 2021 at 03:54:40PM -0700, Trisha Biswas wrote:<br>
> Hi all,<br>
> <br>
> While BIRD 2.x has underlying support for MPLS (applying labels to static<br>
> routes and syncing with kernel), there is no option to set labels via<br>
> configuration. The attached patch adds support to set or read labels using<br>
> filters. Currently this supports the addition of one label per route, and<br>
> can be extended to stacked labels in future.<br>
> <br>
> Please let me know if you have any questions.<br>
<br>
Hi<br>
<br>
The patch looks fine (there is a minor issue that reading the attribute<br>
could check 'labels' field to see if there is any, and if it is not, it<br>
should return implicit-null label (3)).<br>
<br>
There are some conceptual issues with it (and that is also why we did not<br>
implemented it already):<br>
<br>
First, note that all nexthop attributes (gw, iface, ...) a kind of<br>
broken, as they only allow to set the first nexthop and ignore ECMP.<br>
This attribute behaves in a similar manner, so it is not a big issue,<br>
just note that we plan to change/rework that in the future.<br>
<br>
Second, MPLS allows to use whole stack of labels (we allow to set it for<br>
static routes), but we cannot represent it in the filter language, so the<br>
patch allows to access only the first one (so it is handled as simple<br>
integer). You wrote that it can be extended to stacked labels in future,<br>
but it seems to me that it would be more (backwards-incompatible) change<br>
than extension (e.g. from int type to some int-list type).<br>
<br>
Third, note that there are two kinds of MPLS labels for a route - local<br>
label (label expected on received packets) and remote label of nexthop<br>
(label or labels attached to forwarded packets - this is the label stack<br>
in nexthop structure). I would prefer to reserve 'mpls' attribute name<br>
for the first one, so perhaps we could call it 'gw_mpls', 'encap_mpls'<br>
(like in ip-route tools), or something like that. <br>
<br>
It is true that having limited attribute is better than no attribute, so<br>
i would prefer to merge it with some note in documentation that it is<br>
experimental and likely will change in the future to handle multiple<br>
labels and under 'gw_mpls' name. Is that acceptable?<br>
<br>
-- <br>
Elen sila lumenn' omentielvo<br>
<br>
Ondrej 'Santiago' Zajicek (email: <a href="mailto:santiago@crfreenet.org" target="_blank">santiago@crfreenet.org</a>)<br>
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, <a href="http://wwwkeys.pgp.net" rel="noreferrer" target="_blank">wwwkeys.pgp.net</a>)<br>
"To err is human -- to blame it on a computer is even more so."<br>
</blockquote></div>