netdev - Re: [RFC PATCH v1 5/5] wave: Added basic version of TCP Wave

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170728.185118.436256421743075749.davem@davemloft.net>
Date:   Fri, 28 Jul 2017 18:51:18 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     natale.patriciello@...il.com
Cc:     kuznet@....inr.ac.ru, jmorris@...ei.org, yoshfuji@...ux-ipv6.org,
        kaber@...sh.net, netdev@...r.kernel.org, ahmed.said@...roma2.it,
        zampognaro@....uniroma2.it, roseti@....uniroma2.it
Subject: Re: [RFC PATCH v1 5/5] wave: Added basic version of TCP Wave

From: Natale Patriciello <natale.patriciello@...il.com>
Date: Fri, 28 Jul 2017 21:59:19 +0200

> +/* TCP Wave private struct */
> +struct wavetcp {
> +	/* The module flags */
> +	u8 flags;
> +	/* The current transmission timer (us) */
> +	u32 tx_timer;
> +	/* The current burst size (segments) */
> +	u16 burst;

This style of declaring a data structure wastes a lot of vertical
screen space.  Instead use:

	type	name;	/* comment */

> +static void wavetcp_init(struct sock *sk)
> +{
> +	struct tcp_sock *tp = tcp_sk(sk);
> +	struct wavetcp *ca = inet_csk_ca(sk);

Always declare local variables in longest to shortest line order.

> +	DBG("%u sport: %u [%s]\n", tcp_time_stamp, ca->sport,
> +	    __func__);

DO NOT define your own custom debug logging facilities.

The kernel has millions of mechanism by which you can log information
either in the kernel log buffer or in the kernel trace log.  THere is
everything from dynamic fine-grained run time enable/disable, to
compile time controls.

There is absolutely therefore never a reason to define custom
mechanisms like you are here.

Thanks.