--- Begin Message ---
"Denis V. Lunev" <[EMAIL PROTECTED]> writes:
> Hello, Eric!
>
> Unfortunately, I was wrong that your patch is sane :(
>
> It breaks current RTNL socket semantic. Namely, current code relies that
> - netlink from user-space is queued to RTNL socket if RTNL lock is held
> - all pending messages in that queue will be processed in rtnl_unlock
I know we come very close to this but I have a hard time seeing
this being guaranteed. We don't hold a lock so I think it is
possible for a new message to come in via another path on SMP,
and we miss it in rtnl_unlock. Although missing that message
from both paths that grabs rtnl_lock sounds unlikely.
> - so, no message can leave in the RTNL socket queue for indefinite
> period of time
Hopefully anyway.
> Your patch breaks this :( Also, I am not quite sure yet (thinking) that
> this was not broken in the original code with net_lock().
Quite possibly.
I don't yet see where this matters, but I will have to look.
The actual code I wound up with for rtnl_unlock is:
> void rtnl_unlock(void)
> {
> struct net *net;
>
> for_each_net(net) {
> struct sock *rtnl = net->rtnl;
> if (!maybe_get_net(net))
> continue;
>
> mutex_unlock(&rtnl_mutex);
> if (rtnl && rtnl->sk_receive_queue.qlen)
> rtnl->sk_data_ready(rtnl, 0);
> mutex_lock(&rtnl_mutex);
> put_net(net);
> }
> mutex_unlock(&rtnl_mutex);
>
> netdev_run_todo();
> }
But depending on how much it matters this could be handled with
a retry condition. Which gives about as good a guarantee we are going
to process the data as you can get, but that currently feels like
overkill.
void rtnl_unlock(void)
{
struct net *net;
retry:
for_each_net(net) {
struct sock *rtnl = net->rtnl;
if (!rtnl || !rtnl->sk_receive_queue.qlen)
continue;
if (maybe_get_net(net)) {
mutex_unlock(&rtnl_mutex);
rtnl->sk_data_ready(rtnl, 0);
mutex_lock(&rtnl_mutex);
put_net(net);
goto retry;
}
}
mutex_unlock(&rtnl_mutex);
netdev_run_todo();
}
Hmm. I'm half tempted to suggest just doing:
void rtnl_unlock(void)
{
struct net *net;
for_each_net(net) {
struct sock *rtnl = net->rtnl;
unsigned int qlen = 0;
do {
netlink_run_queue(rtnl, qlen, rtnetlink_rcv_msg);
} while(qlen);
}
mutex_unlock(&rtnl_mutex);
netdev_run_todo();
}
But that has issues with not calling netdev_run_todo enough.
Anyway, I need to get back to bed.
Eric
--- End Message ---
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
