experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

* Fixed inefficient RESTCONF HTTP/1 read body by appending bufs instead of re-parsing

Browse source 
* Fixed: http1 by mistake included in fcgi build
* Test: restconf perf test uses restconf for initial setup instead of netconf
This commit is contained in:
Olof hagsand 2022-02-12 21:46:15 +01:00
parent 6ae749c9dd
commit 10886a31a2
6 changed files with 89 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

3
apps/restconf/Makefile.in
View file

@ -114,7 +114,10 @@ APPSRC += restconf_stream_$(with_restconf).c
endif endif
# internal http/1 parser # internal http/1 parser
YACCOBJS =
ifeq ($(with_restconf),native)
YACCOBJS = lex.clixon_http1_parse.o clixon_http1_parse.tab.o YACCOBJS = lex.clixon_http1_parse.o clixon_http1_parse.tab.o
endif
APPOBJ = $(APPSRC:.c=.o) $(YACCOBJS) APPOBJ = $(APPSRC:.c=.o) $(YACCOBJS)

36
apps/restconf/restconf_http1.c
View file

@ -464,29 +464,35 @@ http1_check_expect(clicon_handle h,
/*! Is there more data to be read? /*! Is there more data to be read?
* *
* @param[in] h Clixon handle * Use Content-Length header as an indicator on the status of reading an input message:
* @param[in] sd Restconf stream data (for http1 only stream 0) * 0: No Content-Length or 0
* @retval 1 OK, message is fully read, proceed to processing * Either message header not fully read OR header does not contain Content-Length
* @retval 0 OK, but message is partially read, need more data before processing * 1: Content-Length found but body has fewer bytes, ie remaining bytes to read
* @retval -1 Error * 2: Content-Length found and matches body length. No more bytes to read
* @param[in] h Clixon handle
* @param[in] sd Restconf stream data (for http1 only stream 0)
* @param[out] status 0-2, see description above
* @retval 0 OK, see status param
* @retval -1 Error
*/ */
int int
http1_check_readmore(clicon_handle h, http1_check_content_length(clicon_handle h,
restconf_stream_data *sd) restconf_stream_data *sd,
int *status)
{ {
int retval = -1; int retval = -1;
char *val; char *val;
int len; int len;
if ((val = restconf_param_get(h, "HTTP_CONTENT_LENGTH")) != NULL && if ((val = restconf_param_get(h, "HTTP_CONTENT_LENGTH")) == NULL ||
(len = atoi(val)) != 0){ (len = atoi(val)) == 0)
*status = 0;
else{
if (cbuf_len(sd->sd_indata) < len) if (cbuf_len(sd->sd_indata) < len)
goto readmore; *status = 1;
else
*status = 2;
} }
retval = 1;
done:
return retval;
readmore:
retval = 0; retval = 0;
goto done; return retval;
} }

2
apps/restconf/restconf_http1.h
View file

@ -44,6 +44,6 @@ int clixon_http1_parse_string(clicon_handle h, restconf_conn *rc, char *str);
int clixon_http1_parse_buf(clicon_handle h, restconf_conn *rc, char *buf, size_t n); int clixon_http1_parse_buf(clicon_handle h, restconf_conn *rc, char *buf, size_t n);
int restconf_http1_path_root(clicon_handle h, restconf_conn *rc); int restconf_http1_path_root(clicon_handle h, restconf_conn *rc);
int http1_check_expect(clicon_handle h, restconf_conn *rc, restconf_stream_data *sd); int http1_check_expect(clicon_handle h, restconf_conn *rc, restconf_stream_data *sd);
int http1_check_readmore(clicon_handle h, restconf_stream_data *sd); int http1_check_content_length(clicon_handle h, restconf_stream_data *sd, int *status);
#endif /* _RESTCONF_HTTP1_H_ */ #endif /* _RESTCONF_HTTP1_H_ */

63
apps/restconf/restconf_native.c
View file

@ -499,6 +499,8 @@ restconf_connection(int s,
#ifdef HAVE_HTTP1 #ifdef HAVE_HTTP1
clicon_handle h; clicon_handle h;
restconf_stream_data *sd; restconf_stream_data *sd;
int status;
int http1_headers_read = 0; /* Dont re-parse headers, just append body */
#endif #endif
clicon_debug(1, "%s %d", __FUNCTION__, s); clicon_debug(1, "%s %d", __FUNCTION__, s);
@ -578,15 +580,34 @@ restconf_connection(int s,
clicon_err(OE_RESTCONF, EINVAL, "restconf stream not found"); clicon_err(OE_RESTCONF, EINVAL, "restconf stream not found");
goto done; goto done;
} }
/* multi-buffer for multiple reads */ if (http1_headers_read){
totlen += n; if (cbuf_append_buf(sd->sd_indata, buf, n) < 0){
if ((totbuf = realloc(totbuf, totlen+1)) == NULL){ clicon_err(OE_UNIX, errno, "cbuf_append");
clicon_err(OE_UNIX, errno, "realloc"); goto done;
goto done; }
} }
memcpy(&totbuf[totlen-n], buf, n); else {
totbuf[totlen] = '\0'; /* multi-buffer for multiple reads */
if (clixon_http1_parse_string(h, rc, totbuf) < 0){ totlen += n;
if ((totbuf = realloc(totbuf, totlen+1)) == NULL){
clicon_err(OE_UNIX, errno, "realloc");
goto done;
}
memcpy(&totbuf[totlen-n], buf, n);
totbuf[totlen] = '\0';
}
/*
* The following cases are handled after parsing
* - Parse error
* - More data to read?
* - clear and read more
* - No more data
* - send error
* - Parse OK
* -
*/
if (!http1_headers_read &&
clixon_http1_parse_string(h, rc, totbuf) < 0){
/* Maybe only for non-ssl ? */ /* Maybe only for non-ssl ? */
if ((ret = clixon_event_poll(rc->rc_s)) < 0) if ((ret = clixon_event_poll(rc->rc_s)) < 0)
goto done; goto done;
@ -617,15 +638,31 @@ restconf_connection(int s,
} }
} }
/* Check whole message is read. /* Check whole message is read.
* ret == 0: need more bytes * Only way this could happen is that body is read
* 0: No Content-Length or 0
* header does not contain Content-Length or is 0
* (OR: message header not fully read SHOULDNT HAPPEN IF BODY READ)
* 1: Content-Length found but body has fewer bytes, ie remaining bytes to read
* 2: Content-Length found and matches body length. No more bytes to read
*/ */
if ((ret = http1_check_readmore(h, sd)) < 0) if ((ret = http1_check_content_length(h, sd, &status)) < 0)
goto done; goto done;
if (ret == 0){ switch (status){
if (native_clear_input(h, sd) < 0) case 1:
goto done; /* Next read: keep header state and only append inbody */
http1_headers_read++;
readmore++; readmore++;
continue; continue;
break;
break;
case 0:
case 2:
default:
break;
}
if (status == 0){
} }
if (restconf_http1_path_root(h, rc) < 0) if (restconf_http1_path_root(h, rc) < 0)
goto done; goto done;

28
test/test_perf_restconf.sh
View file

@ -4,6 +4,9 @@
# Lists (and leaf-lists) # Lists (and leaf-lists)
# Add, get and delete entries # Add, get and delete entries
# Override default to use http/1.1
HAVE_LIBNGHTTP2=false
# Magic line must be first in script (see README.md) # Magic line must be first in script (see README.md)
s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@ -27,7 +30,7 @@ APPNAME=example
cfg=$dir/scaling-conf.xml cfg=$dir/scaling-conf.xml
fyang=$dir/scaling.yang fyang=$dir/scaling.yang
fconfigonly=$dir/config.xml # only config for test fdataxml=$dir/config.xml # dataxml
ftest=$dir/test.xml ftest=$dir/test.xml
fconfig=$dir/large.xml fconfig=$dir/large.xml
fconfig2=$dir/large2.xml # leaf-list fconfig2=$dir/large2.xml # leaf-list
@ -108,28 +111,23 @@ wait_restconf
# Check this later with committed data # Check this later with committed data
new "generate config with $perfnr list entries" new "generate config with $perfnr list entries"
echo -n "<x xmlns=\"urn:example:clixon\">" > $fconfigonly echo -n "<x xmlns=\"urn:example:clixon\">" > $fdataxml
for (( i=0; i<$perfnr; i++ )); do for (( i=0; i<$perfnr; i++ )); do
echo -n "<y><a>$i</a><b>$i</b></y>" >> $fconfigonly echo -n "<y><a>$i</a><b>$i</b></y>" >> $fdataxml
done done
echo -n "</x>" >> $fconfigonly # No CR echo -n "</x>" >> $fdataxml # No CR
echo -n "$DEFAULTHELLO<rpc $DEFAULTNS><edit-config><target><candidate/></target><config>" > $fconfig echo -n "$DEFAULTHELLO<rpc $DEFAULTNS><edit-config><target><candidate/></target><config>" > $fconfig
cat $fconfigonly >> $fconfig cat $fdataxml >> $fconfig
echo "</config></edit-config></rpc>]]>]]>" >> $fconfig echo "</config></edit-config></rpc>]]>]]>" >> $fconfig
# Now take large config file and write it via netconf to candidate # Now take large config file and write it via netconf to candidate
new "test time exists" new "test time exists"
expectpart "$(time -p ls)" 0 expectpart "$(time -p ls)" 0
new "netconf write large config" new "restconf write large config"
expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}' expectpart "$(time -p curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+xml" $RCPROTO://localhost/restconf/data -d @$fdataxml )" 0 "HTTP/$HVER 201"
exit
# Here, there are $perfnr entries in candidate
# Now commit it from candidate to running
new "netconf commit large config"
expecteof "time -p $clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO<rpc $DEFAULTNS><commit/></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$" 2>&1 | awk '/real/ {print $2}'
new "Check running-db contents" new "Check running-db contents"
curl $CURLOPTS -X GET -H "Accept: application/yang-data+xml" $RCPROTO://localhost/restconf/data?content=config > $foutput curl $CURLOPTS -X GET -H "Accept: application/yang-data+xml" $RCPROTO://localhost/restconf/data?content=config > $foutput
@ -166,13 +164,13 @@ echo "Content-Type: application/yang-data+xml " >> $ftest
echo "Cache-Control: no-cache " >> $ftest echo "Cache-Control: no-cache " >> $ftest
echo " ">> $ftest echo " ">> $ftest
echo -n "<data>">> $ftest echo -n "<data>">> $ftest
cat $fconfigonly >> $ftest cat $fdataxml >> $ftest
echo "</data> " >> $ftest echo "</data> " >> $ftest
ret=$(diff -i $ftest $foutput) ret=$(diff -i $ftest $foutput)
if [ $? -ne 0 ]; then if [ $? -ne 0 ]; then
echo "diff -i $ftest $foutput" echo "diff -i $ftest $foutput"
err1 "Matching running-db with $fconfigonly" err1 "Matching running-db with $fdataxml"
fi fi
# RESTCONF get # RESTCONF get

1
test/test_restconf_continue.sh
View file

@ -3,6 +3,7 @@
# Trigger Expect by curl -H. Some curls seem to trigger one on large PUTs but not all # Trigger Expect by curl -H. Some curls seem to trigger one on large PUTs but not all
# Override default to use http/1.1 # Override default to use http/1.1
# In http/2 there is no explicit continue
HAVE_LIBNGHTTP2=false HAVE_LIBNGHTTP2=false
# Magic line must be first in script (see README.md) # Magic line must be first in script (see README.md)