Re: [PATCH bpf-next v4 07/11] bpf: Add support for reading sk_state and more
From: Daniel Borkmann <daniel@iogearbox.net>
Date: 2018-01-06 01:46:13
On 01/05/2018 12:55 AM, Lawrence Brakmo wrote:
quoted hunk ↗ jump to hunk
Add support for reading many more tcp_sock fields state, same as sk->sk_state rtt_min same as sk->rtt_min.s[0].v (current rtt_min) snd_ssthresh rcv_nxt snd_nxt snd_una mss_cache ecn_flags rate_delivered rate_interval_us packets_out retrans_out total_retrans segs_in data_segs_in segs_out data_segs_out bytes_received (__u64) bytes_acked (__u64) Signed-off-by: Lawrence Brakmo <redacted> --- include/uapi/linux/bpf.h | 19 +++++++++ net/core/filter.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 1 deletion(-)diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index a1316c7..a8f4cf0 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h@@ -963,6 +963,25 @@ struct bpf_sock_ops { __u32 snd_cwnd; __u32 srtt_us; /* Averaged RTT << 3 in usecs */ __u32 bpf_sock_ops_flags; /* flags defined in uapi/linux/tcp.h */ + __u32 state; + __u32 rtt_min; + __u32 snd_ssthresh; + __u32 rcv_nxt; + __u32 snd_nxt; + __u32 snd_una; + __u32 mss_cache; + __u32 ecn_flags; + __u32 rate_delivered; + __u32 rate_interval_us; + __u32 packets_out; + __u32 retrans_out; + __u32 total_retrans; + __u32 segs_in; + __u32 data_segs_in; + __u32 segs_out; + __u32 data_segs_out; + __u64 bytes_received; + __u64 bytes_acked; }; /* List of known BPF sock_ops operators.diff --git a/net/core/filter.c b/net/core/filter.c index 76fd6e9..d4c5c1a 100644 --- a/net/core/filter.c +++ b/net/core/filter.c@@ -3829,7 +3829,7 @@ static bool __is_valid_sock_ops_access(int off, int size) /* The verifier guarantees that size > 0. */ if (off % size != 0) return false; - if (size != sizeof(__u32)) + if (size != sizeof(__u32) && size != sizeof(__u64)) return false;
Doesn't this have the side-effect that this would kind of let is_valid_access() callback allow not for narrower but wider access, e.g. on the u32 members when doing BPF_DW. It would still get ignored later on in the ctx rewrite, though, but it also means that we could try with 2nd part of u64 member offsets with BPF_W which would then return a 'verifier is misconfigured' error. I think it would be better to enforce BPF_DW access only on the u64 types and BPF_W access only on the u32 types before it becomes uapi instead opening this up generically. E.g. we do similar approach in pe_prog_is_valid_access() for sample_period (taking the the narrow access bits aside from there). Thanks, Daniel