The fix should be in now. Thanks for the report, and the diff pointing
right at the problem.
On Tue., 27 Aug. 2019, 01:45 Gerrie Roos, <gr...@xiplink.com> wrote:
> I just tested your patch and it works! I can see the correct TTL.
>
> -----Original Message-----
> From: "David Gwynne" <da...@gwynne.id.au>
> Sent: Monday, 26 August, 2019 4:21am
> To: "Gerrie Roos" <gr...@xiplink.com>
> Cc: bugs@openbsd.org
> Subject: Re: Bad TTL when applying multiple MPLS labels
>
> On Fri, Aug 23, 2019 at 10:05:25AM -0500, Gerrie Roos wrote:
> > Hi David,
> >
> > Apologies, I haven't used CVS in a while - here you go. Hope it's useful.
> >
> > Gerrie
>
> npz, i can read the attachment fine.
>
> if someone's carrying a non-ip payload over mpls with a nibble that
> makes it look like ip, your diff will cause (very) short payloads
> to be dropped when m_pullup can't find bytes to copy into the first
> mbuf.
>
> could you try the following? it uses m_getptr to reach into the right
> mbuf and byte offset in that mbuf for the ttl. if the packet is
> short, it just returns the default ttl.
>
> i've attached the diff for your convenience too.
>
> cheers,
> dlg
>
> Index: mpls_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netmpls/mpls_output.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 mpls_output.c
> --- mpls_output.c 2 Dec 2015 08:47:00 -0000 1.26
> +++ mpls_output.c 26 Aug 2019 09:11:08 -0000
> @@ -162,41 +162,39 @@ mpls_do_cksum(struct mbuf *m)
> u_int8_t
> mpls_getttl(struct mbuf *m, sa_family_t af)
> {
> - struct shim_hdr *shim;
> - struct ip *ip;
> -#ifdef INET6
> - struct ip6_hdr *ip6hdr;
> -#endif
> + struct mbuf *n;
> + int loc, off;
> u_int8_t ttl = mpls_defttl;
>
> /* If the AF is MPLS then inherit the TTL from the present label.
> */
> - if (af == AF_MPLS) {
> - shim = mtod(m, struct shim_hdr *);
> - ttl = ntohl(shim->shim_label & MPLS_TTL_MASK);
> - return (ttl);
> - }
> - /* Else extract TTL from the encapsualted packet. */
> - switch (*mtod(m, u_char *) >> 4) {
> - case IPVERSION:
> - if (!mpls_mapttl_ip)
> + if (af == AF_MPLS)
> + loc = 3;
> + else {
> + switch (*mtod(m, uint8_t *) >> 4) {
> + case 4:
> + if (!mpls_mapttl_ip)
> + return (ttl);
> +
> + loc = offsetof(struct ip, ip_ttl);
> break;
> - if (m->m_len < sizeof(*ip))
> - break; /* impossible */
> - ip = mtod(m, struct ip *);
> - ttl = ip->ip_ttl;
> - break;
> #ifdef INET6
> - case IPV6_VERSION >> 4:
> - if (!mpls_mapttl_ip6)
> + case 6:
> + if (!mpls_mapttl_ip6)
> + break;
> +
> + loc = offsetof(struct ip6_hdr, ip6_hlim);
> break;
> - if (m->m_len < sizeof(struct ip6_hdr))
> - break; /* impossible */
> - ip6hdr = mtod(m, struct ip6_hdr *);
> - ttl = ip6hdr->ip6_hlim;
> - break;
> #endif
> - default:
> - break;
> + default:
> + return (ttl);
> + }
> }
> +
> + n = m_getptr(m, loc, &off);
> + if (n == NULL)
> + return (ttl);
> +
> + ttl = *(mtod(n, uint8_t *) + off);
> +
> return (ttl);
> }
>
>
>
