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.

Powered by blists - more mailing lists