From: Mat Martineau <martineau@kernel.org>
To: Geliang Tang <geliang@kernel.org>
Cc: mptcp@lists.linux.dev, Geliang Tang <tanggeliang@kylinos.cn>
Subject: Re: [PATCH mptcp-next v7 4/5] Squash to "selftests/bpf: Add bpf_burst scheduler & test"
Date: Tue, 22 Oct 2024 17:03:37 -0700 (PDT) [thread overview]
Message-ID: <2bc8e804-3fca-086b-5050-b178eda1066a@kernel.org> (raw)
In-Reply-To: <a9d3b8bc14bd6e93cee030a80db39f3229e1216e.1729583414.git.tanggeliang@kylinos.cn>
On Tue, 22 Oct 2024, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Use the newly added bpf_for_each() helper to walk the conn_list.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> .../selftests/bpf/progs/mptcp_bpf_burst.c | 79 ++++++++++---------
> 1 file changed, 40 insertions(+), 39 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c
> index eb21119aa8f7..e7df5f048aa4 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_burst.c
> @@ -11,6 +11,10 @@ char _license[] SEC("license") = "GPL";
>
> #define min(a, b) ((a) < (b) ? (a) : (b))
>
> +#define SSK_MODE_ACTIVE 0
> +#define SSK_MODE_BACKUP 1
> +#define SSK_MODE_MAX 2
> +
Hi Geliang -
> struct bpf_subflow_send_info {
> __u8 subflow_id;
If you store a subflow pointer here instead of an index, there's no need
to add the mptcp_lookup_subflow_id() helper function, an extra
iteration over conn_list is eliminated, and the code is more like
mptcp_subflow_get_send().
- Mat
> __u64 linger_time;
> @@ -23,10 +27,6 @@ extern bool tcp_stream_memory_free(const struct sock *sk, int wake) __ksym;
> extern bool bpf_mptcp_subflow_queues_empty(struct sock *sk) __ksym;
> extern void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk) __ksym;
>
> -#define SSK_MODE_ACTIVE 0
> -#define SSK_MODE_BACKUP 1
> -#define SSK_MODE_MAX 2
> -
> static __always_inline __u64 div_u64(__u64 dividend, __u32 divisor)
> {
> return dividend / divisor;
> @@ -57,6 +57,19 @@ static __always_inline bool sk_stream_memory_free(const struct sock *sk)
> return __sk_stream_memory_free(sk, 0);
> }
>
> +static struct mptcp_subflow_context *
> +mptcp_lookup_subflow_by_id(struct mptcp_sock *msk, unsigned int id)
> +{
> + struct mptcp_subflow_context *subflow;
> +
> + bpf_for_each(mptcp_subflow, subflow, msk) {
> + if (subflow->subflow_id == id)
> + return subflow;
> + }
> +
> + return NULL;
> +}
> +
> SEC("struct_ops")
> void BPF_PROG(mptcp_sched_burst_init, struct mptcp_sock *msk)
> {
> @@ -67,8 +80,7 @@ void BPF_PROG(mptcp_sched_burst_release, struct mptcp_sock *msk)
> {
> }
>
> -static int bpf_burst_get_send(struct mptcp_sock *msk,
> - struct mptcp_sched_data *data)
> +static int bpf_burst_get_send(struct mptcp_sock *msk)
> {
> struct bpf_subflow_send_info send_info[SSK_MODE_MAX];
> struct mptcp_subflow_context *subflow;
> @@ -84,16 +96,10 @@ static int bpf_burst_get_send(struct mptcp_sock *msk,
> send_info[i].linger_time = -1;
> }
>
> - for (i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
> - bool backup;
> + bpf_for_each(mptcp_subflow, subflow, msk) {
> + bool backup = subflow->backup || subflow->request_bkup;
>
> - subflow = bpf_mptcp_subflow_ctx_by_pos(data, i);
> - if (!subflow)
> - break;
> -
> - backup = subflow->backup || subflow->request_bkup;
> -
> - ssk = mptcp_subflow_tcp_sock(subflow);
> + ssk = bpf_mptcp_subflow_tcp_sock(subflow);
> if (!mptcp_subflow_active(subflow))
> continue;
>
> @@ -109,7 +115,7 @@ static int bpf_burst_get_send(struct mptcp_sock *msk,
>
> linger_time = div_u64((__u64)ssk->sk_wmem_queued << 32, pace);
> if (linger_time < send_info[backup].linger_time) {
> - send_info[backup].subflow_id = i;
> + send_info[backup].subflow_id = subflow->subflow_id;
> send_info[backup].linger_time = linger_time;
> }
> }
> @@ -119,10 +125,10 @@ static int bpf_burst_get_send(struct mptcp_sock *msk,
> if (!nr_active)
> send_info[SSK_MODE_ACTIVE].subflow_id = send_info[SSK_MODE_BACKUP].subflow_id;
>
> - subflow = bpf_mptcp_subflow_ctx_by_pos(data, send_info[SSK_MODE_ACTIVE].subflow_id);
> + subflow = mptcp_lookup_subflow_by_id(msk, send_info[SSK_MODE_ACTIVE].subflow_id);
> if (!subflow)
> return -1;
> - ssk = mptcp_subflow_tcp_sock(subflow);
> + ssk = bpf_mptcp_subflow_tcp_sock(subflow);
> if (!ssk || !sk_stream_memory_free(ssk))
> return -1;
>
> @@ -141,23 +147,18 @@ static int bpf_burst_get_send(struct mptcp_sock *msk,
> return 0;
> }
>
> -static int bpf_burst_get_retrans(struct mptcp_sock *msk,
> - struct mptcp_sched_data *data)
> +static int bpf_burst_get_retrans(struct mptcp_sock *msk)
> {
> - int backup = MPTCP_SUBFLOWS_MAX, pick = MPTCP_SUBFLOWS_MAX, subflow_id;
> + struct sock *backup = NULL, *pick = NULL;
> struct mptcp_subflow_context *subflow;
> int min_stale_count = INT_MAX;
> - struct sock *ssk;
>
> - for (int i = 0; i < data->subflows && i < MPTCP_SUBFLOWS_MAX; i++) {
> - subflow = bpf_mptcp_subflow_ctx_by_pos(data, i);
> - if (!subflow)
> - break;
> + bpf_for_each(mptcp_subflow, subflow, msk) {
> + struct sock *ssk = bpf_mptcp_subflow_tcp_sock(subflow);
>
> if (!mptcp_subflow_active(subflow))
> continue;
>
> - ssk = mptcp_subflow_tcp_sock(subflow);
> /* still data outstanding at TCP level? skip this */
> if (!tcp_rtx_and_write_queues_empty(ssk)) {
> mptcp_pm_subflow_chk_stale(msk, ssk);
> @@ -166,23 +167,23 @@ static int bpf_burst_get_retrans(struct mptcp_sock *msk,
> }
>
> if (subflow->backup || subflow->request_bkup) {
> - if (backup == MPTCP_SUBFLOWS_MAX)
> - backup = i;
> + if (!backup)
> + backup = ssk;
> continue;
> }
>
> - if (pick == MPTCP_SUBFLOWS_MAX)
> - pick = i;
> + if (!pick)
> + pick = ssk;
> }
>
> - if (pick < MPTCP_SUBFLOWS_MAX) {
> - subflow_id = pick;
> + if (pick)
> goto out;
> - }
> - subflow_id = min_stale_count > 1 ? backup : MPTCP_SUBFLOWS_MAX;
> + pick = min_stale_count > 1 ? backup : NULL;
>
> out:
> - subflow = bpf_mptcp_subflow_ctx_by_pos(data, subflow_id);
> + if (!pick)
> + return -1;
> + subflow = bpf_mptcp_subflow_ctx(pick);
> if (!subflow)
> return -1;
> mptcp_subflow_set_scheduled(subflow, true);
> @@ -194,11 +195,11 @@ int BPF_PROG(bpf_burst_get_subflow, struct mptcp_sock *msk,
> struct mptcp_sched_data *data)
> {
> if (data->reinject)
> - return bpf_burst_get_retrans(msk, data);
> - return bpf_burst_get_send(msk, data);
> + return bpf_burst_get_retrans(msk);
> + return bpf_burst_get_send(msk);
> }
>
> -SEC(".struct_ops")
> +SEC(".struct_ops.link")
> struct mptcp_sched_ops burst = {
> .init = (void *)mptcp_sched_burst_init,
> .release = (void *)mptcp_sched_burst_release,
> --
> 2.45.2
>
>
>
next prev parent reply other threads:[~2024-10-23 0:03 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-22 7:52 [PATCH mptcp-next v7 0/5] use bpf_iter in bpf schedulers Geliang Tang
2024-10-22 7:52 ` [PATCH mptcp-next v7 1/5] Squash to "selftests/bpf: Add bpf_bkup scheduler & test" Geliang Tang
2024-10-22 7:52 ` [PATCH mptcp-next v7 2/5] Squash to "selftests/bpf: Add bpf_rr " Geliang Tang
2024-10-22 23:52 ` Mat Martineau
2024-10-23 8:32 ` Geliang Tang
2024-10-22 7:52 ` [PATCH mptcp-next v7 3/5] Squash to "selftests/bpf: Add bpf_red " Geliang Tang
2024-10-22 7:52 ` [PATCH mptcp-next v7 4/5] Squash to "selftests/bpf: Add bpf_burst " Geliang Tang
2024-10-23 0:03 ` Mat Martineau [this message]
2024-10-23 8:57 ` Geliang Tang
2024-10-22 7:52 ` [PATCH mptcp-next v7 5/5] Squash to "selftests/bpf: Add bpf_first " Geliang Tang
2024-10-23 0:10 ` Mat Martineau
2024-10-23 9:00 ` Geliang Tang
2024-10-22 9:04 ` [PATCH mptcp-next v7 0/5] use bpf_iter in bpf schedulers MPTCP CI
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=2bc8e804-3fca-086b-5050-b178eda1066a@kernel.org \
--to=martineau@kernel.org \
--cc=geliang@kernel.org \
--cc=mptcp@lists.linux.dev \
--cc=tanggeliang@kylinos.cn \
/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