Re: [PATCH net-next 2/3] bpf: add TCP_BPF_RTO_MAX for bpf_setsockopt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 14, 2025 at 2:40 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 2/13/25 10:12 PM, Jason Xing wrote:
> > On Fri, Feb 14, 2025 at 1:41 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
> >>
> >> On 2/13/25 7:09 PM, Jason Xing wrote:
> >>> On Fri, Feb 14, 2025 at 10:14 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
> >>>>
> >>>> On 2/13/25 3:57 PM, Jason Xing wrote:
> >>>>> On Fri, Feb 14, 2025 at 7:41 AM Stanislav Fomichev<stfomichev@xxxxxxxxx> wrote:
> >>>>>> On 02/13, Jason Xing wrote:
> >>>>>>> Support bpf_setsockopt() to set the maximum value of RTO for
> >>>>>>> BPF program.
> >>>>>>>
> >>>>>>> Signed-off-by: Jason Xing<kerneljasonxing@xxxxxxxxx>
> >>>>>>> ---
> >>>>>>>     Documentation/networking/ip-sysctl.rst | 3 ++-
> >>>>>>>     include/uapi/linux/bpf.h               | 2 ++
> >>>>>>>     net/core/filter.c                      | 6 ++++++
> >>>>>>>     tools/include/uapi/linux/bpf.h         | 2 ++
> >>>>>>>     4 files changed, 12 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> >>>>>>> index 054561f8dcae..78eb0959438a 100644
> >>>>>>> --- a/Documentation/networking/ip-sysctl.rst
> >>>>>>> +++ b/Documentation/networking/ip-sysctl.rst
> >>>>>>> @@ -1241,7 +1241,8 @@ tcp_rto_min_us - INTEGER
> >>>>>>>
> >>>>>>>     tcp_rto_max_ms - INTEGER
> >>>>>>>          Maximal TCP retransmission timeout (in ms).
> >>>>>>> -     Note that TCP_RTO_MAX_MS socket option has higher precedence.
> >>>>>>> +     Note that TCP_BPF_RTO_MAX and TCP_RTO_MAX_MS socket option have the
> >>>>>>> +     higher precedence for configuring this setting.
> >>>>>> The cover letter needs more explanation about the motivation.
> >>>>
> >>>> +1
> >>>>
> >>>> I haven't looked at the patches. The cover letter has no word on the use case.
> >>
> >> The question was your _use case_ in bpf. Not what the TCP_RTO_MAX_MS does. Your
> >> current use case is to have bpf setting it after reading the tcp header option,
> >> like the selftest in patch 3?
> >
> > Oops, I misunderstood the real situation of the tcp header option
> > test. My intention is to bpf_setsockopt() just like setget_sockopt
> > does.
> >
> > Thanks for reminding me. I will totally remove the header test in the
> > next version.
>
> If your use case was in the header, it is ok although it won't be the first

I was planning to add a simple test to only see if the rto max for bpf
feature works, so I found the rto min selftests and then did a similar
one.

> useful place I have in my mind. Regardless, it is useful to say a few words
> where you are planning to set it in the bpf. During a cb in sockops or during
> socket create ...etc. Without it, we can only guess from the selftest :(

I see your point. After evaluating and comparing those two tests, I
think the setsock_opt is a better place to go. Do we even apply the
use of rto min to setsock_opt as well?

What do you think?

>
> >
> >>
> >>>
> >>> I will add and copy some words from Eric's patch series :)
> >>
> >>
> >>>>> I am targeting the net-next tree because of recent changes[1] made by
> >>>>> Eric. It probably hasn't merged into the bpf-next tree.
> >>>>
> >>>> There is the bpf-next/net tree. It should have the needed changes.
> >>>
> >>> [1] was recently merged in the net-next tree, so the only one branch I
> >>> can target is net-next.
> >>>
> >>> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=ae9b3c0e79bc
> >>>
> >>> Am I missing something?
> >>
> >> There is a net branch:
>                ^^^
>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
> >
> > But this branch hasn't included the rto max feature. I was trying to
>
> Which branch? I was talking about the **net** branch. Not the master branch. Try
> to pull again if your local copy does not have it. The net branch should have
> the TCP_RTO_MAX_MS patches.

Oh, I always use the master branch, never heard of net branch. You're
right, I checked out the net branch and then found it. Thanks.

One more thing I have to ask in advance is that in this case what the
title looks like? [patch bpf] or [patch bpf net]?

Thanks,
Jason

>
> > say that what I wrote is based on the rto max feature which only
> > exists in the net-next tree for now.
> >
>
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux