On Tue, Dec 24, 2019 at 10:02:37PM +0100, Denis Fondras wrote:
> Refactor link state ack/req in ospf6d so it looks closer to ospfd.
ok remi@
> Index: lsack.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/lsack.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 lsack.c
> --- lsack.c 11 Dec 2019 21:33:56 -0000 1.7
> +++ lsack.c 24 Dec 2019 20:51:56 -0000
> @@ -19,7 +19,7 @@
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <netinet/in.h>
> -#include <netinet/ip.h>
> +#include <netinet/ip6.h>
> #include <arpa/inet.h>
>
> #include <stdlib.h>
> @@ -30,39 +30,66 @@
> #include "log.h"
> #include "ospfe.h"
>
> -void start_ls_ack_tx_timer_now(struct iface *);
> +int send_ls_ack(struct iface *, struct in6_addr, struct ibuf *);
> +struct ibuf *prepare_ls_ack(struct iface *);
> +void start_ls_ack_tx_timer_now(struct iface *);
>
> /* link state acknowledgement packet handling */
> -int
> -send_ls_ack(struct iface *iface, struct in6_addr addr, void *data, size_t
> len)
> +struct ibuf *
> +prepare_ls_ack(struct iface *iface)
> {
> struct ibuf *buf;
> - int ret;
>
> - /* XXX IBUF_READ_SIZE */
> - if ((buf = ibuf_dynamic(PKG_DEF_SIZE, IBUF_READ_SIZE)) == NULL)
> - fatal("send_ls_ack");
> + if ((buf = ibuf_open(iface->mtu - sizeof(struct ip6_hdr))) == NULL) {
> + log_warn("prepare_ls_ack");
> + return (NULL);
> + }
>
> /* OSPF header */
> - if (gen_ospf_hdr(buf, iface, PACKET_TYPE_LS_ACK))
> - goto fail;
> + if (gen_ospf_hdr(buf, iface, PACKET_TYPE_LS_ACK)) {
> + log_warn("prepare_ls_ack");
> + ibuf_free(buf);
> + return (NULL);
> + }
>
> - /* LS ack(s) */
> - if (ibuf_add(buf, data, len))
> - goto fail;
> + return (buf);
> +}
>
> +int
> +send_ls_ack(struct iface *iface, struct in6_addr addr, struct ibuf *buf)
> +{
> /* calculate checksum */
> - if (upd_ospf_hdr(buf, iface))
> - goto fail;
> + if (upd_ospf_hdr(buf, iface)) {
> + log_warn("send_ls_ack");
> + return (-1);
> + }
>
> - ret = send_packet(iface, buf, &addr);
> + if (send_packet(iface, buf, &addr) == -1) {
> + log_warn("send_ls_ack");
> + return (-1);
> + }
> + return (0);
> +}
>
> +int
> +send_direct_ack(struct iface *iface, struct in6_addr addr, void *d, size_t
> len)
> +{
> + struct ibuf *buf;
> + int ret;
> +
> + if ((buf = prepare_ls_ack(iface)) == NULL)
> + return (-1);
> +
> + /* LS ack(s) */
> + if (ibuf_add(buf, d, len)) {
> + log_warn("send_direct_ack");
> + ibuf_free(buf);
> + return (-1);
> + }
> +
> + ret = send_ls_ack(iface, addr, buf);
> ibuf_free(buf);
> return (ret);
> -fail:
> - log_warn("send_ls_ack");
> - ibuf_free(buf);
> - return (-1);
> }
>
> void
> @@ -207,41 +234,44 @@ ls_ack_tx_timer(int fd, short event, voi
> {
> struct in6_addr addr;
> struct iface *iface = arg;
> - struct lsa_hdr *lsa_hdr;
> struct lsa_entry *le, *nle;
> struct nbr *nbr;
> - char *buf;
> - char *ptr;
> - int cnt = 0;
> -
> - if ((buf = calloc(1, READ_BUF_SIZE)) == NULL)
> - fatal("ls_ack_tx_timer");
> + struct ibuf *buf;
> + int cnt;
>
> while (!ls_ack_list_empty(iface)) {
> - ptr = buf;
> + if ((buf = prepare_ls_ack(iface)) == NULL)
> + fatal("ls_ack_tx_timer");
> cnt = 0;
> - for (le = TAILQ_FIRST(&iface->ls_ack_list); le != NULL &&
> - (ptr - buf < iface->mtu - PACKET_HDR); le = nle) {
> +
> + for (le = TAILQ_FIRST(&iface->ls_ack_list); le != NULL;
> + le = nle) {
> nle = TAILQ_NEXT(le, entry);
> - memcpy(ptr, le->le_lsa, sizeof(struct lsa_hdr));
> - ptr += sizeof(*lsa_hdr);
> + if (ibuf_left(buf) < sizeof(struct lsa_hdr))
> + break;
> + if (ibuf_add(buf, le->le_lsa, sizeof(struct lsa_hdr)))
> + break;
> ls_ack_list_free(iface, le);
> cnt++;
> }
> + if (cnt == 0) {
> + log_warnx("ls_ack_tx_timer: lost in space");
> + ibuf_free(buf);
> + return;
> + }
>
> /* send LS ack(s) but first set correct destination */
> switch (iface->type) {
> case IF_TYPE_POINTOPOINT:
> inet_pton(AF_INET6, AllSPFRouters, &addr);
> - send_ls_ack(iface, addr, buf, ptr - buf);
> + send_ls_ack(iface, addr, buf);
> break;
> case IF_TYPE_BROADCAST:
> if (iface->state & IF_STA_DRORBDR)
> inet_pton(AF_INET6, AllSPFRouters, &addr);
> else
> inet_pton(AF_INET6, AllDRouters, &addr);
> -
> - send_ls_ack(iface, addr, buf, ptr - buf);
> + send_ls_ack(iface, addr, buf);
> break;
> case IF_TYPE_NBMA:
> case IF_TYPE_POINTOMULTIPOINT:
> @@ -251,15 +281,14 @@ ls_ack_tx_timer(int fd, short event, voi
> continue;
> if (!(nbr->state & NBR_STA_FLOOD))
> continue;
> - send_ls_ack(iface, nbr->addr, buf, ptr - buf);
> + send_ls_ack(iface, nbr->addr, buf);
> }
> break;
> default:
> fatalx("lsa_ack_tx_timer: unknown interface type");
> }
> + ibuf_free(buf);
> }
> -
> - free(buf);
> }
>
> void
> Index: lsreq.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/lsreq.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 lsreq.c
> --- lsreq.c 11 Dec 2019 21:33:56 -0000 1.10
> +++ lsreq.c 24 Dec 2019 20:51:56 -0000
> @@ -38,7 +38,6 @@ send_ls_req(struct nbr *nbr)
> struct ls_req_hdr ls_req_hdr;
> struct lsa_entry *le, *nle;
> struct ibuf *buf;
> - int ret;
>
> if ((buf = ibuf_open(nbr->iface->mtu - sizeof(struct ip6_hdr))) == NULL)
> fatal("send_ls_req");
> @@ -77,10 +76,11 @@ send_ls_req(struct nbr *nbr)
> if (upd_ospf_hdr(buf, nbr->iface))
> goto fail;
>
> - ret = send_packet(nbr->iface, buf, &dst);
> + if (send_packet(nbr->iface, buf, &dst) == -1)
> + goto fail;
>
> ibuf_free(buf);
> - return (ret);
> + return (0);
> fail:
> log_warn("send_ls_req");
> ibuf_free(buf);
> Index: lsupdate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/lsupdate.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 lsupdate.c
> --- lsupdate.c 11 Dec 2019 21:33:56 -0000 1.14
> +++ lsupdate.c 24 Dec 2019 20:51:56 -0000
> @@ -37,12 +37,14 @@
> extern struct ospfd_conf *oeconf;
> extern struct imsgev *iev_rde;
>
> -struct ibuf *prepare_ls_update(struct iface *, int);
> -int add_ls_update(struct ibuf *, struct iface *, void *, int, u_int16_t);
> -int send_ls_update(struct ibuf *, struct iface *, struct in6_addr,
> u_int32_t);
> +struct ibuf *prepare_ls_update(struct iface *);
> +int add_ls_update(struct ibuf *, struct iface *, void *, u_int16_t,
> + u_int16_t);
> +int send_ls_update(struct ibuf *, struct iface *, struct in6_addr,
> + u_int32_t);
>
> -void ls_retrans_list_insert(struct nbr *, struct lsa_entry *);
> -void ls_retrans_list_remove(struct nbr *, struct lsa_entry *);
> +void ls_retrans_list_insert(struct nbr *, struct lsa_entry *);
> +void ls_retrans_list_remove(struct nbr *, struct lsa_entry *);
>
> /* link state update packet handling */
> int
> @@ -149,27 +151,24 @@ lsa_flood(struct iface *iface, struct nb
> }
>
> struct ibuf *
> -prepare_ls_update(struct iface *iface, int bigpkt)
> +prepare_ls_update(struct iface *iface)
> {
> struct ibuf *buf;
> - size_t size;
> + size_t reserved;
>
> - size = bigpkt ? IPV6_MAXPACKET : iface->mtu;
> - if (size < IPV6_MMTU)
> - size = IPV6_MMTU;
> - size -= sizeof(struct ip6_hdr);
> /*
> * Reserve space for optional ah or esp encryption. The
> * algorithm is taken from ah_output and esp_output, the
> * values are the maxima of crypto/xform.c.
> */
> - size -= max(
> + reserved = max(
> /* base-ah-header replay authsize */
> AH_FLENGTH + sizeof(u_int32_t) + 32,
> /* spi sequence ivlen blocksize pad-length next-header authsize */
> 2 * sizeof(u_int32_t) + 16 + 16 + 2 * sizeof(u_int8_t) + 32);
>
> - if ((buf = ibuf_open(size)) == NULL)
> + if ((buf = ibuf_dynamic(IPV6_MMTU - sizeof(struct ip6_hdr) - reserved,
> + IPV6_MAXPACKET - sizeof(struct ip6_hdr) - reserved)) == NULL)
> fatal("prepare_ls_update");
>
> /* OSPF header */
> @@ -188,16 +187,16 @@ fail:
> }
>
> int
> -add_ls_update(struct ibuf *buf, struct iface *iface, void *data, int len,
> +add_ls_update(struct ibuf *buf, struct iface *iface, void *data, u_int16_t
> len,
> u_int16_t older)
> {
> - size_t pos;
> - u_int16_t age;
> + void *lsage;
> + u_int16_t age;
>
> if (buf->wpos + len >= buf->max)
> return (0);
>
> - pos = buf->wpos;
> + lsage = ibuf_reserve(buf, 0);
> if (ibuf_add(buf, data, len)) {
> log_warn("add_ls_update");
> return (0);
> @@ -209,7 +208,7 @@ add_ls_update(struct ibuf *buf, struct i
> if ((age += older + iface->transmit_delay) >= MAX_AGE)
> age = MAX_AGE;
> age = htons(age);
> - memcpy(ibuf_seek(buf, pos, sizeof(age)), &age, sizeof(age));
> + memcpy(lsage, &age, sizeof(age));
>
> return (1);
> }
> @@ -218,8 +217,6 @@ int
> send_ls_update(struct ibuf *buf, struct iface *iface, struct in6_addr addr,
> u_int32_t nlsa)
> {
> - int ret;
> -
> nlsa = htonl(nlsa);
> memcpy(ibuf_seek(buf, sizeof(struct ospf_hdr), sizeof(nlsa)),
> &nlsa, sizeof(nlsa));
> @@ -227,10 +224,11 @@ send_ls_update(struct ibuf *buf, struct
> if (upd_ospf_hdr(buf, iface))
> goto fail;
>
> - ret = send_packet(iface, buf, &addr);
> + if (send_packet(iface, buf, &addr) == -1)
> + goto fail;
>
> ibuf_free(buf);
> - return (ret);
> + return (0);
> fail:
> log_warn("send_ls_update");
> ibuf_free(buf);
> @@ -443,7 +441,7 @@ ls_retrans_timer(int fd, short event, vo
> struct lsa_entry *le;
> struct ibuf *buf;
> time_t now;
> - int bigpkt, d;
> + int d;
> u_int32_t nlsa = 0;
>
> if ((le = TAILQ_FIRST(&nbr->ls_retrans_list)) != NULL)
> @@ -479,17 +477,7 @@ ls_retrans_timer(int fd, short event, vo
> } else
> memcpy(&addr, &nbr->addr, sizeof(addr));
>
> - /*
> - * Allow big ipv6 packets that may get fragmented if a
> - * single lsa might be too big for an unfragmented packet.
> - * To avoid the exact algorithm duplicated here, just make
> - * a good guess. If the first lsa is bigger than 1024
> - * bytes, reserve a separate big packet for it. The kernel
> - * will figure out if fragmentation is necessary. For
> - * smaller lsas, we avoid big packets and fragmentation.
> - */
> - bigpkt = le->le_ref->len > 1024;
> - if ((buf = prepare_ls_update(nbr->iface, bigpkt)) == NULL) {
> + if ((buf = prepare_ls_update(nbr->iface)) == NULL) {
> le->le_when = 1;
> goto done;
> }
> @@ -504,18 +492,20 @@ ls_retrans_timer(int fd, short event, vo
>
> if (add_ls_update(buf, nbr->iface, le->le_ref->data,
> le->le_ref->len, d) == 0) {
> - if (nlsa)
> - break;
> - /*
> - * A single lsa is too big to fit into an update
> - * packet. In this case drop the lsa, otherwise
> - * we send empty update packets in an endless loop.
> - */
> - log_warnx("ls_retrans_timer: cannot send lsa, dropped");
> - log_debug("ls_retrans_timer: type: %04x len: %u",
> - ntohs(le->le_ref->hdr.type), le->le_ref->len);
> - ls_retrans_list_free(nbr, le);
> - continue;
> + if (nlsa == 0) {
> + /* something bad happened retry later */
> + log_warnx("ls_retrans_timer: sending LS update "
> + "to neighbor ID %s failed",
> + inet_ntoa(nbr->id));
> + log_debug("ls_retrans_timer: type: %04x len:
> %u",
> + ntohs(le->le_ref->hdr.type),
> + le->le_ref->len);
> + TAILQ_REMOVE(&nbr->ls_retrans_list, le, entry);
> + nbr->ls_ret_cnt--;
> + le->le_when = nbr->iface->rxmt_interval;
> + ls_retrans_list_insert(nbr, le);
> + }
> + break;
> }
> nlsa++;
> if (le->le_oneshot)
> @@ -526,11 +516,11 @@ ls_retrans_timer(int fd, short event, vo
> le->le_when = nbr->iface->rxmt_interval;
> ls_retrans_list_insert(nbr, le);
> }
> - /* do not put additional lsa into fragmented big packet */
> - if (bigpkt)
> - break;
> }
> - send_ls_update(buf, nbr->iface, addr, nlsa);
> + if (nlsa)
> + send_ls_update(buf, nbr->iface, addr, nlsa);
> + else
> + ibuf_free(buf);
>
> done:
> if ((le = TAILQ_FIRST(&nbr->ls_retrans_list)) != NULL) {
> Index: ospfe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 ospfe.c
> --- ospfe.c 23 Dec 2019 07:33:49 -0000 1.58
> +++ ospfe.c 24 Dec 2019 20:51:56 -0000
> @@ -699,7 +699,7 @@ ospfe_dispatch_rde(int fd, short event,
> break;
>
> /* send a direct acknowledgement */
> - send_ls_ack(nbr->iface, nbr->addr, imsg.data,
> + send_direct_ack(nbr->iface, nbr->addr, imsg.data,
> imsg.hdr.len - IMSG_HEADER_SIZE);
>
> break;
> Index: ospfe.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.h,v
> retrieving revision 1.21
> diff -u -p -r1.21 ospfe.h
> --- ospfe.h 22 Dec 2019 15:34:52 -0000 1.21
> +++ ospfe.h 24 Dec 2019 20:51:56 -0000
> @@ -149,7 +149,7 @@ int if_set_ipv6_checksum(int);
>
> /* lsack.c */
> int delay_lsa_ack(struct iface *, struct lsa_hdr *);
> -int send_ls_ack(struct iface *, struct in6_addr, void *, size_t);
> +int send_direct_ack(struct iface *, struct in6_addr, void *, size_t);
> void recv_ls_ack(struct nbr *, char *, u_int16_t);
> int lsa_hdr_check(struct nbr *, struct lsa_hdr *);
> void ls_ack_list_add(struct iface *, struct lsa_hdr *);
>
