[PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Yixin Shen]

This commit export the symbol of the function minmax_running_min
to make it accessible to dynamically loaded modules. It can make
this library more general, especially for those congestion
control algorithm modules who wants to implement a windowed min
filter.

Signed-off-by: Yixin Shen <bobankhshen@gmail.com>
---
 lib/win_minmax.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/win_minmax.c b/lib/win_minmax.c
index ec10506834b6..1682e614309c 100644
--- a/lib/win_minmax.c
+++ b/lib/win_minmax.c
@@ -97,3 +97,4 @@ u32 minmax_running_min(struct minmax *m, u32 win, u32 t, u32 meas)
 
 	return minmax_subwin_update(m, win, &val);
 }
+EXPORT_SYMBOL(minmax_running_min);
-- 
2.25.1

Re: [PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Leon Romanovsky]

On Thu, Apr 13, 2023 at 04:47:26PM +0000, Yixin Shen wrote:
> This commit export the symbol of the function minmax_running_min
> to make it accessible to dynamically loaded modules. It can make
> this library more general, especially for those congestion
> control algorithm modules who wants to implement a windowed min
> filter.
> 
> Signed-off-by: Yixin Shen <bobankhshen@gmail.com>
> ---
>  lib/win_minmax.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/win_minmax.c b/lib/win_minmax.c
> index ec10506834b6..1682e614309c 100644
> --- a/lib/win_minmax.c
> +++ b/lib/win_minmax.c
> @@ -97,3 +97,4 @@ u32 minmax_running_min(struct minmax *m, u32 win, u32 t, u32 meas)
>  
>  	return minmax_subwin_update(m, win, &val);
>  }
> +EXPORT_SYMBOL(minmax_running_min);

Please provide in-tree kernel user for that EXPORT_SYMBOL.

Thanks

> -- 
> 2.25.1
> 

Re: [PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Yixin Shen]

> Please provide in-tree kernel user for that EXPORT_SYMBOL.

It is hard to provide such an in-tree kernel user. We are trying to
implement newer congestion control algorithms as dynamically loaded modules.
For example, Copa(NSDI'18) which is adopted by Facebook needs to maintain
such windowed min filters. Althought it is true that we can just
copy-and-paste the code inside lib/win_minmax, it it more convenient to
give the same status of minmax_running_min as minmax_running_max.
It is confusing that only minmax_running_max is exported.
If this patch is rejected because the changes are too significant,
I can also understand.

Thanks.

Re: [PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Leon Romanovsky]

On Fri, Apr 14, 2023 at 02:27:36AM +0000, Yixin Shen wrote:
> > Please provide in-tree kernel user for that EXPORT_SYMBOL.
> 
> It is hard to provide such an in-tree kernel user. 

So once you overcome it, feel free to send this EXPORT_SYMBOL patch
together with in-tree kernel user, but for now it is against kernel
policy.

Thanks

Re: [PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Eric Dumazet]

On Fri, Apr 14, 2023 at 4:27 AM Yixin Shen <bobankhshen@gmail.com> wrote:
>
> > Please provide in-tree kernel user for that EXPORT_SYMBOL.
>
> It is hard to provide such an in-tree kernel user. We are trying to
> implement newer congestion control algorithms as dynamically loaded modules.
> For example, Copa(NSDI'18) which is adopted by Facebook needs to maintain
> such windowed min filters. Althought it is true that we can just
> copy-and-paste the code inside lib/win_minmax, it it more convenient to
> give the same status of minmax_running_min as minmax_running_max.
> It is confusing that only minmax_running_max is exported.

This is needed by net/ipv4/tcp_bbr.c , which can be a module.

> If this patch is rejected because the changes are too significant,

Well, this path would soon be reverted by people using bots/tools to
detect unused functions,
or unused EXPORT symbols.

So there is no point accepting it, before you submit the CC in the
official linux tree.

Re: [PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Andrew Morton]

On Fri, 14 Apr 2023 16:17:40 +0200 Eric Dumazet <edumazet@google.com> wrote:

> On Fri, Apr 14, 2023 at 4:27 AM Yixin Shen <bobankhshen@gmail.com> wrote:
> >
> > > Please provide in-tree kernel user for that EXPORT_SYMBOL.
> >
> > It is hard to provide such an in-tree kernel user. We are trying to
> > implement newer congestion control algorithms as dynamically loaded modules.
> > For example, Copa(NSDI'18) which is adopted by Facebook needs to maintain
> > such windowed min filters. Althought it is true that we can just
> > copy-and-paste the code inside lib/win_minmax, it it more convenient to
> > give the same status of minmax_running_min as minmax_running_max.
> > It is confusing that only minmax_running_max is exported.
> 
> This is needed by net/ipv4/tcp_bbr.c , which can be a module.
> 
> > If this patch is rejected because the changes are too significant,
> 
> Well, this path would soon be reverted by people using bots/tools to
> detect unused functions,
> or unused EXPORT symbols.
> 
> So there is no point accepting it, before you submit the CC in the
> official linux tree.

It seems pretty darn screwy that we export minmax_running_max() but not
minmax_running_min().

I'd be OK taking the patch just so we aren't pretty darn screwy.  But
it would be perfectly OK to include that one-liner within the patchset
which adds a minmax_running_min() user.

Re: [PATCH net-next] lib/win_minmax: export symbol of minmax_running_min

[Jakub Kicinski]

On Fri, 14 Apr 2023 02:27:36 +0000 Yixin Shen wrote:
> For example, Copa(NSDI'18) which is adopted by Facebook needs to maintain
> such windowed min filters.

Perhaps an unnecessary comment, but this should not be misread
as this patch itself having anything to do with Meta.