Re: [tipc-discussion] [net] tipc: fix incorrect setting window for bcast link

[Jon M. <jm...@re...>]

Hi Hoang,
I apologize for not paying enough attention to this problem until now.
See below.


On 9/30/20 9:43 PM, Hoang Huu Le wrote:
> In commit 16ad3f4022bb
> ("tipc: introduce variable window congestion control"), we applied
> the Reno algorithm to select window size from minimum window to the
> configured maximum window for unicast link.
>
> However, when setting window variable we do not specific to unicast link.
> This lead to the window for broadcast link had effect as unexpect value
> (i.e the window size is constantly 32).
>
> We fix this by updating the window for broadcast as its configuration.
>
> Signed-off-by: Hoang Huu Le <hoa...@de...>
> ---
>   net/tipc/bcast.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> index 940d176e0e87..abac9443b4d9 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -585,7 +585,7 @@ static int tipc_bc_link_set_queue_limits(struct net *net, u32 max_win)
>   	if (max_win > TIPC_MAX_LINK_WIN)
>   		return -EINVAL;
>   	tipc_bcast_lock(net);
> -	tipc_link_set_queue_limits(l, BCLINK_WIN_MIN, max_win);
> +	tipc_link_set_queue_limits(l, max_win, max_win);
I think this is dangerous. The broadcast link puts a much higher stress 
on the switch, and the risk of massive packet loss with ditto 
retransmissions is very high.
A safer bet to stay with a fix window of 50 for now, to solve the 
problem you have at sites, and then you can possibly experiment with a 
variable window later.
If it gives significant higher throughput it might be worthwhile trying, 
but I am pretty sure that e.g. the base for calculating ssthresh (300) 
is too big.

///jon
>   	tipc_bcast_unlock(net);
>   	return 0;
>   }