netdev - Re: [Cake] [PATCH net-next v14 6/7] sch_cake: Add overhead compensation support to the rate shaper

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: <20180522202241.GJ5488@localhost.localdomain>
Date:   Tue, 22 May 2018 17:22:41 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Toke Høiland-Jørgensen <toke@...e.dk>
Cc:     netdev@...r.kernel.org, cake@...ts.bufferbloat.net
Subject: Re: [Cake] [PATCH net-next v14 6/7] sch_cake: Add overhead
 compensation support to the rate shaper

On Tue, May 22, 2018 at 10:44:53AM +0200, Toke Høiland-Jørgensen wrote:
> 
> 
> On 22 May 2018 01:45:13 CEST, Marcelo Ricardo Leitner <marcelo.leitner@...il.com> wrote:
> >On Mon, May 21, 2018 at 10:35:58PM +0200, Toke Høiland-Jørgensen wrote:
> >> +static u32 cake_overhead(struct cake_sched_data *q, const struct
> >sk_buff *skb)
> >> +{
> >> +	const struct skb_shared_info *shinfo = skb_shinfo(skb);
> >> +	unsigned int hdr_len, last_len = 0;
> >> +	u32 off = skb_network_offset(skb);
> >> +	u32 len = qdisc_pkt_len(skb);
> >> +	u16 segs = 1;
> >> +
> >> +	q->avg_netoff = cake_ewma(q->avg_netoff, off << 16, 8);
> >> +
> >> +	if (!shinfo->gso_size)
> >> +		return cake_calc_overhead(q, len, off);
> >> +
> >> +	/* borrowed from qdisc_pkt_len_init() */
> >> +	hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
> >> +
> >> +	/* + transport layer */
> >> +	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 |
> >> +						SKB_GSO_TCPV6))) {
> >> +		const struct tcphdr *th;
> >> +		struct tcphdr _tcphdr;
> >> +
> >> +		th = skb_header_pointer(skb, skb_transport_offset(skb),
> >> +					sizeof(_tcphdr), &_tcphdr);
> >> +		if (likely(th))
> >> +			hdr_len += __tcp_hdrlen(th);
> >> +	} else {
> >
> >I didn't see some code limiting GSO packets to just TCP or UDP. Is it
> >safe to assume that this packet is an UDP one, and not SCTP or ESP,
> >for example?
> 
> As the comment says, I nicked this from the qdisc init code.
> So I assume it's safe? :)

As long as it doesn't go further than this, it is. As in, it is just
validating if it can contain an UDP header, and if so, account for its
size, without actually reading the header.

Considering everything !TCP as UDP work as an approximation, which is
quite accurate. SCTP header is just 4 bytes bigger than UDP header and
is equal to ESP header size.

> 
> >> +		struct udphdr _udphdr;
> >> +
> >> +		if (skb_header_pointer(skb, skb_transport_offset(skb),
> >> +				       sizeof(_udphdr), &_udphdr))
> >> +			hdr_len += sizeof(struct udphdr);
> >> +	}
> >> +
> >> +	if (unlikely(shinfo->gso_type & SKB_GSO_DODGY))
> >> +		segs = DIV_ROUND_UP(skb->len - hdr_len,
> >> +				    shinfo->gso_size);
> >> +	else
> >> +		segs = shinfo->gso_segs;
> >> +
> >> +	len = shinfo->gso_size + hdr_len;
> >> +	last_len = skb->len - shinfo->gso_size * (segs - 1);
> >> +
> >> +	return (cake_calc_overhead(q, len, off) * (segs - 1) +
> >> +		cake_calc_overhead(q, last_len, off));
> >> +}
> >> +
>