From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Lawrence Brakmo <brakmo@fb.com>
Cc: netdev <netdev@vger.kernel.org>, Kernel Team <kernel-team@fb.com>,
Blake Matheny <bmatheny@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Eric Dumazet <eric.dumazet@gmail.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v2 bpf-next 10/11] bpf: Add BPF_SOCK_OPS_STATE_CB
Date: Tue, 26 Dec 2017 15:23:59 -0800 [thread overview]
Message-ID: <20171226232357.52ayp76dfwzhg62c@ast-mbp> (raw)
In-Reply-To: <20171222012101.3899534-11-brakmo@fb.com>
On Thu, Dec 21, 2017 at 05:21:00PM -0800, Lawrence Brakmo wrote:
> Adds support for calling sock_ops BPF program when there is a TCP state
> change. Two arguments are used; one for the old state and another for
> the new state.
>
> New op: BPF_SOCK_OPS_STATE_CB.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> include/uapi/linux/bpf.h | 4 ++++
> include/uapi/linux/tcp.h | 1 +
> net/ipv4/tcp.c | 2 ++
> 3 files changed, 7 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 415b951..14040e0 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1024,6 +1024,10 @@ enum {
> * Arg1: sequence number of 1st byte
> * Arg2: # segments
> */
> + BPF_SOCK_OPS_STATE_CB, /* Called when TCP changes state.
> + * Arg1: old_state
> + * Arg2: new_state
> + */
exposing tcp state to tcp-bpf program means that internal enum
in include/net/tcp_states.h
enum {
TCP_ESTABLISHED = 1,
TCP_SYN_SENT,
TCP_SYN_RECV,
TCP_FIN_WAIT1,
TCP_FIN_WAIT2,
...
becomes uapi.
Also since it's only exposed from tcp side, the same sk_state
field used by other protocols is not exposed and things like
DCCP_PASSIVE_CLOSEREQ stay kernel internal.
I think it's ok.
If we would need to add new tcp state we can always add
it to the end of the enum.
Also it's better to make this explicit and move this enum to
uapi/linux/tcp_states.h or ...
An alternative would be to have an array that does state
remapping just to be consumed by bpf program which is ugly
and slow.
Another alternative would be to add:
enum {
BPF_TCP_ESTABLISHED = 1,
BPF_TCP_SYN_SENT,
BPF_TCP_SYN_RECV,
BPF_TCP_FIN_WAIT1,
to uapi/linux/bpf.h
and have BUILD_BUG_ON(BPF_TCP_ESTABLISHED != TCP_ESTABLISHED);
and in case somebody touches that enum in the middle
the issue will be caught.
I'd like to hear Dave and Eric opinion here.
next prev parent reply other threads:[~2017-12-26 23:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-22 1:20 [PATCH v2 bpf-next 00/11] Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 01/11] bpf: Make SOCK_OPS_GET_TCP size independent Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 02/11] bpf: Make SOCK_OPS_GET_TCP struct independent Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 03/11] bpf: Add write access to tcp_sock and sock fields Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 04/11] bpf: Support passing args to sock_ops bpf function Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 05/11] bpf: Adds field bpf_sock_ops_flags to tcp_sock Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 06/11] bpf: Add sock_ops RTO callback Lawrence Brakmo
2017-12-28 17:57 ` Yuchung Cheng
2017-12-31 11:20 ` Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 07/11] bpf: Add support for reading sk_state and more Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 08/11] bpf: Add sock_ops R/W access to tclass & sk_txhash Lawrence Brakmo
2017-12-22 1:20 ` [PATCH v2 bpf-next 09/11] bpf: Add BPF_SOCK_OPS_RETRANS_CB Lawrence Brakmo
2017-12-22 1:21 ` [PATCH v2 bpf-next 10/11] bpf: Add BPF_SOCK_OPS_STATE_CB Lawrence Brakmo
2017-12-26 23:23 ` Alexei Starovoitov [this message]
2017-12-22 1:21 ` [PATCH v2 bpf-next 11/11] bpf: add selftest for tcpbpf Lawrence Brakmo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171226232357.52ayp76dfwzhg62c@ast-mbp \
--to=alexei.starovoitov@gmail.com \
--cc=bmatheny@fb.com \
--cc=brakmo@fb.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).