On Mon, Jul 15, 2019 at 07:32:18AM +0200, Remi Locherer wrote: > Hi, > > I'd like to improve ospfd's logging when sending a packet fails. > > I got a debug output from a ospfd user which contains "send packet: error > ...". > I guess ospfd failed to send an ls ack. With below diff applied it would be > clear which packet could not be sent and to which neighbor. > > OK?
Sure, OK claudio@ Guess ospf6d could use a similar diff. > > Index: database.c > =================================================================== > RCS file: /cvs/src/usr.sbin/ospfd/database.c,v > retrieving revision 1.33 > diff -u -p -r1.33 database.c > --- database.c 18 Feb 2016 15:33:24 -0000 1.33 > +++ database.c 13 Jul 2019 14:08:10 -0000 > @@ -43,7 +43,6 @@ send_db_description(struct nbr *nbr) > struct db_dscrp_hdr dd_hdr; > struct lsa_entry *le, *nle; > struct ibuf *buf; > - int ret = 0; > u_int8_t bits = 0; > > if ((buf = ibuf_open(nbr->iface->mtu - sizeof(struct ip))) == NULL) > @@ -66,8 +65,7 @@ send_db_description(struct nbr *nbr) > log_debug("send_db_description: neighbor ID %s: " > "cannot send packet in state %s", inet_ntoa(nbr->id), > nbr_state_name(nbr->state)); > - ret = -1; > - goto done; > + goto fail; > case NBR_STA_XSTRT: > bits |= OSPF_DBD_MS | OSPF_DBD_M | OSPF_DBD_I; > nbr->dd_more = 1; > @@ -150,12 +148,13 @@ send_db_description(struct nbr *nbr) > goto fail; > > /* transmit packet */ > - ret = send_packet(nbr->iface, buf, &dst); > -done: > + if (send_packet(nbr->iface, buf, &dst) == -1) > + goto fail; > + > ibuf_free(buf); > - return (ret); > + return (0); > fail: > - log_warn("send_db_description"); > + log_warn("%s", __func__); > ibuf_free(buf); > return (-1); > } > Index: hello.c > =================================================================== > RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v > retrieving revision 1.22 > diff -u -p -r1.22 hello.c > --- hello.c 22 Feb 2018 07:42:38 -0000 1.22 > +++ hello.c 13 Jul 2019 14:03:27 -0000 > @@ -41,7 +41,6 @@ send_hello(struct iface *iface) > struct hello_hdr hello; > struct nbr *nbr; > struct ibuf *buf; > - int ret; > > dst.sin_family = AF_INET; > dst.sin_len = sizeof(struct sockaddr_in); > @@ -103,11 +102,13 @@ send_hello(struct iface *iface) > if (auth_gen(buf, iface)) > goto fail; > > - ret = send_packet(iface, buf, &dst); > + if (send_packet(iface, buf, &dst) == -1) > + goto fail; > + > ibuf_free(buf); > - return (ret); > + return (0); > fail: > - log_warn("send_hello"); > + log_warn("%s", __func__); > ibuf_free(buf); > return (-1); > } > Index: lsack.c > =================================================================== > RCS file: /cvs/src/usr.sbin/ospfd/lsack.c,v > retrieving revision 1.21 > diff -u -p -r1.21 lsack.c > --- lsack.c 25 Oct 2014 03:23:49 -0000 1.21 > +++ lsack.c 13 Jul 2019 14:04:59 -0000 > @@ -59,7 +59,6 @@ int > send_ls_ack(struct iface *iface, struct in_addr addr, struct ibuf *buf) > { > struct sockaddr_in dst; > - int ret; > > /* update authentication and calculate checksum */ > if (auth_gen(buf, iface)) { > @@ -71,8 +70,11 @@ send_ls_ack(struct iface *iface, struct > dst.sin_len = sizeof(struct sockaddr_in); > dst.sin_addr.s_addr = addr.s_addr; > > - ret = send_packet(iface, buf, &dst); > - return (ret); > + if (send_packet(iface, buf, &dst) == -1) { > + log_warn("%s", __func__); > + return (-1); > + } > + return (0); > } > > int > Index: lsreq.c > =================================================================== > RCS file: /cvs/src/usr.sbin/ospfd/lsreq.c,v > retrieving revision 1.20 > diff -u -p -r1.20 lsreq.c > --- lsreq.c 17 Jan 2013 09:02:22 -0000 1.20 > +++ lsreq.c 13 Jul 2019 14:04:00 -0000 > @@ -37,7 +37,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 ip))) == NULL) > fatal("send_ls_req"); > @@ -80,12 +79,13 @@ send_ls_req(struct nbr *nbr) > if (auth_gen(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"); > + log_warn("%s", __func__); > ibuf_free(buf); > return (-1); > } > Index: lsupdate.c > =================================================================== > RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v > retrieving revision 1.45 > diff -u -p -r1.45 lsupdate.c > --- lsupdate.c 26 Dec 2016 17:38:14 -0000 1.45 > +++ lsupdate.c 13 Jul 2019 14:07:11 -0000 > @@ -210,7 +210,6 @@ send_ls_update(struct ibuf *buf, struct > u_int32_t nlsa) > { > struct sockaddr_in dst; > - int ret; > > nlsa = htonl(nlsa); > memcpy(ibuf_seek(buf, sizeof(struct ospf_hdr), sizeof(nlsa)), > @@ -224,12 +223,13 @@ send_ls_update(struct ibuf *buf, struct > dst.sin_len = sizeof(struct sockaddr_in); > dst.sin_addr.s_addr = addr.s_addr; > > - ret = send_packet(iface, buf, &dst); > + if (send_packet(iface, buf, &dst) == -1) > + goto fail; > > ibuf_free(buf); > - return (ret); > + return (0); > fail: > - log_warn("send_ls_update"); > + log_warn("%s", __func__); > ibuf_free(buf); > return (-1); > } > Index: packet.c > =================================================================== > RCS file: /cvs/src/usr.sbin/ospfd/packet.c,v > retrieving revision 1.31 > diff -u -p -r1.31 packet.c > --- packet.c 25 Oct 2014 03:23:49 -0000 1.31 > +++ packet.c 13 Jul 2019 14:38:45 -0000 > @@ -96,8 +96,8 @@ send_packet(struct iface *iface, struct > return (-1); > > if (sendmsg(iface->fd, &msg, 0) == -1) { > - log_warn("send_packet: error sending packet on interface %s", > - iface->name); > + log_warn("%s: error sending packet to %s on interface %s", > + __func__, inet_ntoa(ip_hdr.ip_dst), iface->name); > return (-1); > } > > -- :wq Claudio