On 01/05/2018 12:55 AM, Lawrence Brakmo wrote:
> 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 <bra...@fb.com>
> ---
> 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
