From b31107f646566d305fac3608576ba5c45b412ae6 Mon Sep 17 00:00:00 2001 From: Olof Hagsand Date: Fri, 25 Jun 2021 10:46:53 +0000 Subject: [PATCH] - Restconf native: Fixed ssl/non-ssl read/write behaviour for data that is different in freebsd than in linux - test: removed sed -i in tests since it is not portable between linux and bsd --- apps/restconf/restconf_main_native.c | 38 ++++++++++++++++++++++++---- apps/restconf/restconf_nghttp2.c | 16 ++++++++++-- test/test_debug.sh | 5 +++- test/test_perf_restconf.sh | 20 ++++++++++----- 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/apps/restconf/restconf_main_native.c b/apps/restconf/restconf_main_native.c index e257ac34..5e7551f2 100644 --- a/apps/restconf/restconf_main_native.c +++ b/apps/restconf/restconf_main_native.c @@ -679,6 +679,8 @@ restconf_connection(int s, ssize_t n; char buf[BUFSIZ]; /* from stdio.h, typically 8K XXX: reduce for test */ int readmore = 1; + int sslerr; + #ifdef HAVE_LIBEVHTP clicon_handle h; evhtp_connection_t *evconn = NULL; @@ -700,21 +702,47 @@ restconf_connection(int s, curl -Ssik --key /var/tmp/./test_restconf_ssl_certs.sh/certs/limited.key --cert /var/tmp/./test_restconf_ssl_certs.sh/certs/limited.crt -X GET https://localhost/restconf/data/example:x */ if ((n = SSL_read(rc->rc_ssl, buf, sizeof(buf))) < 0){ - clicon_err(OE_XML, errno, "SSL_read"); - goto done; + sslerr = SSL_get_error(rc->rc_ssl, n); + clicon_debug(1, "%s SSL_read() n:%ld errno:%d sslerr:%d", __FUNCTION__, n, errno, sslerr); + switch (sslerr){ + case SSL_ERROR_WANT_READ: /* 2 */ + /* SSL_ERROR_WANT_READ is returned when the last operation was a read operation + * from a nonblocking BIO. + * That is, it can happen if restconf_socket_init() below is called + * with SOCK_NONBLOCK + */ + clicon_debug(1, "%s SSL_read SSL_ERROR_WANT_READ", __FUNCTION__); + usleep(1000); + readmore = 1; + break; + default: + clicon_err(OE_XML, errno, "SSL_read"); + goto done; + } /* switch */ + continue; /* readmore */ } } else{ if ((n = read(rc->rc_s, buf, sizeof(buf))) < 0){ /* XXX atomicio ? */ - if (errno == ECONNRESET) {/* Connection reset by peer */ + switch(errno){ + case ECONNRESET:/* Connection reset by peer */ clicon_debug(1, "%s %d Connection reset by peer", __FUNCTION__, rc->rc_s); clixon_event_unreg_fd(rc->rc_s, restconf_connection); close(rc->rc_s); restconf_conn_free(rc); goto ok; /* Close socket and ssl */ + break; + case EAGAIN: + clicon_debug(1, "%s read EAGAIN", __FUNCTION__); + usleep(1000); + readmore = 1; + break; + default:; + clicon_err(OE_XML, errno, "read"); + goto done; + break; } - clicon_err(OE_XML, errno, "read"); - goto done; + continue; } } clicon_debug(1, "%s read:%ld", __FUNCTION__, n); diff --git a/apps/restconf/restconf_nghttp2.c b/apps/restconf/restconf_nghttp2.c index 9360de89..7f963c51 100644 --- a/apps/restconf/restconf_nghttp2.c +++ b/apps/restconf/restconf_nghttp2.c @@ -182,6 +182,7 @@ session_send_callback(nghttp2_session *session, ssize_t totlen = 0; int s; SSL *ssl; + int sslerr; clicon_debug(1, "%s buflen:%lu", __FUNCTION__, buflen); s = rc->rc_s; @@ -190,7 +191,14 @@ session_send_callback(nghttp2_session *session, if (ssl){ if ((len = SSL_write(ssl, buf+totlen, buflen-totlen)) <= 0){ er = errno; - switch (SSL_get_error(ssl, len)){ + sslerr = SSL_get_error(ssl, len); + clicon_debug(1, "%s errno:;%d sslerr:%d", __FUNCTION__, errno, sslerr); + switch (sslerr){ + case SSL_ERROR_WANT_WRITE: /* 3 */ + clicon_debug(1, "%s write SSL_ERROR_WANT_WRITE", __FUNCTION__); + usleep(1000); + continue; + break; case SSL_ERROR_SYSCALL: /* 5 */ if (er == ECONNRESET) {/* Connection reset by peer */ if (ssl) @@ -200,8 +208,12 @@ session_send_callback(nghttp2_session *session, goto ok; /* Close socket and ssl */ } else if (er == EAGAIN){ + /* same as want_write above, but different behaviour on different + * platforms, linux here, freebsd want_write, or possibly differnt + * ssl lib versions? + */ clicon_debug(1, "%s write EAGAIN", __FUNCTION__); - usleep(10000); + usleep(1000); continue; } else{ diff --git a/test/test_debug.sh b/test/test_debug.sh index 8992fb43..e2391e94 100755 --- a/test/test_debug.sh +++ b/test/test_debug.sh @@ -105,6 +105,9 @@ expectpart "$($clixon_cli -1 -f $cfg -l o debug restconf 1)" 0 "^$" new "get and put config using restconf" expectpart "$(curl $CURLOPTS -H "Accept: application/yang-data+xml" -X GET $RCPROTO://localhost/restconf/data?content=config --next $CURLOPTS -H "Content-Type: application/yang-data+json" -X POST $RCPROTO://localhost/restconf/data -d '{"example:table":{"parameter":{"name":"local0","value":"foo"}}}')" 0 "HTTP/$HVER 200" '' "HTTP/$HVER 201" +# In freebsd, backend dies in stop_restconf below unless sleep +sleep $DEMSLEEP + if [ $RC -ne 0 ]; then new "Kill restconf daemon" stop_restconf @@ -115,7 +118,7 @@ if [ $BE -ne 0 ]; then # Check if premature kill pid=$(pgrep -u root -f clixon_backend) if [ -z "$pid" ]; then - err "backend already dead" + err1 "backend pid !=0" 0 fi # kill backend stop_backend -f $cfg diff --git a/test/test_perf_restconf.sh b/test/test_perf_restconf.sh index b07cd66b..6d8df8db 100755 --- a/test/test_perf_restconf.sh +++ b/test/test_perf_restconf.sh @@ -32,6 +32,7 @@ ftest=$dir/test.xml fconfig=$dir/large.xml fconfig2=$dir/large2.xml # leaf-list foutput=$dir/output.xml +foutput2=$dir/output2.xml cat < $fyang module scaling{ @@ -132,15 +133,20 @@ expecteof "time -p $clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO $foutput +r=$? +if [ $r -ne 0 ]; then + err1 "retval 0" $r +fi # Remove Content-Length line (depends on size) -sed -i '/Content-Length:/d' $foutput -sed -i '/content-length:/d' $foutput +# Note: do not use sed -i since it is not portable between gnu and bsd +sed '/Content-Length:/d' $foutput > $foutput2 && mv $foutput2 $foutput +sed '/content-length:/d' $foutput > $foutput2 && mv $foutput2 $foutput # Remove (nginx) web-server specific lines -sed -i '/Server:/d' $foutput -sed -i '/Date:/d' $foutput -sed -i '/Transfer-Encoding:/d' $foutput -sed -i '/Connection:/d' $foutput +sed '/Server:/d' $foutput > $foutput2 && mv $foutput2 $foutput +sed '/Date:/d' $foutput > $foutput2 && mv $foutput2 $foutput +sed '/Transfer-Encoding:/d' $foutput > $foutput2 && mv $foutput2 $foutput +sed '/Connection:/d' $foutput > $foutput2 && mv $foutput2 $foutput # Create a file to compare with if ${HAVE_LIBNGHTTP2}; then @@ -165,7 +171,7 @@ echo " " >> $ftest ret=$(diff -i $ftest $foutput) if [ $? -ne 0 ]; then - echo "$ret" + echo "diff -i $ftest $foutput" err1 "Matching running-db with $fconfigonly" fi