Hello, Julian Thank you for your a lot of comments. On Sun, Nov 19, 2017 at 06:04:35PM +0200, Julian Anastasov wrote: > > Hello, > > On Sun, 19 Nov 2017, Inju Song wrote: > > > Implements the Google's Maglev hashing algorithm as a IPVS scheduler. > > > > Basically it provides consistent hashing but offers some special > > features about disruption and load balancing. > > > > 1) minimal disruption: when the set of destinations changes, > > a connection will likely be sent to the same destination > > as it was before. > > > > 2) load balancing: each destination will receive an almost > > equal number of connections. > > > > Seel also for detail: [3.4 Consistent Hasing] in > > https://static.googleusercontent.com/media/research.google.com/ko//pubs/archive/44824.pdf > > I see that the PDF shows implementation only for servers > with same weight. It would be better to see implementation that > considers dest->weight. > I agree. In this case, weight value is same weight or zero. So I will consider this case when I patch the source. I have read your comments about memory allocation fail, caching table, setting the entry table with NULLs and so on. I will do fix it and send v2 patchsets. > > Signed-off-by: Inju Song <inju.song@xxxxxxxxxxxxx> > > --- > > net/netfilter/ipvs/ip_vs_mh.c | 402 ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 402 insertions(+) > > create mode 100644 net/netfilter/ipvs/ip_vs_mh.c > > > > diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c > > new file mode 100644 > > index 0000000..534a9f5 > > --- /dev/null > > +++ b/net/netfilter/ipvs/ip_vs_mh.c > > > +static inline int > > +ip_vs_mh_populate(struct ip_vs_mh_state *s, struct ip_vs_service *svc, > > + unsigned int **permutation) > > +{ > > + int i; > > + unsigned int *next; > > + struct ip_vs_mh_lookup *entry, *l; > > + struct list_head *p; > > + struct ip_vs_dest *dest; > > + int dcnt; > > + unsigned int n, c; > > + > > + dcnt = svc->num_dests; > > While calling del_dest svc->num_dests can be 0, this should > be considered in ip_vs_mh_permutate and ip_vs_mh_populate. We should > set the table with NULLs in such case. > > > + next = kcalloc(dcnt, sizeof(unsigned int), GFP_KERNEL); > > + entry = kcalloc(IP_VS_MH_LOOKUP_SIZE, sizeof(*entry), > > + GFP_KERNEL); > > It is current drawback that add_dest, upd_dest and del_dest > are considered as non-failing methods, we can not fail the operation > if memory allocation fails, for example. This is bad for add_dest, > not so for upd_dest and del_dest. In this regard, I think, the MH > scheduler should not crash if allocation fails. > > > +/* Assign all the hash buckets of the specified table with the service. > > + */ > > +static int > > +ip_vs_mh_reassign(struct ip_vs_mh_state *s, struct ip_vs_service *svc) > > +{ > > + int dcnt; > > + unsigned int **permutation; > > + > > + /* flush all the hash entry before assigning mh entry */ > > + ip_vs_mh_flush(s); > > ip_vs_mh_reassign should not call ip_vs_mh_flush. > ip_vs_mh_populate while copying the temp table to s->lookup[] > should put the refcnt to old dests (as in SH:ip_vs_sh_reassign) > and then to assign new value. NULL should be assigned if > svc->num_dests = 0. > > > +/* IPVS MH Scheduler structure */ > > +static struct ip_vs_scheduler ip_vs_mh_scheduler = { > > + .name = "mh", > > + .refcnt = ATOMIC_INIT(0), > > + .module = THIS_MODULE, > > + .n_list = LIST_HEAD_INIT(ip_vs_mh_scheduler.n_list), > > + .init_service = ip_vs_mh_init_svc, > > + .done_service = ip_vs_mh_done_svc, > > + .add_dest = ip_vs_mh_dest_changed, > > + .del_dest = ip_vs_mh_dest_changed, > > + .upd_dest = ip_vs_mh_dest_changed, > > As upd_dest is called only when weight changes, the SH and MH > schedulers should not handle this method. Their goal is to fill > the table with all added dests by ignoring health state (it is > considered only on dest selection). > > I guess, the correct configuration for SH and MH would be: > > 1.1 Add all dests with specific weight, so that the schedulers can > fill their table based on the weights. > 1.2 Set weight to 0 for all servers that are not healthy. SH and MH > should ignore these configuration events. > > after the initial configuration: > > 2. Traffic will ignore non-healthy servers, fallbacks to healthy ones. > 3. If other servers become unavailable, change their weight to 0 > > Regards > > -- > Julian Anastasov <ja@xxxxxx> Regards -- Inju Song NAVER Corporation -- To unsubscribe from this list: send the line "unsubscribe lvs-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
