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