Hi Jon, See my inline comment.
Regards, Hoang > -----Original Message----- > From: Jon Maloy <[email protected]> > Sent: Friday, October 2, 2020 4:40 AM > To: Hoang Huu Le <[email protected]>; > [email protected]; [email protected]; > [email protected]; Xin Long <[email protected]> > Subject: Re: [net] tipc: fix incorrect setting window for bcast link > > > > 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). > This was intentional, although I thought the value was 50, not 32. > In my experience we cannot afford a generous variable window > in the broadcast link the same way we do with the unicast link. > There will be too many losses and retransmissions because of the > way switches work. [Hoang] When it is created, the value is 50. However, if we use the tipc command: i.e $ tipc link set win 50 link broadcast The window is set to 32 - this value is constant since then. Is it counting as bug? > > > > > We fix this by updating the window for broadcast as its configuration. > > > > Signed-off-by: Hoang Huu Le <[email protected]> > > --- > > 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); > > tipc_bcast_unlock(net); > > return 0; > > } > What is the effect of this change? Do we still have a fix window? [Hoang] No, now window can be configured as user intention. If you'd like to use the fix window 50. I will update the code change. > What happens when we have buffer overflow? The broadcast > send link can never be reset I rember correctly. > Did you test this with high load, e.g. using the multicast_blast test > program? [Hoang] I tried to publish hundreds or services that uses SYSTEM_IMPORTANCE - it was not being reset when the link is overflow. For others imp, the overflow issue will not happen because of oversubscription allowing. I will give a try with the test. > > Regards > ///jon _______________________________________________ tipc-discussion mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tipc-discussion
