| Message ID | 0bdf5e5472e44780ce51d9241e33e2e010ecd23a.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, 40 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 mptcp_pm_get_local_id() can directly invoke get_local_id() interface > through "ops" of "msk->pm". Instead of using mptcp_pm_is_userspace() to > check which get_local_id() helper to invoke. As mentioned in a previous comment, I think it would be clearer to add get_local_id() in the PM ops structure (+ the validation, + in the different PM ops) here in this commit. Same on the next patch, and for all future PM ops: new/established/closed connections/subflow, ADD/RM_ADDR received, etc. → everywhere mptcp_pm_is_userspace() or mptcp_pm_is_kernel() is used in pm.c. Note that when it is needed to do actions from the worker, I guess it would be better to schedule the worker if the corresponding pm->ops->XXX is defined, then calling pm->ops->XXX from mptcp_pm_worker(). The idea would be to get rid of __mptcp_pm_kernel_worker(), everything should be done from mptcp_pm_worker(). That should also simplify the hooks for the BPF PMs, only having hooks called while owning the MSK socket. WDYT? Cheers, Matt
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 1ce58d16370a..848393511997 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -872,9 +872,7 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc) skc_local.addr.id = 0; skc_local.flags = MPTCP_PM_ADDR_FLAG_IMPLICIT; - if (mptcp_pm_is_userspace(msk)) - return mptcp_userspace_pm_get_local_id(msk, &skc_local); - return mptcp_pm_nl_get_local_id(msk, &skc_local); + return msk->pm.ops->get_local_id(msk, &skc_local); } bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc) diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c index 8a5966e6e3e3..3d5beaafc3ec 100644 --- a/net/mptcp/pm_kernel.c +++ b/net/mptcp/pm_kernel.c @@ -693,8 +693,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, return err; } -int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, - struct mptcp_pm_addr_entry *skc) +static int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, + struct mptcp_pm_addr_entry *skc) { struct mptcp_pm_addr_entry *entry; struct pm_nl_pernet *pernet; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index d53f44df9641..05495f6e771a 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -127,8 +127,8 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id) return NULL; } -int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, - struct mptcp_pm_addr_entry *skc) +static int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, + struct mptcp_pm_addr_entry *skc) { __be16 msk_sport = ((struct inet_sock *) inet_sk((struct sock *)msk))->inet_sport; diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 3579c31650fa..776f164a21eb 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1137,10 +1137,6 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb, bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining, struct mptcp_rm_list *rm_list); int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc); -int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, - struct mptcp_pm_addr_entry *skc); -int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, - struct mptcp_pm_addr_entry *skc); bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc); bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc); bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);