netdev - Re: [PATCH bpf-next 7/9] bpf: Sample NRM BPF program to limit egress bw

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:   Tue, 19 Feb 2019 10:29:25 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     brakmo <brakmo@...com>, netdev <netdev@...r.kernel.org>
Cc:     Martin Lau <kafai@...com>, Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH bpf-next 7/9] bpf: Sample NRM BPF program to limit egress
 bw



On 02/18/2019 09:38 PM, brakmo wrote:

> +
> +static __always_inline void get_nrm_pkt_info(struct bpf_sock *sk,
> +					     struct nrm_pkt_info *pkti)
> +{
> +	if (sk->family == AF_INET6 || sk->family == AF_INET) {
> +		pkti->is_ip = true;
> +		pkti->is_tcp = (sk->protocol == IPPROTO_TCP);
> +		if (pkti->is_tcp) {
> +			struct bpf_tcp_sock *tp;
> +
> +			tp = bpf_tcp_sock(sk);
> +			if (tp)
> +				pkti->ecn = tp->ecn_flags & TCP_ECN_OK;
> +			else
> +				pkti->ecn = 0;
> +		} else {
> +			pkti->ecn = 0;
> +		}
> +	} else {
> +		pkti->is_ip = false;
> +		pkti->is_tcp = false;
> +		pkti->ecn = 0;
> +	}
> +}
> 

This looks very strange.

ECN capability is per packet, and does not need access to the original
TCP socket really.

We definitely can use ECN with UDP packets.

IMO this sample looks like a work in progress.

EDT model allows to implement full shaping (not only virtual one)
by twaking/advancing skb->tstamp 
(see commit f11216b24219a bpf: add skb->tstamp r/w access from tc clsact and cg skb progs)

Implementing shaping with the need of accessing TCP sockets seems a layering violation,
a lot of things can go wrong with this model.
For instance, you wont be able to offload this.