Tried your diff and it does allow the session to come up! Cool, thanks!
The FT Session TLV is silently ignored now, so it works with default Junos LDP
settings.
However it doesn't always come up now between two openbsd boxes. Check the log
message about
recieved notification from neighbor 192.168.100.1: Session Rejected, No Hello.
When I stopped and started both ldpd again, so I could also take a packet
trace, suddenly it worked. Will keep an eye on it. Anyhow, below the log
messages of openbsd2 that complained about the missing hello.
192.168.100.1 is openbsd1, 192.168.100.2 is openbsd2 (both are lo1
interfaces)
# ldpd -dv
fast = "2"
startup
kernel add route 0.0.0.0/0
if_fsm: event UP resulted in action START and changing state for interface em3
from DOWN to ACTIVE
kernel add route 192.168.56.0/24
if_fsm: event UP resulted in action START and changing state for interface em2
from DOWN to ACTIVE
kernel add route 192.168.91.0/24
if_fsm: event UP resulted in action START and changing state for interface em1
from DOWN to ACTIVE
kernel add route 192.168.91.0/24
if_fsm: interface lo1, event UP not expected in state LOOP
kernel add route 192.168.92.0/24
kernel add route 192.168.92.0/24
kernel add route 192.168.93.0/24
kernel add route 192.168.93.0/24
kernel add route 192.168.94.0/24
kernel add route 192.168.94.0/24
kernel add route 192.168.95.0/24
kernel add route 192.168.95.0/24
kernel add route 192.168.100.2/32
kernel add route 192.168.100.4/32
nbr_fsm: event HELLO RECEIVED resulted in action START INACTIVITY TIMER and
changing state for neighbor ID 192.168.100.1 from DOWN to PRESENT
send_init: neighbor ID 192.168.100.1
nbr_fsm: event INIT SENT resulted in action NOTHING and changing state for
neighbor ID 192.168.100.1 from PRESENT to OPENSENT
recv_notification: neighbor ID 192.168.100.1
recieved notification from neighbor 192.168.100.1: Session Rejected, No Hello
session_close: closing session with nbr ID 192.168.100.1
nbr_fsm: event SESSION CLOSE resulted in action CLOSE SESSION and changing
state for neighbor ID 192.168.100.1 from OPENSENT to PRESENT
nbr_fsm: event HELLO RECEIVED resulted in action START INACTIVITY TIMER and
changing state for neighbor ID 192.168.100.4 from DOWN to PRESENT
nbr_fsm: event HELLO RECEIVED resulted in action START INACTIVITY TIMER and
changing state for neighbor ID 192.168.100.3 from DOWN to PRESENT
nbr_fsm: event SESSION UP resulted in action START NEIGHBOR SESSION and
changing state for neighbor ID 192.168.100.4 from PRESENT to INITIALIZED
recv_init: neighbor ID 192.168.100.4
send_init: neighbor ID 192.168.100.4
nbr_fsm: event INIT RECEIVED resulted in action SEND INIT and changing state
for neighbor ID 192.168.100.4 from INITIALIZED to OPENREC
send_address: neighbor ID 192.168.100.4
nbr_fsm: event KEEPALIVE RECEIVED resulted in action START KEEPALIVE TIMER and
changing state for neighbor ID 192.168.100.4 from OPENREC to OPERATIONAL
recv_address: neighbor ID 192.168.100.4
lde_address_add: added 192.168.94.4
lde_address_add: added 192.168.95.4
lde_address_add: added 192.168.100.4
label mapping from nbr 192.168.100.4, FEC 192.168.100.4/32, label 3
label mapping from nbr 192.168.100.4, FEC 192.168.92.0/24, label 299776
label mapping from nbr 192.168.100.4, FEC 192.168.93.0/24, label 299792
label mapping from nbr 192.168.100.4, FEC 192.168.100.1/32, label 299808
label mapping from nbr 192.168.100.4, FEC 192.168.100.2/32, label 299824
label mapping from nbr 192.168.100.4, FEC 0.0.0.0/0, label 299840
label mapping from nbr 192.168.100.4, FEC 192.168.100.3/32, label 299856
label mapping from nbr 192.168.100.4, FEC 192.168.91.0/24, label 300016
nbr_fsm: event SESSION UP resulted in action START NEIGHBOR SESSION and
changing state for neighbor ID 192.168.100.3 from PRESENT to INITIALIZED
recv_init: neighbor ID 192.168.100.3
send_init: neighbor ID 192.168.100.3
nbr_fsm: event INIT RECEIVED resulted in action SEND INIT and changing state
for neighbor ID 192.168.100.3 from INITIALIZED to OPENREC
send_address: neighbor ID 192.168.100.3
nbr_fsm: event KEEPALIVE RECEIVED resulted in action START KEEPALIVE TIMER and
changing state for neighbor ID 192.168.100.3 from OPENREC to OPERATIONAL
recv_address: neighbor ID 192.168.100.3
lde_address_add: added 192.168.92.3
lde_address_add: added 192.168.93.3
lde_address_add: added 192.168.95.3
label mapping from nbr 192.168.100.3, FEC 192.168.100.3/32, label 3
label mapping from nbr 192.168.100.3, FEC 192.168.100.4/32, label 300848
label mapping from nbr 192.168.100.3, FEC 0.0.0.0/0, label 300864
label mapping from nbr 192.168.100.3, FEC 192.168.100.1/32, label 300896
label mapping from nbr 192.168.100.3, FEC 192.168.94.0/24, label 300944
label mapping from nbr 192.168.100.3, FEC 192.168.100.2/32, label 300992
label mapping from nbr 192.168.100.3, FEC 192.168.91.0/24, label 301024
nbr_idtimer: neighbor ID 192.168.100.1 peerid 3
send_init: neighbor ID 192.168.100.1
nbr_fsm: event INIT SENT resulted in action NOTHING and changing state for
neighbor ID 192.168.100.1 from PRESENT to OPENSENT
recv_notification: neighbor ID 192.168.100.1
recieved notification from neighbor 192.168.100.1: Session Rejected, No Hello
session_close: closing session with nbr ID 192.168.100.1
nbr_fsm: event SESSION CLOSE resulted in action CLOSE SESSION and changing
state for neighbor ID 192.168.100.1 from OPENSENT to PRESENT
nbr_idtimer: neighbor ID 192.168.100.1 peerid 3
send_init: neighbor ID 192.168.100.1
nbr_fsm: event INIT SENT resulted in action NOTHING and changing state for
neighbor ID 192.168.100.1 from PRESENT to OPENSENT
recv_notification: neighbor ID 192.168.100.1
recieved notification from neighbor 192.168.100.1: Session Rejected, No Hello
session_close: closing session with nbr ID 192.168.100.1
nbr_fsm: event SESSION CLOSE resulted in action CLOSE SESSION and changing
state for neighbor ID 192.168.100.1 from OPENSENT to PRESENT
nbr_idtimer: neighbor ID 192.168.100.1 peerid 3
send_init: neighbor ID 192.168.100.1
nbr_fsm: event INIT SENT resulted in action NOTHING and changing state for
neighbor ID 192.168.100.1 from PRESENT to OPENSENT
recv_notification: neighbor ID 192.168.100.1
recieved notification from neighbor 192.168.100.1: Session Rejected, No Hello
session_close: closing session with nbr ID 192.168.100.1
nbr_fsm: event SESSION CLOSE resulted in action CLOSE SESSION and changing
state for neighbor ID 192.168.100.1 from OPENSENT to PRESENT
On Jan 9, 2011, at 4:45 PM, Claudio Jeker wrote:
> On Sat, Jan 08, 2011 at 08:04:33PM +0100, Marcel Wiget wrote:
>> On Jan 8, 2011, at 4:20 PM, Claudio Jeker wrote:
>>
>>> Commited the diff. I guess there is a bit more needed so that we handle
>>> various unknown TLVs correctly in hello and initializaion. I hope I can
>>> provide a diff for this soon. Btw. I would be interested in the ldpd -dv
>>> output of the failures you get when the JUNOS has RFC 3479 enabled or
when
>>> a different transport addr is used.
>>
>> Thanks! Without 'set protocols ldp graceful-restart helper-disable' on
junos,
>> I get the following output from ldpd -dv. Packet dump of the LDP
>> initialization message and openbsd's response further down:
>>
>
> Thanks for all the info. Can you try the following diff?
> This should allow unkown TLVs in hello and initialization messages.
> This should signal the JUNOS neighbor that it needs to disable FT.
>
> --
> :wq Claudio
>
> Index: hello.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/hello.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 hello.c
> --- hello.c 8 Jan 2011 14:50:29 -0000 1.8
> +++ hello.c 9 Jan 2011 14:55:12 -0000
> @@ -202,32 +202,37 @@ int
> tlv_decode_opt_hello_prms(char *buf, u_int16_t len, struct in_addr *addr,
> u_int32_t *conf_number)
> {
> - struct hello_opt_parms_tlv tlv;
> - int cons = 0;
> + struct tlv tlv;
> + int cons = 0;
> + u_int16_t tlv_len;
>
> bzero(addr, sizeof(*addr));
> *conf_number = 0;
>
> while (len >= sizeof(tlv)) {
> bcopy(buf, &tlv, sizeof(tlv));
> -
> - if (ntohs(tlv.length) < sizeof(u_int32_t))
> - return (-1);
> -
> + tlv_len = ntohs(tlv.length);
> switch (ntohs(tlv.type)) {
> case TLV_TYPE_IPV4TRANSADDR:
> - addr->s_addr = tlv.value;
> + if (tlv_len < sizeof(u_int32_t))
> + return (-1);
> + bcopy(buf + TLV_HDR_LEN, addr, sizeof(u_int32_t));
> break;
> case TLV_TYPE_CONFIG:
> - *conf_number = ntohl(tlv.value);
> + if (tlv_len < sizeof(u_int32_t))
> + return (-1);
> + bcopy(buf + TLV_HDR_LEN, conf_number,
> + sizeof(u_int32_t));
> break;
> default:
> - return (-1);
> + /* if unknown flag set, ignore TLV */
> + if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
> + return (-1);
> + break;
> }
> -
> - len -= sizeof(tlv);
> - buf += sizeof(tlv);
> - cons += sizeof(tlv);
> + buf += TLV_HDR_LEN + tlv_len;
> + len -= TLV_HDR_LEN + tlv_len;
> + cons += TLV_HDR_LEN + tlv_len;
> }
>
> return (cons);
> Index: init.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/init.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 init.c
> --- init.c 4 Nov 2010 09:52:16 -0000 1.6
> +++ init.c 9 Jan 2011 14:55:35 -0000
> @@ -38,6 +38,7 @@
> #include "ldpe.h"
>
> int gen_init_prms_tlv(struct ibuf *, struct nbr *, u_int16_t);
> +int tlv_decode_opt_init_prms(char *, u_int16_t);
>
> void
> send_init(struct nbr *nbr)
> @@ -88,12 +89,19 @@ recv_init(struct nbr *nbr, char *buf, u_
> bcopy(buf, &sess, sizeof(sess));
>
> if (ntohs(sess.length) != SESS_PRMS_SIZE - TLV_HDR_LEN ||
> - ntohs(sess.length) != len - TLV_HDR_LEN) {
> + ntohs(sess.length) > len - TLV_HDR_LEN) {
> session_shutdown(nbr, S_BAD_TLV_LEN, init.msgid, init.type);
> return (-1);
> }
>
> - /* ATM and Frame Relay optional attributes not supported */
> + buf += SESS_PRMS_SIZE;
> + len -= SESS_PRMS_SIZE;
> +
> + /* just ignore all optional TLVs for now */
> + if (tlv_decode_opt_init_prms(buf, len) == -1) {
> + session_shutdown(nbr, S_BAD_TLV_VAL, init.msgid, init.type);
> + return (-1);
> + }
>
> if (nbr->iface->keepalive < ntohs(sess.keepalive_time))
> nbr->keepalive = nbr->iface->keepalive;
> @@ -126,4 +134,35 @@ gen_init_prms_tlv(struct ibuf *buf, stru
> parms.lspace_id = 0;
>
> return (ibuf_add(buf, &parms, SESS_PRMS_SIZE));
> +}
> +
> +int
> +tlv_decode_opt_init_prms(char *buf, u_int16_t len)
> +{
> + struct tlv tlv;
> + int cons = 0;
> + u_int16_t tlv_len;
> +
> + while (len >= sizeof(tlv)) {
> + bcopy(buf, &tlv, sizeof(tlv));
> + tlv_len = ntohs(tlv.length);
> + switch (ntohs(tlv.type)) {
> + case TLV_TYPE_ATMSESSIONPAR:
> + log_warnx("ATM session parameter present");
> + return (-1);
> + case TLV_TYPE_FRSESSION:
> + log_warnx("FR session parameter present");
> + return (-1);
> + default:
> + /* if unknown flag set, ignore TLV */
> + if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
> + return (-1);
> + break;
> + }
> + buf += TLV_HDR_LEN + tlv_len;
> + len -= TLV_HDR_LEN + tlv_len;
> + cons += TLV_HDR_LEN + tlv_len;
> + }
> +
> + return (cons);
> }
> Index: labelmapping.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/labelmapping.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 labelmapping.c
> --- labelmapping.c 31 Dec 2010 21:22:42 -0000 1.18
> +++ labelmapping.c 9 Jan 2011 14:33:51 -0000
> @@ -93,7 +93,7 @@ int
> recv_labelmapping(struct nbr *nbr, char *buf, u_int16_t len)
> {
> struct ldp_msg lm;
> - struct fec_tlv ft;
> + struct tlv ft;
> struct map map;
> u_int32_t label;
> int feclen, lbllen, tlen;
> @@ -202,7 +202,7 @@ int
> recv_labelrequest(struct nbr *nbr, char *buf, u_int16_t len)
> {
> struct ldp_msg lr;
> - struct fec_tlv ft;
> + struct tlv ft;
> struct map map;
> int feclen, tlen;
> u_int8_t addr_type;
> @@ -308,7 +308,7 @@ recv_labelwithdraw(struct nbr *nbr, char
> {
> struct map map;
> struct ldp_msg lw;
> - struct fec_tlv ft;
> + struct tlv ft;
> u_int32_t label = NO_LABEL;
> int feclen, tlen, numfec = 0;
> u_int8_t addr_type;
> @@ -449,7 +449,7 @@ recv_labelrelease(struct nbr *nbr, char
> {
> struct map map;
> struct ldp_msg lr;
> - struct fec_tlv ft;
> + struct tlv ft;
> u_int32_t label = NO_LABEL;
> int feclen, tlen, numfec = 0;
> u_int8_t addr_type;
> @@ -568,7 +568,7 @@ recv_labelabortreq(struct nbr *nbr, char
> {
> struct map map;
> struct ldp_msg la;
> - struct fec_tlv ft;
> + struct tlv ft;
> int feclen, tlen;
> u_int8_t addr_type;
>
> @@ -707,7 +707,7 @@ tlv_decode_reqid(char *buf, u_int16_t le
> void
> gen_fec_tlv(struct ibuf *buf, struct in_addr prefix, u_int8_t prefixlen)
> {
> - struct fec_tlv ft;
> + struct tlv ft;
> u_int8_t type;
> u_int16_t family;
> u_int8_t len;
> Index: ldp.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/ldp.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 ldp.h
> --- ldp.h 4 Nov 2010 09:52:16 -0000 1.7
> +++ ldp.h 9 Jan 2011 14:34:20 -0000
> @@ -97,6 +97,12 @@ struct ldp_hdr {
> #define INFINITE_HOLDTIME 0xffff
>
> /* TLV record */
> +struct tlv {
> + u_int16_t type;
> + u_int16_t length;
> +};
> +#define TLV_HDR_LEN 4
> +
> struct ldp_msg {
> u_int16_t type;
> u_int16_t length;
> @@ -106,7 +112,6 @@ struct ldp_msg {
> } __packed;
>
> #define LDP_MSG_LEN 8
> -#define TLV_HDR_LEN 4
>
> #define UNKNOWN_FLAGS_MASK 0xc000
> #define UNKNOWN_FLAG 0x8000
> @@ -189,12 +194,6 @@ struct address_list_tlv {
> #define ADDR_IPV4 0x1
> #define ADDR_IPV6 0x2
>
> -struct fec_tlv {
> - u_int16_t type;
> - u_int16_t length;
> - /* fec elm entries */
> -};
> -
> /* This struct is badly aligned so use two 32 bit fields */
> struct fec_elm {
> u_int32_t hdr;
> @@ -202,10 +201,8 @@ struct fec_elm {
> };
>
> #define FEC_ELM_MIN_LEN 4
> -
> #define FEC_WILDCARD 0x01
> #define FEC_PREFIX 0x02
> -
> #define FEC_IPV4 0x0001
>
> struct label_tlv {
> @@ -223,12 +220,6 @@ struct reqid_tlv {
> };
>
> #define REQID_TLV_LEN 8
> -
> -struct hello_opt_parms_tlv {
> - u_int16_t type;
> - u_int16_t length;
> - u_int32_t value;
> -};
>
> #define NO_LABEL UINT_MAX
