[cmaster-next] [PATCH 0/2] Fix issues with the SO_SNDBUF/SO_RCVBUF sockoptions
David, please queue these two patches for stable/2.0 as well (they fix a completely broken ospf6d on FreeBSD). Renato Westphal (2): ospfd: set the OSPF socket's send buffer size only once *: always set SO_SNDBUF and SO_RCVBUF using a best effort approach bgpd/bgp_network.c | 26 ++---------------- lib/sockopt.c | 32 +++++++++++------------ lib/sockopt.h | 4 +-- ospf6d/ospf6_network.c | 38 ++------------------------- ospfd/ospf_interface.c | 5 ---- ospfd/ospf_network.c | 71 ++------------------------------------------------ ospfd/ospf_network.h | 1 - ospfd/ospf_packet.c | 8 ++---- ospfd/ospfd.c | 4 --- ospfd/ospfd.h | 1 - ripngd/ripngd.c | 4 +-- 11 files changed, 27 insertions(+), 167 deletions(-) -- 1.9.1
This reverts commit b7fe4141, which introduced a logic where the OSPF send buffer size was dynamically updated to reflect the maximum MTU of the OSPF enabled interfaces (this was done to make ospfd work with interfaces configured for jumbo frames). Since commit a78d75b0, this is not necessary anymore because ospf_sock_init() now sets the OSPF send buffer size to a very high value (8MB). Also, the previous logic was broken because it didn't account for run-time interface MTU changes. Signed-off-by: Renato Westphal <renato@opensourcerouting.org> --- ospfd/ospf_interface.c | 5 ----- ospfd/ospf_network.c | 35 ----------------------------------- ospfd/ospf_network.h | 1 - ospfd/ospf_packet.c | 8 ++------ ospfd/ospfd.c | 4 ---- ospfd/ospfd.h | 1 - 6 files changed, 2 insertions(+), 52 deletions(-) diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 2c8124b..8440765 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -797,11 +797,6 @@ ospf_if_up (struct ospf_interface *oi) OSPF_ISM_EVENT_SCHEDULE (oi, ISM_LoopInd); else { - struct ospf *ospf = ospf_lookup (); - if (ospf != NULL) - ospf_adjust_sndbuflen (ospf, oi->ifp->mtu); - else - zlog_warn ("%s: ospf_lookup() returned NULL", __func__); ospf_if_stream_set (oi); OSPF_ISM_EVENT_SCHEDULE (oi, ISM_InterfaceUp); } diff --git a/ospfd/ospf_network.c b/ospfd/ospf_network.c index 088123e..f02fcd1 100644 --- a/ospfd/ospf_network.c +++ b/ospfd/ospf_network.c @@ -41,7 +41,6 @@ extern struct zebra_privs_t ospfd_privs; #include "ospfd/ospf_lsdb.h" #include "ospfd/ospf_neighbor.h" #include "ospfd/ospf_packet.h" -#include "ospfd/ospf_dump.h" @@ -258,37 +257,3 @@ ospf_sock_init (void) return ospf_sock; } - -void -ospf_adjust_sndbuflen (struct ospf * ospf, unsigned int buflen) -{ - int ret, newbuflen; - /* Check if any work has to be done at all. */ - if (ospf->maxsndbuflen >= buflen) - return; - if (IS_DEBUG_OSPF (zebra, ZEBRA_INTERFACE)) - zlog_debug ("%s: adjusting OSPF send buffer size to %d", - __func__, buflen); - if (ospfd_privs.change (ZPRIVS_RAISE)) - zlog_err ("%s: could not raise privs, %s", __func__, - safe_strerror (errno)); - /* Now we try to set SO_SNDBUF to what our caller has requested - * (the MTU of a newly added interface). However, if the OS has - * truncated the actual buffer size to somewhat less size, try - * to detect it and update our records appropriately. The OS - * may allocate more buffer space, than requested, this isn't - * a error. - */ - ret = setsockopt_so_sendbuf (ospf->fd, buflen); - newbuflen = getsockopt_so_sendbuf (ospf->fd); - if (ret < 0 || newbuflen < 0 || newbuflen < (int) buflen) - zlog_warn ("%s: tried to set SO_SNDBUF to %u, but got %d", - __func__, buflen, newbuflen); - if (newbuflen >= 0) - ospf->maxsndbuflen = (unsigned int)newbuflen; - else - zlog_warn ("%s: failed to get SO_SNDBUF", __func__); - if (ospfd_privs.change (ZPRIVS_LOWER)) - zlog_err ("%s: could not lower privs, %s", __func__, - safe_strerror (errno)); -} diff --git a/ospfd/ospf_network.h b/ospfd/ospf_network.h index 8257adb..bc01a84 100644 --- a/ospfd/ospf_network.h +++ b/ospfd/ospf_network.h @@ -34,6 +34,5 @@ extern int ospf_if_drop_alldrouters (struct ospf *, struct prefix *, ifindex_t); extern int ospf_if_ipmulticast (struct ospf *, struct prefix *, ifindex_t); extern int ospf_sock_init (void); -extern void ospf_adjust_sndbuflen (struct ospf *, unsigned int); #endif /* _ZEBRA_OSPF_NETWORK_H */ diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 72ce558..f7d1d0f 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -691,13 +691,9 @@ ospf_write (struct thread *thread) last_serviced_oi = oi; } pkt_count++; - /* convenience - max OSPF data per packet, - * and reliability - not more data, than our - * socket can accept - */ #ifdef WANT_OSPF_WRITE_FRAGMENT - maxdatasize = MIN (oi->ifp->mtu, ospf->maxsndbuflen) - - sizeof (struct ip); + /* convenience - max OSPF data per packet */ + maxdatasize = oi->ifp->mtu - sizeof (struct ip); #endif /* WANT_OSPF_WRITE_FRAGMENT */ /* Get one packet from queue. */ op = ospf_fifo_head (oi->obuf); diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index 7156c1e..1a7691c 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -280,10 +280,6 @@ ospf_new (u_short instance) "a socket"); exit(1); } - new->maxsndbuflen = getsockopt_so_sendbuf (new->fd); - if (IS_DEBUG_OSPF (zebra, ZEBRA_INTERFACE)) - zlog_debug ("%s: starting with OSPF send buffer size %u", - __func__, new->maxsndbuflen); if ((new->ibuf = stream_new(OSPF_MAX_PACKET_SIZE+1)) == NULL) { zlog_err("ospf_new: fatal error: stream_new(%u) failed allocating ibuf", diff --git a/ospfd/ospfd.h b/ospfd/ospfd.h index 93b5ab7..41a7a30 100644 --- a/ospfd/ospfd.h +++ b/ospfd/ospfd.h @@ -252,7 +252,6 @@ struct ospf int write_oi_count; /* Num of packets sent per thread invocation */ struct thread *t_read; int fd; - unsigned int maxsndbuflen; struct stream *ibuf; struct list *oi_write_q; -- 1.9.1
If we fail to set any socket's buffer size, try again with a smaller value and keep going until it succeeds. This is better than just giving up or, even worse, abort the creation of a socket (ospf6d and ripd). Fix broken ospf6d on FreeBSD. Signed-off-by: Renato Westphal <renato@opensourcerouting.org> --- bgpd/bgp_network.c | 26 ++------------------------ lib/sockopt.c | 32 ++++++++++++++++---------------- lib/sockopt.h | 4 ++-- ospf6d/ospf6_network.c | 38 ++------------------------------------ ospfd/ospf_network.c | 36 ++---------------------------------- ripngd/ripngd.c | 4 +--- 6 files changed, 25 insertions(+), 115 deletions(-) diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index c528fcd..9a49051 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -151,28 +151,6 @@ bgp_md5_unset (struct peer *peer) return bgp_md5_set_password (peer, NULL); } -/* Update BGP socket send buffer size */ -static void -bgp_update_sock_send_buffer_size (int fd) -{ - int size = BGP_SOCKET_SNDBUF_SIZE; - int optval; - socklen_t optlen = sizeof(optval); - - if (getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &optval, &optlen) < 0) - { - zlog_err("getsockopt of SO_SNDBUF failed %s\n", safe_strerror(errno)); - return; - } - if (optval < size) - { - if (setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &size, sizeof(size)) < 0) - { - zlog_err("Couldn't increase send buffer: %s\n", safe_strerror(errno)); - } - } -} - int bgp_set_socket_ttl (struct peer *peer, int bgp_sock) { @@ -341,7 +319,7 @@ bgp_accept (struct thread *thread) } /* Set socket send buffer size */ - bgp_update_sock_send_buffer_size(bgp_sock); + setsockopt_so_sendbuf (bgp_sock, BGP_SOCKET_SNDBUF_SIZE); /* Check remote IP address */ peer1 = peer_lookup (bgp, &su); @@ -604,7 +582,7 @@ bgp_connect (struct peer *peer) set_nonblocking (peer->fd); /* Set socket send buffer size */ - bgp_update_sock_send_buffer_size(peer->fd); + setsockopt_so_sendbuf (peer->fd, BGP_SOCKET_SNDBUF_SIZE); if (bgp_set_socket_ttl (peer, peer->fd) < 0) return -1; diff --git a/lib/sockopt.c b/lib/sockopt.c index be3ac0e..461e1f7 100644 --- a/lib/sockopt.c +++ b/lib/sockopt.c @@ -29,30 +29,30 @@ #include "sockopt.h" #include "sockunion.h" -int +void setsockopt_so_recvbuf (int sock, int size) { - int ret; - - if ( (ret = setsockopt (sock, SOL_SOCKET, SO_RCVBUF, (char *) - &size, sizeof (int))) < 0) - zlog_err ("fd %d: can't setsockopt SO_RCVBUF to %d: %s", - sock,size,safe_strerror(errno)); + int orig_req = size; - return ret; + while (setsockopt(sock, SOL_SOCKET, SO_RCVBUF, &size, sizeof (size)) == -1) + size /= 2; + + if (size != orig_req) + zlog_warn ("%s: fd %d: SO_RCVBUF set to %d (requested %d)", __func__, sock, + size, orig_req); } -int +void setsockopt_so_sendbuf (const int sock, int size) { - int ret = setsockopt (sock, SOL_SOCKET, SO_SNDBUF, - (char *)&size, sizeof (int)); - - if (ret < 0) - zlog_err ("fd %d: can't setsockopt SO_SNDBUF to %d: %s", - sock, size, safe_strerror (errno)); + int orig_req = size; - return ret; + while (setsockopt(sock, SOL_SOCKET, SO_SNDBUF, &size, sizeof (size)) == -1) + size /= 2; + + if (size != orig_req) + zlog_warn ("%s: fd %d: SO_SNDBUF set to %d (requested %d)", __func__, sock, + size, orig_req); } int diff --git a/lib/sockopt.h b/lib/sockopt.h index 02f0189..b3ab57a 100644 --- a/lib/sockopt.h +++ b/lib/sockopt.h @@ -24,8 +24,8 @@ #include "sockunion.h" -extern int setsockopt_so_recvbuf (int sock, int size); -extern int setsockopt_so_sendbuf (const int sock, int size); +extern void setsockopt_so_recvbuf (int sock, int size); +extern void setsockopt_so_sendbuf (const int sock, int size); extern int getsockopt_so_sendbuf (const int sock); #ifdef HAVE_IPV6 diff --git a/ospf6d/ospf6_network.c b/ospf6d/ospf6_network.c index 0d730c5..0217d66 100644 --- a/ospf6d/ospf6_network.c +++ b/ospf6d/ospf6_network.c @@ -118,8 +118,6 @@ ospf6_sso (ifindex_t ifindex, struct in6_addr *group, int option) struct ipv6_mreq mreq6; int ret; int bufsize = (8 * 1024 * 1024); - int optval; - socklen_t optlen = sizeof(optval); assert (ifindex); mreq6.ipv6mr_interface = ifindex; @@ -134,40 +132,8 @@ ospf6_sso (ifindex_t ifindex, struct in6_addr *group, int option) return ret; } - if ((ret = setsockopt (ospf6_sock, SOL_SOCKET, SO_SNDBUF, - &bufsize, sizeof (bufsize))) < 0) - { - zlog_err ("Couldn't increase raw wbuf size: %s\n", safe_strerror(errno)); - return ret; - } - - if ((ret = getsockopt (ospf6_sock, SOL_SOCKET, SO_SNDBUF, - &optval, &optlen)) < 0) - { - zlog_err ("getsockopt of SO_SNDBUF failed with error %s\n", safe_strerror(errno)); - return ret; - } - else if (optval < bufsize) - { - zlog_err ("Unable to SO_SNDBUF to %d, set to %d\n", bufsize, optval); - } - - if ((ret = setsockopt (ospf6_sock, SOL_SOCKET, SO_RCVBUF, - &bufsize, sizeof (bufsize))) < 0) - { - zlog_err ("Couldn't increase raw rbuf size: %s\n", safe_strerror(errno)); - } - - if ((ret = getsockopt (ospf6_sock, SOL_SOCKET, SO_RCVBUF, - &optval, &optlen)) < 0) - { - zlog_err ("getsockopt of SO_RCVBUF failed with error %s\n", safe_strerror(errno)); - return ret; - } - else if (optval < bufsize) - { - zlog_err ("Unable to SO_RCVBUF to %d, set to %d\n", bufsize, optval); - } + setsockopt_so_sendbuf (ospf6_sock, bufsize); + setsockopt_so_recvbuf (ospf6_sock, bufsize); return 0; } diff --git a/ospfd/ospf_network.c b/ospfd/ospf_network.c index f02fcd1..d7ec256 100644 --- a/ospfd/ospf_network.c +++ b/ospfd/ospf_network.c @@ -161,8 +161,6 @@ ospf_sock_init (void) int ospf_sock; int ret, hincl = 1; int bufsize = (8 * 1024 * 1024); - int optval; - socklen_t optlen = sizeof(optval); if ( ospfd_privs.change (ZPRIVS_RAISE) ) zlog_err ("ospf_sock_init: could not raise privs, %s", @@ -222,38 +220,8 @@ ospf_sock_init (void) safe_strerror (errno) ); } - if ((ret = setsockopt (ospf_sock, SOL_SOCKET, SO_RCVBUF, - &bufsize, sizeof (bufsize))) < 0) - { - zlog_err ("Couldn't increase raw rbuf size: %s\n", safe_strerror(errno)); - } - - if ((ret = getsockopt (ospf_sock, SOL_SOCKET, SO_RCVBUF, - &optval, &optlen)) < 0) - { - zlog_err("getsockopt of SO_RCVBUF failed with error %s\n", safe_strerror(errno)); - } - if (optval < bufsize) - { - zlog_err("Unable to SO_RCVBUF to %d, set to %d\n", bufsize, optval); - } - + setsockopt_so_sendbuf (ospf_sock, bufsize); + setsockopt_so_recvbuf (ospf_sock, bufsize); - if ((ret = setsockopt (ospf_sock, SOL_SOCKET, SO_SNDBUF, - &bufsize, sizeof (bufsize))) < 0) - { - zlog_err ("Couldn't increase raw wbuf size: %s\n", safe_strerror(errno)); - } - - if ((ret = getsockopt (ospf_sock, SOL_SOCKET, SO_SNDBUF, - &optval, &optlen)) < 0) - { - zlog_err ("getsockopt of SO_SNDBUF failed with error %s\n", safe_strerror(errno)); - } - if (optval < bufsize) - { - zlog_err ("Unable to SO_SNDBUF to %d, set to %d\n", bufsize, optval); - } - return ospf_sock; } diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c index ca8850e..34ad859 100644 --- a/ripngd/ripngd.c +++ b/ripngd/ripngd.c @@ -111,9 +111,7 @@ ripng_make_socket (void) return sock; } - ret = setsockopt_so_recvbuf (sock, 8096); - if (ret < 0) - return ret; + setsockopt_so_recvbuf (sock, 8096); ret = setsockopt_ipv6_pktinfo (sock, 1); if (ret < 0) return ret; -- 1.9.1
Applied @ stable/2.0. isisd-simpl branch merged as well. On Wed, Dec 07, 2016 at 01:21:44PM -0200, Renato Westphal wrote:
David, please queue these two patches for stable/2.0 as well (they fix a completely broken ospf6d on FreeBSD).
Renato Westphal (2): ospfd: set the OSPF socket's send buffer size only once *: always set SO_SNDBUF and SO_RCVBUF using a best effort approach
bgpd/bgp_network.c | 26 ++---------------- lib/sockopt.c | 32 +++++++++++------------ lib/sockopt.h | 4 +-- ospf6d/ospf6_network.c | 38 ++------------------------- ospfd/ospf_interface.c | 5 ---- ospfd/ospf_network.c | 71 ++------------------------------------------------ ospfd/ospf_network.h | 1 - ospfd/ospf_packet.c | 8 ++---- ospfd/ospfd.c | 4 --- ospfd/ospfd.h | 1 - ripngd/ripngd.c | 4 +-- 11 files changed, 27 insertions(+), 167 deletions(-)
-- 1.9.1
_______________________________________________ cmaster-next mailing list cmaster-next@lists.nox.tf https://lists.nox.tf/listinfo/cmaster-next
participants (2)
-
David Lamparter -
Renato Westphal