| Message ID | 599fa718f11e3311f95deaaeac0534889698c66d.1740975633.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | BPF path manager, part 5 | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 43 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/build | success | Build and static analysis OK |
| matttbe/KVM_Validation__normal | warning | Unstable: 1 failed test(s): selftest_mptcp_connect |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf-normal__only_bpftest_all_ | success | Success! ✅ |
| matttbe/KVM_Validation__btf-debug__only_bpftest_all_ | success | Success! ✅ |
Hi Geliang, On 03/03/2025 05:22, Geliang Tang wrote: > From: Geliang Tang <tanggeliang@kylinos.cn> > > Now pm->pm_type can be replaced by pm->ops->name, then "pm_type" filed > of struct mptcp_pm_data can be dropped. > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > --- > net/mptcp/pm.c | 4 +--- > net/mptcp/protocol.h | 5 ++--- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index e8b34f2ecb35..1ce58d16370a 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -979,7 +979,6 @@ void mptcp_pm_destroy(struct mptcp_sock *msk) > void mptcp_pm_data_reset(struct mptcp_sock *msk) > { > const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk)); > - u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk)); > struct mptcp_pm_data *pm = &msk->pm; > int ret; > > @@ -989,7 +988,6 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk) > pm->subflows = 0; > pm->rm_list_tx.nr = 0; > pm->rm_list_rx.nr = 0; > - WRITE_ONCE(pm->pm_type, pm_type); > > rcu_read_lock(); > ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager)); > @@ -997,7 +995,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk) > if (ret) > return; > > - if (pm_type == MPTCP_PM_TYPE_KERNEL) { > + if (mptcp_pm_is_kernel(msk)) { The code here could be done in the new init() callback maybe? So what you introduced in the previous patch 7/11. > bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk); > > /* pm->work_pending must be only be set to 'true' when > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h > index 172450455c2a..56eeee1cbccc 100644 > --- a/net/mptcp/protocol.h > +++ b/net/mptcp/protocol.h > @@ -233,7 +233,6 @@ struct mptcp_pm_data { > u8 add_addr_signaled; > u8 add_addr_accepted; > u8 local_addr_used; > - u8 pm_type; > u8 subflows; > u8 status; > DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1); > @@ -1101,12 +1100,12 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk) > > static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk) > { > - return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE; > + return !strncmp(msk->pm.ops->name, "userspace", MPTCP_PM_NAME_MAX); Please don't do a string comparison here. I think it is better to drop msk->pm.pm_type when mptcp_pm_is_userspace and mptcp_pm_is_kernel are both dropped, no? If you want to drop pm_type before, then you could compare &msk->pm.ops, but I think that's something that should be done later. I guess mptcp_pm_is_userspace() will still be needed, but only from pm_userspace.c with mptcp_userspace_pm_get_sock(). No? Same for mptcp_pm_is_kernel(), only from pm_kernel.c when iterating over all connections. Except if you use introduce a new macro like mptcp_pm_for_each_msk() taking in argument "&mptcp_pm_kernel"? Cheers, Matt
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index e8b34f2ecb35..1ce58d16370a 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -979,7 +979,6 @@ void mptcp_pm_destroy(struct mptcp_sock *msk) void mptcp_pm_data_reset(struct mptcp_sock *msk) { const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk)); - u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk)); struct mptcp_pm_data *pm = &msk->pm; int ret; @@ -989,7 +988,6 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk) pm->subflows = 0; pm->rm_list_tx.nr = 0; pm->rm_list_rx.nr = 0; - WRITE_ONCE(pm->pm_type, pm_type); rcu_read_lock(); ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager)); @@ -997,7 +995,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk) if (ret) return; - if (pm_type == MPTCP_PM_TYPE_KERNEL) { + if (mptcp_pm_is_kernel(msk)) { bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk); /* pm->work_pending must be only be set to 'true' when diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 172450455c2a..56eeee1cbccc 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -233,7 +233,6 @@ struct mptcp_pm_data { u8 add_addr_signaled; u8 add_addr_accepted; u8 local_addr_used; - u8 pm_type; u8 subflows; u8 status; DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1); @@ -1101,12 +1100,12 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk) static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk) { - return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE; + return !strncmp(msk->pm.ops->name, "userspace", MPTCP_PM_NAME_MAX); } static inline bool mptcp_pm_is_kernel(const struct mptcp_sock *msk) { - return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_KERNEL; + return !strncmp(msk->pm.ops->name, "kernel", MPTCP_PM_NAME_MAX); } static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)