[pve-devel] [PATCH v2 firewall 1/2] add conntrack logging via libnetfilter_conntrack

[Wolfgang Bumiller]

On Fri, Dec 07, 2018 at 03:08:20PM +0100, David Limbeck wrote:
> @@ -954,6 +994,22 @@ main(int argc, char *argv[])
>  
>      g_option_context_free(context);
>  
> +    if (!conntrack) {
> +        int log_nf_conntrackfd = open("/var/lib/pve-firewall/log_nf_conntrack", O_RDONLY);

We have a bunch of
    #define XYZFILE "/var/..."
macros at the top, please use one for this path as well (also use it in
the fprintf() below).

> +        if (log_nf_conntrackfd == -1) {
> +            fprintf(stderr, "error: failed to open /var/lib/pve-firewall/log_nf_conntrack: %s\n", strerror(errno));

While for IO/permission/etc. errors this is fine, it should not be an
error if the file does not exist, so please don't print this message
when errno == ENOENT.

> +        } else {
> +            char c = '0';
> +            ssize_t bytes = read(log_nf_conntrackfd, &c, sizeof(c));
> +            if (bytes < 0) {
> +                fprintf(stderr, "error: failed to read value in log_nf_conntrack: %s\n", strerror(errno));
> +            } else {
> +                // should be either '0' or '1' so subtract '0' for either 0 or 1
> +                conntrack = c-'0';

Not an issue, but personally I'd find `conntrack = (c == '1');` more
appealing, so that 'n' isn't true ;-) (and values >1 could be reserved
for verbosity or something...)

> +            }
> +        }
> +    }
> +
>      if (debug) foreground = TRUE;
>  
>      if ((lockfd = open(LOCKFILE, O_RDWR|O_CREAT|O_APPEND, 0644)) == -1) {