netdev - Re: [PATCH v4 net-next 2/2] tcp: Add Redundant Data Bundling (RDB)

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: <1455808685.648.16.camel@edumazet-ThinkPad-T530>
Date:	Thu, 18 Feb 2016 16:18:05 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Bendik Rønning Opstad <bro.devel@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	Yuchung Cheng <ycheng@...gle.com>,
	Neal Cardwell <ncardwell@...gle.com>,
	Andreas Petlund <apetlund@...ula.no>,
	Carsten Griwodz <griff@...ula.no>,
	Pål Halvorsen <paalh@...ula.no>,
	Jonas Markussen <jonassm@....uio.no>,
	Kristian Evensen <kristian.evensen@...il.com>,
	Kenneth Klette Jonassen <kennetkl@....uio.no>
Subject: Re: [PATCH v4 net-next 2/2] tcp: Add Redundant Data Bundling (RDB)

On mar., 2016-02-16 at 14:51 +0100, Bendik Rønning Opstad wrote:
> RDB is a mechanism that enables a TCP sender to bundle redundant
> (already sent) data with TCP packets containing new data. By bundling
> (retransmitting) already sent data with each TCP packet containing new
> data, the connection will be more resistant to sporadic packet loss
> which reduces the application layer latency significantly in congested
> scenarios.
>  
> -static void copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> +void copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>  {
>  	__copy_skb_header(new, old);
>  
> @@ -1061,6 +1061,7 @@ static void copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>  	skb_shinfo(new)->gso_segs = skb_shinfo(old)->gso_segs;
>  	skb_shinfo(new)->gso_type = skb_shinfo(old)->gso_type;
>  }
> +EXPORT_SYMBOL(copy_skb_header);

Why are you exporting this ? tcp is statically linked into vmlinux.


>  
> +/**
> + * skb_append_data() - copy data from an SKB to the end of another
> + *                     update end sequence number and checksum
> + * @from_skb: the SKB to copy data from
> + * @to_skb: the SKB to copy data to
> + */
> +void skb_append_data(struct sk_buff *from_skb, struct sk_buff *to_skb)
> +{
> +	skb_copy_from_linear_data(from_skb, skb_put(to_skb, from_skb->len),
> +				  from_skb->len);
> +	/* Update sequence range on original skb. */
> +	TCP_SKB_CB(to_skb)->end_seq = TCP_SKB_CB(from_skb)->end_seq;
> +
> +	if (from_skb->ip_summed == CHECKSUM_PARTIAL)
> +		to_skb->ip_summed = CHECKSUM_PARTIAL;
> +
> +	if (to_skb->ip_summed != CHECKSUM_PARTIAL)
> +		to_skb->csum = csum_block_add(to_skb->csum, from_skb->csum,
> +					      to_skb->len);
> +}
> +EXPORT_SYMBOL(skb_append_data);

Same remark here.

And this is really a tcp helper, you should add a tcp_ prefix.

About rdb_build_skb() : I do not see where you make sure
@bytes_in_rdb_skb is not too big ?

tcp_rdb_max_bytes & tcp_rdb_max_packets seem to have no .extra2 upper
limit, so a user could do something really stupid and attempt to crash
the kernel.

Presumably I would use SKB_MAX_HEAD(MAX_TCP_HEADER) so that we do not
try high order page allocation.