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

[Eric Dumazet <eric.dumazet@gmail.com>]

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.