include/linux/bpf.h | 32 ----------------------------- kernel/bpf/verifier.c | 20 ------------------ net/mptcp/bpf.c | 47 ++----------------------------------------- 3 files changed, 2 insertions(+), 97 deletions(-)
This patch addressed to the comments in [1] from BPF maintainer Alexei
Starovoitov:
'''
On Fri, Sep 18, 2020 at 02:10:42PM +0200, Nicolas Rybowski wrote:
> +
> +BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
> +{
> + if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
> + struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
Could you add !sk check here as well?
See commit 8c33dadc3e0e ("bpf: Bpf_skc_to_* casting helpers require a NULL check on sk")
It's not strictly necessary yet, but see below.
'''
I added this !sk check as he suggested.
'''
I think we shouldn't extend the verifier with PTR_TO_MPTCP_SOCK and similar concept anymore.
This approach doesn't scale and we have better way to handle such field access with BTF.
> + }
> + return (unsigned long)NULL;
> +}
> +
> +const struct bpf_func_proto bpf_mptcp_sock_proto = {
> + .func = bpf_mptcp_sock,
> + .gpl_only = false,
> + .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
In this particular case you can do:
+ .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
Then bpf_mptcp_sock_convert_ctx_access() will no longer be necessary
and bpf prog will be able to access all mptcp_sock fields right away.
Will that work for your use case?
'''
I dropped RET_PTR_TO_MPTCP_SOCK_OR_NULL, use RET_PTR_TO_BTF_ID_OR_NULL
instead as he suggested. Drop bpf_mptcp_sock_is_valid_access() and
bpf_mptcp_sock_convert_ctx_access() too. I tested our case, it works.
Now only one comment from him I don't know how to implement:
'''
Also this new helper is not exercised from C test. Only from asm.
Could you update patch 4 with such additional logic?
'''
Please give me some suggestions?
[1]
https://lore.kernel.org/netdev/20200922040830.3iis6xiavhvpfq3v@ast-mbp.dhcp.thefacebook.com/
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
include/linux/bpf.h | 32 -----------------------------
kernel/bpf/verifier.c | 20 ------------------
net/mptcp/bpf.c | 47 ++-----------------------------------------
3 files changed, 2 insertions(+), 97 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a45b91db8477..0427b608990d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -405,7 +405,6 @@ enum bpf_return_type {
RET_PTR_TO_MAP_VALUE, /* returns a pointer to map elem value */
RET_PTR_TO_SOCKET, /* returns a pointer to a socket */
RET_PTR_TO_TCP_SOCK, /* returns a pointer to a tcp_sock */
- RET_PTR_TO_MPTCP_SOCK, /* returns a pointer to mptcp_sock */
RET_PTR_TO_SOCK_COMMON, /* returns a pointer to a sock_common */
RET_PTR_TO_ALLOC_MEM, /* returns a pointer to dynamically allocated memory */
RET_PTR_TO_MEM_OR_BTF_ID, /* returns a pointer to a valid memory or a btf_id */
@@ -416,7 +415,6 @@ enum bpf_return_type {
RET_PTR_TO_MAP_VALUE_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_MAP_VALUE,
RET_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET,
RET_PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK,
- RET_PTR_TO_MPTCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_MPTCP_SOCK,
RET_PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON,
RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_ALLOC_MEM,
RET_PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID,
@@ -497,7 +495,6 @@ enum bpf_reg_type {
PTR_TO_SOCKET, /* reg points to struct bpf_sock */
PTR_TO_SOCK_COMMON, /* reg points to sock_common */
PTR_TO_TCP_SOCK, /* reg points to struct tcp_sock */
- PTR_TO_MPTCP_SOCK, /* reg points to struct mptcp_sock */
PTR_TO_TP_BUFFER, /* reg points to a writable raw tp's buffer */
PTR_TO_XDP_SOCK, /* reg points to struct xdp_sock */
/* PTR_TO_BTF_ID points to a kernel struct that does not need
@@ -526,7 +523,6 @@ enum bpf_reg_type {
PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | PTR_TO_SOCKET,
PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | PTR_TO_SOCK_COMMON,
PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | PTR_TO_TCP_SOCK,
- PTR_TO_MPTCP_SOCK_OR_NULL = PTR_MAYBE_NULL | PTR_TO_MPTCP_SOCK,
PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | PTR_TO_BTF_ID,
/* This must be the last entry. Its purpose is to ensure the enum is
@@ -2353,34 +2349,6 @@ static inline u32 bpf_xdp_sock_convert_ctx_access(enum bpf_access_type type,
}
#endif /* CONFIG_INET */
-#ifdef CONFIG_MPTCP
-bool bpf_mptcp_sock_is_valid_access(int off, int size,
- enum bpf_access_type type,
- struct bpf_insn_access_aux *info);
-
-u32 bpf_mptcp_sock_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog,
- u32 *target_size);
-#else /* CONFIG_MPTCP */
-static inline bool bpf_mptcp_sock_is_valid_access(int off, int size,
- enum bpf_access_type type,
- struct bpf_insn_access_aux *info)
-{
- return false;
-}
-
-static inline u32 bpf_mptcp_sock_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog,
- u32 *target_size)
-{
- return 0;
-}
-#endif /* CONFIG_MPTCP */
-
enum bpf_text_poke_type {
BPF_MOD_CALL,
BPF_MOD_JUMP,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fdb8fc0e7368..dcf065ec2774 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -430,7 +430,6 @@ static bool type_is_sk_pointer(enum bpf_reg_type type)
return type == PTR_TO_SOCKET ||
type == PTR_TO_SOCK_COMMON ||
type == PTR_TO_TCP_SOCK ||
- type == PTR_TO_MPTCP_SOCK ||
type == PTR_TO_XDP_SOCK;
}
@@ -438,7 +437,6 @@ static bool reg_type_not_null(enum bpf_reg_type type)
{
return type == PTR_TO_SOCKET ||
type == PTR_TO_TCP_SOCK ||
- type == PTR_TO_MPTCP_SOCK ||
type == PTR_TO_MAP_VALUE ||
type == PTR_TO_MAP_KEY ||
type == PTR_TO_SOCK_COMMON;
@@ -454,7 +452,6 @@ static bool reg_type_may_be_refcounted_or_null(enum bpf_reg_type type)
{
return base_type(type) == PTR_TO_SOCKET ||
base_type(type) == PTR_TO_TCP_SOCK ||
- base_type(type) == PTR_TO_MPTCP_SOCK ||
base_type(type) == PTR_TO_MEM ||
base_type(type) == PTR_TO_BTF_ID;
}
@@ -554,7 +551,6 @@ static const char *reg_type_str(struct bpf_verifier_env *env,
[PTR_TO_SOCKET] = "sock",
[PTR_TO_SOCK_COMMON] = "sock_common",
[PTR_TO_TCP_SOCK] = "tcp_sock",
- [PTR_TO_MPTCP_SOCK] = "mptcp_sock",
[PTR_TO_TP_BUFFER] = "tp_buffer",
[PTR_TO_XDP_SOCK] = "xdp_sock",
[PTR_TO_BTF_ID] = "ptr_",
@@ -2766,7 +2762,6 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
case PTR_TO_BTF_ID:
case PTR_TO_BUF:
@@ -3659,9 +3654,6 @@ static int check_sock_access(struct bpf_verifier_env *env, int insn_idx,
case PTR_TO_TCP_SOCK:
valid = bpf_tcp_sock_is_valid_access(off, size, t, &info);
break;
- case PTR_TO_MPTCP_SOCK:
- valid = bpf_mptcp_sock_is_valid_access(off, size, t, &info);
- break;
case PTR_TO_XDP_SOCK:
valid = bpf_xdp_sock_is_valid_access(off, size, t, &info);
break;
@@ -3818,9 +3810,6 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
case PTR_TO_TCP_SOCK:
pointer_desc = "tcp_sock ";
break;
- case PTR_TO_MPTCP_SOCK:
- pointer_desc = "mptcp_sock ";
- break;
case PTR_TO_XDP_SOCK:
pointer_desc = "xdp_sock ";
break;
@@ -6740,9 +6729,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
} else if (base_type(ret_type) == RET_PTR_TO_TCP_SOCK) {
mark_reg_known_zero(env, regs, BPF_REG_0);
regs[BPF_REG_0].type = PTR_TO_TCP_SOCK | ret_flag;
- } else if (base_type(ret_type) == RET_PTR_TO_MPTCP_SOCK) {
- mark_reg_known_zero(env, regs, BPF_REG_0);
- regs[BPF_REG_0].type = PTR_TO_MPTCP_SOCK | ret_flag;
} else if (base_type(ret_type) == RET_PTR_TO_ALLOC_MEM) {
mark_reg_known_zero(env, regs, BPF_REG_0);
regs[BPF_REG_0].type = PTR_TO_MEM | ret_flag;
@@ -7457,7 +7443,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
verbose(env, "R%d pointer arithmetic on %s prohibited\n",
dst, reg_type_str(env, ptr_reg->type));
@@ -10832,7 +10817,6 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
/* Only valid matches are exact, which memcmp() above
* would have accepted
@@ -11363,7 +11347,6 @@ static bool reg_type_mismatch_ok(enum bpf_reg_type type)
case PTR_TO_SOCKET:
case PTR_TO_SOCK_COMMON:
case PTR_TO_TCP_SOCK:
- case PTR_TO_MPTCP_SOCK:
case PTR_TO_XDP_SOCK:
case PTR_TO_BTF_ID:
return false;
@@ -12788,9 +12771,6 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
case PTR_TO_TCP_SOCK:
convert_ctx_access = bpf_tcp_sock_convert_ctx_access;
break;
- case PTR_TO_MPTCP_SOCK:
- convert_ctx_access = bpf_mptcp_sock_convert_ctx_access;
- break;
case PTR_TO_XDP_SOCK:
convert_ctx_access = bpf_xdp_sock_convert_ctx_access;
break;
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 5332469fbb28..d1dbdf5d60b0 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -11,52 +11,9 @@
#include "protocol.h"
-bool bpf_mptcp_sock_is_valid_access(int off, int size, enum bpf_access_type type,
- struct bpf_insn_access_aux *info)
-{
- if (off < 0 || off >= offsetofend(struct bpf_mptcp_sock, token))
- return false;
-
- if (off % size != 0)
- return false;
-
- switch (off) {
- default:
- return size == sizeof(__u32);
- }
-}
-
-u32 bpf_mptcp_sock_convert_ctx_access(enum bpf_access_type type,
- const struct bpf_insn *si,
- struct bpf_insn *insn_buf,
- struct bpf_prog *prog, u32 *target_size)
-{
- struct bpf_insn *insn = insn_buf;
-
-#define BPF_MPTCP_SOCK_GET_COMMON(FIELD) \
- do { \
- BUILD_BUG_ON(sizeof_field(struct mptcp_sock, FIELD) > \
- sizeof_field(struct bpf_mptcp_sock, FIELD)); \
- *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct mptcp_sock, FIELD), \
- si->dst_reg, si->src_reg, \
- offsetof(struct mptcp_sock, FIELD)); \
- } while (0)
-
- if (insn > insn_buf)
- return insn - insn_buf;
-
- switch (si->off) {
- case offsetof(struct bpf_mptcp_sock, token):
- BPF_MPTCP_SOCK_GET_COMMON(token);
- break;
- }
-
- return insn - insn_buf;
-}
-
BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
{
- if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
+ if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
return (unsigned long)mptcp_sfc->conn;
@@ -67,6 +24,6 @@ BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
const struct bpf_func_proto bpf_mptcp_sock_proto = {
.func = bpf_mptcp_sock,
.gpl_only = false,
- .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
+ .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
.arg1_type = ARG_PTR_TO_SOCK_COMMON,
};
--
2.34.1
Hi Geliang,
On 10/02/2022 12:03, Geliang Tang wrote:
> This patch addressed to the comments in [1] from BPF maintainer Alexei
> Starovoitov:
Thank you for looking at that!
> '''
> On Fri, Sep 18, 2020 at 02:10:42PM +0200, Nicolas Rybowski wrote:
>> +
>> +BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
>> +{
>> + if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
>> + struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
>
> Could you add !sk check here as well?
> See commit 8c33dadc3e0e ("bpf: Bpf_skc_to_* casting helpers require a NULL check on sk")
> It's not strictly necessary yet, but see below.
> '''
>
> I added this !sk check as he suggested.
>
> '''
> I think we shouldn't extend the verifier with PTR_TO_MPTCP_SOCK and similar concept anymore.
> This approach doesn't scale and we have better way to handle such field access with BTF.
>
>> + }
>> + return (unsigned long)NULL;
>> +}
>> +
>> +const struct bpf_func_proto bpf_mptcp_sock_proto = {
>> + .func = bpf_mptcp_sock,
>> + .gpl_only = false,
>> + .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
>
> In this particular case you can do:
> + .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
>
> Then bpf_mptcp_sock_convert_ctx_access() will no longer be necessary
> and bpf prog will be able to access all mptcp_sock fields right away.
> Will that work for your use case?
> '''
>
> I dropped RET_PTR_TO_MPTCP_SOCK_OR_NULL, use RET_PTR_TO_BTF_ID_OR_NULL
> instead as he suggested. Drop bpf_mptcp_sock_is_valid_access() and
> bpf_mptcp_sock_convert_ctx_access() too. I tested our case, it works.
Is it OK to let access to all mptcp_sock fields?
e.g. local/remote keys?
I guess it is fine because not everybody can access them but it is
certainly impotant to check :)
> Now only one comment from him I don't know how to implement:
>
> '''
> Also this new helper is not exercised from C test. Only from asm.
> Could you update patch 4 with such additional logic?
> '''
>
> Please give me some suggestions?
Is there no other selftests looking at tcp/udp socks?
Also, do mind looking at the "userspace" tests Nicolas developed to
check nothing has been broked there too? He pushed everything there with
a README:
https://github.com/multipath-tcp/mptcp_net-next/tree/scripts/bpf/examples
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
On Thu, Feb 10, 2022 at 01:01:43PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 10/02/2022 12:03, Geliang Tang wrote:
> > This patch addressed to the comments in [1] from BPF maintainer Alexei
> > Starovoitov:
>
> Thank you for looking at that!
>
> > '''
> > On Fri, Sep 18, 2020 at 02:10:42PM +0200, Nicolas Rybowski wrote:
> >> +
> >> +BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
> >> +{
> >> + if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
> >> + struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
> >
> > Could you add !sk check here as well?
> > See commit 8c33dadc3e0e ("bpf: Bpf_skc_to_* casting helpers require a NULL check on sk")
> > It's not strictly necessary yet, but see below.
> > '''
> >
> > I added this !sk check as he suggested.
> >
> > '''
> > I think we shouldn't extend the verifier with PTR_TO_MPTCP_SOCK and similar concept anymore.
> > This approach doesn't scale and we have better way to handle such field access with BTF.
> >
> >> + }
> >> + return (unsigned long)NULL;
> >> +}
> >> +
> >> +const struct bpf_func_proto bpf_mptcp_sock_proto = {
> >> + .func = bpf_mptcp_sock,
> >> + .gpl_only = false,
> >> + .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
> >
> > In this particular case you can do:
> > + .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
> >
> > Then bpf_mptcp_sock_convert_ctx_access() will no longer be necessary
> > and bpf prog will be able to access all mptcp_sock fields right away.
> > Will that work for your use case?
> > '''
> >
> > I dropped RET_PTR_TO_MPTCP_SOCK_OR_NULL, use RET_PTR_TO_BTF_ID_OR_NULL
> > instead as he suggested. Drop bpf_mptcp_sock_is_valid_access() and
> > bpf_mptcp_sock_convert_ctx_access() too. I tested our case, it works.
>
> Is it OK to let access to all mptcp_sock fields?
> e.g. local/remote keys?
>
> I guess it is fine because not everybody can access them but it is
> certainly impotant to check :)
>
> > Now only one comment from him I don't know how to implement:
> >
> > '''
> > Also this new helper is not exercised from C test. Only from asm.
> > Could you update patch 4 with such additional logic?
> > '''
> >
> > Please give me some suggestions?
>
> Is there no other selftests looking at tcp/udp socks?
>
> Also, do mind looking at the "userspace" tests Nicolas developed to
> check nothing has been broked there too? He pushed everything there with
> a README:
>
> https://github.com/multipath-tcp/mptcp_net-next/tree/scripts/bpf/examples
Hi Matt,
Sorry for the late reply. It took me some time to run this userspace
tests. These tests still work, although there're several building warnings.
We can't drop RET_PTR_TO_MPTCP_SOCK_OR_NULL as Alexei suggested, it will
break this userspace tests.
So I just sent a v2 of this patch, only do the '!sk' check in it.
Thanks,
-Geliang
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>
Hi Geliang,
On 21/02/2022 10:05, Geliang Tang wrote:
> On Thu, Feb 10, 2022 at 01:01:43PM +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 10/02/2022 12:03, Geliang Tang wrote:
>>> This patch addressed to the comments in [1] from BPF maintainer Alexei
>>> Starovoitov:
>>
>> Thank you for looking at that!
>>
>>> '''
>>> On Fri, Sep 18, 2020 at 02:10:42PM +0200, Nicolas Rybowski wrote:
>>>> +
>>>> +BPF_CALL_1(bpf_mptcp_sock, struct sock *, sk)
>>>> +{
>>>> + if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
>>>> + struct mptcp_subflow_context *mptcp_sfc = mptcp_subflow_ctx(sk);
>>>
>>> Could you add !sk check here as well?
>>> See commit 8c33dadc3e0e ("bpf: Bpf_skc_to_* casting helpers require a NULL check on sk")
>>> It's not strictly necessary yet, but see below.
>>> '''
>>>
>>> I added this !sk check as he suggested.
>>>
>>> '''
>>> I think we shouldn't extend the verifier with PTR_TO_MPTCP_SOCK and similar concept anymore.
>>> This approach doesn't scale and we have better way to handle such field access with BTF.
>>>
>>>> + }
>>>> + return (unsigned long)NULL;
>>>> +}
>>>> +
>>>> +const struct bpf_func_proto bpf_mptcp_sock_proto = {
>>>> + .func = bpf_mptcp_sock,
>>>> + .gpl_only = false,
>>>> + .ret_type = RET_PTR_TO_MPTCP_SOCK_OR_NULL,
>>>
>>> In this particular case you can do:
>>> + .ret_type = RET_PTR_TO_BTF_ID_OR_NULL,
>>>
>>> Then bpf_mptcp_sock_convert_ctx_access() will no longer be necessary
>>> and bpf prog will be able to access all mptcp_sock fields right away.
>>> Will that work for your use case?
>>> '''
>>>
>>> I dropped RET_PTR_TO_MPTCP_SOCK_OR_NULL, use RET_PTR_TO_BTF_ID_OR_NULL
>>> instead as he suggested. Drop bpf_mptcp_sock_is_valid_access() and
>>> bpf_mptcp_sock_convert_ctx_access() too. I tested our case, it works.
>>
>> Is it OK to let access to all mptcp_sock fields?
>> e.g. local/remote keys?
>>
>> I guess it is fine because not everybody can access them but it is
>> certainly impotant to check :)
>>
>>> Now only one comment from him I don't know how to implement:
>>>
>>> '''
>>> Also this new helper is not exercised from C test. Only from asm.
>>> Could you update patch 4 with such additional logic?
>>> '''
>>>
>>> Please give me some suggestions?
>>
>> Is there no other selftests looking at tcp/udp socks?
>>
>> Also, do mind looking at the "userspace" tests Nicolas developed to
>> check nothing has been broked there too? He pushed everything there with
>> a README:
>>
>> https://github.com/multipath-tcp/mptcp_net-next/tree/scripts/bpf/examples
>
> Hi Matt,
>
> Sorry for the late reply. It took me some time to run this userspace
> tests. These tests still work, although there're several building warnings.
No hurry :)
> We can't drop RET_PTR_TO_MPTCP_SOCK_OR_NULL as Alexei suggested, it will
> break this userspace tests.
I don't remember what these userspace tests were doing but for me, it is
fine to modify them. Because the corresponding kernel patches have not
been upstreamed, it is fine to adapt them and "break the uAPI" if we are
still able to do the same things.
Or do you mean if we drop RET_PTR_TO_MPTCP_SOCK_OR_NULL, we can no
longer adapt these scripts to do what was done before?
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
On 21/02/2022 10:12, Matthieu Baerts wrote: > On 21/02/2022 10:05, Geliang Tang wrote: (...) >> We can't drop RET_PTR_TO_MPTCP_SOCK_OR_NULL as Alexei suggested, it will >> break this userspace tests. > > I don't remember what these userspace tests were doing The idea was to be able to use the different hooks for TCP and - have a way to identify the socket is in fact an MPTCP subflow - be able to link this subflow with others of the same MPTCP connections With the implementation from the export branch, it is possible to get the msk from the ssk. From the msk, it is possible to get the token and use it as a key in a hashmap. For some use cases, it might be needed to retrieve other subflow sk from this msk. To validate that, a different mark was set per subflow: all first subflows had a mark of 1, then it was incremented for each subflow. Also, a different TCP CC algorithm was set only on the first subflow. Also just to check if everything was OK there. If it is still possible to do that without RET_PTR_TO_MPTCP_SOCK_OR_NULL but differently, that's perfectly fine and everybody is happy :) Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
© 2016 - 2024 Red Hat, Inc.