On 1 May 2018 at 14:16, Andreas Jaggi <andreas.jaggi@xxxxxxxxxxxx> wrote:
> Extend the JSON output plugin so that the generated JSON stream can be
> sent to a remote host via TCP/UDP or to a local unix socket.
>
> Signed-off-by: Andreas Jaggi <andreas.jaggi@xxxxxxxxxxxx>
> ---
> output/ulogd_output_JSON.c | 225 +++++++++++++++++++++++++++++++++----
> ulogd.conf.in | 11 ++
> 2 files changed, 214 insertions(+), 22 deletions(-)
>
HI Andreas, thanks for working on this.
Some review below.
> +static int _connect_socket(struct ulogd_pluginstance *pi)
> +{
> + struct json_priv *op = (struct json_priv *) &pi->private;
> + struct addrinfo hints;
> + struct addrinfo *result, *rp;
> + struct sockaddr_un u_addr;
> + int sfd, s;
> +
> + if ( op->sock != -1 ) {
> + close(op->sock);
> + op->sock = -1;
> + }
> + if ( op->mode == JSON_MODE_UNIX ) {
> + ulogd_log(ULOGD_DEBUG, "connecting to unix:%s\n", file_ce(pi->config_kset).u.string);
> +
> + sfd = socket(AF_UNIX, SOCK_STREAM, 0);
> + if (sfd == -1 ) {
> + ulogd_log(ULOGD_ERROR, "Could not connect\n");
> + return -1;
> + }
> + u_addr.sun_family = AF_UNIX;
> + strncpy(u_addr.sun_path, file_ce(pi->config_kset).u.string, sizeof(u_addr.sun_path) - 1);
> + if ( connect(sfd, (struct sockaddr *) &u_addr, sizeof(struct sockaddr_un)) == -1 ) {
> + ulogd_log(ULOGD_ERROR, "Could not connect\n");
> + close(sfd);
> + return -1;
> + }
> + } else {
> + ulogd_log(ULOGD_DEBUG, "connecting to %s:%s\n", host_ce(pi->config_kset).u.string, port_ce(pi->config_kset).u.string);
> +
> + memset(&hints, 0, sizeof(struct addrinfo));
> + hints.ai_family = AF_UNSPEC;
> + hints.ai_socktype = op->mode == JSON_MODE_UDP ? SOCK_DGRAM : SOCK_STREAM;
> + hints.ai_protocol = 0;
> + hints.ai_flags = 0;
> +
> + s = getaddrinfo(host_ce(pi->config_kset).u.string, port_ce(pi->config_kset).u.string, &hints, &result);
> + if (s != 0) {
> + ulogd_log(ULOGD_ERROR, "getaddrinfo: %s\n", gai_strerror(s));
> + return -1;
> + }
> +
> + for (rp = result; rp != NULL; rp = rp->ai_next) {
> + int on = 1;
> +
> + sfd = socket(rp->ai_family, rp->ai_socktype,
> + rp->ai_protocol);
> + if (sfd == -1)
> + continue;
> +
> + setsockopt(sfd, SOL_SOCKET, SO_REUSEADDR,
> + (char *) &on, sizeof(on));
> +
> + if (connect(sfd, rp->ai_addr, rp->ai_addrlen) != -1)
> + break;
> +
> + close(sfd);
> + }
> +
> + freeaddrinfo(result);
> +
> + if (rp == NULL) {
> + ulogd_log(ULOGD_ERROR, "Could not connect\n");
> + return -1;
> + }
> + }
> +
> + op->sock = sfd;
> +
> + return 0;
> +}
> +
^^^^
could we split the function above in smaller chunks?
something like _connect_socket_unix() and _connect_socket_net()
> @@ -218,13 +331,41 @@ static int json_interp(struct ulogd_pluginstance *upi)
> }
> }
>
> - json_dumpf(msg, opi->of, 0);
> - fprintf(opi->of, "\n");
>
> + buf = json_dumps(msg, 0);
> json_decref(msg);
> -
> - if (upi->config_kset->ces[JSON_CONF_SYNC].u.value != 0)
> - fflush(opi->of);
> + if (buf == NULL) {
> + ulogd_log(ULOGD_ERROR, "Could not create message\n");
> + return ULOGD_IRET_ERR;
> + }
> + buflen = strlen(buf);
> + buf = realloc(buf, sizeof(char)*(buflen+2));
> + if (buf == NULL) {
> + ulogd_log(ULOGD_ERROR, "Could not create message\n");
> + return ULOGD_IRET_ERR;
> + }
> + strncat(buf, "\n", 1);
> + buflen++;
> +
> + if ( opi->mode == JSON_MODE_FILE ) {
> + fprintf(opi->of, "%s", buf);
> + free(buf);
> + if (upi->config_kset->ces[JSON_CONF_SYNC].u.value != 0)
> + fflush(opi->of);
> + } else {
> + if ( opi->sock != -1 ) {
> + ret = send(opi->sock, buf, buflen, MSG_NOSIGNAL);
> + }
> + free(buf);
> + if (ret != buflen) {
> + ulogd_log(ULOGD_ERROR, "Failure sending message: %s\n", strerror(errno));
> + if (ret == -1 || opi->sock == -1) {
> + return _connect_socket(upi);
> + } else {
> + return ULOGD_IRET_ERR;
> + }
> + }
> + }
^^^^
Also here, I see we check several times if ( opi->mode == JSON_MODE_FILE ).
May I suggest to introduce smaller functions depending on the mode?
Something like:
if ( opi->mode == JSON_MODE_FILE )
json_interp_file()
else
json_interp_xxxx()
> @@ -293,6 +451,22 @@ static int json_init(struct ulogd_pluginstance *upi)
>
> *op->cached_tz = '\0';
>
> + if ( op->mode == JSON_MODE_FILE ) {
> + op->of = fopen(upi->config_kset->ces[0].u.string, "a");
> + if (!op->of) {
> + ulogd_log(ULOGD_FATAL, "can't open JSON log file: %s\n",
> + strerror(errno));
> + return -1;
> + }
> + } else {
> + if ( host_ce(upi->config_kset).u.string == NULL )
> + return -1;
> + if ( port_ce(upi->config_kset).u.string == NULL)
> + return -1;
^^^^
Please check the coding style.
Try "(this)" instead of "( this )" or even "( this)", i.e, no spaces
after '(', or before ')'.
>
> @@ -300,8 +474,15 @@ static int json_fini(struct ulogd_pluginstance *pi)
> {
> struct json_priv *op = (struct json_priv *) &pi->private;
>
> - if (op->of != stdout)
> - fclose(op->of);
> + if ( op->mode == JSON_MODE_FILE ) {
> + if (op->of != stdout)
> + fclose(op->of);
> + } else {
> + if ( op->sock != -1 ) {
> + close(op->sock);
> + op->sock = -1;
> + }
> + }
>
^^^^
same, something like:
if JSON_MODE_FILE
close_file()
else
close_socket()
> return 0;
> }
> diff --git a/ulogd.conf.in b/ulogd.conf.in
> index 2fcf39a..e94a3a2 100644
> --- a/ulogd.conf.in
> +++ b/ulogd.conf.in
> @@ -212,6 +212,17 @@ sync=1
> # Uncomment the following line to use JSON v1 event format that
> # can provide better compatility with some JSON file reader.
> #eventv1=1
> +# Uncomment the following lines to send the JSON logs to a remote host via UDP
> +#mode="udp"
> +#host="192.0.2.10"
> +#port="10210"
> +# Uncomment the following lines to send the JSON logs to a remote host via TCP
> +#mode="tcp"
> +#host="192.0.2.10"
> +#port="10210"
> +# Uncomment the following lines to send the JSON logs to a local unix socket
> +#mode="unix"
> +#file="/var/run/ulogd.socket"
This new feature looks great! thanks again.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
