Merge branch 'Do not limit cb_flags when creating child sk'
Martin KaFai says: ==================== This set fixes an issue that the bpf_skops_init_child() unnecessarily limited the child sk from inheriting all bpf_sock_ops_cb_flags of the listen sk. It also adds a test to check that. ==================== Tested-by: Stanislav Fomichev <sdf@google.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
commit
440c5752a3
@ -2228,34 +2228,6 @@ int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock *psock,
|
||||
#endif /* CONFIG_NET_SOCK_MSG */
|
||||
|
||||
#ifdef CONFIG_CGROUP_BPF
|
||||
/* Copy the listen sk's HDR_OPT_CB flags to its child.
|
||||
*
|
||||
* During 3-Way-HandShake, the synack is usually sent from
|
||||
* the listen sk with the HDR_OPT_CB flags set so that
|
||||
* bpf-prog will be called to write the BPF hdr option.
|
||||
*
|
||||
* In fastopen, the child sk is used to send synack instead
|
||||
* of the listen sk. Thus, inheriting the HDR_OPT_CB flags
|
||||
* from the listen sk gives the bpf-prog a chance to write
|
||||
* BPF hdr option in the synack pkt during fastopen.
|
||||
*
|
||||
* Both fastopen and non-fastopen child will inherit the
|
||||
* HDR_OPT_CB flags to keep the bpf-prog having a consistent
|
||||
* behavior when deciding to clear this cb flags (or not)
|
||||
* during the PASSIVE_ESTABLISHED_CB.
|
||||
*
|
||||
* In the future, other cb flags could be inherited here also.
|
||||
*/
|
||||
static inline void bpf_skops_init_child(const struct sock *sk,
|
||||
struct sock *child)
|
||||
{
|
||||
tcp_sk(child)->bpf_sock_ops_cb_flags =
|
||||
tcp_sk(sk)->bpf_sock_ops_cb_flags &
|
||||
(BPF_SOCK_OPS_PARSE_ALL_HDR_OPT_CB_FLAG |
|
||||
BPF_SOCK_OPS_PARSE_UNKNOWN_HDR_OPT_CB_FLAG |
|
||||
BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG);
|
||||
}
|
||||
|
||||
static inline void bpf_skops_init_skb(struct bpf_sock_ops_kern *skops,
|
||||
struct sk_buff *skb,
|
||||
unsigned int end_offset)
|
||||
@ -2264,11 +2236,6 @@ static inline void bpf_skops_init_skb(struct bpf_sock_ops_kern *skops,
|
||||
skops->skb_data_end = skb->data + end_offset;
|
||||
}
|
||||
#else
|
||||
static inline void bpf_skops_init_child(const struct sock *sk,
|
||||
struct sock *child)
|
||||
{
|
||||
}
|
||||
|
||||
static inline void bpf_skops_init_skb(struct bpf_sock_ops_kern *skops,
|
||||
struct sk_buff *skb,
|
||||
unsigned int end_offset)
|
||||
|
@ -548,7 +548,6 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
|
||||
newtp->fastopen_req = NULL;
|
||||
RCU_INIT_POINTER(newtp->fastopen_rsk, NULL);
|
||||
|
||||
bpf_skops_init_child(sk, newsk);
|
||||
tcp_bpf_clone(sk, newsk);
|
||||
|
||||
__TCP_INC_STATS(sock_net(sk), TCP_MIB_PASSIVEOPENS);
|
||||
|
@ -264,9 +264,19 @@ static int check_error_linum(const struct sk_fds *sk_fds)
|
||||
|
||||
static void check_hdr_and_close_fds(struct sk_fds *sk_fds)
|
||||
{
|
||||
const __u32 expected_inherit_cb_flags =
|
||||
BPF_SOCK_OPS_PARSE_UNKNOWN_HDR_OPT_CB_FLAG |
|
||||
BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG |
|
||||
BPF_SOCK_OPS_STATE_CB_FLAG;
|
||||
|
||||
if (sk_fds_shutdown(sk_fds))
|
||||
goto check_linum;
|
||||
|
||||
if (CHECK(expected_inherit_cb_flags != skel->bss->inherit_cb_flags,
|
||||
"Unexpected inherit_cb_flags", "0x%x != 0x%x\n",
|
||||
skel->bss->inherit_cb_flags, expected_inherit_cb_flags))
|
||||
goto check_linum;
|
||||
|
||||
if (check_hdr_stg(&exp_passive_hdr_stg, sk_fds->passive_fd,
|
||||
"passive_hdr_stg"))
|
||||
goto check_linum;
|
||||
@ -321,6 +331,8 @@ static void reset_test(void)
|
||||
memset(&skel->bss->active_estab_in, 0, optsize);
|
||||
memset(&skel->bss->active_fin_in, 0, optsize);
|
||||
|
||||
skel->bss->inherit_cb_flags = 0;
|
||||
|
||||
skel->data->test_kind = TCPOPT_EXP;
|
||||
skel->data->test_magic = 0xeB9F;
|
||||
|
||||
|
@ -304,10 +304,10 @@ int misc_estab(struct bpf_sock_ops *skops)
|
||||
passive_lport_n = __bpf_htons(passive_lport_h);
|
||||
bpf_setsockopt(skops, SOL_TCP, TCP_SAVE_SYN,
|
||||
&true_val, sizeof(true_val));
|
||||
set_hdr_cb_flags(skops);
|
||||
set_hdr_cb_flags(skops, 0);
|
||||
break;
|
||||
case BPF_SOCK_OPS_TCP_CONNECT_CB:
|
||||
set_hdr_cb_flags(skops);
|
||||
set_hdr_cb_flags(skops, 0);
|
||||
break;
|
||||
case BPF_SOCK_OPS_PARSE_HDR_OPT_CB:
|
||||
return handle_parse_hdr(skops);
|
||||
|
@ -21,6 +21,7 @@
|
||||
|
||||
__u8 test_kind = TCPOPT_EXP;
|
||||
__u16 test_magic = 0xeB9F;
|
||||
__u32 inherit_cb_flags = 0;
|
||||
|
||||
struct bpf_test_option passive_synack_out = {};
|
||||
struct bpf_test_option passive_fin_out = {};
|
||||
@ -467,6 +468,8 @@ static int handle_passive_estab(struct bpf_sock_ops *skops)
|
||||
struct tcphdr *th;
|
||||
int err;
|
||||
|
||||
inherit_cb_flags = skops->bpf_sock_ops_cb_flags;
|
||||
|
||||
err = load_option(skops, &passive_estab_in, true);
|
||||
if (err == -ENOENT) {
|
||||
/* saved_syn is not found. It was in syncookie mode.
|
||||
@ -600,10 +603,10 @@ int estab(struct bpf_sock_ops *skops)
|
||||
case BPF_SOCK_OPS_TCP_LISTEN_CB:
|
||||
bpf_setsockopt(skops, SOL_TCP, TCP_SAVE_SYN,
|
||||
&true_val, sizeof(true_val));
|
||||
set_hdr_cb_flags(skops);
|
||||
set_hdr_cb_flags(skops, BPF_SOCK_OPS_STATE_CB_FLAG);
|
||||
break;
|
||||
case BPF_SOCK_OPS_TCP_CONNECT_CB:
|
||||
set_hdr_cb_flags(skops);
|
||||
set_hdr_cb_flags(skops, 0);
|
||||
break;
|
||||
case BPF_SOCK_OPS_PARSE_HDR_OPT_CB:
|
||||
return handle_parse_hdr(skops);
|
||||
|
@ -110,12 +110,13 @@ static inline void clear_hdr_cb_flags(struct bpf_sock_ops *skops)
|
||||
BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG));
|
||||
}
|
||||
|
||||
static inline void set_hdr_cb_flags(struct bpf_sock_ops *skops)
|
||||
static inline void set_hdr_cb_flags(struct bpf_sock_ops *skops, __u32 extra)
|
||||
{
|
||||
bpf_sock_ops_cb_flags_set(skops,
|
||||
skops->bpf_sock_ops_cb_flags |
|
||||
BPF_SOCK_OPS_PARSE_UNKNOWN_HDR_OPT_CB_FLAG |
|
||||
BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG);
|
||||
BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG |
|
||||
extra);
|
||||
}
|
||||
static inline void
|
||||
clear_parse_all_hdr_cb_flags(struct bpf_sock_ops *skops)
|
||||
|
Loading…
Reference in New Issue
Block a user