On Tue, Jun 20, 2023 at 06:29:59PM +0200, Claudio Jeker wrote:
> On Tue, Jun 20, 2023 at 02:47:41PM +0200, Claudio Jeker wrote:
> > This diff updates ospfd to use the new ibuf API.
> >
> > It mainly removes the use of ibuf_seek() and replaces these calls with
> > ibuf_set().
> >
> > Regress still passes with this diff in.
>
> Here the same diff for ospf6d.
ok
Two minor comments below. Feel free to ignore them.
> --
> :wq Claudio
>
> Index: database.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/database.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 database.c
> --- database.c 8 Mar 2023 04:43:14 -0000 1.22
> +++ database.c 16 Jun 2023 10:27:04 -0000
> @@ -51,7 +51,7 @@ send_db_description(struct nbr *nbr)
> goto fail;
>
> /* reserve space for database description header */
> - if (ibuf_reserve(buf, sizeof(dd_hdr)) == NULL)
> + if (ibuf_add_zero(buf, sizeof(dd_hdr)) == -1)
> goto fail;
>
> switch (nbr->state) {
> @@ -134,8 +134,9 @@ send_db_description(struct nbr *nbr)
> dd_hdr.bits = bits;
> dd_hdr.dd_seq_num = htonl(nbr->dd_seq_num);
>
> - memcpy(ibuf_seek(buf, sizeof(struct ospf_hdr), sizeof(dd_hdr)),
> - &dd_hdr, sizeof(dd_hdr));
> + if (ibuf_set(buf, sizeof(struct ospf_hdr), &dd_hdr,
> + sizeof(dd_hdr)) == -1)
> + goto fail;
>
> /* calculate checksum */
> if (upd_ospf_hdr(buf, nbr->iface))
> Index: lsupdate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/lsupdate.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 lsupdate.c
> --- lsupdate.c 8 Mar 2023 04:43:14 -0000 1.22
> +++ lsupdate.c 16 Jun 2023 10:27:15 -0000
> @@ -177,7 +177,7 @@ prepare_ls_update(struct iface *iface, i
> goto fail;
>
> /* reserve space for number of lsa field */
> - if (ibuf_reserve(buf, sizeof(u_int32_t)) == NULL)
> + if (ibuf_add_zero(buf, sizeof(u_int32_t)) == -1)
> goto fail;
>
> return (buf);
> @@ -208,8 +208,10 @@ add_ls_update(struct ibuf *buf, struct i
> age = ntohs(age);
> if ((age += older + iface->transmit_delay) >= MAX_AGE)
> age = MAX_AGE;
> - age = htons(age);
> - memcpy(ibuf_seek(buf, ageoff, sizeof(age)), &age, sizeof(age));
> + if (ibuf_set_n16(buf, ageoff, age) == -1) {
> + log_warn("add_ls_update");
This error is weid but it's like the others nearby.
> + return (0);
> + }
>
> return (1);
> }
> @@ -218,9 +220,8 @@ int
> send_ls_update(struct ibuf *buf, struct iface *iface, struct in6_addr addr,
> u_int32_t nlsa)
> {
> - nlsa = htonl(nlsa);
> - memcpy(ibuf_seek(buf, sizeof(struct ospf_hdr), sizeof(nlsa)),
> - &nlsa, sizeof(nlsa));
> + if (ibuf_set_n32(buf, sizeof(struct ospf_hdr), nlsa) == -1)
> + goto fail;
> /* calculate checksum */
> if (upd_ospf_hdr(buf, iface))
> goto fail;
> Index: ospfe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/ospfe.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 ospfe.c
> --- ospfe.c 8 Mar 2023 04:43:14 -0000 1.68
> +++ ospfe.c 20 Jun 2023 16:25:52 -0000
> @@ -780,11 +780,11 @@ orig_rtr_lsa(struct area *area)
> fatal("orig_rtr_lsa");
>
> /* reserve space for LSA header and LSA Router header */
> - if (ibuf_reserve(buf, sizeof(lsa_hdr)) == NULL)
> - fatal("orig_rtr_lsa: ibuf_reserve failed");
> + if (ibuf_add_zero(buf, sizeof(lsa_hdr)) == -1)
> + fatal("orig_rtr_lsa: ibuf_add_zero failed");
>
> - if (ibuf_reserve(buf, sizeof(lsa_rtr)) == NULL)
> - fatal("orig_rtr_lsa: ibuf_reserve failed");
> + if (ibuf_add_zero(buf, sizeof(lsa_rtr)) == -1)
> + fatal("orig_rtr_lsa: ibuf_add_zero failed");
These two could be combined like the others below, but I'm fine with
keeping it as close to mechanical as possible.
>
> /* links */
> LIST_FOREACH(iface, &area->iface_list, entry) {
> @@ -944,8 +944,8 @@ orig_rtr_lsa(struct area *area)
> LSA_24_SETLO(lsa_rtr.opts, area_ospf_options(area));
> LSA_24_SETHI(lsa_rtr.opts, flags);
> lsa_rtr.opts = htonl(lsa_rtr.opts);
> - memcpy(ibuf_seek(buf, sizeof(lsa_hdr), sizeof(lsa_rtr)),
> - &lsa_rtr, sizeof(lsa_rtr));
> + if (ibuf_set(buf, sizeof(lsa_hdr), &lsa_rtr, sizeof(lsa_rtr)) == -1)
> + fatal("orig_rtr_lsa: ibuf_set failed");
>
> /* LSA header */
> lsa_hdr.age = htons(DEFAULT_AGE);
> @@ -956,11 +956,12 @@ orig_rtr_lsa(struct area *area)
> lsa_hdr.seq_num = htonl(INIT_SEQ_NUM);
> lsa_hdr.len = htons(buf->wpos);
> lsa_hdr.ls_chksum = 0; /* updated later */
> - memcpy(ibuf_seek(buf, 0, sizeof(lsa_hdr)), &lsa_hdr, sizeof(lsa_hdr));
> + if (ibuf_set(buf, 0, &lsa_hdr, sizeof(lsa_hdr)) == -1)
> + fatal("orig_rtr_lsa: ibuf_set failed");
>
> - chksum = htons(iso_cksum(buf->buf, buf->wpos, LS_CKSUM_OFFSET));
> - memcpy(ibuf_seek(buf, LS_CKSUM_OFFSET, sizeof(chksum)),
> - &chksum, sizeof(chksum));
> + chksum = iso_cksum(buf->buf, buf->wpos, LS_CKSUM_OFFSET);
> + if (ibuf_set_n16(buf, LS_CKSUM_OFFSET, chksum) == -1)
> + fatal("orig_rtr_lsa: ibuf_set_n16 failed");
>
> if (self)
> imsg_compose_event(iev_rde, IMSG_LS_UPD, self->peerid, 0,
> @@ -987,8 +988,8 @@ orig_net_lsa(struct iface *iface)
> fatal("orig_net_lsa");
>
> /* reserve space for LSA header and options field */
> - if (ibuf_reserve(buf, sizeof(lsa_hdr) + sizeof(lsa_net)) == NULL)
> - fatal("orig_net_lsa: ibuf_reserve failed");
> + if (ibuf_add_zero(buf, sizeof(lsa_hdr) + sizeof(lsa_net)) == -1)
> + fatal("orig_net_lsa: ibuf_add_zero failed");
>
> lsa_net.opts = 0;
> /* fully adjacent neighbors + self */
> @@ -1019,15 +1020,16 @@ orig_net_lsa(struct iface *iface)
> lsa_hdr.seq_num = htonl(INIT_SEQ_NUM);
> lsa_hdr.len = htons(buf->wpos);
> lsa_hdr.ls_chksum = 0; /* updated later */
> - memcpy(ibuf_seek(buf, 0, sizeof(lsa_hdr)), &lsa_hdr, sizeof(lsa_hdr));
> + if (ibuf_set(buf, 0, &lsa_hdr, sizeof(lsa_hdr)) == -1)
> + fatal("orig_net_lsa: ibuf_set failed");
>
> lsa_net.opts &= lsa_net.opts & htonl(LSA_24_MASK);
> - memcpy(ibuf_seek(buf, sizeof(lsa_hdr), sizeof(lsa_net)), &lsa_net,
> - sizeof(lsa_net));
> + if (ibuf_set(buf, sizeof(lsa_hdr), &lsa_net, sizeof(lsa_net)) == -1)
> + fatal("orig_net_lsa: ibuf_set failed");
>
> - chksum = htons(iso_cksum(buf->buf, buf->wpos, LS_CKSUM_OFFSET));
> - memcpy(ibuf_seek(buf, LS_CKSUM_OFFSET, sizeof(chksum)),
> - &chksum, sizeof(chksum));
> + chksum = iso_cksum(buf->buf, buf->wpos, LS_CKSUM_OFFSET);
> + if (ibuf_set_n16(buf, LS_CKSUM_OFFSET, chksum) == -1)
> + fatal("orig_net_lsa: ibuf_set_n16 failed");
>
> imsg_compose_event(iev_rde, IMSG_LS_UPD, iface->self->peerid, 0,
> -1, buf->buf, buf->wpos);
> @@ -1073,8 +1075,8 @@ orig_link_lsa(struct iface *iface)
> fatal("orig_link_lsa");
>
> /* reserve space for LSA header and LSA link header */
> - if (ibuf_reserve(buf, sizeof(lsa_hdr) + sizeof(lsa_link)) == NULL)
> - fatal("orig_link_lsa: ibuf_reserve failed");
> + if (ibuf_add_zero(buf, sizeof(lsa_hdr) + sizeof(lsa_link)) == -1)
> + fatal("orig_link_lsa: ibuf_add_zero failed");
>
> /* link-local address, and all prefixes configured on interface */
> TAILQ_FOREACH(ia, &iface->ifa_list, entry) {
> @@ -1104,8 +1106,8 @@ orig_link_lsa(struct iface *iface)
> LSA_24_SETLO(lsa_link.opts, options);
> lsa_link.opts = htonl(lsa_link.opts);
> lsa_link.numprefix = htonl(num_prefix);
> - memcpy(ibuf_seek(buf, sizeof(lsa_hdr), sizeof(lsa_link)),
> - &lsa_link, sizeof(lsa_link));
> + if (ibuf_set(buf, sizeof(lsa_hdr), &lsa_link, sizeof(lsa_link)) == -1)
> + fatal("orig_link_lsa: ibuf_set failed");
>
> /* LSA header */
> lsa_hdr.age = htons(DEFAULT_AGE);
> @@ -1116,11 +1118,12 @@ orig_link_lsa(struct iface *iface)
> lsa_hdr.seq_num = htonl(INIT_SEQ_NUM);
> lsa_hdr.len = htons(buf->wpos);
> lsa_hdr.ls_chksum = 0; /* updated later */
> - memcpy(ibuf_seek(buf, 0, sizeof(lsa_hdr)), &lsa_hdr, sizeof(lsa_hdr));
> + if (ibuf_set(buf, 0, &lsa_hdr, sizeof(lsa_hdr)) == -1)
> + fatal("orig_link_lsa: ibuf_set failed");
>
> - chksum = htons(iso_cksum(buf->buf, buf->wpos, LS_CKSUM_OFFSET));
> - memcpy(ibuf_seek(buf, LS_CKSUM_OFFSET, sizeof(chksum)),
> - &chksum, sizeof(chksum));
> + chksum = iso_cksum(buf->buf, buf->wpos, LS_CKSUM_OFFSET);
> + if (ibuf_set_n16(buf, LS_CKSUM_OFFSET, chksum) == -1)
> + fatal("orig_link_lsa: ibuf_set_n16 failed");
>
> imsg_compose_event(iev_rde, IMSG_LS_UPD, iface->self->peerid, 0,
> -1, buf->buf, buf->wpos);
> Index: packet.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/packet.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 packet.c
> --- packet.c 19 Jan 2021 16:02:06 -0000 1.20
> +++ packet.c 16 Jun 2023 10:27:59 -0000
> @@ -64,16 +64,16 @@ gen_ospf_hdr(struct ibuf *buf, struct if
> int
> upd_ospf_hdr(struct ibuf *buf, struct iface *iface)
> {
> - struct ospf_hdr *ospf_hdr;
> -
> - if ((ospf_hdr = ibuf_seek(buf, 0, sizeof(*ospf_hdr))) == NULL)
> - fatalx("upd_ospf_hdr: buf_seek failed");
> -
> /* update length */
> - if (buf->wpos > USHRT_MAX)
> + if (ibuf_size(buf) > USHRT_MAX)
> fatalx("upd_ospf_hdr: resulting ospf packet too big");
> - ospf_hdr->len = htons((u_int16_t)buf->wpos);
> - ospf_hdr->chksum = 0; /* calculated via IPV6_CHECKSUM */
> + if (ibuf_set_n16(buf, offsetof(struct ospf_hdr, len),
> + ibuf_size(buf)) == -1)
> + fatalx("upd_ospf_hdr: ibuf_set_n16 failed");
> +
> + /* checksum calculated via IPV6_CHECKSUM */
> + if (ibuf_set_n16(buf, offsetof(struct ospf_hdr, chksum), 0) == -1)
> + fatalx("upd_ospf_hdr: ibuf_set_n16 failed");
>
> return (0);
> }
>
