Hello,
On Mon, 10 Jul 2023, Dust Li wrote:
> On Sun, Jul 09, 2023 at 08:45:19PM +0300, Julian Anastasov wrote:
> >
> > Hello,
> >
> >On Fri, 7 Jul 2023, Dust Li wrote:
> >
> >> @@ -2303,9 +2293,9 @@ static struct ip_vs_service *ip_vs_info_array(struct seq_file *seq, loff_t pos)
> >>
> >> /* look in hash by protocol */
> >> for (idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
> >> - hlist_for_each_entry_rcu(svc, &ip_vs_svc_table[idx], s_list) {
> >> + hlist_for_each_entry_rcu(svc, &ipvs->ip_vs_svc_table[idx], s_list) {
> >> if ((svc->ipvs == ipvs) && pos-- == 0) {
> >> - iter->table = ip_vs_svc_table;
> >> + iter->table = ipvs->ip_vs_svc_table;
> >
> > We can change iter->table to int table_id, 0 (ip_vs_svc_table)
> >and 1 (ip_vs_svc_fwm_table), for all these ip_vs_info_* funcs.
>
> Sorry, I don't get this. Why do we need to change this ?
It is a hint which table to walk, no need for such
dereferences. For example:
iter->table = ip_vs_svc_table;
becomes
iter->table_id = 0;
iter->table = ip_vs_svc_fwm_table;
becomes
iter->table_id = 1;
etc
> >> @@ -3392,9 +3384,9 @@ static int ip_vs_genl_dump_services(struct sk_buff *skb,
> >> struct net *net = sock_net(skb->sk);
> >> struct netns_ipvs *ipvs = net_ipvs(net);
> >>
> >> - mutex_lock(&__ip_vs_mutex);
> >> + mutex_lock(&ipvs->service_mutex);
> >
> > While do_ip_vs_get_ctl is a reader that can block we
> >can not use RCU but in ip_vs_genl_dump_services() we can replace
> >the __ip_vs_mutex with rcu_read_lock because we just fill the skb.
> >
>
> I think we can replace mutex in ip_vs_genl_dump_services() by RCU.
> Do you prefer replacing it in this patch or later ?
You can include these RCU conversions
> > Also, there is more work if we want independent
> >namespaces and to give power to users:
> >
> >- account memory: GFP_KERNEL -> GFP_KERNEL_ACCOUNT
> >
> >- the connections table is still global but it should be possible
> >to make all tables per-net and to grow on load from NULL to max bits
> >
> >- convert GENL_ADMIN_PERM -> GENL_UNS_ADMIN_PERM and make
> >sysctls visible to other namespaces
> >
> > If the plan is to have multiple netns loaded with many
> >conns may be I can follow your patch with more optimization
> >patches.
>
> Yes, we do missed those details. I think moving those into different
> netns is good, besides we already have netns supported. So why not do
> it more completely ?
>
> If it is convenient for you to do more optimizations, we would greatly
> appreciate it !
Yes, I'll prepare more patches on top of your patch.
Regards
--
Julian Anastasov <ja@xxxxxx>
