netdev - Re: [RFC PATCH net-next] tcp: add NV congestion control

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]

Date:	Fri, 3 Jul 2015 16:30:01 -0400
From:	Neal Cardwell <ncardwell@...gle.com>
To:	Tom Herbert <tom@...bertland.com>
Cc:	Lawrence Brakmo <brakmo@...com>, netdev <netdev@...r.kernel.org>,
	Kernel Team <kernel-team@...com>,
	Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [RFC PATCH net-next] tcp: add NV congestion control

> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 48c3696..05e0da5 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -254,6 +254,10 @@ struct tcp_sock {
>         u32     lost_out;       /* Lost packets                 */
>         u32     sacked_out;     /* SACK'd packets                       */
>         u32     fackets_out;    /* FACK'd packets                       */
> +       u32     ack_in_flight;  /* This field is populated when new acks
> +                                * are received. It contains the number of
> +                                * bytes in flight when the last packet
> +                                * acked was sent. Used by tcp-nv. */

AFAICT the tcp_sock struct does not really need to grow to hold the
ack_in_flight field, because this field does not need to be remembered
between ACKs. I would recommend putting it in a small struct ("struct
ack_sample"?) that is allocated on the stack in tcp_ack() and passed
into the pkts_acked() function for congestion control modules that
want extra info beyond the number of packets ACKed and the RTT.

In fact it might be cleaner to put the number of packets ACKed and the
RTT in that struct as well, so in the future we don't have to modify
all the congestion control modules' pkts_acked() function
every time a new piece of info is provided by the core TCP stack.

>  /* This is what the send packet queuing engine uses to pass
>   * TCP per-packet control information to the transmission code.
>   * We also store the host-order sequence numbers in here too.
> - * This is 44 bytes if IPV6 is enabled.
> + * This is 48 bytes if IPV6 is enabled.
>   * If this grows please adjust skbuff.h:skbuff->cb[xxx] size appropriately.
>   */
>  struct tcp_skb_cb {
>         __u32           seq;            /* Starting sequence number     */
>         __u32           end_seq;        /* SEQ + FIN + SYN + datalen    */
> +       __u32           in_flight;      /* bytes in flight when this packet
> +                                        * was sent. */

AFAICT this patch would not require an increase in the size of sk_buff
cb[] if it were to take advantage of the fact that the tcp_skb_cb
header.h4 and header.h6 fields are only used in the packet reception
code path, and this in_flight field is only used on the transmit
side. So the in_flight field could be placed in a struct that is
itself placed in a union with the "header" union. Like this:

        union {
                struct {
                        /* bytes in flight when this packet was sent */
                        __u32 in_flight;
                } tx;   /* only used for outgoing skbs */

                union {
                        struct inet_skb_parm    h4;
#if IS_ENABLED(CONFIG_IPV6)
                        struct inet6_skb_parm   h6;
#endif
                } header;  /* only used for incoming skbs */
        };

That way the sender code can remember the in_flight value
without requiring any extra space. And in the future other
sender-side info could be stored in the "tx" struct, if needed.

neal
--
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