[patch] ldp & ldpctl implicit null, inline patches

[Thomas Habets]

Hi.

Let's try that again, with inline patches and pointers to them
since they seemed to be stripped from the last mail:

http://www.habets.pp.se/synscan/files/ldpctl-impnull.patch
http://www.habets.pp.se/synscan/files/ldpd-impnull.patch

I played around a bit with ospfd and ldpd against real cisco routers, and
noticed some strange things.

ldpctl
------
First of all I saw that the output from "ldpctl show lib" and lfib showed label
"3" instead of the well-known "implicit null". I've patched ldpctl to show what
I think is more appropriate output. ldpctl-impnull.patch.

Apologies if this isn't the output you had in mind.

ldpd
----
Then I saw that any packet *using* the implicit null label actually went
out over the wire with a label of 3. That's not very implicit. In fact, that's
explicit.

I added a check to ldpd if it's about to tell the kernel to place an imp-null
label (swap or push, for MPLS and IP) and if so change it to skipping that
label (using pop or return instead).

Also, I'm not quite sure what "In use" is supposed to mean, since if I take
this line for example:
123.0.0.1/32         123.0.0.1         23             -              no

It's obviously in use and being announced over LDP to the other routers.

c3640-2#show mpls ldp bindings 123.0.0.1 32
   tib entry: 123.0.0.1/32, rev 30
  remote binding: tsr: 1.0.0.100:0, tag: 23

So I changed the "in use" bit to be "yes" even if there's no outgoing label. As
long as there's an incoming label the entry is, in my eyes, in use.

One more issue that my patches don't address is that the local label of
loopbacks aren't imp-null. I take it this is because loopbacks don't get the C
flag in the routing table.

Oh, and one last thing: that 123.0.0.1/32 above is actually a loopback
123.0.0.1/24. And while they can both be found in the routing table, lib and
lfib, the /32 is not announced by ospfd and should therefore not be in LDP
either.

---------
typedef struct me_s {
   char name[]      = { "Thomas Habets" };
   char email[]     = { "[hidden email]" };
   char kernel[]    = { "Linux" };
   char *pgpKey[]   = { "http://www.habets.pp.se/pubkey.txt" };
   char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE  0945 286A E90A AD48 E854" };
   char coolcmd[]   = { "echo '. ./_&. ./_'>_;. ./_" };
} me_t;


Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldpd/kroute.c,v
retrieving revision 1.7
diff -u -r1.7 kroute.c
--- kroute.c 3 Mar 2010 10:17:05 -0000 1.7
+++ kroute.c 10 Apr 2010 15:55:56 -0000
@@ -1091,10 +1091,17 @@
  iov[iovcnt].iov_base = &label_out;
  iov[iovcnt++].iov_len = sizeof(label_out);

- if (family == AF_MPLS)
- hdr.rtm_mpls = MPLS_OP_SWAP;
- else
- hdr.rtm_mpls = MPLS_OP_PUSH;
+ if (ntohl(kroute->remote_label) >> MPLS_LABEL_OFFSET == MPLS_LABEL_IMPLNULL) {
+ if (family == AF_MPLS)
+ hdr.rtm_mpls = MPLS_OP_POP;
+ else
+ return 0;
+ } else {
+ if (family == AF_MPLS)
+ hdr.rtm_mpls = MPLS_OP_SWAP;
+ else
+ hdr.rtm_mpls = MPLS_OP_PUSH;
+ }
  }


Index: lde_lib.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldpd/lde_lib.c,v
retrieving revision 1.14
diff -u -r1.14 lde_lib.c
--- lde_lib.c 3 Mar 2010 10:17:05 -0000 1.14
+++ lde_lib.c 10 Apr 2010 15:55:57 -0000
@@ -118,7 +118,7 @@
  rtctl.local_label = r->local_label;
  rtctl.remote_label = r->remote_label;

- if (!r->present || r->remote_label == NO_LABEL)
+ if (!r->present)
  rtctl.in_use = 0;
  else
  rtctl.in_use = 1;



-------------------------------------------------------------
ldpctl
-------------------------------------------------------------

Index: ldpctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldpctl/ldpctl.c,v
retrieving revision 1.8
diff -u -r1.8 ldpctl.c
--- ldpctl.c 3 Mar 2010 10:18:35 -0000 1.8
+++ ldpctl.c 10 Apr 2010 15:56:13 -0000
@@ -320,19 +320,29 @@
  {
  struct ctl_rt *rt;
  char *dstnet, *remote;
+ int             remote_label, no_label;

+ no_label = ((NO_LABEL << MPLS_LABEL_OFFSET)
+    & MPLS_LABEL_MASK) >> MPLS_LABEL_OFFSET;
  switch (imsg->hdr.type) {
  case IMSG_CTL_SHOW_LIB:
  rt = imsg->data;
  if (asprintf(&dstnet, "%s/%d", inet_ntoa(rt->prefix),
     rt->prefixlen) == -1)
  err(1, NULL);
-
- if (rt->connected || !rt->in_use) {
+ remote_label = ntohl(rt->remote_label) >> MPLS_LABEL_OFFSET;
+ if (!rt->in_use) {
  if (asprintf(&remote, "-") == -1)
  err(1, NULL);
+ } else  if (rt->connected
+    || (remote_label == no_label)) {
+ if (asprintf(&remote, "Untagged") == -1)
+ err(1, NULL);
+ } else if (remote_label == MPLS_LABEL_IMPLNULL) {
+ if (asprintf(&remote, "Pop tag") == -1)
+ err(1, NULL);
  } else {
- if (asprintf(&remote, "%u", (ntohl(rt->remote_label) >> MPLS_LABEL_OFFSET)) == -1)
+        if (asprintf(&remote, "%u", remote_label) == -1)
  err(1, NULL);
  }

@@ -429,17 +439,24 @@
  else if (k->flags & F_CONNECTED)
  printf("link#%-13u", k->ifindex);

- if (k->local_label != NO_LABEL) {
+ if (k->local_label == NO_LABEL) {
+ printf("-                 ");
+ } else if (ntohl(k->local_label) >> MPLS_LABEL_OFFSET
+   == MPLS_LABEL_IMPLNULL) {
+ printf("imp-null          ");
+ } else
  printf("%-18u", (ntohl(k->local_label) >>
     MPLS_LABEL_OFFSET));
- } else
- printf("-                 ");

- if (k->remote_label != NO_LABEL) {
+ if (k->remote_label == NO_LABEL) {
+ printf("-");
+ } else if (htonl(k->remote_label) >> MPLS_LABEL_OFFSET
+   == MPLS_LABEL_IMPLNULL) {
+        printf("Pop");
+ } else {
  printf("%u", (ntohl(k->remote_label) >>
     MPLS_LABEL_OFFSET));
- } else
- printf("-");
+ }

  printf("\n");

[Michele Marchetto-3]

Il giorno dom, 11/04/2010 alle 10.21 +0200, Thomas Habets ha scritto:
> I played around a bit with ospfd and ldpd against real cisco routers, and
> noticed some strange things.

Glad to see that someone is testing MPLS and ldpd. That is going to help
us.
Keep in mind that it is still in a development state.

> First of all I saw that the output from "ldpctl show lib" and lfib showed label
> "3" instead of the well-known "implicit null". I've patched ldpctl to show what
> I think is more appropriate output. ldpctl-impnull.patch.

This part is going to be committed with some little tweaks. Thanks.

> Then I saw that any packet *using* the implicit null label actually went
> out over the wire with a label of 3. That's not very implicit. In fact, that's
> explicit.
>
> I added a check to ldpd if it's about to tell the kernel to place an imp-null
> label (swap or push, for MPLS and IP) and if so change it to skipping that
> label (using pop or return instead).

This part is also right. Current ldpd behaviour is just for testing
purposes, but a correct PHP would require to pop instead of sending out
packets with label 3. Also this part is going to be committed.

> Also, I'm not quite sure what "In use" is supposed to mean, since if I take
> this line for example:
> 123.0.0.1/32         123.0.0.1         23             -              no

The "in use" column means that we received a label for a prefix that we
don't have in the kernel routing table, or that is somehow unusable.
I think your diff makes things more clear.

Thanks.