ospfd: improve logging when sendig packets fail

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

ospfd: improve logging when sendig packets fail

Remi Locherer
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?

Remi

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);
  }
 

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: improve logging when sendig packets fail

Claudio Jeker
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