netdev - Re: [PATCH V2 net-next 1/3] tcp: introduce TCP experimental option for SMC

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: <20150715.212837.233151628267116088.davem@davemloft.net>
Date:	Wed, 15 Jul 2015 21:28:37 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	ubraun@...ux.vnet.ibm.com
Cc:	utz.bacher@...ibm.com, netdev@...r.kernel.org,
	linux-s390@...r.kernel.org, schwidefsky@...ibm.com,
	heiko.carstens@...ibm.com, ursula.braun@...ibm.com
Subject: Re: [PATCH V2 net-next 1/3] tcp: introduce TCP experimental option
 for SMC

From: Ursula Braun <ubraun@...ux.vnet.ibm.com>
Date: Tue, 14 Jul 2015 14:42:33 +0200

> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 48c3696..1b9a698 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -90,15 +90,28 @@ struct tcp_options_received {
>  		sack_ok : 4,	/* SACK seen on SYN packet		*/
>  		snd_wscale : 4,	/* Window scaling received from sender	*/
>  		rcv_wscale : 4;	/* Window scaling to send to receiver	*/
> +	u8	smc_capability:1; /* SMC capability			*/
>  	u8	num_sacks;	/* Number of SACK blocks		*/
>  	u16	user_mss;	/* mss requested by user in ioctl	*/
>  	u16	mss_clamp;	/* Maximal mss, negotiated at connection setup */
>  };

This adds new space to this structure, which can be avoided.

sack_ok only actually needs 3 bits, not 4, then you can use the extra
bit for smc_capability.

> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 87935ca..dee47d2 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -55,7 +55,8 @@ struct request_sock {
>  	struct sock			*rsk_listener;
>  	u16				mss;
>  	u8				num_retrans; /* number of retransmits */
> -	u8				cookie_ts:1; /* syncookie: encode tcpopts in timestamp */
> +	u8				cookie_ts:1, /* syncookie: encode tcpopts in timestamp */
> +					smc_capability:1;
>  	u8				num_timeout:7; /* number of timeouts */
>  	/* The following two fields can be easily recomputed I think -AK */
>  	u32				window_clamp; /* window clamp at creation time */

Again, similar situation here.

Please find a way to add your new boolean value without expanding the size of
this structure.  Here is it clear that a single u8 is fully consumed by the
cookie_ts and num_timeout bit fields.

> +#if IS_ENABLED(CONFIG_AFSMC)

I think this ifdef is completely pointless.

What do you think every Linux distribution is going to do for their kernels?
They are going to turn everything on.

So you need to find a way for your new feature to be nearly zero cost,
especially in the fast paths, assuming the code is enabled.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html